f: Create IncludedHTLC type to enforce dust-vs-nondust sorting to TxBuilder

This is a temporary type that will be removed once we let `TxBuilder` choose
which HTLCs are dust and which are non-dust.

Author: tankyleo

lightning/src/ln/chan_utils.rs

@@ -647,23 +647,6 @@ impl HTLCOutputInCommitment {
 			&& self.cltv_expiry == other.cltv_expiry
 			&& self.payment_hash == other.payment_hash
 	}
-
-	pub(crate) fn is_dust(
-		&self, feerate_per_kw: u32, broadcaster_dust_limit_sat: u64, features: &ChannelTypeFeatures,
-	) -> bool {
-		let htlc_tx_fee_sat = if features.supports_anchors_zero_fee_htlc_tx() {
-			0
-		} else {
-			let htlc_tx_weight = if self.offered {
-				htlc_timeout_tx_weight(features)
-			} else {
-				htlc_success_tx_weight(features)
-			};
-			// As required by the spec, round down
-			feerate_per_kw as u64 * htlc_tx_weight / 1000
-		};
-		self.amount_msat / 1000 < broadcaster_dust_limit_sat + htlc_tx_fee_sat
-	}
 }
 
 impl_writeable_tlv_based!(HTLCOutputInCommitment, {

lightning/src/ln/channel.rs

@@ -450,6 +450,15 @@ impl OutboundHTLCOutput {
 	}
 }
 
+#[derive(Clone, Debug)]
+pub(crate) struct IncludedHTLC {
+	pub offered: bool,
+	pub amount_msat: u64,
+	pub cltv_expiry: u32,
+	pub payment_hash: PaymentHash,
+	pub is_dust: bool,
+}
+
 /// See AwaitingRemoteRevoke ChannelState for more info
 #[cfg_attr(test, derive(Clone, Debug, PartialEq))]
 enum HTLCUpdateAwaitingACK {
@@ -4566,7 +4575,8 @@ where
 		let feerate_per_kw = self.get_commitment_feerate(funding, generated_by_local);
 
 		let num_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len();
-		let mut htlcs_included: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(num_htlcs);
+		let mut htlc_source_table: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(num_htlcs);
+		let mut htlcs_included: Vec<IncludedHTLC> = Vec::with_capacity(num_htlcs);
 		let mut value_to_self_claimed_msat = 0;
 		let mut value_to_remote_claimed_msat = 0;
 
@@ -4583,15 +4593,29 @@ where
 					amount_msat: $htlc.amount_msat,
 					cltv_expiry: $htlc.cltv_expiry,
 					payment_hash: $htlc.payment_hash,
-					transaction_output_index: None
+					transaction_output_index: None,
+				}
+			}
+		}
+
+		macro_rules! get_included_htlc {
+			($htlc: expr, $offered: expr, $is_dust: expr) => {
+				IncludedHTLC {
+					offered: $offered,
+					amount_msat: $htlc.amount_msat,
+					cltv_expiry: $htlc.cltv_expiry,
+					payment_hash: $htlc.payment_hash,
+					is_dust: $is_dust,
 				}
 			}
 		}
 
 		macro_rules! add_htlc_output {
-			($htlc: expr, $outbound: expr, $source: expr) => {
-				let htlc_in_tx = get_htlc_in_commitment!($htlc, $outbound == local);
-				htlcs_included.push((htlc_in_tx, $source));
+			($htlc: expr, $outbound: expr, $source: expr, $is_dust: expr) => {
+				let table_htlc = get_htlc_in_commitment!($htlc, $outbound == local);
+				let included_htlc = get_included_htlc!($htlc, $outbound == local, $is_dust);
+				htlc_source_table.push((table_htlc, $source));
+				htlcs_included.push(included_htlc);
 			}
 		}
 
@@ -4601,7 +4625,8 @@ where
 		for htlc in self.pending_inbound_htlcs.iter() {
 			if htlc.state.included_in_commitment(generated_by_local) {
 				log_trace!(logger, "   ...including inbound {} HTLC {} (hash {}) with value {}", htlc.state, htlc.htlc_id, htlc.payment_hash, htlc.amount_msat);
-				add_htlc_output!(htlc, false, None);
+				let is_dust = htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, funding.get_channel_type());
+				add_htlc_output!(htlc, false, None, is_dust);
 			} else {
 				log_trace!(logger, "   ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state);
 				if let Some(preimage) = htlc.state.preimage() {
@@ -4617,7 +4642,8 @@ where
 			}
 			if htlc.state.included_in_commitment(generated_by_local) {
 				log_trace!(logger, "   ...including outbound {} HTLC {} (hash {}) with value {}", htlc.state, htlc.htlc_id, htlc.payment_hash, htlc.amount_msat);
-				add_htlc_output!(htlc, true, Some(&htlc.source));
+				let is_dust = htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, funding.get_channel_type());
+				add_htlc_output!(htlc, true, Some(&htlc.source), is_dust);
 			} else {
 				log_trace!(logger, "   ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state);
 				if htlc.state.preimage().is_some() {
@@ -4639,7 +4665,7 @@ where
 			&funding.channel_transaction_parameters,
 			&self.secp_ctx,
 			value_to_self_msat,
-			htlcs_included.iter().map(|(htlc, _source)| htlc).cloned().collect(),
+			htlcs_included,
 			feerate_per_kw,
 			broadcaster_dust_limit_sat,
 			logger,
@@ -4652,7 +4678,7 @@ where
 		// This brute-force search is O(n^2) over ~1k HTLCs in the worst case. This case is very
 		// rare at the moment.
 		for nondust_htlc in tx.nondust_htlcs() {
-			let htlc = htlcs_included
+			let htlc = htlc_source_table
 				.iter_mut()
 				.filter(|(htlc, _source)| htlc.transaction_output_index.is_none())
 				.find_map(|(htlc, _source)| {
@@ -4668,7 +4694,7 @@ where
 
 		// This places the non-dust HTLC-source pairs first, in the order they appear in the
 		// commitment transaction, followed by the dust HTLC-source pairs.
-		htlcs_included.sort_unstable_by(|(htlc_a, _), (htlc_b, _)| {
+		htlc_source_table.sort_unstable_by(|(htlc_a, _), (htlc_b, _)| {
 			match (htlc_a.transaction_output_index, htlc_b.transaction_output_index) {
 				// `None` is smaller than `Some`, but we want `Some` ordered before `None` in the vector
 				(None, Some(_)) => cmp::Ordering::Greater,
@@ -4680,7 +4706,7 @@ where
 		CommitmentData {
 			tx,
 			stats,
-			htlcs_included,
+			htlcs_included: htlc_source_table,
 			inbound_htlc_preimages,
 			outbound_htlc_preimages,
 		}

lightning/src/sign/tx_builder.rs

@@ -7,7 +7,7 @@ use bitcoin::secp256k1::{self, PublicKey, Secp256k1};
 use crate::ln::chan_utils::{
 	commit_tx_fee_sat, ChannelTransactionParameters, CommitmentTransaction, HTLCOutputInCommitment,
 };
-use crate::ln::channel::{CommitmentStats, ANCHOR_OUTPUT_VALUE_SATOSHI};
+use crate::ln::channel::{CommitmentStats, IncludedHTLC, ANCHOR_OUTPUT_VALUE_SATOSHI};
 use crate::prelude::*;
 use crate::types::features::ChannelTypeFeatures;
 use crate::util::logger::Logger;
@@ -21,7 +21,7 @@ pub(crate) trait TxBuilder {
 	fn build_commitment_transaction<L: Deref>(
 		&self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey,
 		channel_parameters: &ChannelTransactionParameters, secp_ctx: &Secp256k1<secp256k1::All>,
-		value_to_self_msat: u64, htlcs_in_tx: Vec<HTLCOutputInCommitment>, feerate_per_kw: u32,
+		value_to_self_msat: u64, htlcs_in_tx: Vec<IncludedHTLC>, feerate_per_kw: u32,
 		broadcaster_dust_limit_satoshis: u64, logger: &L,
 	) -> (CommitmentTransaction, CommitmentStats)
 	where
@@ -72,7 +72,7 @@ impl TxBuilder for SpecTxBuilder {
 	fn build_commitment_transaction<L: Deref>(
 		&self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey,
 		channel_parameters: &ChannelTransactionParameters, secp_ctx: &Secp256k1<secp256k1::All>,
-		value_to_self_msat: u64, mut htlcs_in_tx: Vec<HTLCOutputInCommitment>, feerate_per_kw: u32,
+		value_to_self_msat: u64, mut htlcs_in_tx: Vec<IncludedHTLC>, feerate_per_kw: u32,
 		broadcaster_dust_limit_satoshis: u64, logger: &L,
 	) -> (CommitmentTransaction, CommitmentStats)
 	where
@@ -89,11 +89,7 @@ impl TxBuilder for SpecTxBuilder {
 			} else {
 				remote_htlc_total_msat += htlc.amount_msat;
 			}
-			if htlc.is_dust(
-				feerate_per_kw,
-				broadcaster_dust_limit_satoshis,
-				&channel_parameters.channel_type_features,
-			) {
+			if htlc.is_dust {
 				log_trace!(
 					logger,
 					"   ...trimming {} HTLC with value {}sat, hash {}, due to dust limit {}",
@@ -182,7 +178,20 @@ impl TxBuilder for SpecTxBuilder {
 			to_broadcaster_value_sat,
 			to_countersignatory_value_sat,
 			feerate_per_kw,
-			htlcs_in_tx,
+			htlcs_in_tx
+				.into_iter()
+				.map(|htlc| {
+					// We filtered out all dust HTLCs above
+					debug_assert!(!htlc.is_dust);
+					HTLCOutputInCommitment {
+						offered: htlc.offered,
+						amount_msat: htlc.amount_msat,
+						cltv_expiry: htlc.cltv_expiry,
+						payment_hash: htlc.payment_hash,
+						transaction_output_index: None,
+					}
+				})
+				.collect(),
 			&directed_parameters,
 			secp_ctx,
 		);