Push splice initiation through the quiescent pipeline

Now that we have a `QuiescentAction` to track what we intend to do
once we reach quiescence, we need to use it to initiate splices.

Here we do so, adding a new `SpliceInstructions` to track the
arguments that are currently passed to `splice_channel`. While
these may not be exactly the right arguments to track in the end,
a lot of the splice logic is still in flight, so we can worry about
it later.

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -2443,13 +2443,42 @@ impl PendingSplice {
 	}
 }
 
+pub(crate) struct SpliceInstructions {
+	our_funding_contribution_satoshis: i64,
+	our_funding_inputs: Vec<(TxIn, Transaction, Weight)>,
+	change_script: Option<ScriptBuf>,
+	funding_feerate_per_kw: u32,
+	locktime: u32,
+}
+
+impl_writeable_tlv_based!(SpliceInstructions, {
+	(1, our_funding_contribution_satoshis, required),
+	(3, our_funding_inputs, required_vec),
+	(5, change_script, option),
+	(7, funding_feerate_per_kw, required),
+	(9, locktime, required),
+});
+
 pub(crate) enum QuiescentAction {
-	// TODO: Make this test-only once we have another variant (as some code requires *a* variant).
+	Splice(SpliceInstructions),
+	#[cfg(any(test, fuzzing))]
 	DoNothing,
 }
 
+pub(crate) enum StfuResponse {
+	Stfu(msgs::Stfu),
+	#[cfg_attr(not(splicing), allow(unused))]
+	SpliceInit(msgs::SpliceInit),
+}
+
+#[cfg(any(test, fuzzing))]
 impl_writeable_tlv_based_enum_upgradable!(QuiescentAction,
-	(99, DoNothing) => {},
+	(0, DoNothing) => {},
+	{1, Splice} => (),
+);
+#[cfg(not(any(test, fuzzing)))]
+impl_writeable_tlv_based_enum_upgradable!(QuiescentAction,,
+	{1, Splice} => (),
 );
 
 /// Wrapper around a [`Transaction`] useful for caching the result of [`Transaction::compute_txid`].
@@ -5926,7 +5955,7 @@ fn estimate_v2_funding_transaction_fee(
 fn check_v2_funding_inputs_sufficient(
 	contribution_amount: i64, funding_inputs: &[(TxIn, Transaction, Weight)], is_initiator: bool,
 	is_splice: bool, funding_feerate_sat_per_1000_weight: u32,
-) -> Result<u64, ChannelError> {
+) -> Result<u64, String> {
 	let mut total_input_witness_weight = Weight::from_wu(funding_inputs.iter().map(|(_, _, w)| w.to_wu()).sum());
 	let mut funding_inputs_len = funding_inputs.len();
 	if is_initiator && is_splice {
@@ -5941,10 +5970,10 @@ fn check_v2_funding_inputs_sufficient(
 		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!(
+			return Err(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
-			)));
+			));
 		}
 	}
 
