Introduce interactive signing state flags and session persistence

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

@@ -434,6 +434,12 @@ macro_rules! define_state_flags {
 			is_remote_shutdown_sent, set_remote_shutdown_sent, clear_remote_shutdown_sent);
 		define_state_flags!($flag_type, FundedStateFlags::LOCAL_SHUTDOWN_SENT,
 			is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent);
+		define_state_flags!($flag_type, FundedStateFlags::INTERACTIVE_SIGNING,
+			is_interactive_signing, set_interactive_signing, clear_interactive_signing);
+		define_state_flags!($flag_type, FundedStateFlags::THEIR_TX_SIGNATURES_SENT,
+			is_their_tx_signatures_sent, set_their_tx_signatures_sent, clear_their_tx_signatures_sent);
+		define_state_flags!($flag_type, FundedStateFlags::OUR_TX_SIGNATURES_SENT,
+			is_our_tx_signatures_sent, set_our_tx_signatures_sent, clear_our_tx_signatures_sent);
 
 		impl core::ops::BitOr<FundedStateFlags> for $flag_type {
 			type Output = Self;
@@ -479,6 +485,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!(
@@ -497,7 +506,16 @@ define_state_flags!(
 			is_remote_shutdown_sent, set_remote_shutdown_sent, clear_remote_shutdown_sent),
 		("Indicates we sent a `shutdown` message. At this point, we may not add any new HTLCs to \
 			the channel.", LOCAL_SHUTDOWN_SENT, state_flags::LOCAL_SHUTDOWN_SENT,
-			is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent)
+			is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent),
+		("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_singing),
+		("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)
 	]
 );
 
@@ -568,7 +586,8 @@ 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.
+	/// `funding_signed` upon receipt of `funding_created`, so simply skip this state. For dual-funded (V2)
+	/// channels, this state is also skipped.
 	FundingNegotiated,
 	/// We've received/sent `funding_created` and `funding_signed` and are thus now waiting on the
 	/// funding transaction to confirm.
@@ -685,6 +704,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, FUNDED_STATES);
+	impl_state_flag!(is_our_tx_signatures_sent, set_our_tx_signatures_sent, clear_our_tx_signatures_sent, FUNDED_STATES);
+	impl_state_flag!(is_their_tx_signatures_sent, set_their_tx_signatures_sent, clear_their_tx_signatures_sent, FUNDED_STATES);
 	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);
@@ -2094,10 +2116,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
@@ -2358,7 +2382,9 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 			)));
 		};
 
-		self.context.channel_state = ChannelState::FundingNegotiated;
+		let mut channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
+		channel_state.set_interactive_signing();
+		self.context.channel_state = channel_state;
 
 		// Clear the interactive transaction constructor
 		self.interactive_tx_constructor.take();
@@ -4441,7 +4467,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			ChannelState::FundingNegotiated => f(),
 			ChannelState::AwaitingChannelReady(flags) =>
 				if flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH) ||
