diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1fb712e0856..b973e7eeb6b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -885,11 +885,11 @@ struct CommitmentData<'a> { } /// A struct 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. @@ -3478,8 +3478,6 @@ impl ChannelContext where SP::Target: SignerProvider { 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; @@ -3579,93 +3577,12 @@ impl ChannelContext where SP::Target: SignerProvider { } } - // Trim dust htlcs - let mut included_non_dust_htlcs: Vec<_> = htlcs_in_tx.iter_mut().map(|(htlc, _)| htlc).collect(); - included_non_dust_htlcs.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 self.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - 0 - } else { - feerate_per_kw as u64 - * if htlc.offered { - chan_utils::htlc_timeout_tx_weight(self.get_channel_type()) - } else { - chan_utils::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 - ); - 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 - } - }); - // 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 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 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 { funding.channel_transaction_parameters.as_holder_broadcastable() } - else { funding.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, - &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, &funding.channel_transaction_parameters, &self.secp_ctx, + funding.get_value_satoshis(), value_to_self_with_offset_msat, htlcs_in_tx.iter_mut().map(|(htlc, _)| htlc).collect(), feerate_per_kw, broadcaster_dust_limit_satoshis, logger); #[cfg(debug_assertions)] { @@ -3676,10 +3593,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, _)| { @@ -3691,13 +3608,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 9f04968198a..6db42f230c3 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..7e67c9ee3a6 --- /dev/null +++ b/lightning/src/sign/tx_builder.rs @@ -0,0 +1,186 @@ +//! 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_in_tx` with their output indices. + /// Leave the output index of dust htlcs set to `None`. + fn build_commitment_transaction( + &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: Vec<&mut HTLCOutputInCommitment>, + 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_transaction_parameters: &ChannelTransactionParameters, + secp_ctx: &Secp256k1, channel_value_satoshis: u64, + value_to_self_with_offset_msat: u64, mut htlcs_in_tx: Vec<&mut HTLCOutputInCommitment>, + 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 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 + 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_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_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, + htlcs_in_tx, + &channel_parameters, + secp_ctx, + ); + + CommitmentStats { + tx, + total_fee_sat, + local_balance_msat: value_to_self_msat, + remote_balance_msat: value_to_remote_msat, + } + } +}