Wipe splice state upon failed interactive funding construction

An interactive funding construction can be considered failed upon a
disconnect or a `tx_abort` message. So far, we've consumed the
`InteractiveTxConstructor` in the latter case, but not the former.
Additionally, we may have splice-specific state that needs to be
consumed as well to allow us to negotiate another splice later on.

This commit ensures that we properly consume all splice and interactive
funding state whenever possible upon a disconnect or `tx_abort`.

The interactive funding state is safe to consume as long as we have
either yet to reach `AwaitingSignatures`, or we have but `tx_signatures`
has not been sent/received.

The splice state is safe to consume as long as we don't have a pending
`FundingNegotiation::AwaitingSignatures` with a `tx_signatures`
sent/received and we don't have any negotiated candidates. Note that
until splice RBF is supported, it is not currently possible to have any
negotiated candidates with a pending interactive funding transaction.

Author: wpaulino

lightning/src/ln/channel.rs

@@ -1694,10 +1694,17 @@ where
 			},
 			ChannelPhase::Funded(funded_channel) => {
 				funded_channel.context.channel_state.clear_quiescent();
-				funded_channel
-					.pending_splice
-					.as_mut()
-					.and_then(|pending_splice| pending_splice.funding_negotiation.take());
+				if funded_channel.should_reset_pending_splice_funding_negotiation() {
+					funded_channel
+						.pending_splice
+						.as_mut()
+						.and_then(|pending_splice| pending_splice.funding_negotiation.take());
+				} else {
+					debug_assert!(false);
+				}
+				if funded_channel.should_reset_pending_splice_state() {
+					funded_channel.pending_splice.take();
+				}
 			},
 		};
 
@@ -1826,11 +1833,18 @@ where
 			},
 			ChannelPhase::Funded(funded_channel) => {
 				funded_channel.context.channel_state.clear_quiescent();
-				funded_channel
-					.pending_splice
-					.as_mut()
-					.and_then(|pending_splice| pending_splice.funding_negotiation.take())
-					.is_some()
+				let has_funding_negotiation =
+					funded_channel.should_reset_pending_splice_funding_negotiation();
+				if has_funding_negotiation {
+					funded_channel
+						.pending_splice
+						.as_mut()
+						.and_then(|pending_splice| pending_splice.funding_negotiation.take());
+				}
+				if funded_channel.should_reset_pending_splice_state() {
+					funded_channel.pending_splice.take();
+				}
+				has_funding_negotiation
 			},
 		};
 
@@ -2549,6 +2563,15 @@ impl FundingNegotiation {
 }
 
 impl PendingFunding {
+	fn can_abandon_funding_negotiation(&self) -> bool {
+		self.funding_negotiation
+			.as_ref()
+			.map(|funding_negotiation| {
+				!matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. })
+			})
+			.unwrap_or(true)
+	}
+
 	fn check_get_splice_locked<SP: Deref>(
 		&mut self, context: &ChannelContext<SP>, confirmed_funding_index: usize, height: u32,
 	) -> Option<msgs::SpliceLocked>
@@ -6734,6 +6757,33 @@ where
 		)
 	}
 
+	fn should_reset_pending_splice_funding_negotiation(&self) -> bool {
+		self.pending_splice
+			.as_ref()
+			.map(|pending_splice| {
+				if pending_splice.can_abandon_funding_negotiation() {
+					true
+				} else {
+					self.context
+						.interactive_tx_signing_session
+						.as_ref()
+						.map(|signing_session| {
+							signing_session.holder_tx_signatures().is_some()
+								|| signing_session.has_received_tx_signatures()
+						})
+						.unwrap_or_else(|| {
+							debug_assert!(false);
+							false
+						})
+				}
+			})
+			.unwrap_or(false)
+	}
+
+	fn should_reset_pending_splice_state(&self) -> bool {
+		self.should_reset_pending_splice_funding_negotiation() && self.pending_funding().is_empty()
+	}
+
 	#[rustfmt::skip]
 	fn check_remote_fee<F: Deref, L: Deref>(
 		channel_type: &ChannelTypeFeatures, fee_estimator: &LowerBoundedFeeEstimator<F>,
@@ -8862,6 +8912,15 @@ where
 			self.context.channel_state.clear_quiescent();
 		}
 
+		if self.should_reset_pending_splice_funding_negotiation() {
+			self.pending_splice
+				.as_mut()
+				.and_then(|pending_splice| pending_splice.funding_negotiation.take());
+		}
+		if self.should_reset_pending_splice_state() {
+			self.pending_splice.take();
+		}
+
 		self.context.channel_state.set_peer_disconnected();
 		log_trace!(logger, "Peer disconnection resulted in {} remote-announced HTLC drops on channel {}", inbound_drop_count, &self.context.channel_id());
 		Ok(())
@@ -14233,6 +14292,14 @@ where
 		let holder_commitment_point_next = self.holder_commitment_point.next_point();
 		let holder_commitment_point_pending_next = self.holder_commitment_point.pending_next_point;
 
