Consider HTLC in-flight count limits 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 the number of in-flight HTLCs which we are allowed
to push towards a counterparty at once, setting the available
balance to zero if we cannot push any further HTLCs.

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

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index bda0bfa509ca61a6b7d691e317726d091e72edbb..e0be0af5d72af39451fedb715b1b987e79eb192d 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2725,6 +2725,14 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                                available_capacity_msat = capacity_minus_commitment_fee_msat as u64;
                        }
                }
+
+               available_capacity_msat = cmp::min(available_capacity_msat,
+                       self.counterparty_max_htlc_value_in_flight_msat - outbound_stats.pending_htlcs_value_msat);
+
+               if outbound_stats.pending_htlcs + 1 > self.counterparty_max_accepted_htlcs as u32 {
+                       available_capacity_msat = 0;
+               }
+
                AvailableBalances {
                        inbound_capacity_msat: cmp::max(self.channel_value_satoshis as i64 * 1000
                                        - self.value_to_self_msat as i64
@@ -2732,10 +2740,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(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,
+                       next_outbound_htlc_limit_msat: available_capacity_msat,
                        balance_msat,
                }
        }
@@ -5885,10 +5890,12 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                let inbound_stats = self.get_inbound_pending_htlc_stats(None);
                let outbound_stats = self.get_outbound_pending_htlc_stats(None);
                if outbound_stats.pending_htlcs + 1 > self.counterparty_max_accepted_htlcs as u32 {
+                       debug_assert!(amount_msat > self.get_available_balances().next_outbound_htlc_limit_msat);
                        return Err(ChannelError::Ignore(format!("Cannot push more than their max accepted HTLCs ({})", self.counterparty_max_accepted_htlcs)));
                }
                // Check their_max_htlc_value_in_flight_msat
                if outbound_stats.pending_htlcs_value_msat + amount_msat > self.counterparty_max_htlc_value_in_flight_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 us over the max HTLC value in flight our peer will accept ({})", self.counterparty_max_htlc_value_in_flight_msat)));
                }
 

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 81c42861a8d8df22b1881b81552f92d0e43a2591..dfb101b1f907c85b82fddf68b86632e3b08d8e02 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -1102,6 +1102,9 @@ fn holding_cell_htlc_counting() {
        create_announced_chan_between_nodes(&nodes, 0, 1);
        let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2);
 
+       // Fetch a route in advance as we will be unable to once we're unable to send.
+       let (route, payment_hash_1, _, payment_secret_1) = get_route_and_payment_hash!(nodes[1], nodes[2], 100000);
+
        let mut payments = Vec::new();
        for _ in 0..50 {
                let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[2], 100000);
@@ -1119,7 +1122,6 @@ fn holding_cell_htlc_counting() {
        // There is now one HTLC in an outbound commitment transaction and (OUR_MAX_HTLCS - 1) HTLCs in
        // the holding cell waiting on B's RAA to send. At this point we should not be able to add
        // another HTLC.
-       let (route, payment_hash_1, _, payment_secret_1) = get_route_and_payment_hash!(nodes[1], nodes[2], 100000);
        {
                unwrap_send_err!(nodes[1].node.send_payment_with_route(&route, payment_hash_1,
                                RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0)
@@ -6112,6 +6114,8 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_num_and_htlc_id_increment()
        let max_accepted_htlcs = nodes[1].node.per_peer_state.read().unwrap().get(&nodes[0].node.get_our_node_id())
                .unwrap().lock().unwrap().channel_by_id.get(&chan.2).unwrap().counterparty_max_accepted_htlcs as u64;
 
+       // 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[0], nodes[1], 100000);
        for i in 0..max_accepted_htlcs {
                let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100000);
                let payment_event = {
@@ -6135,7 +6139,6 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_num_and_htlc_id_increment()
                expect_pending_htlcs_forwardable!(nodes[1]);
                expect_payment_claimable!(nodes[1], our_payment_hash, our_payment_secret, 100000);
        }
-       let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100000);
        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 },