Introduce interactive signing state flags

This intoduces the INTERACTIVE_SIGNING, THEIR_TX_SIGNATURES_SENT, and
OUR_TX_SIGNATURES_SENT funded state flags.

A top-level state flag for INTERACTIVE_SIGNING was avoided so that this
work is compatible with splicing as well as V2 channel establishment
(dual-funding).

Author: dunxen

lightning/src/ln/channel.rs

@@ -479,6 +479,9 @@ mod state_flags {
 	pub const LOCAL_STFU_SENT: u32 = 1 << 15;
 	pub const REMOTE_STFU_SENT: u32 = 1 << 16;
 	pub const QUIESCENT: u32 = 1 << 17;
+	pub const INTERACTIVE_SIGNING: u32 = 1 << 18;
+	pub const OUR_TX_SIGNATURES_SENT: u32 = 1 << 19;
+	pub const THEIR_TX_SIGNATURES_SENT: u32 = 1 << 20;
 }
 
 define_state_flags!(
@@ -511,6 +514,21 @@ define_state_flags!(
 	]
 );
 
+define_state_flags!(
+	"Flags that only apply to [`ChannelState::FundingNegotiated`].",
+	FUNDED_STATE, FundingNegotiatedFlags, [
+		("Indicates we have an active interactive signing session for an interactive transaction",
+			INTERACTIVE_SIGNING, state_flags::INTERACTIVE_SIGNING,
+			is_interactive_signing, set_interactive_signing, clear_interactive_signing),
+		("Indicates they sent us a `tx_signatures` message.",
+			THEIR_TX_SIGNATURES_SENT, state_flags::THEIR_TX_SIGNATURES_SENT,
+			is_their_tx_signatures_sent, set_their_tx_signatures_sent, clear_their_tx_signatures_sent),
+		("Indicates we sent them a `tx_signatures` message.",
+			OUR_TX_SIGNATURES_SENT, state_flags::OUR_TX_SIGNATURES_SENT,
+			is_our_tx_signatures_sent, set_our_tx_signatures_sent, clear_our_tx_signatures_sent)
+	]
+);
+
 define_state_flags!(
 	"Flags that only apply to [`ChannelState::AwaitingChannelReady`].",
 	FUNDED_STATE, AwaitingChannelReadyFlags, [
@@ -568,8 +586,9 @@ enum ChannelState {
 	NegotiatingFunding(NegotiatingFundingFlags),
 	/// We have sent `funding_created` and are awaiting a `funding_signed` to advance to
 	/// `AwaitingChannelReady`. Note that this is nonsense for an inbound channel as we immediately generate
-	/// `funding_signed` upon receipt of `funding_created`, so simply skip this state.
-	FundingNegotiated,
+	/// `funding_signed` upon receipt of `funding_created`, so simply skip this state. For dual-funded (V2)
+	/// channels, this state is also skipped.
+	FundingNegotiated(FundingNegotiatedFlags),
 	/// We've received/sent `funding_created` and `funding_signed` and are thus now waiting on the
 	/// funding transaction to confirm.
 	AwaitingChannelReady(AwaitingChannelReadyFlags),
@@ -612,7 +631,7 @@ macro_rules! impl_state_flag {
 		}
 	};
 	($get: ident, $set: ident, $clear: ident, FUNDED_STATES) => {
-		impl_state_flag!($get, $set, $clear, [AwaitingChannelReady, ChannelReady]);
+		impl_state_flag!($get, $set, $clear, [FundingNegotiated, AwaitingChannelReady, ChannelReady]);
 	};
 	($get: ident, $set: ident, $clear: ident, $state: ident) => {
 		impl_state_flag!($get, $set, $clear, [$state]);
@@ -622,10 +641,12 @@ macro_rules! impl_state_flag {
 impl ChannelState {
 	fn from_u32(state: u32) -> Result<Self, ()> {
 		match state {
-			state_flags::FUNDING_NEGOTIATED => Ok(ChannelState::FundingNegotiated),
 			state_flags::SHUTDOWN_COMPLETE => Ok(ChannelState::ShutdownComplete),
 			val => {
-				if val & state_flags::AWAITING_CHANNEL_READY == state_flags::AWAITING_CHANNEL_READY {
+				if val & state_flags::FUNDING_NEGOTIATED == state_flags::FUNDING_NEGOTIATED {
+					FundingNegotiatedFlags::from_u32(val & !state_flags::FUNDING_NEGOTIATED)
+						.map(|flags| ChannelState::FundingNegotiated(flags))
+				} else if val & state_flags::AWAITING_CHANNEL_READY == state_flags::AWAITING_CHANNEL_READY {
 					AwaitingChannelReadyFlags::from_u32(val & !state_flags::AWAITING_CHANNEL_READY)
 						.map(|flags| ChannelState::AwaitingChannelReady(flags))
 				} else if val & state_flags::CHANNEL_READY == state_flags::CHANNEL_READY {
@@ -643,15 +664,15 @@ impl ChannelState {
 	fn to_u32(self) -> u32 {
 		match self {
 			ChannelState::NegotiatingFunding(flags) => flags.0,
-			ChannelState::FundingNegotiated => state_flags::FUNDING_NEGOTIATED,
+			ChannelState::FundingNegotiated(flags) => state_flags::FUNDING_NEGOTIATED | flags.0,
 			ChannelState::AwaitingChannelReady(flags) => state_flags::AWAITING_CHANNEL_READY | flags.0,
 			ChannelState::ChannelReady(flags) => state_flags::CHANNEL_READY | flags.0,
 			ChannelState::ShutdownComplete => state_flags::SHUTDOWN_COMPLETE,
 		}
 	}
 
 	fn is_pre_funded_state(&self) -> bool {
-		matches!(self, ChannelState::NegotiatingFunding(_)|ChannelState::FundingNegotiated)
+		matches!(self, ChannelState::NegotiatingFunding(_)|ChannelState::FundingNegotiated(_))
 	}
 
 	fn is_both_sides_shutdown(&self) -> bool {
@@ -685,6 +706,9 @@ impl ChannelState {
 	impl_state_flag!(is_monitor_update_in_progress, set_monitor_update_in_progress, clear_monitor_update_in_progress, FUNDED_STATES);
 	impl_state_flag!(is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent, FUNDED_STATES);
 	impl_state_flag!(is_remote_shutdown_sent, set_remote_shutdown_sent, clear_remote_shutdown_sent, FUNDED_STATES);
+	impl_state_flag!(is_interactive_signing, set_interactive_signing, clear_interactive_signing, FundingNegotiated);
+	impl_state_flag!(is_our_tx_signatures_sent, set_our_tx_signatures_sent, clear_our_tx_signatures_sent, FundingNegotiated);
+	impl_state_flag!(is_their_tx_signatures_sent, set_their_tx_signatures_sent, clear_their_tx_signatures_sent, FundingNegotiated);
 	impl_state_flag!(is_our_channel_ready, set_our_channel_ready, clear_our_channel_ready, AwaitingChannelReady);
 	impl_state_flag!(is_their_channel_ready, set_their_channel_ready, clear_their_channel_ready, AwaitingChannelReady);
 	impl_state_flag!(is_waiting_for_batch, set_waiting_for_batch, clear_waiting_for_batch, AwaitingChannelReady);
@@ -2101,10 +2125,12 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide
 		context.channel_id = channel_id;
 
 		assert!(!context.channel_state.is_monitor_update_in_progress()); // We have not had any monitor(s) yet to fail update!
-		if context.is_batch_funding() {
-			context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::WAITING_FOR_BATCH);
-		} else {
-			context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
+		if !context.channel_state.is_interactive_signing() {
+			if context.is_batch_funding() {
+				context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::WAITING_FOR_BATCH);
+			} else {
+				context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
+			}
 		}
 		if holder_commitment_point.advance(&context.holder_signer, &context.secp_ctx, logger).is_err() {
 			// We only fail to advance our commitment point/number if we're currently
@@ -2356,7 +2382,9 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 			)));
 		};
 
-		self.context.channel_state = ChannelState::FundingNegotiated;
+		let mut channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
+		channel_state.set_interactive_signing();
+		self.context.channel_state = channel_state;
 
 		// Clear the interactive transaction constructor
 		self.interactive_tx_constructor.take();
@@ -3447,7 +3475,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 
 	fn unset_funding_info(&mut self, funding: &mut FundingScope) {
 		debug_assert!(
-			matches!(self.channel_state, ChannelState::FundingNegotiated)
+			matches!(self.channel_state, ChannelState::FundingNegotiated(_))
 				|| matches!(self.channel_state, ChannelState::AwaitingChannelReady(_))
 		);
 		funding.channel_transaction_parameters.funding_outpoint = None;
@@ -4411,7 +4439,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 
 	fn if_unbroadcasted_funding<F, O>(&self, f: F) -> Option<O> where F: Fn() -> Option<O> {
 		match self.channel_state {
-			ChannelState::FundingNegotiated => f(),
+			ChannelState::FundingNegotiated(_) => f(),
 			ChannelState::AwaitingChannelReady(flags) =>
 				if flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH) ||
 					flags.is_set(FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS.into())
@@ -5622,7 +5650,7 @@ impl<SP: Deref> FundedChannel<SP> where
 	) -> Result<ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>, ChannelError>
 	where L::Target: Logger
 	{
-		if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) {
+		if !matches!(self.context.channel_state, ChannelState::FundingNegotiated(_)) {
 			return Err(ChannelError::Close(
 				(
 					"Received initial commitment_signed before funding transaction constructed!".to_owned(),
@@ -5639,7 +5667,6 @@ impl<SP: Deref> FundedChannel<SP> where
 		log_info!(logger, "Received initial commitment_signed from peer for channel {}", &self.context.channel_id());
 
 		let need_channel_ready = self.check_get_channel_ready(0, logger).is_some();
-		self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
 		self.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
 
 		if let Some(tx_signatures) = self.interactive_tx_signing_session.as_mut().and_then(
@@ -6226,10 +6253,10 @@ impl<SP: Deref> FundedChannel<SP> where
 		}
 	}
 
-	pub fn tx_signatures<L: Deref>(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<Option<msgs::TxSignatures>, ChannelError>
+	pub fn tx_signatures<L: Deref>(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<(Option<Transaction>, Option<msgs::TxSignatures>), ChannelError>
 		where L::Target: Logger
 	{
-		if !matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(_)) {
+		if !matches!(self.context.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing()) {
 			return Err(ChannelError::close("Received tx_signatures in strange state!".to_owned()));
 		}
 
@@ -6266,21 +6293,28 @@ impl<SP: Deref> FundedChannel<SP> where
 			let (holder_tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg.clone())
 				.map_err(|_| ChannelError::Warn("Witness count did not match contributed input count".to_string()))?;
 
+			// Set `THEIR_TX_SIGNATURES_SENT` flag after all potential errors.
+			self.context.channel_state.set_their_tx_signatures_sent();
 
 			if funding_tx_opt.is_some() {
 				// We have a finalized funding transaction, so we can set the funding transaction and reset the
 				// signing session fields.
-				self.funding.funding_transaction = funding_tx_opt;
+				self.funding.funding_transaction = funding_tx_opt.clone();
 				self.interactive_tx_signing_session = None;
 			}
 
 			if holder_tx_signatures_opt.is_some() && self.is_awaiting_initial_mon_persist() {
 				log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures.");
 				self.context.monitor_pending_tx_signatures = holder_tx_signatures_opt;
-				return Ok(None);
+				return Ok((None, None));
 			}
 
-			Ok(holder_tx_signatures_opt)
+			if holder_tx_signatures_opt.is_some() {
+				self.context.channel_state.set_our_tx_signatures_sent();
+			}
+
+			self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
+			Ok((funding_tx_opt, holder_tx_signatures_opt))
 		} else {
 			Err(ChannelError::Close((
 				"Unexpected tx_signatures. No funding transaction awaiting signatures".to_string(),
@@ -6541,6 +6575,9 @@ impl<SP: Deref> FundedChannel<SP> where
 		// MonitorUpdateInProgress (and we assume the user will never directly broadcast the funding
 		// transaction and waits for us to do it).
 		let tx_signatures = self.context.monitor_pending_tx_signatures.take();
+		if tx_signatures.is_some() {
+			self.context.channel_state.set_our_tx_signatures_sent();
+		}
 
 		if self.context.channel_state.is_peer_disconnected() {
 			self.context.monitor_pending_revoke_and_ack = false;
@@ -7011,7 +7048,7 @@ impl<SP: Deref> FundedChannel<SP> where
 					if session.unsigned_tx().compute_txid() == next_funding_txid {
 						debug_assert_eq!(session.unsigned_tx().compute_txid(), self.maybe_get_next_funding_txid().unwrap());
 
-						let commitment_update = if !session.counterparty_sent_tx_signatures() && msg.next_local_commitment_number == 0 {
+						let commitment_update = if !self.context.channel_state.is_their_tx_signatures_sent() && msg.next_local_commitment_number == 0 {
 							// if it has not received tx_signatures for that funding transaction AND
 							// if next_commitment_number is zero:
 							//   MUST retransmit its commitment_signed for that funding transaction.
@@ -7030,10 +7067,10 @@ impl<SP: Deref> FundedChannel<SP> where
 							// if it has not received tx_signatures for that funding transaction AND
 							// if it has already received commitment_signed AND it should sign first, as specified in the tx_signatures requirements:
 							//   MUST send its tx_signatures for that funding transaction.
-							!session.counterparty_sent_tx_signatures() && session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first()
+							!self.context.channel_state.is_their_tx_signatures_sent() && session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first()
 							// else if it has already received tx_signatures for that funding transaction:
 							//   MUST send its tx_signatures for that funding transaction.
-						) || session.counterparty_sent_tx_signatures() {
+						) || self.context.channel_state.is_their_tx_signatures_sent() {
 							if self.context.channel_state.is_monitor_update_in_progress() {
 								// The `monitor_pending_tx_signatures` field should have already been set in `commitment_signed_initial_v2`
 								// if we were up first for signing and had a monitor update in progress, but check again just in case.
@@ -8407,9 +8444,9 @@ impl<SP: Deref> FundedChannel<SP> where
 		// If we've sent `commtiment_signed` for an interactively constructed transaction
 		// during a signing session, but have not received `tx_signatures` we MUST set `next_funding_txid`
 		// to the txid of that interactive transaction, else we MUST NOT set it.
-		if let Some(signing_session) = &self.interactive_tx_signing_session {
+		if self.context.channel_state.is_interactive_signing() {
 			// Since we have a signing_session, this implies we've sent an initial `commitment_signed`...
-			if !signing_session.counterparty_sent_tx_signatures() {
+			if !self.context.channel_state.is_their_tx_signatures_sent() {
 				// ...but we didn't receive a `tx_signatures` from the counterparty yet.
 				Some(self.funding_outpoint().txid)
 			} else {
@@ -9391,7 +9428,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
 
 		// Now that we're past error-generating stuff, update our local state:
 
-		self.context.channel_state = ChannelState::FundingNegotiated;
+		self.context.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
 		self.context.channel_id = ChannelId::v1_from_funding_outpoint(funding_txo);
 
 		// If the funding transaction is a coinbase transaction, we need to set the minimum depth to 100.
@@ -9508,7 +9545,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
 		if !self.funding.is_outbound() {
 			return Err((self, ChannelError::close("Received funding_signed for an inbound channel?".to_owned())));
 		}
-		if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) {
+		if !matches!(self.context.channel_state, ChannelState::FundingNegotiated(_)) {
 			return Err((self, ChannelError::close("Received funding_signed in strange state!".to_owned())));
 		}
 		let mut holder_commitment_point = match self.unfunded_context.holder_commitment_point {
@@ -11236,11 +11273,12 @@ mod tests {
 	#[test]
 	fn test_channel_state_order() {
 		use crate::ln::channel::NegotiatingFundingFlags;
+		use crate::ln::channel::FundingNegotiatedFlags;
 		use crate::ln::channel::AwaitingChannelReadyFlags;
 		use crate::ln::channel::ChannelReadyFlags;
 
-		assert!(ChannelState::NegotiatingFunding(NegotiatingFundingFlags::new()) < ChannelState::FundingNegotiated);
-		assert!(ChannelState::FundingNegotiated < ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()));
+		assert!(ChannelState::NegotiatingFunding(NegotiatingFundingFlags::new()) < ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()));
+		assert!(ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()) < ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()));
 		assert!(ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()) < ChannelState::ChannelReady(ChannelReadyFlags::new()));
 		assert!(ChannelState::ChannelReady(ChannelReadyFlags::new()) < ChannelState::ShutdownComplete);
 	}

lightning/src/ln/channelmanager.rs

@@ -8573,14 +8573,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 				match chan_entry.get_mut().as_funded_mut() {
 					Some(chan) => {
 						let logger = WithChannelContext::from(&self.logger, &chan.context, None);
-						let tx_signatures_opt = try_channel_entry!(self, peer_state, chan.tx_signatures(msg, &&logger), chan_entry);
+						let (funding_tx_opt, tx_signatures_opt) = try_channel_entry!(self, peer_state, chan.tx_signatures(msg, &&logger), chan_entry);
 						if let Some(tx_signatures) = tx_signatures_opt {
 							peer_state.pending_msg_events.push(MessageSendEvent::SendTxSignatures {
 								node_id: *counterparty_node_id,
 								msg: tx_signatures,
 							});
 						}
-						if let Some(ref funding_tx) = chan.context.unbroadcasted_funding(&chan.funding) {
+						if let Some(ref funding_tx) = funding_tx_opt {
 							self.tx_broadcaster.broadcast_transactions(&[funding_tx]);
 							{
 								let mut pending_events = self.pending_events.lock().unwrap();

lightning/src/ln/dual_funding_tests.rs

@@ -208,13 +208,6 @@ fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession)
 	assert!(events.is_empty());
 	nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id);
 
-	let events = nodes[1].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	match events[0] {
-		Event::ChannelPending { channel_id: chan_id, .. } => assert_eq!(chan_id, channel_id),
-		_ => panic!("Unexpected event"),
-	};
-
 	let tx_signatures_msg = get_event_msg!(
 		nodes[1],
 		MessageSendEvent::SendTxSignatures,
@@ -236,6 +229,13 @@ fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession)
 		},
 	);
 
+	let events = nodes[1].node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1);
+	match events[0] {
+		Event::ChannelPending { channel_id: chan_id, .. } => assert_eq!(chan_id, channel_id),
+		_ => panic!("Unexpected event"),
+	};
+
 	// For an inbound channel V2 channel the transaction should be broadcast once receiving a
 	// tx_signature and applying local tx_signatures:
 	let broadcasted_txs = nodes[1].tx_broadcaster.txn_broadcast();

lightning/src/ln/interactivetxs.rs

@@ -289,7 +289,6 @@ impl ConstructedTransaction {
 #[derive(Debug, Clone, PartialEq)]
 pub(crate) struct InteractiveTxSigningSession {
 	unsigned_tx: ConstructedTransaction,
-	counterparty_sent_tx_signatures: bool,
 	holder_sends_tx_signatures_first: bool,
 	has_received_commitment_signed: bool,
 	holder_tx_signatures: Option<TxSignatures>,
@@ -300,10 +299,6 @@ impl InteractiveTxSigningSession {
 		&self.unsigned_tx
 	}
 
-	pub fn counterparty_sent_tx_signatures(&self) -> bool {
-		self.counterparty_sent_tx_signatures
-	}
-
 	pub fn holder_sends_tx_signatures_first(&self) -> bool {
 		self.holder_sends_tx_signatures_first
 	}
@@ -325,14 +320,6 @@ impl InteractiveTxSigningSession {
 		}
 	}
 
-	pub fn get_tx_signatures(&self) -> Option<TxSignatures> {
-		if self.has_received_commitment_signed {
-			self.holder_tx_signatures.clone()
-		} else {
-			None
-		}
-	}
-
 	/// Handles a `tx_signatures` message received from the counterparty.
 	///
 	/// If the holder is required to send their `tx_signatures` message and these signatures have
@@ -351,7 +338,6 @@ impl InteractiveTxSigningSession {
 			return Err(());
 		}
 		self.unsigned_tx.add_remote_witnesses(tx_signatures.witnesses.clone());
-		self.counterparty_sent_tx_signatures = true;
 
 		let holder_tx_signatures = if !self.holder_sends_tx_signatures_first {
 			self.holder_tx_signatures.clone()
@@ -1009,7 +995,6 @@ macro_rules! define_state_transitions {
 					unsigned_tx: tx,
 					has_received_commitment_signed: false,
 					holder_tx_signatures: None,
-					counterparty_sent_tx_signatures: false,
 				};
 				Ok(NegotiationComplete(signing_session))
 			}