From 1ff0e4187fb966e8b0da34ace5b5aacaa5badbdd Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Wed, 5 Mar 2025 04:17:17 +0000 Subject: [PATCH] [Custom Transactions] Define the `TxBuilder` trait This commit defines the `TxBuilder` trait to give users the ability to customize the build of the lightning commitment transactions. The `TxBuilder` trait has a single method, `build_commitment_transaction`, which builds the commitment transaction, and populates the output indices of the HTLCs passed in. Remove generic from `CommitmentTransaction::new` For the greatest compatibility with bindings, the argument is now a `Vec`. These non-dust HTLCs are populated during the construction of `CommitmentTransaction` and eventually cached by the object. Anything interested in these populated HTLCs should now use `CommitmentTransaction::htlcs`. Back in channel, we choose to do a brute force search over the HTLC-source pairs to assign their output indices. This is O(n^2) over ~1k HTLCs in the worst case. This case is very rare/non-existent at the moment. Feel free to optimize if LN channels start seeing an increasing number of HTLCs on their commitment transactions. --- lightning/src/chain/channelmonitor.rs | 48 ++++--- lightning/src/chain/onchaintx.rs | 11 +- lightning/src/ln/chan_utils.rs | 109 +++++++-------- lightning/src/ln/channel.rs | 152 +++++++-------------- lightning/src/ln/functional_tests.rs | 13 +- lightning/src/sign/mod.rs | 1 + lightning/src/sign/tx_builder.rs | 183 ++++++++++++++++++++++++++ 7 files changed, 324 insertions(+), 193 deletions(-) create mode 100644 lightning/src/sign/tx_builder.rs diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 6075d4f3513..349112548ee 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -3432,12 +3432,12 @@ impl ChannelMonitorImpl { fn build_counterparty_commitment_tx( &self, commitment_number: u64, their_per_commitment_point: &PublicKey, to_broadcaster_value: u64, to_countersignatory_value: u64, feerate_per_kw: u32, - mut nondust_htlcs: Vec<(HTLCOutputInCommitment, Option>)> + nondust_htlcs: Vec, ) -> CommitmentTransaction { let channel_parameters = &self.onchain_tx_handler.channel_transaction_parameters.as_counterparty_broadcastable(); - CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number, their_per_commitment_point, - to_broadcaster_value, to_countersignatory_value, feerate_per_kw, &mut nondust_htlcs, channel_parameters, &self.onchain_tx_handler.secp_ctx) + CommitmentTransaction::new(commitment_number, their_per_commitment_point, + to_broadcaster_value, to_countersignatory_value, feerate_per_kw, nondust_htlcs, channel_parameters, &self.onchain_tx_handler.secp_ctx) } fn counterparty_commitment_txs_from_update(&self, update: &ChannelMonitorUpdate) -> Vec { @@ -3450,8 +3450,8 @@ impl ChannelMonitorImpl { to_countersignatory_value_sat: Some(to_countersignatory_value) } => { let nondust_htlcs = htlc_outputs.iter().filter_map(|(htlc, _)| { - htlc.transaction_output_index.map(|_| (htlc.clone(), None)) - }).collect::>(); + htlc.transaction_output_index.map(|_| htlc) + }).cloned().collect::>(); let commitment_tx = self.build_counterparty_commitment_tx(commitment_number, &their_per_commitment_point, to_broadcaster_value, @@ -5320,13 +5320,13 @@ mod tests { { let mut res = Vec::new(); for (idx, preimage) in $preimages_slice.iter().enumerate() { - res.push((HTLCOutputInCommitment { + res.push(HTLCOutputInCommitment { offered: true, amount_msat: 0, cltv_expiry: 0, payment_hash: preimage.1.clone(), transaction_output_index: Some(idx as u32), - }, ())); + }); } res } @@ -5334,7 +5334,7 @@ mod tests { } macro_rules! preimages_slice_to_htlc_outputs { ($preimages_slice: expr) => { - preimages_slice_to_htlcs!($preimages_slice).into_iter().map(|(htlc, _)| (htlc, None)).collect() + preimages_slice_to_htlcs!($preimages_slice).into_iter().map(|htlc| (htlc, None)).collect() } } let dummy_sig = crate::crypto::utils::sign(&secp_ctx, @@ -5390,14 +5390,16 @@ mod tests { let monitor = ChannelMonitor::new(Secp256k1::new(), keys, Some(ShutdownScript::new_p2wpkh_from_pubkey(shutdown_pubkey).into_inner()), 0, &ScriptBuf::new(), (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, ScriptBuf::new()), - &channel_parameters, true, ScriptBuf::new(), 46, 0, HolderCommitmentTransaction::dummy(0, &mut Vec::new()), + &channel_parameters, true, ScriptBuf::new(), 46, 0, HolderCommitmentTransaction::dummy(0, Vec::new()), best_block, dummy_key, channel_id); - let mut htlcs = preimages_slice_to_htlcs!(preimages[0..10]); - let dummy_commitment_tx = HolderCommitmentTransaction::dummy(0, &mut htlcs); + let htlcs = preimages_slice_to_htlcs!(preimages[0..10]); + let dummy_commitment_tx = HolderCommitmentTransaction::dummy(0, htlcs); + // These htlcs now have their output indices assigned + let htlcs = dummy_commitment_tx.htlcs().clone(); - monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(), - htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()); + monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx, + htlcs.into_iter().map(|htlc| (htlc, Some(dummy_sig), None)).collect()); monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"1").to_byte_array()), preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger); monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"2").to_byte_array()), @@ -5432,10 +5434,12 @@ mod tests { // Now update holder commitment tx info, pruning only element 18 as we still care about the // previous commitment tx's preimages too - let mut htlcs = preimages_slice_to_htlcs!(preimages[0..5]); - let dummy_commitment_tx = HolderCommitmentTransaction::dummy(0, &mut htlcs); - monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(), - htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()); + let htlcs = preimages_slice_to_htlcs!(preimages[0..5]); + let dummy_commitment_tx = HolderCommitmentTransaction::dummy(0, htlcs); + // These htlcs now have their output indices assigned + let htlcs = dummy_commitment_tx.htlcs().clone(); + monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx, + htlcs.into_iter().map(|htlc| (htlc, Some(dummy_sig), None)).collect()); secret[0..32].clone_from_slice(&>::from_hex("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap()); monitor.provide_secret(281474976710653, secret.clone()).unwrap(); assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 12); @@ -5443,10 +5447,12 @@ mod tests { test_preimages_exist!(&preimages[18..20], monitor); // But if we do it again, we'll prune 5-10 - let mut htlcs = preimages_slice_to_htlcs!(preimages[0..3]); - let dummy_commitment_tx = HolderCommitmentTransaction::dummy(0, &mut htlcs); + let htlcs = preimages_slice_to_htlcs!(preimages[0..3]); + let dummy_commitment_tx = HolderCommitmentTransaction::dummy(0, htlcs); + // These htlcs now have their output indices assigned + let htlcs = dummy_commitment_tx.htlcs().clone(); monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx, - htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()); + htlcs.into_iter().map(|htlc| (htlc, Some(dummy_sig), None)).collect()); secret[0..32].clone_from_slice(&>::from_hex("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap()); monitor.provide_secret(281474976710652, secret.clone()).unwrap(); assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 5); @@ -5641,7 +5647,7 @@ mod tests { let monitor = ChannelMonitor::new(Secp256k1::new(), keys, Some(ShutdownScript::new_p2wpkh_from_pubkey(shutdown_pubkey).into_inner()), 0, &ScriptBuf::new(), (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, ScriptBuf::new()), - &channel_parameters, true, ScriptBuf::new(), 46, 0, HolderCommitmentTransaction::dummy(0, &mut Vec::new()), + &channel_parameters, true, ScriptBuf::new(), 46, 0, HolderCommitmentTransaction::dummy(0, Vec::new()), best_block, dummy_key, channel_id); let chan_id = monitor.inner.lock().unwrap().channel_id(); diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index e0c2bd1b306..e1884cc3acf 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -1363,7 +1363,7 @@ mod tests { for i in 0..3 { let preimage = PaymentPreimage([i; 32]); let hash = PaymentHash(Sha256::hash(&preimage.0[..]).to_byte_array()); - htlcs.push(( + htlcs.push( HTLCOutputInCommitment { offered: true, amount_msat: 10000, @@ -1371,10 +1371,11 @@ mod tests { payment_hash: hash, transaction_output_index: Some(i as u32), }, - (), - )); + ); } - let holder_commit = HolderCommitmentTransaction::dummy(1000000, &mut htlcs); + let holder_commit = HolderCommitmentTransaction::dummy(1000000, htlcs); + // These htlcs now have their output indices assigned + let htlcs = holder_commit.htlcs().clone(); let mut tx_handler = OnchainTxHandler::new( 1000000, [0; 32], @@ -1400,7 +1401,7 @@ mod tests { // Request claiming of each HTLC on the holder's commitment, with current block height 1. let holder_commit_txid = tx_handler.get_unsigned_holder_commitment_tx().compute_txid(); let mut requests = Vec::new(); - for (htlc, _) in htlcs { + for htlc in htlcs { requests.push(PackageTemplate::build_package( holder_commit_txid, htlc.transaction_output_index.unwrap(), diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 23a03efe177..c68fad838a3 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -608,6 +608,13 @@ impl HTLCOutputInCommitment { pub const fn to_bitcoin_amount(&self) -> Amount { Amount::from_sat(self.amount_msat / 1000) } + + pub(crate) fn partially_equal(&self, other: &HTLCOutputInCommitment) -> bool { + self.cltv_expiry == other.cltv_expiry && + self.amount_msat == other.amount_msat && + self.payment_hash == other.payment_hash && + self.offered == other.offered + } } impl_writeable_tlv_based!(HTLCOutputInCommitment, { @@ -1166,7 +1173,7 @@ impl_writeable_tlv_based!(HolderCommitmentTransaction, { impl HolderCommitmentTransaction { #[cfg(test)] - pub fn dummy(channel_value_satoshis: u64, htlcs: &mut Vec<(HTLCOutputInCommitment, ())>) -> Self { + pub fn dummy(channel_value_satoshis: u64, nondust_htlcs: Vec) -> Self { let secp_ctx = Secp256k1::new(); let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let dummy_sig = sign(&secp_ctx, &secp256k1::Message::from_digest([42; 32]), &SecretKey::from_slice(&[42; 32]).unwrap()); @@ -1189,11 +1196,10 @@ impl HolderCommitmentTransaction { channel_value_satoshis, }; let mut counterparty_htlc_sigs = Vec::new(); - for _ in 0..htlcs.len() { + for _ in 0..nondust_htlcs.len() { counterparty_htlc_sigs.push(dummy_sig); } - let inner = CommitmentTransaction::new_with_auxiliary_htlc_data(0, &dummy_key, 0, 0, 0, htlcs, &channel_parameters.as_counterparty_broadcastable(), &secp_ctx); - htlcs.sort_by_key(|htlc| htlc.0.transaction_output_index); + let inner = CommitmentTransaction::new(0, &dummy_key, 0, 0, 0, nondust_htlcs, &channel_parameters.as_counterparty_broadcastable(), &secp_ctx); HolderCommitmentTransaction { inner, counterparty_sig: dummy_sig, @@ -1492,23 +1498,16 @@ impl Readable for CommitmentTransaction { } impl CommitmentTransaction { - /// Construct an object of the class while assigning transaction output indices to HTLCs. - /// - /// Populates HTLCOutputInCommitment.transaction_output_index in htlcs_with_aux. - /// - /// The generic T allows the caller to match the HTLC output index with auxiliary data. - /// This auxiliary data is not stored in this object. + /// Construct an object of the class while assigning transaction output indices to HTLCs in `nondust_htlcs`. /// - /// Only include HTLCs that are above the dust limit for the channel. - /// - /// This is not exported to bindings users due to the generic though we likely should expose a version without - pub fn new_with_auxiliary_htlc_data(commitment_number: u64, per_commitment_point: &PublicKey, to_broadcaster_value_sat: u64, to_countersignatory_value_sat: u64, feerate_per_kw: u32, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters, secp_ctx: &Secp256k1) -> CommitmentTransaction { + /// `nondust_htlcs` should only include HTLCs that are above the dust limit for the channel. + pub fn new(commitment_number: u64, per_commitment_point: &PublicKey, to_broadcaster_value_sat: u64, to_countersignatory_value_sat: u64, feerate_per_kw: u32, nondust_htlcs: Vec, channel_parameters: &DirectedChannelTransactionParameters, secp_ctx: &Secp256k1) -> CommitmentTransaction { let to_broadcaster_value_sat = Amount::from_sat(to_broadcaster_value_sat); let to_countersignatory_value_sat = Amount::from_sat(to_countersignatory_value_sat); let keys = TxCreationKeys::from_channel_static_keys(per_commitment_point, channel_parameters.broadcaster_pubkeys(), channel_parameters.countersignatory_pubkeys(), secp_ctx); - // Sort outputs and populate output indices while keeping track of the auxiliary data - let (outputs, htlcs) = Self::internal_build_outputs(&keys, to_broadcaster_value_sat, to_countersignatory_value_sat, htlcs_with_aux, channel_parameters); + // Sort outputs and populate output indices + let (outputs, nondust_htlcs) = Self::build_outputs_and_htlcs(&keys, to_broadcaster_value_sat, to_countersignatory_value_sat, nondust_htlcs, channel_parameters); let (obscured_commitment_transaction_number, txins) = Self::internal_build_inputs(commitment_number, channel_parameters); let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs); @@ -1519,7 +1518,7 @@ impl CommitmentTransaction { to_countersignatory_value_sat, to_broadcaster_delay: Some(channel_parameters.contest_delay()), feerate_per_kw, - htlcs, + htlcs: nondust_htlcs, channel_type_features: channel_parameters.channel_type_features().clone(), keys, built: BuiltCommitmentTransaction { @@ -1540,8 +1539,8 @@ impl CommitmentTransaction { fn internal_rebuild_transaction(&self, keys: &TxCreationKeys, channel_parameters: &DirectedChannelTransactionParameters) -> BuiltCommitmentTransaction { let (obscured_commitment_transaction_number, txins) = Self::internal_build_inputs(self.commitment_number, channel_parameters); - let mut htlcs_with_aux = self.htlcs.iter().map(|h| (h.clone(), ())).collect(); - let (outputs, _) = Self::internal_build_outputs(keys, self.to_broadcaster_value_sat, self.to_countersignatory_value_sat, &mut htlcs_with_aux, channel_parameters); + let txout_htlc_pairs = Self::internal_build_outputs(keys, self.to_broadcaster_value_sat, self.to_countersignatory_value_sat, self.htlcs(), channel_parameters); + let outputs = txout_htlc_pairs.into_iter().map(|(output, _)| output).collect(); let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs); let txid = transaction.compute_txid(); @@ -1561,18 +1560,29 @@ impl CommitmentTransaction { } } - // This is used in two cases: - // - initial sorting of outputs / HTLCs in the constructor, in which case T is auxiliary data the - // caller needs to have sorted together with the HTLCs so it can keep track of the output index - // - building of a bitcoin transaction during a verify() call, in which case T is just () - fn internal_build_outputs(keys: &TxCreationKeys, to_broadcaster_value_sat: Amount, to_countersignatory_value_sat: Amount, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters) -> (Vec, Vec) { + fn build_outputs_and_htlcs(keys: &TxCreationKeys, to_broadcaster_value_sat: Amount, to_countersignatory_value_sat: Amount, mut nondust_htlcs: Vec, channel_parameters: &DirectedChannelTransactionParameters) -> (Vec, Vec) { + let mut txout_htlc_pairs: Vec<(TxOut, Option)> = Self::internal_build_outputs(keys, to_broadcaster_value_sat, to_countersignatory_value_sat, &nondust_htlcs, channel_parameters); + let mut outputs: Vec = Vec::with_capacity(txout_htlc_pairs.len()); + let mut nondust_htlcs_iter = nondust_htlcs.iter_mut(); + for (idx, out) in txout_htlc_pairs.drain(..).enumerate() { + if let Some(mut htlc) = out.1 { + htlc.transaction_output_index = Some(idx as u32); + *nondust_htlcs_iter.next().unwrap() = htlc; + } + outputs.push(out.0); + } + (outputs, nondust_htlcs) + } + + fn internal_build_outputs(keys: &TxCreationKeys, to_broadcaster_value_sat: Amount, to_countersignatory_value_sat: Amount, nondust_htlcs: &Vec, channel_parameters: &DirectedChannelTransactionParameters) -> Vec<(TxOut, Option)> { let countersignatory_payment_point = &channel_parameters.countersignatory_pubkeys().payment_point; let countersignatory_funding_key = &channel_parameters.countersignatory_pubkeys().funding_pubkey; let broadcaster_funding_key = &channel_parameters.broadcaster_pubkeys().funding_pubkey; let channel_type = channel_parameters.channel_type_features(); let contest_delay = channel_parameters.contest_delay(); - let mut txouts: Vec<(TxOut, Option<&mut HTLCOutputInCommitment>)> = Vec::new(); + // Assumes the max number of outputs on the transaction is equal to n_htlcs + to_local + to_remote + local_anchor + remote_anchor + let mut txout_htlc_pairs: Vec<(TxOut, Option)> = Vec::with_capacity(nondust_htlcs.len() + 4); if to_countersignatory_value_sat > Amount::ZERO { let script = if channel_type.supports_anchors_zero_fee_htlc_tx() { @@ -1580,9 +1590,9 @@ impl CommitmentTransaction { } else { ScriptBuf::new_p2wpkh(&Hash160::hash(&countersignatory_payment_point.serialize()).into()) }; - txouts.push(( + txout_htlc_pairs.push(( TxOut { - script_pubkey: script.clone(), + script_pubkey: script, value: to_countersignatory_value_sat, }, None, @@ -1595,7 +1605,7 @@ impl CommitmentTransaction { contest_delay, &keys.broadcaster_delayed_payment_key, ); - txouts.push(( + txout_htlc_pairs.push(( TxOut { script_pubkey: redeem_script.to_p2wsh(), value: to_broadcaster_value_sat, @@ -1605,9 +1615,9 @@ impl CommitmentTransaction { } if channel_type.supports_anchors_zero_fee_htlc_tx() { - if to_broadcaster_value_sat > Amount::ZERO || !htlcs_with_aux.is_empty() { + if to_broadcaster_value_sat > Amount::ZERO || !nondust_htlcs.is_empty() { let anchor_script = get_anchor_redeemscript(broadcaster_funding_key); - txouts.push(( + txout_htlc_pairs.push(( TxOut { script_pubkey: anchor_script.to_p2wsh(), value: Amount::from_sat(ANCHOR_OUTPUT_VALUE_SATOSHI), @@ -1616,9 +1626,9 @@ impl CommitmentTransaction { )); } - if to_countersignatory_value_sat > Amount::ZERO || !htlcs_with_aux.is_empty() { + if to_countersignatory_value_sat > Amount::ZERO || !nondust_htlcs.is_empty() { let anchor_script = get_anchor_redeemscript(countersignatory_funding_key); - txouts.push(( + txout_htlc_pairs.push(( TxOut { script_pubkey: anchor_script.to_p2wsh(), value: Amount::from_sat(ANCHOR_OUTPUT_VALUE_SATOSHI), @@ -1628,21 +1638,20 @@ impl CommitmentTransaction { } } - let mut htlcs = Vec::with_capacity(htlcs_with_aux.len()); - for (htlc, _) in htlcs_with_aux { + for htlc in nondust_htlcs { let script = get_htlc_redeemscript(htlc, channel_type, keys); let txout = TxOut { script_pubkey: script.to_p2wsh(), value: htlc.to_bitcoin_amount(), }; - txouts.push((txout, Some(htlc))); + txout_htlc_pairs.push((txout, Some(htlc.clone()))); } // Sort output in BIP-69 order (amount, scriptPubkey). Tie-breaks based on HTLC // CLTV expiration height. - sort_outputs(&mut txouts, |a, b| { - if let &Some(ref a_htlcout) = a { - if let &Some(ref b_htlcout) = b { + sort_outputs(&mut txout_htlc_pairs, |a, b| { + if let Some(a_htlcout) = a { + if let Some(b_htlcout) = b { a_htlcout.cltv_expiry.cmp(&b_htlcout.cltv_expiry) // Note that due to hash collisions, we have to have a fallback comparison // here for fuzzing mode (otherwise at least chanmon_fail_consistency @@ -1654,15 +1663,7 @@ impl CommitmentTransaction { } else { cmp::Ordering::Equal } }); - let mut outputs = Vec::with_capacity(txouts.len()); - for (idx, out) in txouts.drain(..).enumerate() { - if let Some(htlc) = out.1 { - htlc.transaction_output_index = Some(idx as u32); - htlcs.push(htlc.clone()); - } - outputs.push(out.0); - } - (outputs, htlcs) + txout_htlc_pairs } fn internal_build_inputs(commitment_number: u64, channel_parameters: &DirectedChannelTransactionParameters) -> (u64, Vec) { @@ -1973,7 +1974,7 @@ mod tests { commitment_number: u64, per_commitment_point: PublicKey, feerate_per_kw: u32, - htlcs_with_aux: Vec<(HTLCOutputInCommitment, ())>, + nondust_htlcs: Vec, channel_parameters: ChannelTransactionParameters, counterparty_pubkeys: ChannelPublicKeys, secp_ctx: Secp256k1::, @@ -2001,13 +2002,13 @@ mod tests { channel_type_features: ChannelTypeFeatures::only_static_remote_key(), channel_value_satoshis: 3000, }; - let htlcs_with_aux = Vec::new(); + let nondust_htlcs = Vec::new(); Self { commitment_number: 0, per_commitment_point, feerate_per_kw: 1, - htlcs_with_aux, + nondust_htlcs, channel_parameters, counterparty_pubkeys, secp_ctx, @@ -2015,9 +2016,9 @@ mod tests { } fn build(&mut self, to_broadcaster_sats: u64, to_countersignatory_sats: u64) -> CommitmentTransaction { - CommitmentTransaction::new_with_auxiliary_htlc_data( + CommitmentTransaction::new( self.commitment_number, &self.per_commitment_point, to_broadcaster_sats, to_countersignatory_sats, self.feerate_per_kw, - &mut self.htlcs_with_aux, &self.channel_parameters.as_holder_broadcastable(), &self.secp_ctx + self.nondust_htlcs.clone(), &self.channel_parameters.as_holder_broadcastable(), &self.secp_ctx ) } } @@ -2063,7 +2064,7 @@ mod tests { // Generate broadcaster output and received and offered HTLC outputs, w/o anchors builder.channel_parameters.channel_type_features = ChannelTypeFeatures::only_static_remote_key(); - builder.htlcs_with_aux = vec![(received_htlc.clone(), ()), (offered_htlc.clone(), ())]; + builder.nondust_htlcs = vec![received_htlc.clone(), offered_htlc.clone()]; let tx = builder.build(3000, 0); let keys = tx.trust().keys(); assert_eq!(tx.built.transaction.output.len(), 3); @@ -2076,7 +2077,7 @@ mod tests { // Generate broadcaster output and received and offered HTLC outputs, with anchors builder.channel_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); - builder.htlcs_with_aux = vec![(received_htlc.clone(), ()), (offered_htlc.clone(), ())]; + builder.nondust_htlcs = vec![received_htlc.clone(), offered_htlc.clone()]; let tx = builder.build(3000, 0); assert_eq!(tx.built.transaction.output.len(), 5); assert_eq!(tx.built.transaction.output[2].script_pubkey, get_htlc_redeemscript(&received_htlc, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), &keys).to_p2wsh()); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f586c669c1f..656208076cf 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -886,11 +886,11 @@ struct CommitmentData<'a> { } /// A struct gathering stats on a commitment transaction, either local or remote. -struct CommitmentStats { - tx: CommitmentTransaction, // the transaction info - total_fee_sat: u64, // the total fee included in the transaction - local_balance_msat: u64, // local balance before fees *not* considering dust limits - remote_balance_msat: u64, // remote balance before fees *not* considering dust limits +pub(crate) struct CommitmentStats { + pub(crate) tx: CommitmentTransaction, // the transaction info + pub(crate) total_fee_sat: u64, // the total fee included in the transaction + pub(crate) local_balance_msat: u64, // local balance before fees *not* considering dust limits + pub(crate) remote_balance_msat: u64, // remote balance before fees *not* considering dust limits } /// Used when calculating whether we or the remote can afford an additional HTLC. @@ -3629,13 +3629,13 @@ impl ChannelContext where SP::Target: SignerProvider { fn build_commitment_transaction(&self, funding: &FundingScope, commitment_number: u64, per_commitment_point: &PublicKey, local: bool, generated_by_local: bool, logger: &L) -> CommitmentData where L::Target: Logger { - let mut included_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::new(); let num_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len(); - let mut included_non_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(num_htlcs); + + // This will contain all the htlcs included on the commitment transaction due to their state, both dust, and non-dust. + // Non-dust htlcs will come first, with their output indices populated, then dust htlcs, with their output indices set to `None`. + let mut htlcs_in_tx: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(num_htlcs); let broadcaster_dust_limit_satoshis = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis }; - let mut remote_htlc_total_msat = 0; - let mut local_htlc_total_msat = 0; let mut value_to_self_msat_offset = 0; let mut feerate_per_kw = self.feerate_per_kw; @@ -3669,40 +3669,7 @@ impl ChannelContext where SP::Target: SignerProvider { } } - macro_rules! add_htlc_output { - ($htlc: expr, $outbound: expr, $source: expr, $state_name: expr) => { - if $outbound == local { // "offered HTLC output" - let htlc_in_tx = get_htlc_in_commitment!($htlc, true); - let htlc_tx_fee = if self.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - 0 - } else { - feerate_per_kw as u64 * htlc_timeout_tx_weight(self.get_channel_type()) / 1000 - }; - if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_satoshis + htlc_tx_fee { - log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, &$htlc.payment_hash, $htlc.amount_msat); - included_non_dust_htlcs.push((htlc_in_tx, $source)); - } else { - log_trace!(logger, " ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, &$htlc.payment_hash, $htlc.amount_msat); - included_dust_htlcs.push((htlc_in_tx, $source)); - } - } else { - let htlc_in_tx = get_htlc_in_commitment!($htlc, false); - let htlc_tx_fee = if self.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - 0 - } else { - feerate_per_kw as u64 * htlc_success_tx_weight(self.get_channel_type()) / 1000 - }; - if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_satoshis + htlc_tx_fee { - log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, &$htlc.payment_hash, $htlc.amount_msat); - included_non_dust_htlcs.push((htlc_in_tx, $source)); - } else { - log_trace!(logger, " ...including {} {} dust HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, &$htlc.payment_hash, $htlc.amount_msat); - included_dust_htlcs.push((htlc_in_tx, $source)); - } - } - } - } - + // Read the state of htlcs and determine their inclusion in the commitment transaction let mut inbound_htlc_preimages: Vec = Vec::new(); for ref htlc in self.pending_inbound_htlcs.iter() { @@ -3715,8 +3682,9 @@ impl ChannelContext where SP::Target: SignerProvider { }; if include { - add_htlc_output!(htlc, false, None, state_name); - remote_htlc_total_msat += htlc.amount_msat; + log_trace!(logger, " ...including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, &htlc.payment_hash, htlc.amount_msat, state_name); + let htlc = get_htlc_in_commitment!(htlc, !local); + htlcs_in_tx.push((htlc, None)); } else { log_trace!(logger, " ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, &htlc.payment_hash, htlc.amount_msat, state_name); match &htlc.state { @@ -3753,8 +3721,9 @@ impl ChannelContext where SP::Target: SignerProvider { } if include { - add_htlc_output!(htlc, true, Some(&htlc.source), state_name); - local_htlc_total_msat += htlc.amount_msat; + log_trace!(logger, " ...including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, &htlc.payment_hash, htlc.amount_msat, state_name); + let htlc_in_tx = get_htlc_in_commitment!(htlc, local); + htlcs_in_tx.push((htlc_in_tx, Some(&htlc.source))); } else { log_trace!(logger, " ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, &htlc.payment_hash, htlc.amount_msat, state_name); match htlc.state { @@ -3769,14 +3738,11 @@ impl ChannelContext where SP::Target: SignerProvider { } // TODO: When MSRV >= 1.66.0, use u64::checked_add_signed - let mut value_to_self_msat = u64::try_from(funding.value_to_self_msat as i64 + value_to_self_msat_offset).unwrap(); - // Note that in case they have several just-awaiting-last-RAA fulfills in-progress (ie - // AwaitingRemoteRevokeToRemove or AwaitingRemovedRemoteRevoke) we may have allowed them to - // "violate" their reserve value by couting those against it. Thus, we have to do checked subtraction - // as otherwise we can overflow. - let mut value_to_remote_msat = u64::checked_sub(funding.get_value_satoshis() * 1000, value_to_self_msat).unwrap(); - value_to_self_msat = u64::checked_sub(value_to_self_msat, local_htlc_total_msat).unwrap(); - value_to_remote_msat = u64::checked_sub(value_to_remote_msat, remote_htlc_total_msat).unwrap(); + let value_to_self_with_offset_msat = u64::try_from(funding.value_to_self_msat as i64 + value_to_self_msat_offset).unwrap(); + + use crate::sign::tx_builder::{TxBuilder, SpecTxBuilder}; + let stats = TxBuilder::build_commitment_transaction(&SpecTxBuilder {}, local, commitment_number, per_commitment_point, &funding.channel_transaction_parameters, &self.secp_ctx, + value_to_self_with_offset_msat, htlcs_in_tx.iter().map(|(htlc, _)| htlc).cloned().collect(), feerate_per_kw, broadcaster_dust_limit_satoshis, logger); #[cfg(debug_assertions)] { @@ -3787,62 +3753,36 @@ impl ChannelContext where SP::Target: SignerProvider { } else { funding.counterparty_max_commitment_tx_output.lock().unwrap() }; - debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat || value_to_self_msat / 1000 >= funding.counterparty_selected_channel_reserve_satoshis.unwrap()); - broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat); - debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat || value_to_remote_msat / 1000 >= funding.holder_selected_channel_reserve_satoshis); - broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat); - } - - let total_fee_sat = commit_tx_fee_sat(feerate_per_kw, included_non_dust_htlcs.len(), &funding.channel_transaction_parameters.channel_type_features); - let anchors_val = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; - let (value_to_self, value_to_remote) = if funding.is_outbound() { - ((value_to_self_msat / 1000).saturating_sub(anchors_val).saturating_sub(total_fee_sat), value_to_remote_msat / 1000) - } else { - (value_to_self_msat / 1000, (value_to_remote_msat / 1000).saturating_sub(anchors_val).saturating_sub(total_fee_sat)) - }; - - let mut to_broadcaster_value_sat = if local { value_to_self } else { value_to_remote }; - let mut to_countersignatory_value_sat = if local { value_to_remote } else { value_to_self }; - - if to_broadcaster_value_sat >= broadcaster_dust_limit_satoshis { - log_trace!(logger, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, to_broadcaster_value_sat); - } else { - to_broadcaster_value_sat = 0; + debug_assert!(broadcaster_max_commitment_tx_output.0 <= stats.local_balance_msat || stats.local_balance_msat / 1000 >= funding.counterparty_selected_channel_reserve_satoshis.unwrap()); + broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, stats.local_balance_msat); + debug_assert!(broadcaster_max_commitment_tx_output.1 <= stats.remote_balance_msat || stats.remote_balance_msat / 1000 >= funding.holder_selected_channel_reserve_satoshis); + broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, stats.remote_balance_msat); + } + + // This brute-force search is O(n^2) over ~1k HTLCs in the worst case. This case is very + // rare (non-existent?) at the moment. Feel free to optimize if LN channels start seeing an + // increasing number of HTLCs on their commitment transactions. + for nondust_htlc in stats.tx.htlcs() { + for (htlc, _) in htlcs_in_tx.iter_mut() { + if htlc.transaction_output_index.is_none() && nondust_htlc.partially_equal(htlc) { + htlc.transaction_output_index = nondust_htlc.transaction_output_index; + break; + } + } } - if to_countersignatory_value_sat >= broadcaster_dust_limit_satoshis { - log_trace!(logger, " ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, to_countersignatory_value_sat); - } else { - to_countersignatory_value_sat = 0; - } - - let channel_parameters = - if local { funding.channel_transaction_parameters.as_holder_broadcastable() } - else { funding.channel_transaction_parameters.as_counterparty_broadcastable() }; - let tx = CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number, - per_commitment_point, - to_broadcaster_value_sat, - to_countersignatory_value_sat, - feerate_per_kw, - &mut included_non_dust_htlcs, - &channel_parameters, - &self.secp_ctx, - ); - let mut htlcs_included = included_non_dust_htlcs; - // The unwrap is safe, because all non-dust HTLCs have been assigned an output index - htlcs_included.sort_unstable_by_key(|h| h.0.transaction_output_index.unwrap()); - htlcs_included.append(&mut included_dust_htlcs); - - let stats = CommitmentStats { - tx, - total_fee_sat, - local_balance_msat: value_to_self_msat, - remote_balance_msat: value_to_remote_msat, - }; + htlcs_in_tx.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, + (Some(_), None) => cmp::Ordering::Less, + (l, r) => cmp::Ord::cmp(&l, &r), + } + }); CommitmentData { stats, - htlcs_included, + htlcs_included: htlcs_in_tx, inbound_htlc_preimages, outbound_htlc_preimages, } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 8b88df247e8..f0d9c9803ba 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -26,7 +26,7 @@ use crate::ln::channel::{get_holder_selected_channel_reserve_satoshis, Channel, use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, RecipientOnionFields, BREAKDOWN_TIMEOUT, ENABLE_GOSSIP_TICKS, DISABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA}; use crate::ln::channel::{DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, ChannelError, MIN_CHAN_DUST_LIMIT_SATOSHIS}; use crate::ln::{chan_utils, onion_utils}; -use crate::ln::chan_utils::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, OFFERED_HTLC_SCRIPT_WEIGHT, htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment}; +use crate::ln::chan_utils::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, OFFERED_HTLC_SCRIPT_WEIGHT, htlc_success_tx_weight, htlc_timeout_tx_weight}; use crate::routing::gossip::{NetworkGraph, NetworkUpdate}; use crate::routing::router::{Path, PaymentParameters, Route, RouteHop, get_route, RouteParameters}; use crate::types::features::{ChannelFeatures, ChannelTypeFeatures, NodeFeatures}; @@ -743,14 +743,13 @@ pub fn test_update_fee_that_funder_cannot_afford() { let local_chan_lock = per_peer_state.get(&nodes[1].node.get_our_node_id()).unwrap().lock().unwrap(); let local_chan = local_chan_lock.channel_by_id.get(&chan.2).and_then(Channel::as_funded).unwrap(); let local_chan_signer = local_chan.get_signer(); - let mut htlcs: Vec<(HTLCOutputInCommitment, ())> = vec![]; - let commitment_tx = CommitmentTransaction::new_with_auxiliary_htlc_data( + let commitment_tx = CommitmentTransaction::new( INITIAL_COMMITMENT_NUMBER - 1, &remote_point, push_sats, channel_value - push_sats - commit_tx_fee_msat(non_buffer_feerate + 4, 0, &channel_type_features) / 1000, non_buffer_feerate + 4, - &mut htlcs, + Vec::new(), &local_chan.funding.channel_transaction_parameters.as_counterparty_broadcastable(), &secp_ctx, ); @@ -1463,7 +1462,7 @@ pub fn test_fee_spike_violation_fails_htlc() { // signature for the commitment_signed message. let local_chan_balance = 1313; - let accepted_htlc_info = chan_utils::HTLCOutputInCommitment { + let mut accepted_htlc_info = chan_utils::HTLCOutputInCommitment { offered: false, amount_msat: 3460001, cltv_expiry: htlc_cltv, @@ -1478,13 +1477,13 @@ pub fn test_fee_spike_violation_fails_htlc() { let local_chan_lock = per_peer_state.get(&nodes[1].node.get_our_node_id()).unwrap().lock().unwrap(); let local_chan = local_chan_lock.channel_by_id.get(&chan.2).and_then(Channel::as_funded).unwrap(); let local_chan_signer = local_chan.get_signer(); - let commitment_tx = CommitmentTransaction::new_with_auxiliary_htlc_data( + let commitment_tx = CommitmentTransaction::new( commitment_number, &remote_point, 95000, local_chan_balance, feerate_per_kw, - &mut vec![(accepted_htlc_info, ())], + vec![accepted_htlc_info], &local_chan.funding.channel_transaction_parameters.as_counterparty_broadcastable(), &secp_ctx, ); diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 0539a696852..a5408043a9d 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -76,6 +76,7 @@ pub(crate) mod type_resolver; pub mod ecdsa; #[cfg(taproot)] pub mod taproot; +pub(crate) mod tx_builder; /// Information about a spendable output to a P2WSH script. /// diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs new file mode 100644 index 00000000000..f706496ef5f --- /dev/null +++ b/lightning/src/sign/tx_builder.rs @@ -0,0 +1,183 @@ +//! Defines the `TxBuilder` trait, and the `SpecTxBuilder` type +#![allow(dead_code)] +#![allow(unused_variables)] + +use core::ops::Deref; + +use bitcoin::secp256k1::{self, PublicKey, Secp256k1}; + +use crate::ln::chan_utils::{ + self, ChannelTransactionParameters, CommitmentTransaction, HTLCOutputInCommitment, +}; +use crate::ln::channel::{self, CommitmentStats}; +use crate::prelude::*; +use crate::util::logger::Logger; + +/// A trait for types that can build commitment transactions, both for the holder, and the counterparty. +pub(crate) trait TxBuilder { + fn build_commitment_transaction( + &self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey, + channel_parameters: &ChannelTransactionParameters, secp_ctx: &Secp256k1, + value_to_self_with_offset_msat: u64, htlcs_in_tx: Vec, + feerate_per_kw: u32, broadcaster_dust_limit_satoshis: u64, logger: &L, + ) -> CommitmentStats + where + L::Target: Logger; +} + +/// A type that builds commitment transactions according to the Lightning Specification. +#[derive(Clone, Debug, Default)] +pub(crate) struct SpecTxBuilder {} + +impl TxBuilder for SpecTxBuilder { + fn build_commitment_transaction( + &self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey, + channel_parameters: &ChannelTransactionParameters, secp_ctx: &Secp256k1, + value_to_self_with_offset_msat: u64, mut htlcs_in_tx: Vec, + feerate_per_kw: u32, broadcaster_dust_limit_satoshis: u64, logger: &L, + ) -> CommitmentStats + where + L::Target: Logger, + { + let mut remote_htlc_total_msat = 0; + let mut local_htlc_total_msat = 0; + + let directed_parameters = if local { + channel_parameters.as_holder_broadcastable() + } else { + channel_parameters.as_counterparty_broadcastable() + }; + let channel_type = directed_parameters.channel_type_features(); + + // Trim dust htlcs + htlcs_in_tx.retain(|htlc| { + let outbound = local == htlc.offered; + if outbound { + local_htlc_total_msat += htlc.amount_msat; + } else { + remote_htlc_total_msat += htlc.amount_msat; + } + let htlc_tx_fee = if channel_type.supports_anchors_zero_fee_htlc_tx() { + 0 + } else { + feerate_per_kw as u64 + * if htlc.offered { + chan_utils::htlc_timeout_tx_weight(&channel_type) + } else { + chan_utils::htlc_success_tx_weight(&channel_type) + } / 1000 + }; + if htlc.amount_msat / 1000 >= broadcaster_dust_limit_satoshis + htlc_tx_fee { + log_trace!( + logger, + " ...creating output for {} non-dust HTLC (hash {}) with value {}", + if outbound { "outbound" } else { "inbound" }, + htlc.payment_hash, + htlc.amount_msat + ); + true + } else { + log_trace!( + logger, + " ...trimming {} HTLC (hash {}) with value {} due to dust limit", + if outbound { "outbound" } else { "inbound" }, + htlc.payment_hash, + htlc.amount_msat + ); + false + } + }); + + // Note that in case they have several just-awaiting-last-RAA fulfills in-progress (ie + // AwaitingRemoteRevokeToRemove or AwaitingRemovedRemoteRevoke) we may have allowed them to + // "violate" their reserve value by couting those against it. Thus, we have to do checked subtraction + // as otherwise we can overflow. + let mut value_to_remote_msat = u64::checked_sub( + channel_parameters.channel_value_satoshis * 1000, + value_to_self_with_offset_msat, + ) + .unwrap(); + let value_to_self_msat = + u64::checked_sub(value_to_self_with_offset_msat, local_htlc_total_msat).unwrap(); + value_to_remote_msat = + u64::checked_sub(value_to_remote_msat, remote_htlc_total_msat).unwrap(); + + let total_fee_sat = + chan_utils::commit_tx_fee_sat(feerate_per_kw, htlcs_in_tx.len(), &channel_type); + let anchors_val = if channel_type.supports_anchors_zero_fee_htlc_tx() { + channel::ANCHOR_OUTPUT_VALUE_SATOSHI * 2 + } else { + 0 + }; + let (value_to_self, value_to_remote) = if channel_parameters.is_outbound_from_holder { + ( + (value_to_self_msat / 1000) + .saturating_sub(anchors_val) + .saturating_sub(total_fee_sat), + value_to_remote_msat / 1000, + ) + } else { + ( + value_to_self_msat / 1000, + (value_to_remote_msat / 1000) + .saturating_sub(anchors_val) + .saturating_sub(total_fee_sat), + ) + }; + + let mut value_to_a = if local { value_to_self } else { value_to_remote }; + let mut value_to_b = if local { value_to_remote } else { value_to_self }; + + if value_to_a >= broadcaster_dust_limit_satoshis { + log_trace!( + logger, + " ...creating {} output with value {}", + if local { "to_local" } else { "to_remote" }, + value_to_a + ); + } else { + log_trace!( + logger, + " ...trimming {} output with value {} due to dust limit", + if local { "to_local" } else { "to_remote" }, + value_to_a + ); + value_to_a = 0; + } + + if value_to_b >= broadcaster_dust_limit_satoshis { + log_trace!( + logger, + " ...creating {} output with value {}", + if local { "to_remote" } else { "to_local" }, + value_to_b + ); + } else { + log_trace!( + logger, + " ...trimming {} output with value {} due to dust limit", + if local { "to_remote" } else { "to_local" }, + value_to_b + ); + value_to_b = 0; + } + + let tx = CommitmentTransaction::new( + commitment_number, + per_commitment_point, + value_to_a, + value_to_b, + feerate_per_kw, + htlcs_in_tx, + &directed_parameters, + secp_ctx, + ); + + CommitmentStats { + tx, + total_fee_sat, + local_balance_msat: value_to_self_msat, + remote_balance_msat: value_to_remote_msat, + } + } +}