diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index b06a13aea9a..556f47de7f2 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -884,11 +884,11 @@ struct CommitmentData<'a> { } /// An enum gathering stats on 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. @@ -3425,14 +3425,7 @@ impl ChannelContext where SP::Target: SignerProvider { // 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); - // We allocate this vector because we need to count the number of non-dust htlcs and calculate the total fee of the transaction - // before calling `CommitmentTransaction::new`. - // We could drop this vector and create two iterators: one to count the number of non-dust htlcs, and another to pass to `CommitmentTransaction::new` - let mut included_non_dust_htlcs: Vec<&mut HTLCOutputInCommitment> = 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; @@ -3532,94 +3525,12 @@ impl ChannelContext where SP::Target: SignerProvider { } } - // Trim dust htlcs - for (htlc, _) in htlcs_in_tx.iter_mut() { - if htlc.offered { - let outbound = local; - if outbound { - local_htlc_total_msat += htlc.amount_msat; - } else { - remote_htlc_total_msat += htlc.amount_msat; - } - 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, " ...creating output for {} non-dust HTLC (hash {}) with value {}", if outbound { "outbound" } else { "inbound" }, htlc.payment_hash, htlc.amount_msat); - included_non_dust_htlcs.push(htlc); - } else { - log_trace!(logger, " ...trimming {} HTLC (hash {}) with value {} due to dust limit", if outbound { "outbound" } else { "inbound" }, htlc.payment_hash, htlc.amount_msat); - } - } else { - let outbound = !local; - if outbound { - local_htlc_total_msat += htlc.amount_msat; - } else { - remote_htlc_total_msat += htlc.amount_msat; - } - 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, " ...creating output for {} non-dust HTLC (hash {}) with value {}", if outbound { "outbound" } else { "inbound" }, htlc.payment_hash, htlc.amount_msat); - included_non_dust_htlcs.push(htlc); - } else { - log_trace!(logger, " ...trimming {} HTLC (hash {}) with value {} due to dust limit", if outbound { "outbound" } else { "inbound" }, htlc.payment_hash, htlc.amount_msat); - } - } - } - // 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.channel_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 total_fee_sat = commit_tx_fee_sat(feerate_per_kw, included_non_dust_htlcs.len(), &self.channel_transaction_parameters.channel_type_features); - let anchors_val = if self.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 self.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 value_to_self_with_offset_msat = u64::try_from(funding.value_to_self_msat as i64 + value_to_self_msat_offset).unwrap(); - 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 channel_parameters = - if local { self.channel_transaction_parameters.as_holder_broadcastable() } - else { self.channel_transaction_parameters.as_counterparty_broadcastable() }; - let tx = CommitmentTransaction::new(commitment_number, - &per_commitment_point, - value_to_a, - value_to_b, - feerate_per_kw, - included_non_dust_htlcs.into_iter(), - &channel_parameters, - &self.secp_ctx, - ); + use crate::sign::tx_builder::{TxBuilder, SpecTxBuilder}; + let stats = TxBuilder::build_commitment_transaction(&SpecTxBuilder {}, local, commitment_number, per_commitment_point, &self.channel_transaction_parameters, &self.secp_ctx, + funding.channel_value_satoshis, value_to_self_with_offset_msat, htlcs_in_tx.iter_mut().map(|(htlc, _)| htlc), feerate_per_kw, broadcaster_dust_limit_satoshis, logger); #[cfg(debug_assertions)] { @@ -3630,10 +3541,10 @@ 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); + 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); } htlcs_in_tx.sort_unstable_by(|(htlc_a, _), (htlc_b, _)| { @@ -3645,13 +3556,6 @@ impl ChannelContext where SP::Target: SignerProvider { } }); - let stats = CommitmentStats { - tx, - total_fee_sat, - local_balance_msat: value_to_self_msat, - remote_balance_msat: value_to_remote_msat, - }; - CommitmentData { stats, htlcs_included: htlcs_in_tx, diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 3ba950f548d..08a5db8ec86 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -81,6 +81,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..f8f2e0bf481 --- /dev/null +++ b/lightning/src/sign/tx_builder.rs @@ -0,0 +1,221 @@ +//! 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 { + /// Build a commitment transaction, and populate the elements of `htlcs` with their output indices. + fn build_commitment_transaction<'a, L: Deref>( + &self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey, + channel_transaction_parameters: &ChannelTransactionParameters, + secp_ctx: &Secp256k1, channel_value_satoshis: u64, + value_to_self_with_offset_msat: u64, + htlcs_in_tx: impl Iterator, 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<'a, L: Deref>( + &self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey, + channel_transaction_parameters: &ChannelTransactionParameters, + secp_ctx: &Secp256k1, channel_value_satoshis: u64, + value_to_self_with_offset_msat: u64, + htlcs_in_tx: impl Iterator, feerate_per_kw: u32, + broadcaster_dust_limit_satoshis: u64, logger: &L, + ) -> CommitmentStats + where + L::Target: Logger, + { + // We allocate this vector because we need to count the number of non-dust htlcs and calculate the total fee of the transaction + // before calling `CommitmentTransaction::new`. + // We could drop this vector and create two iterators: one to count the number of non-dust htlcs, and another to pass to `CommitmentTransaction::new` + let mut included_non_dust_htlcs: Vec<&mut HTLCOutputInCommitment> = Vec::with_capacity(htlcs_in_tx.size_hint().0); + + let mut remote_htlc_total_msat = 0; + let mut local_htlc_total_msat = 0; + + let channel_parameters = if local { + channel_transaction_parameters.as_holder_broadcastable() + } else { + channel_transaction_parameters.as_counterparty_broadcastable() + }; + let channel_type = channel_parameters.channel_type_features(); + + // Trim dust htlcs + for htlc in htlcs_in_tx { + if htlc.offered { + let outbound = local; + 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 * chan_utils::htlc_timeout_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 + ); + included_non_dust_htlcs.push(htlc); + } else { + log_trace!( + logger, + " ...trimming {} HTLC (hash {}) with value {} due to dust limit", + if outbound { "outbound" } else { "inbound" }, + htlc.payment_hash, + htlc.amount_msat + ); + } + } else { + let outbound = !local; + 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 * 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 + ); + included_non_dust_htlcs.push(htlc); + } else { + log_trace!( + logger, + " ...trimming {} HTLC (hash {}) with value {} due to dust limit", + if outbound { "outbound" } else { "inbound" }, + htlc.payment_hash, + htlc.amount_msat + ); + } + } + } + + // 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_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, + included_non_dust_htlcs.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_transaction_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, + included_non_dust_htlcs.into_iter(), + &channel_parameters, + &secp_ctx, + ); + + CommitmentStats { + tx, + total_fee_sat, + local_balance_msat: value_to_self_msat, + remote_balance_msat: value_to_remote_msat, + } + } +}