Support async signing of splice shared input

While user signatures may be provided whenever ready at the user's
discretion when handling a `FundingTransactionReadyForSigning` event, it
does not cover the user's signature for the 2-of-2 multisig input in a
splice. This signature is obtained via the `EcdsaChannelSigner`, which
did not support providing it asynchronously.

Since the splice shared input signature is part of the `tx_signatures`
message, we're not allowed to send the message until it's complete. This
results in us needing to explicitly handle the signature exchange logic
when the signer unblocks the shared input signature.

Author: wpaulino

lightning/src/ln/async_signer_tests.rs

@@ -1742,3 +1742,80 @@ fn test_async_splice_initial_commit_sig_waits_for_monitor_before_tx_signatures()
 	let _ = get_event!(initiator, Event::SpliceNegotiated);
 	let _ = get_event!(acceptor, Event::SpliceNegotiated);
 }
+
+#[test]
+fn test_async_splice_shared_input_signature_released_on_unblock() {
+	// Test that we can provide the signature of a splice's shared input asynchronously, and check
+	// that the holding cell is freed after exiting quiescence due to exchanging `tx_signatures`.
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
+
+	let (initiator, acceptor) = (&nodes[0], &nodes[1]);
+	let initiator_node_id = initiator.node.get_our_node_id();
+	let acceptor_node_id = acceptor.node.get_our_node_id();
+
+	initiator.disable_channel_signer_op(
+		&acceptor_node_id,
+		&channel_id,
+		SignerOp::SignSpliceSharedInput,
+	);
+
+	let outputs = vec![TxOut {
+		value: Amount::from_sat(1_000),
+		script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
+	}];
+	let contribution = initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap();
+	negotiate_splice_tx(initiator, acceptor, channel_id, contribution);
+
+	let event = get_event!(initiator, Event::FundingTransactionReadyForSigning);
+	if let Event::FundingTransactionReadyForSigning { unsigned_transaction, .. } = event {
+		let partially_signed_tx = initiator.wallet_source.sign_tx(unsigned_transaction).unwrap();
+		initiator
+			.node
+			.funding_transaction_signed(&channel_id, &acceptor_node_id, partially_signed_tx)
+			.unwrap();
+	}
+
+	let initiator_commit_sig = get_htlc_update_msgs(initiator, &acceptor_node_id);
+	acceptor
+		.node
+		.handle_commitment_signed(initiator_node_id, &initiator_commit_sig.commitment_signed[0]);
+	check_added_monitors(acceptor, 1);
+
+	let acceptor_msg_events = acceptor.node.get_and_clear_pending_msg_events();
+	assert_eq!(acceptor_msg_events.len(), 2, "{acceptor_msg_events:?}");
+	for msg_event in &acceptor_msg_events {
+		match msg_event {
+			MessageSendEvent::UpdateHTLCs { updates, .. } => {
+				initiator
+					.node
+					.handle_commitment_signed(acceptor_node_id, &updates.commitment_signed[0]);
+				check_added_monitors(initiator, 1);
+			},
+			MessageSendEvent::SendTxSignatures { msg, .. } => {
+				initiator.node.handle_tx_signatures(acceptor_node_id, msg);
+			},
+			_ => panic!("Unexpected event"),
+		}
+	}
+
+	assert!(initiator.node.get_and_clear_pending_msg_events().is_empty());
+
+	initiator.enable_channel_signer_op(
+		&acceptor_node_id,
+		&channel_id,
+		SignerOp::SignSpliceSharedInput,
+	);
+	initiator.node.signer_unblocked(None);
+
+	let tx_signatures =
+		get_event_msg!(initiator, MessageSendEvent::SendTxSignatures, acceptor_node_id);
+	acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures);
+
+	let _ = get_event!(initiator, Event::SpliceNegotiated);
+	let _ = get_event!(acceptor, Event::SpliceNegotiated);
+}

lightning/src/ln/channel.rs