-					flags.is_set(FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS.into())
+					flags.is_set(FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS.into()) ||
+					(flags.is_set(FundedStateFlags::THEIR_TX_SIGNATURES_SENT.into()) && flags.is_set(FundedStateFlags::OUR_TX_SIGNATURES_SENT.into()))
 				{
 					f()
 				} else {
@@ -5648,7 +5675,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::AwaitingChannelReady(_)) {
 			return Err(ChannelError::Close(
 				(
 					"Received initial commitment_signed before funding transaction constructed!".to_owned(),
@@ -5666,7 +5693,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(
@@ -6256,7 +6282,7 @@ impl<SP: Deref> FundedChannel<SP> where
 	pub fn tx_signatures<L: Deref>(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<Option<msgs::TxSignatures>, ChannelError>
 		where L::Target: Logger
 	{
-		if !matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(_)) {
+		if !matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(flags) if flags.is_interactive_signing()) {
 			return Err(ChannelError::close("Received tx_signatures in strange state!".to_owned()));
 		}
 
@@ -6293,6 +6319,8 @@ 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
@@ -6307,6 +6335,10 @@ impl<SP: Deref> FundedChannel<SP> where
 				return Ok(None);
 			}
 
+			if holder_tx_signatures_opt.is_some() {
+				self.context.channel_state.set_our_tx_signatures_sent();
+			}
+
 			Ok(holder_tx_signatures_opt)
 		} else {
 			Err(ChannelError::Close((
@@ -6529,7 +6561,7 @@ impl<SP: Deref> FundedChannel<SP> where
 		// first received the funding_signed.
 		let mut funding_broadcastable = None;
 		if let Some(funding_transaction) = &self.funding.funding_transaction {
-			if (self.funding.is_outbound() || self.is_v2_established()) &&
+			if (self.funding.is_outbound() || (self.is_v2_established() && self.context.channel_state.is_their_tx_signatures_sent())) &&
 				(matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(flags) if !flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH)) ||
 				matches!(self.context.channel_state, ChannelState::ChannelReady(_)))
 			{
@@ -6567,6 +6599,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;
@@ -7035,7 +7070,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.
@@ -7054,10 +7089,10 @@ impl<SP: Deref> FundedChannel<SP> where
 						//   MUST send its tx_signatures for that funding transaction.
 						// else if it HAS received commitment_signed AND has received tx_signatures for that funding transaction:
 						//   MUST send its tx_signatures for that funding transaction.
-						let tx_signatures = if session.has_received_commitment_signed() && ((
-							!session.counterparty_sent_tx_signatures() &&
+						let tx_signatures = if (our_commitment_transaction == INITIAL_COMMITMENT_NUMBER - 1) && ((
+							!self.context.channel_state.is_their_tx_signatures_sent() &&
 							session.holder_sends_tx_signatures_first()
-						) || session.counterparty_sent_tx_signatures()) {
+						) || self.context.channel_state.is_their_tx_signatures_sent()) {
 							// This should have already been set in `commitment_signed_initial_v2`, but check again
 							// just in case.
 							if self.context.channel_state.is_monitor_update_in_progress() {
@@ -7073,7 +7108,7 @@ impl<SP: Deref> FundedChannel<SP> where
 								session.holder_tx_signatures().clone()
 							}
 						} else {
-							if !session.has_received_commitment_signed() {
+							if our_commitment_transaction == INITIAL_COMMITMENT_NUMBER - 1 {
 								self.context.expecting_peer_commitment_signed = true;
 							}
 							None
@@ -8429,9 +8464,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 {
@@ -10612,6 +10647,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 			(49, self.context.local_initiated_shutdown, option), // Added in 0.0.122
 			(51, is_manual_broadcast, option), // Added in 0.0.124
 			(53, funding_tx_broadcast_safe_event_emitted, option), // Added in 0.0.124
+			(55, self.interactive_tx_signing_session, option) // Added in 0.2
 		});
 
 		Ok(())
@@ -10891,6 +10927,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 		let mut next_holder_commitment_point_opt: Option<PublicKey> = None;
 		let mut is_manual_broadcast = None;
 
+		let mut interactive_tx_signing_session: Option<InteractiveTxSigningSession> = None;
+
 		read_tlv_fields!(reader, {
 			(0, announcement_sigs, option),
 			(1, minimum_depth, option),
@@ -10926,6 +10964,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 			(49, local_initiated_shutdown, option),
 			(51, is_manual_broadcast, option),
 			(53, funding_tx_broadcast_safe_event_emitted, option),
+			(55, interactive_tx_signing_session, option),
 		});
 
 		let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);
@@ -11048,6 +11087,15 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 			&channel_parameters.counterparty_parameters.as_ref()
 				.expect("Persisted channel must have counterparty parameters").pubkeys.revocation_basepoint);
 
+		let interactive_tx_signing_session = if channel_state.is_interactive_signing() {
+			interactive_tx_signing_session.zip(funding_transaction.as_ref()).map(|(mut session, funding_tx)| {
+				session.update_from_funding_tx_after_read(funding_tx);
+				session
+			})
+		} else {
+			None
+		};
+
 		Ok(FundedChannel {
 			funding: FundingScope {
 				value_to_self_msat,
@@ -11180,7 +11228,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 
 				is_holder_quiescence_initiator: None,
 			},
-			interactive_tx_signing_session: None,
+			interactive_tx_signing_session,
 			is_v2_established,
 			holder_commitment_point,
 			#[cfg(splicing)]