WIP: Add FundingNeeded event for splicing

Rather than requiring the user to pass FundingTxInputs when initiating a
splice, generate a FundingNeeded event once the channel has become
quiescent. This simplifies error handling and UTXO / change address
clean-up by consolidating it in SpliceFailed event handling.

Later, this event will be used for opportunistic contributions (i.e.,
when the counterparty wins quiescence or initiates), dual-funding, and
RBF.

Author: jkczyz

lightning/src/events/mod.rs

@@ -25,6 +25,7 @@ use crate::blinded_path::payment::{
 use crate::chain::transaction;
 use crate::ln::channel::FUNDING_CONF_DEADLINE_BLOCKS;
 use crate::ln::channelmanager::{InterceptId, PaymentId, RecipientOnionFields};
+use crate::ln::funding::FundingContribution;
 use crate::ln::types::ChannelId;
 use crate::ln::{msgs, LocalHTLCFailureReason};
 use crate::offers::invoice::Bolt12Invoice;
@@ -1816,6 +1817,28 @@ pub enum Event {
 		/// [`ChannelManager::respond_to_static_invoice_request`]: crate::ln::channelmanager::ChannelManager::respond_to_static_invoice_request
 		invoice_request: InvoiceRequest,
 	},
+	///
+	FundingNeeded {
+		/// The `channel_id` of the channel which you'll need to pass back into
+		/// [`ChannelManager::funding_contributed`].
+		///
+		/// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed
+		channel_id: ChannelId,
+		/// The counterparty's `node_id`, which you'll need to pass back into
+		/// [`ChannelManager::funding_contributed`].
+		///
+		/// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed
+		counterparty_node_id: PublicKey,
+		/// The `user_channel_id` value passed in for outbound channels, or for inbound channels if
+		/// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise
+		/// `user_channel_id` will be randomized for inbound channels.
+		///
+		/// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels
+		user_channel_id: u128,
+
+		///
+		contribution: FundingContribution,
+	},
 	/// Indicates that a channel funding transaction constructed interactively is ready to be
 	/// signed. This event will only be triggered if at least one input was contributed.
 	///
@@ -2347,6 +2370,10 @@ impl Writeable for Event {
 					(13, *contributed_outputs, optional_vec),
 				});
 			},
+			&Event::FundingNeeded { .. } => {
+				54u8.write(writer)?;
+				todo!();
+			},
 			// Note that, going forward, all new events must only write data inside of
 			// `write_tlv_fields`. Versions 0.0.101+ will ignore odd-numbered events that write
 			// data via `write_tlv_fields`.
@@ -2978,6 +3005,7 @@ impl MaybeReadable for Event {
 				};
 				f()
 			},
+			54u8 => todo!(),
 			// Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue.
 			// Version 0.0.100 failed to properly ignore odd types, possibly resulting in corrupt
 			// reads.

lightning/src/ln/channel.rs

@@ -55,7 +55,7 @@ use crate::ln::channelmanager::{
 	RAACommitmentOrder, SentHTLCId, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT,
 	MIN_CLTV_EXPIRY_DELTA,
 };
