@@ -56,6 +56,7 @@ use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBounde
5656use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS};
5757use crate::chain::transaction::{OutPoint, TransactionData};
5858use crate::sign::ecdsa::EcdsaChannelSigner;
59+ use crate::sign::tx_builder::{SpecTxBuilder, TxBuilder};
5960use crate::sign::{EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient};
6061use crate::events::{ClosureReason, Event};
6162use crate::events::bump_transaction::BASE_INPUT_WEIGHT;
@@ -265,24 +266,6 @@ struct InboundHTLCOutput {
265266 state: InboundHTLCState,
266267}
267268
268- impl InboundHTLCOutput {
269- fn is_dust(&self, local: bool, feerate_per_kw: u32, broadcaster_dust_limit_sat: u64, features: &ChannelTypeFeatures) -> bool {
270- let htlc_tx_fee_sat = if features.supports_anchors_zero_fee_htlc_tx() {
271- 0
272- } else {
273- let htlc_tx_weight = if !local {
274- // this is an offered htlc
275- htlc_timeout_tx_weight(features)
276- } else {
277- htlc_success_tx_weight(features)
278- };
279- // As required by the spec, round down
280- feerate_per_kw as u64 * htlc_tx_weight / 1000
281- };
282- self.amount_msat / 1000 < broadcaster_dust_limit_sat + htlc_tx_fee_sat
283- }
284- }
285-
286269#[cfg_attr(test, derive(Clone, Debug, PartialEq))]
287270enum OutboundHTLCState {
288271 /// Added by us and included in a commitment_signed (if we were AwaitingRemoteRevoke when we
@@ -405,24 +388,6 @@ struct OutboundHTLCOutput {
405388 send_timestamp: Option<Duration>,
406389}
407390
408- impl OutboundHTLCOutput {
409- fn is_dust(&self, local: bool, feerate_per_kw: u32, broadcaster_dust_limit_sat: u64, features: &ChannelTypeFeatures) -> bool {
410- let htlc_tx_fee_sat = if features.supports_anchors_zero_fee_htlc_tx() {
411- 0
412- } else {
413- let htlc_tx_weight = if local {
414- // this is an offered htlc
415- htlc_timeout_tx_weight(features)
416- } else {
417- htlc_success_tx_weight(features)
418- };
419- // As required by the spec, round down
420- feerate_per_kw as u64 * htlc_tx_weight / 1000
421- };
422- self.amount_msat / 1000 < broadcaster_dust_limit_sat + htlc_tx_fee_sat
423- }
424- }
425-
426391/// See AwaitingRemoteRevoke ChannelState for more info
427392#[cfg_attr(test, derive(Clone, Debug, PartialEq))]
428393enum HTLCUpdateAwaitingACK {
@@ -985,13 +950,13 @@ struct CommitmentData<'a> {
985950}
986951
987952/// A struct gathering stats on a commitment transaction, either local or remote.
988- struct CommitmentStats {
989- feerate_per_kw: u32, // the feerate of the commitment transaction
990- total_fee_sat: u64, // the total fee included in the transaction
991- total_anchors_sat: u64, // the sum of the anchors' amounts
992- broadcaster_dust_limit_sat: u64, // the broadcaster's dust limit
993- local_balance_before_fee_anchors_msat: u64, // local balance before fees and anchors *not* considering dust limits
994- remote_balance_before_fee_anchors_msat: u64, // remote balance before fees and anchors *not* considering dust limits
953+ pub(crate) struct CommitmentStats {
954+ pub(crate) feerate_per_kw: u32, // the feerate of the commitment transaction
955+ pub(crate) total_fee_sat: u64, // the total fee included in the transaction
956+ pub(crate) total_anchors_sat: u64, // the sum of the anchors' amounts
957+ pub(crate) broadcaster_dust_limit_sat: u64, // the broadcaster's dust limit
958+ pub(crate) local_balance_before_fee_anchors_msat: u64, // local balance before fees and anchors *not* considering dust limits
959+ pub(crate) remote_balance_before_fee_anchors_msat: u64, // remote balance before fees and anchors *not* considering dust limits
995960}
996961
997962/// Used when calculating whether we or the remote can afford an additional HTLC.
@@ -3850,16 +3815,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38503815 })
38513816 }
38523817
3853- /// Builds stats on a potential commitment transaction build, without actually building the
3854- /// commitment transaction. See `build_commitment_transaction` for further docs.
3855- #[inline]
3856- fn build_commitment_stats(&self, funding: &FundingScope, local: bool, generated_by_local: bool) -> CommitmentStats {
3857- let broadcaster_dust_limit_sat = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis };
3858- let mut non_dust_htlc_count = 0;
3859- let mut remote_htlc_total_msat = 0;
3860- let mut local_htlc_total_msat = 0;
3861- let mut value_to_self_msat_offset = 0;
3862-
3818+ fn get_commitment_feerate(&self, funding: &FundingScope, generated_by_local: bool) -> u32 {
38633819 let mut feerate_per_kw = self.feerate_per_kw;
38643820 if let Some((feerate, update_state)) = self.pending_update_fee {
38653821 if match update_state {
@@ -3872,13 +3828,36 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38723828 feerate_per_kw = feerate;
38733829 }
38743830 }
3831+ feerate_per_kw
3832+ }
3833+
3834+ /// Builds stats on a potential commitment transaction build, without actually building the
3835+ /// commitment transaction. See `build_commitment_transaction` for further docs.
3836+ #[inline]
3837+ #[allow(dead_code)]
3838+ fn build_commitment_stats(&self, funding: &FundingScope, local: bool, generated_by_local: bool) -> CommitmentStats {
3839+ let broadcaster_dust_limit_sat = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis };
3840+ let feerate_per_kw = self.get_commitment_feerate(funding, generated_by_local);
3841+
3842+ let num_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len();
3843+ let mut htlcs_included = Vec::with_capacity(num_htlcs);
3844+ let mut value_to_self_msat_offset = 0;
3845+
3846+ macro_rules! get_htlc_in_commitment {
3847+ ($htlc: expr, $offered: expr) => {
3848+ HTLCOutputInCommitment {
3849+ offered: $offered,
3850+ amount_msat: $htlc.amount_msat,
3851+ cltv_expiry: $htlc.cltv_expiry,
3852+ payment_hash: $htlc.payment_hash,
3853+ transaction_output_index: None
3854+ }
3855+ }
3856+ }
38753857
38763858 for htlc in self.pending_inbound_htlcs.iter() {
38773859 if htlc.state.included_in_commitment(generated_by_local) {
3878- if !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, funding.get_channel_type()) {
3879- non_dust_htlc_count += 1;
3880- }
3881- remote_htlc_total_msat += htlc.amount_msat;
3860+ htlcs_included.push(get_htlc_in_commitment!(htlc, !local));
38823861 } else {
38833862 if htlc.state.preimage().is_some() {
38843863 value_to_self_msat_offset += htlc.amount_msat as i64;
@@ -3888,10 +3867,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38883867
38893868 for htlc in self.pending_outbound_htlcs.iter() {
38903869 if htlc.state.included_in_commitment(generated_by_local) {
3891- if !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, funding.get_channel_type()) {
3892- non_dust_htlc_count += 1;
3893- }
3894- local_htlc_total_msat += htlc.amount_msat;
3870+ htlcs_included.push(get_htlc_in_commitment!(htlc, local));
38953871 } else {
38963872 if htlc.state.preimage().is_some() {
38973873 value_to_self_msat_offset -= htlc.amount_msat as i64;
@@ -3901,15 +3877,21 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
39013877
39023878 // # Panics
39033879 //
3904- // While we expect `value_to_self_msat_offset` to be negative in some cases, the value going
3905- // to each party MUST be 0 or positive, even if all HTLCs pending in the commitment clear by
3906- // failure.
3880+ // While we expect `value_to_self_msat_offset` to be negative in some cases, the local
3881+ // balance MUST remain greater than or equal to 0.
39073882
39083883 // TODO: When MSRV >= 1.66.0, use u64::checked_add_signed
3909- let mut value_to_self_msat = (funding.value_to_self_msat as i64 + value_to_self_msat_offset).try_into().unwrap();
3910- let mut value_to_remote_msat = (funding.get_value_satoshis() * 1000).checked_sub(value_to_self_msat).unwrap();
3911- value_to_self_msat = value_to_self_msat.checked_sub(local_htlc_total_msat).unwrap();
3912- value_to_remote_msat = value_to_remote_msat.checked_sub(remote_htlc_total_msat).unwrap();
3884+ let value_to_self_with_offset_msat = (funding.value_to_self_msat as i64 + value_to_self_msat_offset).try_into().unwrap();
3885+
3886+ let stats = TxBuilder::build_commitment_stats(
3887+ &SpecTxBuilder {},
3888+ local,
3889+ &funding.channel_transaction_parameters,
3890+ value_to_self_with_offset_msat,
3891+ &htlcs_included,
3892+ feerate_per_kw,
3893+ broadcaster_dust_limit_sat,
3894+ );
39133895
39143896 #[cfg(debug_assertions)]
39153897 {
@@ -3920,23 +3902,13 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
39203902 } else {
39213903 funding.counterparty_max_commitment_tx_output.lock().unwrap()
39223904 };
3923- debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat || value_to_self_msat / 1000 >= funding.counterparty_selected_channel_reserve_satoshis.unwrap());
3924- broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat );
3925- debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat || value_to_remote_msat / 1000 >= funding.holder_selected_channel_reserve_satoshis);
3926- broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat );
3905+ debug_assert!(broadcaster_max_commitment_tx_output.0 <= stats.local_balance_before_fee_anchors_msat || stats.local_balance_before_fee_anchors_msat / 1000 >= funding.counterparty_selected_channel_reserve_satoshis.unwrap());
3906+ broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, stats.local_balance_before_fee_anchors_msat );
3907+ debug_assert!(broadcaster_max_commitment_tx_output.1 <= stats.remote_balance_before_fee_anchors_msat || stats.remote_balance_before_fee_anchors_msat / 1000 >= funding.holder_selected_channel_reserve_satoshis);
3908+ broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, stats.remote_balance_before_fee_anchors_msat );
39273909 }
39283910
3929- let total_fee_sat = commit_tx_fee_sat(feerate_per_kw, non_dust_htlc_count, &funding.channel_transaction_parameters.channel_type_features);
3930- let total_anchors_sat = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 };
3931-
3932- CommitmentStats {
3933- feerate_per_kw,
3934- total_fee_sat,
3935- total_anchors_sat,
3936- broadcaster_dust_limit_sat,
3937- local_balance_before_fee_anchors_msat: value_to_self_msat,
3938- remote_balance_before_fee_anchors_msat: value_to_remote_msat,
3939- }
3911+ stats
39403912 }
39413913
39423914 /// Transaction nomenclature is somewhat confusing here as there are many different cases - a
@@ -3956,19 +3928,12 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
39563928 fn build_commitment_transaction<L: Deref>(&self, funding: &FundingScope, commitment_number: u64, per_commitment_point: &PublicKey, local: bool, generated_by_local: bool, logger: &L) -> CommitmentData
39573929 where L::Target: Logger
39583930 {
3959- let stats = self.build_commitment_stats(funding, local, generated_by_local);
3960- let CommitmentStats {
3961- feerate_per_kw,
3962- total_fee_sat,
3963- total_anchors_sat,
3964- broadcaster_dust_limit_sat,
3965- local_balance_before_fee_anchors_msat,
3966- remote_balance_before_fee_anchors_msat
3967- } = stats;
3931+ let broadcaster_dust_limit_sat = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis };
3932+ let feerate_per_kw = self.get_commitment_feerate(funding, generated_by_local);
39683933
39693934 let num_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len();
39703935 let mut htlcs_included: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(num_htlcs);
3971- let mut nondust_htlcs: Vec<HTLCOutputInCommitment> = Vec::with_capacity(num_htlcs) ;
3936+ let mut value_to_self_msat_offset = 0 ;
39723937
39733938 log_trace!(logger, "Building commitment transaction number {} (really {} xor {}) for channel {} for {}, generated by {} with fee {}...",
39743939 commitment_number, (INITIAL_COMMITMENT_NUMBER - commitment_number),
@@ -3991,13 +3956,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
39913956 macro_rules! add_htlc_output {
39923957 ($htlc: expr, $outbound: expr, $source: expr) => {
39933958 let htlc_in_tx = get_htlc_in_commitment!($htlc, $outbound == local);
3994- htlcs_included.push((htlc_in_tx.clone(), $source));
3995- if $htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, funding.get_channel_type()) {
3996- log_trace!(logger, " ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $htlc.state, $htlc.htlc_id, $htlc.payment_hash, $htlc.amount_msat);
3997- } else {
3998- log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $htlc.state, $htlc.htlc_id, $htlc.payment_hash, $htlc.amount_msat);
3999- nondust_htlcs.push(htlc_in_tx);
4000- }
3959+ htlcs_included.push((htlc_in_tx, $source));
40013960 }
40023961 }
40033962
@@ -4006,11 +3965,13 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
40063965
40073966 for htlc in self.pending_inbound_htlcs.iter() {
40083967 if htlc.state.included_in_commitment(generated_by_local) {
3968+ log_trace!(logger, " ...including inbound {} HTLC {} (hash {}) with value {}", htlc.state, htlc.htlc_id, htlc.payment_hash, htlc.amount_msat);
40093969 add_htlc_output!(htlc, false, None);
40103970 } else {
40113971 log_trace!(logger, " ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state);
40123972 if let Some(preimage) = htlc.state.preimage() {
40133973 inbound_htlc_preimages.push(preimage);
3974+ value_to_self_msat_offset += htlc.amount_msat as i64;
40143975 }
40153976 }
40163977 };
@@ -4020,54 +3981,53 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
40203981 outbound_htlc_preimages.push(preimage);
40213982 }
40223983 if htlc.state.included_in_commitment(generated_by_local) {
3984+ log_trace!(logger, " ...including outbound {} HTLC {} (hash {}) with value {}", htlc.state, htlc.htlc_id, htlc.payment_hash, htlc.amount_msat);
40233985 add_htlc_output!(htlc, true, Some(&htlc.source));
40243986 } else {
40253987 log_trace!(logger, " ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state);
3988+ if htlc.state.preimage().is_some() {
3989+ value_to_self_msat_offset -= htlc.amount_msat as i64;
3990+ }
40263991 }
40273992 };
40283993
4029- // We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater
4030- // than or equal to the sum of `total_fee_sat` and `total_anchors_sat`.
3994+ // # Panics
40313995 //
4032- // This is because when the remote party sends an `update_fee` message, we build the new
4033- // commitment transaction *before* checking whether the remote party's balance is enough to
4034- // cover the total fee and the anchors.
4035-
4036- let (value_to_self, value_to_remote) = if funding.is_outbound() {
4037- ((local_balance_before_fee_anchors_msat / 1000).saturating_sub(total_anchors_sat).saturating_sub(total_fee_sat), remote_balance_before_fee_anchors_msat / 1000)
4038- } else {
4039- (local_balance_before_fee_anchors_msat / 1000, (remote_balance_before_fee_anchors_msat / 1000).saturating_sub(total_anchors_sat).saturating_sub(total_fee_sat))
4040- };
3996+ // While we expect `value_to_self_msat_offset` to be negative in some cases, the local
3997+ // balance MUST remain greater than or equal to 0.
40413998
4042- let mut to_broadcaster_value_sat = if local { value_to_self } else { value_to_remote };
4043- let mut to_countersignatory_value_sat = if local { value_to_remote } else { value_to_self };
4044-
4045- if to_broadcaster_value_sat >= broadcaster_dust_limit_sat {
4046- log_trace!(logger, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, to_broadcaster_value_sat);
4047- } else {
4048- to_broadcaster_value_sat = 0;
4049- }
4050-
4051- if to_countersignatory_value_sat >= broadcaster_dust_limit_sat {
4052- log_trace!(logger, " ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, to_countersignatory_value_sat);
4053- } else {
4054- to_countersignatory_value_sat = 0;
4055- }
3999+ // TODO: When MSRV >= 1.66.0, use u64::checked_add_signed
4000+ let value_to_self_with_offset_msat = (funding.value_to_self_msat as i64 + value_to_self_msat_offset).try_into().unwrap();
40564001
4057- let channel_parameters =
4058- if local { funding.channel_transaction_parameters.as_holder_broadcastable() }
4059- else { funding.channel_transaction_parameters.as_counterparty_broadcastable() };
4060- let tx = CommitmentTransaction::new(
4002+ let (tx, stats) = TxBuilder::build_commitment_transaction(
4003+ &SpecTxBuilder {},
4004+ local,
40614005 commitment_number,
40624006 per_commitment_point,
4063- to_broadcaster_value_sat,
4064- to_countersignatory_value_sat,
4065- feerate_per_kw,
4066- nondust_htlcs,
4067- &channel_parameters,
4007+ &funding.channel_transaction_parameters,
40684008 &self.secp_ctx,
4009+ value_to_self_with_offset_msat,
4010+ htlcs_included.iter().map(|(htlc, _source)| htlc).cloned().collect(),
4011+ feerate_per_kw,
4012+ broadcaster_dust_limit_sat,
4013+ logger,
40694014 );
40704015
4016+ #[cfg(debug_assertions)]
4017+ {
4018+ // Make sure that the to_self/to_remote is always either past the appropriate
4019+ // channel_reserve *or* it is making progress towards it.
4020+ let mut broadcaster_max_commitment_tx_output = if generated_by_local {
4021+ funding.holder_max_commitment_tx_output.lock().unwrap()
4022+ } else {
4023+ funding.counterparty_max_commitment_tx_output.lock().unwrap()
4024+ };
4025+ debug_assert!(broadcaster_max_commitment_tx_output.0 <= stats.local_balance_before_fee_anchors_msat || stats.local_balance_before_fee_anchors_msat / 1000 >= funding.counterparty_selected_channel_reserve_satoshis.unwrap());
4026+ broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, stats.local_balance_before_fee_anchors_msat);
4027+ debug_assert!(broadcaster_max_commitment_tx_output.1 <= stats.remote_balance_before_fee_anchors_msat || stats.remote_balance_before_fee_anchors_msat / 1000 >= funding.holder_selected_channel_reserve_satoshis);
4028+ broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, stats.remote_balance_before_fee_anchors_msat);
4029+ }
4030+
40714031 // This populates the HTLC-source table with the indices from the HTLCs in the commitment
40724032 // transaction.
40734033 //
0 commit comments