From 19748faf289a9b2da1a4c926ba4db4c50a69c92a Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Sun, 5 Jan 2025 08:37:05 +0100 Subject: [PATCH 01/15] New splice_channel() for initiating splicing, handle splice_init and splice_ack messages, but fail afterwards --- lightning/src/ln/channel.rs | 291 ++++++++++++++++- lightning/src/ln/channelmanager.rs | 177 ++++++++++- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/functional_tests_splice.rs | 332 ++++++++++++++++++++ lightning/src/ln/mod.rs | 3 + 5 files changed, 793 insertions(+), 12 deletions(-) create mode 100644 lightning/src/ln/functional_tests_splice.rs diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1d64a396bfa..154e95b4bc7 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -11,7 +11,6 @@ use bitcoin::amount::Amount; use bitcoin::constants::ChainHash; use bitcoin::script::{Script, ScriptBuf, Builder, WScriptHash}; use bitcoin::transaction::{Transaction, TxIn}; -use bitcoin::sighash; use bitcoin::sighash::EcdsaSighashType; use bitcoin::consensus::encode; use bitcoin::absolute::LockTime; @@ -25,7 +24,7 @@ use bitcoin::hash_types::{Txid, BlockHash}; use bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE; use bitcoin::secp256k1::{PublicKey,SecretKey}; use bitcoin::secp256k1::{Secp256k1,ecdsa::Signature}; -use bitcoin::secp256k1; +use bitcoin::{secp256k1, sighash}; use crate::ln::types::ChannelId; use crate::types::payment::{PaymentPreimage, PaymentHash}; @@ -1541,6 +1540,30 @@ impl UnfundedChannelContext { } } +/// Info about a pending splice, used in the pre-splice channel +#[cfg(splicing)] +#[derive(Clone)] +struct PendingSplice { + pub our_funding_contribution: i64, +} + +#[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_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 SP::Target: SignerProvider { config: LegacyChannelConfig, @@ -2186,6 +2209,8 @@ impl PendingV2Channel where SP::Target: SignerProvider { context: self.context, interactive_tx_signing_session: Some(signing_session), holder_commitment_point, + #[cfg(splicing)] + pending_splice_pre: None, }; Ok((funded_chan, commitment_signed, funding_ready_for_sig_event)) }, @@ -3945,6 +3970,33 @@ impl ChannelContext where SP::Target: SignerProvider { (context.holder_selected_channel_reserve_satoshis, context.counterparty_selected_channel_reserve_satoshis) } + /// Check that a balance value meets the channel reserve requirements or violates them (below reserve). + /// The channel value is an input as opposed to using from self, so that this can be used in case of splicing + /// to checks with new channel value (before being comitted to it). + #[cfg(splicing)] + pub fn check_balance_meets_reserve_requirements(&self, balance: u64, channel_value: u64) -> Result<(), ChannelError> { + if balance == 0 { + return Ok(()); + } + let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( + channel_value, self.holder_dust_limit_satoshis); + if balance < holder_selected_channel_reserve_satoshis { + return Err(ChannelError::Warn(format!( + "Balance below reserve mandated by holder, {} vs {}", + balance, holder_selected_channel_reserve_satoshis, + ))); + } + let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( + channel_value, self.counterparty_dust_limit_satoshis); + if balance < counterparty_selected_channel_reserve_satoshis { + return Err(ChannelError::Warn(format!( + "Balance below reserve mandated by counterparty, {} vs {}", + balance, counterparty_selected_channel_reserve_satoshis, + ))); + } + Ok(()) + } + /// Get the commitment tx fee for the local's (i.e. our) next commitment transaction based on the /// number of pending HTLCs that are on track to be in our next commitment tx. /// @@ -4411,6 +4463,38 @@ impl ChannelContext where SP::Target: SignerProvider { self.channel_transaction_parameters = channel_transaction_parameters; self.get_initial_counterparty_commitment_signature(logger) } + + /// Get the splice message that can be sent during splice initiation. + #[cfg(splicing)] + pub fn get_splice_init(&self, our_funding_contribution_satoshis: i64, + funding_feerate_perkw: u32, locktime: u32, + ) -> msgs::SpliceInit { + // Reuse the existing funding pubkey, in spite of the channel value changing + // (though at this point we don't know the new value yet, due tue the optional counterparty contribution) + // Note that channel_keys_id is supposed NOT to change + let funding_pubkey = self.get_holder_pubkeys().funding_pubkey.clone(); + msgs::SpliceInit { + channel_id: self.channel_id, + funding_contribution_satoshis: our_funding_contribution_satoshis, + funding_feerate_perkw, + locktime, + funding_pubkey, + require_confirmed_inputs: None, + } + } + + /// Get the splice_ack message that can be sent in response to splice initiation. + #[cfg(splicing)] + pub fn get_splice_ack(&self, our_funding_contribution_satoshis: i64) -> msgs::SpliceAck { + // Reuse the existing funding pubkey, in spite of the channel value changing + let funding_pubkey = self.get_holder_pubkeys().funding_pubkey; + msgs::SpliceAck { + channel_id: self.channel_id, + funding_contribution_satoshis: our_funding_contribution_satoshis, + funding_pubkey, + require_confirmed_inputs: None, + } + } } // Internal utility functions for channels @@ -4535,6 +4619,9 @@ pub(super) struct FundedChannel where SP::Target: SignerProvider { pub context: ChannelContext, pub interactive_tx_signing_session: Option, holder_commitment_point: HolderCommitmentPoint, + /// Info about an in-progress, pending splice (if any), on the pre-splice channel + #[cfg(splicing)] + pending_splice_pre: Option, } #[cfg(any(test, fuzzing))] @@ -8114,6 +8201,135 @@ impl FundedChannel where } } + /// Initiate splicing + #[cfg(splicing)] + pub fn splice_channel(&mut self, our_funding_contribution_satoshis: i64, + our_funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_perkw: u32, locktime: u32, + ) -> Result { + // Check if a splice has been initiated already. + // Note: this could be handled more nicely, and support multiple outstanding splice's, the incoming splice_ack matters anyways. + if let Some(splice_info) = &self.pending_splice_pre { + return Err(ChannelError::Warn(format!( + "Channel has already a splice pending, contribution {}", splice_info.our_funding_contribution + ))); + } + + if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) { + return Err(ChannelError::Warn(format!("Cannot initiate splicing, as channel is not Ready"))); + } + + let pre_channel_value = self.context.get_value_satoshis(); + // Sanity check: capacity cannot decrease below 0 + if (pre_channel_value as i64).saturating_add(our_funding_contribution_satoshis) < 0 { + return Err(ChannelError::Warn(format!( + "Post-splicing channel value cannot be negative. It was {} + {}", + pre_channel_value, our_funding_contribution_satoshis + ))); + } + + if our_funding_contribution_satoshis < 0 { + return Err(ChannelError::Warn(format!( + "TODO(splicing): Splice-out not supported, only splice in, contribution {}", + our_funding_contribution_satoshis, + ))); + } + + // Note: post-splice channel value is not yet known at this point, counterpary contribution is not known + // (Cannot test for miminum required post-splice channel value) + + // Check that inputs are sufficient to cover our contribution + let sum_input: i64 = our_funding_inputs.into_iter().map( + |(txin, tx)| tx.output.get(txin.previous_output.vout as usize).map(|tx| tx.value.to_sat() as i64).unwrap_or(0) + ).sum(); + if sum_input < our_funding_contribution_satoshis { + return Err(ChannelError::Warn(format!( + "Provided inputs are insufficient for our contribution, {} {}", + sum_input, our_funding_contribution_satoshis, + ))); + } + + self.pending_splice_pre = Some(PendingSplice { + our_funding_contribution: our_funding_contribution_satoshis, + }); + + let msg = self.context.get_splice_init(our_funding_contribution_satoshis, funding_feerate_perkw, locktime); + Ok(msg) + } + + /// Handle splice_init + #[cfg(splicing)] + pub fn splice_init(&mut self, msg: &msgs::SpliceInit) -> Result { + let their_funding_contribution_satoshis = msg.funding_contribution_satoshis; + // TODO(splicing): Currently not possible to contribute on the splicing-acceptor side + let our_funding_contribution_satoshis = 0i64; + + // Check if a splice has been initiated already. + // Note: this could be handled more nicely, and support multiple outstanding splice's, the incoming splice_ack matters anyways. + if let Some(splice_info) = &self.pending_splice_pre { + return Err(ChannelError::Warn(format!( + "Channel has already a splice pending, contribution {}", splice_info.our_funding_contribution, + ))); + } + + if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) { + return Err(ChannelError::Warn(format!("Splicing requested on a channel that is not Ready"))); + } + + let pre_channel_value = self.context.get_value_satoshis(); + // Sanity check: capacity cannot decrease below 0 + if (pre_channel_value as i64) + .saturating_add(their_funding_contribution_satoshis) + .saturating_add(our_funding_contribution_satoshis) < 0 + { + return Err(ChannelError::Warn(format!( + "Post-splicing channel value cannot be negative. It was {} + {} + {}", + pre_channel_value, their_funding_contribution_satoshis, our_funding_contribution_satoshis, + ))); + } + + if their_funding_contribution_satoshis.saturating_add(our_funding_contribution_satoshis) < 0 { + return Err(ChannelError::Warn(format!( + "Splice-out not supported, only splice in, relative {} + {}", + their_funding_contribution_satoshis, our_funding_contribution_satoshis, + ))); + } + + let post_channel_value = PendingSplice::compute_post_value(pre_channel_value, their_funding_contribution_satoshis, our_funding_contribution_satoshis); + let post_balance = PendingSplice::add_checked(self.context.value_to_self_msat, our_funding_contribution_satoshis); + // Early check for reserve requirement, assuming maximum balance of full channel value + // This will also be checked later at tx_complete + let _res = self.context.check_balance_meets_reserve_requirements(post_balance, post_channel_value)?; + + // TODO(splicing): Store msg.funding_pubkey + // TODO(splicing): Apply start of splice (splice_start) + + let splice_ack_msg = self.context.get_splice_ack(our_funding_contribution_satoshis); + // TODO(splicing): start interactive funding negotiation + Ok(splice_ack_msg) + } + + /// Handle splice_ack + #[cfg(splicing)] + pub fn splice_ack(&mut self, msg: &msgs::SpliceAck) -> Result<(), ChannelError> { + let their_funding_contribution_satoshis = msg.funding_contribution_satoshis; + + // check if splice is pending + let pending_splice = if let Some(pending_splice) = &self.pending_splice_pre { + pending_splice + } else { + return Err(ChannelError::Warn(format!("Channel is not in pending splice"))); + }; + + let our_funding_contribution = pending_splice.our_funding_contribution; + + let pre_channel_value = self.context.get_value_satoshis(); + let post_channel_value = PendingSplice::compute_post_value(pre_channel_value, our_funding_contribution, their_funding_contribution_satoshis); + let post_balance = PendingSplice::add_checked(self.context.value_to_self_msat, our_funding_contribution); + // Early check for reserve requirement, assuming maximum balance of full channel value + // This will also be checked later at tx_complete + let _res = self.context.check_balance_meets_reserve_requirements(post_balance, post_channel_value)?; + Ok(()) + } // Send stuff to our remote peers: @@ -8808,6 +9024,8 @@ impl OutboundV1Channel where SP::Target: SignerProvider { context: self.context, interactive_tx_signing_session: None, holder_commitment_point, + #[cfg(splicing)] + pending_splice_pre: None, }; let need_channel_ready = channel.check_get_channel_ready(0, logger).is_some() @@ -9073,6 +9291,8 @@ impl InboundV1Channel where SP::Target: SignerProvider { context: self.context, interactive_tx_signing_session: None, holder_commitment_point, + #[cfg(splicing)] + pending_splice_pre: None, }; let need_channel_ready = channel.check_get_channel_ready(0, logger).is_some() || channel.context.signer_pending_channel_ready; @@ -10454,6 +10674,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch }, interactive_tx_signing_session: None, holder_commitment_point, + #[cfg(splicing)] + pending_splice_pre: None, }) } } @@ -12236,4 +12458,69 @@ 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()); } + + #[cfg(all(test, splicing))] + fn get_pre_and_post(pre_channel_value: u64, our_funding_contribution: i64, their_funding_contribution: i64) -> (u64, u64) { + use crate::ln::channel::PendingSplice; + + let post_channel_value = PendingSplice::compute_post_value(pre_channel_value, our_funding_contribution, their_funding_contribution); + (pre_channel_value, post_channel_value) + } + + #[cfg(all(test, splicing))] + #[test] + fn test_splice_compute_post_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/channelmanager.rs b/lightning/src/ln/channelmanager.rs index eac2a1474d7..f7906679927 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -49,7 +49,7 @@ use crate::ln::inbound_payment; use crate::ln::types::ChannelId; use crate::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; use crate::ln::channel::{self, Channel, ChannelError, ChannelUpdateStatus, FundedChannel, ShutdownResult, UpdateFulfillCommitFetch, OutboundV1Channel, ReconnectionMsg, InboundV1Channel, WithChannelContext}; -#[cfg(any(dual_funding, splicing))] +#[cfg(dual_funding)] use crate::ln::channel::PendingV2Channel; use crate::ln::channel_state::ChannelDetails; use crate::types::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures}; @@ -4231,6 +4231,63 @@ where } } + /// Initiate a splice, to change the channel capacity of an existing funded channel. + /// After completion of splicing, the funding transaction will be replaced by a new one, spending the old funding transaction, + /// with optional extra inputs (splice-in) and/or extra outputs (splice-out or change). + /// TODO(splicing): Implementation is currently incomplete. + /// Note: Currently only splice-in is supported (increase in channel capacity), splice-out is not. + /// - our_funding_contribution_satoshis: the amount contributed by us to the channel. This will increase our channel balance. + /// - our_funding_inputs: the funding inputs provided by us. If our contribution is positive, our funding inputs must cover at least that amount. + #[cfg(splicing)] + pub fn splice_channel( + &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, our_funding_contribution_satoshis: i64, + our_funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_perkw: u32, locktime: u32, + ) -> Result<(), APIError> { + let per_peer_state = self.per_peer_state.read().unwrap(); + + let peer_state_mutex = per_peer_state.get(counterparty_node_id) + .ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })?; + + let mut peer_state_lock = peer_state_mutex.lock().unwrap(); + let peer_state = &mut *peer_state_lock; + + // Look for the channel + match peer_state.channel_by_id.entry(*channel_id) { + hash_map::Entry::Occupied(mut chan_phase_entry) => { + if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() { + let msg = chan.splice_channel(our_funding_contribution_satoshis, our_funding_inputs, funding_feerate_perkw, locktime) + .map_err(|err| APIError::APIMisuseError { + err: format!( + "Cannot initiate Splicing, {}, channel ID {}", err, channel_id + ) + })?; + + peer_state.pending_msg_events.push(events::MessageSendEvent::SendSpliceInit { + node_id: *counterparty_node_id, + msg, + }); + + Ok(()) + } else { + Err(APIError::ChannelUnavailable { + err: format!( + "Channel with id {} is not funded, cannot splice it", + channel_id + ) + }) + } + }, + hash_map::Entry::Vacant(_) => { + return Err(APIError::ChannelUnavailable { + err: format!( + "Channel with id {} not found for the passed counterparty node_id {}", + channel_id, counterparty_node_id, + ) + }); + }, + } + } + fn can_forward_htlc_to_outgoing_channel( &self, chan: &mut FundedChannel, msg: &msgs::UpdateAddHTLC, next_packet: &NextPacketDetails ) -> Result<(), (&'static str, u16)> { @@ -9245,6 +9302,94 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Ok(NotifyOption::SkipPersistHandleEvents) } + /// Handle incoming splice request, transition channel to splice-pending (unless some check fails). + #[cfg(splicing)] + fn internal_splice_init(&self, counterparty_node_id: &PublicKey, msg: &msgs::SpliceInit) -> Result<(), MsgHandleErrInternal> { + // TODO(splicing): if we accept splicing, quiescence + + let per_peer_state = self.per_peer_state.read().unwrap(); + let peer_state_mutex = per_peer_state.get(counterparty_node_id) + .ok_or_else(|| { + debug_assert!(false); + MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id) + })?; + let mut peer_state_lock = peer_state_mutex.lock().unwrap(); + let peer_state = &mut *peer_state_lock; + + // Look for the channel + match peer_state.channel_by_id.entry(msg.channel_id) { + hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!( + "Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}, channel_id {}", + counterparty_node_id, msg.channel_id, + ), msg.channel_id)), + hash_map::Entry::Occupied(mut chan_entry) => { + if let Some(chan) = chan_entry.get_mut().as_funded_mut() { + match chan.splice_init(msg) { + Ok(splice_ack_msg) => { + peer_state.pending_msg_events.push(events::MessageSendEvent::SendSpliceAck { + node_id: *counterparty_node_id, + msg: splice_ack_msg, + }); + }, + Err(err) => { + return Err(MsgHandleErrInternal::from_chan_no_close(err, msg.channel_id)); + } + } + } else { + return Err(MsgHandleErrInternal::send_err_msg_no_close("Channel is not funded, cannot be spliced".to_owned(), msg.channel_id)); + } + }, + }; + + // TODO(splicing): + // Change channel, change phase (remove and add) + // Create new post-splice channel + // etc. + + Ok(()) + } + + /// Handle incoming splice request ack, transition channel to splice-pending (unless some check fails). + #[cfg(splicing)] + fn internal_splice_ack(&self, counterparty_node_id: &PublicKey, msg: &msgs::SpliceAck) -> Result<(), MsgHandleErrInternal> { + let per_peer_state = self.per_peer_state.read().unwrap(); + let peer_state_mutex = per_peer_state.get(counterparty_node_id) + .ok_or_else(|| { + debug_assert!(false); + MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id) + })?; + let mut peer_state_lock = peer_state_mutex.lock().unwrap(); + let peer_state = &mut *peer_state_lock; + + // Look for the channel + match peer_state.channel_by_id.entry(msg.channel_id) { + hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!( + "Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", + counterparty_node_id + ), msg.channel_id)), + hash_map::Entry::Occupied(mut chan) => { + if let Some(chan) = chan.get_mut().as_funded_mut() { + match chan.splice_ack(msg) { + Ok(_) => {} + Err(err) => { + return Err(MsgHandleErrInternal::from_chan_no_close(err, msg.channel_id)); + } + } + } else { + return Err(MsgHandleErrInternal::send_err_msg_no_close("Channel is not funded, cannot splice".to_owned(), msg.channel_id)); + } + }, + }; + + // TODO(splicing): + // Change channel, change phase (remove and add) + // Create new post-splice channel + // Start splice funding transaction negotiation + // etc. + + Err(MsgHandleErrInternal::send_err_msg_no_close("TODO(splicing): Splicing is not implemented (splice_ack)".to_owned(), msg.channel_id)) + } + /// Process pending events from the [`chain::Watch`], returning whether any events were processed. fn process_pending_monitor_events(&self) -> bool { debug_assert!(self.total_consistency_lock.try_write().is_err()); // Caller holds read lock @@ -11371,28 +11516,42 @@ where fn handle_stfu(&self, counterparty_node_id: PublicKey, msg: &msgs::Stfu) { let _: Result<(), _> = handle_error!(self, Err(MsgHandleErrInternal::send_err_msg_no_close( "Quiescence not supported".to_owned(), - msg.channel_id.clone())), counterparty_node_id); + msg.channel_id)), counterparty_node_id); } #[cfg(splicing)] fn handle_splice_init(&self, counterparty_node_id: PublicKey, msg: &msgs::SpliceInit) { - let _: Result<(), _> = handle_error!(self, Err(MsgHandleErrInternal::send_err_msg_no_close( - "Splicing not supported".to_owned(), - msg.channel_id.clone())), counterparty_node_id); + let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || { + let res = self.internal_splice_init(&counterparty_node_id, msg); + let persist = match &res { + Err(e) if e.closes_channel() => NotifyOption::DoPersist, + Err(_) => NotifyOption::SkipPersistHandleEvents, + Ok(()) => NotifyOption::SkipPersistNoEvents, + }; + let _ = handle_error!(self, res, counterparty_node_id); + persist + }); } #[cfg(splicing)] fn handle_splice_ack(&self, counterparty_node_id: PublicKey, msg: &msgs::SpliceAck) { - let _: Result<(), _> = handle_error!(self, Err(MsgHandleErrInternal::send_err_msg_no_close( - "Splicing not supported (splice_ack)".to_owned(), - msg.channel_id.clone())), counterparty_node_id); + let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || { + let res = self.internal_splice_ack(&counterparty_node_id, msg); + let persist = match &res { + Err(e) if e.closes_channel() => NotifyOption::DoPersist, + Err(_) => NotifyOption::SkipPersistHandleEvents, + Ok(()) => NotifyOption::SkipPersistNoEvents, + }; + let _ = handle_error!(self, res, counterparty_node_id); + persist + }); } #[cfg(splicing)] fn handle_splice_locked(&self, counterparty_node_id: PublicKey, msg: &msgs::SpliceLocked) { let _: Result<(), _> = handle_error!(self, Err(MsgHandleErrInternal::send_err_msg_no_close( "Splicing not supported (splice_locked)".to_owned(), - msg.channel_id.clone())), counterparty_node_id); + msg.channel_id)), counterparty_node_id); } fn handle_shutdown(&self, counterparty_node_id: PublicKey, msg: &msgs::Shutdown) { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index f1361337b35..f8c5910e637 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -806,7 +806,7 @@ macro_rules! get_event_msg { assert_eq!(*node_id, $node_id); (*msg).clone() }, - _ => panic!("Unexpected event"), + _ => panic!("Unexpected event {:?}", events[0]), } } } diff --git a/lightning/src/ln/functional_tests_splice.rs b/lightning/src/ln/functional_tests_splice.rs new file mode 100644 index 00000000000..c17334ef63a --- /dev/null +++ b/lightning/src/ln/functional_tests_splice.rs @@ -0,0 +1,332 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +//! Tests that test standing up a network of ChannelManagers, creating channels, sending +//! payments/messages between them, and often checking the resulting ChannelMonitors are able to +//! claim outputs on-chain. + +use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider}; +use crate::ln::functional_test_utils::*; +use crate::ln::msgs::ChannelMessageHandler; +use crate::util::config::{ChannelHandshakeConfig, UserConfig}; + +/// Splicing test, simple splice-in flow. Starts with opening a V1 channel first. +/// Builds on test_channel_open_simple() +#[test] +fn test_v1_splice_in() { + // Set up a network of 2 nodes + let cfg = UserConfig { + channel_handshake_config: ChannelHandshakeConfig { ..Default::default() }, + ..Default::default() + }; + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(cfg), None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + // Initiator and Acceptor nodes + let initiator_node_index = 0; + let acceptor_node_index = 1; + let initiator_node = &nodes[initiator_node_index]; + let acceptor_node = &nodes[acceptor_node_index]; + + // Instantiate channel parameters where we push the maximum msats given our funding satoshis + let channel_value_sat = 100_000; // same as funding satoshis + let push_msat = 0; + let channel_reserve_amnt_sat = 1_000; + + let expected_funded_channel_id = + "ae3367da2c13bc1ceb86bf56418f62828f7ce9d6bfb15a46af5ba1f1ed8b124f"; + + // Have initiator_node initiate a channel to acceptor_node with aforementioned parameters + let channel_id_temp1 = initiator_node + .node + .create_channel( + acceptor_node.node.get_our_node_id(), + channel_value_sat, + push_msat, + 42, + None, + None, + ) + .unwrap(); + + // Extract the channel open message from initiator_node to acceptor_node + let open_channel_message = get_event_msg!( + initiator_node, + MessageSendEvent::SendOpenChannel, + acceptor_node.node.get_our_node_id() + ); + let expected_initiator_funding_key = + "03c21e841cbc0b48197d060c71e116c185fa0ac281b7d0aa5924f535154437ca3b"; + assert_eq!( + open_channel_message.common_fields.funding_pubkey.to_string(), + expected_initiator_funding_key + ); + + let _res = acceptor_node + .node + .handle_open_channel(initiator_node.node.get_our_node_id(), &open_channel_message.clone()); + // Extract the accept channel message from acceptor_node to initiator_node + let accept_channel_message = get_event_msg!( + acceptor_node, + MessageSendEvent::SendAcceptChannel, + initiator_node.node.get_our_node_id() + ); + let expected_acceptor_funding_key = + "039481c28b904cbe12681e79937373fc76245c1b29871028ae60ba3152162c319b"; + assert_eq!( + accept_channel_message.common_fields.funding_pubkey.to_string(), + expected_acceptor_funding_key + ); + + let _res = initiator_node.node.handle_accept_channel( + acceptor_node.node.get_our_node_id(), + &accept_channel_message.clone(), + ); + // Note: FundingGenerationReady emitted, checked and used below + let (_channel_id_temp2, funding_tx, _funding_output) = create_funding_transaction( + &initiator_node, + &acceptor_node.node.get_our_node_id(), + channel_value_sat, + 42, + ); + + // Funding transation created, provide it + let _res = initiator_node + .node + .funding_transaction_generated( + channel_id_temp1, + acceptor_node.node.get_our_node_id(), + funding_tx.clone(), + ) + .unwrap(); + + let funding_created_message = get_event_msg!( + initiator_node, + MessageSendEvent::SendFundingCreated, + acceptor_node.node.get_our_node_id() + ); + + let _res = acceptor_node + .node + .handle_funding_created(initiator_node.node.get_our_node_id(), &funding_created_message); + + assert_eq!(initiator_node.node.list_channels().len(), 1); + { + let channel = &initiator_node.node.list_channels()[0]; + assert!(!channel.is_channel_ready); + } + // do checks on the acceptor node as well (capacity, etc.) + assert_eq!(acceptor_node.node.list_channels().len(), 1); + { + let channel = &acceptor_node.node.list_channels()[0]; + assert!(!channel.is_channel_ready); + } + + let funding_signed_message = get_event_msg!( + acceptor_node, + MessageSendEvent::SendFundingSigned, + initiator_node.node.get_our_node_id() + ); + let _res = initiator_node + .node + .handle_funding_signed(acceptor_node.node.get_our_node_id(), &funding_signed_message); + // Take new channel ID + let channel_id2 = funding_signed_message.channel_id; + assert_eq!(channel_id2.to_string(), expected_funded_channel_id); + + // Check that funding transaction has been broadcasted + assert_eq!( + chanmon_cfgs[initiator_node_index].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), + 1 + ); + let broadcasted_funding_tx = + chanmon_cfgs[initiator_node_index].tx_broadcaster.txn_broadcasted.lock().unwrap()[0] + .clone(); + + check_added_monitors!(initiator_node, 1); + let _ev = get_event!(initiator_node, Event::ChannelPending); + check_added_monitors!(acceptor_node, 1); + let _ev = get_event!(acceptor_node, Event::ChannelPending); + + // Simulate confirmation of the funding tx + confirm_transaction(&initiator_node, &broadcasted_funding_tx); + let channel_ready_message = get_event_msg!( + initiator_node, + MessageSendEvent::SendChannelReady, + acceptor_node.node.get_our_node_id() + ); + + confirm_transaction(&acceptor_node, &broadcasted_funding_tx); + let channel_ready_message2 = get_event_msg!( + acceptor_node, + MessageSendEvent::SendChannelReady, + initiator_node.node.get_our_node_id() + ); + + let _res = acceptor_node + .node + .handle_channel_ready(initiator_node.node.get_our_node_id(), &channel_ready_message); + let _ev = get_event!(acceptor_node, Event::ChannelReady); + let _channel_update = get_event_msg!( + acceptor_node, + MessageSendEvent::SendChannelUpdate, + initiator_node.node.get_our_node_id() + ); + + let _res = initiator_node + .node + .handle_channel_ready(acceptor_node.node.get_our_node_id(), &channel_ready_message2); + let _ev = get_event!(initiator_node, Event::ChannelReady); + let _channel_update = get_event_msg!( + initiator_node, + MessageSendEvent::SendChannelUpdate, + acceptor_node.node.get_our_node_id() + ); + + // check channel capacity and other parameters + assert_eq!(initiator_node.node.list_channels().len(), 1); + { + let channel = &initiator_node.node.list_channels()[0]; + assert_eq!(channel.channel_id.to_string(), expected_funded_channel_id); + assert!(channel.is_usable); + assert!(channel.is_channel_ready); + assert_eq!(channel.channel_value_satoshis, channel_value_sat); + assert_eq!( + channel.outbound_capacity_msat, + 1000 * (channel_value_sat - channel_reserve_amnt_sat) + ); + assert_eq!(channel.funding_txo.unwrap().txid, funding_tx.compute_txid()); + assert_eq!(channel.confirmations.unwrap(), 10); + } + // do checks on the acceptor node as well (capacity, etc.) + assert_eq!(acceptor_node.node.list_channels().len(), 1); + { + let channel = &acceptor_node.node.list_channels()[0]; + assert_eq!(channel.channel_id.to_string(), expected_funded_channel_id); + assert!(channel.is_usable); + assert!(channel.is_channel_ready); + assert_eq!(channel.channel_value_satoshis, channel_value_sat); + assert_eq!(channel.outbound_capacity_msat, 0); + assert_eq!(channel.funding_txo.unwrap().txid, funding_tx.compute_txid()); + assert_eq!(channel.confirmations.unwrap(), 10); + } + + // ==== Channel is now ready for normal operation + + // === Start of Splicing + println!("Start of Splicing ..., channel_id {}", channel_id2); + + // Amount being added to the channel through the splice-in + let splice_in_sats: u64 = 20000; + let funding_feerate_perkw = 1024; // TODO + let locktime = 0; // TODO + + // Create additional inputs + let extra_splice_funding_input_sats = 35_000; + let funding_inputs = create_dual_funding_utxos_with_prev_txs( + &initiator_node, + &[extra_splice_funding_input_sats], + ); + // Initiate splice-in (on initiator_node) + let _res = initiator_node + .node + .splice_channel( + &channel_id2, + &acceptor_node.node.get_our_node_id(), + splice_in_sats as i64, + funding_inputs, + funding_feerate_perkw, + locktime, + ) + .unwrap(); + // Extract the splice message from node0 to node1 + let splice_init_msg = get_event_msg!( + initiator_node, + MessageSendEvent::SendSpliceInit, + acceptor_node.node.get_our_node_id() + ); + assert_eq!(splice_init_msg.funding_contribution_satoshis, splice_in_sats as i64); + assert_eq!(splice_init_msg.funding_feerate_perkw, funding_feerate_perkw); + assert_eq!(splice_init_msg.funding_pubkey.to_string(), expected_initiator_funding_key); + assert!(splice_init_msg.require_confirmed_inputs.is_none()); + + let _res = acceptor_node + .node + .handle_splice_init(initiator_node.node.get_our_node_id(), &splice_init_msg); + // Extract the splice_ack message from node1 to node0 + let splice_ack_msg = get_event_msg!( + acceptor_node, + MessageSendEvent::SendSpliceAck, + initiator_node.node.get_our_node_id() + ); + assert_eq!(splice_ack_msg.funding_contribution_satoshis, 0); + assert_eq!(splice_ack_msg.funding_pubkey.to_string(), expected_acceptor_funding_key); + assert!(splice_ack_msg.require_confirmed_inputs.is_none()); + + // still pre-splice channel: capacity not updated, channel usable, and funding tx set + assert_eq!(acceptor_node.node.list_channels().len(), 1); + { + let channel = &acceptor_node.node.list_channels()[0]; + assert_eq!(channel.channel_id.to_string(), expected_funded_channel_id); + assert!(channel.is_usable); + assert!(channel.is_channel_ready); + assert_eq!(channel.channel_value_satoshis, channel_value_sat); + assert_eq!(channel.outbound_capacity_msat, 0); + assert!(channel.funding_txo.is_some()); + assert!(channel.confirmations.unwrap() > 0); + } + + let _res = initiator_node + .node + .handle_splice_ack(acceptor_node.node.get_our_node_id(), &splice_ack_msg); + + // still pre-splice channel: capacity not updated, channel usable, and funding tx set + assert_eq!(initiator_node.node.list_channels().len(), 1); + { + let channel = &initiator_node.node.list_channels()[0]; + assert_eq!(channel.channel_id.to_string(), expected_funded_channel_id); + assert!(channel.is_usable); + assert!(channel.is_channel_ready); + assert_eq!(channel.channel_value_satoshis, channel_value_sat); + assert_eq!( + channel.outbound_capacity_msat, + 1000 * (channel_value_sat - channel_reserve_amnt_sat) + ); + assert!(channel.funding_txo.is_some()); + assert!(channel.confirmations.unwrap() > 0); + } + + let _error_msg = get_err_msg(initiator_node, &acceptor_node.node.get_our_node_id()); + + // TODO(splicing): continue with splice transaction negotiation + + // === Close channel, cooperatively + initiator_node.node.close_channel(&channel_id2, &acceptor_node.node.get_our_node_id()).unwrap(); + let node0_shutdown_message = get_event_msg!( + initiator_node, + MessageSendEvent::SendShutdown, + acceptor_node.node.get_our_node_id() + ); + acceptor_node + .node + .handle_shutdown(initiator_node.node.get_our_node_id(), &node0_shutdown_message); + let nodes_1_shutdown = get_event_msg!( + acceptor_node, + MessageSendEvent::SendShutdown, + initiator_node.node.get_our_node_id() + ); + initiator_node.node.handle_shutdown(acceptor_node.node.get_our_node_id(), &nodes_1_shutdown); + let _ = get_event_msg!( + initiator_node, + MessageSendEvent::SendClosingSigned, + acceptor_node.node.get_our_node_id() + ); +} diff --git a/lightning/src/ln/mod.rs b/lightning/src/ln/mod.rs index 201f2ecfeff..6d9c387e91c 100644 --- a/lightning/src/ln/mod.rs +++ b/lightning/src/ln/mod.rs @@ -61,6 +61,9 @@ mod async_payments_tests; #[cfg(test)] #[allow(unused_mut)] mod functional_tests; +#[cfg(all(test, splicing))] +#[allow(unused_mut)] +mod functional_tests_splice; #[cfg(test)] #[allow(unused_mut)] mod max_payment_path_len_tests; From 89f55dc20307c9b95a208c4d455c151c5c3393c4 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Wed, 15 Jan 2025 08:21:06 +0100 Subject: [PATCH 02/15] Minor rename (review, funding_feerate_per_kw) --- lightning/src/ln/channel.rs | 8 ++++---- lightning/src/ln/channelmanager.rs | 4 ++-- lightning/src/ln/functional_tests_splice.rs | 6 +++--- lightning/src/ln/msgs.rs | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 154e95b4bc7..5f0b2bc337e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4467,7 +4467,7 @@ impl ChannelContext where SP::Target: SignerProvider { /// Get the splice message that can be sent during splice initiation. #[cfg(splicing)] pub fn get_splice_init(&self, our_funding_contribution_satoshis: i64, - funding_feerate_perkw: u32, locktime: u32, + funding_feerate_per_kw: u32, locktime: u32, ) -> msgs::SpliceInit { // Reuse the existing funding pubkey, in spite of the channel value changing // (though at this point we don't know the new value yet, due tue the optional counterparty contribution) @@ -4476,7 +4476,7 @@ impl ChannelContext where SP::Target: SignerProvider { msgs::SpliceInit { channel_id: self.channel_id, funding_contribution_satoshis: our_funding_contribution_satoshis, - funding_feerate_perkw, + funding_feerate_per_kw, locktime, funding_pubkey, require_confirmed_inputs: None, @@ -8204,7 +8204,7 @@ impl FundedChannel where /// Initiate splicing #[cfg(splicing)] pub fn splice_channel(&mut self, our_funding_contribution_satoshis: i64, - our_funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_perkw: u32, locktime: u32, + our_funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_per_kw: u32, locktime: u32, ) -> Result { // Check if a splice has been initiated already. // Note: this could be handled more nicely, and support multiple outstanding splice's, the incoming splice_ack matters anyways. @@ -8252,7 +8252,7 @@ impl FundedChannel where our_funding_contribution: our_funding_contribution_satoshis, }); - let msg = self.context.get_splice_init(our_funding_contribution_satoshis, funding_feerate_perkw, locktime); + let msg = self.context.get_splice_init(our_funding_contribution_satoshis, funding_feerate_per_kw, locktime); Ok(msg) } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f7906679927..ac9a173498d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4241,7 +4241,7 @@ where #[cfg(splicing)] pub fn splice_channel( &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, our_funding_contribution_satoshis: i64, - our_funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_perkw: u32, locktime: u32, + our_funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_per_kw: u32, locktime: u32, ) -> Result<(), APIError> { let per_peer_state = self.per_peer_state.read().unwrap(); @@ -4255,7 +4255,7 @@ where match peer_state.channel_by_id.entry(*channel_id) { hash_map::Entry::Occupied(mut chan_phase_entry) => { if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() { - let msg = chan.splice_channel(our_funding_contribution_satoshis, our_funding_inputs, funding_feerate_perkw, locktime) + let msg = chan.splice_channel(our_funding_contribution_satoshis, our_funding_inputs, funding_feerate_per_kw, locktime) .map_err(|err| APIError::APIMisuseError { err: format!( "Cannot initiate Splicing, {}, channel ID {}", err, channel_id diff --git a/lightning/src/ln/functional_tests_splice.rs b/lightning/src/ln/functional_tests_splice.rs index c17334ef63a..6c9fa111454 100644 --- a/lightning/src/ln/functional_tests_splice.rs +++ b/lightning/src/ln/functional_tests_splice.rs @@ -226,7 +226,7 @@ fn test_v1_splice_in() { // Amount being added to the channel through the splice-in let splice_in_sats: u64 = 20000; - let funding_feerate_perkw = 1024; // TODO + let funding_feerate_per_kw = 1024; // TODO let locktime = 0; // TODO // Create additional inputs @@ -243,7 +243,7 @@ fn test_v1_splice_in() { &acceptor_node.node.get_our_node_id(), splice_in_sats as i64, funding_inputs, - funding_feerate_perkw, + funding_feerate_per_kw, locktime, ) .unwrap(); @@ -254,7 +254,7 @@ fn test_v1_splice_in() { acceptor_node.node.get_our_node_id() ); assert_eq!(splice_init_msg.funding_contribution_satoshis, splice_in_sats as i64); - assert_eq!(splice_init_msg.funding_feerate_perkw, funding_feerate_perkw); + assert_eq!(splice_init_msg.funding_feerate_per_kw, funding_feerate_per_kw); assert_eq!(splice_init_msg.funding_pubkey.to_string(), expected_initiator_funding_key); assert!(splice_init_msg.require_confirmed_inputs.is_none()); diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 659ec65f6cf..839583ddca5 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -469,7 +469,7 @@ pub struct SpliceInit { /// or remove from its channel balance (splice-out). pub funding_contribution_satoshis: i64, /// The feerate for the new funding transaction, set by the splice initiator - pub funding_feerate_perkw: u32, + pub funding_feerate_per_kw: u32, /// The locktime for the new funding transaction pub locktime: u32, /// The key of the sender (splice initiator) controlling the new funding transaction @@ -2191,7 +2191,7 @@ impl_writeable_msg!(Stfu, { impl_writeable_msg!(SpliceInit, { channel_id, funding_contribution_satoshis, - funding_feerate_perkw, + funding_feerate_per_kw, locktime, funding_pubkey, }, { @@ -4040,7 +4040,7 @@ mod tests { let splice_init = msgs::SpliceInit { channel_id: ChannelId::from_bytes([2; 32]), funding_contribution_satoshis: -123456, - funding_feerate_perkw: 2000, + funding_feerate_per_kw: 2000, locktime: 0, funding_pubkey: pubkey_1, require_confirmed_inputs: Some(()), From a965c64296df719d0053b5d5ca47cfd3e8246414 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Wed, 15 Jan 2025 08:22:47 +0100 Subject: [PATCH 03/15] Minor comment/TODO/typo changes (review) --- lightning/src/ln/channel.rs | 8 +++++--- lightning/src/ln/channelmanager.rs | 2 ++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 5f0b2bc337e..cd835aa899b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4470,7 +4470,7 @@ impl ChannelContext where SP::Target: SignerProvider { funding_feerate_per_kw: u32, locktime: u32, ) -> msgs::SpliceInit { // Reuse the existing funding pubkey, in spite of the channel value changing - // (though at this point we don't know the new value yet, due tue the optional counterparty contribution) + // (though at this point we don't know the new value yet, due to the optional counterparty contribution) // Note that channel_keys_id is supposed NOT to change let funding_pubkey = self.get_holder_pubkeys().funding_pubkey.clone(); msgs::SpliceInit { @@ -8207,7 +8207,7 @@ impl FundedChannel where our_funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_per_kw: u32, locktime: u32, ) -> Result { // Check if a splice has been initiated already. - // Note: this could be handled more nicely, and support multiple outstanding splice's, the incoming splice_ack matters anyways. + // Note: only a single outstanding splice is supported (per spec) if let Some(splice_info) = &self.pending_splice_pre { return Err(ChannelError::Warn(format!( "Channel has already a splice pending, contribution {}", splice_info.our_funding_contribution @@ -8218,6 +8218,8 @@ impl FundedChannel where return Err(ChannelError::Warn(format!("Cannot initiate splicing, as channel is not Ready"))); } + // TODO(splicing): check for quiescence + let pre_channel_value = self.context.get_value_satoshis(); // Sanity check: capacity cannot decrease below 0 if (pre_channel_value as i64).saturating_add(our_funding_contribution_satoshis) < 0 { @@ -8234,7 +8236,7 @@ impl FundedChannel where ))); } - // Note: post-splice channel value is not yet known at this point, counterpary contribution is not known + // Note: post-splice channel value is not yet known at this point, counterparty contribution is not known // (Cannot test for miminum required post-splice channel value) // Check that inputs are sufficient to cover our contribution diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index ac9a173498d..88bcd40859e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -11519,6 +11519,7 @@ where msg.channel_id)), counterparty_node_id); } + /// TODO(splicing): Implement persisting #[cfg(splicing)] fn handle_splice_init(&self, counterparty_node_id: PublicKey, msg: &msgs::SpliceInit) { let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || { @@ -11533,6 +11534,7 @@ where }); } + /// TODO(splicing): Implement persisting #[cfg(splicing)] fn handle_splice_ack(&self, counterparty_node_id: PublicKey, msg: &msgs::SpliceAck) { let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || { From 32b215d6c71e085d7587d9a51429a4b229cabe71 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Wed, 15 Jan 2025 18:28:18 +0100 Subject: [PATCH 04/15] Comments (review) --- lightning/src/ln/channel.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index cd835aa899b..6ff6cadb13f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3974,7 +3974,7 @@ impl ChannelContext where SP::Target: SignerProvider { /// The channel value is an input as opposed to using from self, so that this can be used in case of splicing /// to checks with new channel value (before being comitted to it). #[cfg(splicing)] - pub fn check_balance_meets_reserve_requirements(&self, balance: u64, channel_value: u64) -> Result<(), ChannelError> { + pub fn check_balance_meets_v2_reserve_requirements(&self, balance: u64, channel_value: u64) -> Result<(), ChannelError> { if balance == 0 { return Ok(()); } @@ -4469,8 +4469,7 @@ impl ChannelContext where SP::Target: SignerProvider { pub fn get_splice_init(&self, our_funding_contribution_satoshis: i64, funding_feerate_per_kw: u32, locktime: u32, ) -> msgs::SpliceInit { - // Reuse the existing funding pubkey, in spite of the channel value changing - // (though at this point we don't know the new value yet, due to the optional counterparty contribution) + // TODO(splicing): The exisiting pubkey is reused, but a new one should be generated. See #3542. // Note that channel_keys_id is supposed NOT to change let funding_pubkey = self.get_holder_pubkeys().funding_pubkey.clone(); msgs::SpliceInit { @@ -4486,7 +4485,8 @@ impl ChannelContext where SP::Target: SignerProvider { /// Get the splice_ack message that can be sent in response to splice initiation. #[cfg(splicing)] pub fn get_splice_ack(&self, our_funding_contribution_satoshis: i64) -> msgs::SpliceAck { - // Reuse the existing funding pubkey, in spite of the channel value changing + // TODO(splicing): The exisiting pubkey is reused, but a new one should be generated. See #3542. + // Note that channel_keys_id is supposed NOT to change let funding_pubkey = self.get_holder_pubkeys().funding_pubkey; msgs::SpliceAck { channel_id: self.channel_id, @@ -8300,7 +8300,7 @@ impl FundedChannel where let post_balance = PendingSplice::add_checked(self.context.value_to_self_msat, our_funding_contribution_satoshis); // Early check for reserve requirement, assuming maximum balance of full channel value // This will also be checked later at tx_complete - let _res = self.context.check_balance_meets_reserve_requirements(post_balance, post_channel_value)?; + let _res = self.context.check_balance_meets_v2_reserve_requirements(post_balance, post_channel_value)?; // TODO(splicing): Store msg.funding_pubkey // TODO(splicing): Apply start of splice (splice_start) @@ -8329,7 +8329,7 @@ impl FundedChannel where let post_balance = PendingSplice::add_checked(self.context.value_to_self_msat, our_funding_contribution); // Early check for reserve requirement, assuming maximum balance of full channel value // This will also be checked later at tx_complete - let _res = self.context.check_balance_meets_reserve_requirements(post_balance, post_channel_value)?; + let _res = self.context.check_balance_meets_v2_reserve_requirements(post_balance, post_channel_value)?; Ok(()) } From 8f2dd0bab8a63e5012c7fc04be1a531cefee1ecd Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Fri, 17 Jan 2025 16:58:22 +0100 Subject: [PATCH 05/15] Comments; reg. reserve check; review --- lightning/src/ln/channel.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 6ff6cadb13f..e2a783eeac9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8239,7 +8239,8 @@ impl FundedChannel where // Note: post-splice channel value is not yet known at this point, counterparty contribution is not known // (Cannot test for miminum required post-splice channel value) - // Check that inputs are sufficient to cover our contribution + // Pre-check that inputs are sufficient to cover our contribution. + // Note: fees are not taken into account here. let sum_input: i64 = our_funding_inputs.into_iter().map( |(txin, tx)| tx.output.get(txin.previous_output.vout as usize).map(|tx| tx.value.to_sat() as i64).unwrap_or(0) ).sum(); @@ -8296,11 +8297,9 @@ impl FundedChannel where ))); } - let post_channel_value = PendingSplice::compute_post_value(pre_channel_value, their_funding_contribution_satoshis, our_funding_contribution_satoshis); - let post_balance = PendingSplice::add_checked(self.context.value_to_self_msat, our_funding_contribution_satoshis); - // Early check for reserve requirement, assuming maximum balance of full channel value - // This will also be checked later at tx_complete - let _res = self.context.check_balance_meets_v2_reserve_requirements(post_balance, post_channel_value)?; + // Note on channel reserve requirement pre-check: as the splice acceptor does not contribute, + // it can go below reserve, therefore no pre-check is done here. + // TODO(splicing): Once splice acceptor can contribute, add reserve pre-check, similar to the one in `splice_ack`. // TODO(splicing): Store msg.funding_pubkey // TODO(splicing): Apply start of splice (splice_start) @@ -8327,7 +8326,7 @@ impl FundedChannel where let pre_channel_value = self.context.get_value_satoshis(); let post_channel_value = PendingSplice::compute_post_value(pre_channel_value, our_funding_contribution, their_funding_contribution_satoshis); let post_balance = PendingSplice::add_checked(self.context.value_to_self_msat, our_funding_contribution); - // Early check for reserve requirement, assuming maximum balance of full channel value + // Pre-check for reserve requirement, assuming maximum balance of full channel value // This will also be checked later at tx_complete let _res = self.context.check_balance_meets_v2_reserve_requirements(post_balance, post_channel_value)?; Ok(()) From 9c6663cc8f4417efd41fb1277a0633aca26e15b9 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Thu, 23 Jan 2025 10:53:25 +0100 Subject: [PATCH 06/15] Add check for shutdown-received --- lightning/src/ln/channel.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e2a783eeac9..d463774596d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8278,6 +8278,13 @@ impl FundedChannel where return Err(ChannelError::Warn(format!("Splicing requested on a channel that is not Ready"))); } + // - If it has received shutdown: + // MUST send a warning and close the connection or send an error + // and fail the channel. + if self.context.channel_state.is_remote_shutdown_sent() { + return Err(ChannelError::close("Got splice_init when channel was not in an operational state".to_owned())); + } + let pre_channel_value = self.context.get_value_satoshis(); // Sanity check: capacity cannot decrease below 0 if (pre_channel_value as i64) From d07a2890db9ab08d20cbe398a41929ca85f1737c Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Fri, 24 Jan 2025 22:54:29 +0100 Subject: [PATCH 07/15] Add check for sufficient contributions when starting splicing --- lightning/src/ln/channel.rs | 70 +++++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 22 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index d463774596d..368e7b1729b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -54,8 +54,9 @@ use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBounde use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::chain::transaction::{OutPoint, TransactionData}; use crate::sign::ecdsa::EcdsaChannelSigner; -use crate::sign::{EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient}; +use crate::sign::{EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient, P2WPKH_WITNESS_WEIGHT}; use crate::events::{ClosureReason, Event}; +use crate::events::bump_transaction::BASE_INPUT_WEIGHT; use crate::routing::gossip::NodeId; use crate::util::ser::{Readable, ReadableArgs, TransactionU16LenLimited, Writeable, Writer}; use crate::util::logger::{Logger, Record, WithContext}; @@ -4552,15 +4553,44 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis)) } +/// Estimate our part of the fee of the new funding transaction. +/// input_count: Number of contributed inputs. +/// witness_weight: The witness weight for contributed inputs. +#[allow(dead_code)] // TODO(dual_funding): Remove once V2 channels is enabled. +fn estimate_funding_transaction_fee( + is_initiator: bool, input_count: usize, witness_weight: Weight, + funding_feerate_sat_per_1000_weight: u32, +) -> u64 { + // Inputs + let mut weight = (input_count as u64) * BASE_INPUT_WEIGHT; + + // Witnesses + weight = weight.saturating_add(witness_weight.to_wu()); + + // If we are the initiator, we must pay for weight of all common fields in the funding transaction. + if is_initiator { + weight = weight + .saturating_add(TX_COMMON_FIELDS_WEIGHT) + // The weight of the funding output, a P2WSH output + // NOTE: The witness script hash given here is irrelevant as it's a fixed size and we just want + // to calculate the contributed weight, so we use an all-zero hash. + .saturating_add(get_output_weight(&ScriptBuf::new_p2wsh( + &WScriptHash::from_raw_hash(Hash::all_zeros()) + )).to_wu()) + } + + fee_for_weight(funding_feerate_sat_per_1000_weight, weight) +} + #[allow(dead_code)] // TODO(dual_funding): Remove once V2 channels is enabled. pub(super) fn calculate_our_funding_satoshis( is_initiator: bool, funding_inputs: &[(TxIn, TransactionU16LenLimited)], total_witness_weight: Weight, funding_feerate_sat_per_1000_weight: u32, holder_dust_limit_satoshis: u64, ) -> Result { - let mut total_input_satoshis = 0u64; - let mut our_contributed_weight = 0u64; + let estimated_fee = estimate_funding_transaction_fee(is_initiator, funding_inputs.len(), total_witness_weight, funding_feerate_sat_per_1000_weight); + let mut total_input_satoshis = 0u64; for (idx, input) in funding_inputs.iter().enumerate() { if let Some(output) = input.1.as_transaction().output.get(input.0.previous_output.vout as usize) { total_input_satoshis = total_input_satoshis.saturating_add(output.value.to_sat()); @@ -4570,23 +4600,8 @@ pub(super) fn calculate_our_funding_satoshis( input.1.as_transaction().compute_txid(), input.0.previous_output.vout, idx) }); } } - our_contributed_weight = our_contributed_weight.saturating_add(total_witness_weight.to_wu()); - // If we are the initiator, we must pay for weight of all common fields in the funding transaction. - if is_initiator { - our_contributed_weight = our_contributed_weight - .saturating_add(TX_COMMON_FIELDS_WEIGHT) - // The weight of a P2WSH output to be added later. - // - // NOTE: The witness script hash given here is irrelevant as it's a fixed size and we just want - // to calculate the contributed weight, so we use an all-zero hash. - .saturating_add(get_output_weight(&ScriptBuf::new_p2wsh( - &WScriptHash::from_raw_hash(Hash::all_zeros()) - )).to_wu()) - } - - let funding_satoshis = total_input_satoshis - .saturating_sub(fee_for_weight(funding_feerate_sat_per_1000_weight, our_contributed_weight)); + let funding_satoshis = total_input_satoshis.saturating_sub(estimated_fee); if funding_satoshis < holder_dust_limit_satoshis { Ok(0) } else { @@ -8241,10 +8256,21 @@ impl FundedChannel where // Pre-check that inputs are sufficient to cover our contribution. // Note: fees are not taken into account here. - let sum_input: i64 = our_funding_inputs.into_iter().map( - |(txin, tx)| tx.output.get(txin.previous_output.vout as usize).map(|tx| tx.value.to_sat() as i64).unwrap_or(0) + let sum_input: u64 = our_funding_inputs.iter().map( + |(txin, tx)| tx.output.get(txin.previous_output.vout as usize).map(|tx| tx.value.to_sat()).unwrap_or(0) ).sum(); - if sum_input < our_funding_contribution_satoshis { + + // The +1 is to include the input of the old funding + let funding_input_count = our_funding_inputs.len() + 1; + // Add weight for inputs (estimated as P2WPKH) *and* spending old funding + let total_witness_weight = Weight::from_wu( + our_funding_inputs.len() as u64 * P2WPKH_WITNESS_WEIGHT + + 2 * P2WPKH_WITNESS_WEIGHT + ); + let estimated_fee = estimate_funding_transaction_fee(true, funding_input_count, total_witness_weight, funding_feerate_per_kw); + let available_input = sum_input.saturating_sub(estimated_fee); + + if (available_input as i64) < our_funding_contribution_satoshis { return Err(ChannelError::Warn(format!( "Provided inputs are insufficient for our contribution, {} {}", sum_input, our_funding_contribution_satoshis, From 776183e6e1d677ebb43a9231b66dfed91bfcb4cd Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Sat, 25 Jan 2025 00:00:59 +0100 Subject: [PATCH 08/15] Include input weight in splice_channel; test util to return weight as well --- lightning/src/ln/channel.rs | 16 ++++++++++------ lightning/src/ln/channelmanager.rs | 6 ++++-- lightning/src/ln/dual_funding_tests.rs | 4 ++-- lightning/src/ln/functional_test_utils.rs | 15 ++++++++++----- lightning/src/ln/functional_tests_splice.rs | 3 ++- 5 files changed, 28 insertions(+), 16 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 368e7b1729b..68a3cd3cb02 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -54,7 +54,9 @@ use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBounde use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::chain::transaction::{OutPoint, TransactionData}; use crate::sign::ecdsa::EcdsaChannelSigner; -use crate::sign::{EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient, P2WPKH_WITNESS_WEIGHT}; +use crate::sign::{EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient}; +#[cfg(splicing)] +use crate::sign::P2WPKH_WITNESS_WEIGHT; use crate::events::{ClosureReason, Event}; use crate::events::bump_transaction::BASE_INPUT_WEIGHT; use crate::routing::gossip::NodeId; @@ -8216,10 +8218,12 @@ impl FundedChannel where } } - /// Initiate splicing + /// Initiate splicing. + /// - witness_weight: The witness weight for contributed inputs. #[cfg(splicing)] pub fn splice_channel(&mut self, our_funding_contribution_satoshis: i64, - our_funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_per_kw: u32, locktime: u32, + our_funding_inputs: Vec<(TxIn, Transaction)>, witness_weight: Weight, + funding_feerate_per_kw: u32, locktime: u32, ) -> Result { // Check if a splice has been initiated already. // Note: only a single outstanding splice is supported (per spec) @@ -8262,10 +8266,10 @@ impl FundedChannel where // The +1 is to include the input of the old funding let funding_input_count = our_funding_inputs.len() + 1; - // Add weight for inputs (estimated as P2WPKH) *and* spending old funding + // Input witness weight, extended with weight for spending old funding let total_witness_weight = Weight::from_wu( - our_funding_inputs.len() as u64 * P2WPKH_WITNESS_WEIGHT + - 2 * P2WPKH_WITNESS_WEIGHT + witness_weight.to_wu() + .saturating_add(2 * P2WPKH_WITNESS_WEIGHT) ); let estimated_fee = estimate_funding_transaction_fee(true, funding_input_count, total_witness_weight, funding_feerate_per_kw); let available_input = sum_input.saturating_sub(estimated_fee); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 88bcd40859e..d0922e06fb5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4238,10 +4238,12 @@ where /// Note: Currently only splice-in is supported (increase in channel capacity), splice-out is not. /// - our_funding_contribution_satoshis: the amount contributed by us to the channel. This will increase our channel balance. /// - our_funding_inputs: the funding inputs provided by us. If our contribution is positive, our funding inputs must cover at least that amount. + /// - witness_weight: The witness weight for contributed inputs. #[cfg(splicing)] pub fn splice_channel( &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, our_funding_contribution_satoshis: i64, - our_funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_per_kw: u32, locktime: u32, + our_funding_inputs: Vec<(TxIn, Transaction)>, witness_weight: Weight, + funding_feerate_per_kw: u32, locktime: u32, ) -> Result<(), APIError> { let per_peer_state = self.per_peer_state.read().unwrap(); @@ -4255,7 +4257,7 @@ where match peer_state.channel_by_id.entry(*channel_id) { hash_map::Entry::Occupied(mut chan_phase_entry) => { if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() { - let msg = chan.splice_channel(our_funding_contribution_satoshis, our_funding_inputs, funding_feerate_per_kw, locktime) + let msg = chan.splice_channel(our_funding_contribution_satoshis, our_funding_inputs, witness_weight, funding_feerate_per_kw, locktime) .map_err(|err| APIError::APIMisuseError { err: format!( "Cannot initiate Splicing, {}, channel ID {}", err, channel_id diff --git a/lightning/src/ln/dual_funding_tests.rs b/lightning/src/ln/dual_funding_tests.rs index 8cd47ad1765..ee5b5ea080f 100644 --- a/lightning/src/ln/dual_funding_tests.rs +++ b/lightning/src/ln/dual_funding_tests.rs @@ -51,7 +51,7 @@ fn do_test_v2_channel_establishment( let logger_a = test_utils::TestLogger::with_id("node a".to_owned()); // Create a funding input for the new channel along with its previous transaction. - let initiator_funding_inputs: Vec<_> = create_dual_funding_utxos_with_prev_txs( + let (initiator_funding_inputs, total_weight) = create_dual_funding_utxos_with_prev_txs( &nodes[0], &[session.initiator_input_value_satoshis], ) @@ -66,7 +66,7 @@ fn do_test_v2_channel_establishment( let funding_satoshis = calculate_our_funding_satoshis( true, &initiator_funding_inputs[..], - Weight::from_wu(P2WPKH_WITNESS_WEIGHT), + total_weight, funding_feerate, MIN_CHAN_DUST_LIMIT_SATOSHIS, ) diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index f8c5910e637..c5844c931fd 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -38,7 +38,7 @@ use crate::util::test_utils; use crate::util::test_utils::{TestChainMonitor, TestScorer, TestKeysInterface}; use crate::util::ser::{ReadableArgs, Writeable}; -use bitcoin::WPubkeyHash; +use bitcoin::{Weight, WPubkeyHash}; use bitcoin::amount::Amount; use bitcoin::block::{Block, Header, Version as BlockVersion}; use bitcoin::locktime::absolute::{LockTime, LOCK_TIME_THRESHOLD}; @@ -60,6 +60,7 @@ use core::mem; use core::ops::Deref; use crate::io; use crate::prelude::*; +use crate::sign::P2WPKH_WITNESS_WEIGHT; use crate::sync::{Arc, Mutex, LockTestExt, RwLock}; pub const CHAN_CONFIRM_DEPTH: u32 = 10; @@ -1245,9 +1246,11 @@ fn internal_create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, } } +/// Create test inputs for a funding transaction. +/// Return the inputs (with prev tx), and the total witness weight for these inputs pub fn create_dual_funding_utxos_with_prev_txs( node: &Node<'_, '_, '_>, utxo_values_in_satoshis: &[u64], -) -> Vec<(TxIn, Transaction)> { +) -> (Vec<(TxIn, Transaction)>, Weight) { // Ensure we have unique transactions per node by using the locktime. let tx = Transaction { version: TxVersion::TWO, @@ -1260,9 +1263,9 @@ pub fn create_dual_funding_utxos_with_prev_txs( }).collect() }; - let mut result = vec![]; + let mut inputs = vec![]; for i in 0..utxo_values_in_satoshis.len() { - result.push( + inputs.push( (TxIn { previous_output: OutPoint { txid: tx.compute_txid(), @@ -1273,7 +1276,9 @@ pub fn create_dual_funding_utxos_with_prev_txs( witness: Witness::new(), }, tx.clone())); } - result + let total_weight = Weight::from_wu(utxo_values_in_satoshis.len() as u64 * P2WPKH_WITNESS_WEIGHT); + + (inputs, total_weight) } pub fn sign_funding_transaction<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, expected_temporary_channel_id: ChannelId) -> Transaction { diff --git a/lightning/src/ln/functional_tests_splice.rs b/lightning/src/ln/functional_tests_splice.rs index 6c9fa111454..07fb125aade 100644 --- a/lightning/src/ln/functional_tests_splice.rs +++ b/lightning/src/ln/functional_tests_splice.rs @@ -231,7 +231,7 @@ fn test_v1_splice_in() { // Create additional inputs let extra_splice_funding_input_sats = 35_000; - let funding_inputs = create_dual_funding_utxos_with_prev_txs( + let (funding_inputs, total_weight) = create_dual_funding_utxos_with_prev_txs( &initiator_node, &[extra_splice_funding_input_sats], ); @@ -243,6 +243,7 @@ fn test_v1_splice_in() { &acceptor_node.node.get_our_node_id(), splice_in_sats as i64, funding_inputs, + total_weight, funding_feerate_per_kw, locktime, ) From 09237f75eb0e48cf9a26bc7a631bd7b9d672f8c9 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Thu, 30 Jan 2025 23:08:55 +0100 Subject: [PATCH 09/15] Fix weight computation for funding input (multisig) --- lightning/src/ln/chan_utils.rs | 21 +++++++++++++++++++++ lightning/src/ln/channel.rs | 7 +++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index ae76308f0ce..f959f5e6541 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -81,6 +81,27 @@ pub const HTLC_TIMEOUT_INPUT_ANCHOR_WITNESS_WEIGHT: u64 = 288; /// outputs. pub const HTLC_SUCCESS_INPUT_ANCHOR_WITNESS_WEIGHT: u64 = 327; +/// The size of the 2-of-2 multisig script +const MULTISIG_SCRIPT_SIZE: u64 = + 1 + // OP_2 + 1 + // data len + 33 + // pubkey1 + 1 + // data len + 33 + // pubkey2 + 1 + // OP_2 + 1; // OP_CHECKMULTISIG +/// The weight of a funding transaction input (2-of-2 P2WSH) +/// See https://github.com/lightning/bolts/blob/master/03-transactions.md#expected-weight-of-the-commitment-transaction +pub const FUNDING_TRANSACTION_WITNESS_WEIGHT: u64 = + 1 + // number_of_witness_elements + 1 + // nil_len + 1 + // sig len + 73 + // sig1 + 1 + // sig len + 73 + // sig2 + 1 + // witness_script_length + MULTISIG_SCRIPT_SIZE; + /// Gets the weight for an HTLC-Success transaction. #[inline] pub fn htlc_success_tx_weight(channel_type_features: &ChannelTypeFeatures) -> u64 { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 68a3cd3cb02..afb8fcf0e82 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -47,6 +47,8 @@ use crate::ln::chan_utils::{ get_commitment_transaction_number_obscure_factor, ClosingTransaction, commit_tx_fee_sat, per_outbound_htlc_counterparty_commit_tx_fee_msat, }; +#[cfg(splicing)] +use crate::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT; use crate::ln::chan_utils; use crate::ln::onion_utils::HTLCFailReason; use crate::chain::BestBlock; @@ -55,8 +57,6 @@ use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, Channel use crate::chain::transaction::{OutPoint, TransactionData}; use crate::sign::ecdsa::EcdsaChannelSigner; use crate::sign::{EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient}; -#[cfg(splicing)] -use crate::sign::P2WPKH_WITNESS_WEIGHT; use crate::events::{ClosureReason, Event}; use crate::events::bump_transaction::BASE_INPUT_WEIGHT; use crate::routing::gossip::NodeId; @@ -8268,8 +8268,7 @@ impl FundedChannel where let funding_input_count = our_funding_inputs.len() + 1; // Input witness weight, extended with weight for spending old funding let total_witness_weight = Weight::from_wu( - witness_weight.to_wu() - .saturating_add(2 * P2WPKH_WITNESS_WEIGHT) + witness_weight.to_wu().saturating_add(FUNDING_TRANSACTION_WITNESS_WEIGHT) ); let estimated_fee = estimate_funding_transaction_fee(true, funding_input_count, total_witness_weight, funding_feerate_per_kw); let available_input = sum_input.saturating_sub(estimated_fee); From 49da8758cee39537fa8d162cabb8a9779615e586 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Sat, 1 Feb 2025 06:20:54 +0100 Subject: [PATCH 10/15] Include input weights together with inputs (not separate param) --- lightning/src/ln/channel.rs | 54 ++++++++++++++++++--- lightning/src/ln/channelmanager.rs | 6 +-- lightning/src/ln/dual_funding_tests.rs | 15 +++--- lightning/src/ln/functional_test_utils.rs | 10 ++-- lightning/src/ln/functional_tests_splice.rs | 3 +- 5 files changed, 66 insertions(+), 22 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index afb8fcf0e82..a769fe9f733 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4559,7 +4559,7 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos /// input_count: Number of contributed inputs. /// witness_weight: The witness weight for contributed inputs. #[allow(dead_code)] // TODO(dual_funding): Remove once V2 channels is enabled. -fn estimate_funding_transaction_fee( +fn estimate_v2_funding_transaction_fee( is_initiator: bool, input_count: usize, witness_weight: Weight, funding_feerate_sat_per_1000_weight: u32, ) -> u64 { @@ -4590,7 +4590,7 @@ pub(super) fn calculate_our_funding_satoshis( total_witness_weight: Weight, funding_feerate_sat_per_1000_weight: u32, holder_dust_limit_satoshis: u64, ) -> Result { - let estimated_fee = estimate_funding_transaction_fee(is_initiator, funding_inputs.len(), total_witness_weight, funding_feerate_sat_per_1000_weight); + let estimated_fee = estimate_v2_funding_transaction_fee(is_initiator, funding_inputs.len(), total_witness_weight, funding_feerate_sat_per_1000_weight); let mut total_input_satoshis = 0u64; for (idx, input) in funding_inputs.iter().enumerate() { @@ -8219,10 +8219,11 @@ impl FundedChannel where } /// Initiate splicing. - /// - witness_weight: The witness weight for contributed inputs. + /// - our_funding_inputs: the inputs we contribute to the new funding transaction. + /// Includes the witness weight for this input (e.g. P2WPKH_WITNESS_WEIGHT=109 for typical P2WPKH inputs). #[cfg(splicing)] pub fn splice_channel(&mut self, our_funding_contribution_satoshis: i64, - our_funding_inputs: Vec<(TxIn, Transaction)>, witness_weight: Weight, + our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, funding_feerate_per_kw: u32, locktime: u32, ) -> Result { // Check if a splice has been initiated already. @@ -8261,16 +8262,17 @@ impl FundedChannel where // Pre-check that inputs are sufficient to cover our contribution. // Note: fees are not taken into account here. let sum_input: u64 = our_funding_inputs.iter().map( - |(txin, tx)| tx.output.get(txin.previous_output.vout as usize).map(|tx| tx.value.to_sat()).unwrap_or(0) + |(txin, tx, _)| tx.output.get(txin.previous_output.vout as usize).map(|tx| tx.value.to_sat()).unwrap_or(0) ).sum(); // The +1 is to include the input of the old funding let funding_input_count = our_funding_inputs.len() + 1; // Input witness weight, extended with weight for spending old funding let total_witness_weight = Weight::from_wu( - witness_weight.to_wu().saturating_add(FUNDING_TRANSACTION_WITNESS_WEIGHT) + our_funding_inputs.iter().map(|(_, _, w)| w.to_wu()).sum::() + .saturating_add(FUNDING_TRANSACTION_WITNESS_WEIGHT) ); - let estimated_fee = estimate_funding_transaction_fee(true, funding_input_count, total_witness_weight, funding_feerate_per_kw); + let estimated_fee = estimate_v2_funding_transaction_fee(true, funding_input_count, total_witness_weight, funding_feerate_per_kw); let available_input = sum_input.saturating_sub(estimated_fee); if (available_input as i64) < our_funding_contribution_satoshis { @@ -9504,6 +9506,7 @@ impl PendingV2Channel where SP::Target: SignerProvider { /// Creates a new dual-funded channel from a remote side's request for one. /// Assumes chain_hash has already been checked and corresponds with what we expect! + /// TODO(dual_funding): Include witness weight info with the inputs. #[allow(dead_code)] // TODO(dual_funding): Remove once V2 channels is enabled. pub fn new_inbound( fee_estimator: &LowerBoundedFeeEstimator, entropy_source: &ES, signer_provider: &SP, @@ -12496,6 +12499,43 @@ mod tests { assert!(node_a_chan.check_get_channel_ready(0, &&logger).is_some()); } + + #[test] + fn test_estimate_v2_funding_transaction_fee() { + use crate::ln::channel::estimate_v2_funding_transaction_fee; + use bitcoin::Weight; + + // 2 inputs with weight 300, initiator, 2000 sat/kw feerate + assert_eq!( + estimate_v2_funding_transaction_fee(true, 2, Weight::from_wu(300), 2000), + 1668 + ); + + // higher feerate + assert_eq!( + estimate_v2_funding_transaction_fee(true, 2, Weight::from_wu(300), 3000), + 2502 + ); + + // only 1 input + assert_eq!( + estimate_v2_funding_transaction_fee(true, 1, Weight::from_wu(300), 2000), + 1348 + ); + + // 0 input weight + assert_eq!( + estimate_v2_funding_transaction_fee(true, 1, Weight::from_wu(0), 2000), + 748 + ); + + // not initiator + assert_eq!( + estimate_v2_funding_transaction_fee(false, 1, Weight::from_wu(0), 2000), + 320 + ); + } + #[cfg(all(test, splicing))] fn get_pre_and_post(pre_channel_value: u64, our_funding_contribution: i64, their_funding_contribution: i64) -> (u64, u64) { use crate::ln::channel::PendingSplice; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d0922e06fb5..1dbe1232d58 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4238,11 +4238,11 @@ where /// Note: Currently only splice-in is supported (increase in channel capacity), splice-out is not. /// - our_funding_contribution_satoshis: the amount contributed by us to the channel. This will increase our channel balance. /// - our_funding_inputs: the funding inputs provided by us. If our contribution is positive, our funding inputs must cover at least that amount. - /// - witness_weight: The witness weight for contributed inputs. + /// Includes the witness weight for this input (e.g. P2WPKH_WITNESS_WEIGHT=109 for typical P2WPKH inputs). #[cfg(splicing)] pub fn splice_channel( &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, our_funding_contribution_satoshis: i64, - our_funding_inputs: Vec<(TxIn, Transaction)>, witness_weight: Weight, + our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, funding_feerate_per_kw: u32, locktime: u32, ) -> Result<(), APIError> { let per_peer_state = self.per_peer_state.read().unwrap(); @@ -4257,7 +4257,7 @@ where match peer_state.channel_by_id.entry(*channel_id) { hash_map::Entry::Occupied(mut chan_phase_entry) => { if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() { - let msg = chan.splice_channel(our_funding_contribution_satoshis, our_funding_inputs, witness_weight, funding_feerate_per_kw, locktime) + let msg = chan.splice_channel(our_funding_contribution_satoshis, our_funding_inputs, funding_feerate_per_kw, locktime) .map_err(|err| APIError::APIMisuseError { err: format!( "Cannot initiate Splicing, {}, channel ID {}", err, channel_id diff --git a/lightning/src/ln/dual_funding_tests.rs b/lightning/src/ln/dual_funding_tests.rs index ee5b5ea080f..e675e10dbd7 100644 --- a/lightning/src/ln/dual_funding_tests.rs +++ b/lightning/src/ln/dual_funding_tests.rs @@ -26,7 +26,7 @@ use { crate::ln::msgs::{CommitmentSigned, TxAddInput, TxAddOutput, TxComplete}, crate::ln::types::ChannelId, crate::prelude::*, - crate::sign::{ChannelSigner as _, P2WPKH_WITNESS_WEIGHT}, + crate::sign::ChannelSigner as _, crate::util::ser::TransactionU16LenLimited, crate::util::test_utils, bitcoin::Weight, @@ -51,13 +51,16 @@ fn do_test_v2_channel_establishment( let logger_a = test_utils::TestLogger::with_id("node a".to_owned()); // Create a funding input for the new channel along with its previous transaction. - let (initiator_funding_inputs, total_weight) = create_dual_funding_utxos_with_prev_txs( + let initiator_funding_inputs = create_dual_funding_utxos_with_prev_txs( &nodes[0], &[session.initiator_input_value_satoshis], - ) - .into_iter() - .map(|(txin, tx)| (txin, TransactionU16LenLimited::new(tx).unwrap())) - .collect(); + ); + let total_weight = + Weight::from_wu(initiator_funding_inputs.iter().map(|(_, _, w)| w.to_wu()).sum()); + let initiator_funding_inputs: Vec<_> = initiator_funding_inputs + .into_iter() + .map(|(txin, tx, _)| (txin, TransactionU16LenLimited::new(tx).unwrap())) + .collect(); // Alice creates a dual-funded channel as initiator. let funding_feerate = node_cfgs[0] diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index c5844c931fd..58ff349f75d 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1250,7 +1250,7 @@ fn internal_create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, /// Return the inputs (with prev tx), and the total witness weight for these inputs pub fn create_dual_funding_utxos_with_prev_txs( node: &Node<'_, '_, '_>, utxo_values_in_satoshis: &[u64], -) -> (Vec<(TxIn, Transaction)>, Weight) { +) -> Vec<(TxIn, Transaction, Weight)> { // Ensure we have unique transactions per node by using the locktime. let tx = Transaction { version: TxVersion::TWO, @@ -1274,11 +1274,13 @@ pub fn create_dual_funding_utxos_with_prev_txs( script_sig: ScriptBuf::new(), sequence: Sequence::ZERO, witness: Witness::new(), - }, tx.clone())); + }, + tx.clone(), + Weight::from_wu(P2WPKH_WITNESS_WEIGHT), + )); } - let total_weight = Weight::from_wu(utxo_values_in_satoshis.len() as u64 * P2WPKH_WITNESS_WEIGHT); - (inputs, total_weight) + inputs } pub fn sign_funding_transaction<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, expected_temporary_channel_id: ChannelId) -> Transaction { diff --git a/lightning/src/ln/functional_tests_splice.rs b/lightning/src/ln/functional_tests_splice.rs index 07fb125aade..6c9fa111454 100644 --- a/lightning/src/ln/functional_tests_splice.rs +++ b/lightning/src/ln/functional_tests_splice.rs @@ -231,7 +231,7 @@ fn test_v1_splice_in() { // Create additional inputs let extra_splice_funding_input_sats = 35_000; - let (funding_inputs, total_weight) = create_dual_funding_utxos_with_prev_txs( + let funding_inputs = create_dual_funding_utxos_with_prev_txs( &initiator_node, &[extra_splice_funding_input_sats], ); @@ -243,7 +243,6 @@ fn test_v1_splice_in() { &acceptor_node.node.get_our_node_id(), splice_in_sats as i64, funding_inputs, - total_weight, funding_feerate_per_kw, locktime, ) From 84f659e04747ba8faf417efab4d4c3dcdc280135 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Sat, 1 Feb 2025 23:08:57 +0100 Subject: [PATCH 11/15] Add method check_v2_funding_inputs_sufficient() --- lightning/src/ln/channel.rs | 193 ++++++++++++++++++++++--- lightning/src/ln/dual_funding_tests.rs | 30 ++-- 2 files changed, 181 insertions(+), 42 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a769fe9f733..e01982dcdd5 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4611,6 +4611,54 @@ pub(super) fn calculate_our_funding_satoshis( } } +/// Verify that the provided inputs by a counterparty to the funding transaction are enough +/// to cover the intended contribution amount *plus* the proportional fees of the counterparty. +/// Fees are computed using `estimate_v2_funding_transaction_fee`, and contain +/// the fees of the inputs, fees of the inputs weight, and for the initiator, +/// the fees of the common fields as well as the output and extra input weights. +/// Returns estimated (partial) fees as additional information +pub(super) fn check_v2_funding_inputs_sufficient( + contribution_amount: i64, funding_inputs: &[(TxIn, Transaction, Weight)], is_initiator: bool, + extra_common_input_weight: Option, funding_feerate_sat_per_1000_weight: u32, +) -> Result { + let mut total_input_witness_weight = Weight::from_wu(funding_inputs.iter().map(|(_, _, w)| w.to_wu()).sum()); + if is_initiator { + if let Some(extra) = extra_common_input_weight { + total_input_witness_weight += extra; + } + } + let estimated_fee = estimate_v2_funding_transaction_fee(is_initiator, funding_inputs.len(), total_input_witness_weight, funding_feerate_sat_per_1000_weight); + + let mut total_input_sats = 0u64; + for (idx, input) in funding_inputs.iter().enumerate() { + if let Some(output) = input.1.output.get(input.0.previous_output.vout as usize) { + total_input_sats = total_input_sats.saturating_add(output.value.to_sat()); + } else { + return Err(ChannelError::Warn(format!( + "Transaction with txid {} does not have an output with vout of {} corresponding to TxIn at funding_inputs[{}]", + input.1.compute_txid(), input.0.previous_output.vout, idx + ))); + } + } + + // If the inputs are enough to cover intended contribution amount, with fees even when + // there is a change output, we are fine. + // If the inputs are less, but enough to cover intended contribution amount, with + // (lower) fees with no change, we are also fine (change will not be generated). + // So it's enough to check considering the lower, no-change fees. + // Note: dust limit is not relevant in this check. + + let minimal_input_amount_needed = contribution_amount.saturating_add(estimated_fee as i64); + if (total_input_sats as i64) < minimal_input_amount_needed { + Err(ChannelError::Warn(format!( + "Total input amount {} is lower than needed for contribution {}, considering fees of {}. Need more inputs.", + total_input_sats, contribution_amount, estimated_fee, + ))) + } else { + Ok(estimated_fee) + } +} + /// Context for dual-funded channels. pub(super) struct DualFundingChannelContext { /// The amount in satoshis we will be contributing to the channel. @@ -8259,28 +8307,10 @@ impl FundedChannel where // Note: post-splice channel value is not yet known at this point, counterparty contribution is not known // (Cannot test for miminum required post-splice channel value) - // Pre-check that inputs are sufficient to cover our contribution. - // Note: fees are not taken into account here. - let sum_input: u64 = our_funding_inputs.iter().map( - |(txin, tx, _)| tx.output.get(txin.previous_output.vout as usize).map(|tx| tx.value.to_sat()).unwrap_or(0) - ).sum(); - - // The +1 is to include the input of the old funding - let funding_input_count = our_funding_inputs.len() + 1; - // Input witness weight, extended with weight for spending old funding - let total_witness_weight = Weight::from_wu( - our_funding_inputs.iter().map(|(_, _, w)| w.to_wu()).sum::() - .saturating_add(FUNDING_TRANSACTION_WITNESS_WEIGHT) - ); - let estimated_fee = estimate_v2_funding_transaction_fee(true, funding_input_count, total_witness_weight, funding_feerate_per_kw); - let available_input = sum_input.saturating_sub(estimated_fee); - - if (available_input as i64) < our_funding_contribution_satoshis { - return Err(ChannelError::Warn(format!( - "Provided inputs are insufficient for our contribution, {} {}", - sum_input, our_funding_contribution_satoshis, - ))); - } + // Check that inputs are sufficient to cover our contribution. + // Extra common weight is the weight for spending the old funding + let extra_input_weight = Weight::from_wu(FUNDING_TRANSACTION_WITNESS_WEIGHT); + let _fee = check_v2_funding_inputs_sufficient(our_funding_contribution_satoshis, &our_funding_inputs, true, Some(extra_input_weight), funding_feerate_per_kw)?; self.pending_splice_pre = Some(PendingSplice { our_funding_contribution: our_funding_contribution_satoshis, @@ -10727,8 +10757,12 @@ mod tests { use bitcoin::constants::ChainHash; use bitcoin::script::{ScriptBuf, Builder}; use bitcoin::transaction::{Transaction, TxOut, Version}; + #[cfg(splicing)] + use bitcoin::transaction::TxIn; use bitcoin::opcodes; use bitcoin::network::Network; + #[cfg(splicing)] + use bitcoin::Weight; use crate::ln::onion_utils::INVALID_ONION_BLINDING; use crate::types::payment::{PaymentHash, PaymentPreimage}; use crate::ln::channel_keys::{RevocationKey, RevocationBasepoint}; @@ -12536,7 +12570,118 @@ mod tests { ); } - #[cfg(all(test, splicing))] + #[cfg(splicing)] + fn funding_input_sats(input_value_sats: u64) -> (TxIn, Transaction, Weight) { + use crate::sign::P2WPKH_WITNESS_WEIGHT; + + let input_1_prev_out = TxOut { value: Amount::from_sat(input_value_sats), script_pubkey: ScriptBuf::default() }; + let input_1_prev_tx = Transaction { + input: vec![], output: vec![input_1_prev_out], + version: Version::TWO, lock_time: bitcoin::absolute::LockTime::ZERO, + }; + let input_1_txin = TxIn { + previous_output: bitcoin::OutPoint { txid: input_1_prev_tx.compute_txid(), vout: 0 }, + ..Default::default() + }; + (input_1_txin, input_1_prev_tx, Weight::from_wu(P2WPKH_WITNESS_WEIGHT)) + } + + #[cfg(splicing)] + #[test] + fn test_check_v2_funding_inputs_sufficient() { + use crate::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT; + use crate::ln::channel::check_v2_funding_inputs_sufficient; + use bitcoin::Weight; + + // positive case, inputs well over intended contribution + assert_eq!( + check_v2_funding_inputs_sufficient( + 220_000, + &[ + funding_input_sats(200_000), + funding_input_sats(100_000), + ], + true, + Some(Weight::from_wu(FUNDING_TRANSACTION_WITNESS_WEIGHT)), + 2000, + ).unwrap(), + 1948, + ); + + // negative case, inputs clearly insufficient + { + let res = check_v2_funding_inputs_sufficient( + 220_000, + &[ + funding_input_sats(100_000), + ], + true, + Some(Weight::from_wu(FUNDING_TRANSACTION_WITNESS_WEIGHT)), + 2000, + ); + assert_eq!( + format!("{:?}", res.err().unwrap()), + "Warn : Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1410. Need more inputs.", + ); + } + + // barely covers + { + let expected_fee: u64 = 1948; + assert_eq!( + check_v2_funding_inputs_sufficient( + (300_000 - expected_fee - 20) as i64, + &[ + funding_input_sats(200_000), + funding_input_sats(100_000), + ], + true, + Some(Weight::from_wu(FUNDING_TRANSACTION_WITNESS_WEIGHT)), + 2000, + ).unwrap(), + expected_fee, + ); + } + + // higher fee rate, does not cover + { + let expected_fee: u64 = 1948; + let res = check_v2_funding_inputs_sufficient( + 298032, + &[ + funding_input_sats(200_000), + funding_input_sats(100_000), + ], + true, + Some(Weight::from_wu(FUNDING_TRANSACTION_WITNESS_WEIGHT)), + 2200, + ); + assert_eq!( + format!("{:?}", res.err().unwrap()), + "Warn : Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2143. Need more inputs.", + ); + } + + // barely covers, less fees (no extra weight, no init) + { + let expected_fee: u64 = 1076; + assert_eq!( + check_v2_funding_inputs_sufficient( + (300_000 - expected_fee - 20) as i64, + &[ + funding_input_sats(200_000), + funding_input_sats(100_000), + ], + false, + None, + 2000, + ).unwrap(), + expected_fee, + ); + } + } + + #[cfg(splicing)] fn get_pre_and_post(pre_channel_value: u64, our_funding_contribution: i64, their_funding_contribution: i64) -> (u64, u64) { use crate::ln::channel::PendingSplice; @@ -12544,7 +12689,7 @@ mod tests { (pre_channel_value, post_channel_value) } - #[cfg(all(test, splicing))] + #[cfg(splicing)] #[test] fn test_splice_compute_post_value() { { diff --git a/lightning/src/ln/dual_funding_tests.rs b/lightning/src/ln/dual_funding_tests.rs index e675e10dbd7..aff17a67186 100644 --- a/lightning/src/ln/dual_funding_tests.rs +++ b/lightning/src/ln/dual_funding_tests.rs @@ -11,15 +11,13 @@ #[cfg(dual_funding)] use { - crate::chain::chaininterface::{ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator}, + crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator}, crate::events::{Event, MessageSendEvent, MessageSendEventsProvider}, crate::ln::chan_utils::{ make_funding_redeemscript, ChannelPublicKeys, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, }, - crate::ln::channel::{ - calculate_our_funding_satoshis, PendingV2Channel, MIN_CHAN_DUST_LIMIT_SATOSHIS, - }, + crate::ln::channel::PendingV2Channel, crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint, RevocationBasepoint}, crate::ln::functional_test_utils::*, crate::ln::msgs::ChannelMessageHandler, @@ -29,12 +27,12 @@ use { crate::sign::ChannelSigner as _, crate::util::ser::TransactionU16LenLimited, crate::util::test_utils, - bitcoin::Weight, }; #[cfg(dual_funding)] // Dual-funding: V2 Channel Establishment Tests struct V2ChannelEstablishmentTestSession { + funding_input_sats: u64, initiator_input_value_satoshis: u64, } @@ -63,17 +61,7 @@ fn do_test_v2_channel_establishment( .collect(); // Alice creates a dual-funded channel as initiator. - let funding_feerate = node_cfgs[0] - .fee_estimator - .get_est_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); - let funding_satoshis = calculate_our_funding_satoshis( - true, - &initiator_funding_inputs[..], - total_weight, - funding_feerate, - MIN_CHAN_DUST_LIMIT_SATOSHIS, - ) - .unwrap(); + let funding_satoshis = session.funding_input_sats; let mut channel = PendingV2Channel::new_outbound( &LowerBoundedFeeEstimator(node_cfgs[0].fee_estimator), &nodes[0].node.entropy_source, @@ -263,12 +251,18 @@ fn do_test_v2_channel_establishment( fn test_v2_channel_establishment() { // Only initiator contributes, no persist pending do_test_v2_channel_establishment( - V2ChannelEstablishmentTestSession { initiator_input_value_satoshis: 100_000 }, + V2ChannelEstablishmentTestSession { + funding_input_sats: 100_000, + initiator_input_value_satoshis: 150_000, + }, false, ); // Only initiator contributes, persist pending do_test_v2_channel_establishment( - V2ChannelEstablishmentTestSession { initiator_input_value_satoshis: 100_000 }, + V2ChannelEstablishmentTestSession { + funding_input_sats: 100_000, + initiator_input_value_satoshis: 150_000, + }, true, ); } From 454d7e9f64f07e0258d2f0e063cd7b1339782d11 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Sun, 2 Feb 2025 00:33:11 +0100 Subject: [PATCH 12/15] Remove superfluous channel-value-goes-below-0 checks --- lightning/src/ln/channel.rs | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e01982dcdd5..2905be4d1a7 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4617,6 +4617,7 @@ pub(super) fn calculate_our_funding_satoshis( /// the fees of the inputs, fees of the inputs weight, and for the initiator, /// the fees of the common fields as well as the output and extra input weights. /// Returns estimated (partial) fees as additional information +#[cfg(splicing)] pub(super) fn check_v2_funding_inputs_sufficient( contribution_amount: i64, funding_inputs: &[(TxIn, Transaction, Weight)], is_initiator: bool, extra_common_input_weight: Option, funding_feerate_sat_per_1000_weight: u32, @@ -8288,15 +8289,6 @@ impl FundedChannel where // TODO(splicing): check for quiescence - let pre_channel_value = self.context.get_value_satoshis(); - // Sanity check: capacity cannot decrease below 0 - if (pre_channel_value as i64).saturating_add(our_funding_contribution_satoshis) < 0 { - return Err(ChannelError::Warn(format!( - "Post-splicing channel value cannot be negative. It was {} + {}", - pre_channel_value, our_funding_contribution_satoshis - ))); - } - if our_funding_contribution_satoshis < 0 { return Err(ChannelError::Warn(format!( "TODO(splicing): Splice-out not supported, only splice in, contribution {}", @@ -8304,6 +8296,9 @@ impl FundedChannel where ))); } + // TODO(splicing): Once splice-out is supported, check that channel balance does not go below 0 + // (or below channel reserve) + // Note: post-splice channel value is not yet known at this point, counterparty contribution is not known // (Cannot test for miminum required post-splice channel value) @@ -8346,18 +8341,6 @@ impl FundedChannel where return Err(ChannelError::close("Got splice_init when channel was not in an operational state".to_owned())); } - let pre_channel_value = self.context.get_value_satoshis(); - // Sanity check: capacity cannot decrease below 0 - if (pre_channel_value as i64) - .saturating_add(their_funding_contribution_satoshis) - .saturating_add(our_funding_contribution_satoshis) < 0 - { - return Err(ChannelError::Warn(format!( - "Post-splicing channel value cannot be negative. It was {} + {} + {}", - pre_channel_value, their_funding_contribution_satoshis, our_funding_contribution_satoshis, - ))); - } - if their_funding_contribution_satoshis.saturating_add(our_funding_contribution_satoshis) < 0 { return Err(ChannelError::Warn(format!( "Splice-out not supported, only splice in, relative {} + {}", @@ -8365,8 +8348,11 @@ impl FundedChannel where ))); } + // 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 go below reserve, therefore no pre-check is done here. + // it can't go below reserve, therefore no pre-check is done here. // TODO(splicing): Once splice acceptor can contribute, add reserve pre-check, similar to the one in `splice_ack`. // TODO(splicing): Store msg.funding_pubkey @@ -12645,7 +12631,6 @@ mod tests { // higher fee rate, does not cover { - let expected_fee: u64 = 1948; let res = check_v2_funding_inputs_sufficient( 298032, &[ From 720a85ffc23bfd40bd68872fe3c5d446dd891e80 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Mon, 3 Feb 2025 18:15:43 +0100 Subject: [PATCH 13/15] Refined balance reserve check; add locking (review) --- lightning/src/ln/channel.rs | 46 ++++++++++++++++-------------- lightning/src/ln/channelmanager.rs | 1 + 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 2905be4d1a7..25369000384 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3973,29 +3973,30 @@ impl ChannelContext where SP::Target: SignerProvider { (context.holder_selected_channel_reserve_satoshis, context.counterparty_selected_channel_reserve_satoshis) } - /// Check that a balance value meets the channel reserve requirements or violates them (below reserve). + /// Check that balances meet the channel reserve requirements or violates them (below reserve). /// The channel value is an input as opposed to using from self, so that this can be used in case of splicing /// to checks with new channel value (before being comitted to it). #[cfg(splicing)] - pub fn check_balance_meets_v2_reserve_requirements(&self, balance: u64, channel_value: u64) -> Result<(), ChannelError> { - if balance == 0 { - return Ok(()); - } - let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( - channel_value, self.holder_dust_limit_satoshis); - if balance < holder_selected_channel_reserve_satoshis { - return Err(ChannelError::Warn(format!( - "Balance below reserve mandated by holder, {} vs {}", - balance, holder_selected_channel_reserve_satoshis, - ))); + pub fn check_balance_meets_v2_reserve_requirements(&self, self_balance: u64, counterparty_balance: u64, channel_value: u64) -> Result<(), ChannelError> { + if self_balance > 0 { + let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( + channel_value, self.holder_dust_limit_satoshis); + if self_balance < holder_selected_channel_reserve_satoshis { + return Err(ChannelError::Warn(format!( + "Balance below reserve mandated by holder, {} vs {}", + self_balance, holder_selected_channel_reserve_satoshis, + ))); + } } - let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( - channel_value, self.counterparty_dust_limit_satoshis); - if balance < counterparty_selected_channel_reserve_satoshis { - return Err(ChannelError::Warn(format!( - "Balance below reserve mandated by counterparty, {} vs {}", - balance, counterparty_selected_channel_reserve_satoshis, - ))); + if counterparty_balance > 0 { + let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( + channel_value, self.counterparty_dust_limit_satoshis); + if counterparty_balance < counterparty_selected_channel_reserve_satoshis { + return Err(ChannelError::Warn(format!( + "Balance below reserve mandated by counterparty, {} vs {}", + counterparty_balance, counterparty_selected_channel_reserve_satoshis, + ))); + } } Ok(()) } @@ -8379,10 +8380,11 @@ impl FundedChannel where let pre_channel_value = self.context.get_value_satoshis(); let post_channel_value = PendingSplice::compute_post_value(pre_channel_value, our_funding_contribution, their_funding_contribution_satoshis); - let post_balance = PendingSplice::add_checked(self.context.value_to_self_msat, our_funding_contribution); - // Pre-check for reserve requirement, assuming maximum balance of full channel value + let post_balance_self = PendingSplice::add_checked(self.context.value_to_self_msat, our_funding_contribution); + 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.context.check_balance_meets_v2_reserve_requirements(post_balance, post_channel_value)?; + let _res = self.context.check_balance_meets_v2_reserve_requirements(post_balance_self, post_balance_counterparty, post_channel_value)?; Ok(()) } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 1dbe1232d58..736db2a9f75 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4245,6 +4245,7 @@ where our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, funding_feerate_per_kw: u32, locktime: u32, ) -> Result<(), APIError> { + let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); let per_peer_state = self.per_peer_state.read().unwrap(); let peer_state_mutex = per_peer_state.get(counterparty_node_id) From 5d8519b2b9d7e199deb88b585a8e99d364d558bd Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Mon, 3 Feb 2025 18:22:01 +0100 Subject: [PATCH 14/15] Move the 2 get_splice_X() methods from Context to FundedChannel --- lightning/src/ln/channel.rs | 68 ++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 25369000384..fb61011f485 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4467,38 +4467,6 @@ impl ChannelContext where SP::Target: SignerProvider { self.channel_transaction_parameters = channel_transaction_parameters; self.get_initial_counterparty_commitment_signature(logger) } - - /// Get the splice message that can be sent during splice initiation. - #[cfg(splicing)] - pub fn get_splice_init(&self, our_funding_contribution_satoshis: i64, - funding_feerate_per_kw: u32, locktime: u32, - ) -> msgs::SpliceInit { - // TODO(splicing): The exisiting pubkey is reused, but a new one should be generated. See #3542. - // Note that channel_keys_id is supposed NOT to change - let funding_pubkey = self.get_holder_pubkeys().funding_pubkey.clone(); - msgs::SpliceInit { - channel_id: self.channel_id, - funding_contribution_satoshis: our_funding_contribution_satoshis, - funding_feerate_per_kw, - locktime, - funding_pubkey, - require_confirmed_inputs: None, - } - } - - /// Get the splice_ack message that can be sent in response to splice initiation. - #[cfg(splicing)] - pub fn get_splice_ack(&self, our_funding_contribution_satoshis: i64) -> msgs::SpliceAck { - // TODO(splicing): The exisiting pubkey is reused, but a new one should be generated. See #3542. - // Note that channel_keys_id is supposed NOT to change - let funding_pubkey = self.get_holder_pubkeys().funding_pubkey; - msgs::SpliceAck { - channel_id: self.channel_id, - funding_contribution_satoshis: our_funding_contribution_satoshis, - funding_pubkey, - require_confirmed_inputs: None, - } - } } // Internal utility functions for channels @@ -8312,10 +8280,28 @@ impl FundedChannel where our_funding_contribution: our_funding_contribution_satoshis, }); - let msg = self.context.get_splice_init(our_funding_contribution_satoshis, funding_feerate_per_kw, locktime); + let msg = self.get_splice_init(our_funding_contribution_satoshis, funding_feerate_per_kw, locktime); Ok(msg) } + /// Get the splice message that can be sent during splice initiation. + #[cfg(splicing)] + pub fn get_splice_init(&self, our_funding_contribution_satoshis: i64, + funding_feerate_per_kw: u32, locktime: u32, + ) -> msgs::SpliceInit { + // TODO(splicing): The exisiting pubkey is reused, but a new one should be generated. See #3542. + // Note that channel_keys_id is supposed NOT to change + let funding_pubkey = self.context.get_holder_pubkeys().funding_pubkey.clone(); + msgs::SpliceInit { + channel_id: self.context.channel_id, + funding_contribution_satoshis: our_funding_contribution_satoshis, + funding_feerate_per_kw, + locktime, + funding_pubkey, + require_confirmed_inputs: None, + } + } + /// Handle splice_init #[cfg(splicing)] pub fn splice_init(&mut self, msg: &msgs::SpliceInit) -> Result { @@ -8359,11 +8345,25 @@ impl FundedChannel where // TODO(splicing): Store msg.funding_pubkey // TODO(splicing): Apply start of splice (splice_start) - let splice_ack_msg = self.context.get_splice_ack(our_funding_contribution_satoshis); + let splice_ack_msg = self.get_splice_ack(our_funding_contribution_satoshis); // TODO(splicing): start interactive funding negotiation Ok(splice_ack_msg) } + /// Get the splice_ack message that can be sent in response to splice initiation. + #[cfg(splicing)] + pub fn get_splice_ack(&self, our_funding_contribution_satoshis: i64) -> msgs::SpliceAck { + // TODO(splicing): The exisiting pubkey is reused, but a new one should be generated. See #3542. + // Note that channel_keys_id is supposed NOT to change + let funding_pubkey = self.context.get_holder_pubkeys().funding_pubkey; + msgs::SpliceAck { + channel_id: self.context.channel_id, + funding_contribution_satoshis: our_funding_contribution_satoshis, + funding_pubkey, + require_confirmed_inputs: None, + } + } + /// Handle splice_ack #[cfg(splicing)] pub fn splice_ack(&mut self, msg: &msgs::SpliceAck) -> Result<(), ChannelError> { From b9f9022ea2c067ad14dab65a28350acd15eda486 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Mon, 3 Feb 2025 18:29:01 +0100 Subject: [PATCH 15/15] splice_channel: Make locktime optional --- lightning/src/ln/channelmanager.rs | 4 +++- lightning/src/ln/functional_tests_splice.rs | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 736db2a9f75..3ad4ef431e2 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4239,11 +4239,12 @@ where /// - our_funding_contribution_satoshis: the amount contributed by us to the channel. This will increase our channel balance. /// - our_funding_inputs: the funding inputs provided by us. If our contribution is positive, our funding inputs must cover at least that amount. /// Includes the witness weight for this input (e.g. P2WPKH_WITNESS_WEIGHT=109 for typical P2WPKH inputs). + /// - locktime: Optional locktime for the new funding transaction. If None, set to the current block height. #[cfg(splicing)] pub fn splice_channel( &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, our_funding_contribution_satoshis: i64, our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, - funding_feerate_per_kw: u32, locktime: u32, + funding_feerate_per_kw: u32, locktime: Option, ) -> Result<(), APIError> { let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); let per_peer_state = self.per_peer_state.read().unwrap(); @@ -4257,6 +4258,7 @@ where // Look for the channel match peer_state.channel_by_id.entry(*channel_id) { hash_map::Entry::Occupied(mut chan_phase_entry) => { + let locktime = locktime.unwrap_or(self.current_best_block().height); if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() { let msg = chan.splice_channel(our_funding_contribution_satoshis, our_funding_inputs, funding_feerate_per_kw, locktime) .map_err(|err| APIError::APIMisuseError { diff --git a/lightning/src/ln/functional_tests_splice.rs b/lightning/src/ln/functional_tests_splice.rs index 6c9fa111454..4d386f1e3c1 100644 --- a/lightning/src/ln/functional_tests_splice.rs +++ b/lightning/src/ln/functional_tests_splice.rs @@ -227,7 +227,6 @@ fn test_v1_splice_in() { // Amount being added to the channel through the splice-in let splice_in_sats: u64 = 20000; let funding_feerate_per_kw = 1024; // TODO - let locktime = 0; // TODO // Create additional inputs let extra_splice_funding_input_sats = 35_000; @@ -244,7 +243,7 @@ fn test_v1_splice_in() { splice_in_sats as i64, funding_inputs, funding_feerate_per_kw, - locktime, + None, // locktime ) .unwrap(); // Extract the splice message from node0 to node1