From 562a1118a218a9e2dc938c52f2211b9a29c9b9fe Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Wed, 13 Nov 2024 22:05:20 +0100 Subject: [PATCH] Review comments: reserve check, etc --- lightning/src/ln/channel.rs | 44 +++++++++++++++++++----------- lightning/src/ln/channelmanager.rs | 29 ++++++++++++-------- 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 2bee0a2ea1d..5590e1241ed 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -23,8 +23,6 @@ use bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE; use bitcoin::secp256k1::{PublicKey,SecretKey}; use bitcoin::secp256k1::{Secp256k1,ecdsa::Signature}; use bitcoin::{secp256k1, sighash}; -#[cfg(splicing)] -use bitcoin::TxIn; use crate::ln::types::ChannelId; use crate::types::payment::{PaymentPreimage, PaymentHash}; @@ -1185,11 +1183,12 @@ impl UnfundedChannelContext { #[derive(Clone)] pub(crate) struct PendingSpliceInfoPre { pub our_funding_contribution: i64, - pub funding_feerate_perkw: u32, - pub locktime: u32, - /// The funding inputs we will be contributing to the splice. - /// TODO(splice): will be changed to TransactionU16LenLimited - pub our_funding_inputs: Vec<(TxIn, Transaction)>, + // TODO(splicing): Enable below fields + // pub funding_feerate_perkw: u32, + // pub locktime: u32, + // /// The funding inputs we will be contributing to the splice. + // /// TODO(splice): will be changed to TransactionU16LenLimited + // pub our_funding_inputs: Vec<(TxIn, Transaction)>, } #[cfg(splicing)] @@ -3425,6 +3424,27 @@ impl ChannelContext where SP::Target: SignerProvider { (context.holder_selected_channel_reserve_satoshis, context.counterparty_selected_channel_reserve_satoshis) } + /// Check that a proposed channel value meets the channel reserve requirements or violates them (below reserve) + pub fn check_channel_value_meets_reserve_requirements(&self, proposed_channel_value: u64) -> Result<(), ChannelError> { + let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( + proposed_channel_value, self.holder_dust_limit_satoshis); + if proposed_channel_value < holder_selected_channel_reserve_satoshis { + return Err(ChannelError::Warn(format!( + "Proposed channel value below reserve mandated by holder, {} vs {}", + proposed_channel_value, holder_selected_channel_reserve_satoshis, + ))); + } + let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( + proposed_channel_value, self.counterparty_dust_limit_satoshis); + if proposed_channel_value < counterparty_selected_channel_reserve_satoshis { + return Err(ChannelError::Warn(format!( + "Proposed channel value below reserve mandated by counterparty, {} vs {}", + proposed_channel_value, 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. /// @@ -3828,10 +3848,6 @@ impl ChannelContext where SP::Target: SignerProvider { pub fn get_splice_init(&self, our_funding_contribution_satoshis: i64, signer_provider: &SP, funding_feerate_perkw: u32, locktime: u32, ) -> msgs::SpliceInit { - if !self.is_outbound() { - panic!("Tried to initiate a splice on an inbound channel!"); - } - // At this point we are not committed to the new channel value yet, but the funding pubkey // depends on the channel value, so we create here a new funding pubkey with the new // channel value. @@ -3861,10 +3877,6 @@ 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(&mut self, our_funding_contribution_satoshis: i64) -> Result { - if self.is_outbound() { - panic!("Tried to accept a splice on an outound channel!"); - } - // TODO(splicing): checks // Note: at this point keys are already updated @@ -7458,7 +7470,7 @@ impl Channel where #[cfg(splicing)] pub fn splice_init( &mut self, our_funding_contribution_satoshis: i64, - _signer_provider: &SP, _entropy_source: &ES, _holder_node_id: PublicKey, logger: &L + _signer_provider: &SP, _entropy_source: &ES, _holder_node_id: PublicKey, _logger: &L ) -> Result where ES::Target: EntropySource, L::Target: Logger diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c51fcbec756..96603602978 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4011,7 +4011,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_perkw: u32, locktime: u32, ) -> Result<(), APIError> { let per_peer_state = self.per_peer_state.read().unwrap(); @@ -4027,7 +4027,7 @@ where if let ChannelPhase::Funded(chan) = chan_phase_entry.get_mut() { let pre_channel_value = chan.context.get_value_satoshis(); // Sanity check: capacity cannot decrease below 0 - if our_funding_contribution_satoshis < 0 && -our_funding_contribution_satoshis > (pre_channel_value as i64) { + if (pre_channel_value as i64).saturating_add(our_funding_contribution_satoshis) < 0 { return Err(APIError::APIMisuseError { err: format!( "Post-splicing channel value cannot be negative. It was {} - {}", @@ -4054,7 +4054,6 @@ where chan.context.pending_splice_pre = Some(PendingSpliceInfoPre { our_funding_contribution: our_funding_contribution_satoshis, - funding_feerate_perkw, locktime, our_funding_inputs, }); let msg = chan.context.get_splice_init(our_funding_contribution_satoshis, &self.signer_provider, funding_feerate_perkw, locktime); @@ -9013,7 +9012,7 @@ where if let ChannelPhase::Funded(chan) = chan_entry.get_mut() { let pre_channel_value = chan.context.get_value_satoshis(); // Sanity check: capacity cannot decrease below 0 - if msg.funding_contribution_satoshis < 0 && -msg.funding_contribution_satoshis > (pre_channel_value as i64) { + if (pre_channel_value as i64).saturating_add(msg.funding_contribution_satoshis) < 0 { return Err(MsgHandleErrInternal::send_err_msg_no_close(format!( "Post-splicing channel value cannot be negative. It was {} - {}", pre_channel_value, -msg.funding_contribution_satoshis, ), msg.channel_id)); @@ -9026,11 +9025,10 @@ where } let post_channel_value = PendingSpliceInfoPre::compute_post_value(pre_channel_value, msg.funding_contribution_satoshis, our_funding_contribution); - if post_channel_value < 1000 { - return Err(MsgHandleErrInternal::send_err_msg_no_close(format!( - "Post-splicing channel value must be at least 1000 satoshis. It was {}", post_channel_value, - ), msg.channel_id)); - } + + // Check for reserve requirement, it will also be checked later at tx_complete + let _res = chan.context.check_channel_value_meets_reserve_requirements(post_channel_value) + .map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.channel_id))?; // 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. @@ -9083,7 +9081,7 @@ where let peer_state = &mut *peer_state_lock; // Look for the channel - let _pending_splice = match peer_state.channel_by_id.entry(msg.channel_id) { + 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 @@ -9091,12 +9089,19 @@ where hash_map::Entry::Occupied(mut chan) => { if let ChannelPhase::Funded(chan) = chan.get_mut() { // check if splice is pending - if let Some(pending_splice) = &chan.context.pending_splice_pre { + let pending_splice = if let Some(pending_splice) = &chan.context.pending_splice_pre { // Note: this is incomplete (their funding contribution is not set) pending_splice.clone() } else { return Err(MsgHandleErrInternal::send_err_msg_no_close("Channel is not in pending splice".to_owned(), msg.channel_id)); - } + }; + + let pre_channel_value = chan.context.get_value_satoshis(); + let post_channel_value = PendingSpliceInfoPre::compute_post_value(pre_channel_value, pending_splice.our_funding_contribution, msg.funding_contribution_satoshis); + + // Check for reserve requirement, it will also be checked later at tx_complete + let _res = chan.context.check_channel_value_meets_reserve_requirements(post_channel_value) + .map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.channel_id))?; } else { return Err(MsgHandleErrInternal::send_err_msg_no_close("Channel is not funded, cannot splice".to_owned(), msg.channel_id)); }