Include excess commitment transaction fees in dust exposure

Transaction fees on counterparty commitment transactions are
ultimately not our money and thus are really "dust" from our PoV -
they're funds that may be ours during off-chain updates but are not
ours once we go on-chain.

Thus, here, we count any such fees in excess of our own fee
estimates towards dust exposure. We don't bother to make an
inbound/outbound channel distinction here as in most cases users
will use `MaxDustExposure::FeeRateMultiplier` which will scale
with the fee we set on outbound channels anyway.

Note that this also enables the dust exposure checks on anchor
channels during feerate updates. We'd previously elided these as
increases in the channel feerates do not change the HTLC dust
exposure, but now do for the fee dust exposure.

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -2339,15 +2339,16 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 		cmp::max(self.config.options.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA)
 	}
 
-	pub fn get_max_dust_htlc_exposure_msat<F: Deref>(&self,
-		fee_estimator: &LowerBoundedFeeEstimator<F>) -> u64
-	where F::Target: FeeEstimator
-	{
+	fn get_dust_exposure_limiting_feerate<F: Deref>(&self,
+		fee_estimator: &LowerBoundedFeeEstimator<F>,
+	) -> u32 where F::Target: FeeEstimator {
+		fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::OnChainSweep)
+	}
+
+	pub fn get_max_dust_htlc_exposure_msat(&self, limiting_feerate_sat_per_kw: u32) -> u64 {
 		match self.config.options.max_dust_htlc_exposure {
 			MaxDustHTLCExposure::FeeRateMultiplier(multiplier) => {
-				let feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(
-					ConfirmationTarget::OnChainSweep) as u64;
-				feerate_per_kw.saturating_mul(multiplier)
+				(limiting_feerate_sat_per_kw as u64).saturating_mul(multiplier)
 			},
 			MaxDustHTLCExposure::FixedLimitMsat(limit) => limit,
 		}
@@ -2741,29 +2742,35 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 	}
 
 	/// Returns a HTLCStats about pending htlcs
