Split InteractiveTxConstructor::new into outbound/inbound variants

jkczyz · claude · jkczyz · commit fc3e1da8766e · 2026-02-12T16:17:09.000-06:00
Replace the single public InteractiveTxConstructor::new() with separate
new_for_outbound() and new_for_inbound() constructors. This moves the
initiator's first message preparation out of the core constructor,
making it infallible and removing is_initiator from the args struct.

Callers no longer need to handle constructor errors, which avoids having
to generate SpliceFailed/DiscardFunding events after the QuiescentAction
has already been consumed during splice_init/splice_ack handling.

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -58,8 +58,7 @@ use crate::ln::funding::{
 };
 use crate::ln::interactivetxs::{
 	AbortReason, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxConstructorArgs,
-	InteractiveTxMessageSend, InteractiveTxSigningSession, NegotiationError, SharedOwnedInput,
-	SharedOwnedOutput,
+	InteractiveTxMessageSend, InteractiveTxSigningSession, SharedOwnedInput, SharedOwnedOutput,
 };
 use crate::ln::msgs;
 use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket};
@@ -6586,7 +6585,7 @@ impl FundingNegotiationContext {
 	fn into_interactive_tx_constructor<SP: SignerProvider, ES: EntropySource>(
 		self, context: &ChannelContext<SP>, funding: &FundingScope, entropy_source: &ES,
 		holder_node_id: PublicKey,
-	) -> Result<InteractiveTxConstructor, NegotiationError> {
+	) -> InteractiveTxConstructor {
 		debug_assert_eq!(
 			self.shared_funding_input.is_some(),
 			funding.channel_transaction_parameters.splice_parent_funding_txid.is_some(),
@@ -6609,7 +6608,6 @@ impl FundingNegotiationContext {
 			counterparty_node_id: context.counterparty_node_id,
 			channel_id: context.channel_id(),
 			feerate_sat_per_kw: self.funding_feerate_sat_per_1000_weight,
-			is_initiator: self.is_initiator,
 			funding_tx_locktime: self.funding_tx_locktime,
 			inputs_to_contribute: self.our_funding_inputs,
 			shared_funding_input: self.shared_funding_input,
@@ -6619,7 +6617,11 @@ impl FundingNegotiationContext {
 			),
 			outputs_to_contribute: self.our_funding_outputs,
 		};
-		InteractiveTxConstructor::new(constructor_args)
+		if self.is_initiator {
+			InteractiveTxConstructor::new_for_outbound(constructor_args)
+		} else {
+			InteractiveTxConstructor::new_for_inbound(constructor_args)
+		}
 	}
 
 	fn into_contributed_inputs_and_outputs(self) -> (Vec<bitcoin::OutPoint>, Vec<TxOut>) {
@@ -12133,13 +12135,7 @@ where
 				&splice_funding,
 				entropy_source,
 				holder_node_id.clone(),
-			)
-			.map_err(|err| {
-				ChannelError::WarnAndDisconnect(format!(
-					"Failed to start interactive transaction construction, {:?}",
-					err
-				))
-			})?;
+			);
 		debug_assert!(interactive_tx_constructor.take_initiator_first_message().is_none());
 
 		let new_funding_pubkey = splice_funding.get_holder_pubkeys().funding_pubkey;
@@ -12193,13 +12189,7 @@ where
 				&splice_funding,
 				entropy_source,
 				holder_node_id.clone(),
-			)
-			.map_err(|err| {
-				ChannelError::WarnAndDisconnect(format!(
-					"Failed to start interactive transaction construction, {:?}",
-					err
-				))
-			})?;
+			);
 		let tx_msg_opt = interactive_tx_constructor.take_initiator_first_message();
 
 		debug_assert!(self.context.interactive_tx_signing_session.is_none());
@@ -14087,24 +14077,20 @@ impl<SP: SignerProvider> PendingV2Channel<SP> {
 			script_pubkey: funding.get_funding_redeemscript().to_p2wsh(),
 		};
 
