Consider commitment tx fee while assembling a route
authorGleb Naumenko <naumenko.gs@gmail.com>
Tue, 7 Mar 2023 08:01:05 +0000 (10:01 +0200)
committerMatt Corallo <git@bluematt.me>
Sun, 21 May 2023 19:05:22 +0000 (19:05 +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 include the cost of the commitment transaction fee in our
calculation, subtracting the commitment tx fee cost from the
available as we do in `send_payment`.

We also add some testing when sending to ensure that send failures
are accounted for in our balance calculations.

This commit is based on original work by
Gleb Naumenko <naumenko.gs@gmail.com> and modified by
Matt Corallo <git@bluematt.me>.

lightning/src/ln/channel.rs
lightning/src/ln/functional_tests.rs

index 829ce3a997663ea5886144f878b0b027eaff2c0b..bda0bfa509ca61a6b7d691e317726d091e72edbb 100644 (file)
@@ -2686,10 +2686,45 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                }
                balance_msat -= outbound_stats.pending_htlcs_value_msat;
 
-               let outbound_capacity_msat = cmp::max(self.value_to_self_msat as i64
-                               - outbound_stats.pending_htlcs_value_msat as i64
-                               - self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000,
-                       0) as u64;
+               let outbound_capacity_msat = self.value_to_self_msat
+                               .saturating_sub(outbound_stats.pending_htlcs_value_msat)
+                               .saturating_sub(
+                                       self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) * 1000);
+
+               let mut available_capacity_msat = outbound_capacity_msat;
+
+               if self.is_outbound() {
+                       // We should mind channel commit tx fee when computing how much of the available capacity
+                       // can be used in the next htlc. Mirrors the logic in send_htlc.
+                       //
+                       // The fee depends on whether the amount we will be sending is above dust or not,
+                       // and the answer will in turn change the amount itself — making it a circular
+                       // dependency.
+                       // This complicates the computation around dust-values, up to the one-htlc-value.
+                       let mut real_dust_limit_timeout_sat = self.holder_dust_limit_satoshis;
+                       if !self.opt_anchors() {
+                               real_dust_limit_timeout_sat += self.feerate_per_kw as u64 * htlc_timeout_tx_weight(false) / 1000;
+                       }
+
+                       let htlc_above_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000, HTLCInitiator::LocalOffered);
+                       let max_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * self.next_local_commit_tx_fee_msat(htlc_above_dust, Some(()));
+                       let htlc_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000 - 1, HTLCInitiator::LocalOffered);
+                       let min_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * self.next_local_commit_tx_fee_msat(htlc_dust, Some(()));
+
+                       // We will first subtract the fee as if we were above-dust. Then, if the resulting
+                       // value ends up being below dust, we have this fee available again. In that case,
+                       // match the value to right-below-dust.
+                       let mut capacity_minus_commitment_fee_msat: i64 = (available_capacity_msat as i64) - (max_reserved_commit_tx_fee_msat as i64);
+                       if capacity_minus_commitment_fee_msat < (real_dust_limit_timeout_sat as i64) * 1000 {
+                               let one_htlc_difference_msat = max_reserved_commit_tx_fee_msat - min_reserved_commit_tx_fee_msat;
+                               debug_assert!(one_htlc_difference_msat != 0);
+                               capacity_minus_commitment_fee_msat += one_htlc_difference_msat as i64;
+                               capacity_minus_commitment_fee_msat = cmp::min(real_dust_limit_timeout_sat as i64 * 1000 - 1, capacity_minus_commitment_fee_msat);
+                               available_capacity_msat = cmp::max(0, cmp::min(capacity_minus_commitment_fee_msat, available_capacity_msat as i64)) as u64;
+                       } else {
+                               available_capacity_msat = capacity_minus_commitment_fee_msat as u64;
+                       }
+               }
                AvailableBalances {
                        inbound_capacity_msat: cmp::max(self.channel_value_satoshis as i64 * 1000
                                        - self.value_to_self_msat as i64
@@ -2697,7 +2732,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                                        - self.holder_selected_channel_reserve_satoshis as i64 * 1000,
                                0) as u64,
                        outbound_capacity_msat,
-                       next_outbound_htlc_limit_msat: cmp::max(cmp::min(outbound_capacity_msat as i64,
+                       next_outbound_htlc_limit_msat: cmp::max(cmp::min(available_capacity_msat as i64,
                                        self.counterparty_max_htlc_value_in_flight_msat as i64
                                                - outbound_stats.pending_htlcs_value_msat as i64),
                                0) as u64,
@@ -5896,6 +5931,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                let holder_balance_msat = self.value_to_self_msat
                        .saturating_sub(outbound_stats.pending_htlcs_value_msat);
                if holder_balance_msat < amount_msat {
+                       debug_assert!(amount_msat > self.get_available_balances().next_outbound_htlc_limit_msat);
                        return Err(ChannelError::Ignore(format!("Cannot send value that would overdraw remaining funds. Amount: {}, pending value to self {}", amount_msat, holder_balance_msat)));
                }
 
@@ -5905,6 +5941,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                        FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * self.next_local_commit_tx_fee_msat(htlc_candidate, Some(()))
                } else { 0 };
                if holder_balance_msat - amount_msat < commit_tx_fee_msat {
+                       debug_assert!(amount_msat > self.get_available_balances().next_outbound_htlc_limit_msat);
                        return Err(ChannelError::Ignore(format!("Cannot send value that would not leave enough to pay for fees. Pending value to self: {}. local_commit_tx_fee {}", holder_balance_msat, commit_tx_fee_msat)));
                }
 
@@ -5912,6 +5949,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                // reserve for the remote to have something to claim if we misbehave)
                let chan_reserve_msat = self.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000;
                if holder_balance_msat - amount_msat - commit_tx_fee_msat < chan_reserve_msat {
+                       debug_assert!(amount_msat > self.get_available_balances().next_outbound_htlc_limit_msat);
                        return Err(ChannelError::Ignore(format!("Cannot send value that would put our balance under counterparty-announced channel reserve value ({})", chan_reserve_msat)));
                }
 
index 92a6e2695a0b071048ed70d723ed624e2feda643..81c42861a8d8df22b1881b81552f92d0e43a2591 100644 (file)
@@ -1342,7 +1342,9 @@ fn test_basic_channel_reserve() {
        // The 2* and +1 are for the fee spike reserve.
        let commit_tx_fee = 2 * commit_tx_fee_msat(get_feerate!(nodes[0], nodes[1], chan.2), 1 + 1, get_opt_anchors!(nodes[0], nodes[1], chan.2));
        let max_can_send = 5000000 - channel_reserve - commit_tx_fee;
-       let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send + 1);
+       let (mut route, our_payment_hash, _, our_payment_secret) =
+               get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send);
+       route.paths[0].hops.last_mut().unwrap().fee_msat += 1;
        let err = nodes[0].node.send_payment_with_route(&route, our_payment_hash,
                RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0)).err().unwrap();
        match err {
@@ -1369,7 +1371,9 @@ fn test_fee_spike_violation_fails_htlc() {
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
        let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000);
 
-       let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 3460001);
+       let (mut route, payment_hash, _, payment_secret) =
+               get_route_and_payment_hash!(nodes[0], nodes[1], 3460000);
+       route.paths[0].hops[0].fee_msat += 1;
        // 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]).expect("RNG is bad!");
