Consider dust exposure 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 how much adding one additional (dust) HTLC would
impact our total dust exposure, setting the new next-HTLC-minimum
field to require HTLCs be non-dust if required or set our next-HTLC
maximum if we cannot send a dust HTLC but do have some additional
exposure remaining.

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

Fixes #2252.

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -2749,6 +2749,45 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 			}
 		}
 
+		let mut next_outbound_htlc_min_msat = self.counterparty_htlc_minimum_msat;
+
+		// If we get close to our maximum dust exposure, we end up in a situation where we can send
+		// between zero and the remaining dust exposure limit remaining OR above the dust limit.
+		// Because we cannot express this as a simple min/max, we prefer to tell the user they can
+		// send above the dust limit (as the router can always overpay to meet the dust limit).
+		let mut remaining_msat_below_dust_exposure_limit = None;
+		let mut dust_exposure_dust_limit_msat = 0;
+
+		let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if self.opt_anchors() {
+			(self.counterparty_dust_limit_satoshis, self.holder_dust_limit_satoshis)
+		} else {
+			let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64;
+			(self.counterparty_dust_limit_satoshis + dust_buffer_feerate * htlc_success_tx_weight(false) / 1000,
+			 self.holder_dust_limit_satoshis       + dust_buffer_feerate * htlc_timeout_tx_weight(false) / 1000)
+		};
+		let on_counterparty_dust_htlc_exposure_msat = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat;
+		if on_counterparty_dust_htlc_exposure_msat + htlc_success_dust_limit * 1000 - 1 > self.get_max_dust_htlc_exposure_msat() {
+			remaining_msat_below_dust_exposure_limit =
+				Some(self.get_max_dust_htlc_exposure_msat().saturating_sub(on_counterparty_dust_htlc_exposure_msat));
+			dust_exposure_dust_limit_msat = cmp::max(dust_exposure_dust_limit_msat, htlc_success_dust_limit * 1000);
+		}
+
+		let on_holder_dust_htlc_exposure_msat = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat;
+		if on_holder_dust_htlc_exposure_msat + htlc_timeout_dust_limit * 1000 - 1 > self.get_max_dust_htlc_exposure_msat() {
+			remaining_msat_below_dust_exposure_limit = Some(cmp::max(
+				remaining_msat_below_dust_exposure_limit.unwrap_or(0),
+				self.get_max_dust_htlc_exposure_msat().saturating_sub(on_holder_dust_htlc_exposure_msat)));
+			dust_exposure_dust_limit_msat = cmp::max(dust_exposure_dust_limit_msat, htlc_timeout_dust_limit * 1000);
+		}
+
+		if let Some(remaining_limit_msat) = remaining_msat_below_dust_exposure_limit {
+			if available_capacity_msat < dust_exposure_dust_limit_msat {
+				available_capacity_msat = cmp::min(available_capacity_msat, remaining_limit_msat);
+			} else {
+				next_outbound_htlc_min_msat = cmp::max(next_outbound_htlc_min_msat, dust_exposure_dust_limit_msat);
+			}
+		}
+
 		available_capacity_msat = cmp::min(available_capacity_msat,
 			self.counterparty_max_htlc_value_in_flight_msat - outbound_stats.pending_htlcs_value_msat);
 
@@ -2764,7 +2803,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 				0) as u64,
 			outbound_capacity_msat,
 			next_outbound_htlc_limit_msat: available_capacity_msat,
-			next_outbound_htlc_min_msat: 0,
+			next_outbound_htlc_min_msat,
 			balance_msat,
 		}
 	}
@@ -5898,6 +5937,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 		}
 
 		if amount_msat < self.counterparty_htlc_minimum_msat {
+			debug_assert!(amount_msat < self.get_available_balances().next_outbound_htlc_min_msat);
 			return Err(ChannelError::Ignore(format!("Cannot send less than their minimum HTLC value ({})", self.counterparty_htlc_minimum_msat)));
 		}
 
@@ -5946,6 +5986,8 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 		if amount_msat / 1000 < exposure_dust_limit_success_sats {
 			let on_counterparty_dust_htlc_exposure_msat = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat + amount_msat;
 			if on_counterparty_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() {
+				debug_assert!(amount_msat > self.get_available_balances().next_outbound_htlc_limit_msat ||
+					amount_msat < self.get_available_balances().next_outbound_htlc_min_msat);
 				return Err(ChannelError::Ignore(format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx",
 					on_counterparty_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat())));
 			}
@@ -5955,6 +5997,8 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 		if amount_msat / 1000 <  exposure_dust_limit_timeout_sats {
 			let on_holder_dust_htlc_exposure_msat = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat + amount_msat;
 			if on_holder_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() {
+				debug_assert!(amount_msat > self.get_available_balances().next_outbound_htlc_limit_msat ||
+					amount_msat < self.get_available_balances().next_outbound_htlc_min_msat);
 				return Err(ChannelError::Ignore(format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx",
 					on_holder_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat())));
 			}

lightning/src/ln/functional_tests.rs

@@ -9635,6 +9635,10 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
 	let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &channel_ready);
 	update_nodes_with_chan_announce(&nodes, 0, 1, &announcement, &as_update, &bs_update);
 