-		let interactive_tx_constructor = Some(InteractiveTxConstructor::new(
+		let interactive_tx_constructor = Some(InteractiveTxConstructor::new_for_inbound(
 			InteractiveTxConstructorArgs {
 				entropy_source,
 				holder_node_id,
 				counterparty_node_id,
 				channel_id: context.channel_id,
 				feerate_sat_per_kw: funding_negotiation_context.funding_feerate_sat_per_1000_weight,
 				funding_tx_locktime: funding_negotiation_context.funding_tx_locktime,
-				is_initiator: false,
 				inputs_to_contribute: our_funding_inputs,
 				shared_funding_input: None,
 				shared_funding_output: SharedOwnedOutput::new(shared_funding_output, our_funding_contribution_sats),
 				outputs_to_contribute: funding_negotiation_context.our_funding_outputs.clone(),
 			}
-		).map_err(|err| {
-			let reason = ClosureReason::ProcessingError { err: err.reason.to_string() };
-			ChannelError::Close((err.reason.to_string(), reason))
-		})?);
+		));
 
 		let unfunded_context = UnfundedChannelContext {
 			unfunded_channel_age_ticks: 0,
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
@@ -2012,7 +2012,6 @@ pub(super) struct InteractiveTxConstructorArgs<'a, ES: EntropySource> {
 	pub counterparty_node_id: PublicKey,
 	pub channel_id: ChannelId,
 	pub feerate_sat_per_kw: u32,
-	pub is_initiator: bool,
 	pub funding_tx_locktime: AbsoluteLockTime,
 	pub inputs_to_contribute: Vec<FundingTxInput>,
 	pub shared_funding_input: Option<SharedOwnedInput>,
@@ -2023,18 +2022,15 @@ pub(super) struct InteractiveTxConstructorArgs<'a, ES: EntropySource> {
 impl InteractiveTxConstructor {
 	/// Instantiates a new `InteractiveTxConstructor`.
 	///
-	/// If the holder is the initiator, they need to send the first message which is a `TxAddInput`
-	/// message.
-	pub fn new<ES: EntropySource>(
-		args: InteractiveTxConstructorArgs<ES>,
-	) -> Result<Self, NegotiationError> {
+	/// Use [`Self::new_for_outbound`] or [`Self::new_for_inbound`] instead to also prepare the
+	/// first message for the initiator.
+	fn new<ES: EntropySource>(args: InteractiveTxConstructorArgs<ES>, is_initiator: bool) -> Self {
 		let InteractiveTxConstructorArgs {
 			entropy_source,
 			holder_node_id,
 			counterparty_node_id,
 			channel_id,
 			feerate_sat_per_kw,
-			is_initiator,
 			funding_tx_locktime,
 			inputs_to_contribute,
 			shared_funding_input,
@@ -2104,7 +2100,7 @@ impl InteractiveTxConstructor {
 		let next_input_index = (!inputs_to_contribute.is_empty()).then_some(0);
 		let next_output_index = (!outputs_to_contribute.is_empty()).then_some(0);
 
-		let mut constructor = Self {
+		Self {
 			state_machine,
 			is_initiator,
 			initiator_first_message: None,
@@ -2113,19 +2109,32 @@ impl InteractiveTxConstructor {
 			outputs_to_contribute,
 			next_input_index,
 			next_output_index,
-		};
-		// We'll store the first message for the initiator.
-		if is_initiator {
-			match constructor.maybe_send_message() {
-				Ok(message) => {
-					constructor.initiator_first_message = Some(message);
-				},
-				Err(reason) => {
-					return Err(constructor.into_negotiation_error(reason));
-				},
-			}
 		}
-		Ok(constructor)
+	}
+
+	/// Instantiates a new `InteractiveTxConstructor` for the initiator (outbound splice).
+	///
+	/// The initiator always has the shared funding output added internally, so preparing the
+	/// first message should never fail. Debug asserts verify this invariant.
+	pub fn new_for_outbound<ES: EntropySource>(args: InteractiveTxConstructorArgs<ES>) -> Self {
+		let mut constructor = Self::new(args, true);
+		match constructor.maybe_send_message() {
+			Ok(message) => constructor.initiator_first_message = Some(message),
+			Err(reason) => {
+				debug_assert!(
+					false,
+					"Outbound constructor should always have inputs: {:?}",
+					reason
+				);
+			},
+		}
+		constructor
+	}
+
+	/// Instantiates a new `InteractiveTxConstructor` for the non-initiator (inbound splice or
+	/// dual-funded channel acceptor).
+	pub fn new_for_inbound<ES: EntropySource>(args: InteractiveTxConstructorArgs<ES>) -> Self {
+		Self::new(args, false)
 	}
 
 	fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError {
@@ -2428,84 +2437,62 @@ mod tests {
 			&SecretKey::from_slice(&[43; 32]).unwrap(),
 		);
 
-		let mut constructor_a = match InteractiveTxConstructor::new(InteractiveTxConstructorArgs {
-			entropy_source,
-			channel_id,
-			feerate_sat_per_kw: TEST_FEERATE_SATS_PER_KW,
-			holder_node_id,
-			counterparty_node_id,
-			is_initiator: true,
-			funding_tx_locktime,
-			inputs_to_contribute: session.inputs_a,
-			shared_funding_input: session.a_shared_input.map(|(op, prev_output, lo)| {
-				SharedOwnedInput::new(
-					TxIn {
-						previous_output: op,
-						sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
-						..Default::default()
-					},
-					prev_output,
-					lo,
-					true,                             // holder_sig_first
-					generate_funding_script_pubkey(), // witness_script for test
-				)
-			}),
-			shared_funding_output: SharedOwnedOutput::new(
-				session.shared_output_a.0,
-				session.shared_output_a.1,
-			),
-			outputs_to_contribute: session.outputs_a,
-		}) {
-			Ok(r) => Some(r),
-			Err(e) => {
-				assert_eq!(
-					Some((e.reason, ErrorCulprit::NodeA)),
-					session.expect_error,
-					"Test: {}",
-					session.description
-				);
-				return;
-			},
-		};
-		let mut constructor_b = match InteractiveTxConstructor::new(InteractiveTxConstructorArgs {
-			entropy_source,
-			holder_node_id,
-			counterparty_node_id,
-			channel_id,
-			feerate_sat_per_kw: TEST_FEERATE_SATS_PER_KW,
-			is_initiator: false,
-			funding_tx_locktime,
-			inputs_to_contribute: session.inputs_b,
-			shared_funding_input: session.b_shared_input.map(|(op, prev_output, lo)| {
-				SharedOwnedInput::new(
-					TxIn {
-						previous_output: op,
-						sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
-						..Default::default()
-					},
-					prev_output,
-					lo,
-					false,                            // holder_sig_first
-					generate_funding_script_pubkey(), // witness_script for test
-				)
-			}),
-			shared_funding_output: SharedOwnedOutput::new(
-				session.shared_output_b.0,
-				session.shared_output_b.1,
-			),
-			outputs_to_contribute: session.outputs_b,
-		}) {
-			Ok(r) => Some(r),
-			Err(e) => {
-				assert_eq!(
-					Some((e.reason, ErrorCulprit::NodeB)),
-					session.expect_error,
-					"Test: {}",
-					session.description
-				);
-				return;
-			},
-		};
+		let mut constructor_a =
+			Some(InteractiveTxConstructor::new_for_outbound(InteractiveTxConstructorArgs {
+				entropy_source,
+				channel_id,
+				feerate_sat_per_kw: TEST_FEERATE_SATS_PER_KW,
+				holder_node_id,
+				counterparty_node_id,
+				funding_tx_locktime,
+				inputs_to_contribute: session.inputs_a,
+				shared_funding_input: session.a_shared_input.map(|(op, prev_output, lo)| {
+					SharedOwnedInput::new(
+						TxIn {
+							previous_output: op,
+							sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
+							..Default::default()
+						},
+						prev_output,
+						lo,
+						true,                             // holder_sig_first
+						generate_funding_script_pubkey(), // witness_script for test
+					)
+				}),
+				shared_funding_output: SharedOwnedOutput::new(
+					session.shared_output_a.0,
+					session.shared_output_a.1,
+				),
+				outputs_to_contribute: session.outputs_a,
+			}));
+		let mut constructor_b =
+			Some(InteractiveTxConstructor::new_for_inbound(InteractiveTxConstructorArgs {
+				entropy_source,
+				holder_node_id,
+				counterparty_node_id,
+				channel_id,
+				feerate_sat_per_kw: TEST_FEERATE_SATS_PER_KW,
+				funding_tx_locktime,
+				inputs_to_contribute: session.inputs_b,
+				shared_funding_input: session.b_shared_input.map(|(op, prev_output, lo)| {
+					SharedOwnedInput::new(
+						TxIn {
+							previous_output: op,
+							sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
+							..Default::default()
+						},
+						prev_output,
+						lo,
+						false,                            // holder_sig_first
+						generate_funding_script_pubkey(), // witness_script for test
+					)
+				}),
+				shared_funding_output: SharedOwnedOutput::new(
+					session.shared_output_b.0,
+					session.shared_output_b.1,
+				),
+				outputs_to_contribute: session.outputs_b,
+			}));
 
 		let handle_message_send =
 			|msg: InteractiveTxMessageSend, for_constructor: &mut InteractiveTxConstructor| {