@@ -5961,10 +5990,10 @@ fn check_v2_funding_inputs_sufficient(
 
 	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!(
+		Err(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)
 	}
@@ -10614,11 +10643,14 @@ where
 	/// - `change_script`: an option change output script. If `None` and needed, one will be
 	///   generated by `SignerProvider::get_destination_script`.
 	#[cfg(splicing)]
-	pub fn splice_channel(
+	pub fn splice_channel<L: Deref>(
 		&mut self, our_funding_contribution_satoshis: i64,
 		our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, change_script: Option<ScriptBuf>,
-		funding_feerate_per_kw: u32, locktime: u32,
-	) -> Result<msgs::SpliceInit, APIError> {
+		funding_feerate_per_kw: u32, locktime: u32, logger: &L,
+	) -> Result<Option<msgs::Stfu>, APIError>
+	where
+		L::Target: Logger,
+	{
 		// Check if a splice has been initiated already.
 		// Note: only a single outstanding splice is supported (per spec)
 		if self.pending_splice.is_some() {
@@ -10671,11 +10703,44 @@ where
 				err,
 			),
 		})?;
-		// Convert inputs
-		let mut funding_inputs = Vec::new();
-		for (tx_in, tx, _w) in our_funding_inputs.into_iter() {
-			let tx16 = TransactionU16LenLimited::new(tx)
-				.map_err(|_e| APIError::APIMisuseError { err: format!("Too large transaction") })?;
+
+		// TODO(splicing): Check that transactions aren't too big for the splice_init message here.
+
+		let action = QuiescentAction::Splice(SpliceInstructions {
+			our_funding_contribution_satoshis,
+			our_funding_inputs,
+			change_script,
+			funding_feerate_per_kw,
+			locktime,
+		});
+		self.propose_quiescence(logger, action)
+			.map_err(|e| APIError::APIMisuseError { err: e.to_owned() })
+	}
+
+	#[cfg(splicing)]
+	fn send_splice_init(
+		&mut self, instructions: SpliceInstructions,
+	) -> Result<msgs::SpliceInit, String> {
+		let SpliceInstructions {
+			our_funding_contribution_satoshis,
+			our_funding_inputs,
+			change_script,
+			funding_feerate_per_kw,
+			locktime,
+		} = instructions;
+
+		// Check that the channel value hasn't changed out from under us.
+		let _fee = check_v2_funding_inputs_sufficient(
+			our_funding_contribution_satoshis,
+			&our_funding_inputs,
+			true,
+			true,
+			funding_feerate_per_kw,
+		)?;
+
+		let mut funding_inputs = Vec::with_capacity(our_funding_inputs.len());
+		for (tx_in, tx, _weight) in our_funding_inputs {
+			let tx16 = TransactionU16LenLimited::new(tx).map_err(|_e| "tx too big".to_owned())?;
 			funding_inputs.push((tx_in, tx16));
 		}
 
@@ -11567,23 +11632,21 @@ where
 		);
 	}
 
-	#[cfg(any(test, fuzzing))]
+	#[cfg(any(splicing, test, fuzzing))]
 	#[rustfmt::skip]
 	pub fn propose_quiescence<L: Deref>(
 		&mut self, logger: &L, action: QuiescentAction,
-	) -> Result<Option<msgs::Stfu>, ChannelError>
+	) -> Result<Option<msgs::Stfu>, &'static str>
 	where
 		L::Target: Logger,
 	{
 		log_debug!(logger, "Attempting to initiate quiescence");
 
 		if !self.context.is_usable() {
-			return Err(ChannelError::Ignore(
-				"Channel is not in a usable state to propose quiescence".to_owned()
-			));
+			return Err("Channel is not in a usable state to propose quiescence");
 		}
 		if self.context.post_quiescence_action.is_some() {
-			return Err(ChannelError::Ignore("Channel is already quiescing".to_owned()));
+			return Err("Channel is already quiescing");
 		}
 
 		self.context.post_quiescence_action = Some(action);
@@ -11604,7 +11667,7 @@ where
 
 	// Assumes we are either awaiting quiescence or our counterparty has requested quiescence.
 	#[rustfmt::skip]
-	pub fn send_stfu<L: Deref>(&mut self, logger: &L) -> Result<msgs::Stfu, ChannelError>
+	pub fn send_stfu<L: Deref>(&mut self, logger: &L) -> Result<msgs::Stfu, &'static str>
 	where
 		L::Target: Logger,
 	{
@@ -11618,9 +11681,7 @@ where
 		if self.context.is_waiting_on_peer_pending_channel_update()
 			|| self.context.is_monitor_or_signer_pending_channel_update()
 		{
-			return Err(ChannelError::Ignore(
-				"We cannot send `stfu` while state machine is pending".to_owned()
-			));
+			return Err("We cannot send `stfu` while state machine is pending")
 		}
 
 		let initiator = if self.context.channel_state.is_remote_stfu_sent() {
@@ -11646,7 +11707,7 @@ where
 	#[rustfmt::skip]
 	pub fn stfu<L: Deref>(
 		&mut self, msg: &msgs::Stfu, logger: &L
-	) -> Result<Option<msgs::Stfu>, ChannelError> where L::Target: Logger {
+	) -> Result<Option<StfuResponse>, ChannelError> where L::Target: Logger {
 		if self.context.channel_state.is_quiescent() {
 			return Err(ChannelError::Warn("Channel is already quiescent".to_owned()));
 		}
@@ -11677,7 +11738,10 @@ where
 			self.context.channel_state.set_remote_stfu_sent();
 
 			log_debug!(logger, "Received counterparty stfu proposing quiescence");
-			return self.send_stfu(logger).map(|stfu| Some(stfu));
+			return self
+				.send_stfu(logger)
+				.map(|stfu| Some(StfuResponse::Stfu(stfu)))
+				.map_err(|e| ChannelError::Ignore(e.to_owned()));
 		}
 
 		// We already sent `stfu` and are now processing theirs. It may be in response to ours, or
@@ -11718,6 +11782,13 @@ where
 						"Internal Error: Didn't have anything to do after reaching quiescence".to_owned()
 					));
 				},
