Persist negotiated splice candidates on reload

Prior to supporting RBF, we would avoid persisting
`FundedChannel::pending_splice` when there was a pending funding
negotiation that could not be resumed on channel reestablishment. With
the addition of RBF support, this would cause our previously negotiated
splices (but still pending) to be dropped unintentionally.

Author: wpaulino

lightning/src/ln/channel.rs

@@ -2989,15 +2989,6 @@ struct PendingFunding {
 	contributions: Vec<FundingContribution>,
 }
 
-impl_ser_tlv_based!(PendingFunding, {
-	(1, funding_negotiation, upgradable_option),
-	(3, negotiated_candidates, required_vec),
-	(5, sent_funding_txid, option),
-	(7, received_funding_txid, option),
-	(8, last_funding_feerate_sat_per_1000_weight, option),
-	(10, contributions, optional_vec),
-});
-
 #[derive(Debug)]
 enum FundingNegotiation {
 	AwaitingAck {
@@ -3037,6 +3028,57 @@ impl_writeable_tlv_based_enum_upgradable!(FundingNegotiation,
 	unread_variants: AwaitingAck, ConstructingTransaction
 );
 
+struct PendingFundingWriteable<'a> {
+	pending_funding: &'a PendingFunding,
+	reset_funding_negotiation: bool,
+}
+
+impl Writeable for PendingFundingWriteable<'_> {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+		let funding_negotiation = if self.reset_funding_negotiation {
+			None
+		} else {
+			self.pending_funding.funding_negotiation.as_ref()
+		};
+		debug_assert!(
+			funding_negotiation.is_none()
+				|| matches!(
+					funding_negotiation,
+					Some(FundingNegotiation::AwaitingSignatures { .. })
+				)
+		);
+		let contributions_len = if self.reset_funding_negotiation
+			&& self.pending_funding.funding_negotiation.is_some()
+		{
+			self.pending_funding.contributions.len().saturating_sub(1)
+		} else {
+			self.pending_funding.contributions.len()
+		};
+		write_tlv_fields!(writer, {
+			(1, funding_negotiation, upgradable_option),
+			(3, self.pending_funding.negotiated_candidates, required_vec),
+			(5, self.pending_funding.sent_funding_txid, option),
+			(7, self.pending_funding.received_funding_txid, option),
+			(8, self.pending_funding.last_funding_feerate_sat_per_1000_weight, option),
+			(10, self.pending_funding.contributions[..contributions_len], optional_vec),
+		});
+		Ok(())
+	}
+}
+
+impl Readable for PendingFunding {
+	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+		Ok(_decode_and_build!(reader, Self, {
+			(1, funding_negotiation, upgradable_option),
+			(3, negotiated_candidates, required_vec),
+			(5, sent_funding_txid, option),
+			(7, received_funding_txid, option),
+			(8, last_funding_feerate_sat_per_1000_weight, option),
+			(10, contributions, optional_vec),
+		}))
+	}
+}
+
 impl FundingNegotiation {
 	fn as_funding(&self) -> Option<&FundingScope> {
 		match self {
@@ -16305,10 +16347,18 @@ impl<SP: SignerProvider> Writeable for FundedChannel<SP> {
 		let holder_commitment_point_next = self.holder_commitment_point.next_point();
 		let holder_commitment_point_pending_next = self.holder_commitment_point.pending_next_point;
 
-		// We don't have to worry about resetting the pending `FundingNegotiation` because we
-		// can only read `FundingNegotiation::AwaitingSignatures` variants anyway.
-		let pending_splice =
-			self.pending_splice.as_ref().filter(|_| !self.should_reset_pending_splice_state(true));
+		// Avoid writing any negotiations that are not at the signing stage yet, as they cannot be
+		// resumed on reestablishment, but keep any already-negotiated candidates.
+		let reset_funding_negotiation = self.should_reset_pending_splice_state(true);
+		let should_persist_pending_splice =
+			!reset_funding_negotiation || !self.pending_funding().is_empty();
+		let pending_splice = should_persist_pending_splice
+			.then(|| ())
+			.and_then(|_| self.pending_splice.as_ref())
+			.map(|pending_funding| PendingFundingWriteable {
+				pending_funding,
+				reset_funding_negotiation,
+			});
 
 		let monitor_pending_tx_signatures =
 			self.context.monitor_pending_tx_signatures.then_some(());

lightning/src/ln/splicing_tests.rs

@@ -1152,6 +1152,55 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) {
 	lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1);
 }
 
+#[test]
+fn test_reload_resets_splice_negotiation_without_dropping_candidates() {
+	// A reload should abort an in-flight RBF negotiation, but it must not drop the previously
+	// negotiated splice candidate that the monitor is still tracking.
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let (persister_0, chain_monitor_0);
+	let node_0;
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let node_id_1 = nodes[1].node.get_our_node_id();
+	let initial_channel_value_sat = 100_000;
+	let (_, _, channel_id, _) =
+		create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0);
+
+	let added_value = Amount::from_sat(50_000);
+	provide_utxo_reserves(&nodes, 2, added_value * 2);
+
+	let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value);
+	let (_splice_tx, _) =
+		splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution.clone());
+
+	let rbf_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25);
+	let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap();
+	assert_eq!(funding_template.min_rbf_feerate(), Some(rbf_feerate));
+	assert!(funding_template.prior_contribution().is_some());
+
+	let rbf_contribution =
+		funding_template.with_prior_contribution(rbf_feerate, FeeRate::MAX).build().unwrap();
+	nodes[0].node.funding_contributed(&channel_id, &node_id_1, rbf_contribution, None).unwrap();
+	complete_rbf_handshake(&nodes[0], &nodes[1]);
+
+	let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode();
+	reload_node!(
+		nodes[0],
+		nodes[0].node.encode(),
+		&[&encoded_monitor_0],
+		persister_0,
+		chain_monitor_0,
+		node_0
+	);
+	let _ = get_event!(&nodes[0], Event::SpliceNegotiationFailed);
+
+	let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap();
+	assert_eq!(funding_template.min_rbf_feerate(), Some(rbf_feerate));
+	assert_eq!(funding_template.prior_contribution().unwrap(), &funding_contribution);
+}
+
 #[test]
 fn test_config_reject_inbound_splices() {
 	// Tests that nodes with `reject_inbound_splices` properly reject inbound splices but still