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.

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -2725,17 +2725,22 @@ 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
 					- self.get_inbound_pending_htlc_stats(None).pending_htlcs_value_msat as i64
 					- 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)));
 		}
 

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 },