+				Some(QuiescentAction::Splice(_instructions)) => {
+					#[cfg(splicing)]
+					return self.send_splice_init(_instructions)
+						.map(|splice_init| Some(StfuResponse::SpliceInit(splice_init)))
+						.map_err(|e| ChannelError::Ignore(e.to_owned()));
+				},
+				#[cfg(any(test, fuzzing))]
 				Some(QuiescentAction::DoNothing) => {
 					// In quiescence test we want to just hang out here, letting the test manually
 					// leave quiescence.
@@ -11750,7 +11821,10 @@ where
 			|| (self.context.channel_state.is_remote_stfu_sent()
 				&& !self.context.channel_state.is_local_stfu_sent())
 		{
-			return self.send_stfu(logger).map(|stfu| Some(stfu));
+			return self
+				.send_stfu(logger)
+				.map(|stfu| Some(stfu))
+				.map_err(|e| ChannelError::Ignore(e.to_owned()));
 		}
 
 		// We're either:
@@ -15920,8 +15994,8 @@ mod tests {
 				2000,
 			);
 			assert_eq!(
-				format!("{:?}", res.err().unwrap()),
-				"Warn: Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1730. Need more inputs.",
+				res.err().unwrap(),
+				"Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1730. Need more inputs.",
 			);
 		}
 
@@ -15956,8 +16030,8 @@ mod tests {
 				2200,
 			);
 			assert_eq!(
-				format!("{:?}", res.err().unwrap()),
-				"Warn: Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2495. Need more inputs.",
+				res.err().unwrap(),
+				"Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2495. Need more inputs.",
 			);
 		}
 

lightning/src/ln/channelmanager.rs

@@ -62,7 +62,7 @@ use crate::ln::channel::QuiescentAction;
 use crate::ln::channel::{
 	self, hold_time_since, Channel, ChannelError, ChannelUpdateStatus, FundedChannel,
 	InboundV1Channel, OutboundV1Channel, PendingV2Channel, ReconnectionMsg, ShutdownResult,
-	UpdateFulfillCommitFetch, WithChannelContext,
+	StfuResponse, UpdateFulfillCommitFetch, WithChannelContext,
 };
 use crate::ln::channel_state::ChannelDetails;
 use crate::ln::inbound_payment;
