Move checks&handling to Channel, cleanup, (review)

optout21 · optout21 · commit 3bfcb2347b1c · 2024-11-20T12:30:01.000+01:00
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -1181,12 +1181,13 @@ impl UnfundedChannelContext {
 /// Info about a pending splice, used in the pre-splice channel
 #[cfg(splicing)]
 #[derive(Clone)]
-pub(crate) struct PendingSpliceInfoPre {
+struct PendingSpliceInfoPre {
 	pub our_funding_contribution: i64,
 }
 
 #[cfg(splicing)]
 impl PendingSpliceInfoPre {
+	#[inline]
 	fn add_checked(base: u64, delta: i64) -> u64 {
 		if delta >= 0 {
 			base.saturating_add(delta as u64)
@@ -1238,7 +1239,7 @@ pub(super) struct ChannelContext<SP: Deref> where SP::Target: SignerProvider {
 
 	/// Info about an in-progress, pending splice (if any), on the pre-splice channel
 	#[cfg(splicing)]
-	pub(crate) pending_splice_pre: Option<PendingSpliceInfoPre>,
+	pending_splice_pre: Option<PendingSpliceInfoPre>,
 	
 	latest_monitor_update_id: u64,
 
@@ -3419,7 +3420,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 	}
 
 	/// Check that a balance value meets the channel reserve requirements or violates them (below reserve).
-	/// The channel value is an input, so that this can be used for checks with new planned channel value.
+	/// 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(any(dual_funding, splicing))]
 	pub fn check_balance_meets_reserve_requirements(&self, channel_value: u64, balance: u64) -> Result<(), ChannelError> {
 		let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
@@ -3848,17 +3850,15 @@ impl<SP: Deref> ChannelContext<SP> 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<msgs::SpliceAck, ChannelError> {
-		// TODO(splicing): checks
-
+	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;
-		Ok(msgs::SpliceAck {
+		msgs::SpliceAck {
 			channel_id: self.channel_id,
 			funding_contribution_satoshis: our_funding_contribution_satoshis,
 			funding_pubkey,
 			require_confirmed_inputs: None,
-		})
+		}
 	}
 }
 
@@ -7439,23 +7439,118 @@ impl<SP: Deref> Channel<SP> where
 		}
 	}
 
+	/// Initiate splicing
 	#[cfg(splicing)]
-	pub fn splice_init<ES: Deref, L: Deref>(
-		&mut self, our_funding_contribution_satoshis: i64,
-		_signer_provider: &SP, _entropy_source: &ES, _holder_node_id: PublicKey, _logger: &L
-	)
-		-> Result<msgs::SpliceAck, ChannelError>
-		where ES::Target: EntropySource, L::Target: Logger
-	{
-		if !self.context.is_outbound() {
-			// TODO(splicing): Apply start of splice (splice_start)
+	pub fn splice_channel(&mut self, our_funding_contribution_satoshis: i64,
+		funding_feerate_perkw: u32, locktime: u32,
+	) -> Result<msgs::SpliceInit, ChannelError> {
+		// 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.context.pending_splice_pre {
+			return Err(ChannelError::Warn(format!(
+				"Channel has already a splice pending, contribution {}", splice_info.our_funding_contribution
+			)));
+		}
 
-			let splice_ack_msg = self.context.get_splice_ack(our_funding_contribution_satoshis)?;
-			// TODO(splicing): start interactive funding negotiation
-			Ok(splice_ack_msg)
-		} else {
-			Err(ChannelError::Warn("Internal consistency error: splice_init on inbound channel".into()))
+		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)
+
+		self.context.pending_splice_pre = Some(PendingSpliceInfoPre {
+			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, their_funding_contribution_satoshis: i64, our_funding_contribution_satoshis: i64,
+	) -> Result<msgs::SpliceAck, ChannelError> {
+		// 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.context.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 = PendingSpliceInfoPre::compute_post_value(pre_channel_value, their_funding_contribution_satoshis, 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_channel_value, post_channel_value)?;
+
+		// 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, their_funding_contribution_satoshis: i64,
+	) -> Result<(), ChannelError> {
+		// check if splice is pending
+		let pending_splice = if let Some(pending_splice) = &self.context.pending_splice_pre {
+			pending_splice
+		} else {
+			return Err(ChannelError::Warn(format!("Channel is not in pending splice")));
+		};
+
+		let pre_channel_value = self.context.get_value_satoshis();
+		let post_channel_value = PendingSpliceInfoPre::compute_post_value(pre_channel_value, pending_splice.our_funding_contribution, their_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_channel_value, post_channel_value)?;
+		Ok(())
 	}
 
 	// Send stuff to our remote peers:
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -52,8 +52,6 @@ use crate::ln::inbound_payment;
 use crate::ln::types::ChannelId;
 use crate::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret};
 use crate::ln::channel::{self, Channel, ChannelPhase, ChannelError, ChannelUpdateStatus, ShutdownResult, UpdateFulfillCommitFetch, OutboundV1Channel, InboundV1Channel, WithChannelContext};
-#[cfg(splicing)]
-use crate::ln::channel::PendingSpliceInfoPre;
 use crate::ln::channel_state::ChannelDetails;
 use crate::types::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
 #[cfg(any(feature = "_test_utils", test))]
@@ -4126,41 +4124,17 @@ where
 		let peer_state = &mut *peer_state_lock;
 
 		// Look for the channel
-		match peer_state.channel_by_id.entry(channel_id.clone()) {
+		match peer_state.channel_by_id.entry(*channel_id) {
 			hash_map::Entry::Occupied(mut chan_phase_entry) => {
 				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 (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 {} - -{}",
-								pre_channel_value, our_funding_contribution_satoshis
-							)
-						});
-					}
-
-					if our_funding_contribution_satoshis < 0 {
-						return Err(APIError::APIMisuseError {
+					let msg = match chan.splice_channel(our_funding_contribution_satoshis, funding_feerate_perkw, locktime) {
+						Ok(msg) => msg,
+						Err(err) => return Err(APIError::APIMisuseError {
 							err: format!(
-								"TODO(splicing): Splice-out not supported, only splice in, contribution {}, channel_id {}",
-								our_funding_contribution_satoshis, channel_id
+								"Cannot initiate Splicing, {}, channel ID {}", err, channel_id
 							)
-						});
-					}
-
-					// 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)
-
-					if chan.context.pending_splice_pre.is_some() {
-						return Err(APIError::ChannelUnavailable { err: format!("Channel has already a splice pending, channel id {}", channel_id) });
-					}
-
-					chan.context.pending_splice_pre = Some(PendingSpliceInfoPre {
-						our_funding_contribution: our_funding_contribution_satoshis,
-					});
-
-					let msg = chan.context.get_splice_init(our_funding_contribution_satoshis, funding_feerate_perkw, locktime);
+						}),
+					};
 
 					peer_state.pending_msg_events.push(events::MessageSendEvent::SendSpliceInit {
 						node_id: *counterparty_node_id,
@@ -9038,38 +9012,7 @@ where
 				), msg.channel_id)),
 			hash_map::Entry::Occupied(mut chan_entry) => {
 				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 (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));
-					}
-
-					if msg.funding_contribution_satoshis < 0 {
-						return Err(MsgHandleErrInternal::send_err_msg_no_close(format!(
-							"Splice-out not supported, only splice in, relative {}", -msg.funding_contribution_satoshis,
-						), msg.channel_id));
-					}
-
-					let post_channel_value = PendingSpliceInfoPre::compute_post_value(pre_channel_value, msg.funding_contribution_satoshis, 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 = chan.context.check_balance_meets_reserve_requirements(post_channel_value, 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.
-					if let Some(splice_info) = &chan.context.pending_splice_pre {
-						return Err(MsgHandleErrInternal::send_err_msg_no_close(format!(
-							"Channel has already a splice pending, channel id {}, contribution {}",
-							msg.channel_id, splice_info.our_funding_contribution,
-						), msg.channel_id));
-					}
-
-					// TODO(splicing): Below step should come after channel phase change (after re-adding to channel map)
-					match chan.splice_init(our_funding_contribution, &self.signer_provider, &self.entropy_source, self.get_our_node_id(), &self.logger) {
+					match chan.splice_init(msg.funding_contribution_satoshis, our_funding_contribution) {
 						Ok(splice_ack_msg) => {
 							peer_state.pending_msg_events.push(events::MessageSendEvent::SendSpliceAck {
 								node_id: *counterparty_node_id,
@@ -9117,21 +9060,12 @@ where
 				), msg.channel_id)),
 			hash_map::Entry::Occupied(mut chan) => {
 				if let ChannelPhase::Funded(chan) = chan.get_mut() {
-					// check if splice is pending
-					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);
-
-					// Early check for reserve requirement, assuming maximum balance of full channel value
-					// This will also be checked later at tx_complete
-					let _res = chan.context.check_balance_meets_reserve_requirements(post_channel_value, post_channel_value)
-						.map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.channel_id))?;
+					match chan.splice_ack(msg.funding_contribution_satoshis) {
+						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));
 				}
