diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 079201e748b..e03bc833c7e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2248,7 +2248,7 @@ impl FundingScope { fn for_splice( prev_funding: &Self, context: &ChannelContext, our_funding_contribution: SignedAmount, their_funding_contribution: SignedAmount, counterparty_funding_pubkey: PublicKey, - ) -> Result + ) -> Self where SP::Target: SignerProvider, { @@ -2298,7 +2298,7 @@ impl FundingScope { let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(post_channel_value, MIN_CHAN_DUST_LIMIT_SATOSHIS); - Ok(Self { + Self { channel_transaction_parameters: post_channel_transaction_parameters, value_to_self_msat: post_value_to_self_msat, funding_transaction: None, @@ -2322,7 +2322,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. @@ -11089,6 +11089,8 @@ where ))); } + // TODO(splicing): Once splice acceptor can contribute, check that inputs are sufficient, + // similarly to the check in `splice_channel`. debug_assert_eq!(our_funding_contribution, SignedAmount::ZERO); // TODO(splicing): Move this check once user-provided contributions are supported for @@ -11110,33 +11112,18 @@ where } let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis); - self.validate_splice_contribution(their_funding_contribution)?; - - // TODO(splicing): Check that channel balance does not go below the channel reserve - - let splice_funding = FundingScope::for_splice( - &self.funding, - &self.context, + self.validate_splice_contribution( our_funding_contribution, their_funding_contribution, msg.funding_pubkey, - )?; - - // TODO(splicing): Once splice acceptor can contribute, check that inputs are sufficient, - // similarly to the check in `splice_channel`. - - // Note on channel reserve requirement pre-check: as the splice acceptor does not contribute, - // it can't go below reserve, therefore no pre-check is done here. - - // TODO(splicing): Early check for reserve requirement - - Ok(splice_funding) + ) } #[cfg(splicing)] fn validate_splice_contribution( - &self, their_funding_contribution: SignedAmount, - ) -> Result<(), ChannelError> { + &self, our_funding_contribution: SignedAmount, their_funding_contribution: SignedAmount, + counterparty_funding_pubkey: PublicKey, + ) -> Result { if their_funding_contribution > SignedAmount::MAX_MONEY { return Err(ChannelError::WarnAndDisconnect(format!( "Channel {} cannot be spliced in; their {} contribution exceeds the total bitcoin supply", @@ -11160,6 +11147,11 @@ where their_funding_contribution.to_sat(), ); + // While we check that the remote can afford the HTLCs, anchors, and the reserve + // after creating the new `FundingScope` below, we MUST do a basic check here to + // make sure that their funding contribution doesn't completely exhaust their + // balance because an invariant of `FundingScope` is that `value_to_self_msat` + // MUST be smaller than or equal to `channel_value_satoshis * 1000`. if post_channel_balance.is_none() { return Err(ChannelError::WarnAndDisconnect(format!( "Channel {} cannot be spliced out; their {} contribution exhausts their channel balance: {}", @@ -11169,10 +11161,21 @@ where ))); } - Ok(()) + let splice_funding = FundingScope::for_splice( + &self.funding, + &self.context, + our_funding_contribution, + their_funding_contribution, + counterparty_funding_pubkey, + ); + + if their_funding_contribution != SignedAmount::ZERO { + self.validate_their_funding_contribution_reserve(&splice_funding)?; + } + + Ok(splice_funding) } - /// See also [`validate_splice_init`] #[cfg(splicing)] pub(crate) fn splice_init( &mut self, msg: &msgs::SpliceInit, our_funding_contribution_satoshis: i64, @@ -11240,7 +11243,6 @@ where }) } - /// Handle splice_ack #[cfg(splicing)] pub(crate) fn splice_ack( &mut self, msg: &msgs::SpliceAck, signer_provider: &SP, entropy_source: &ES, @@ -11250,53 +11252,7 @@ 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"))); - }; - - // 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" - ))); - }, - }; - - let our_funding_contribution = funding_negotiation_context.our_funding_contribution; - debug_assert!(our_funding_contribution.abs() <= SignedAmount::MAX_MONEY); - - let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis); - self.validate_splice_contribution(their_funding_contribution)?; - - let splice_funding = FundingScope::for_splice( - &self.funding, - &self.context, - our_funding_contribution, - their_funding_contribution, - msg.funding_pubkey, - )?; - - // TODO(splicing): Pre-check for reserve requirement - // (Note: It should also be checked later at tx_complete) + let splice_funding = self.validate_splice_ack(msg)?; log_info!( logger, @@ -11306,6 +11262,17 @@ where self.funding.get_value_satoshis(), ); + let pending_splice = + self.pending_splice.as_mut().expect("We should have returned an error earlier!"); + // TODO: Good candidate for a let else statement once MSRV >= 1.65 + let funding_negotiation_context = if let Some(FundingNegotiation::AwaitingAck(context)) = + pending_splice.funding_negotiation.take() + { + context + } else { + panic!("We should have returned an error earlier!"); + }; + let mut interactive_tx_constructor = funding_negotiation_context .into_interactive_tx_constructor( &self.context, @@ -11324,8 +11291,6 @@ where debug_assert!(self.interactive_tx_signing_session.is_none()); - let pending_splice = - self.pending_splice.as_mut().expect("pending_splice should still be set"); pending_splice.funding_negotiation = Some(FundingNegotiation::ConstructingTransaction( splice_funding, interactive_tx_constructor, @@ -11334,6 +11299,110 @@ where Ok(tx_msg_opt) } + /// Checks during handling splice_ack + #[cfg(splicing)] + fn validate_splice_ack(&self, msg: &msgs::SpliceAck) -> Result { + // TODO(splicing): Add check that we are the splice (quiescence) initiator + + let funding_negotiation_context = match &self + .pending_splice + .as_ref() + .ok_or(ChannelError::Ignore(format!("Channel is not in pending splice")))? + .funding_negotiation + { + Some(FundingNegotiation::AwaitingAck(context)) => context, + Some(FundingNegotiation::ConstructingTransaction(_, _)) + | Some(FundingNegotiation::AwaitingSignatures(_)) => { + 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 = funding_negotiation_context.our_funding_contribution; + debug_assert!(our_funding_contribution.abs() <= SignedAmount::MAX_MONEY); + + let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis); + self.validate_splice_contribution( + our_funding_contribution, + their_funding_contribution, + msg.funding_pubkey, + ) + } + + /// Used to validate a negative `funding_contribution_satoshis` in `splice_init` and `splice_ack` messages. + #[cfg(splicing)] + fn validate_their_funding_contribution_reserve( + &self, splice_funding: &FundingScope, + ) -> Result<(), ChannelError> { + // We don't care about the exact value of `dust_exposure_limiting_feerate` here as + // we do not validate dust exposure below, but we want to avoid triggering a debug + // assert. + // + // TODO: clean this up here and elsewhere. + let dust_exposure_limiting_feerate = + if splice_funding.get_channel_type().supports_anchor_zero_fee_commitments() { + None + } else { + Some(self.context.feerate_per_kw) + }; + // This *should* have no effect because no HTLC updates should be pending, but even if it does, + // the result may be a failed negotiation (and not a force-close), so we choose to include them. + let include_remote_unknown_htlcs = true; + // Make sure that that the funder of the channel can pay the transaction fees for an additional + // nondust HTLC on the channel. + let addl_nondust_htlc_count = 1; + + let validate_stats = |stats: NextCommitmentStats| { + let (_, remote_balance_incl_fee_msat) = stats.get_balances_including_fee_msat(); + let splice_remote_balance_msat = remote_balance_incl_fee_msat + .ok_or(ChannelError::WarnAndDisconnect(format!("Remote balance does not cover the sum of HTLCs, anchors, and commitment transaction fee")))?; + + // Check if the remote's new balance is under the specified reserve + if splice_remote_balance_msat + < splice_funding.holder_selected_channel_reserve_satoshis * 1000 + { + return Err(ChannelError::WarnAndDisconnect(format!( + "Remote balance below reserve mandated by holder: {} vs {}", + splice_remote_balance_msat, + splice_funding.holder_selected_channel_reserve_satoshis * 1000, + ))); + } + Ok(()) + }; + + // Reserve check on local commitment transaction + + let splice_local_commitment_stats = self.context.get_next_local_commitment_stats( + splice_funding, + None, // htlc_candidate + include_remote_unknown_htlcs, + addl_nondust_htlc_count, + self.context.feerate_per_kw, + dust_exposure_limiting_feerate, + ); + + validate_stats(splice_local_commitment_stats)?; + + // Reserve check on remote commitment transaction + + let splice_remote_commitment_stats = self.context.get_next_remote_commitment_stats( + splice_funding, + None, // htlc_candidate + include_remote_unknown_htlcs, + addl_nondust_htlc_count, + self.context.feerate_per_kw, + dust_exposure_limiting_feerate, + ); + + validate_stats(splice_remote_commitment_stats) + } + #[cfg(splicing)] pub fn splice_locked( &mut self, msg: &msgs::SpliceLocked, node_signer: &NS, chain_hash: ChainHash, diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs index f9c871d59b5..ed3f4746cab 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -35,6 +35,7 @@ impl HTLCAmountDirection { } pub(crate) struct NextCommitmentStats { + pub is_outbound_from_holder: bool, pub inbound_htlcs_count: usize, pub inbound_htlcs_value_msat: u64, pub holder_balance_before_fee_msat: Option, @@ -48,6 +49,26 @@ pub(crate) struct NextCommitmentStats { pub extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat: Option, } +impl NextCommitmentStats { + pub(crate) fn get_balances_including_fee_msat(&self) -> (Option, Option) { + if self.is_outbound_from_holder { + ( + self.holder_balance_before_fee_msat.and_then(|balance_msat| { + balance_msat.checked_sub(self.commit_tx_fee_sat * 1000) + }), + self.counterparty_balance_before_fee_msat, + ) + } else { + ( + self.holder_balance_before_fee_msat, + self.counterparty_balance_before_fee_msat.and_then(|balance_msat| { + balance_msat.checked_sub(self.commit_tx_fee_sat * 1000) + }), + ) + } + } +} + fn excess_fees_on_counterparty_tx_dust_exposure_msat( next_commitment_htlcs: &[HTLCAmountDirection], dust_buffer_feerate: u32, excess_feerate: u32, counterparty_dust_limit_satoshis: u64, dust_htlc_exposure_msat: u64, @@ -126,21 +147,19 @@ fn subtract_addl_outputs( // commitment transaction *before* checking whether the remote party's balance is enough to // cover the total anchor sum. - let local_balance_before_fee_msat = if is_outbound_from_holder { - value_to_self_after_htlcs_msat - .and_then(|balance_msat| balance_msat.checked_sub(total_anchors_sat * 1000)) - } else { - value_to_self_after_htlcs_msat - }; - - let remote_balance_before_fee_msat = if !is_outbound_from_holder { - value_to_remote_after_htlcs_msat - .and_then(|balance_msat| balance_msat.checked_sub(total_anchors_sat * 1000)) + if is_outbound_from_holder { + ( + value_to_self_after_htlcs_msat + .and_then(|balance_msat| balance_msat.checked_sub(total_anchors_sat * 1000)), + value_to_remote_after_htlcs_msat, + ) } else { - value_to_remote_after_htlcs_msat - }; - - (local_balance_before_fee_msat, remote_balance_before_fee_msat) + ( + value_to_self_after_htlcs_msat, + value_to_remote_after_htlcs_msat + .and_then(|balance_msat| balance_msat.checked_sub(total_anchors_sat * 1000)), + ) + } } fn get_dust_buffer_feerate(feerate_per_kw: u32) -> u32 { @@ -280,6 +299,7 @@ impl TxBuilder for SpecTxBuilder { }; NextCommitmentStats { + is_outbound_from_holder, inbound_htlcs_count, inbound_htlcs_value_msat, holder_balance_before_fee_msat,