-	fn get_pending_htlc_stats(&self, outbound_feerate_update: Option<u32>) -> HTLCStats {
+	fn get_pending_htlc_stats(&self, outbound_feerate_update: Option<u32>, dust_exposure_limiting_feerate: u32) -> HTLCStats {
 		let context = self;
 		let uses_0_htlc_fee_anchors = self.get_channel_type().supports_anchors_zero_fee_htlc_tx();
 
+		let dust_buffer_feerate = context.get_dust_buffer_feerate(outbound_feerate_update);
 		let (htlc_timeout_dust_limit, htlc_success_dust_limit) = if uses_0_htlc_fee_anchors {
 			(0, 0)
 		} else {
-			let dust_buffer_feerate = context.get_dust_buffer_feerate(outbound_feerate_update) as u64;
-			(dust_buffer_feerate * htlc_timeout_tx_weight(context.get_channel_type()) / 1000,
-				dust_buffer_feerate * htlc_success_tx_weight(context.get_channel_type()) / 1000)
+			(dust_buffer_feerate as u64 * htlc_timeout_tx_weight(context.get_channel_type()) / 1000,
+				dust_buffer_feerate as u64 * htlc_success_tx_weight(context.get_channel_type()) / 1000)
 		};
 
 		let mut on_holder_tx_dust_exposure_msat = 0;
 		let mut on_counterparty_tx_dust_exposure_msat = 0;
 
+		let mut on_counterparty_tx_offered_nondust_htlcs = 0;
+		let mut on_counterparty_tx_accepted_nondust_htlcs = 0;
+
 		let mut pending_inbound_htlcs_value_msat = 0;
+
 		{
 			let counterparty_dust_limit_timeout_sat = htlc_timeout_dust_limit + context.counterparty_dust_limit_satoshis;
 			let holder_dust_limit_success_sat = htlc_success_dust_limit + context.holder_dust_limit_satoshis;
 			for ref htlc in context.pending_inbound_htlcs.iter() {
 				pending_inbound_htlcs_value_msat += htlc.amount_msat;
 				if htlc.amount_msat / 1000 < counterparty_dust_limit_timeout_sat {
 					on_counterparty_tx_dust_exposure_msat += htlc.amount_msat;
+				} else {
+					on_counterparty_tx_offered_nondust_htlcs += 1;
 				}
 				if htlc.amount_msat / 1000 < holder_dust_limit_success_sat {
 					on_holder_tx_dust_exposure_msat += htlc.amount_msat;
@@ -2782,6 +2789,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 				pending_outbound_htlcs_value_msat += htlc.amount_msat;
 				if htlc.amount_msat / 1000 < counterparty_dust_limit_success_sat {
 					on_counterparty_tx_dust_exposure_msat += htlc.amount_msat;
+				} else {
+					on_counterparty_tx_accepted_nondust_htlcs += 1;
 				}
 				if htlc.amount_msat / 1000 < holder_dust_limit_timeout_sat {
 					on_holder_tx_dust_exposure_msat += htlc.amount_msat;
@@ -2795,6 +2804,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 					outbound_holding_cell_msat += amount_msat;
 					if *amount_msat / 1000 < counterparty_dust_limit_success_sat {
 						on_counterparty_tx_dust_exposure_msat += amount_msat;
+					} else {
+						on_counterparty_tx_accepted_nondust_htlcs += 1;
 					}
 					if *amount_msat / 1000 < holder_dust_limit_timeout_sat {
 						on_holder_tx_dust_exposure_msat += amount_msat;
@@ -2805,6 +2816,26 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 			}
 		}
 
+		// Include any mining "excess" fees in the dust calculation
+		let excess_feerate_opt = outbound_feerate_update
+			.or(self.pending_update_fee.map(|(fee, _)| fee))
+			.unwrap_or(self.feerate_per_kw)
+			.checked_sub(dust_exposure_limiting_feerate);
+		if let Some(excess_feerate) = excess_feerate_opt {
+			let on_counterparty_tx_nondust_htlcs =
+				on_counterparty_tx_accepted_nondust_htlcs + on_counterparty_tx_offered_nondust_htlcs;
+			on_counterparty_tx_dust_exposure_msat +=
+				commit_tx_fee_msat(excess_feerate, on_counterparty_tx_nondust_htlcs, &self.channel_type);
+			if !self.channel_type.supports_anchors_zero_fee_htlc_tx() {
+				on_counterparty_tx_dust_exposure_msat +=
+					on_counterparty_tx_accepted_nondust_htlcs as u64 * htlc_success_tx_weight(&self.channel_type)
+					* excess_feerate as u64 / 1000;
+				on_counterparty_tx_dust_exposure_msat +=
+					on_counterparty_tx_offered_nondust_htlcs as u64 * htlc_timeout_tx_weight(&self.channel_type)
+					* excess_feerate as u64 / 1000;
+			}
+		}
+
 		HTLCStats {
 			pending_inbound_htlcs: self.pending_inbound_htlcs.len(),
 			pending_outbound_htlcs,
@@ -2919,8 +2950,11 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 	where F::Target: FeeEstimator
 	{
 		let context = &self;
-		// Note that we have to handle overflow due to the above case.
-		let htlc_stats = context.get_pending_htlc_stats(None);
+		// Note that we have to handle overflow due to the case mentioned in the docs in general
+		// here.
+
+		let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(&fee_estimator);
+		let htlc_stats = context.get_pending_htlc_stats(None, dust_exposure_limiting_feerate);
 
 		let mut balance_msat = context.value_to_self_msat;
 		for ref htlc in context.pending_inbound_htlcs.iter() {
@@ -3008,7 +3042,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 		// 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 max_dust_htlc_exposure_msat = context.get_max_dust_htlc_exposure_msat(fee_estimator);
+		let max_dust_htlc_exposure_msat = context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate);
 
 		let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
 			(context.counterparty_dust_limit_satoshis, context.holder_dust_limit_satoshis)
@@ -3017,7 +3051,23 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 			(context.counterparty_dust_limit_satoshis + dust_buffer_feerate * htlc_success_tx_weight(context.get_channel_type()) / 1000,
 			 context.holder_dust_limit_satoshis       + dust_buffer_feerate * htlc_timeout_tx_weight(context.get_channel_type()) / 1000)
 		};
-		if htlc_stats.on_counterparty_tx_dust_exposure_msat as i64 + htlc_success_dust_limit as i64 * 1000 - 1 > max_dust_htlc_exposure_msat.try_into().unwrap_or(i64::max_value()) {
+
+		let excess_feerate_opt = self.feerate_per_kw.checked_sub(dust_exposure_limiting_feerate);
+		if let Some(excess_feerate) = excess_feerate_opt {
+			let htlc_dust_exposure_msat =
+				per_outbound_htlc_counterparty_commit_tx_fee_msat(excess_feerate, &context.channel_type);
+			let nondust_htlc_counterparty_tx_dust_exposure =
+				htlc_stats.on_counterparty_tx_dust_exposure_msat.saturating_add(htlc_dust_exposure_msat);
+			if nondust_htlc_counterparty_tx_dust_exposure > max_dust_htlc_exposure_msat {
+				// If adding an extra HTLC would put us over the dust limit in total fees, we cannot
+				// send any non-dust HTLCs.
+				available_capacity_msat = cmp::min(available_capacity_msat, htlc_success_dust_limit * 1000);
+			}
+		}
+
+		if htlc_stats.on_counterparty_tx_dust_exposure_msat.saturating_add(htlc_success_dust_limit * 1000) > max_dust_htlc_exposure_msat.saturating_add(1) {
+			// Note that we don't use the `counterparty_tx_dust_exposure` (with
+			// `htlc_dust_exposure_msat`) here as it only applies to non-dust HTLCs.
 			remaining_msat_below_dust_exposure_limit =
 				Some(max_dust_htlc_exposure_msat.saturating_sub(htlc_stats.on_counterparty_tx_dust_exposure_msat));
 			dust_exposure_dust_limit_msat = cmp::max(dust_exposure_dust_limit_msat, htlc_success_dust_limit * 1000);
@@ -3517,6 +3567,17 @@ pub(crate) fn commit_tx_fee_msat(feerate_per_kw: u32, num_htlcs: usize, channel_
 	(commitment_tx_base_weight(channel_type_features) + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * feerate_per_kw as u64 / 1000 * 1000
 }
 
+pub(crate) fn per_outbound_htlc_counterparty_commit_tx_fee_msat(feerate_per_kw: u32, channel_type_features: &ChannelTypeFeatures) -> u64 {
+	// Note that we need to divide before multiplying to round properly,
+	// since the lowest denomination of bitcoin on-chain is the satoshi.
+	let commitment_tx_fee = COMMITMENT_TX_WEIGHT_PER_HTLC * feerate_per_kw as u64 / 1000 * 1000;
+	if channel_type_features.supports_anchors_zero_fee_htlc_tx() {
+		commitment_tx_fee + htlc_success_tx_weight(channel_type_features) * feerate_per_kw as u64 / 1000
+	} else {
+		commitment_tx_fee
+	}
+}
+
 /// Context for dual-funded channels.
 #[cfg(any(dual_funding, splicing))]
 pub(super) struct DualFundingChannelContext {
@@ -4114,9 +4175,10 @@ impl<SP: Deref> Channel<SP> where
 		Ok(self.get_announcement_sigs(node_signer, chain_hash, user_config, best_block.height, logger))
 	}
 
-	pub fn update_add_htlc(
+	pub fn update_add_htlc<F: Deref>(
 		&mut self, msg: &msgs::UpdateAddHTLC, pending_forward_status: PendingHTLCStatus,
-	) -> Result<(), ChannelError> {
+		fee_estimator: &LowerBoundedFeeEstimator<F>,
+	) -> Result<(), ChannelError> where F::Target: FeeEstimator {
 		if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) {
 			return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state".to_owned()));
 		}
@@ -4137,7 +4199,8 @@ impl<SP: Deref> Channel<SP> where
 			return Err(ChannelError::Close(format!("Remote side tried to send less than our minimum HTLC value. Lower limit: ({}). Actual: ({})", self.context.holder_htlc_minimum_msat, msg.amount_msat)));
 		}
 
-		let htlc_stats = self.context.get_pending_htlc_stats(None);
+		let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator);
+		let htlc_stats = self.context.get_pending_htlc_stats(None, dust_exposure_limiting_feerate);
 		if htlc_stats.pending_inbound_htlcs + 1 > self.context.holder_max_accepted_htlcs as usize {
 			return Err(ChannelError::Close(format!("Remote tried to push more than our max accepted HTLCs ({})", self.context.holder_max_accepted_htlcs)));
 		}
@@ -4989,7 +5052,8 @@ impl<SP: Deref> Channel<SP> where
 		}
 
 		// Before proposing a feerate update, check that we can actually afford the new fee.
-		let htlc_stats = self.context.get_pending_htlc_stats(Some(feerate_per_kw));
+		let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator);
+		let htlc_stats = self.context.get_pending_htlc_stats(Some(feerate_per_kw), dust_exposure_limiting_feerate);
 		let keys = self.context.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
 		let commitment_stats = self.context.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &keys, true, true, logger);
 		let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_stats.num_nondust_htlcs + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, self.context.get_channel_type()) * 1000;
@@ -5001,7 +5065,7 @@ impl<SP: Deref> Channel<SP> where
 		}
 
 		// Note, we evaluate pending htlc "preemptive" trimmed-to-dust threshold at the proposed `feerate_per_kw`.
-		let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(fee_estimator);
+		let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate);
 		if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat {
 			log_debug!(logger, "Cannot afford to send new feerate at {} without infringing max dust htlc exposure", feerate_per_kw);
 			return None;
@@ -5239,17 +5303,16 @@ impl<SP: Deref> Channel<SP> where
 		self.context.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced));
 		self.context.update_time_counter += 1;
 		// Check that we won't be pushed over our dust exposure limit by the feerate increase.
-		if !self.context.channel_type.supports_anchors_zero_fee_htlc_tx() {
-			let htlc_stats = self.context.get_pending_htlc_stats(None);
-			let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(fee_estimator);
-			if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat {
-				return Err(ChannelError::Close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our own transactions (totaling {} msat)",
-					msg.feerate_per_kw, htlc_stats.on_holder_tx_dust_exposure_msat)));
-			}
-			if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat {
-				return Err(ChannelError::Close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our counterparty's transactions (totaling {} msat)",
-					msg.feerate_per_kw, htlc_stats.on_counterparty_tx_dust_exposure_msat)));
-			}
+		let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator);
+		let htlc_stats = self.context.get_pending_htlc_stats(None, dust_exposure_limiting_feerate);
+		let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate);
+		if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat {
+			return Err(ChannelError::Close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our own transactions (totaling {} msat)",
+				msg.feerate_per_kw, htlc_stats.on_holder_tx_dust_exposure_msat)));
+		}
+		if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat {
+			return Err(ChannelError::Close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our counterparty's transactions (totaling {} msat)",
+				msg.feerate_per_kw, htlc_stats.on_counterparty_tx_dust_exposure_msat)));
 		}
 		Ok(())
 	}