@@ -1732,7 +1736,8 @@ fn test_chan_reserve_violation_inbound_htlc_inbound_chan() {
        let commit_tx_fee_2_htlcs = commit_tx_fee_msat(feerate, 2, opt_anchors);
        let recv_value_2 = chan_stat.value_to_self_msat - amt_msat_1 - chan_stat.channel_reserve_msat - total_routing_fee_msat - commit_tx_fee_2_htlcs + 1;
        let amt_msat_2 = recv_value_2 + total_routing_fee_msat;
-       let (route_2, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[2], amt_msat_2);
+       let mut route_2 = route_1.clone();
+       route_2.paths[0].hops.last_mut().unwrap().fee_msat = amt_msat_2;
 
        // Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc()
        let secp_ctx = Secp256k1::new();
@@ -1901,7 +1906,9 @@ fn test_channel_reserve_holding_cell_htlcs() {
        // channel reserve test with htlc pending output > 0
        let recv_value_2 = stat01.value_to_self_msat - amt_msat_1 - stat01.channel_reserve_msat - total_fee_msat - commit_tx_fee_2_htlcs;
        {
-               let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], recv_value_2 + 1);
+               let mut route = route_1.clone();
+               route.paths[0].hops.last_mut().unwrap().fee_msat = recv_value_2 + 1;
+               let (_, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(nodes[2]);
                unwrap_send_err!(nodes[0].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 },
@@ -1930,7 +1937,9 @@ fn test_channel_reserve_holding_cell_htlcs() {
 
        // test with outbound holding cell amount > 0
        {
-               let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], recv_value_22+1);
+               let (mut route, our_payment_hash, _, our_payment_secret) =
+                       get_route_and_payment_hash!(nodes[0], nodes[2], recv_value_22);
+               route.paths[0].hops.last_mut().unwrap().fee_msat += 1;
                unwrap_send_err!(nodes[0].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 },
@@ -6241,12 +6250,15 @@ fn test_update_add_htlc_bolt2_receiver_check_max_htlc_limit() {
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
        let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000);
 
-       let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 3999999);
+       let send_amt = 3999999;
+       let (mut route, our_payment_hash, _, our_payment_secret) =
+               get_route_and_payment_hash!(nodes[0], nodes[1], 1000);
+       route.paths[0].hops[0].fee_msat = send_amt;
        let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
        let cur_height = nodes[0].node.best_block.read().unwrap().height() + 1;
        let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::signing_only(), &route.paths[0], &session_priv).unwrap();
        let (onion_payloads, _htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(
-               &route.paths[0], 3999999, RecipientOnionFields::secret_only(our_payment_secret), cur_height, &None).unwrap();
+               &route.paths[0], send_amt, RecipientOnionFields::secret_only(our_payment_secret), cur_height, &None).unwrap();
        let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash).unwrap();
 
        let mut msg = msgs::UpdateAddHTLC {