diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 557a988cc92..535649f7895 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -236,7 +236,7 @@ pub(crate) fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, channel_t } /// Returns the fees for success and timeout second stage HTLC transactions. -pub(super) fn second_stage_tx_fees_sat( +pub(crate) fn second_stage_tx_fees_sat( channel_type: &ChannelTypeFeatures, feerate_sat_per_1000_weight: u32, ) -> (u64, u64) { if channel_type.supports_anchors_zero_fee_htlc_tx() diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index b5b77972a6c..c949ff2985a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -70,7 +70,7 @@ use crate::ln::script::{self, ShutdownScript}; use crate::ln::types::ChannelId; use crate::routing::gossip::NodeId; use crate::sign::ecdsa::EcdsaChannelSigner; -use crate::sign::tx_builder::{SpecTxBuilder, TxBuilder}; +use crate::sign::tx_builder::{HTLCAmountDirection, NextCommitmentStats, SpecTxBuilder, TxBuilder}; use crate::sign::{ChannelSigner, EntropySource, NodeSigner, Recipient, SignerProvider}; use crate::types::features::{ChannelTypeFeatures, InitFeatures}; use crate::types::payment::{PaymentHash, PaymentPreimage}; @@ -1097,12 +1097,12 @@ pub enum AnnouncementSigsState { /// An enum indicating whether the local or remote side offered a given HTLC. enum HTLCInitiator { LocalOffered, + #[allow(dead_code)] RemoteOffered, } /// Current counts of various HTLCs, useful for calculating current balances available exactly. struct HTLCStats { - pending_inbound_htlcs: usize, pending_outbound_htlcs: usize, pending_inbound_htlcs_value_msat: u64, pending_outbound_htlcs_value_msat: u64, @@ -1112,8 +1112,6 @@ struct HTLCStats { // htlc on the counterparty's commitment transaction. extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat: Option, on_holder_tx_dust_exposure_msat: u64, - outbound_holding_cell_msat: u64, - on_holder_tx_outbound_holding_cell_htlcs_count: u32, // dust HTLCs *non*-included } /// A struct gathering data on a commitment, either local or remote. @@ -1978,14 +1976,13 @@ pub(super) struct FundingScope { /// Max to_local and to_remote outputs in a remote-generated commitment transaction counterparty_max_commitment_tx_output: Mutex<(u64, u64)>, - // We save these values so we can make sure `next_local_commit_tx_fee_msat` and - // `next_remote_commit_tx_fee_msat` properly predict what the next commitment transaction fee will - // be, by comparing the cached values to the fee of the transaction generated by - // `build_commitment_transaction`. + // We save these values so we can make sure validation of channel updates properly predicts + // what the next commitment transaction fee will be, by comparing the cached values to the + // fee of the transaction generated by `build_commitment_transaction`. #[cfg(any(test, fuzzing))] - next_local_commitment_tx_fee_info_cached: Mutex>, + next_local_fee: Mutex, #[cfg(any(test, fuzzing))] - next_remote_commitment_tx_fee_info_cached: Mutex>, + next_remote_fee: Mutex, pub(super) channel_transaction_parameters: ChannelTransactionParameters, @@ -2059,9 +2056,9 @@ impl Readable for FundingScope { short_channel_id, minimum_depth_override, #[cfg(any(test, fuzzing))] - next_local_commitment_tx_fee_info_cached: Mutex::new(None), + next_local_fee: Mutex::new(PredictedNextFee::default()), #[cfg(any(test, fuzzing))] - next_remote_commitment_tx_fee_info_cached: Mutex::new(None), + next_remote_fee: Mutex::new(PredictedNextFee::default()), }) } } @@ -3205,9 +3202,9 @@ where counterparty_max_commitment_tx_output: Mutex::new((value_to_self_msat, (channel_value_satoshis * 1000 - msg_push_msat).saturating_sub(value_to_self_msat))), #[cfg(any(test, fuzzing))] - next_local_commitment_tx_fee_info_cached: Mutex::new(None), + next_local_fee: Mutex::new(PredictedNextFee::default()), #[cfg(any(test, fuzzing))] - next_remote_commitment_tx_fee_info_cached: Mutex::new(None), + next_remote_fee: Mutex::new(PredictedNextFee::default()), channel_transaction_parameters: ChannelTransactionParameters { holder_pubkeys: pubkeys, @@ -3448,9 +3445,9 @@ where counterparty_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), #[cfg(any(test, fuzzing))] - next_local_commitment_tx_fee_info_cached: Mutex::new(None), + next_local_fee: Mutex::new(PredictedNextFee::default()), #[cfg(any(test, fuzzing))] - next_remote_commitment_tx_fee_info_cached: Mutex::new(None), + next_remote_fee: Mutex::new(PredictedNextFee::default()), channel_transaction_parameters: ChannelTransactionParameters { holder_pubkeys: pubkeys, @@ -4104,6 +4101,227 @@ where ); } + /// Returns a best-effort guess of the set of HTLCs that will be present + /// on the next local or remote commitment. We cannot be certain as the + /// actual set of HTLCs present on the next commitment depends on the + /// ordering of commitment_signed and revoke_and_ack messages. + /// + /// We take the conservative approach and only assume that a HTLC will + /// not be in the next commitment when it is guaranteed that it won't be. + #[rustfmt::skip] + fn get_next_commitment_htlcs( + &self, local: bool, htlc_candidate: Option, include_counterparty_unknown_htlcs: bool, + ) -> Vec { + let mut commitment_htlcs = Vec::with_capacity( + 1 + self.pending_inbound_htlcs.len() + + self.pending_outbound_htlcs.len() + + self.holding_cell_htlc_updates.len(), + ); + // `LocalRemoved` HTLCs will certainly not be present on any future remote + // commitments, but they could be in a future local commitment as the remote has + // not yet acknowledged the removal. + let pending_inbound_htlcs = self + .pending_inbound_htlcs + .iter() + .filter(|InboundHTLCOutput { state, .. }| match (state, local) { + (InboundHTLCState::RemoteAnnounced(..), _) => true, + (InboundHTLCState::AwaitingRemoteRevokeToAnnounce(..), _) => true, + (InboundHTLCState::AwaitingAnnouncedRemoteRevoke(..), _) => true, + (InboundHTLCState::Committed, _) => true, + (InboundHTLCState::LocalRemoved(..), true) => true, + (InboundHTLCState::LocalRemoved(..), false) => false, + }) + .map(|&InboundHTLCOutput { amount_msat, .. }| HTLCAmountDirection { outbound: false, amount_msat }); + // `RemoteRemoved` HTLCs can still be present on the next remote commitment if + // local produces a commitment before acknowledging the update. These HTLCs + // will for sure not be present on the next local commitment. + let pending_outbound_htlcs = self + .pending_outbound_htlcs + .iter() + .filter(|OutboundHTLCOutput { state, .. }| match (state, local) { + (OutboundHTLCState::LocalAnnounced(..), _) => include_counterparty_unknown_htlcs, + (OutboundHTLCState::Committed, _) => true, + (OutboundHTLCState::RemoteRemoved(..), true) => false, + (OutboundHTLCState::RemoteRemoved(..), false) => true, + (OutboundHTLCState::AwaitingRemoteRevokeToRemove(..), _) => false, + (OutboundHTLCState::AwaitingRemovedRemoteRevoke(..), _) => false, + }) + .map(|&OutboundHTLCOutput { amount_msat, .. }| HTLCAmountDirection { outbound: true, amount_msat }); + + let holding_cell_htlcs = self.holding_cell_htlc_updates.iter().filter_map(|htlc| { + if let &HTLCUpdateAwaitingACK::AddHTLC { amount_msat, .. } = htlc { + Some(HTLCAmountDirection { outbound: true, amount_msat }) + } else { + None + } + }); + + if include_counterparty_unknown_htlcs { + commitment_htlcs.extend( + htlc_candidate.into_iter().chain(pending_inbound_htlcs).chain(pending_outbound_htlcs).chain(holding_cell_htlcs) + ); + } else { + commitment_htlcs.extend( + htlc_candidate.into_iter().chain(pending_inbound_htlcs).chain(pending_outbound_htlcs) + ); + } + + commitment_htlcs + } + + /// This returns the value of `value_to_self_msat` after accounting for all the + /// successful inbound and outbound HTLCs that won't be present on the next + /// commitment. + /// + /// To determine which HTLC claims to account for, we take the cases where a HTLC + /// will *not* be present on the next commitment from `next_commitment_htlcs`, and + /// check if their outcome is successful. If it is, we add the value of this claimed + /// HTLC to the balance of the claimer. + #[rustfmt::skip] + fn get_next_commitment_value_to_self_msat(&self, local: bool, funding: &FundingScope) -> u64 { + let inbound_claimed_htlc_msat: u64 = + self.pending_inbound_htlcs + .iter() + .filter(|InboundHTLCOutput { state, .. }| match (state, local) { + (InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_, _)), true) => false, + (InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_, _)), false) => true, + _ => false, + }) + .map(|InboundHTLCOutput { amount_msat, .. }| amount_msat) + .sum(); + let outbound_claimed_htlc_msat: u64 = + self.pending_outbound_htlcs + .iter() + .filter(|OutboundHTLCOutput { state, .. }| match (state, local) { + (OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(_, _)), true) => true, + (OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(_, _)), false) => false, + (OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_, _)), _) => true, + (OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_, _)), _) => true, + _ => false, + }) + .map(|OutboundHTLCOutput { amount_msat, .. }| amount_msat) + .sum(); + + funding + .value_to_self_msat + .saturating_sub(outbound_claimed_htlc_msat) + .saturating_add(inbound_claimed_htlc_msat) + } + + fn get_next_local_commitment_stats( + &self, funding: &FundingScope, htlc_candidate: Option, + include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize, + feerate_per_kw: u32, dust_exposure_limiting_feerate: Option, + ) -> NextCommitmentStats { + let next_commitment_htlcs = self.get_next_commitment_htlcs( + true, + htlc_candidate, + include_counterparty_unknown_htlcs, + ); + let next_value_to_self_msat = self.get_next_commitment_value_to_self_msat(true, funding); + + let ret = SpecTxBuilder {}.get_next_commitment_stats( + true, + funding.is_outbound(), + funding.get_value_satoshis(), + next_value_to_self_msat, + &next_commitment_htlcs, + addl_nondust_htlc_count, + feerate_per_kw, + dust_exposure_limiting_feerate, + self.holder_dust_limit_satoshis, + funding.get_channel_type(), + ); + + #[cfg(any(test, fuzzing))] + { + if addl_nondust_htlc_count == 0 { + *funding.next_local_fee.lock().unwrap() = PredictedNextFee { + predicted_feerate: feerate_per_kw, + predicted_nondust_htlc_count: ret.nondust_htlc_count, + predicted_fee_sat: ret.commit_tx_fee_sat, + }; + } else { + let predicted_stats = SpecTxBuilder {}.get_next_commitment_stats( + true, + funding.is_outbound(), + funding.get_value_satoshis(), + next_value_to_self_msat, + &next_commitment_htlcs, + 0, + feerate_per_kw, + dust_exposure_limiting_feerate, + self.holder_dust_limit_satoshis, + funding.get_channel_type(), + ); + *funding.next_local_fee.lock().unwrap() = PredictedNextFee { + predicted_feerate: feerate_per_kw, + predicted_nondust_htlc_count: predicted_stats.nondust_htlc_count, + predicted_fee_sat: predicted_stats.commit_tx_fee_sat, + }; + } + } + + ret + } + + fn get_next_remote_commitment_stats( + &self, funding: &FundingScope, htlc_candidate: Option, + include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize, + feerate_per_kw: u32, dust_exposure_limiting_feerate: Option, + ) -> NextCommitmentStats { + let next_commitment_htlcs = self.get_next_commitment_htlcs( + false, + htlc_candidate, + include_counterparty_unknown_htlcs, + ); + let next_value_to_self_msat = self.get_next_commitment_value_to_self_msat(false, funding); + + let ret = SpecTxBuilder {}.get_next_commitment_stats( + false, + funding.is_outbound(), + funding.get_value_satoshis(), + next_value_to_self_msat, + &next_commitment_htlcs, + addl_nondust_htlc_count, + feerate_per_kw, + dust_exposure_limiting_feerate, + self.counterparty_dust_limit_satoshis, + funding.get_channel_type(), + ); + + #[cfg(any(test, fuzzing))] + { + if addl_nondust_htlc_count == 0 { + *funding.next_remote_fee.lock().unwrap() = PredictedNextFee { + predicted_feerate: feerate_per_kw, + predicted_nondust_htlc_count: ret.nondust_htlc_count, + predicted_fee_sat: ret.commit_tx_fee_sat, + }; + } else { + let predicted_stats = SpecTxBuilder {}.get_next_commitment_stats( + false, + funding.is_outbound(), + funding.get_value_satoshis(), + next_value_to_self_msat, + &next_commitment_htlcs, + 0, + feerate_per_kw, + dust_exposure_limiting_feerate, + self.counterparty_dust_limit_satoshis, + funding.get_channel_type(), + ); + *funding.next_remote_fee.lock().unwrap() = PredictedNextFee { + predicted_feerate: feerate_per_kw, + predicted_nondust_htlc_count: predicted_stats.nondust_htlc_count, + predicted_fee_sat: predicted_stats.commit_tx_fee_sat, + }; + } + } + + ret + } + #[rustfmt::skip] fn validate_update_add_htlc( &self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC, @@ -4119,15 +4337,25 @@ where let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate( &fee_estimator, funding.get_channel_type(), ); - let htlc_stats = self.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); - if htlc_stats.pending_inbound_htlcs + 1 > self.holder_max_accepted_htlcs as usize { + // Don't include outbound update_add_htlc's in the holding cell, or those which haven't yet been ACK'ed by the counterparty (ie. LocalAnnounced HTLCs) + let include_counterparty_unknown_htlcs = false; + // Don't include the extra fee spike buffer HTLC in calculations + let fee_spike_buffer_htlc = 0; + let next_remote_commitment_stats = self.get_next_remote_commitment_stats(funding, Some(HTLCAmountDirection { outbound: false, amount_msat: msg.amount_msat }), include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, self.feerate_per_kw, dust_exposure_limiting_feerate); + + if next_remote_commitment_stats.inbound_htlcs_count > self.holder_max_accepted_htlcs as usize { return Err(ChannelError::close(format!("Remote tried to push more than our max accepted HTLCs ({})", self.holder_max_accepted_htlcs))); } - if htlc_stats.pending_inbound_htlcs_value_msat + msg.amount_msat > self.holder_max_htlc_value_in_flight_msat { + if next_remote_commitment_stats.inbound_htlcs_value_msat > self.holder_max_htlc_value_in_flight_msat { return Err(ChannelError::close(format!("Remote HTLC add would put them over our max HTLC value ({})", self.holder_max_htlc_value_in_flight_msat))); } - // Check holder_selected_channel_reserve_satoshis (we're getting paid, so they have to at least meet + let remote_balance_before_fee_msat = next_remote_commitment_stats.counterparty_balance_msat.ok_or(ChannelError::close("Remote HTLC add would overdraw remaining funds".to_owned()))?; + + // Check that the remote can afford to pay for this HTLC on-chain at the current + // feerate_per_kw, while maintaining their channel reserve (as required by the spec). + // + // We check holder_selected_channel_reserve_satoshis (we're getting paid, so they have to at least meet // the reserve_satoshis we told them to always have as direct payment so that they lose // something if we punish them for broadcasting an old state). // Note that we don't really care about having a small/no to_remote output in our local @@ -4139,50 +4367,22 @@ where // violate the reserve value if we do not do this (as we forget inbound HTLCs from the // Channel state once they will not be present in the next received commitment // transaction). - let (local_balance_before_fee_msat, remote_balance_before_fee_msat) = { - let removed_outbound_total_msat: u64 = self.pending_outbound_htlcs - .iter() - .filter_map(|htlc| { - matches!( - htlc.state, - OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_, _)) - | OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_, _)) - ) - .then_some(htlc.amount_msat) - }) - .sum(); - let pending_value_to_self_msat = - funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; - let pending_remote_value_msat = - funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; - - // Subtract any non-HTLC outputs from the local and remote balances - SpecTxBuilder {}.subtract_non_htlc_outputs(funding.is_outbound(), funding.value_to_self_msat, pending_remote_value_msat, funding.get_channel_type()) - }; - if remote_balance_before_fee_msat < msg.amount_msat { - return Err(ChannelError::close("Remote HTLC add would overdraw remaining funds".to_owned())); - } - - // Check that the remote can afford to pay for this HTLC on-chain at the current - // feerate_per_kw, while maintaining their channel reserve (as required by the spec). { let remote_commit_tx_fee_msat = if funding.is_outbound() { 0 } else { - let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); - self.next_remote_commit_tx_fee_msat(funding, Some(htlc_candidate), None) // Don't include the extra fee spike buffer HTLC in calculations + next_remote_commitment_stats.commit_tx_fee_sat * 1000 }; - if remote_balance_before_fee_msat.saturating_sub(msg.amount_msat) < remote_commit_tx_fee_msat { + if remote_balance_before_fee_msat < remote_commit_tx_fee_msat { return Err(ChannelError::close("Remote HTLC add would not leave enough to pay for fees".to_owned())); }; - if remote_balance_before_fee_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat) < funding.holder_selected_channel_reserve_satoshis * 1000 { + if remote_balance_before_fee_msat.saturating_sub(remote_commit_tx_fee_msat) < funding.holder_selected_channel_reserve_satoshis * 1000 { return Err(ChannelError::close("Remote HTLC add would put them under remote reserve value".to_owned())); } } + let next_local_commitment_stats = self.get_next_local_commitment_stats(funding, Some(HTLCAmountDirection { outbound: false, amount_msat: msg.amount_msat }), include_counterparty_unknown_htlcs, 0, self.feerate_per_kw, dust_exposure_limiting_feerate); if funding.is_outbound() { // Check that they won't violate our local required channel reserve by adding this HTLC. - let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); - let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(funding, htlc_candidate, None); - if local_balance_before_fee_msat < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat { + if next_local_commitment_stats.holder_balance_msat.unwrap_or(0) < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + next_local_commitment_stats.commit_tx_fee_sat * 1000 { return Err(ChannelError::close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned())); } } @@ -4202,16 +4402,21 @@ where let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate( &fee_estimator, funding.get_channel_type(), ); - let htlc_stats = self.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); + // Do not include outbound update_add_htlc's in the holding cell, or those which haven't yet been ACK'ed by the counterparty (ie. LocalAnnounced HTLCs) + let include_counterparty_unknown_htlcs = false; + let next_local_commitment_stats = self.get_next_local_commitment_stats(funding, None, include_counterparty_unknown_htlcs, 0, msg.feerate_per_kw, dust_exposure_limiting_feerate); + let next_remote_commitment_stats = self.get_next_remote_commitment_stats(funding, None, include_counterparty_unknown_htlcs, 0, msg.feerate_per_kw, dust_exposure_limiting_feerate); + let max_dust_htlc_exposure_msat = self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); - if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { + if next_local_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat { return Err(ChannelError::close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our own transactions (totaling {} msat)", - msg.feerate_per_kw, htlc_stats.on_holder_tx_dust_exposure_msat))); + msg.feerate_per_kw, next_local_commitment_stats.dust_exposure_msat))); } - if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { + if next_remote_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat { return Err(ChannelError::close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our counterparty's transactions (totaling {} msat)", - msg.feerate_per_kw, htlc_stats.on_counterparty_tx_dust_exposure_msat))); + msg.feerate_per_kw, next_remote_commitment_stats.dust_exposure_msat))); } + Ok(()) } @@ -4257,19 +4462,9 @@ where } #[cfg(any(test, fuzzing))] { - if funding.is_outbound() { - let projected_commit_tx_info = funding.next_local_commitment_tx_fee_info_cached.lock().unwrap().take(); - *funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None; - if let Some(info) = projected_commit_tx_info { - let total_pending_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len() - + self.holding_cell_htlc_updates.len(); - if info.total_pending_htlcs == total_pending_htlcs - && info.next_holder_htlc_id == self.next_holder_htlc_id - && info.next_counterparty_htlc_id == self.next_counterparty_htlc_id - && info.feerate == self.feerate_per_kw { - assert_eq!(commitment_data.stats.commit_tx_fee_sat, info.fee / 1000); - } - } + let PredictedNextFee { predicted_feerate, predicted_nondust_htlc_count, predicted_fee_sat } = *funding.next_local_fee.lock().unwrap(); + if predicted_feerate == commitment_data.tx.feerate_per_kw() && predicted_nondust_htlc_count == commitment_data.tx.nondust_htlcs().len() { + assert_eq!(predicted_fee_sat, commitment_data.stats.commit_tx_fee_sat); } } @@ -4321,11 +4516,11 @@ where let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate( &fee_estimator, funding.get_channel_type(), ); - let htlc_stats = self.get_pending_htlc_stats(funding, Some(feerate_per_kw), dust_exposure_limiting_feerate); - let stats = self.build_commitment_stats(funding, true, true, Some(feerate_per_kw), Some(htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize)); - let holder_balance_msat = stats.local_balance_before_fee_msat - htlc_stats.outbound_holding_cell_msat; + // Include outbound update_add_htlc's in the holding cell, and those which haven't yet been ACK'ed by the counterparty (ie. LocalAnnounced HTLCs) + let include_counterparty_unknown_htlcs = true; + let next_remote_commitment_stats = self.get_next_remote_commitment_stats(funding, None, include_counterparty_unknown_htlcs, CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, feerate_per_kw, dust_exposure_limiting_feerate); // Note that `stats.commit_tx_fee_sat` accounts for any HTLCs that transition from non-dust to dust under a higher feerate (in the case where HTLC-transactions pay endogenous fees). - if holder_balance_msat < stats.commit_tx_fee_sat * 1000 + funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 { + if next_remote_commitment_stats.holder_balance_msat.unwrap_or(0) < next_remote_commitment_stats.commit_tx_fee_sat * 1000 + funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 { //TODO: auto-close after a number of failures? log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw); return false; @@ -4333,11 +4528,12 @@ where // Note, we evaluate pending htlc "preemptive" trimmed-to-dust threshold at the proposed `feerate_per_kw`. let max_dust_htlc_exposure_msat = self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); - if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { + if next_remote_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat { log_debug!(logger, "Cannot afford to send new feerate at {} without infringing max dust htlc exposure", feerate_per_kw); return false; } - if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { + + if next_remote_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat { log_debug!(logger, "Cannot afford to send new feerate at {} without infringing max dust htlc exposure", feerate_per_kw); return false; } @@ -4347,78 +4543,47 @@ where #[rustfmt::skip] fn can_accept_incoming_htlc( - &self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC, + &self, funding: &FundingScope, dust_exposure_limiting_feerate: Option, logger: &L, ) -> Result<(), LocalHTLCFailureReason> where L::Target: Logger, { - let htlc_stats = self.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); + // The fee spike buffer (an additional nondust HTLC) we keep for the remote if the channel + // is not zero fee. This deviates from the spec because the fee spike buffer requirement + // doesn't exist on the receiver's side, only on the sender's. Note that with anchor + // outputs we are no longer as sensitive to fee spikes, so we need to account for them. + let fee_spike_buffer_htlc = if funding.get_channel_type().supports_anchor_zero_fee_commitments() { + 0 + } else { + 1 + }; + // Do not include outbound update_add_htlc's in the holding cell, or those which haven't yet been ACK'ed by the counterparty (ie. LocalAnnounced HTLCs) + let include_counterparty_unknown_htlcs = false; + // A `None` `HTLCCandidate` is used as in this case because we're already accounting for + // the incoming HTLC as it has been fully committed by both sides. + let next_local_commitment_stats = self.get_next_local_commitment_stats(funding, None, include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, self.feerate_per_kw, dust_exposure_limiting_feerate); + let next_remote_commitment_stats = self.get_next_remote_commitment_stats(funding, None, include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, self.feerate_per_kw, dust_exposure_limiting_feerate); + let max_dust_htlc_exposure_msat = self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); - let on_counterparty_tx_dust_htlc_exposure_msat = htlc_stats.on_counterparty_tx_dust_exposure_msat; - if on_counterparty_tx_dust_htlc_exposure_msat > max_dust_htlc_exposure_msat { + if next_remote_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat { // Note that the total dust exposure includes both the dust HTLCs and the excess mining fees of the counterparty commitment transaction log_info!(logger, "Cannot accept value that would put our total dust exposure at {} over the limit {} on counterparty commitment tx", - on_counterparty_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat); + next_remote_commitment_stats.dust_exposure_msat, max_dust_htlc_exposure_msat); return Err(LocalHTLCFailureReason::DustLimitCounterparty) } - let dust_buffer_feerate = self.get_dust_buffer_feerate(None); - let (htlc_success_tx_fee_sat, _) = second_stage_tx_fees_sat( - &funding.get_channel_type(), dust_buffer_feerate, - ); - let exposure_dust_limit_success_sats = htlc_success_tx_fee_sat + self.holder_dust_limit_satoshis; - if msg.amount_msat / 1000 < exposure_dust_limit_success_sats { - let on_holder_tx_dust_htlc_exposure_msat = htlc_stats.on_holder_tx_dust_exposure_msat; - if on_holder_tx_dust_htlc_exposure_msat > max_dust_htlc_exposure_msat { - log_info!(logger, "Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", - on_holder_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat); - return Err(LocalHTLCFailureReason::DustLimitHolder) - } + if next_local_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat { + log_info!(logger, "Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", + next_local_commitment_stats.dust_exposure_msat, max_dust_htlc_exposure_msat); + return Err(LocalHTLCFailureReason::DustLimitHolder) } if !funding.is_outbound() { - let removed_outbound_total_msat: u64 = self.pending_outbound_htlcs - .iter() - .filter_map(|htlc| { - matches!( - htlc.state, - OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_, _)) - | OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_, _)) - ) - .then_some(htlc.amount_msat) - }) - .sum(); - let pending_value_to_self_msat = - funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; - let pending_remote_value_msat = - funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; - // Subtract any non-HTLC outputs from the local and remote balances - let (_, remote_balance_before_fee_msat) = SpecTxBuilder {}.subtract_non_htlc_outputs( - funding.is_outbound(), - pending_value_to_self_msat, - pending_remote_value_msat, - funding.get_channel_type() - ); - - // `Some(())` is for the fee spike buffer we keep for the remote if the channel is - // not zero fee. This deviates from the spec because the fee spike buffer requirement - // doesn't exist on the receiver's side, only on the sender's. Note that with anchor - // outputs we are no longer as sensitive to fee spikes, so we need to account for them. - // - // A `None` `HTLCCandidate` is used as in this case because we're already accounting for - // the incoming HTLC as it has been fully committed by both sides. - let fee_spike_buffer_htlc = if funding.get_channel_type().supports_anchor_zero_fee_commitments() { - None - } else { - Some(()) - }; - - let mut remote_fee_cost_incl_stuck_buffer_msat = self.next_remote_commit_tx_fee_msat( - funding, None, fee_spike_buffer_htlc, - ); + let mut remote_fee_cost_incl_stuck_buffer_msat = next_remote_commitment_stats.commit_tx_fee_sat * 1000; if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { remote_fee_cost_incl_stuck_buffer_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; } + let remote_balance_before_fee_msat = next_remote_commitment_stats.counterparty_balance_msat.unwrap_or(0); if remote_balance_before_fee_msat.saturating_sub(funding.holder_selected_channel_reserve_satoshis * 1000) < remote_fee_cost_incl_stuck_buffer_msat { log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", &self.channel_id()); return Err(LocalHTLCFailureReason::FeeSpikeBuffer); @@ -4732,8 +4897,6 @@ where } let mut pending_outbound_htlcs_value_msat = 0; - let mut outbound_holding_cell_msat = 0; - let mut on_holder_tx_outbound_holding_cell_htlcs_count = 0; let mut pending_outbound_htlcs = self.pending_outbound_htlcs.len(); { let counterparty_dust_limit_success_sat = htlc_success_tx_fee_sat + context.counterparty_dust_limit_satoshis; @@ -4754,7 +4917,6 @@ where if let &HTLCUpdateAwaitingACK::AddHTLC { ref amount_msat, .. } = update { pending_outbound_htlcs += 1; pending_outbound_htlcs_value_msat += amount_msat; - outbound_holding_cell_msat += amount_msat; if *amount_msat / 1000 < counterparty_dust_limit_success_sat { on_counterparty_tx_dust_exposure_msat += amount_msat; } else { @@ -4762,8 +4924,6 @@ where } if *amount_msat / 1000 < holder_dust_limit_timeout_sat { on_holder_tx_dust_exposure_msat += amount_msat; - } else { - on_holder_tx_outbound_holding_cell_htlcs_count += 1; } } } @@ -4795,15 +4955,12 @@ where }); HTLCStats { - pending_inbound_htlcs: self.pending_inbound_htlcs.len(), pending_outbound_htlcs, pending_inbound_htlcs_value_msat, pending_outbound_htlcs_value_msat, on_counterparty_tx_dust_exposure_msat, extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat, on_holder_tx_dust_exposure_msat, - outbound_holding_cell_msat, - on_holder_tx_outbound_holding_cell_htlcs_count, } } @@ -5134,31 +5291,7 @@ where } let num_htlcs = included_htlcs + addl_htlcs; - let commit_tx_fee_msat = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000; - #[cfg(any(test, fuzzing))] - { - let mut fee = commit_tx_fee_msat; - if fee_spike_buffer_htlc.is_some() { - fee = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000; - } - let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len() - + context.holding_cell_htlc_updates.len(); - let commitment_tx_info = CommitmentTxInfoCached { - fee, - total_pending_htlcs, - next_holder_htlc_id: match htlc.origin { - HTLCInitiator::LocalOffered => context.next_holder_htlc_id + 1, - HTLCInitiator::RemoteOffered => context.next_holder_htlc_id, - }, - next_counterparty_htlc_id: match htlc.origin { - HTLCInitiator::LocalOffered => context.next_counterparty_htlc_id, - HTLCInitiator::RemoteOffered => context.next_counterparty_htlc_id + 1, - }, - feerate: context.feerate_per_kw, - }; - *funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info); - } - commit_tx_fee_msat + SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000 } /// Get the commitment tx fee for the remote's next commitment transaction based on the number of @@ -5235,30 +5368,7 @@ where } let num_htlcs = included_htlcs + addl_htlcs; - let commit_tx_fee_msat = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000; - #[cfg(any(test, fuzzing))] - if let Some(htlc) = &htlc { - let mut fee = commit_tx_fee_msat; - if fee_spike_buffer_htlc.is_some() { - fee = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000; - } - let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len(); - let commitment_tx_info = CommitmentTxInfoCached { - fee, - total_pending_htlcs, - next_holder_htlc_id: match htlc.origin { - HTLCInitiator::LocalOffered => context.next_holder_htlc_id + 1, - HTLCInitiator::RemoteOffered => context.next_holder_htlc_id, - }, - next_counterparty_htlc_id: match htlc.origin { - HTLCInitiator::LocalOffered => context.next_counterparty_htlc_id, - HTLCInitiator::RemoteOffered => context.next_counterparty_htlc_id + 1, - }, - feerate: context.feerate_per_kw, - }; - *funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info); - } - commit_tx_fee_msat + SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000 } #[rustfmt::skip] @@ -5914,12 +6024,11 @@ macro_rules! promote_splice_funding { } #[cfg(any(test, fuzzing))] -struct CommitmentTxInfoCached { - fee: u64, - total_pending_htlcs: usize, - next_holder_htlc_id: u64, - next_counterparty_htlc_id: u64, - feerate: u32, +#[derive(Clone, Copy, Default)] +struct PredictedNextFee { + predicted_feerate: u32, + predicted_nondust_htlc_count: usize, + predicted_fee_sat: u64, } /// Contents of a wire message that fails an HTLC backwards. Useful for [`FundedChannel::fail_htlc`] to @@ -7422,16 +7531,6 @@ where return Err(ChannelError::close("Received an unexpected revoke_and_ack".to_owned())); } - #[cfg(any(test, fuzzing))] - { - for funding in - core::iter::once(&mut self.funding).chain(self.pending_funding.iter_mut()) - { - *funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None; - *funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None; - } - } - match &self.context.holder_signer { ChannelSignerType::Ecdsa(ecdsa) => { ecdsa @@ -9387,7 +9486,7 @@ where /// this function determines whether to fail the HTLC, or forward / claim it. #[rustfmt::skip] pub fn can_accept_incoming_htlc( - &self, msg: &msgs::UpdateAddHTLC, fee_estimator: &LowerBoundedFeeEstimator, logger: L + &self, fee_estimator: &LowerBoundedFeeEstimator, logger: L ) -> Result<(), LocalHTLCFailureReason> where F::Target: FeeEstimator, @@ -9403,7 +9502,7 @@ where core::iter::once(&self.funding) .chain(self.pending_funding.iter()) - .try_for_each(|funding| self.context.can_accept_incoming_htlc(funding, msg, dust_exposure_limiting_feerate, &logger)) + .try_for_each(|funding| self.context.can_accept_incoming_htlc(funding, dust_exposure_limiting_feerate, &logger)) } pub fn get_cur_holder_commitment_transaction_number(&self) -> u64 { @@ -10873,18 +10972,9 @@ where #[cfg(any(test, fuzzing))] { - if !funding.is_outbound() { - let projected_commit_tx_info = funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take(); - *funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None; - if let Some(info) = projected_commit_tx_info { - let total_pending_htlcs = self.context.pending_inbound_htlcs.len() + self.context.pending_outbound_htlcs.len(); - if info.total_pending_htlcs == total_pending_htlcs - && info.next_holder_htlc_id == self.context.next_holder_htlc_id - && info.next_counterparty_htlc_id == self.context.next_counterparty_htlc_id - && info.feerate == self.context.feerate_per_kw { - assert_eq!(commitment_data.stats.commit_tx_fee_sat, info.fee); - } - } + let PredictedNextFee { predicted_feerate, predicted_nondust_htlc_count, predicted_fee_sat } = *funding.next_remote_fee.lock().unwrap(); + if predicted_feerate == counterparty_commitment_tx.feerate_per_kw() && predicted_nondust_htlc_count == counterparty_commitment_tx.nondust_htlcs().len() { + assert_eq!(predicted_fee_sat, commitment_data.stats.commit_tx_fee_sat); } } @@ -13508,9 +13598,9 @@ where counterparty_max_commitment_tx_output: Mutex::new((0, 0)), #[cfg(any(test, fuzzing))] - next_local_commitment_tx_fee_info_cached: Mutex::new(None), + next_local_fee: Mutex::new(PredictedNextFee::default()), #[cfg(any(test, fuzzing))] - next_remote_commitment_tx_fee_info_cached: Mutex::new(None), + next_remote_fee: Mutex::new(PredictedNextFee::default()), channel_transaction_parameters: channel_parameters, funding_transaction, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8bac6c2fa3a..64ef1b531d1 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6262,7 +6262,7 @@ where &chan.context, Some(update_add_htlc.payment_hash), ); - chan.can_accept_incoming_htlc(update_add_htlc, &self.fee_estimator, &logger) + chan.can_accept_incoming_htlc(&self.fee_estimator, &logger) }, ) { Some(Ok(_)) => {}, diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs index 6e623d1a7db..7644f55e363 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -1,19 +1,128 @@ //! Defines the `TxBuilder` trait, and the `SpecTxBuilder` type +#![allow(dead_code)] +use core::cmp; use core::ops::Deref; use bitcoin::secp256k1::{self, PublicKey, Secp256k1}; use crate::ln::chan_utils::{ - commit_tx_fee_sat, htlc_success_tx_weight, htlc_timeout_tx_weight, - ChannelTransactionParameters, CommitmentTransaction, HTLCOutputInCommitment, + commit_tx_fee_sat, htlc_success_tx_weight, htlc_timeout_tx_weight, htlc_tx_fees_sat, + second_stage_tx_fees_sat, ChannelTransactionParameters, CommitmentTransaction, + HTLCOutputInCommitment, }; use crate::ln::channel::{CommitmentStats, ANCHOR_OUTPUT_VALUE_SATOSHI}; use crate::prelude::*; use crate::types::features::ChannelTypeFeatures; use crate::util::logger::Logger; +pub(crate) struct HTLCAmountDirection { + pub outbound: bool, + pub amount_msat: u64, +} + +impl HTLCAmountDirection { + fn is_dust( + &self, local: bool, feerate_per_kw: u32, broadcaster_dust_limit_satoshis: u64, + channel_type: &ChannelTypeFeatures, + ) -> bool { + let (success_tx_fee_sat, timeout_tx_fee_sat) = + second_stage_tx_fees_sat(channel_type, feerate_per_kw); + let htlc_tx_fee_sat = + if self.outbound == local { timeout_tx_fee_sat } else { success_tx_fee_sat }; + self.amount_msat / 1000 < broadcaster_dust_limit_satoshis + htlc_tx_fee_sat + } +} + +pub(crate) struct NextCommitmentStats { + pub inbound_htlcs_count: usize, + pub inbound_htlcs_value_msat: u64, + pub holder_balance_msat: Option, + pub counterparty_balance_msat: Option, + pub nondust_htlc_count: usize, + pub commit_tx_fee_sat: u64, + pub dust_exposure_msat: u64, + // If the counterparty sets a feerate on the channel in excess of our dust_exposure_limiting_feerate, + // this should be set to the dust exposure that would result from us adding an additional nondust outbound + // htlc on the counterparty's commitment transaction. + pub extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat: Option, +} + +#[rustfmt::skip] +fn excess_fees_on_counterparty_tx_dust_exposure_msat( + next_commitment_htlcs: &[HTLCAmountDirection], dust_buffer_feerate: u32, + excess_feerate: u32, counterparty_dust_limit_satoshis: u64, dust_htlc_exposure_msat: u64, + channel_type: &ChannelTypeFeatures, +) -> (u64, u64) { + + let on_counterparty_tx_accepted_nondust_htlcs = next_commitment_htlcs.iter().filter(|htlc| htlc.outbound && !htlc.is_dust(false, dust_buffer_feerate, counterparty_dust_limit_satoshis, channel_type)).count(); + let on_counterparty_tx_offered_nondust_htlcs = next_commitment_htlcs.iter().filter(|htlc| !htlc.outbound && !htlc.is_dust(false, dust_buffer_feerate, counterparty_dust_limit_satoshis, channel_type)).count(); + + let commitment_fee_sat = commit_tx_fee_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + on_counterparty_tx_offered_nondust_htlcs, channel_type); + let second_stage_fees_sat = htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs, on_counterparty_tx_offered_nondust_htlcs, channel_type); + let on_counterparty_tx_dust_exposure_msat = dust_htlc_exposure_msat + (commitment_fee_sat + second_stage_fees_sat) * 1000; + + let extra_htlc_commitment_fee_sat = commit_tx_fee_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1 + on_counterparty_tx_offered_nondust_htlcs, channel_type); + let extra_htlc_second_stage_fees_sat = htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1, on_counterparty_tx_offered_nondust_htlcs, channel_type); + let extra_htlc_dust_exposure_msat = dust_htlc_exposure_msat + (extra_htlc_commitment_fee_sat + extra_htlc_second_stage_fees_sat) * 1000; + + ( + on_counterparty_tx_dust_exposure_msat, + extra_htlc_dust_exposure_msat, + ) +} + +fn subtract_addl_outputs( + is_outbound_from_holder: bool, value_to_self_after_htlcs: Option, + value_to_remote_after_htlcs: Option, channel_type: &ChannelTypeFeatures, +) -> (Option, Option) { + let total_anchors_sat = if channel_type.supports_anchors_zero_fee_htlc_tx() { + ANCHOR_OUTPUT_VALUE_SATOSHI * 2 + } else { + 0 + }; + + // We MUST use checked subs here, as the funder's balance is not guaranteed to be greater + // than or equal to `total_anchors_sat`. + // + // This is because when the remote party sends an `update_fee` message, we build the new + // commitment transaction *before* checking whether the remote party's balance is enough to + // cover the total anchor sum. + + let local_balance_before_fee_msat = if is_outbound_from_holder { + value_to_self_after_htlcs.and_then(|balance| balance.checked_sub(total_anchors_sat * 1000)) + } else { + value_to_self_after_htlcs + }; + + let remote_balance_before_fee_msat = if !is_outbound_from_holder { + value_to_remote_after_htlcs + .and_then(|balance| balance.checked_sub(total_anchors_sat * 1000)) + } else { + value_to_remote_after_htlcs + }; + + (local_balance_before_fee_msat, remote_balance_before_fee_msat) +} + +fn get_dust_buffer_feerate(feerate_per_kw: u32) -> u32 { + // When calculating our exposure to dust HTLCs, we assume that the channel feerate + // may, at any point, increase by at least 10 sat/vB (i.e 2530 sat/kWU) or 25%, + // whichever is higher. This ensures that we aren't suddenly exposed to significantly + // more dust balance if the feerate increases when we have several HTLCs pending + // which are near the dust limit. + let feerate_plus_quarter = feerate_per_kw.checked_mul(1250).map(|v| v / 1000); + cmp::max(feerate_per_kw.saturating_add(2530), feerate_plus_quarter.unwrap_or(u32::MAX)) +} + pub(crate) trait TxBuilder { + fn get_next_commitment_stats( + &self, local: bool, is_outbound_from_holder: bool, channel_value_satoshis: u64, + value_to_holder_msat: u64, next_commitment_htlcs: &[HTLCAmountDirection], + addl_nondust_htlc_count: usize, feerate_per_kw: u32, + dust_exposure_limiting_feerate: Option, broadcaster_dust_limit_satoshis: u64, + channel_type: &ChannelTypeFeatures, + ) -> NextCommitmentStats; fn commit_tx_fee_sat( &self, feerate_per_kw: u32, nondust_htlc_count: usize, channel_type: &ChannelTypeFeatures, ) -> u64; @@ -25,7 +134,7 @@ pub(crate) trait TxBuilder { &self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey, channel_parameters: &ChannelTransactionParameters, secp_ctx: &Secp256k1, value_to_self_msat: u64, htlcs_in_tx: Vec, feerate_per_kw: u32, - broadcaster_dust_limit_sat: u64, logger: &L, + broadcaster_dust_limit_satoshis: u64, logger: &L, ) -> (CommitmentTransaction, CommitmentStats) where L::Target: Logger; @@ -34,6 +143,114 @@ pub(crate) trait TxBuilder { pub(crate) struct SpecTxBuilder {} impl TxBuilder for SpecTxBuilder { + fn get_next_commitment_stats( + &self, local: bool, is_outbound_from_holder: bool, channel_value_satoshis: u64, + value_to_holder_msat: u64, next_commitment_htlcs: &[HTLCAmountDirection], + addl_nondust_htlc_count: usize, feerate_per_kw: u32, + dust_exposure_limiting_feerate: Option, broadcaster_dust_limit_satoshis: u64, + channel_type: &ChannelTypeFeatures, + ) -> NextCommitmentStats { + let excess_feerate_opt = + feerate_per_kw.checked_sub(dust_exposure_limiting_feerate.unwrap_or(0)); + // Dust exposure is only decoupled from feerate for zero fee commitment channels. + let is_zero_fee_comm = channel_type.supports_anchor_zero_fee_commitments(); + debug_assert_eq!(is_zero_fee_comm, dust_exposure_limiting_feerate.is_none()); + if is_zero_fee_comm { + debug_assert_eq!(feerate_per_kw, 0); + debug_assert_eq!(excess_feerate_opt, Some(0)); + debug_assert_eq!(addl_nondust_htlc_count, 0); + } + + // Calculate inbound htlc count + let inbound_htlcs_count = + next_commitment_htlcs.iter().filter(|htlc| !htlc.outbound).count(); + + // Calculate balances after htlcs + let value_to_counterparty_msat = (channel_value_satoshis * 1000) + .checked_sub(value_to_holder_msat) + .expect("value_to_holder_msat outgrew the value of the channel!"); + let outbound_htlcs_value_msat: u64 = next_commitment_htlcs + .iter() + .filter_map(|htlc| htlc.outbound.then_some(htlc.amount_msat)) + .sum(); + let inbound_htlcs_value_msat: u64 = next_commitment_htlcs + .iter() + .filter_map(|htlc| (!htlc.outbound).then_some(htlc.amount_msat)) + .sum(); + // Note there is no guarantee that the subtractions of the HTLC amounts don't + // overflow, so we do not panic. Instead, we return `None` to signal an overflow + // to channel, and let channel take the appropriate action. + let value_to_holder_after_htlcs = + value_to_holder_msat.checked_sub(outbound_htlcs_value_msat); + let value_to_counterparty_after_htlcs = + value_to_counterparty_msat.checked_sub(inbound_htlcs_value_msat); + + // Subtract the anchors from the channel funder + let (holder_balance_msat, counterparty_balance_msat) = subtract_addl_outputs( + is_outbound_from_holder, + value_to_holder_after_htlcs, + value_to_counterparty_after_htlcs, + channel_type, + ); + + // Increment the feerate by a buffer to calculate dust exposure + let dust_buffer_feerate = get_dust_buffer_feerate(feerate_per_kw); + + // Calculate fees on commitment transaction + let nondust_htlc_count = next_commitment_htlcs + .iter() + .filter(|htlc| { + !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_satoshis, channel_type) + }) + .count(); + let commit_tx_fee_sat = commit_tx_fee_sat( + feerate_per_kw, + nondust_htlc_count + addl_nondust_htlc_count, + channel_type, + ); + + // Calculate dust exposure on commitment transaction + let dust_exposure_msat = next_commitment_htlcs + .iter() + .filter_map(|htlc| { + htlc.is_dust( + local, + dust_buffer_feerate, + broadcaster_dust_limit_satoshis, + channel_type, + ) + .then_some(htlc.amount_msat) + }) + .sum(); + + // Count the excess fees on the counterparty's transaction as dust + let (dust_exposure_msat, extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat) = + if let (Some(excess_feerate), false) = (excess_feerate_opt, local) { + let (dust_exposure_msat, extra_nondust_htlc_exposure_msat) = + excess_fees_on_counterparty_tx_dust_exposure_msat( + &next_commitment_htlcs, + dust_buffer_feerate, + excess_feerate, + broadcaster_dust_limit_satoshis, + dust_exposure_msat, + channel_type, + ); + (dust_exposure_msat, Some(extra_nondust_htlc_exposure_msat)) + } else { + (dust_exposure_msat, None) + }; + + NextCommitmentStats { + inbound_htlcs_count, + inbound_htlcs_value_msat, + holder_balance_msat, + counterparty_balance_msat, + nondust_htlc_count: nondust_htlc_count + addl_nondust_htlc_count, + commit_tx_fee_sat, + dust_exposure_msat, + extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat, + } + } fn commit_tx_fee_sat( &self, feerate_per_kw: u32, nondust_htlc_count: usize, channel_type: &ChannelTypeFeatures, ) -> u64 { @@ -74,7 +291,7 @@ impl TxBuilder for SpecTxBuilder { &self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey, channel_parameters: &ChannelTransactionParameters, secp_ctx: &Secp256k1, value_to_self_msat: u64, mut htlcs_in_tx: Vec, feerate_per_kw: u32, - broadcaster_dust_limit_sat: u64, logger: &L, + broadcaster_dust_limit_satoshis: u64, logger: &L, ) -> (CommitmentTransaction, CommitmentStats) where L::Target: Logger, @@ -95,7 +312,7 @@ impl TxBuilder for SpecTxBuilder { // As required by the spec, round down feerate_per_kw as u64 * htlc_tx_weight / 1000 }; - amount_msat / 1000 < broadcaster_dust_limit_sat + htlc_tx_fee_sat + amount_msat / 1000 < broadcaster_dust_limit_satoshis + htlc_tx_fee_sat }; // Trim dust htlcs @@ -107,7 +324,7 @@ impl TxBuilder for SpecTxBuilder { remote_htlc_total_msat += htlc.amount_msat; } if is_dust(htlc.offered, htlc.amount_msat) { - log_trace!(logger, " ...trimming {} HTLC with value {}sat, hash {}, due to dust limit {}", if htlc.offered == local { "outbound" } else { "inbound" }, htlc.amount_msat / 1000, htlc.payment_hash, broadcaster_dust_limit_sat); + log_trace!(logger, " ...trimming {} HTLC with value {}sat, hash {}, due to dust limit {}", if htlc.offered == local { "outbound" } else { "inbound" }, htlc.amount_msat / 1000, htlc.payment_hash, broadcaster_dust_limit_satoshis); false } else { true @@ -142,13 +359,13 @@ impl TxBuilder for SpecTxBuilder { 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_sat { + 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; } - if to_countersignatory_value_sat >= broadcaster_dust_limit_sat { + 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;