@@ -1220,8 +1220,7 @@ pub(super) struct SignerResumeUpdates {
 	pub accept_channel: Option<msgs::AcceptChannel>,
 	pub funding_created: Option<msgs::FundingCreated>,
 	pub funding_signed: Option<msgs::FundingSigned>,
-	pub funding_commit_sig: Option<msgs::CommitmentSigned>,
-	pub tx_signatures: Option<msgs::TxSignatures>,
+	pub funding_tx_signed: Option<FundingTxSigned>,
 	pub channel_ready: Option<msgs::ChannelReady>,
 	pub order: RAACommitmentOrder,
 	pub closing_signed: Option<msgs::ClosingSigned>,
@@ -1654,11 +1653,11 @@ where
 
 	#[rustfmt::skip]
 	pub fn signer_maybe_unblocked<L: Logger, CBP>(
-		&mut self, chain_hash: ChainHash, logger: &L, path_for_release_htlc: CBP
+		&mut self, chain_hash: ChainHash, best_block_height: u32, logger: &L, path_for_release_htlc: CBP
 	) -> Result<Option<SignerResumeUpdates>, ChannelError> where CBP: Fn(u64) -> BlindedMessagePath {
 		match &mut self.phase {
 			ChannelPhase::Undefined => unreachable!(),
-			ChannelPhase::Funded(chan) => chan.signer_maybe_unblocked(logger, path_for_release_htlc).map(|r| Some(r)),
+			ChannelPhase::Funded(chan) => chan.signer_maybe_unblocked(best_block_height, logger, path_for_release_htlc).map(|r| Some(r)),
 			ChannelPhase::UnfundedOutboundV1(chan) => {
 				let (open_channel, funding_created) = chan.signer_maybe_unblocked(chain_hash, logger);
 				Ok(Some(SignerResumeUpdates {
@@ -1668,8 +1667,7 @@ where
 					accept_channel: None,
 					funding_created,
 					funding_signed: None,
-					funding_commit_sig: None,
-					tx_signatures: None,
+					funding_tx_signed: None,
 					channel_ready: None,
 					order: chan.context.resend_order.clone(),
 					closing_signed: None,
@@ -1686,8 +1684,7 @@ where
 					accept_channel,
 					funding_created: None,
 					funding_signed: None,
-					funding_commit_sig: None,
-					tx_signatures: None,
+					funding_tx_signed: None,
 					channel_ready: None,
 					order: chan.context.resend_order.clone(),
 					closing_signed: None,
@@ -2188,9 +2185,7 @@ where
 					.unwrap_or(false));
 			}
 
-			if signing_session.has_holder_tx_signatures() {
-				// Our `tx_signatures` either should've been the first time we processed them,
-				// or we're waiting for our counterparty to send theirs first.
+			if signing_session.has_holder_tx_witnesses() {
 				return Ok(FundingTxSigned {
 					commitment_signed: None,
 					counterparty_initial_commitment_signed_result: None,
@@ -2219,36 +2214,42 @@ where
 			return Err(APIError::APIMisuseError { err });
 		};
 
-		let tx = signing_session.unsigned_tx().tx();
-		if funding_txid_signed != tx.compute_txid() {
-			return Err(APIError::APIMisuseError {
-				err: "Transaction was malleated prior to signing".to_owned(),
-			});
-		}
+		let (mut tx_signatures, mut funding_tx) = signing_session
+			.provide_holder_witnesses(
+				context.channel_id,
+				funding_txid_signed,
+				witnesses,
+				&context.secp_ctx,
+			)
+			.map_err(|err| APIError::APIMisuseError { err })?;
 
-		let shared_input_signature =
-			if let Some(splice_input_index) = signing_session.unsigned_tx().shared_input_index() {
-				let sig = context.holder_signer.sign_splice_shared_input(
+		debug_assert_eq!(
+			pending_splice.is_some(),
+			signing_session.unsigned_tx().shared_input_index().is_some()
+		);
+		if let Some(splice_input_index) = signing_session.unsigned_tx().shared_input_index() {
+			let sig = context
+				.holder_signer
+				.sign_splice_shared_input(
 					&funding.channel_transaction_parameters,
-					tx,
+					signing_session.unsigned_tx().tx(),
 					splice_input_index as usize,
 					&context.secp_ctx,
-				);
-				Some(sig)
+				)
+				.ok();
+			if let Some(sig) = sig {
+				(tx_signatures, funding_tx) = signing_session
+					.provide_holder_shared_input_signature(sig)
+					.map_err(|err| APIError::APIMisuseError { err })?;
 			} else {
-				None
-			};
-		debug_assert_eq!(pending_splice.is_some(), shared_input_signature.is_some());
-
-		let tx_signatures = msgs::TxSignatures {
-			channel_id: context.channel_id,
-			tx_hash: funding_txid_signed,
-			witnesses,
-			shared_input_signature,
-		};
-		let (tx_signatures, funding_tx) = signing_session
-			.provide_holder_witnesses(tx_signatures, &context.secp_ctx)
-			.map_err(|err| APIError::APIMisuseError { err })?;
+				log_debug!(
+					logger,
+					"Splice shared input signature not available, waiting on async signer"
+				);
+				debug_assert!(tx_signatures.is_none());
+				debug_assert!(funding_tx.is_none());
+			}
+		}
 
 		let logger = WithChannelContext::from(logger, &context, None);
 		if tx_signatures.is_some() {
@@ -2380,18 +2381,17 @@ where
 					// which must always come after the initial commitment signed is sent.
 					.unwrap_or(true);
 				let res = if has_negotiated_pending_splice && !session_received_commitment_signed {
-					let has_holder_tx_signatures = funded_channel
+					let has_holder_tx_witnesses = funded_channel
 						.context
 						.interactive_tx_signing_session
 						.as_ref()
-						.map(|session| session.has_holder_tx_signatures())
+						.map(|session| session.has_holder_tx_witnesses())
 						.unwrap_or(false);
 
 					// We delay processing this until the user manually approves the splice via
-					// [`Channel::funding_transaction_signed`], as otherwise, there would be a
-					// [`ChannelMonitorUpdateStep::RenegotiatedFunding`] committed that we would
-					// need to undo if they no longer wish to proceed.
-					if has_holder_tx_signatures {
+					// [`Channel::funding_transaction_signed`], as otherwise, it would prevent the
+					// user from canceling their contribution if they no longer wish to proceed.
+					if has_holder_tx_witnesses {
 						funded_channel
 							.splice_initial_commitment_signed(msg, fee_estimator, logger)
 							.map(|monitor_update_opt| (None, monitor_update_opt))
@@ -5135,7 +5135,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 			ChannelState::FundingNegotiated(_) => self
 				.interactive_tx_signing_session
 				.as_ref()
-				.map(|signing_session| signing_session.has_holder_tx_signatures())
+				.map(|signing_session| signing_session.has_holder_tx_witnesses())
 				.unwrap_or(false),
 			ChannelState::AwaitingChannelReady(flags) => !flags.is_waiting_for_batch(),
 			_ => true,
@@ -7847,7 +7847,7 @@ where
 			.interactive_tx_signing_session
 			.as_ref()
 			.map(|signing_session| {
-				signing_session.has_holder_tx_signatures()
+				signing_session.has_holder_tx_witnesses()
 					|| signing_session.has_received_tx_signatures()
 			})
 			.unwrap_or(false);
@@ -9514,6 +9514,8 @@ where
 			}
 		}
 
+		let awaiting_holder_shared_input_signature =
+			signing_session.awaiting_holder_shared_input_signature();
 		let (holder_tx_signatures, funding_tx) =
 			signing_session.received_tx_signatures(msg).map_err(|msg| ChannelError::Warn(msg))?;
 
@@ -9552,6 +9554,11 @@ where
 				best_block_height,
 				&logger,
 			);
+		} else if awaiting_holder_shared_input_signature {
+			log_debug!(
+				logger,
+				"Waiting for funding transaction shared input signature before finalizing negotiation"
+			);
 		} else {
 			debug_assert!(
 				false,
@@ -9946,7 +9953,7 @@ where
 	/// blocked.
 	#[rustfmt::skip]
 	pub fn signer_maybe_unblocked<L: Logger, CBP>(
-		&mut self, logger: &L, path_for_release_htlc: CBP
+		&mut self, best_block_height: u32, logger: &L, path_for_release_htlc: CBP
 	) -> Result<SignerResumeUpdates, ChannelError> where CBP: Fn(u64) -> BlindedMessagePath {
 		if let Some((commitment_number, commitment_secret)) = self.context.signer_pending_stale_state_verification.clone() {
 			if let Ok(expected_point) = self
@@ -10002,16 +10009,65 @@ where
 			None
 		};
 
-		let tx_signatures = if funding_commit_sig.is_some() {
+		let mut shared_input_signature_unblocked = false;
+		{
+			if let Some(signing_session) = self.context.interactive_tx_signing_session.as_mut() {
+				if signing_session.awaiting_holder_shared_input_signature() {
+					let splice_input_index = signing_session
+						.unsigned_tx()
+						.shared_input_index()
+						.expect("Missing shared input index while awaiting a splice signature");
+					log_trace!(logger, "Attempting to generate pending splice shared input signature...");
+					if let Ok(shared_input_signature) = self.context.holder_signer.sign_splice_shared_input(
+						&self.funding.channel_transaction_parameters,
+						signing_session.unsigned_tx().tx(),
+						splice_input_index as usize,
+						&self.context.secp_ctx,
+					) {
+						shared_input_signature_unblocked = true;
+						signing_session
+							.provide_holder_shared_input_signature(shared_input_signature)
+							.map_err(ChannelError::close)?;
+					}
+				}
+			}
+		}
+
+		let mut tx_signatures = None;
+		let mut funding_tx = None;
+		if funding_commit_sig.is_some() || shared_input_signature_unblocked {
 			if let Some(signing_session) = self.context.interactive_tx_signing_session.as_ref() {
-				signing_session.holder_tx_signatures().filter(|_| !self.is_awaiting_monitor_update())
+				if !self.is_awaiting_monitor_update() && !self.context.signer_pending_funding {
+					tx_signatures = signing_session.holder_tx_signatures();
+					funding_tx = tx_signatures.as_ref().and_then(|_| signing_session.signed_tx());
+				}
 			} else {
 				debug_assert!(false);
-				None
 			}
-		} else {
-			None
-		};
+		}
+
+		let mut funding_tx_signed = None;
+		if funding_commit_sig.is_some() || tx_signatures.is_some() || funding_tx.is_some() {
+			let mut resumed = FundingTxSigned {
+				commitment_signed: funding_commit_sig,
+				counterparty_initial_commitment_signed_result: None,
+				tx_signatures,
+				funding_tx: None,
+				splice_negotiated: None,
+				splice_locked: None,
+			};
+			if let Some(funding_tx) = funding_tx {
+				let funding_logger = WithChannelContext::from(logger, &self.context, None);
+				debug_assert!(resumed.tx_signatures.is_some());
+				self.on_tx_signatures_exchange(
+					&mut resumed,
+					funding_tx,
+					best_block_height,
+					&funding_logger,
+				);
+			}
+			funding_tx_signed = Some(resumed);
+		}
 
 		// Provide a `channel_ready` message if we need to, but only if we're _not_ still pending
 		// funding.
@@ -10077,8 +10133,8 @@ where
 			if revoke_and_ack.is_some() { "a" } else { "no" },
 			self.context.resend_order,
 			if funding_signed.is_some() { "a" } else { "no" },
-			if funding_commit_sig.is_some() { "a" } else { "no" },
-			if tx_signatures.is_some() { "a" } else { "no" },
+			if funding_tx_signed.as_ref().map(|v| v.commitment_signed.is_some()).unwrap_or(false) { "a" } else { "no" },
+			if funding_tx_signed.as_ref().map(|v| v.tx_signatures.is_some()).unwrap_or(false) { "a" } else { "no" },
 			if channel_ready.is_some() { "a" } else { "no" },
 			if closing_signed.is_some() { "a" } else { "no" },
 			if signed_closing_tx.is_some() { "a" } else { "no" },
@@ -10091,8 +10147,7 @@ where
 			accept_channel: None,
 			funding_created: None,
 			funding_signed,
-			funding_commit_sig,
-			tx_signatures,
+			funding_tx_signed,
 			channel_ready,
 			order: self.context.resend_order.clone(),
 			closing_signed,
@@ -10442,7 +10497,7 @@ where
 					} else {
 						tx_signatures = Some(holder_tx_signatures);
 					}
-				} else if !session.has_holder_tx_signatures() {
+				} else if !session.has_holder_tx_witnesses() {
 					log_debug!(logger, "Waiting for funding transaction signatures to be provided");
 				}
 			} else {
@@ -10878,7 +10933,7 @@ where
 			matches!(self.context.channel_state, ChannelState::NegotiatingFunding(_));
 		if matches!(self.context.channel_state, ChannelState::FundingNegotiated(_)) {
 			if let Some(signing_session) = self.context.interactive_tx_signing_session.as_ref() {
-				if !signing_session.has_holder_tx_signatures() {
+				if !signing_session.has_holder_tx_witnesses() {
 					// If we're a V1 channel or we haven't yet sent our `tx_signatures` for a dual
 					// funded channel, the funding tx couldn't be broadcasted yet, so just short-circuit
 					// the shutdown logic.