From ba195ce8d2ba7837987c0eaf607797dbabbad00d Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Mon, 10 Mar 2025 17:52:05 +0100 Subject: [PATCH 1/4] Add check for channel reserve --- lightning/src/ln/channel.rs | 183 ++++++++++++++++++++++++++++++------ 1 file changed, 155 insertions(+), 28 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 0796369d4c6..64837aa1568 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2477,6 +2477,28 @@ impl<'a> From<&'a Transaction> for ConfirmedTransaction<'a> { } } +#[cfg(splicing)] +impl PendingSplice { + #[inline] + fn add_checked(base: u64, delta: i64) -> u64 { + if delta >= 0 { + base.saturating_add(delta as u64) + } else { + base.saturating_sub(delta.abs() as u64) + } + } + + // /// Compute the post-splice channel value from the pre-splice values and the peer contributions + // pub fn compute_post_splice_value( + // pre_channel_value: u64, our_funding_contribution: i64, their_funding_contribution: i64, + // ) -> u64 { + // Self::add_checked( + // pre_channel_value, + // our_funding_contribution.saturating_add(their_funding_contribution), + // ) + // } +} + /// Contains everything about the channel including state, and various flags. pub(super) struct ChannelContext where @@ -10839,35 +10861,37 @@ where ES::Target: EntropySource, L::Target: Logger, { - let pending_splice = if let Some(ref mut pending_splice) = &mut self.pending_splice { - pending_splice - } else { - return Err(ChannelError::Ignore(format!("Channel is not in pending splice"))); - }; + let funding_negotiation_context = { + let pending_splice = if let Some(ref mut pending_splice) = &mut self.pending_splice { + pending_splice + } else { + return Err(ChannelError::Ignore(format!("Channel is not in pending splice"))); + }; - // TODO(splicing): Add check that we are the splice (quiescence) initiator + // TODO(splicing): Add check that we are the splice (quiescence) initiator - let funding_negotiation_context = match pending_splice.funding_negotiation.take() { - Some(FundingNegotiation::AwaitingAck(context)) => context, - Some(FundingNegotiation::ConstructingTransaction(funding, constructor)) => { - pending_splice.funding_negotiation = - Some(FundingNegotiation::ConstructingTransaction(funding, constructor)); - return Err(ChannelError::WarnAndDisconnect(format!( - "Got unexpected splice_ack; splice negotiation already in progress" - ))); - }, - Some(FundingNegotiation::AwaitingSignatures(funding)) => { - pending_splice.funding_negotiation = - Some(FundingNegotiation::AwaitingSignatures(funding)); - return Err(ChannelError::WarnAndDisconnect(format!( - "Got unexpected splice_ack; splice negotiation already in progress" - ))); - }, - None => { - return Err(ChannelError::Ignore(format!( - "Got unexpected splice_ack; no splice negotiation in progress" - ))); - }, + match pending_splice.funding_negotiation.take() { + Some(FundingNegotiation::AwaitingAck(context)) => context, + Some(FundingNegotiation::ConstructingTransaction(funding, constructor)) => { + pending_splice.funding_negotiation = + Some(FundingNegotiation::ConstructingTransaction(funding, constructor)); + return Err(ChannelError::WarnAndDisconnect(format!( + "Got unexpected splice_ack; splice negotiation already in progress" + ))); + }, + Some(FundingNegotiation::AwaitingSignatures(funding)) => { + pending_splice.funding_negotiation = + Some(FundingNegotiation::AwaitingSignatures(funding)); + return Err(ChannelError::WarnAndDisconnect(format!( + "Got unexpected splice_ack; splice negotiation already in progress" + ))); + }, + None => { + return Err(ChannelError::Ignore(format!( + "Got unexpected splice_ack; no splice negotiation in progress" + ))); + }, + } }; let our_funding_contribution_satoshis = @@ -10882,8 +10906,28 @@ where msg.funding_pubkey, )?; - // TODO(splicing): Pre-check for reserve requirement + // Pre-check for reserve requirement // (Note: It should also be checked later at tx_complete) + let pre_channel_value = self.funding.get_value_satoshis(); + let post_channel_value = self.funding.compute_post_splice_value( + our_funding_contribution_satoshis, + their_funding_contribution_satoshis, + ); + let pre_balance_self = self.funding.value_to_self_msat; + let post_balance_self = + PendingSplice::add_checked(pre_balance_self, our_funding_contribution_satoshis); + let pre_balance_counterparty = pre_channel_value.saturating_sub(pre_balance_self); + let post_balance_counterparty = post_channel_value.saturating_sub(post_balance_self); + // Pre-check for reserve requirement + // This will also be checked later at tx_complete + let _res = self.check_splice_balances_meet_v2_reserve_requirements( + pre_balance_self, + post_balance_self, + pre_balance_counterparty, + post_balance_counterparty, + pre_channel_value, + post_channel_value, + )?; log_info!( logger, @@ -10910,6 +10954,11 @@ where let tx_msg_opt = interactive_tx_constructor.take_initiator_first_message(); debug_assert!(self.interactive_tx_signing_session.is_none()); + let pending_splice = if let Some(ref mut pending_splice) = &mut self.pending_splice { + pending_splice + } else { + return Err(ChannelError::Ignore(format!("Channel is not in pending splice"))); + }; pending_splice.funding_negotiation = Some(FundingNegotiation::ConstructingTransaction( splice_funding, interactive_tx_constructor, @@ -10970,6 +11019,84 @@ where )) } + /// Check that post-splicing balance meets reserve requirements, but only if it met it pre-splice as well. + /// In case of error, it returns the minimum channel reserve that was violated (in sats) + #[cfg(splicing)] + pub fn check_splice_balance_meets_v2_reserve_requirement( + &self, pre_balance_msat: u64, post_balance_msat: u64, pre_channel_value_sats: u64, + post_channel_value_sats: u64, dust_limit_sats: u64, + ) -> Result<(), u64> { + let post_channel_reserve_sats = + get_v2_channel_reserve_satoshis(post_channel_value_sats, dust_limit_sats); + if post_balance_msat >= (post_channel_reserve_sats * 1000) { + return Ok(()); + } + // We're not allowed to dip below the reserve once we've been above, + // check differently for originally v1 and v2 channels + if self.is_v2_established() { + let pre_channel_reserve_sats = + get_v2_channel_reserve_satoshis(pre_channel_value_sats, dust_limit_sats); + if pre_balance_msat >= (pre_channel_reserve_sats * 1000) { + return Err(post_channel_reserve_sats); + } + } else { + if pre_balance_msat >= (self.funding.holder_selected_channel_reserve_satoshis * 1000) { + return Err(post_channel_reserve_sats); + } + if let Some(cp_reserve) = self.funding.counterparty_selected_channel_reserve_satoshis { + if pre_balance_msat >= (cp_reserve * 1000) { + return Err(post_channel_reserve_sats); + } + } + } + // Make sure we either remain with the same balance or move towards the reserve. + if post_balance_msat >= pre_balance_msat { + Ok(()) + } else { + Err(post_channel_reserve_sats) + } + } + + /// Check that balances (self and counterparty) meet the channel reserve requirements or violates them (below reserve). + /// The channel value is an input as opposed to using from the FundingScope, so that this can be used in case of splicing + /// to check with new channel value (before being committed to it). + #[cfg(splicing)] + pub fn check_splice_balances_meet_v2_reserve_requirements( + &self, self_balance_pre_msat: u64, self_balance_post_msat: u64, + counterparty_balance_pre_msat: u64, counterparty_balance_post_msat: u64, + channel_value_pre_sats: u64, channel_value_post_sats: u64, + ) -> Result<(), ChannelError> { + let is_ok_self = self.check_splice_balance_meets_v2_reserve_requirement( + self_balance_pre_msat, + self_balance_post_msat, + channel_value_pre_sats, + channel_value_post_sats, + self.context.holder_dust_limit_satoshis, + ); + if let Err(channel_reserve_self) = is_ok_self { + return Err(ChannelError::Warn(format!( + "Balance below reserve, mandated by holder, {} vs {}", + self_balance_post_msat, channel_reserve_self, + ))); + } + let is_ok_cp = self.check_splice_balance_meets_v2_reserve_requirement( + counterparty_balance_pre_msat, + counterparty_balance_post_msat, + channel_value_pre_sats, + channel_value_post_sats, + self.context.counterparty_dust_limit_satoshis, + ); + if let Err(channel_reserve_cp) = is_ok_cp { + return Err(ChannelError::Warn(format!( + "Balance below reserve mandated by counterparty, {} vs {}", + counterparty_balance_post_msat, channel_reserve_cp, + ))); + } + Ok(()) + } + + // Send stuff to our remote peers: + /// Queues up an outbound HTLC to send by placing it in the holding cell. You should call /// [`Self::maybe_free_holding_cell_htlcs`] in order to actually generate and send the /// commitment update. From a5fa952aad300eec6ea01f0a594e24b09ad2a52d Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Tue, 4 Mar 2025 20:36:46 +0100 Subject: [PATCH 2/4] Consider fees and anchor in balances when checking reserve requirements --- lightning/src/ln/channel.rs | 99 ++++++++++++++++++++++++++++++++----- 1 file changed, 86 insertions(+), 13 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 64837aa1568..3da281fb7d6 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10851,6 +10851,75 @@ where }) } + /// Compute the channel balances (local & remote, in msats) by taking into account fees, + /// anchor values, and dust limits. + /// Pending HTLCs are not taken into account, this method should be used when there is no such, + /// e.g. in quiescence state + #[cfg(splicing)] + fn compute_balances_less_fees( + &self, channel_value_sats: u64, value_to_self_msat: u64, is_local: bool, + ) -> (u64, u64) { + // We should get here only when there are no pending HTLCs, as they are not taken into account + debug_assert!(self.context.pending_inbound_htlcs.is_empty()); + debug_assert!(self.context.pending_outbound_htlcs.is_empty()); + + let feerate_per_kw = self.context.feerate_per_kw; + + // compute 'raw' counterparty balance + let value_to_remote_msat: i64 = + ((channel_value_sats * 1000) as i64).saturating_sub(value_to_self_msat as i64); + debug_assert!(value_to_remote_msat >= 0); + + let total_fee_sats = SpecTxBuilder {}.commit_tx_fee_sat( + feerate_per_kw, + 0, + &self.funding.channel_transaction_parameters.channel_type_features, + ); + let anchors_val_sats = if self + .funding + .channel_transaction_parameters + .channel_type_features + .supports_anchors_zero_fee_htlc_tx() + { + ANCHOR_OUTPUT_VALUE_SATOSHI * 2 + } else { + 0 + } as i64; + + // consider fees and anchor values + let (mut new_value_to_self_msat, mut new_value_to_remote_msat) = if self + .funding + .is_outbound() + { + ( + value_to_self_msat as i64 - anchors_val_sats * 1000 - total_fee_sats as i64 * 1000, + value_to_remote_msat, + ) + } else { + ( + value_to_self_msat as i64, + value_to_remote_msat - anchors_val_sats * 1000 - total_fee_sats as i64 * 1000, + ) + }; + + // consider dust limit + let broadcaster_dust_limit_sats = if is_local { + self.context.holder_dust_limit_satoshis + } else { + self.context.counterparty_dust_limit_satoshis + } as i64; + if new_value_to_self_msat < (broadcaster_dust_limit_sats * 1000) { + new_value_to_self_msat = 0; + } + debug_assert!(new_value_to_self_msat >= 0); + if new_value_to_remote_msat < (broadcaster_dust_limit_sats * 1000) { + new_value_to_remote_msat = 0; + } + debug_assert!(new_value_to_remote_msat >= 0); + + (new_value_to_self_msat as u64, new_value_to_remote_msat as u64) + } + /// Handle splice_ack #[cfg(splicing)] pub(crate) fn splice_ack( @@ -10908,25 +10977,29 @@ where // Pre-check for reserve requirement // (Note: It should also be checked later at tx_complete) - let pre_channel_value = self.funding.get_value_satoshis(); - let post_channel_value = self.funding.compute_post_splice_value( + let pre_channel_value_sats = self.funding.get_value_satoshis(); + let post_channel_value_sats = self.funding.compute_post_splice_value( our_funding_contribution_satoshis, their_funding_contribution_satoshis, ); - let pre_balance_self = self.funding.value_to_self_msat; - let post_balance_self = - PendingSplice::add_checked(pre_balance_self, our_funding_contribution_satoshis); - let pre_balance_counterparty = pre_channel_value.saturating_sub(pre_balance_self); - let post_balance_counterparty = post_channel_value.saturating_sub(post_balance_self); + let pre_balance_self_msat = self.funding.value_to_self_msat; + let post_balance_self_msat = PendingSplice::add_checked( + pre_balance_self_msat, + our_funding_contribution_satoshis * 1000, + ); + let (pre_balance_self_less_fees_msat, pre_balance_counterparty_less_fees_msat) = + self.compute_balances_less_fees(pre_channel_value_sats, pre_balance_self_msat, true); + let (post_balance_self_less_fees_msat, post_balance_counterparty_less_fees_msat) = + self.compute_balances_less_fees(post_channel_value_sats, post_balance_self_msat, true); // Pre-check for reserve requirement // This will also be checked later at tx_complete let _res = self.check_splice_balances_meet_v2_reserve_requirements( - pre_balance_self, - post_balance_self, - pre_balance_counterparty, - post_balance_counterparty, - pre_channel_value, - post_channel_value, + pre_balance_self_less_fees_msat, + post_balance_self_less_fees_msat, + pre_balance_counterparty_less_fees_msat, + post_balance_counterparty_less_fees_msat, + pre_channel_value_sats, + post_channel_value_sats, )?; log_info!( From 863ec36efdde2d247fed4de088b3de318efd92bc Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Thu, 26 Jun 2025 13:45:52 +0200 Subject: [PATCH 3/4] fix Change the order of the 2 check methods --- lightning/src/ln/channel.rs | 76 ++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 3da281fb7d6..b10d5f57c13 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -11092,44 +11092,6 @@ where )) } - /// Check that post-splicing balance meets reserve requirements, but only if it met it pre-splice as well. - /// In case of error, it returns the minimum channel reserve that was violated (in sats) - #[cfg(splicing)] - pub fn check_splice_balance_meets_v2_reserve_requirement( - &self, pre_balance_msat: u64, post_balance_msat: u64, pre_channel_value_sats: u64, - post_channel_value_sats: u64, dust_limit_sats: u64, - ) -> Result<(), u64> { - let post_channel_reserve_sats = - get_v2_channel_reserve_satoshis(post_channel_value_sats, dust_limit_sats); - if post_balance_msat >= (post_channel_reserve_sats * 1000) { - return Ok(()); - } - // We're not allowed to dip below the reserve once we've been above, - // check differently for originally v1 and v2 channels - if self.is_v2_established() { - let pre_channel_reserve_sats = - get_v2_channel_reserve_satoshis(pre_channel_value_sats, dust_limit_sats); - if pre_balance_msat >= (pre_channel_reserve_sats * 1000) { - return Err(post_channel_reserve_sats); - } - } else { - if pre_balance_msat >= (self.funding.holder_selected_channel_reserve_satoshis * 1000) { - return Err(post_channel_reserve_sats); - } - if let Some(cp_reserve) = self.funding.counterparty_selected_channel_reserve_satoshis { - if pre_balance_msat >= (cp_reserve * 1000) { - return Err(post_channel_reserve_sats); - } - } - } - // Make sure we either remain with the same balance or move towards the reserve. - if post_balance_msat >= pre_balance_msat { - Ok(()) - } else { - Err(post_channel_reserve_sats) - } - } - /// Check that balances (self and counterparty) meet the channel reserve requirements or violates them (below reserve). /// The channel value is an input as opposed to using from the FundingScope, so that this can be used in case of splicing /// to check with new channel value (before being committed to it). @@ -11168,6 +11130,44 @@ where Ok(()) } + /// Check that post-splicing balance meets reserve requirements, but only if it met it pre-splice as well. + /// In case of error, it returns the minimum channel reserve that was violated (in sats) + #[cfg(splicing)] + pub fn check_splice_balance_meets_v2_reserve_requirement( + &self, pre_balance_msat: u64, post_balance_msat: u64, pre_channel_value_sats: u64, + post_channel_value_sats: u64, dust_limit_sats: u64, + ) -> Result<(), u64> { + let post_channel_reserve_sats = + get_v2_channel_reserve_satoshis(post_channel_value_sats, dust_limit_sats); + if post_balance_msat >= (post_channel_reserve_sats * 1000) { + return Ok(()); + } + // We're not allowed to dip below the reserve once we've been above, + // check differently for originally v1 and v2 channels + if self.is_v2_established() { + let pre_channel_reserve_sats = + get_v2_channel_reserve_satoshis(pre_channel_value_sats, dust_limit_sats); + if pre_balance_msat >= (pre_channel_reserve_sats * 1000) { + return Err(post_channel_reserve_sats); + } + } else { + if pre_balance_msat >= (self.funding.holder_selected_channel_reserve_satoshis * 1000) { + return Err(post_channel_reserve_sats); + } + if let Some(cp_reserve) = self.funding.counterparty_selected_channel_reserve_satoshis { + if pre_balance_msat >= (cp_reserve * 1000) { + return Err(post_channel_reserve_sats); + } + } + } + // Make sure we either remain with the same balance or move towards the reserve. + if post_balance_msat >= pre_balance_msat { + Ok(()) + } else { + Err(post_channel_reserve_sats) + } + } + // Send stuff to our remote peers: /// Queues up an outbound HTLC to send by placing it in the holding cell. You should call From b2d874c3c45ca33ac166cf339d6193bf1b769353 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Tue, 1 Jul 2025 19:10:48 +0200 Subject: [PATCH 4/4] fix Use v2 reserve condition for spliced-from-v1 v2 channels as well --- lightning/src/ln/channel.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index b10d5f57c13..93eed1b40df 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2996,6 +2996,8 @@ where "commitment_signed" } + /// Checks whether the channel was opened through V2 channel open (negotiation). + /// See also: is_v2() fn is_v2_established(&self) -> bool { let channel_parameters = &self.funding().channel_transaction_parameters; // This will return false if `counterparty_parameters` is `None`, but for a `FundedChannel`, it @@ -11144,7 +11146,7 @@ where } // We're not allowed to dip below the reserve once we've been above, // check differently for originally v1 and v2 channels - if self.is_v2_established() { + if self.is_v2() { let pre_channel_reserve_sats = get_v2_channel_reserve_satoshis(pre_channel_value_sats, dust_limit_sats); if pre_balance_msat >= (pre_channel_reserve_sats * 1000) { @@ -11992,6 +11994,18 @@ where // CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY confirmations. self.context.historical_scids.drain(0..end) } + + /// Check is channel is currently v2: + /// - established as v2 + /// - or past a splice (which implicitly makes the channel v2) + #[cfg(splicing)] + fn is_v2(&self) -> bool { + if self.funding.channel_transaction_parameters.splice_parent_funding_txid.is_some() { + true + } else { + self.is_v2_established() + } + } } /// A not-yet-funded outbound (from holder) channel using V1 channel establishment.