From 52a90577f2b8eb67d6abe26242c63041212e8115 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 16 May 2023 03:26:21 +0000 Subject: [PATCH] Consider counterparty commitment tx fees when assembling a route When calculating the amount available to send for the next HTLC, if we over-count we may create routes which are not actually usable. Historically this has been an issue, which we resolve over a few commits. Here we consider whether one additional HTLC's commitment tx fees would result in the counterparty's commitment tx fees being greater than the reserve we've picked for them and, if so, limit our next HTLC value to only include dust HTLCs. We also add some testing when sending to ensure that send failures are accounted for in our balance calculations. This, and the previous few commits, fixes #1126. --- lightning/src/ln/channel.rs | 22 ++++++++++++++++++++++ lightning/src/ln/functional_tests.rs | 11 ++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e0be0af5..23e56f94 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2676,6 +2676,7 @@ impl Channel { /// corner case properly. pub fn get_available_balances(&self) -> AvailableBalances { // Note that we have to handle overflow due to the above case. + let inbound_stats = self.get_inbound_pending_htlc_stats(None); let outbound_stats = self.get_outbound_pending_htlc_stats(None); let mut balance_msat = self.value_to_self_msat; @@ -2724,6 +2725,26 @@ impl Channel { } else { available_capacity_msat = capacity_minus_commitment_fee_msat as u64; } + } else { + // If the channel is inbound (i.e. counterparty pays the fee), we need to make sure + // sending a new HTLC won't reduce their balance below our reserve threshold. + let mut real_dust_limit_success_sat = self.counterparty_dust_limit_satoshis; + if !self.opt_anchors() { + real_dust_limit_success_sat += self.feerate_per_kw as u64 * htlc_success_tx_weight(false) / 1000; + } + + let htlc_above_dust = HTLCCandidate::new(real_dust_limit_success_sat * 1000, HTLCInitiator::LocalOffered); + let max_reserved_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(htlc_above_dust, None); + + let holder_selected_chan_reserve_msat = self.holder_selected_channel_reserve_satoshis * 1000; + let remote_balance_msat = (self.channel_value_satoshis * 1000 - self.value_to_self_msat) + .saturating_sub(inbound_stats.pending_htlcs_value_msat); + + if remote_balance_msat < max_reserved_commit_tx_fee_msat + holder_selected_chan_reserve_msat { + // If another HTLC's fee would reduce the remote's balance below the reserve limit + // we've selected for them, we can only send dust HTLCs. + available_capacity_msat = cmp::min(available_capacity_msat, real_dust_limit_success_sat * 1000 - 1); + } } available_capacity_msat = cmp::min(available_capacity_msat, @@ -5906,6 +5927,7 @@ impl Channel { let holder_selected_chan_reserve_msat = self.holder_selected_channel_reserve_satoshis * 1000; let remote_balance_msat = (self.channel_value_satoshis * 1000 - self.value_to_self_msat).saturating_sub(inbound_stats.pending_htlcs_value_msat); if remote_balance_msat < counterparty_commit_tx_fee_msat + holder_selected_chan_reserve_msat { + debug_assert!(amount_msat > self.get_available_balances().next_outbound_htlc_limit_msat); return Err(ChannelError::Ignore("Cannot send value that would put counterparty balance under holder-announced channel reserve value".to_owned())); } } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index dfb101b1..3f400d9b 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -1527,13 +1527,14 @@ fn test_chan_reserve_violation_outbound_htlc_inbound_chan() { let _ = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, push_amt); + // Fetch a route in advance as we will be unable to once we're unable to send. + let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 1_000_000); // Sending exactly enough to hit the reserve amount should be accepted for _ in 0..MIN_AFFORDABLE_HTLC_COUNT { let (_, _, _) = route_payment(&nodes[1], &[&nodes[0]], 1_000_000); } // However one more HTLC should be significantly over the reserve amount and fail. - let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 1_000_000); unwrap_send_err!(nodes[1].node.send_payment_with_route(&route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), true, APIError::ChannelUnavailable { ref err }, @@ -1565,7 +1566,9 @@ fn test_chan_reserve_violation_inbound_htlc_outbound_channel() { let (_, _, _) = route_payment(&nodes[1], &[&nodes[0]], 1_000_000); } - let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 700_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(); @@ -1627,7 +1630,9 @@ fn test_chan_reserve_dust_inbound_htlcs_outbound_chan() { } // One more than the dust amt should fail, however. - let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], dust_amt + 1); + let (mut route, our_payment_hash, _, our_payment_secret) = + get_route_and_payment_hash!(nodes[1], nodes[0], dust_amt); + route.paths[0].hops[0].fee_msat += 1; unwrap_send_err!(nodes[1].node.send_payment_with_route(&route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), true, APIError::ChannelUnavailable { ref err }, -- 2.30.2