+	// Fetch a route in advance as we will be unable to once we're unable to send.
+	let (mut route, payment_hash, _, payment_secret) =
+		get_route_and_payment_hash!(nodes[0], nodes[1], 1000);
+
 	let dust_buffer_feerate = {
 		let per_peer_state = nodes[0].node.per_peer_state.read().unwrap();
 		let chan_lock = per_peer_state.get(&nodes[1].node.get_our_node_id()).unwrap().lock().unwrap();
@@ -9647,7 +9651,7 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
 	let dust_inbound_htlc_on_holder_tx_msat: u64 = (dust_buffer_feerate * htlc_success_tx_weight(opt_anchors) / 1000 + open_channel.dust_limit_satoshis - 1) * 1000;
 	let dust_inbound_htlc_on_holder_tx: u64 = config.channel_config.max_dust_htlc_exposure_msat / dust_inbound_htlc_on_holder_tx_msat;
 
-	let dust_htlc_on_counterparty_tx: u64 = 25;
+	let dust_htlc_on_counterparty_tx: u64 = 4;
 	let dust_htlc_on_counterparty_tx_msat: u64 = config.channel_config.max_dust_htlc_exposure_msat / dust_htlc_on_counterparty_tx;
 
 	if on_holder_tx {
@@ -9672,23 +9676,23 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
 		if dust_outbound_balance {
 			// Outbound dust threshold: 2132 sats (`dust_buffer_feerate` * HTLC_TIMEOUT_TX_WEIGHT / 1000 + counteparty's `dust_limit_satoshis`)
 			// Outbound dust balance: 5000 sats
-			for _ in 0..dust_htlc_on_counterparty_tx {
+			for _ in 0..dust_htlc_on_counterparty_tx - 1 {
 				let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], dust_htlc_on_counterparty_tx_msat);
 				nodes[0].node.send_payment_with_route(&route, payment_hash,
 					RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
 			}
 		} else {
 			// Inbound dust threshold: 2031 sats (`dust_buffer_feerate` * HTLC_TIMEOUT_TX_WEIGHT / 1000 + counteparty's `dust_limit_satoshis`)
 			// Inbound dust balance: 5000 sats
-			for _ in 0..dust_htlc_on_counterparty_tx {
+			for _ in 0..dust_htlc_on_counterparty_tx - 1 {
 				route_payment(&nodes[1], &[&nodes[0]], dust_htlc_on_counterparty_tx_msat);
 			}
 		}
 	}
 
-	let dust_overflow = dust_htlc_on_counterparty_tx_msat * (dust_htlc_on_counterparty_tx + 1);
 	if exposure_breach_event == ExposureEvent::AtHTLCForward {
-		let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], if on_holder_tx { dust_outbound_htlc_on_holder_tx_msat } else { dust_htlc_on_counterparty_tx_msat });
+		route.paths[0].hops.last_mut().unwrap().fee_msat =
+			if on_holder_tx { dust_outbound_htlc_on_holder_tx_msat } else { dust_htlc_on_counterparty_tx_msat + 1 };
 		let mut config = UserConfig::default();
 		// With default dust exposure: 5000 sats
 		if on_holder_tx {
@@ -9702,10 +9706,13 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
 			unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, payment_hash,
 					RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)
 				), true, APIError::ChannelUnavailable { ref err },
-				assert_eq!(err, &format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", dust_overflow, config.channel_config.max_dust_htlc_exposure_msat)));
+				assert_eq!(err,
+					&format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx",
+						dust_htlc_on_counterparty_tx_msat * (dust_htlc_on_counterparty_tx - 1) + dust_htlc_on_counterparty_tx_msat + 1,
+						config.channel_config.max_dust_htlc_exposure_msat)));
 		}
 	} else if exposure_breach_event == ExposureEvent::AtHTLCReception {
-		let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], if on_holder_tx { dust_inbound_htlc_on_holder_tx_msat } else { dust_htlc_on_counterparty_tx_msat });
+		let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], if on_holder_tx { dust_inbound_htlc_on_holder_tx_msat } else { dust_htlc_on_counterparty_tx_msat + 1 });
 		nodes[1].node.send_payment_with_route(&route, payment_hash,
 			RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
 		check_added_monitors!(nodes[1], 1);
@@ -9721,10 +9728,13 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
 			nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", if dust_outbound_balance { dust_outbound_overflow } else { dust_inbound_overflow }, config.channel_config.max_dust_htlc_exposure_msat), 1);
 		} else {
 			// Outbound dust balance: 5200 sats
-			nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", dust_overflow, config.channel_config.max_dust_htlc_exposure_msat), 1);
+			nodes[0].logger.assert_log("lightning::ln::channel".to_string(),
+				format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx",
+					dust_htlc_on_counterparty_tx_msat * (dust_htlc_on_counterparty_tx - 1) + dust_htlc_on_counterparty_tx_msat + 1,
+					config.channel_config.max_dust_htlc_exposure_msat), 1);
 		}
 	} else if exposure_breach_event == ExposureEvent::AtUpdateFeeOutbound {
-		let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 2_500_000);
+		route.paths[0].hops.last_mut().unwrap().fee_msat = 2_500_000;
 		nodes[0].node.send_payment_with_route(&route, payment_hash,
 			RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
 		{