From b09ccd10be82a0093ae7b1b196c739fb8f32c42b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 15 May 2023 03:34:18 +0000 Subject: [PATCH] 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. --- lightning/src/ln/channel.rs | 15 +++++++++++---- lightning/src/ln/functional_tests.rs | 7 +++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index bda0bfa50..e0be0af5d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2725,6 +2725,14 @@ impl Channel { 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 Channel { - 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 Channel { 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 81c42861a..dfb101b1f 100644 --- 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 }, -- 2.39.5