Consider counterparty commitment tx fees 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 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.

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -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()));
 			}
 		}

lightning/src/ln/functional_tests.rs

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