Consider counterparty commitment tx fees when assembling a route
authorMatt Corallo <git@bluematt.me>
Tue, 16 May 2023 03:26:21 +0000 (03:26 +0000)
committerMatt Corallo <git@bluematt.me>
Tue, 6 Jun 2023 23:57:55 +0000 (23:57 +0000)
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
lightning/src/ln/functional_tests.rs

index e0be0af5d72af39451fedb715b1b987e79eb192d..23e56f94bd78d4e7b2418aea9a9fe6c834f38e18 100644 (file)
@@ -2676,6 +2676,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
        /// 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<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                        } 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<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                        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()));
                        }
                }
index dfb101b1f907c85b82fddf68b86632e3b08d8e02..3f400d9b8224306e2de5b9b8cd9c33303c7af33a 100644 (file)
@@ -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 },