diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9c16c3b02f8..f6df7f244a6 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -84,7 +84,7 @@ use crate::util::config::{ use crate::util::errors::APIError; use crate::util::logger::{Level as LoggerLevel, Logger, Record, WithContext}; use crate::util::scid_utils::{block_from_scid, scid_from_parts}; -use crate::util::ser::{Readable, ReadableArgs, RequiredWrapper, Writeable, Writer}; +use crate::util::ser::{Iterable, Readable, ReadableArgs, RequiredWrapper, Writeable, Writer}; use crate::util::wallet_utils::{ConfirmedUtxo, Input}; use crate::{impl_readable_for_vec, impl_writeable_for_vec}; @@ -2989,15 +2989,6 @@ struct PendingFunding { contributions: Vec, } -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,62 @@ 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(&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() + }; + let contributions = if contributions_len == 0 { + None + } else { + Some(Iterable(self.pending_funding.contributions.iter().take(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, contributions, option), + }); + Ok(()) + } +} + +impl Readable for PendingFunding { + fn read(reader: &mut R) -> Result { + 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 +16352,19 @@ impl Writeable for FundedChannel { 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_pending_splice_state = self.should_reset_pending_splice_state(true); + let pending_splice = self + .pending_splice + .as_ref() + .filter(|pending_splice| { + !reset_pending_splice_state || !pending_splice.negotiated_candidates.is_empty() + }) + .map(|pending_splice| PendingFundingWriteable { + pending_funding: pending_splice, + reset_funding_negotiation: reset_pending_splice_state, + }); let monitor_pending_tx_signatures = self.context.monitor_pending_tx_signatures.then_some(()); diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index dfcc339b83b..75ff238bc35 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/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