@@ -6093,8 +6156,9 @@ impl<SP: Deref> Channel<SP> where
 			return Err(("Shutdown was already sent", 0x4000|8))
 		}
 
-		let htlc_stats = self.context.get_pending_htlc_stats(None);
-		let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(fee_estimator);
+		let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator);
+		let htlc_stats = self.context.get_pending_htlc_stats(None, dust_exposure_limiting_feerate);
+		let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate);
 		let (htlc_timeout_dust_limit, htlc_success_dust_limit) = if self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
 			(0, 0)
 		} else {
@@ -6110,6 +6174,16 @@ impl<SP: Deref> Channel<SP> where
 					on_counterparty_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat);
 				return Err(("Exceeded our dust exposure limit on counterparty commitment tx", 0x1000|7))
 			}
+		} else {
+			let htlc_dust_exposure_msat =
+				per_outbound_htlc_counterparty_commit_tx_fee_msat(self.context.feerate_per_kw, &self.context.channel_type);
+			let counterparty_tx_dust_exposure =
+				htlc_stats.on_counterparty_tx_dust_exposure_msat.saturating_add(htlc_dust_exposure_msat);
+			if counterparty_tx_dust_exposure > max_dust_htlc_exposure_msat {
+				log_info!(logger, "Cannot accept value that would put our exposure to tx fee dust at {} over the limit {} on counterparty commitment tx",
+					counterparty_tx_dust_exposure, max_dust_htlc_exposure_msat);
+				return Err(("Exceeded our tx fee dust exposure limit on counterparty commitment tx", 0x1000|7))
+			}
 		}
 
 		let exposure_dust_limit_success_sats = htlc_success_dust_limit + self.context.holder_dust_limit_satoshis;

lightning/src/ln/channelmanager.rs

@@ -7673,7 +7673,7 @@ where
 							}
 						}
 					}
-					try_chan_phase_entry!(self, chan.update_add_htlc(&msg, pending_forward_info), chan_phase_entry);
+					try_chan_phase_entry!(self, chan.update_add_htlc(&msg, pending_forward_info, &self.fee_estimator), chan_phase_entry);
 				} else {
 					return try_chan_phase_entry!(self, Err(ChannelError::Close(
 						"Got an update_add_htlc message for an unfunded channel!".into())), chan_phase_entry);