Don't fail channel if inbound UA breaches counterparty-selected reserve
What the developer wrote
Authored by Leo Nash
Don't fail channel if inbound UA breaches counterparty-selected reserve
We do not care if our balance drops below the counterparty-selected
reserve upon an inbound `update_add_htlc`. This is the counterparty's
problem.
Hence, we drop the assumption that once our balance rises above the
counterparty-selected reserve, it will always remain above this reserve
for the lifetime of a funding scope.
In the following commit, we make the assumption that the counterparty
does not complain if we push them below our selected reserve when adding
a HTLC, so we accommodate this assumption here.
What changed, and why it matters
This commit changes how the Lightning node handles incoming payment requests (HTLCs) that would push the node's own balance below the 'channel reserve' amount chosen by the other party. Previously, the node would reject such HTLCs and close the channel, treating it as a violation. The new behavior accepts these HTLCs, because the developers consider it the counterparty's problem, not theirs. A related safety assertion is now only checked in tests, not in production code. This is a protocol-behavior change rather than a traditional memory-safety or cryptography bug, but it removes a defensive check that previously prevented the local balance from dropping below a reserve threshold on inbound HTLCs.
Review whether removing this reserve check is safe under all channel configurations, especially for zero-reserve or low-reserve channels. Confirm that the counterparty cannot exploit the relaxed check to force the local node into a state where it cannot afford its own commitment transaction fees or where fee pinning becomes easier. Re-enable or replace the removed test with coverage that reflects the new intended behavior, and monitor for any follow-up commits that complete the rationale described in the commit message.
Security signals we found
Removal of channel-reserve enforcement on inbound HTLC acceptance
Production-only disabling of a balance-reserve invariant assertion (`#[cfg(test)]`)
Deletion of a unit test that asserted channel closure for reserve-violating inbound HTLCs
Behavioral change in Lightning channel reserve policy, which is a protocol-level economic safeguard
Evidence from the diff
In lightning/src/ln/channel.rs, the validation for inbound update_add_htlc no longer checks whether the resulting local holder balance falls below counterparty_selected_channel_reserve_satoshis. The removed branch returned ChannelError::close(...) with message ‘Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value’. A related invariant assertion in the commitment-building path is now gated with #[cfg(test)], meaning it runs only in test builds. The commit also removes the unit test test_chan_reserve_violation_inbound_htlc_outbound_channel, which previously verified that such an HTLC caused channel closure. The stated rationale is that the counterparty selected the reserve, so an inbound HTLC breaching it is the counterparty’s concern, and an upcoming commit will assume the counterparty does not complain when pushed below the local reserve.
Changed components
lightning/src/ln/channel.rslightning/src/ln/htlc_reserve_unit_tests.rsInbound HTLC validation logicChannel reserve enforcementInspect captured patch +9 / −89
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index a0b3bb1..67ada5a 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -5162,7 +5162,10 @@ impl<SP: SignerProvider> ChannelContext<SP> {
));
}
- let (local_stats, _local_htlcs) = self
+ // Here we check two things 1) that our local commitment still has at least 1 output
+ // (particularly relevant in 0-reserve channels), and 2) that the counterparty can
+ // still afford the fee on our commitment if they are the funder.
+ let (_local_stats, _local_htlcs) = self
.get_next_local_commitment_stats(
funding,
Some(HTLCAmountDirection { outbound: false, amount_msat: msg.amount_msat }),
@@ -5175,16 +5178,6 @@ impl<SP: SignerProvider> ChannelContext<SP> {
ChannelError::close(String::from("Balance exhausted on local commitment"))
})?;
- // Check that they won't violate our local required channel reserve by adding this HTLC.
- if funding.is_outbound()
- && local_stats.commitment_stats.holder_balance_msat
- < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000
- {
- return Err(ChannelError::close(
- "Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned()
- ));
- }
-
Ok(())
}
@@ -5717,6 +5710,11 @@ impl<SP: SignerProvider> ChannelContext<SP> {
funding.counterparty_prev_commitment_tx_balance.lock().unwrap()
};
+ // This assumes that once our balance rises above the counterparty selected
+ // reserve, it never drops below again. But we allow our counterparty to
+ // push us under our reserve when we are the funder and they add a HTLC, as
+ // this is really their problem. Hence, we only run this assert in tests.
+ #[cfg(test)]
if _stats.local_balance_before_fee_msat / 1000 < funding.counterparty_selected_channel_reserve_satoshis.unwrap() {
// If the local balance is below the reserve on this new commitment, it MUST be
// greater than or equal to the one on the previous commitment.
diff --git a/lightning/src/ln/htlc_reserve_unit_tests.rs b/lightning/src/ln/htlc_reserve_unit_tests.rs
index 3c91808..608ac14 100644
--- a/lightning/src/ln/htlc_reserve_unit_tests.rs
+++ b/lightning/src/ln/htlc_reserve_unit_tests.rs
@@ -1023,84 +1023,6 @@ pub fn test_chan_reserve_violation_outbound_htlc_inbound_chan() {
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
}
-#[xtest(feature = "_externalize_tests")]
-pub fn test_chan_reserve_violation_inbound_htlc_outbound_channel() {
- let mut chanmon_cfgs = create_chanmon_cfgs(2);
- let feerate_per_kw = *chanmon_cfgs[0].fee_estimator.sat_per_kw.lock().unwrap();
- let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
- let legacy_cfg = test_legacy_channel_config();
- let node_chanmgrs =
- create_node_chanmgrs(2, &node_cfgs, &[Some(legacy_cfg.clone()), Some(legacy_cfg)]);
- let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-
- let node_b_id = nodes[1].node.get_our_node_id();
-
- let default_config = UserConfig::default();
- let channel_type_features = ChannelTypeFeatures::only_static_remote_key();
-
- // Set nodes[0]'s balance such that they will consider any above-dust received HTLC to be a
- // channel reserve violation (so their balance is channel reserve (1000 sats) + commitment
- // transaction fee with 0 HTLCs (183 sats)).
- let mut push_amt = 100_000_000;
- push_amt -= commit_tx_fee_msat(
- feerate_per_kw,
- MIN_AFFORDABLE_HTLC_COUNT as u64,
- &channel_type_features,
- );
- push_amt -=
- get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) * 1000;
- let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, push_amt);
-
- // Send four HTLCs to cover the initial push_msat buffer we're required to include
- for _ in 0..MIN_AFFORDABLE_HTLC_COUNT {
- route_payment(&nodes[1], &[&nodes[0]], 1_000_000);
- }
-
- let (mut route, payment_hash, _, payment_secret) =
- get_route_and_payment_hash!(nodes[1], nodes[0], 1000);
- route.paths[0].hops[0].fee_msat = 700_000;
- // Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc()
- let secp_ctx = Secp256k1::new();
- let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
- let cur_height = nodes[1].node.best_block.read().unwrap().height + 1;
- let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv);
- let recipient_onion_fields = RecipientOnionFields::secret_only(payment_secret, 700_000);
- let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::test_build_onion_payloads(
- &route.paths[0],
- &recipient_onion_fields,
- cur_height,
- &None,
- None,
- None,
- )
- .unwrap();
- let onion_packet =
- onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash)
- .unwrap();
- let msg = msgs::UpdateAddHTLC {
- channel_id: chan.2,
- htlc_id: MIN_AFFORDABLE_HTLC_COUNT as u64,
- amount_msat: htlc_msat,
- payment_hash,
- cltv_expiry: htlc_cltv,
- onion_routing_packet: onion_packet,
- skimmed_fee_msat: None,
- blinding_point: None,
- hold_htlc: None,
- accountable: None,
- };
-
- nodes[0].node.handle_update_add_htlc(node_b_id, &msg);
- // Check that the payment failed and the channel is closed in response to the malicious UpdateAdd.
- nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value", 3);
- assert_eq!(nodes[0].node.list_channels().len(), 0);
- let err_msg = check_closed_broadcast(&nodes[0], 1, true).pop().unwrap();
- assert_eq!(err_msg.data, "Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value");
- let reason = ClosureReason::ProcessingError { err: "Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_string() };
- check_added_monitors(&nodes[0], 1);
- check_closed_event(&nodes[0], 1, reason, &[node_b_id], 100000);
-}
-
#[xtest(feature = "_externalize_tests")]
pub fn test_chan_reserve_dust_inbound_htlcs_outbound_chan() {
// Test that if we receive many dust HTLCs over an outbound channel, they don't count when
Why this scored 47/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.