-use crate::ln::funding::{FundingTxInput, SpliceContribution};
+use crate::ln::funding::{FundingContribution, FundingTxInput, SpliceContribution};
 use crate::ln::interactivetxs::{
 	calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteValue,
 	InteractiveTxConstructor, InteractiveTxConstructorArgs, InteractiveTxMessageSend,
@@ -2802,24 +2802,28 @@ impl_writeable_tlv_based!(SpliceInstructions, {
 
 #[derive(Debug)]
 pub(crate) enum QuiescentAction {
-	Splice(SpliceInstructions),
+	LegacySplice(SpliceInstructions),
+	Splice(SpliceContribution),
 	#[cfg(any(test, fuzzing))]
 	DoNothing,
 }
 
 pub(crate) enum StfuResponse {
 	Stfu(msgs::Stfu),
 	SpliceInit(msgs::SpliceInit),
+	FundingNeeded(FundingContribution),
 }
 
 #[cfg(any(test, fuzzing))]
 impl_writeable_tlv_based_enum_upgradable!(QuiescentAction,
 	(0, DoNothing) => {},
-	{1, Splice} => (),
+	{1, LegacySplice} => (),
+	{2, Splice} => (),
 );
 #[cfg(not(any(test, fuzzing)))]
 impl_writeable_tlv_based_enum_upgradable!(QuiescentAction,,
-	{1, Splice} => (),
+	{1, LegacySplice} => (),
+	{2, Splice} => (),
 );
 
 /// Wrapper around a [`Transaction`] useful for caching the result of [`Transaction::compute_txid`].
@@ -6495,7 +6499,7 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos
 }
 
 fn check_splice_contribution_sufficient(
-	contribution: &SpliceContribution, is_initiator: bool, funding_feerate: FeeRate,
+	contribution: &FundingContribution, is_initiator: bool, funding_feerate: FeeRate,
 ) -> Result<SignedAmount, String> {
 	if contribution.inputs().is_empty() {
 		let estimated_fee = Amount::from_sat(estimate_v2_funding_transaction_fee(
@@ -6621,30 +6625,30 @@ fn check_v2_funding_inputs_sufficient(
 
 /// Context for negotiating channels (dual-funded V2 open, splicing)
 #[derive(Debug)]
-pub(super) struct FundingNegotiationContext {
+pub struct FundingNegotiationContext {
 	/// Whether we initiated the funding negotiation.
-	pub is_initiator: bool,
+	pub(super) is_initiator: bool,
 	/// The amount in satoshis we will be contributing to the channel.
-	pub our_funding_contribution: SignedAmount,
+	pub(super) our_funding_contribution: SignedAmount,
 	/// The funding transaction locktime suggested by the initiator. If set by us, it is always set
 	/// to the current block height to align incentives against fee-sniping.
-	pub funding_tx_locktime: LockTime,
+	pub(super) funding_tx_locktime: LockTime,
 	/// The feerate set by the initiator to be used for the funding transaction.
 	#[allow(dead_code)] // TODO(dual_funding): Remove once V2 channels is enabled.
-	pub funding_feerate_sat_per_1000_weight: u32,
+	pub(super) funding_feerate_sat_per_1000_weight: u32,
 	/// The input spending the previous funding output, if this is a splice.
 	#[allow(dead_code)] // TODO(splicing): Remove once splicing is enabled.
-	pub shared_funding_input: Option<SharedOwnedInput>,
+	pub(super) shared_funding_input: Option<SharedOwnedInput>,
 	/// The funding inputs we will be contributing to the channel.
 	#[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled.
-	pub our_funding_inputs: Vec<FundingTxInput>,
+	pub(super) our_funding_inputs: Vec<FundingTxInput>,
 	/// The funding outputs we will be contributing to the channel.
 	#[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled.
-	pub our_funding_outputs: Vec<TxOut>,
+	pub(super) our_funding_outputs: Vec<TxOut>,
 	/// The change output script. This will be used if needed or -- if not set -- generated using
 	/// `SignerProvider::get_destination_script`.
 	#[allow(dead_code)] // TODO(splicing): Remove once splicing is enabled.
-	pub change_script: Option<ScriptBuf>,
+	pub(super) change_script: Option<ScriptBuf>,
 }
 
 impl FundingNegotiationContext {
@@ -6970,7 +6974,7 @@ where
 				self.reset_pending_splice_state()
 			} else {
 				match self.quiescent_action.take() {
-					Some(QuiescentAction::Splice(instructions)) => {
+					Some(QuiescentAction::LegacySplice(instructions)) => {
 						self.context.channel_state.clear_awaiting_quiescence();
 						let (inputs, outputs) = instructions.into_contributed_inputs_and_outputs();
 						Some(SpliceFundingFailed {
@@ -6980,6 +6984,15 @@ where
 							contributed_outputs: outputs,
 						})
 					},
+					Some(QuiescentAction::Splice(contribution)) => {
+						self.context.channel_state.clear_awaiting_quiescence();
+						Some(SpliceFundingFailed {
+							funding_txo: None,
+							channel_type: None,
+							contributed_inputs: vec![],
+							contributed_outputs: contribution.into_outputs(),
+						})
+					},
 					#[cfg(any(test, fuzzing))]
 					Some(quiescent_action) => {
 						self.quiescent_action = Some(quiescent_action);
@@ -11274,7 +11287,7 @@ where
 			self.get_announcement_sigs(node_signer, chain_hash, user_config, block_height, logger);
 
 		if let Some(quiescent_action) = self.quiescent_action.as_ref() {
-			if matches!(quiescent_action, QuiescentAction::Splice(_)) {
+			if matches!(quiescent_action, QuiescentAction::Splice(_) | QuiescentAction::LegacySplice(_)) {
 				self.context.channel_state.set_awaiting_quiescence();
 			}
 		}
@@ -11924,8 +11937,7 @@ where
 	/// - `change_script`: an option change output script. If `None` and needed, one will be
 	///   generated by `SignerProvider::get_destination_script`.
 	pub fn splice_channel<L: Deref>(
-		&mut self, contribution: SpliceContribution, funding_feerate_per_kw: u32, locktime: u32,
-		logger: &L,
+		&mut self, contribution: SpliceContribution, logger: &L,
 	) -> Result<Option<msgs::Stfu>, APIError>
 	where
 		L::Target: Logger,
@@ -11939,8 +11951,15 @@ where
 			});
 		}
 
-		// Check if a splice has been initiated already.
-		// Note: only a single outstanding splice is supported (per spec)
+		if self.context.channel_state.is_quiescent() {
+			return Err(APIError::APIMisuseError {
+				err: format!(
+					"Channel {} cannot be spliced as it is already quiescent",
+					self.context.channel_id(),
+				),
+			});
+		}
+
 		if self.pending_splice.is_some() || self.quiescent_action.is_some() {
 			return Err(APIError::APIMisuseError {
 				err: format!(
@@ -11969,71 +11988,71 @@ where
 			});
 		}
 
-		// Fees for splice-out are paid from the channel balance whereas fees for splice-in
-		// are paid by the funding inputs. Therefore, in the case of splice-out, we add the
-		// fees on top of the user-specified contribution. We leave the user-specified
-		// contribution as-is for splice-ins.
-		let adjusted_funding_contribution = check_splice_contribution_sufficient(
-			&contribution,
-			true,
-			FeeRate::from_sat_per_kwu(u64::from(funding_feerate_per_kw)),
-		)
-		.map_err(|e| APIError::APIMisuseError {
-			err: format!(
-				"Channel {} cannot be {}; {}",
-				self.context.channel_id(),
-				if our_funding_contribution.is_positive() { "spliced in" } else { "spliced out" },
-				e
-			),
-		})?;
+		//// Fees for splice-out are paid from the channel balance whereas fees for splice-in
+		//// are paid by the funding inputs. Therefore, in the case of splice-out, we add the
+		//// fees on top of the user-specified contribution. We leave the user-specified
+		//// contribution as-is for splice-ins.
+		//let adjusted_funding_contribution = check_splice_contribution_sufficient(
+		//	&contribution,
+		//	true,
+		//	FeeRate::from_sat_per_kwu(u64::from(funding_feerate_per_kw)),
+		//)
+		//.map_err(|e| APIError::APIMisuseError {
+		//	err: format!(
+		//		"Channel {} cannot be {}; {}",
+		//		self.context.channel_id(),
+		//		if our_funding_contribution.is_positive() { "spliced in" } else { "spliced out" },
+		//		e
+		//	),
+		//})?;
 
 		// 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)
 		let their_funding_contribution = SignedAmount::ZERO;
 		self.validate_splice_contributions(
-			adjusted_funding_contribution,
+			//adjusted_funding_contribution,
+			our_funding_contribution,
 			their_funding_contribution,
 		)
 		.map_err(|err| APIError::APIMisuseError { err })?;
 
-		for FundingTxInput { utxo, prevtx, .. } in contribution.inputs().iter() {
-			const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput {
-				channel_id: ChannelId([0; 32]),
-				serial_id: 0,
-				prevtx: None,
-				prevtx_out: 0,
-				sequence: 0,
-				// Mutually exclusive with prevtx, which is accounted for below.
-				shared_input_txid: None,
-			};
-			let message_len = MESSAGE_TEMPLATE.serialized_length() + prevtx.serialized_length();
-			if message_len > LN_MAX_MSG_LEN {
-				return Err(APIError::APIMisuseError {
-					err: format!(
-						"Funding input references a prevtx that is too large for tx_add_input: {}",
-						utxo.outpoint,
-					),
-				});
-			}
-		}
-
-		let (our_funding_inputs, our_funding_outputs, change_script) = contribution.into_tx_parts();
-
-		let action = QuiescentAction::Splice(SpliceInstructions {
-			adjusted_funding_contribution,
-			our_funding_inputs,
-			our_funding_outputs,
-			change_script,
-			funding_feerate_per_kw,
-			locktime,
-		});
-		self.propose_quiescence(logger, action)
+		//for FundingTxInput { utxo, prevtx, .. } in contribution.inputs().iter() {
+		//	const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput {
+		//		channel_id: ChannelId([0; 32]),
+		//		serial_id: 0,
+		//		prevtx: None,
+		//		prevtx_out: 0,
+		//		sequence: 0,
+		//		// Mutually exclusive with prevtx, which is accounted for below.
+		//		shared_input_txid: None,
+		//	};
+		//	let message_len = MESSAGE_TEMPLATE.serialized_length() + prevtx.serialized_length();
+		//	if message_len > LN_MAX_MSG_LEN {
+		//		return Err(APIError::APIMisuseError {
+		//			err: format!(
+		//				"Funding input references a prevtx that is too large for tx_add_input: {}",
+		//				utxo.outpoint,
+		//			),
+		//		});
+		//	}
+		//}
+
+		self.propose_quiescence(logger, QuiescentAction::Splice(contribution))
 			.map_err(|e| APIError::APIMisuseError { err: e.to_owned() })
 	}
 
-	fn send_splice_init(&mut self, instructions: SpliceInstructions) -> msgs::SpliceInit {
-		debug_assert!(self.pending_splice.is_none());
+	pub fn funding_contributed<L: Deref>(
+		&mut self, context: FundingNegotiationContext, logger: &L,
+	) -> Result<msgs::SpliceInit, APIError>
+	where
+		L::Target: Logger,
+	{
+		// TODO: Add any checks or move them to FundingNegotiationContext construction. Probably
+		// emit a `SpliceFailed` event instead of returning an error
+		Ok(self.send_splice_init_internal(context))
+	}
 
+	fn send_splice_init(&mut self, instructions: SpliceInstructions) -> msgs::SpliceInit {
 		let SpliceInstructions {
 			adjusted_funding_contribution,
 			our_funding_inputs,
@@ -12055,6 +12074,11 @@ where
 			change_script,
 		};
 
+		self.send_splice_init_internal(context)
+	}
+
+	fn send_splice_init_internal(&mut self, context: FundingNegotiationContext) -> msgs::SpliceInit {
+		debug_assert!(self.pending_splice.is_none());
 		// Rotate the funding pubkey using the prev_funding_txid as a tweak
 		let prev_funding_txid = self.funding.get_funding_txid();
 		let funding_pubkey = match (prev_funding_txid, &self.context.holder_signer) {
@@ -12069,6 +12093,10 @@ where
 			_ => todo!(),
 		};
 
+		let funding_feerate_per_kw = context.funding_feerate_sat_per_1000_weight;
+		let funding_contribution_satoshis = context.our_funding_contribution.to_sat();
+		let locktime = context.funding_tx_locktime.to_consensus_u32();
+
 		let funding_negotiation =
 			FundingNegotiation::AwaitingAck { context, new_holder_funding_key: funding_pubkey };
 		self.pending_splice = Some(PendingFunding {
@@ -12080,7 +12108,7 @@ where
 
 		msgs::SpliceInit {
 			channel_id: self.context.channel_id,
-			funding_contribution_satoshis: adjusted_funding_contribution.to_sat(),
+			funding_contribution_satoshis,
 			funding_feerate_per_kw,
 			locktime,
 			funding_pubkey,
@@ -13302,9 +13330,9 @@ where
 						"Internal Error: Didn't have anything to do after reaching quiescence".to_owned()
 					));
 				},
-				Some(QuiescentAction::Splice(instructions)) => {
+				Some(QuiescentAction::LegacySplice(instructions)) => {
 					if self.pending_splice.is_some() {
-						self.quiescent_action = Some(QuiescentAction::Splice(instructions));
+						self.quiescent_action = Some(QuiescentAction::LegacySplice(instructions));
 
 						return Err(ChannelError::WarnAndDisconnect(
 							format!(
@@ -13317,6 +13345,21 @@ where
 					let splice_init = self.send_splice_init(instructions);
 					return Ok(Some(StfuResponse::SpliceInit(splice_init)));
 				},
+				Some(QuiescentAction::Splice(contribution)) => {
+					if self.pending_splice.is_some() {
+						self.quiescent_action = Some(QuiescentAction::Splice(contribution));
+
+						return Err(ChannelError::WarnAndDisconnect(
+							format!(
+								"Channel {} cannot be spliced as it already has a splice pending",
+								self.context.channel_id(),
+							),
+						));
+					}
+
+					let contribution = contribution.into_funding_contribution();
+					return Ok(Some(StfuResponse::FundingNeeded(contribution)));
+				},
 				#[cfg(any(test, fuzzing))]
 				Some(QuiescentAction::DoNothing) => {
 					// In quiescence test we want to just hang out here, letting the test manually