@@ -4503,17 +4503,21 @@ where
 			hash_map::Entry::Occupied(mut chan_phase_entry) => {
 				let locktime = locktime.unwrap_or_else(|| self.current_best_block().height);
 				if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() {
-					let msg = chan.splice_channel(
+					let logger = WithChannelContext::from(&self.logger, &chan.context, None);
+					let msg_opt = chan.splice_channel(
 						our_funding_contribution_satoshis,
 						our_funding_inputs,
 						change_script,
 						funding_feerate_per_kw,
 						locktime,
+						&&logger,
 					)?;
-					peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceInit {
-						node_id: *counterparty_node_id,
-						msg,
-					});
+					if let Some(msg) = msg_opt {
+						peer_state.pending_msg_events.push(MessageSendEvent::SendStfu {
+							node_id: *counterparty_node_id,
+							msg,
+						});
+					}
 					Ok(())
 				} else {
 					Err(APIError::ChannelUnavailable {
@@ -10841,22 +10845,31 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 			));
 		}
 
-		let mut sent_stfu = false;
 		match peer_state.channel_by_id.entry(msg.channel_id) {
 			hash_map::Entry::Occupied(mut chan_entry) => {
 				if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
 					let logger = WithContext::from(
 						&self.logger, Some(*counterparty_node_id), Some(msg.channel_id), None
 					);
 
-					if let Some(stfu) = try_channel_entry!(
-						self, peer_state, chan.stfu(&msg, &&logger), chan_entry
-					) {
-						sent_stfu = true;
-						peer_state.pending_msg_events.push(MessageSendEvent::SendStfu {
-							node_id: *counterparty_node_id,
-							msg: stfu,
-						});
+					let res = chan.stfu(&msg, &&logger);
+					let resp = try_channel_entry!(self, peer_state, res, chan_entry);
+					match resp {
+						None => Ok(false),
+						Some(StfuResponse::Stfu(msg)) => {
+							peer_state.pending_msg_events.push(MessageSendEvent::SendStfu {
+								node_id: *counterparty_node_id,
+								msg,
+							});
+							Ok(true)
+						},
+						Some(StfuResponse::SpliceInit(msg)) => {
+							peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceInit {
+								node_id: *counterparty_node_id,
+								msg,
+							});
+							Ok(true)
+						},
 					}
 				} else {
 					let msg = "Peer sent `stfu` for an unfunded channel";
@@ -10871,8 +10884,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 				msg.channel_id
 			))
 		}
-
-		Ok(sent_stfu)
 	}
 
 	#[rustfmt::skip]
@@ -13884,8 +13895,8 @@ where
 			let persist = match &res {
 				Err(e) if e.closes_channel() => NotifyOption::DoPersist,
 				Err(_) => NotifyOption::SkipPersistHandleEvents,
-				Ok(sent_stfu) => {
-					if *sent_stfu {
+				Ok(responded) => {
+					if *responded {
 						NotifyOption::SkipPersistHandleEvents
 					} else {
 						NotifyOption::SkipPersistNoEvents

lightning/src/ln/splicing_tests.rs

@@ -25,6 +25,8 @@ fn test_v1_splice_in() {
 	let acceptor_node_index = 1;
 	let initiator_node = &nodes[initiator_node_index];
 	let acceptor_node = &nodes[acceptor_node_index];
+	let initiator_node_id = initiator_node.node.get_our_node_id();
+	let acceptor_node_id = acceptor_node.node.get_our_node_id();
 
 	let channel_value_sat = 100_000;
 	let channel_reserve_amnt_sat = 1_000;
@@ -79,12 +81,16 @@ fn test_v1_splice_in() {
 			None, // locktime
 		)
 		.unwrap();
+
+	let init_stfu = get_event_msg!(initiator_node, MessageSendEvent::SendStfu, acceptor_node_id);
+	acceptor_node.node.handle_stfu(initiator_node_id, &init_stfu);
+
+	let ack_stfu = get_event_msg!(acceptor_node, MessageSendEvent::SendStfu, initiator_node_id);
+	initiator_node.node.handle_stfu(acceptor_node_id, &ack_stfu);
+
 	// Extract the splice_init message
-	let splice_init_msg = get_event_msg!(
-		initiator_node,
-		MessageSendEvent::SendSpliceInit,
-		acceptor_node.node.get_our_node_id()
-	);
+	let splice_init_msg =
+		get_event_msg!(initiator_node, MessageSendEvent::SendSpliceInit, acceptor_node_id);
 	assert_eq!(splice_init_msg.funding_contribution_satoshis, splice_in_sats as i64);
 	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);