diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 2e5b15fc7f4..5bf9650ebad 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -248,6 +248,7 @@ pub fn do_test(data: &[u8], out: Out) { outbound_capacity_msat: capacity.saturating_mul(1000), next_outbound_htlc_limit_msat: capacity.saturating_mul(1000), next_outbound_htlc_minimum_msat: 0, + next_splice_out_maximum_sat: capacity, inbound_htlc_minimum_msat: None, inbound_htlc_maximum_msat: None, config: None, diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 10801edef01..cf5f4e69e3c 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -122,6 +122,8 @@ pub struct AvailableBalances { pub next_outbound_htlc_limit_msat: u64, /// The minimum value we can assign to the next outbound HTLC pub next_outbound_htlc_minimum_msat: u64, + /// The maximum value of the next splice-out + pub next_splice_out_maximum_sat: u64, } #[derive(Debug, Clone, Copy, PartialEq)] @@ -2746,20 +2748,50 @@ impl FundingScope { prev_funding: &Self, context: &ChannelContext, our_funding_contribution: SignedAmount, their_funding_contribution: SignedAmount, counterparty_funding_pubkey: PublicKey, our_new_holder_keys: ChannelPublicKeys, - ) -> Self { - debug_assert!(our_funding_contribution.unsigned_abs() <= Amount::MAX_MONEY); - debug_assert!(their_funding_contribution.unsigned_abs() <= Amount::MAX_MONEY); + ) -> Result { + if our_funding_contribution.unsigned_abs() > Amount::MAX_MONEY { + return Err(format!( + "Channel {} cannot be spliced; our {} contribution exceeds the total bitcoin supply", + context.channel_id(), + our_funding_contribution, + )); + } - let post_channel_value = prev_funding.compute_post_splice_value( - our_funding_contribution.to_sat(), - their_funding_contribution.to_sat(), - ); + if their_funding_contribution.unsigned_abs() > Amount::MAX_MONEY { + return Err(format!( + "Channel {} cannot be spliced; their {} contribution exceeds the total bitcoin supply", + context.channel_id(), + their_funding_contribution, + )); + } + + let channel_value_satoshis = prev_funding.get_value_satoshis(); + let value_to_self_satoshis = prev_funding.get_value_to_self_msat() / 1000; + let value_to_counterparty_satoshis = channel_value_satoshis + .checked_sub(value_to_self_satoshis) + .expect("value_to_self is greater than channel value"); + let our_funding_contribution_sat = our_funding_contribution.to_sat(); + let their_funding_contribution_sat = their_funding_contribution.to_sat(); let post_value_to_self_msat = prev_funding - .value_to_self_msat - .checked_add_signed(our_funding_contribution.to_sat() * 1000); - debug_assert!(post_value_to_self_msat.is_some()); - let post_value_to_self_msat = post_value_to_self_msat.unwrap(); + .get_value_to_self_msat() + .checked_add_signed(our_funding_contribution_sat * 1000) + .ok_or(format!( + "Our contribution candidate {our_funding_contribution_sat}sat is \ + greater than our total balance in the channel {value_to_self_satoshis}sat" + ))?; + + value_to_counterparty_satoshis.checked_add_signed(their_funding_contribution_sat).ok_or( + format!( + "Their contribution candidate {their_funding_contribution_sat}sat is \ + greater than their total balance in the channel {value_to_counterparty_satoshis}sat" + ), + )?; + + let post_channel_value = prev_funding.get_value_satoshis() + .checked_add_signed(our_funding_contribution.to_sat()) + .and_then(|v| v.checked_add_signed(their_funding_contribution.to_sat())) + .ok_or(format!("The sum of contributions {our_funding_contribution} and {their_funding_contribution} is greater than the channel's value"))?; let channel_parameters = &prev_funding.channel_transaction_parameters; let mut post_channel_transaction_parameters = ChannelTransactionParameters { @@ -2795,7 +2827,7 @@ impl FundingScope { prev_funding.holder_selected_channel_reserve_satoshis == 0, ); - Self { + Ok(Self { channel_transaction_parameters: post_channel_transaction_parameters, value_to_self_msat: post_value_to_self_msat, funding_transaction: None, @@ -2810,12 +2842,6 @@ impl FundingScope { prev.0.saturating_add_signed(our_funding_contribution.to_sat() * 1000); let new_counterparty_balance_msat = prev.1.saturating_add_signed(their_funding_contribution.to_sat() * 1000); - if new_holder_balance_msat < counterparty_selected_channel_reserve_satoshis { - assert_eq!(new_holder_balance_msat, prev.0); - } - if new_counterparty_balance_msat < holder_selected_channel_reserve_satoshis { - assert_eq!(new_counterparty_balance_msat, prev.1); - } Mutex::new((new_holder_balance_msat, new_counterparty_balance_msat)) }, #[cfg(debug_assertions)] @@ -2825,12 +2851,6 @@ impl FundingScope { prev.0.saturating_add_signed(our_funding_contribution.to_sat() * 1000); let new_counterparty_balance_msat = prev.1.saturating_add_signed(their_funding_contribution.to_sat() * 1000); - if new_holder_balance_msat < counterparty_selected_channel_reserve_satoshis { - assert_eq!(new_holder_balance_msat, prev.0); - } - if new_counterparty_balance_msat < holder_selected_channel_reserve_satoshis { - assert_eq!(new_counterparty_balance_msat, prev.1); - } Mutex::new((new_holder_balance_msat, new_counterparty_balance_msat)) }, #[cfg(any(test, fuzzing))] @@ -2841,16 +2861,7 @@ impl FundingScope { funding_tx_confirmed_in: None, minimum_depth_override: None, short_channel_id: None, - } - } - - /// Compute the post-splice channel value from each counterparty's contributions. - pub(super) fn compute_post_splice_value( - &self, our_funding_contribution: i64, their_funding_contribution: i64, - ) -> u64 { - self.get_value_satoshis().saturating_add_signed( - our_funding_contribution.saturating_add(their_funding_contribution), - ) + }) } /// Returns a `SharedOwnedInput` for using this `FundingScope` as the input to a new splice. @@ -4176,6 +4187,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, channel_context.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -4472,6 +4484,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, channel_context.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| APIError::APIMisuseError { @@ -5262,7 +5275,8 @@ impl ChannelContext { 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, + feerate_per_kw: u32, include_fee_spike_multiple: bool, + dust_exposure_limiting_feerate: Option, ) -> Result<(ChannelStats, Vec), ()> { let next_commitment_htlcs = self.get_next_commitment_htlcs( true, @@ -5284,6 +5298,7 @@ impl ChannelContext { &next_commitment_htlcs, addl_nondust_htlc_count, feerate_per_kw, + include_fee_spike_multiple, dust_exposure_limiting_feerate, max_dust_htlc_exposure_msat, channel_constraints, @@ -5308,6 +5323,7 @@ impl ChannelContext { &next_commitment_htlcs, 0, feerate_per_kw, + false, dust_exposure_limiting_feerate, max_dust_htlc_exposure_msat, channel_constraints, @@ -5329,7 +5345,8 @@ impl ChannelContext { 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, + feerate_per_kw: u32, include_fee_spike_multiple: bool, + dust_exposure_limiting_feerate: Option, ) -> Result<(ChannelStats, Vec), ()> { let next_commitment_htlcs = self.get_next_commitment_htlcs( false, @@ -5351,6 +5368,7 @@ impl ChannelContext { &next_commitment_htlcs, addl_nondust_htlc_count, feerate_per_kw, + include_fee_spike_multiple, dust_exposure_limiting_feerate, max_dust_htlc_exposure_msat, channel_constraints, @@ -5375,6 +5393,7 @@ impl ChannelContext { &next_commitment_htlcs, 0, feerate_per_kw, + false, dust_exposure_limiting_feerate, max_dust_htlc_exposure_msat, channel_constraints, @@ -5417,6 +5436,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5475,6 +5495,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5501,6 +5522,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, 0, new_feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5522,6 +5544,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, 0, new_feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5702,6 +5725,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, feerate_per_kw, + false, dust_exposure_limiting_feerate, ) { stats @@ -5741,6 +5765,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, feerate_per_kw, + false, dust_exposure_limiting_feerate, ) { stats @@ -5788,6 +5813,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, feerate, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5804,6 +5830,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, feerate, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5840,21 +5867,14 @@ impl ChannelContext { if !funding.is_outbound() { // Note that with anchor outputs we are no longer as sensitive to fee spikes, so we don't need // to account for them. - let fee_spike_multiple = - if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 - } else { - 1 - }; - // Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop - let spiked_feerate = feerate.saturating_mul(fee_spike_multiple); let (remote_stats, _remote_htlcs) = self .get_next_remote_commitment_stats( funding, None, include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, - spiked_feerate, + feerate, + true, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -6209,6 +6229,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map(|(remote_stats, _)| remote_stats.available_balances)?; @@ -6230,6 +6251,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .unwrap(); @@ -6758,7 +6780,7 @@ pub(crate) fn get_legacy_default_holder_selected_channel_reserve_satoshis( /// /// This is used both for outbound and inbound channels and has lower bound /// of `dust_limit_satoshis`. -fn get_v2_channel_reserve_satoshis( +pub(crate) fn get_v2_channel_reserve_satoshis( channel_value_satoshis: u64, dust_limit_satoshis: u64, is_0reserve: bool, ) -> u64 { if is_0reserve { @@ -12365,9 +12387,8 @@ where .as_ref() .and_then(|pending_splice| pending_splice.contributions.last()) { - let holder_balance = self - .get_holder_counterparty_balances_floor_incl_fee(&self.funding) - .map(|(h, _)| h) + let spliceable_balance = self + .get_next_splice_out_maximum(&self.funding) .map_err(|e| APIError::ChannelUnavailable { err: format!( "Channel {} cannot be spliced at this time: {}", @@ -12375,7 +12396,7 @@ where e ), })?; - Some(PriorContribution::new(prior.clone(), holder_balance)) + Some(PriorContribution::new(prior.clone(), spliceable_balance)) } else { None } @@ -12471,16 +12492,13 @@ where return contribution; } - let holder_balance = match self - .get_holder_counterparty_balances_floor_incl_fee(&self.funding) - .map(|(holder, _)| holder) - { + let spliceable_balance = match self.get_next_splice_out_maximum(&self.funding) { Ok(balance) => balance, Err(_) => return contribution, }; if let Err(e) = - contribution.net_value_for_initiator_at_feerate(min_rbf_feerate, holder_balance) + contribution.net_value_for_initiator_at_feerate(min_rbf_feerate, spliceable_balance) { log_info!( logger, @@ -12501,7 +12519,7 @@ where min_rbf_feerate, ); contribution - .for_initiator_at_feerate(min_rbf_feerate, holder_balance) + .for_initiator_at_feerate(min_rbf_feerate, spliceable_balance) .expect("feerate compatibility already checked") } @@ -12572,9 +12590,13 @@ where } let our_funding_contribution = contribution.net_value(); - - if let Err(e) = - self.validate_splice_contributions(our_funding_contribution, SignedAmount::ZERO) + let unsigned_contribution = our_funding_contribution.unsigned_abs(); + if let Err(e) = self.get_next_splice_out_maximum(&self.funding) + .and_then(|splice_max| splice_max + .to_sat() + .checked_add_signed(our_funding_contribution.to_sat()) + .ok_or(format!("Our splice-out value of {unsigned_contribution} is greater than the maximum {splice_max}")) + ) { log_error!(logger, "Channel {} cannot be funded: {}", self.context.channel_id(), e); return Err(QuiescentError::FailSplice(self.splice_funding_failed_for(contribution))); @@ -12762,9 +12784,6 @@ where ))); } - self.validate_splice_contributions(our_funding_contribution, their_funding_contribution) - .map_err(|e| ChannelError::WarnAndDisconnect(e))?; - // Rotate the pubkeys using the prev_funding_txid as a tweak let prev_funding_txid = self.funding.get_funding_txid(); let funding_pubkey = match prev_funding_txid { @@ -12780,73 +12799,44 @@ where let mut new_keys = self.funding.get_holder_pubkeys().clone(); new_keys.funding_pubkey = funding_pubkey; - Ok(FundingScope::for_splice( - &self.funding, - &self.context, - our_funding_contribution, - their_funding_contribution, - msg.funding_pubkey, - new_keys, - )) + let new_funding = self + .validate_splice_contributions( + our_funding_contribution, + their_funding_contribution, + msg.funding_pubkey, + new_keys, + ) + .map_err(|e| ChannelError::WarnAndDisconnect(e))?; + + Ok(new_funding) } fn validate_splice_contributions( &self, our_funding_contribution: SignedAmount, their_funding_contribution: SignedAmount, - ) -> Result<(), String> { - if our_funding_contribution.unsigned_abs() > Amount::MAX_MONEY { - return Err(format!( - "Channel {} cannot be spliced; our {} contribution exceeds the total bitcoin supply", - self.context.channel_id(), - our_funding_contribution, - )); - } - - if their_funding_contribution.unsigned_abs() > Amount::MAX_MONEY { - return Err(format!( - "Channel {} cannot be spliced; their {} contribution exceeds the total bitcoin supply", - self.context.channel_id(), - their_funding_contribution, - )); - } + counterparty_funding_pubkey: PublicKey, our_new_holder_keys: ChannelPublicKeys, + ) -> Result { + let candidate_scope = FundingScope::for_splice( + &self.funding, + self.context(), + our_funding_contribution, + their_funding_contribution, + counterparty_funding_pubkey, + our_new_holder_keys, + )?; - let (holder_balance_remaining, counterparty_balance_remaining) = - self.get_holder_counterparty_balances_floor_incl_fee(&self.funding).map_err(|e| { - format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e) - })?; + let (post_splice_holder_balance, post_splice_counterparty_balance) = + self.get_holder_counterparty_balances_floor_incl_fee(&candidate_scope).map_err( + |e| format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e), + )?; - let post_channel_value = self.funding.compute_post_splice_value( - our_funding_contribution.to_sat(), - their_funding_contribution.to_sat(), + let holder_selected_channel_reserve = + Amount::from_sat(candidate_scope.holder_selected_channel_reserve_satoshis); + let counterparty_selected_channel_reserve = Amount::from_sat( + candidate_scope.counterparty_selected_channel_reserve_satoshis.expect("Reserve is set"), ); - let counterparty_selected_channel_reserve = - Amount::from_sat(get_v2_channel_reserve_satoshis( - post_channel_value, - MIN_CHAN_DUST_LIMIT_SATOSHIS, - self.funding - .counterparty_selected_channel_reserve_satoshis - .expect("counterparty reserve is set") - == 0, - )); - let holder_selected_channel_reserve = Amount::from_sat(get_v2_channel_reserve_satoshis( - post_channel_value, - self.context.counterparty_dust_limit_satoshis, - self.funding.holder_selected_channel_reserve_satoshis == 0, - )); // We allow parties to draw from their previous reserve, as long as they satisfy their v2 reserve - if our_funding_contribution != SignedAmount::ZERO { - let post_splice_holder_balance = Amount::from_sat( - holder_balance_remaining.to_sat() - .checked_add_signed(our_funding_contribution.to_sat()) - .ok_or(format!( - "Channel {} cannot be spliced out; our remaining balance {} does not cover our negative funding contribution {}", - self.context.channel_id(), - holder_balance_remaining, - our_funding_contribution, - ))?, - ); - post_splice_holder_balance.checked_sub(counterparty_selected_channel_reserve) .ok_or(format!( "Channel {} cannot be {}; our post-splice channel balance {} is smaller than their selected v2 reserve {}", @@ -12858,17 +12848,6 @@ where } if their_funding_contribution != SignedAmount::ZERO { - let post_splice_counterparty_balance = Amount::from_sat( - counterparty_balance_remaining.to_sat() - .checked_add_signed(their_funding_contribution.to_sat()) - .ok_or(format!( - "Channel {} cannot be spliced out; their remaining balance {} does not cover their negative funding contribution {}", - self.context.channel_id(), - counterparty_balance_remaining, - their_funding_contribution, - ))?, - ); - post_splice_counterparty_balance.checked_sub(holder_selected_channel_reserve) .ok_or(format!( "Channel {} cannot be {}; their post-splice channel balance {} is smaller than our selected v2 reserve {}", @@ -12879,15 +12858,41 @@ where ))?; } - Ok(()) + #[cfg(debug_assertions)] + { + let (old_holder_balance_msat, old_counterparty_balance_msat) = + *self.funding.holder_prev_commitment_tx_balance.lock().unwrap(); + let (new_holder_balance_msat, new_counterparty_balance_msat) = + *candidate_scope.holder_prev_commitment_tx_balance.lock().unwrap(); + if new_holder_balance_msat < counterparty_selected_channel_reserve.to_sat() * 1000 { + debug_assert_eq!(new_holder_balance_msat, old_holder_balance_msat); + } + if new_counterparty_balance_msat < holder_selected_channel_reserve.to_sat() * 1000 { + debug_assert_eq!(new_counterparty_balance_msat, old_counterparty_balance_msat); + } + } + #[cfg(debug_assertions)] + { + let (old_holder_balance_msat, old_counterparty_balance_msat) = + *self.funding.counterparty_prev_commitment_tx_balance.lock().unwrap(); + let (new_holder_balance_msat, new_counterparty_balance_msat) = + *candidate_scope.counterparty_prev_commitment_tx_balance.lock().unwrap(); + if new_holder_balance_msat < counterparty_selected_channel_reserve.to_sat() * 1000 { + debug_assert_eq!(new_holder_balance_msat, old_holder_balance_msat); + } + if new_counterparty_balance_msat < holder_selected_channel_reserve.to_sat() * 1000 { + debug_assert_eq!(new_counterparty_balance_msat, old_counterparty_balance_msat); + } + } + + Ok(candidate_scope) } fn resolve_queued_contribution( &self, feerate: FeeRate, logger: &L, ) -> Result<(Option, Option), ChannelError> { - let holder_balance = self - .get_holder_counterparty_balances_floor_incl_fee(&self.funding) - .map(|(holder, _)| holder) + let spliceable_balance = self + .get_next_splice_out_maximum(&self.funding) .map_err(|e| { log_info!( logger, @@ -12899,9 +12904,9 @@ where }) .ok(); - let net_value = match holder_balance.and_then(|_| self.queued_funding_contribution()) { + let net_value = match spliceable_balance.and_then(|_| self.queued_funding_contribution()) { Some(c) => { - match c.net_value_for_acceptor_at_feerate(feerate, holder_balance.unwrap()) { + match c.net_value_for_acceptor_at_feerate(feerate, spliceable_balance.unwrap()) { Ok(net_value) => Some(net_value), Err(FeeRateAdjustmentError::FeeRateTooHigh { .. }) => { return Err(ChannelError::Abort(AbortReason::FeeRateTooHigh)); @@ -12921,7 +12926,7 @@ where None => None, }; - Ok((net_value, holder_balance)) + Ok((net_value, spliceable_balance)) } pub(crate) fn splice_init( @@ -13064,22 +13069,21 @@ where None => SignedAmount::ZERO, }; - self.validate_splice_contributions(our_funding_contribution, their_funding_contribution) - .map_err(|e| ChannelError::WarnAndDisconnect(e))?; - // Reuse funding pubkeys from the last negotiated candidate since all RBF candidates // for the same splice share the same funding output script. let holder_pubkeys = last_candidate.get_holder_pubkeys().clone(); let counterparty_funding_pubkey = *last_candidate.counterparty_funding_pubkey(); - Ok(FundingScope::for_splice( - &self.funding, - &self.context, - our_funding_contribution, - their_funding_contribution, - counterparty_funding_pubkey, - holder_pubkeys, - )) + let new_funding = self + .validate_splice_contributions( + our_funding_contribution, + their_funding_contribution, + counterparty_funding_pubkey, + holder_pubkeys, + ) + .map_err(|e| ChannelError::WarnAndDisconnect(e))?; + + Ok(new_funding) } pub(crate) fn tx_init_rbf( @@ -13202,8 +13206,6 @@ where Some(value) => SignedAmount::from_sat(value), None => SignedAmount::ZERO, }; - self.validate_splice_contributions(our_funding_contribution, their_funding_contribution) - .map_err(|e| ChannelError::WarnAndDisconnect(e))?; let last_candidate = pending_splice.negotiated_candidates.last().ok_or_else(|| { ChannelError::WarnAndDisconnect("No negotiated splice candidates for RBF".to_owned()) @@ -13211,14 +13213,16 @@ where let holder_pubkeys = last_candidate.get_holder_pubkeys().clone(); let counterparty_funding_pubkey = *last_candidate.counterparty_funding_pubkey(); - Ok(FundingScope::for_splice( - &self.funding, - &self.context, - our_funding_contribution, - their_funding_contribution, - counterparty_funding_pubkey, - holder_pubkeys, - )) + let new_funding = self + .validate_splice_contributions( + our_funding_contribution, + their_funding_contribution, + counterparty_funding_pubkey, + holder_pubkeys, + ) + .map_err(|e| ChannelError::WarnAndDisconnect(e))?; + + Ok(new_funding) } pub(crate) fn tx_ack_rbf( @@ -13303,22 +13307,35 @@ where let our_funding_contribution = funding_negotiation_context.our_funding_contribution; let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis); - self.validate_splice_contributions(our_funding_contribution, their_funding_contribution) - .map_err(|e| ChannelError::WarnAndDisconnect(e))?; let mut new_keys = self.funding.get_holder_pubkeys().clone(); new_keys.funding_pubkey = *new_holder_funding_key; - Ok(FundingScope::for_splice( - &self.funding, - &self.context, - our_funding_contribution, - their_funding_contribution, - msg.funding_pubkey, - new_keys, - )) + let new_funding = self + .validate_splice_contributions( + our_funding_contribution, + their_funding_contribution, + msg.funding_pubkey, + new_keys, + ) + .map_err(|e| ChannelError::WarnAndDisconnect(e))?; + + Ok(new_funding) } + /// The balances returned here should only be used to check that both parties still hold + /// their respective reserves *after* a splice. This function also checks that both local + /// and remote commitments still have at least one output after the splice, which is + /// particularly relevant for zero-reserve channels. + /// + /// Do NOT use this to determine how much the holder can splice out of the channel. The balance + /// of the holder after a splice is not necessarily equal to the funds they can splice out + /// of the channel due to the v2 reserve, and the zero-reserve-at-least-one-output + /// requirements. Note you cannot simply subtract out the reserve, as splicing funds out + /// of the channel changes the reserve the holder must keep in the channel. + /// + /// See [`FundedChannel::get_next_splice_out_maximum`] for the maximum value of the next + /// splice out of the holder's balance. fn get_holder_counterparty_balances_floor_incl_fee( &self, funding: &FundingScope, ) -> Result<(Amount, Amount), String> { @@ -13329,16 +13346,16 @@ where // We are not interested in dust exposure let dust_exposure_limiting_feerate = None; - // Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop - let feerate_per_kw = if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - // Similar to HTLC additions, require the funder to have enough funds reserved for - // fees such that the feerate can jump without rendering the channel useless. - let spike_mul = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32; - self.context.feerate_per_kw.saturating_mul(spike_mul) - } else { - self.context.feerate_per_kw - }; - + // Different dust limits on the local and remote commitments cause the commitment + // transaction fee to be different depending on the commitment, so we grab the floor + // of both balances across both commitments here. + // + // `get_channel_stats` also checks for at least one output on the commitment given + // these parameters. This is particularly relevant for zero-reserve channels. + // + // This "at-least-one-output" check is why we still run both checks on + // zero-fee-commitment channels, even though those channels don't suffer from the + // commitment transaction fee asymmetry. let (local_stats, _local_htlcs) = self .context .get_next_local_commitment_stats( @@ -13346,7 +13363,8 @@ where None, // htlc_candidate include_counterparty_unknown_htlcs, addl_nondust_htlc_count, - feerate_per_kw, + self.context.feerate_per_kw, + true, dust_exposure_limiting_feerate, ) .map_err(|()| "Balance exhausted on local commitment")?; @@ -13358,7 +13376,8 @@ where None, // htlc_candidate include_counterparty_unknown_htlcs, addl_nondust_htlc_count, - feerate_per_kw, + self.context.feerate_per_kw, + true, dust_exposure_limiting_feerate, ) .map_err(|()| "Balance exhausted on remote commitment")?; @@ -13379,6 +13398,56 @@ where Ok((holder_balance_floor, counterparty_balance_floor)) } + /// Determines the maximum value that the holder can splice out of the channel, accounting + /// for the updated reserves after said splice. This maximum also makes sure the local + /// commitment retains at least one output after the splice, which is particularly relevant + /// for zero-reserve channels. + fn get_next_splice_out_maximum(&self, funding: &FundingScope) -> Result { + let include_counterparty_unknown_htlcs = true; + // We are not interested in dust exposure + let dust_exposure_limiting_feerate = None; + + // When reading the available balances, we take the remote's view of the pending + // HTLCs, see `tx_builder` for further details + let (remote_stats, _remote_htlcs) = self + .context + .get_next_remote_commitment_stats( + funding, + None, // htlc_candidate + include_counterparty_unknown_htlcs, + 0, + self.context.feerate_per_kw, + false, + dust_exposure_limiting_feerate, + ) + .map_err(|()| "Balance exhausted on remote commitment")?; + + let next_splice_out_maximum_sat = + remote_stats.available_balances.next_splice_out_maximum_sat; + + #[cfg(debug_assertions)] + { + // After this max splice out, validation passes, accounting for the updated reserves + self.validate_splice_contributions( + SignedAmount::from_sat(-(next_splice_out_maximum_sat as i64)), + SignedAmount::ZERO, + funding.counterparty_funding_pubkey().clone(), + funding.get_holder_pubkeys().clone(), + ) + .unwrap(); + // Splice-out an additional satoshi, and validation fails! + self.validate_splice_contributions( + SignedAmount::from_sat(-((next_splice_out_maximum_sat + 1) as i64)), + SignedAmount::ZERO, + funding.counterparty_funding_pubkey().clone(), + funding.get_holder_pubkeys().clone(), + ) + .unwrap_err(); + } + + Ok(Amount::from_sat(next_splice_out_maximum_sat)) + } + pub fn splice_locked( &mut self, msg: &msgs::SpliceLocked, node_signer: &NS, chain_hash: ChainHash, user_config: &UserConfig, block_height: u32, logger: &L, @@ -13604,6 +13673,9 @@ where next_outbound_htlc_minimum_msat: acc .next_outbound_htlc_minimum_msat .max(e.next_outbound_htlc_minimum_msat), + next_splice_out_maximum_sat: acc + .next_splice_out_maximum_sat + .min(e.next_splice_out_maximum_sat), }) }) } @@ -14140,10 +14212,14 @@ where // balance. If invalid, disconnect and return the contribution so // the user can reclaim their inputs. let our_funding_contribution = contribution.net_value(); - if let Err(e) = self.validate_splice_contributions( - our_funding_contribution, - SignedAmount::ZERO, - ) { + let unsigned_contribution = our_funding_contribution.unsigned_abs(); + if let Err(e) = self.get_next_splice_out_maximum(&self.funding) + .and_then(|splice_max| splice_max + .to_sat() + .checked_add_signed(our_funding_contribution.to_sat()) + .ok_or(format!("Our splice-out value of {unsigned_contribution} is greater than the maximum {splice_max}")) + ) + { let failed = self.splice_funding_failed_for(contribution); return Err(( ChannelError::WarnAndDisconnect(format!( @@ -16802,7 +16878,7 @@ mod tests { use crate::chain::chaininterface::LowerBoundedFeeEstimator; use crate::chain::transaction::OutPoint; use crate::chain::BestBlock; - use crate::ln::chan_utils::{self, commit_tx_fee_sat, ChannelTransactionParameters}; + use crate::ln::chan_utils::{self, commit_tx_fee_sat}; use crate::ln::channel::{ AwaitingChannelReadyFlags, ChannelState, FundedChannel, HTLCUpdateAwaitingACK, InboundHTLCOutput, InboundHTLCState, InboundUpdateAdd, InboundV1Channel, @@ -16820,6 +16896,7 @@ mod tests { use crate::sign::tx_builder::HTLCAmountDirection; #[cfg(ldk_test_vectors)] use crate::sign::{ChannelSigner, EntropySource, InMemorySigner, SignerProvider}; + #[cfg(ldk_test_vectors)] use crate::sync::Mutex; #[cfg(ldk_test_vectors)] use crate::types::features::ChannelTypeFeatures; @@ -17069,7 +17146,7 @@ mod tests { // Make sure when Node A calculates their local commitment transaction, none of the HTLCs pass // the dust limit check. let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amount_msat, outbound: true }; - let local_commit_tx_fee = node_a_chan.context.get_next_local_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let local_commit_tx_fee = node_a_chan.context.get_next_local_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; let local_commit_fee_0_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 0, node_a_chan.funding.get_channel_type()) * 1000; assert_eq!(local_commit_tx_fee, local_commit_fee_0_htlcs); @@ -17078,7 +17155,7 @@ mod tests { node_a_chan.funding.channel_transaction_parameters.is_outbound_from_holder = false; let remote_commit_fee_3_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 3, node_a_chan.funding.get_channel_type()) * 1000; let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amount_msat, outbound: true }; - let remote_commit_tx_fee = node_a_chan.context.get_next_remote_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let remote_commit_tx_fee = node_a_chan.context.get_next_remote_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; assert_eq!(remote_commit_tx_fee, remote_commit_fee_3_htlcs); } @@ -17113,13 +17190,13 @@ mod tests { // counted as dust when it shouldn't be. let htlc_amt_above_timeout = (htlc_timeout_tx_fee_sat + chan.context.holder_dust_limit_satoshis + 1) * 1000; let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amt_above_timeout, outbound: true }; - let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc); // If swapped: this HTLC would be counted as non-dust when it shouldn't be. let dust_htlc_amt_below_success = (htlc_success_tx_fee_sat + chan.context.holder_dust_limit_satoshis - 1) * 1000; let htlc_candidate = HTLCAmountDirection { amount_msat: dust_htlc_amt_below_success, outbound: false }; - let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs); chan.funding.channel_transaction_parameters.is_outbound_from_holder = false; @@ -17127,13 +17204,13 @@ mod tests { // If swapped: this HTLC would be counted as non-dust when it shouldn't be. let dust_htlc_amt_above_timeout = (htlc_timeout_tx_fee_sat + chan.context.counterparty_dust_limit_satoshis + 1) * 1000; let htlc_candidate = HTLCAmountDirection { amount_msat: dust_htlc_amt_above_timeout, outbound: true }; - let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs); // If swapped: this HTLC would be counted as dust when it shouldn't be. let htlc_amt_below_success = (htlc_success_tx_fee_sat + chan.context.counterparty_dust_limit_satoshis - 1) * 1000; let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amt_below_success, outbound: false }; - let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc); } @@ -19240,95 +19317,4 @@ mod tests { assert_eq!(node_a_chan.context.channel_state, ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::THEIR_CHANNEL_READY)); assert!(node_a_chan.check_get_channel_ready(0, &&logger).is_some()); } - - fn get_pre_and_post( - pre_channel_value: u64, our_funding_contribution: i64, their_funding_contribution: i64, - ) -> (u64, u64) { - use crate::ln::channel::{FundingScope, PredictedNextFee}; - - let funding = FundingScope { - value_to_self_msat: 0, - counterparty_selected_channel_reserve_satoshis: None, - holder_selected_channel_reserve_satoshis: 0, - - #[cfg(debug_assertions)] - holder_prev_commitment_tx_balance: Mutex::new((0, 0)), - #[cfg(debug_assertions)] - counterparty_prev_commitment_tx_balance: Mutex::new((0, 0)), - - #[cfg(any(test, fuzzing))] - next_local_fee: Mutex::new(PredictedNextFee::default()), - #[cfg(any(test, fuzzing))] - next_remote_fee: Mutex::new(PredictedNextFee::default()), - - channel_transaction_parameters: ChannelTransactionParameters::test_dummy( - pre_channel_value, - ), - funding_transaction: None, - funding_tx_confirmed_in: None, - funding_tx_confirmation_height: 0, - short_channel_id: None, - minimum_depth_override: None, - }; - let post_channel_value = - funding.compute_post_splice_value(our_funding_contribution, their_funding_contribution); - (pre_channel_value, post_channel_value) - } - - #[test] - fn test_compute_post_splice_value() { - { - // increase, small amounts - let (pre_channel_value, post_channel_value) = get_pre_and_post(9_000, 6_000, 0); - assert_eq!(pre_channel_value, 9_000); - assert_eq!(post_channel_value, 15_000); - } - { - // increase, small amounts - let (pre_channel_value, post_channel_value) = get_pre_and_post(9_000, 4_000, 2_000); - assert_eq!(pre_channel_value, 9_000); - assert_eq!(post_channel_value, 15_000); - } - { - // increase, small amounts - let (pre_channel_value, post_channel_value) = get_pre_and_post(9_000, 0, 6_000); - assert_eq!(pre_channel_value, 9_000); - assert_eq!(post_channel_value, 15_000); - } - { - // decrease, small amounts - let (pre_channel_value, post_channel_value) = get_pre_and_post(15_000, -6_000, 0); - assert_eq!(pre_channel_value, 15_000); - assert_eq!(post_channel_value, 9_000); - } - { - // decrease, small amounts - let (pre_channel_value, post_channel_value) = get_pre_and_post(15_000, -4_000, -2_000); - assert_eq!(pre_channel_value, 15_000); - assert_eq!(post_channel_value, 9_000); - } - { - // increase and decrease - let (pre_channel_value, post_channel_value) = get_pre_and_post(15_000, 4_000, -2_000); - assert_eq!(pre_channel_value, 15_000); - assert_eq!(post_channel_value, 17_000); - } - let base2: u64 = 2; - let huge63i3 = (base2.pow(63) - 3) as i64; - assert_eq!(huge63i3, 9223372036854775805); - assert_eq!(-huge63i3, -9223372036854775805); - { - // increase, large amount - let (pre_channel_value, post_channel_value) = get_pre_and_post(9_000, huge63i3, 3); - assert_eq!(pre_channel_value, 9_000); - assert_eq!(post_channel_value, 9223372036854784807); - } - { - // increase, large amounts - let (pre_channel_value, post_channel_value) = - get_pre_and_post(9_000, huge63i3, huge63i3); - assert_eq!(pre_channel_value, 9_000); - assert_eq!(post_channel_value, 9223372036854784807); - } - } } diff --git a/lightning/src/ln/channel_state.rs b/lightning/src/ln/channel_state.rs index 5547bee8f4c..9fd0df4a1bf 100644 --- a/lightning/src/ln/channel_state.rs +++ b/lightning/src/ln/channel_state.rs @@ -399,6 +399,8 @@ pub struct ChannelDetails { /// an upper-bound. This is intended for use when routing, allowing us to ensure we pick a /// route which is valid. pub next_outbound_htlc_minimum_msat: u64, + /// The maximum value of the next splice out from our channel balance. + pub next_splice_out_maximum_sat: u64, /// The available inbound capacity for the remote peer to send HTLCs to us. This does not /// include any pending HTLCs which are not yet fully resolved (and, thus, whose balance is not /// available for inclusion in new inbound HTLCs). @@ -533,6 +535,7 @@ impl ChannelDetails { outbound_capacity_msat: 0, next_outbound_htlc_limit_msat: 0, next_outbound_htlc_minimum_msat: u64::MAX, + next_splice_out_maximum_sat: 0, } }); let (to_remote_reserve_satoshis, to_self_reserve_satoshis) = @@ -582,6 +585,7 @@ impl ChannelDetails { outbound_capacity_msat: balance.outbound_capacity_msat, next_outbound_htlc_limit_msat: balance.next_outbound_htlc_limit_msat, next_outbound_htlc_minimum_msat: balance.next_outbound_htlc_minimum_msat, + next_splice_out_maximum_sat: balance.next_splice_out_maximum_sat, user_channel_id: context.get_user_id(), confirmations_required: channel.minimum_depth(), confirmations: Some(funding.get_funding_tx_confirmations(best_block_height)), @@ -621,6 +625,7 @@ impl_writeable_tlv_based!(ChannelDetails, { (20, inbound_capacity_msat, required), (21, next_outbound_htlc_minimum_msat, (default_value, 0)), (22, confirmations_required, option), + (23, next_splice_out_maximum_sat, (default_value, u64::from(outbound_capacity_msat.0.unwrap()) / 1000)), (24, force_close_spend_delay, option), (26, is_outbound, required), (28, is_channel_ready, required), @@ -725,6 +730,7 @@ mod tests { outbound_capacity_msat: 24_300, next_outbound_htlc_limit_msat: 20_000, next_outbound_htlc_minimum_msat: 132, + next_splice_out_maximum_sat: 20, inbound_capacity_msat: 42, unspendable_punishment_reserve: Some(8273), confirmations_required: Some(5), diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 73d9a67f50f..38cc193460d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8133,6 +8133,7 @@ impl< outbound_capacity_msat: 0, next_outbound_htlc_limit_msat: 0, next_outbound_htlc_minimum_msat: u64::MAX, + next_splice_out_maximum_sat: 0, } }); let is_in_range = (balances.next_outbound_htlc_minimum_msat diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 20366fe772a..386aa3d92a3 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -192,7 +192,7 @@ impl core::fmt::Display for FundingContributionError { #[derive(Debug, Clone, PartialEq, Eq)] pub(super) struct PriorContribution { contribution: FundingContribution, - /// The holder's balance, used for feerate adjustment. + /// The holder's spliceable balance, used for feerate adjustment. /// /// This value is captured at [`ChannelManager::splice_channel`] time and may become stale /// if balances change before the contribution is used. Staleness is acceptable here because @@ -203,12 +203,12 @@ pub(super) struct PriorContribution { /// /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel /// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed - holder_balance: Amount, + spliceable_balance: Amount, } impl PriorContribution { - pub(super) fn new(contribution: FundingContribution, holder_balance: Amount) -> Self { - Self { contribution, holder_balance } + pub(super) fn new(contribution: FundingContribution, spliceable_balance: Amount) -> Self { + Self { contribution, spliceable_balance } } } @@ -632,14 +632,14 @@ impl FundingContribution { /// `target_feerate`. If dropping change leaves surplus value, that surplus remains in the /// channel contribution. /// - /// For input-less contributions, `holder_balance` must be provided to cover the outputs and + /// For input-less contributions, `spliceable_balance` must be provided to cover the outputs and /// fees from the channel balance. /// /// Returns `None` if the request would require new wallet inputs or cannot accommodate the /// requested feerate. fn amend_without_coin_selection( self, inputs: FundingInputs, outputs: &[TxOut], target_feerate: FeeRate, - max_feerate: FeeRate, holder_balance: Amount, + max_feerate: FeeRate, spliceable_balance: Amount, ) -> Option { // NOTE: The contribution returned is not guaranteed to be valid. We defer doing so until // `compute_feerate_adjustment`. @@ -717,7 +717,7 @@ impl FundingContribution { let new_contribution_at_current_feerate = adjust_for_inputs_and_outputs(self, inputs, outputs)?; let mut new_contribution_at_target_feerate = new_contribution_at_current_feerate - .at_feerate(target_feerate, holder_balance, true) + .at_feerate(target_feerate, spliceable_balance, true) .ok()?; new_contribution_at_target_feerate.max_feerate = max_feerate; @@ -771,7 +771,7 @@ impl FundingContribution { /// /// Returns `Err` if the contribution cannot accommodate the target feerate. fn compute_feerate_adjustment( - &self, target_feerate: FeeRate, holder_balance: Amount, is_initiator: bool, + &self, target_feerate: FeeRate, spliceable_balance: Amount, is_initiator: bool, ) -> Result<(Amount, Option), FeeRateAdjustmentError> { if target_feerate < self.feerate { return Err(FeeRateAdjustmentError::FeeRateTooLow { @@ -864,10 +864,12 @@ impl FundingContribution { let total_cost = target_fee .checked_add(value_removed) .ok_or(FeeRateAdjustmentError::FeeBufferOverflow)?; - if total_cost > holder_balance { + if total_cost > spliceable_balance { return Err(FeeRateAdjustmentError::FeeBufferInsufficient { source: "channel balance - withdrawal outputs", - available: holder_balance.checked_sub(value_removed).unwrap_or(Amount::ZERO), + available: spliceable_balance + .checked_sub(value_removed) + .unwrap_or(Amount::ZERO), required: target_fee, }); } @@ -879,10 +881,10 @@ impl FundingContribution { /// estimate, and feerate. Returns the adjusted contribution, or an error if the feerate /// can't be accommodated. fn at_feerate( - mut self, feerate: FeeRate, holder_balance: Amount, is_initiator: bool, + mut self, feerate: FeeRate, spliceable_balance: Amount, is_initiator: bool, ) -> Result { let (new_estimated_fee, new_change) = - self.compute_feerate_adjustment(feerate, holder_balance, is_initiator)?; + self.compute_feerate_adjustment(feerate, spliceable_balance, is_initiator)?; match new_change { Some(value) => self.change_output.as_mut().unwrap().value = value, None => self.change_output = None, @@ -899,9 +901,9 @@ impl FundingContribution { /// This adjusts the change output so the acceptor pays their target fee at the target /// feerate. pub(super) fn for_acceptor_at_feerate( - self, feerate: FeeRate, holder_balance: Amount, + self, feerate: FeeRate, spliceable_balance: Amount, ) -> Result { - self.at_feerate(feerate, holder_balance, false) + self.at_feerate(feerate, spliceable_balance, false) } /// Adjusts the contribution's change output for the minimum RBF feerate. @@ -910,9 +912,9 @@ impl FundingContribution { /// below the minimum RBF feerate, this adjusts the change output so the initiator pays fees /// at the minimum RBF feerate. pub(super) fn for_initiator_at_feerate( - self, feerate: FeeRate, holder_balance: Amount, + self, feerate: FeeRate, spliceable_balance: Amount, ) -> Result { - self.at_feerate(feerate, holder_balance, true) + self.at_feerate(feerate, spliceable_balance, true) } /// Returns the net value at the given target feerate without mutating `self`. @@ -921,10 +923,10 @@ impl FundingContribution { /// can't be accommodated) and computes the adjusted net value (returning `Ok` with the value /// accounting for the target feerate). fn net_value_at_feerate( - &self, target_feerate: FeeRate, holder_balance: Amount, is_initiator: bool, + &self, target_feerate: FeeRate, spliceable_balance: Amount, is_initiator: bool, ) -> Result { let (new_estimated_fee, new_change) = - self.compute_feerate_adjustment(target_feerate, holder_balance, is_initiator)?; + self.compute_feerate_adjustment(target_feerate, spliceable_balance, is_initiator)?; let prev_fee = self .estimated_fee @@ -952,17 +954,17 @@ impl FundingContribution { /// Returns the net value at the given target feerate without mutating `self`, /// assuming acceptor fee responsibility. pub(super) fn net_value_for_acceptor_at_feerate( - &self, target_feerate: FeeRate, holder_balance: Amount, + &self, target_feerate: FeeRate, spliceable_balance: Amount, ) -> Result { - self.net_value_at_feerate(target_feerate, holder_balance, false) + self.net_value_at_feerate(target_feerate, spliceable_balance, false) } /// Returns the net value at the given target feerate without mutating `self`, /// assuming initiator fee responsibility. pub(super) fn net_value_for_initiator_at_feerate( - &self, target_feerate: FeeRate, holder_balance: Amount, + &self, target_feerate: FeeRate, spliceable_balance: Amount, ) -> Result { - self.net_value_at_feerate(target_feerate, holder_balance, true) + self.net_value_at_feerate(target_feerate, spliceable_balance, true) } /// The net value contributed to a channel by the splice. @@ -1059,13 +1061,13 @@ impl FundingBuilderInner { fn build_from_prior_contribution( &mut self, contribution: PriorContribution, ) -> Result { - let PriorContribution { contribution, holder_balance } = contribution; + let PriorContribution { contribution, spliceable_balance } = contribution; if self.request_matches_prior(&contribution) { // Same request, but the feerate may have changed. Adjust the prior contribution // to the new feerate if possible. return contribution - .for_initiator_at_feerate(self.feerate, holder_balance) + .for_initiator_at_feerate(self.feerate, spliceable_balance) .map(|mut adjusted| { adjusted.max_feerate = self.max_feerate; adjusted @@ -1084,7 +1086,7 @@ impl FundingBuilderInner { &self.outputs, self.feerate, self.max_feerate, - holder_balance, + spliceable_balance, ) .ok_or_else(|| FundingContributionError::MissingCoinSelectionSource); } @@ -2181,8 +2183,8 @@ mod tests { }; // Balance of 55,000 sats can't cover outputs (50,000) + target_fee at 50k sat/kwu. - let holder_balance = Amount::from_sat(55_000); - let result = contribution.for_acceptor_at_feerate(target_feerate, holder_balance); + let spliceable_balance = Amount::from_sat(55_000); + let result = contribution.for_acceptor_at_feerate(target_feerate, spliceable_balance); assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); } @@ -2601,8 +2603,8 @@ mod tests { }; // Balance of 40,000 sats is less than outputs (50,000) + target_fee. - let holder_balance = Amount::from_sat(40_000); - let result = contribution.for_acceptor_at_feerate(target_feerate, holder_balance); + let spliceable_balance = Amount::from_sat(40_000); + let result = contribution.for_acceptor_at_feerate(target_feerate, spliceable_balance); assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); } @@ -2627,9 +2629,9 @@ mod tests { }; // Balance of 100,000 sats is more than outputs (50,000) + target_fee. - let holder_balance = Amount::from_sat(100_000); + let spliceable_balance = Amount::from_sat(100_000); let contribution = - contribution.for_acceptor_at_feerate(target_feerate, holder_balance).unwrap(); + contribution.for_acceptor_at_feerate(target_feerate, spliceable_balance).unwrap(); let expected_target_fee = estimate_transaction_fee(&[], &outputs, None, false, true, target_feerate); assert_eq!(contribution.estimated_fee, expected_target_fee); @@ -2657,8 +2659,9 @@ mod tests { }; // Balance of 40,000 sats is less than outputs (50,000) + target_fee. - let holder_balance = Amount::from_sat(40_000); - let result = contribution.net_value_for_acceptor_at_feerate(target_feerate, holder_balance); + let spliceable_balance = Amount::from_sat(40_000); + let result = + contribution.net_value_for_acceptor_at_feerate(target_feerate, spliceable_balance); assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); } diff --git a/lightning/src/ln/htlc_reserve_unit_tests.rs b/lightning/src/ln/htlc_reserve_unit_tests.rs index aaf81b87be7..e288c4ec34d 100644 --- a/lightning/src/ln/htlc_reserve_unit_tests.rs +++ b/lightning/src/ln/htlc_reserve_unit_tests.rs @@ -3,14 +3,15 @@ use crate::events::{ClosureReason, Event, HTLCHandlingFailureType, PaymentPurpose}; use crate::ln::chan_utils::{ self, commit_tx_fee_sat, commitment_tx_base_weight, second_stage_tx_fees_sat, - shared_anchor_script_pubkey, CommitmentTransaction, COMMITMENT_TX_WEIGHT_PER_HTLC, - TRUC_CHILD_MAX_WEIGHT, + shared_anchor_script_pubkey, CommitmentTransaction, HTLCOutputInCommitment, + COMMITMENT_TX_WEIGHT_PER_HTLC, TRUC_CHILD_MAX_WEIGHT, }; use crate::ln::channel::{ get_holder_selected_channel_reserve_satoshis, Channel, ANCHOR_OUTPUT_VALUE_SATOSHI, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, MIN_CHAN_DUST_LIMIT_SATOSHIS, }; +use crate::ln::channel_state::ChannelDetails; use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder, TrustedChannelFeatures}; use crate::ln::functional_test_utils::*; use crate::ln::msgs::{self, BaseMessageHandler, ChannelMessageHandler, MessageSendEvent}; @@ -887,7 +888,7 @@ pub fn do_test_fee_spike_buffer(cfg: Option, htlc_fails: bool) { // Build the remote commitment transaction so we can sign it, and then later use the // signature for the commitment_signed message. - let accepted_htlc_info = chan_utils::HTLCOutputInCommitment { + let accepted_htlc_info = HTLCOutputInCommitment { offered: false, amount_msat: payment_amt_msat, cltv_expiry: htlc_cltv, @@ -2142,7 +2143,7 @@ pub fn do_test_dust_limit_fee_accounting(can_afford: bool) { let (_payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], HTLC_AMT_SAT * 1000); // Grab a snapshot of these HTLCs to manually build the commitment transaction later... - let accepted_htlc = chan_utils::HTLCOutputInCommitment { + let accepted_htlc = HTLCOutputInCommitment { offered: false, amount_msat: HTLC_AMT_SAT * 1000, // Hard-coded to match the expected value @@ -2256,7 +2257,7 @@ pub fn do_test_dust_limit_fee_accounting(can_afford: bool) { &channel_type, ); - let accepted_htlc_info = chan_utils::HTLCOutputInCommitment { + let accepted_htlc_info = HTLCOutputInCommitment { offered: false, amount_msat: HTLC_AMT_SAT * 1000, cltv_expiry, @@ -2581,7 +2582,7 @@ fn test_0reserve_no_outputs() { do_test_0reserve_no_outputs_p2a_anchor(); } -fn setup_0reserve_no_outputs_channels<'a, 'b, 'c, 'd>( +pub(crate) fn setup_0reserve_no_outputs_channels<'a, 'b, 'c, 'd>( nodes: &'a Vec>, channel_value_sat: u64, dust_limit_satoshis: u64, ) -> (ChannelId, Transaction) { let node_a_id = nodes[0].node.get_our_node_id(); @@ -2837,21 +2838,30 @@ fn do_test_0reserve_no_outputs_legacy(no_outputs_case: LegacyChannelsNoOutputs) return; } + let htlcs_in_commitment = vec![HTLCOutputInCommitment { + offered: false, + amount_msat: receiver_amount_msat, + cltv_expiry: htlc_cltv, + payment_hash, + transaction_output_index: Some(1), + }]; + manually_trigger_update_fail_htlc( &nodes, channel_id, - channel_value_sat, + channel_value_sat * 1000, dust_limit_satoshis, - receiver_amount_msat, - htlc_cltv, payment_hash, + htlcs_in_commitment, + false, ); } } fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( - nodes: &'a Vec>, channel_id: ChannelId, channel_value_sat: u64, - dust_limit_satoshis: u64, receiver_amount_msat: u64, htlc_cltv: u32, payment_hash: PaymentHash, + nodes: &'a Vec>, channel_id: ChannelId, value_to_self_msat: u64, + dust_limit_satoshis: u64, payment_hash: PaymentHash, + htlcs_in_commitment: Vec, can_afford_but_reserve_is_breached: bool, ) { let node_a_id = nodes[0].node.get_our_node_id(); let node_b_id = nodes[1].node.get_our_node_id(); @@ -2863,8 +2873,6 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( let feerate_per_kw = get_feerate!(nodes[0], nodes[1], channel_id); - const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1; - let (local_secret, next_local_point) = { let per_peer_state = nodes[0].node.per_peer_state.read().unwrap(); let chan_lock = per_peer_state.get(&node_b_id).unwrap().lock().unwrap(); @@ -2872,36 +2880,29 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( chan_lock.channel_by_id.get(&channel_id).and_then(Channel::as_funded).unwrap(); let chan_signer = local_chan.get_signer(); // Make the signer believe we validated another commitment, so we can release the secret + let commit_number = chan_signer.get_enforcement_state().last_holder_commitment; chan_signer.get_enforcement_state().last_holder_commitment -= 1; ( - chan_signer.release_commitment_secret(INITIAL_COMMITMENT_NUMBER).unwrap(), - chan_signer.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 2, &secp_ctx).unwrap(), + chan_signer.release_commitment_secret(commit_number).unwrap(), + chan_signer.get_per_commitment_point(commit_number - 2, &secp_ctx).unwrap(), ) }; - let remote_point = { + let (remote_commit_number, remote_point) = { let per_peer_lock; let mut peer_state_lock; let channel = get_channel_ref!(nodes[1], nodes[0], per_peer_lock, peer_state_lock, channel_id); let chan_signer = channel.as_funded().unwrap().get_signer(); - chan_signer.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx).unwrap() + let commit_number = chan_signer.get_enforcement_state().last_holder_commitment; + let remote_point = + chan_signer.get_per_commitment_point(commit_number - 1, &secp_ctx).unwrap(); + (commit_number - 1, remote_point) }; // Build the remote commitment transaction so we can sign it, and then later use the // signature for the commitment_signed message. - let accepted_htlc_info = chan_utils::HTLCOutputInCommitment { - offered: false, - amount_msat: receiver_amount_msat, - cltv_expiry: htlc_cltv, - payment_hash, - transaction_output_index: Some(1), - }; - - let local_chan_balance_msat = channel_value_sat * 1000; - let commitment_number = INITIAL_COMMITMENT_NUMBER - 1; - let res = { let per_peer_lock; let mut peer_state_lock; @@ -2912,12 +2913,12 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( let (commitment_tx, _stats) = SpecTxBuilder {}.build_commitment_transaction( false, - commitment_number, + remote_commit_number, &remote_point, &channel.funding().channel_transaction_parameters, &secp_ctx, - local_chan_balance_msat, - vec![accepted_htlc_info], + value_to_self_msat, + htlcs_in_commitment, feerate_per_kw, dust_limit_satoshis, &nodes[0].logger, @@ -2968,11 +2969,13 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( }, _ => panic!("Unexpected event"), }; - nodes[1].logger.assert_log( - "lightning::ln::channel", - "Attempting to fail HTLC due to balance exhausted on remote commitment".to_string(), - 1, - ); + let log_string = + if can_afford_but_reserve_is_breached { + String::from("Attempting to fail HTLC due to fee spike buffer violation. Rebalancing is required.") + } else { + String::from("Attempting to fail HTLC due to balance exhausted on remote commitment") + }; + nodes[1].logger.assert_log("lightning::ln::channel", log_string, 1); check_added_monitors(&nodes[1], 3); } @@ -3406,3 +3409,432 @@ fn test_0reserve_zero_conf_combined() { assert_eq!(node_1_max_htlc, node_0_max_htlc - node_1_reserve * 1000); send_payment(&nodes[1], &[&nodes[0]], node_1_max_htlc); } + +#[xtest(feature = "_externalize_tests")] +fn test_0reserve_outbound_vs_available_capacity_outbound_htlc_limit() { + let mut config = test_default_channel_config(); + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + config.channel_handshake_config.announced_channel_max_inbound_htlc_value_in_flight_percentage = + 100; + + let channel_type = ChannelTypeFeatures::only_static_remote_key(); + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let _node_a_id = nodes[0].node.get_our_node_id(); + let _node_b_id = nodes[1].node.get_our_node_id(); + + const FEERATE: u32 = 253; + const MULTIPLE: u32 = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32; + const SPIKED_FEERATE: u32 = FEERATE * MULTIPLE; + const DUST_LIMIT_MSAT: u64 = 546 * 1000; + const CHANNEL_VALUE_MSAT: u64 = 10_000 * 1000; + const NODE_0_VALUE_TO_SELF_MSAT: u64 = 5000 * 1000; + const NODE_1_VALUE_TO_SELF_MSAT: u64 = 5000 * 1000; + + // Find the HTLC amount that will be non-dust at the current feerate, but dust at the spiked feerate + const SPIKED_DUST_HTLC_MSAT: u64 = 880 * 1000; + const HTLC_SPIKE_DUST_LIMIT_MSAT: u64 = 881 * 1000; + let htlc_timeout_spike_tx_fee_msat = + second_stage_tx_fees_sat(&channel_type, SPIKED_FEERATE).1 * 1000; + assert_eq!(HTLC_SPIKE_DUST_LIMIT_MSAT, DUST_LIMIT_MSAT + htlc_timeout_spike_tx_fee_msat); + + let real_htlc_timeout_tx_fee_msat = second_stage_tx_fees_sat(&channel_type, FEERATE).1 * 1000; + let real_htlc_timeout_dust_limit_msat = DUST_LIMIT_MSAT + real_htlc_timeout_tx_fee_msat; + + let (channel_id, _funding_tx) = setup_0reserve_no_outputs_channels( + &nodes, + CHANNEL_VALUE_MSAT / 1000, + DUST_LIMIT_MSAT / 1000, + ); + assert_eq!(nodes[0].node.list_channels()[0].channel_type.as_ref().unwrap(), &channel_type); + + // Balance the channel so each side has 5_000 sats + send_payment(&nodes[0], &[&nodes[1]], NODE_1_VALUE_TO_SELF_MSAT); + + let count_total_htlcs = |details: &ChannelDetails| { + details.pending_outbound_htlcs.len() + details.pending_inbound_htlcs.len() + }; + let count_node_0_nondust_htlcs = || { + let mut txs = get_local_commitment_txn!(nodes[0], channel_id); + let commitment_tx = &txs[0]; + commitment_tx + .output + .iter() + .filter(|output| output.value.to_sat() * 1000 == SPIKED_DUST_HTLC_MSAT) + .count() + }; + let count_node_1_nondust_htlcs = || { + let mut txs = get_local_commitment_txn!(nodes[1], channel_id); + let commitment_tx = &txs[0]; + commitment_tx + .output + .iter() + .filter(|output| output.value.to_sat() * 1000 == SPIKED_DUST_HTLC_MSAT) + .count() + }; + + // Sanity check + { + let reserved_fee_sat = commit_tx_fee_sat(SPIKED_FEERATE, 2, &channel_type); + let node_0_outbound_capacity_msat = NODE_0_VALUE_TO_SELF_MSAT; + let node_0_available_capacity_msat = + node_0_outbound_capacity_msat - reserved_fee_sat * 1000; + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!(node_0_details.outbound_capacity_msat, node_0_outbound_capacity_msat); + assert_eq!(node_0_details.next_outbound_htlc_limit_msat, node_0_available_capacity_msat); + assert_eq!(count_total_htlcs(&node_0_details), 0); + assert_eq!(count_node_0_nondust_htlcs(), 0); + } + + // Route 3 880sat HTLCs from node 0 to node 1 + for i in 1..4 { + route_payment(&nodes[0], &[&nodes[1]], SPIKED_DUST_HTLC_MSAT); + + let max_reserved_fee_msat = commit_tx_fee_sat(SPIKED_FEERATE, 2 + i, &channel_type) * 1000; + let node_0_outbound_capacity_msat = + NODE_0_VALUE_TO_SELF_MSAT - SPIKED_DUST_HTLC_MSAT * i as u64; + let node_0_available_capacity_msat = node_0_outbound_capacity_msat - max_reserved_fee_msat; + // Node 0 can send non-dust HTLCs throughout + assert!(node_0_available_capacity_msat >= HTLC_SPIKE_DUST_LIMIT_MSAT); + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!(node_0_details.outbound_capacity_msat, node_0_outbound_capacity_msat); + assert_eq!(node_0_details.next_outbound_htlc_limit_msat, node_0_available_capacity_msat); + assert_eq!(count_total_htlcs(&node_0_details), i); + assert_eq!(count_node_0_nondust_htlcs(), i); + } + + // Route one last 880sat HTLC, after which node 0 can only send dust HTLCs + route_payment(&nodes[0], &[&nodes[1]], SPIKED_DUST_HTLC_MSAT); + + let node_0_details = &nodes[0].node.list_channels()[0]; + let local_nondust_htlc_count = 4; + assert_eq!(count_total_htlcs(&node_0_details), local_nondust_htlc_count); + assert_eq!(count_node_0_nondust_htlcs(), local_nondust_htlc_count); + + let node_0_outbound_capacity_msat = + NODE_0_VALUE_TO_SELF_MSAT - SPIKED_DUST_HTLC_MSAT * local_nondust_htlc_count as u64; + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!(node_0_details.outbound_capacity_msat, node_0_outbound_capacity_msat); + + // Node 0 can only send dust HTLCs + let min_reserved_fee_msat = + commit_tx_fee_sat(SPIKED_FEERATE, local_nondust_htlc_count + 1, &channel_type) * 1000; + let node_0_available_capacity_msat = node_0_outbound_capacity_msat - min_reserved_fee_msat; + let node_0_details = &nodes[0].node.list_channels()[0]; + assert!(node_0_details.next_outbound_htlc_limit_msat < real_htlc_timeout_dust_limit_msat); + assert_eq!(node_0_details.next_outbound_htlc_limit_msat, node_0_available_capacity_msat); + + // Route a dust HTLC, and confirm node 0's main output is now below its dust limit + let current_commit_tx_fee_msat = + commit_tx_fee_sat(FEERATE, local_nondust_htlc_count, &channel_type) * 1000; + let to_local_msat = NODE_0_VALUE_TO_SELF_MSAT + - SPIKED_DUST_HTLC_MSAT * local_nondust_htlc_count as u64 + - current_commit_tx_fee_msat; + assert!(to_local_msat > DUST_LIMIT_MSAT); + assert!(to_local_msat - 600_000 < DUST_LIMIT_MSAT); + assert!(600_000 <= node_0_available_capacity_msat); + + route_payment(&nodes[0], &[&nodes[1]], 600_000); + + let node_0_balance_before_fee_msat = + NODE_0_VALUE_TO_SELF_MSAT - SPIKED_DUST_HTLC_MSAT * 4 - 600_000; + assert_eq!( + nodes[0].node.list_channels()[0].outbound_capacity_msat, + node_0_balance_before_fee_msat + ); + assert_eq!( + nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat, + node_0_balance_before_fee_msat - min_reserved_fee_msat + ); + assert_eq!(count_node_0_nondust_htlcs(), local_nondust_htlc_count); + assert_eq!(count_node_1_nondust_htlcs(), local_nondust_htlc_count); + + // Route 4 880sat HTLCs from node 1 to node 0 + for i in 1..5 { + route_payment(&nodes[1], &[&nodes[0]], SPIKED_DUST_HTLC_MSAT); + + let node_1_outbound_capacity_msat = + NODE_1_VALUE_TO_SELF_MSAT - SPIKED_DUST_HTLC_MSAT * i as u64; + assert!(node_1_outbound_capacity_msat >= HTLC_SPIKE_DUST_LIMIT_MSAT); + let node_1_details = &nodes[1].node.list_channels()[0]; + assert_eq!(node_1_details.outbound_capacity_msat, node_1_outbound_capacity_msat); + assert_eq!(node_1_details.next_outbound_htlc_limit_msat, node_1_outbound_capacity_msat); + + // Sending the greatest dust HTLC still lands node 1's main output above its dust limit + assert!( + node_1_outbound_capacity_msat - (HTLC_SPIKE_DUST_LIMIT_MSAT - 1) >= DUST_LIMIT_MSAT + ); + + let nondust_htlc_count = 4 + i; + // At the current feerate, 880sat HTLCs are present on both commitments + assert_eq!(count_node_0_nondust_htlcs(), nondust_htlc_count); + assert_eq!(count_node_1_nondust_htlcs(), nondust_htlc_count); + + // Node 0's outbound capacity does not budge, and its available capacity is 0 + // TODO: Node 0 should really be rejecting HTLCs here, but it only checks against the spiked buffer + // if it is the fundee, and here it is the funder. + assert!( + node_0_balance_before_fee_msat + >= commit_tx_fee_sat(FEERATE, nondust_htlc_count, &channel_type) + ); + assert_eq!( + nodes[0].node.list_channels()[0].outbound_capacity_msat, + node_0_balance_before_fee_msat + ); + assert_eq!(nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat, 0); + } + + // Route another 880sat HTLC + let (preimage, _hash, _secret, _id) = + route_payment(&nodes[1], &[&nodes[0]], SPIKED_DUST_HTLC_MSAT); + + let node_1_outbound_capacity_msat = NODE_1_VALUE_TO_SELF_MSAT - 5 * SPIKED_DUST_HTLC_MSAT; + let node_1_details = &nodes[1].node.list_channels()[0]; + assert_eq!(node_1_details.outbound_capacity_msat, node_1_outbound_capacity_msat); + // At this point, sending the greatest dust HTLC pushes node 1's main output below the dust limit + assert!( + node_1_outbound_capacity_msat.saturating_sub(HTLC_SPIKE_DUST_LIMIT_MSAT - 1) + < DUST_LIMIT_MSAT + ); + // So we subtract the dust limit from node 1's outbound capacity to arrive at node 1's available capacity + let node_1_available_capacity_msat = node_1_outbound_capacity_msat - DUST_LIMIT_MSAT; + assert_eq!( + nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat, + node_1_available_capacity_msat + ); + + // A few sanity checks + let nondust_htlc_count = 9; + assert_eq!(count_node_0_nondust_htlcs(), nondust_htlc_count); + assert_eq!(count_node_1_nondust_htlcs(), nondust_htlc_count); + // Node 0, the funder, can still afford all the HTLCs on the commitments + let node_0_balance_before_fee_msat = nodes[0].node.list_channels()[0].outbound_capacity_msat; + assert!( + node_0_balance_before_fee_msat + >= commit_tx_fee_sat(FEERATE, nondust_htlc_count, &channel_type) + ); + + // Node 0 claims an 880sat HTLC, which brings its output back above its dust limit + claim_payment(&nodes[1], &[&nodes[0]], preimage); + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!( + node_0_details.outbound_capacity_msat, + node_0_balance_before_fee_msat + SPIKED_DUST_HTLC_MSAT + ); + + // Node 1 is now free to withdraw all of its channel balance + let node_1_details = &nodes[1].node.list_channels()[0]; + assert_eq!(node_1_details.outbound_capacity_msat, node_1_outbound_capacity_msat); + assert_eq!( + nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat, + node_1_outbound_capacity_msat + ); + + send_payment(&nodes[1], &[&nodes[0]], node_1_outbound_capacity_msat); + + let node_1_details = &nodes[1].node.list_channels()[0]; + assert_eq!(node_1_details.outbound_capacity_msat, 0); + assert_eq!(nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat, 0); +} + +/// Make sure that we do not account for HTLCs going from non-dust to dust at the spiked feerate +/// when checking the fee spike buffer in `can_accept_incoming_htlc`. This is required to make sure +/// that we can afford *any* increase in the feerate between 1x to 2x, instead of checking whether +/// we can afford only the 2x increase in the feerate. +#[xtest(feature = "_externalize_tests")] +fn test_fail_cannot_afford_dust_htlcs_at_spike_multiple_if_nondust_at_base_feerate() { + let mut config = test_default_channel_config(); + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + config.channel_handshake_config.announced_channel_max_inbound_htlc_value_in_flight_percentage = + 100; + + let channel_type = ChannelTypeFeatures::only_static_remote_key(); + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_a_id = nodes[0].node.get_our_node_id(); + let _node_b_id = nodes[1].node.get_our_node_id(); + + const FEERATE: u32 = 253; + const MULTIPLE: u32 = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32; + const SPIKED_FEERATE: u32 = FEERATE * MULTIPLE; + const DUST_LIMIT_MSAT: u64 = 354 * 1000; + const CHANNEL_VALUE_MSAT: u64 = 10_000 * 1000; + const NODE_0_VALUE_TO_SELF_MSAT: u64 = 5_000 * 1000; + const NODE_1_VALUE_TO_SELF_MSAT: u64 = 5_000 * 1000; + const CHANNEL_RESERVE_MSAT: u64 = 1_000 * 1_000; + + let channel_id = create_announced_chan_between_nodes_with_value( + &nodes, + 0, + 1, + CHANNEL_VALUE_MSAT / 1000, + NODE_1_VALUE_TO_SELF_MSAT, + ) + .2; + assert_eq!(nodes[0].node.list_channels()[0].channel_type.as_ref().unwrap(), &channel_type); + + // Find the HTLC amount that will be non-dust at the current feerate, + // but dust at the spiked feerate. + const SPIKED_DUST_HTLC_MSAT: u64 = 688 * 1000; + const HTLC_SPIKE_DUST_LIMIT_MSAT: u64 = 689 * 1000; + // When checking the fee spike buffer in `can_accept_incoming_htlc`, we check the remote + // commitment, hence inbound HTLCs will be offered HTLCs, and use the timeout dust limit. + let htlc_timeout_spike_tx_fee_msat = + second_stage_tx_fees_sat(&channel_type, SPIKED_FEERATE).1 * 1000; + assert_eq!(HTLC_SPIKE_DUST_LIMIT_MSAT, DUST_LIMIT_MSAT + htlc_timeout_spike_tx_fee_msat); + + // Calculate here the dust limit at the current feerate so we know when node 0 cannot send + // any further non-dust HTLCs at the current feerate. + let htlc_timeout_tx_fee_msat = second_stage_tx_fees_sat(&channel_type, FEERATE).1 * 1000; + let htlc_dust_limit_msat = DUST_LIMIT_MSAT + htlc_timeout_tx_fee_msat; + // Make sure the HTLC will be non-dust at the current feerate + assert!(SPIKED_DUST_HTLC_MSAT > htlc_dust_limit_msat); + + // Place a few non-dust HTLCs on the commitment, these HTLCs would get trimmed upon a 2x + // increase in the feerate. + let mut sent_htlcs_count: usize = 0; + let mut payment_hashes = Vec::new(); + while nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat >= htlc_dust_limit_msat { + let (_preimage, hash, _secret, _id) = + route_payment(&nodes[0], &[&nodes[1]], SPIKED_DUST_HTLC_MSAT); + payment_hashes.push(hash); + sent_htlcs_count += 1; + } + assert_eq!(sent_htlcs_count, 4); + + // Check the outbound and available capacities + let node_0_outbound_capacity_msat = NODE_0_VALUE_TO_SELF_MSAT + - sent_htlcs_count as u64 * SPIKED_DUST_HTLC_MSAT + - CHANNEL_RESERVE_MSAT; + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!(node_0_details.outbound_capacity_msat, node_0_outbound_capacity_msat); + // Node 0 can now only send dust HTLCs, so we reserve the fees for a single additional + // inbound non-dust HTLC. + let min_reserved_fee_msat = + commit_tx_fee_sat(SPIKED_FEERATE, sent_htlcs_count + 1, &channel_type) * 1000; + let node_0_available_capacity_msat = node_0_outbound_capacity_msat - min_reserved_fee_msat; + assert_eq!(node_0_details.next_outbound_htlc_limit_msat, node_0_available_capacity_msat); + + // Then send an identical, 5th non-dust HTLC, bypass the validation from the holder, and + // check that the counterparty fails it due to a fee spike buffer violation. + + // First check the maths + + // Node 0 can afford an exact 2x increase in the feerate + let spiked_commit_tx_fee_msat = commit_tx_fee_sat(SPIKED_FEERATE, 0, &channel_type) * 1000; + assert!((node_0_outbound_capacity_msat - SPIKED_DUST_HTLC_MSAT) + .checked_sub(spiked_commit_tx_fee_msat) + .is_some()); + // Node 0 can afford a 5th non-dust HTLC at the current feerate, so `update_add_htlc` + // validation will pass. + let real_commit_tx_fee_msat = commit_tx_fee_sat(FEERATE, 5, &channel_type) * 1000; + assert!((node_0_outbound_capacity_msat - SPIKED_DUST_HTLC_MSAT) + .checked_sub(real_commit_tx_fee_msat) + .is_some()); + // But we don't account for the HTLC trimming effect of the spike multiple feerate increase, + // so the 5th HTLC should be rejected at `can_accept_incoming_htlc`! + let expected_commit_tx_fee_msat = commit_tx_fee_sat(SPIKED_FEERATE, 5, &channel_type) * 1000; + assert!((node_0_outbound_capacity_msat - SPIKED_DUST_HTLC_MSAT) + .checked_sub(expected_commit_tx_fee_msat) + .is_none()); + + // Then run the experiment + + let sender_amount_msat = node_0_available_capacity_msat; + let receiver_amount_msat = SPIKED_DUST_HTLC_MSAT; + let (route, payment_hash, _, payment_secret) = + get_route_and_payment_hash!(nodes[0], nodes[1], sender_amount_msat); + let secp_ctx = Secp256k1::new(); + let session_priv = SecretKey::from_slice(&[42; 32]).unwrap(); + let cur_height = nodes[0].node.best_block.read().unwrap().height + 1; + let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv); + let recipient_onion_fields = + RecipientOnionFields::secret_only(payment_secret, sender_amount_msat); + let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::test_build_onion_payloads( + &route.paths[0], + &recipient_onion_fields, + cur_height, + &None, + None, + None, + ) + .unwrap(); + assert_eq!(htlc_msat, sender_amount_msat); + let onion_packet = + onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash) + .unwrap(); + let msg = msgs::UpdateAddHTLC { + channel_id, + htlc_id: sent_htlcs_count as u64, + amount_msat: receiver_amount_msat, + payment_hash, + cltv_expiry: htlc_cltv, + onion_routing_packet: onion_packet, + skimmed_fee_msat: None, + blinding_point: None, + hold_htlc: None, + accountable: None, + }; + + nodes[1].node.handle_update_add_htlc(node_a_id, &msg); + + let htlcs_in_tx = vec![ + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x75).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(0), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x64).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(1), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash, + amount_msat: 688_000, + transaction_output_index: Some(2), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x72).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(3), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x66).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(4), + }, + ]; + + manually_trigger_update_fail_htlc( + &nodes, + channel_id, + NODE_0_VALUE_TO_SELF_MSAT, + DUST_LIMIT_MSAT / 1000, + payment_hash, + htlcs_in_tx, + true, + ); +} diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index fa22ccb61c7..173273b57e0 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -16,7 +16,8 @@ use crate::chain::ChannelMonitorUpdateStatus; use crate::events::{ClosureReason, Event, FundingInfo, HTLCHandlingFailureType}; use crate::ln::chan_utils; use crate::ln::channel::{ - CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, + ANCHOR_OUTPUT_VALUE_SATOSHI, CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY, + FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, }; use crate::ln::channelmanager::{provided_init_features, PaymentId, BREAKDOWN_TIMEOUT}; use crate::ln::functional_test_utils::*; @@ -4045,8 +4046,8 @@ fn do_test_splice_pending_htlcs(config: UserConfig) { format!("Channel {} cannot accept funding contribution", channel_id); assert_eq!(error, APIError::APIMisuseError { err: cannot_accept_contribution }); let cannot_be_funded = format!( - "Channel {} cannot be funded: Channel {} cannot be spliced out; our post-splice channel balance {} is smaller than their selected v2 reserve {}", - channel_id, channel_id, post_splice_reserve - Amount::ONE_SAT, post_splice_reserve + "Channel {} cannot be funded: Our splice-out value of {} is greater than the maximum {}", + channel_id, splice_out_incl_fees + Amount::ONE_SAT, splice_out_incl_fees, ); initiator.logger.assert_log("lightning::ln::channel", cannot_be_funded, 1); @@ -6857,3 +6858,353 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { other => panic!("Expected SpliceFailed, got {:?}", other), } } + +#[test] +fn test_0reserve_splice() { + let mut config = test_default_channel_config(); + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + let a = do_test_0reserve_splice_holder_validation(false, false, false, config.clone()); + let _b = do_test_0reserve_splice_holder_validation(true, false, false, config.clone()); + let _c = do_test_0reserve_splice_holder_validation(false, true, false, config.clone()); + let _d = do_test_0reserve_splice_holder_validation(true, true, false, config.clone()); + + let _e = do_test_0reserve_splice_holder_validation(false, false, true, config.clone()); + let _f = do_test_0reserve_splice_holder_validation(true, false, true, config.clone()); + let _g = do_test_0reserve_splice_holder_validation(false, true, true, config.clone()); + let _h = do_test_0reserve_splice_holder_validation(true, true, true, config.clone()); + + assert_eq!(a, ChannelTypeFeatures::only_static_remote_key()); + + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + let a = do_test_0reserve_splice_holder_validation(false, false, false, config.clone()); + let _b = do_test_0reserve_splice_holder_validation(true, false, false, config.clone()); + let _c = do_test_0reserve_splice_holder_validation(false, true, false, config.clone()); + let _d = do_test_0reserve_splice_holder_validation(true, true, false, config.clone()); + + let _e = do_test_0reserve_splice_holder_validation(false, false, true, config.clone()); + let _f = do_test_0reserve_splice_holder_validation(true, false, true, config.clone()); + let _g = do_test_0reserve_splice_holder_validation(false, true, true, config.clone()); + let _h = do_test_0reserve_splice_holder_validation(true, true, true, config.clone()); + + assert_eq!(a, ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()); + + let mut config = test_default_channel_config(); + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + let a = do_test_0reserve_splice_counterparty_validation(false, false, false, config.clone()); + let _b = do_test_0reserve_splice_counterparty_validation(true, false, false, config.clone()); + let _c = do_test_0reserve_splice_counterparty_validation(false, true, false, config.clone()); + let _d = do_test_0reserve_splice_counterparty_validation(true, true, false, config.clone()); + + let _e = do_test_0reserve_splice_counterparty_validation(false, false, true, config.clone()); + let _f = do_test_0reserve_splice_counterparty_validation(true, false, true, config.clone()); + let _g = do_test_0reserve_splice_counterparty_validation(false, true, true, config.clone()); + let _h = do_test_0reserve_splice_counterparty_validation(true, true, true, config.clone()); + assert_eq!(a, ChannelTypeFeatures::only_static_remote_key()); + + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + let a = do_test_0reserve_splice_counterparty_validation(false, false, false, config.clone()); + let _b = do_test_0reserve_splice_counterparty_validation(true, false, false, config.clone()); + let _c = do_test_0reserve_splice_counterparty_validation(false, true, false, config.clone()); + let _d = do_test_0reserve_splice_counterparty_validation(true, true, false, config.clone()); + + let _e = do_test_0reserve_splice_counterparty_validation(false, false, true, config.clone()); + let _f = do_test_0reserve_splice_counterparty_validation(true, false, true, config.clone()); + let _g = do_test_0reserve_splice_counterparty_validation(false, true, true, config.clone()); + let _h = do_test_0reserve_splice_counterparty_validation(true, true, true, config.clone()); + assert_eq!(a, ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()); + + // TODO: Skip 0FC channels for now as these always have an output on the commitment, the P2A + // output. We will be able to withdraw up to the dust limit of the funding script, which + // is checked in interactivetx. Still need to double check whether that's what we actually + // want. +} + +#[cfg(test)] +fn do_test_0reserve_splice_holder_validation( + splice_passes: bool, counterparty_has_output: bool, node_0_is_initiator: bool, + mut config: UserConfig, +) -> ChannelTypeFeatures { + use crate::ln::htlc_reserve_unit_tests::setup_0reserve_no_outputs_channels; + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + config.channel_handshake_config.announced_channel_max_inbound_htlc_value_in_flight_percentage = + 100; + let node_chanmgrs = + create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config.clone())]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let _node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_1 = nodes[1].node.get_our_node_id(); + + let channel_value_sat = 100_000; + // Some dust limit, does not matter + let dust_limit_satoshis = 546; + + let (channel_id, _tx) = + setup_0reserve_no_outputs_channels(&nodes, channel_value_sat, dust_limit_satoshis); + let details = &nodes[0].node.list_channels()[0]; + let channel_type = details.channel_type.clone().unwrap(); + + let feerate = 253; + let spiked_feerate = if channel_type == ChannelTypeFeatures::only_static_remote_key() { + feerate * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 + } else if channel_type == ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() { + feerate + } else { + panic!("Unexpected channel type"); + }; + let anchors_sat = + if channel_type == ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() { + ANCHOR_OUTPUT_VALUE_SATOSHI * 2 + } else { + 0 + }; + + let initiator_value_to_self_sat = if counterparty_has_output { + send_payment(&nodes[0], &[&nodes[1]], channel_value_sat / 2 * 1_000); + channel_value_sat / 2 + } else if !node_0_is_initiator { + let tx_fee_msat = chan_utils::commit_tx_fee_sat(spiked_feerate, 2, &channel_type) * 1000; + let node_0_details = &nodes[0].node.list_channels()[0]; + let outbound_capacity_msat = node_0_details.outbound_capacity_msat; + let available_capacity_msat = node_0_details.next_outbound_htlc_limit_msat; + assert_eq!(outbound_capacity_msat, (channel_value_sat - anchors_sat) * 1000); + assert_eq!(available_capacity_msat, outbound_capacity_msat - tx_fee_msat); + send_payment(&nodes[0], &[&nodes[1]], available_capacity_msat); + + // Make sure node 0 has no output on the commitment at this point + let node_0_to_local_output_msat = channel_value_sat * 1000 + - available_capacity_msat + - anchors_sat * 1000 + - chan_utils::commit_tx_fee_sat(feerate, 0, &channel_type) * 1000; + assert!(node_0_to_local_output_msat / 1000 < dust_limit_satoshis); + let commit_tx = &get_local_commitment_txn!(nodes[0], channel_id)[0]; + assert_eq!(commit_tx.output.len(), if anchors_sat == 0 { 1 } else { 2 }); + assert_eq!( + commit_tx.output.last().unwrap().value, + Amount::from_sat(available_capacity_msat / 1000) + ); + if anchors_sat != 0 { + assert_eq!(commit_tx.output[0].value, Amount::from_sat(330)); + } + + available_capacity_msat / 1000 + } else { + channel_value_sat + }; + + // The estimated fees to splice out a single output at 253sat/kw + let estimated_fees = 183; + let splice_out_max_value = if counterparty_has_output && node_0_is_initiator { + let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(spiked_feerate, 1, &channel_type); + Amount::from_sat( + initiator_value_to_self_sat - commit_tx_fee_sat - anchors_sat - estimated_fees, + ) + } else if !counterparty_has_output && node_0_is_initiator { + let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(spiked_feerate, 0, &channel_type); + Amount::from_sat( + initiator_value_to_self_sat + - commit_tx_fee_sat + - anchors_sat - estimated_fees + - dust_limit_satoshis, + ) + } else if counterparty_has_output && !node_0_is_initiator { + Amount::from_sat(initiator_value_to_self_sat - estimated_fees) + } else if !counterparty_has_output && !node_0_is_initiator { + Amount::from_sat(initiator_value_to_self_sat - estimated_fees - dust_limit_satoshis) + } else { + panic!("unexpected case!"); + }; + let outputs = vec![TxOut { + value: splice_out_max_value + if splice_passes { Amount::ZERO } else { Amount::ONE_SAT }, + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }]; + + let (initiator, acceptor) = + if node_0_is_initiator { (&nodes[0], &nodes[1]) } else { (&nodes[1], &nodes[0]) }; + + let initiator_details = &initiator.node.list_channels()[0]; + assert_eq!( + initiator_details.next_splice_out_maximum_sat, + splice_out_max_value.to_sat() + estimated_fees + ); + + if splice_passes { + let contribution = initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap(); + + let (splice_tx, _) = splice_channel(initiator, acceptor, channel_id, contribution); + mine_transaction(initiator, &splice_tx); + mine_transaction(acceptor, &splice_tx); + lock_splice_after_blocks(initiator, acceptor, ANTI_REORG_DELAY - 1); + } else { + assert!(initiate_splice_out(initiator, acceptor, channel_id, outputs).is_err()); + let splice_out_value = + splice_out_max_value + Amount::from_sat(estimated_fees) + Amount::ONE_SAT; + let splice_out_max_value = splice_out_max_value + Amount::from_sat(estimated_fees); + let cannot_be_funded = format!( + "Channel {channel_id} cannot be funded: Our \ + splice-out value of {splice_out_value} is greater than the maximum \ + {splice_out_max_value}" + ); + initiator.logger.assert_log("lightning::ln::channel", cannot_be_funded, 1); + } + + channel_type +} + +#[cfg(test)] +fn do_test_0reserve_splice_counterparty_validation( + splice_passes: bool, counterparty_has_output: bool, node_0_is_initiator: bool, + mut config: UserConfig, +) -> ChannelTypeFeatures { + use crate::ln::htlc_reserve_unit_tests::setup_0reserve_no_outputs_channels; + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + config.channel_handshake_config.announced_channel_max_inbound_htlc_value_in_flight_percentage = + 100; + let node_chanmgrs = + create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config.clone())]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let _node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_1 = nodes[1].node.get_our_node_id(); + + let channel_value_sat = 100_000; + // Some dust limit, does not matter + let dust_limit_satoshis = 546; + + let (channel_id, _tx) = + setup_0reserve_no_outputs_channels(&nodes, channel_value_sat, dust_limit_satoshis); + let details = &nodes[0].node.list_channels()[0]; + let channel_type = details.channel_type.clone().unwrap(); + + let feerate = 253; + let spiked_feerate = if channel_type == ChannelTypeFeatures::only_static_remote_key() { + feerate * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 + } else if channel_type == ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() { + feerate + } else { + panic!("Unexpected channel type"); + }; + let anchors_sat = + if channel_type == ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() { + ANCHOR_OUTPUT_VALUE_SATOSHI * 2 + } else { + 0 + }; + + let initiator_value_to_self_sat = if counterparty_has_output { + send_payment(&nodes[0], &[&nodes[1]], channel_value_sat / 2 * 1_000); + channel_value_sat / 2 + } else if !node_0_is_initiator { + let tx_fee_msat = chan_utils::commit_tx_fee_sat(spiked_feerate, 2, &channel_type) * 1000; + let node_0_details = &nodes[0].node.list_channels()[0]; + let outbound_capacity_msat = node_0_details.outbound_capacity_msat; + let available_capacity_msat = node_0_details.next_outbound_htlc_limit_msat; + assert_eq!(outbound_capacity_msat, (channel_value_sat - anchors_sat) * 1000); + assert_eq!(available_capacity_msat, outbound_capacity_msat - tx_fee_msat); + send_payment(&nodes[0], &[&nodes[1]], available_capacity_msat); + + // Make sure node 0 has no output on the commitment at this point + let node_0_to_local_output_msat = channel_value_sat * 1000 + - available_capacity_msat + - anchors_sat * 1000 + - chan_utils::commit_tx_fee_sat(spiked_feerate, 0, &channel_type) * 1000; + assert!(node_0_to_local_output_msat / 1000 < dust_limit_satoshis); + let commit_tx = &get_local_commitment_txn!(nodes[0], channel_id)[0]; + assert_eq!(commit_tx.output.len(), if anchors_sat == 0 { 1 } else { 2 }); + assert_eq!( + commit_tx.output.last().unwrap().value, + Amount::from_sat(available_capacity_msat / 1000) + ); + if anchors_sat != 0 { + assert_eq!(commit_tx.output[0].value, Amount::from_sat(330)); + } + + available_capacity_msat / 1000 + } else { + channel_value_sat + }; + + let splice_out_value_incl_fees = if counterparty_has_output && node_0_is_initiator { + let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(spiked_feerate, 1, &channel_type); + Amount::from_sat(initiator_value_to_self_sat - commit_tx_fee_sat - anchors_sat) + } else if !counterparty_has_output && node_0_is_initiator { + let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(spiked_feerate, 0, &channel_type); + Amount::from_sat( + initiator_value_to_self_sat - commit_tx_fee_sat - anchors_sat - dust_limit_satoshis, + ) + } else if counterparty_has_output && !node_0_is_initiator { + Amount::from_sat(initiator_value_to_self_sat) + } else if !counterparty_has_output && !node_0_is_initiator { + Amount::from_sat(initiator_value_to_self_sat - dust_limit_satoshis) + } else { + panic!("unexpected case!"); + }; + + let (initiator, acceptor) = + if node_0_is_initiator { (&nodes[0], &nodes[1]) } else { (&nodes[1], &nodes[0]) }; + + let initiator_details = &initiator.node.list_channels()[0]; + assert_eq!(initiator_details.next_splice_out_maximum_sat, splice_out_value_incl_fees.to_sat()); + + let funding_contribution_sat = + -(splice_out_value_incl_fees.to_sat() as i64) - if splice_passes { 0 } else { 1 }; + let outputs = vec![TxOut { + // Splice out some dummy amount to get past the initiator's validation, + // we'll modify the message in-flight. + value: Amount::from_sat(1_000), + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }]; + let _contribution = initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap(); + + let node_id_initiator = initiator.node.get_our_node_id(); + let node_id_acceptor = acceptor.node.get_our_node_id(); + + let stfu_init = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor); + acceptor.node.handle_stfu(node_id_initiator, &stfu_init); + let stfu_ack = get_event_msg!(acceptor, MessageSendEvent::SendStfu, node_id_initiator); + initiator.node.handle_stfu(node_id_acceptor, &stfu_ack); + + let mut splice_init = + get_event_msg!(initiator, MessageSendEvent::SendSpliceInit, node_id_acceptor); + // Make the modification here + splice_init.funding_contribution_satoshis = funding_contribution_sat; + + if splice_passes { + acceptor.node.handle_splice_init(node_id_initiator, &splice_init); + let _splice_ack = + get_event_msg!(acceptor, MessageSendEvent::SendSpliceAck, node_id_initiator); + } else { + acceptor.node.handle_splice_init(node_id_initiator, &splice_init); + let msg_events = acceptor.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1); + if let MessageSendEvent::HandleError { action, .. } = &msg_events[0] { + assert!(matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. })); + } else { + panic!("Expected MessageSendEvent::HandleError"); + } + let cannot_splice_out = if u64::try_from(funding_contribution_sat.abs()).unwrap() + > initiator_value_to_self_sat + { + format!( + "Got non-closing error: Their contribution candidate {funding_contribution_sat}sat \ + is greater than their total balance in the channel {initiator_value_to_self_sat}sat" + ) + } else { + format!( + "Got non-closing error: Channel {channel_id} cannot \ + be spliced; Balance exhausted on local commitment" + ) + }; + acceptor.logger.assert_log("lightning::ln::channelmanager", cannot_splice_out, 1); + } + + channel_type +} diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index edb048c8c7d..fce3996efa1 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -4150,6 +4150,7 @@ mod tests { outbound_capacity_msat, next_outbound_htlc_limit_msat: outbound_capacity_msat, next_outbound_htlc_minimum_msat: 0, + next_splice_out_maximum_sat: outbound_capacity_msat / 1000, inbound_capacity_msat: 42, unspendable_punishment_reserve: None, confirmations_required: None, @@ -9649,6 +9650,7 @@ pub(crate) mod bench_utils { outbound_capacity_msat: 10_000_000_000, next_outbound_htlc_minimum_msat: 0, next_outbound_htlc_limit_msat: 10_000_000_000, + next_splice_out_maximum_sat: 10_000_000, inbound_capacity_msat: 0, unspendable_punishment_reserve: None, confirmations_required: None, diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs index f51759db5e9..d1ea012d912 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -9,7 +9,10 @@ use crate::ln::chan_utils::{ second_stage_tx_fees_sat, ChannelTransactionParameters, CommitmentTransaction, HTLCOutputInCommitment, }; -use crate::ln::channel::{CommitmentStats, ANCHOR_OUTPUT_VALUE_SATOSHI}; +use crate::ln::channel::{ + get_v2_channel_reserve_satoshis, CommitmentStats, ANCHOR_OUTPUT_VALUE_SATOSHI, + FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, +}; use crate::prelude::*; use crate::types::features::ChannelTypeFeatures; use crate::util::logger::Logger; @@ -216,7 +219,7 @@ fn has_output( fn get_next_commitment_stats( 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, + addl_nondust_htlc_count: usize, feerate_per_kw: u32, include_fee_spike_multiple: bool, dust_exposure_limiting_feerate: Option, broadcaster_dust_limit_satoshis: u64, channel_type: &ChannelTypeFeatures, ) -> Result { @@ -267,11 +270,17 @@ fn get_next_commitment_stats( channel_type, ); - // Calculate fees on commitment transaction - let nondust_htlc_count = next_commitment_htlcs + let spiked_feerate = + if include_fee_spike_multiple && !channel_type.supports_anchors_zero_fee_htlc_tx() { + feerate_per_kw * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 + } else { + feerate_per_kw + }; + + let spiked_nondust_htlc_count = next_commitment_htlcs .iter() .filter(|htlc| { - !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_satoshis, channel_type) + !htlc.is_dust(local, spiked_feerate, broadcaster_dust_limit_satoshis, channel_type) }) .count(); @@ -281,8 +290,8 @@ fn get_next_commitment_stats( is_outbound_from_holder, holder_balance_before_fee_msat, counterparty_balance_before_fee_msat, - feerate_per_kw, - nondust_htlc_count, + spiked_feerate, + spiked_nondust_htlc_count, broadcaster_dust_limit_satoshis, channel_type, ) { @@ -292,8 +301,17 @@ fn get_next_commitment_stats( // 2) Now including any additional non-dust HTLCs (usually the fee spike buffer HTLC), does the funder cover // this bigger transaction fee ? The funder can dip below their dust limit to cover this case, as the // commitment will have at least one output: the non-dust fee spike buffer HTLC offered by the counterparty. + let nondust_htlc_count = next_commitment_htlcs + .iter() + .filter(|htlc| { + !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_satoshis, channel_type) + }) + .count(); + // Note here we use the htlc count at the current feerate together with the spiked feerate; + // this makes sure that the holder can afford any fee bump between 1x to 2x from the current + // feerate if the fee spike multiple is included. let commit_tx_fee_sat = commit_tx_fee_sat( - feerate_per_kw, + spiked_feerate, nondust_htlc_count + addl_nondust_htlc_count, channel_type, ); @@ -315,6 +333,108 @@ fn get_next_commitment_stats( }) } +/// Determines the maximum value that the holder can splice out of the channel, accounting +/// for the updated reserves after said splice. This maximum also makes sure the local commitment +/// retains at least one output after the splice, which is particularly relevant for +/// zero-reserve channels. +// +// The equation to determine `max_splice_percentage_constraint_sat` is: +// 1) floor((c - s) / 100) == h - s - d +// We want the maximum value of s that will satisfy equation 1, therefore, we solve: +// 2) (c - s) / 100 < h - s - d + 1 +// where c: `channel_value_satoshis` +// s: `max_splice_percentage_constraint_sat` +// h: `local_balance_before_fee_sat` +// d: `post_splice_delta_above_reserve_sat` +// This results in: +// 3) s < (100h + 100 - 100d - c) / 99 +fn get_next_splice_out_maximum_sat( + is_outbound_from_holder: bool, channel_value_satoshis: u64, local_balance_before_fee_msat: u64, + remote_balance_before_fee_msat: u64, spiked_feerate: u32, + spiked_feerate_nondust_htlc_count: usize, post_splice_delta_above_reserve_sat: u64, + channel_constraints: &ChannelConstraints, channel_type: &ChannelTypeFeatures, +) -> u64 { + let local_balance_before_fee_sat = local_balance_before_fee_msat / 1000; + let mut next_splice_out_maximum_sat = if channel_constraints + .counterparty_selected_channel_reserve_satoshis + != 0 + { + let dividend_sat = local_balance_before_fee_sat + .saturating_mul(100) + .saturating_add(100) + .saturating_sub(post_splice_delta_above_reserve_sat.saturating_mul(100)) + .saturating_sub(channel_value_satoshis); + // Calculate the greatest integer that is strictly less than the RHS of inequality 3 above + let max_splice_percentage_constraint_sat = dividend_sat.saturating_sub(1) / 99; + let max_splice_dust_limit_constraint_sat = local_balance_before_fee_sat + .saturating_sub(channel_constraints.holder_dust_limit_satoshis) + .saturating_sub(post_splice_delta_above_reserve_sat); + // Both constraints must be satisfied, so take the minimum of the two maximums + let max_splice_out_sat = + cmp::min(max_splice_percentage_constraint_sat, max_splice_dust_limit_constraint_sat); + #[cfg(debug_assertions)] + if max_splice_out_sat == 0 { + let current_balance_sat = + local_balance_before_fee_sat.saturating_sub(post_splice_delta_above_reserve_sat); + let v2_reserve_sat = get_v2_channel_reserve_satoshis( + channel_value_satoshis, + channel_constraints.holder_dust_limit_satoshis, + false, + ); + // If the holder cannot splice out anything, they must be at or + // below the v2 reserve + debug_assert!(current_balance_sat <= v2_reserve_sat); + } else { + let post_splice_reserve_sat = get_v2_channel_reserve_satoshis( + channel_value_satoshis.saturating_sub(max_splice_out_sat), + channel_constraints.holder_dust_limit_satoshis, + false, + ); + // If the holder can splice out some maximum, splicing out that + // maximum lands them at exactly the new v2 reserve + the + // `post_splice_delta_above_reserve_sat` + debug_assert_eq!( + local_balance_before_fee_sat.saturating_sub(max_splice_out_sat), + post_splice_reserve_sat.saturating_add(post_splice_delta_above_reserve_sat) + ); + } + max_splice_out_sat + } else { + // In a zero-reserve channel, the holder is free to withdraw up to its `post_splice_delta_above_reserve_sat` + local_balance_before_fee_sat.saturating_sub(post_splice_delta_above_reserve_sat) + }; + + // We only bother to check the local commitment here, the counterparty will check its own commitment. + // + // If the current `next_splice_out_maximum_sat` would produce a local commitment with no + // outputs, bump this maximum such that, after the splice, the holder's balance covers at + // least `dust_limit_satoshis` and, if they are the funder, `current_spiked_tx_fee_sat`. + // We don't include an additional non-dust inbound HTLC in the `current_spiked_tx_fee_sat`, + // because we don't mind if the holder dips below their dust limit to cover the fee for that + // inbound non-dust HTLC. + if !has_output( + is_outbound_from_holder, + local_balance_before_fee_msat.saturating_sub(next_splice_out_maximum_sat * 1000), + remote_balance_before_fee_msat, + spiked_feerate, + spiked_feerate_nondust_htlc_count, + channel_constraints.holder_dust_limit_satoshis, + channel_type, + ) { + let dust_limit_satoshis = channel_constraints.holder_dust_limit_satoshis; + let current_spiked_tx_fee_sat = commit_tx_fee_sat(spiked_feerate, 0, channel_type); + let min_balance_sat = if is_outbound_from_holder { + dust_limit_satoshis.saturating_add(current_spiked_tx_fee_sat) + } else { + dust_limit_satoshis + }; + next_splice_out_maximum_sat = + (local_balance_before_fee_msat / 1000).saturating_sub(min_balance_sat); + } + + next_splice_out_maximum_sat +} + fn get_available_balances( is_outbound_from_holder: bool, channel_value_satoshis: u64, value_to_holder_msat: u64, pending_htlcs: &[HTLCAmountDirection], feerate_per_kw: u32, @@ -336,15 +456,25 @@ fn get_available_balances( if channel_type.supports_anchor_zero_fee_commitments() { 0 } else { 1 }; // Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop - let spiked_feerate = feerate_per_kw.saturating_mul( - if is_outbound_from_holder && !channel_type.supports_anchors_zero_fee_htlc_tx() { + let spiked_feerate = + feerate_per_kw.saturating_mul(if !channel_type.supports_anchors_zero_fee_htlc_tx() { crate::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 } else { 1 - }, - ); + }); let local_nondust_htlc_count = pending_htlcs + .iter() + .filter(|htlc| { + !htlc.is_dust( + true, + feerate_per_kw, + channel_constraints.holder_dust_limit_satoshis, + channel_type, + ) + }) + .count(); + let local_spiked_nondust_htlc_count = pending_htlcs .iter() .filter(|htlc| { !htlc.is_dust( @@ -355,6 +485,10 @@ fn get_available_balances( ) }) .count(); + + // Note here we use the htlc count at the current feerate together with the spiked feerate; + // this makes sure that the holder can afford any fee bump between 1x to 2x from the current + // feerate. let local_max_commit_tx_fee_sat = commit_tx_fee_sat( spiked_feerate, local_nondust_htlc_count + fee_spike_buffer_htlc + 1, @@ -411,6 +545,20 @@ fn get_available_balances( total_anchors_sat.saturating_mul(1000), ); + let next_splice_out_maximum_sat = get_next_splice_out_maximum_sat( + is_outbound_from_holder, + channel_value_satoshis, + local_balance_before_fee_msat, + remote_balance_before_fee_msat, + spiked_feerate, + // The number of non-dust HTLCs on the local commitment at the spiked feerate + local_spiked_nondust_htlc_count, + // The post-splice minimum balance of the holder + if is_outbound_from_holder { local_min_commit_tx_fee_sat } else { 0 }, + &channel_constraints, + channel_type, + ); + let outbound_capacity_msat = local_balance_before_fee_msat .saturating_sub(channel_constraints.counterparty_selected_channel_reserve_satoshis * 1000); @@ -537,7 +685,7 @@ fn get_available_balances( // Now adjust our min and max size HTLC to make sure both the local and the remote commitments still have // at least one output at the spiked feerate. - let remote_nondust_htlc_count = pending_htlcs + let remote_spiked_nondust_htlc_count = pending_htlcs .iter() .filter(|htlc| { !htlc.is_dust( @@ -555,8 +703,8 @@ fn get_available_balances( is_outbound_from_holder, local_balance_before_fee_msat, remote_balance_before_fee_msat, - local_nondust_htlc_count, spiked_feerate, + local_spiked_nondust_htlc_count, channel_constraints.holder_dust_limit_satoshis, channel_type, next_outbound_htlc_minimum_msat, @@ -569,8 +717,8 @@ fn get_available_balances( is_outbound_from_holder, local_balance_before_fee_msat, remote_balance_before_fee_msat, - remote_nondust_htlc_count, spiked_feerate, + remote_spiked_nondust_htlc_count, channel_constraints.counterparty_dust_limit_satoshis, channel_type, next_outbound_htlc_minimum_msat, @@ -583,14 +731,16 @@ fn get_available_balances( outbound_capacity_msat, next_outbound_htlc_limit_msat: available_capacity_msat, next_outbound_htlc_minimum_msat, + next_splice_out_maximum_sat, } } fn adjust_boundaries_if_max_dust_htlc_produces_no_output( local: bool, is_outbound_from_holder: bool, holder_balance_before_fee_msat: u64, - counterparty_balance_before_fee_msat: u64, nondust_htlc_count: usize, spiked_feerate: u32, - dust_limit_satoshis: u64, channel_type: &ChannelTypeFeatures, - next_outbound_htlc_minimum_msat: u64, available_capacity_msat: u64, + counterparty_balance_before_fee_msat: u64, spiked_feerate: u32, + spiked_feerate_nondust_htlc_count: usize, dust_limit_satoshis: u64, + channel_type: &ChannelTypeFeatures, next_outbound_htlc_minimum_msat: u64, + available_capacity_msat: u64, ) -> (u64, u64) { // First, determine the biggest dust HTLC we could send let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = @@ -606,7 +756,7 @@ fn adjust_boundaries_if_max_dust_htlc_produces_no_output( holder_balance_before_fee_msat.saturating_sub(max_dust_htlc_msat), counterparty_balance_before_fee_msat, spiked_feerate, - nondust_htlc_count, + spiked_feerate_nondust_htlc_count, dust_limit_satoshis, channel_type, ) { @@ -659,7 +809,7 @@ pub(crate) trait TxBuilder { fn get_channel_stats( &self, local: bool, is_outbound_from_holder: bool, channel_value_satoshis: u64, value_to_holder_msat: u64, pending_htlcs: &[HTLCAmountDirection], - addl_nondust_htlc_count: usize, feerate_per_kw: u32, + addl_nondust_htlc_count: usize, feerate_per_kw: u32, include_fee_spike_multiple: bool, dust_exposure_limiting_feerate: Option, max_dust_htlc_exposure_msat: u64, channel_constraints: ChannelConstraints, channel_type: &ChannelTypeFeatures, ) -> Result; @@ -677,7 +827,7 @@ impl TxBuilder for SpecTxBuilder { fn get_channel_stats( &self, local: bool, is_outbound_from_holder: bool, channel_value_satoshis: u64, value_to_holder_msat: u64, pending_htlcs: &[HTLCAmountDirection], - addl_nondust_htlc_count: usize, feerate_per_kw: u32, + addl_nondust_htlc_count: usize, feerate_per_kw: u32, include_fee_spike_multiple: bool, dust_exposure_limiting_feerate: Option, max_dust_htlc_exposure_msat: u64, channel_constraints: ChannelConstraints, channel_type: &ChannelTypeFeatures, ) -> Result { @@ -690,6 +840,7 @@ impl TxBuilder for SpecTxBuilder { pending_htlcs, addl_nondust_htlc_count, feerate_per_kw, + include_fee_spike_multiple, dust_exposure_limiting_feerate, channel_constraints.holder_dust_limit_satoshis, channel_type, @@ -703,6 +854,7 @@ impl TxBuilder for SpecTxBuilder { pending_htlcs, addl_nondust_htlc_count, feerate_per_kw, + include_fee_spike_multiple, dust_exposure_limiting_feerate, channel_constraints.counterparty_dust_limit_satoshis, channel_type,