+		let pending_splice = if self.should_reset_pending_splice_state() {
+			None
+		} else {
+			// We don't have to worry about resetting the pending `FundingNegotiation` because we
+			// can only read `FundingNegotiation::AwaitingSignatures` variants anyway.
+			self.pending_splice.as_ref()
+		};
+
 		write_tlv_fields!(writer, {
 			(0, self.context.announcement_sigs, option),
 			// minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a
@@ -14281,7 +14348,7 @@ where
 			(60, self.context.historical_scids, optional_vec), // Added in 0.2
 			(61, fulfill_attribution_data, optional_vec), // Added in 0.2
 			(63, holder_commitment_point_current, option), // Added in 0.2
-			(64, self.pending_splice, option), // Added in 0.2
+			(64, pending_splice, option), // Added in 0.2
 			(65, self.quiescent_action, option), // Added in 0.2
 			(67, pending_outbound_held_htlc_flags, optional_vec), // Added in 0.2
 			(69, holding_cell_held_htlc_flags, optional_vec), // Added in 0.2
@@ -14945,6 +15012,15 @@ where
 			}
 		};
 
+		if let Some(funding_negotiation) = pending_splice
+			.as_ref()
+			.and_then(|pending_splice| pending_splice.funding_negotiation.as_ref())
+		{
+			if !matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. }) {
+				return Err(DecodeError::InvalidValue);
+			}
+		}
+
 		Ok(FundedChannel {
 			funding: FundingScope {
 				value_to_self_msat,

lightning/src/ln/splicing_tests.rs

@@ -19,6 +19,7 @@ use crate::ln::funding::{FundingTxInput, SpliceContribution};
 use crate::ln::msgs::{self, BaseMessageHandler, ChannelMessageHandler, MessageSendEvent};
 use crate::ln::types::ChannelId;
 use crate::util::errors::APIError;
+use crate::util::ser::Writeable;
 
 use bitcoin::{Amount, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut};
 
@@ -315,6 +316,118 @@ fn lock_splice_after_blocks<'a, 'b, 'c, 'd>(
 	node_b.chain_source.remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script);
 }
 
+#[test]
+fn test_splice_state_reset_on_disconnect() {
+	do_test_splice_state_reset_on_disconnect(false);
+	do_test_splice_state_reset_on_disconnect(true);
+}
+
+fn do_test_splice_state_reset_on_disconnect(reload: bool) {
+	// Tests that we're able to forget our pending splice state after a disconnect such that we can
+	// retry later.
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let (persister_0a, persister_0b, persister_1a, persister_1b);
+	let (chain_monitor_0a, chain_monitor_0b, chain_monitor_1a, chain_monitor_1b);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let (node_0a, node_0b, node_1a, node_1b);
+	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let node_id_0 = nodes[0].node.get_our_node_id();
+	let node_id_1 = nodes[1].node.get_our_node_id();
+
+	let (_, _, channel_id, _) =
+		create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 50_000_000);
+
+	let contribution = SpliceContribution::SpliceOut {
+		outputs: vec![TxOut {
+			value: Amount::from_sat(1_000),
+			script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
+		}],
+	};
+	nodes[0]
+		.node
+		.splice_channel(
+			&channel_id,
+			&node_id_1,
+			contribution.clone(),
+			FEERATE_FLOOR_SATS_PER_KW,
+			None,
+		)
+		.unwrap();
+
+	let stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1);
+	nodes[1].node.handle_stfu(node_id_0, &stfu);
+	let stfu = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0);
+	nodes[0].node.handle_stfu(node_id_1, &stfu);
+
+	let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1);
+	nodes[1].node.handle_splice_init(node_id_0, &splice_init);
+	let _ = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0);
+
+	if reload {
+		let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode();
+		reload_node!(
+			nodes[0],
+			&nodes[0].node.encode(),
+			&[&encoded_monitor_0],
+			persister_0a,
+			chain_monitor_0a,
+			node_0a
+		);
+		let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode();
+		reload_node!(
+			nodes[1],
+			&nodes[1].node.encode(),
+			&[&encoded_monitor_1],
+			persister_1a,
+			chain_monitor_1a,
+			node_1a
+		);
+	} else {
+		nodes[0].node.peer_disconnected(node_id_1);
+		nodes[1].node.peer_disconnected(node_id_0);
+	}
+
+	let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
+	reconnect_args.send_channel_ready = (true, true);
+	reconnect_nodes(reconnect_args);
+
+	let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, contribution);
+
+	if reload {
+		let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode();
+		reload_node!(
+			nodes[0],
+			&nodes[0].node.encode(),
+			&[&encoded_monitor_0],
+			persister_0b,
+			chain_monitor_0b,
+			node_0b
+		);
+		let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode();
+		reload_node!(
+			nodes[1],
+			&nodes[1].node.encode(),
+			&[&encoded_monitor_1],
+			persister_1b,
+			chain_monitor_1b,
+			node_1b
+		);
+	} else {
+		nodes[0].node.peer_disconnected(node_id_1);
+		nodes[1].node.peer_disconnected(node_id_0);
+	}
+
+	let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
+	reconnect_args.send_channel_ready = (true, true);
+	reconnect_nodes(reconnect_args);
+
+	mine_transaction(&nodes[0], &splice_tx);
+	mine_transaction(&nodes[1], &splice_tx);
+	lock_splice_after_blocks(&nodes[0], &nodes[1], channel_id, ANTI_REORG_DELAY - 1);
+}
+
 #[test]
 fn test_splice_in() {
 	let chanmon_cfgs = create_chanmon_cfgs(2);