diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 576ae665ade..26e385082f9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -936,6 +936,7 @@ pub(crate) struct ShutdownResult { pub(crate) user_channel_id: u128, pub(crate) channel_capacity_satoshis: u64, pub(crate) counterparty_node_id: PublicKey, + pub(crate) is_manual_broadcast: bool, pub(crate) unbroadcasted_funding_tx: Option, pub(crate) channel_funding_txo: Option, } @@ -3457,6 +3458,9 @@ impl ChannelContext where SP::Target: SignerProvider { /// Returns the transaction if there is a pending funding transaction that is yet to be /// broadcast. + /// + /// Note that if [`Self::is_manual_broadcast`] is true the transaction will be a dummy + /// transaction. pub fn unbroadcasted_funding(&self) -> Option { self.if_unbroadcasted_funding(|| self.funding_transaction.clone()) } @@ -3537,6 +3541,7 @@ impl ChannelContext where SP::Target: SignerProvider { channel_capacity_satoshis: self.channel_value_satoshis, counterparty_node_id: self.counterparty_node_id, unbroadcasted_funding_tx, + is_manual_broadcast: self.is_manual_broadcast, channel_funding_txo: self.get_funding_txo(), } } @@ -6240,6 +6245,7 @@ impl Channel where channel_capacity_satoshis: self.context.channel_value_satoshis, counterparty_node_id: self.context.counterparty_node_id, unbroadcasted_funding_tx: self.context.unbroadcasted_funding(), + is_manual_broadcast: self.context.is_manual_broadcast, channel_funding_txo: self.context.get_funding_txo(), }; let tx = self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig); @@ -6272,6 +6278,7 @@ impl Channel where channel_capacity_satoshis: self.context.channel_value_satoshis, counterparty_node_id: self.context.counterparty_node_id, unbroadcasted_funding_tx: self.context.unbroadcasted_funding(), + is_manual_broadcast: self.context.is_manual_broadcast, channel_funding_txo: self.context.get_funding_txo(), }; if closing_signed.is_some() { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b14369c432a..0e377b348db 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3510,7 +3510,6 @@ where let _ = self.chain_monitor.update_channel(funding_txo, &monitor_update); } let mut shutdown_results = Vec::new(); - let mut is_manual_broadcast = false; if let Some(txid) = shutdown_res.unbroadcasted_batch_funding_txid { let mut funding_batch_states = self.funding_batch_states.lock().unwrap(); let affected_channels = funding_batch_states.remove(&txid).into_iter().flatten(); @@ -3520,10 +3519,6 @@ where if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) { let mut peer_state = peer_state_mutex.lock().unwrap(); if let Some(mut chan) = peer_state.channel_by_id.remove(&channel_id) { - // We override the previous value, so we could change from true -> false, - // but this is fine because if a channel has manual_broadcast set to false - // we should choose the safier condition. - is_manual_broadcast = chan.context().is_manual_broadcast(); update_maps_on_chan_removal!(self, &chan.context()); shutdown_results.push(chan.context_mut().force_shutdown(false, ClosureReason::FundingBatchClosure)); } @@ -3547,12 +3542,15 @@ where channel_funding_txo: shutdown_res.channel_funding_txo, }, None)); - let funding_info = if is_manual_broadcast { - shutdown_res.channel_funding_txo.map(|outpoint| FundingInfo::OutPoint{ outpoint }) - } else { - shutdown_res.unbroadcasted_funding_tx.map(|transaction| FundingInfo::Tx{ transaction }) - }; - if let Some(funding_info) = funding_info { + if let Some(transaction) = shutdown_res.unbroadcasted_funding_tx { + let funding_info = if shutdown_res.is_manual_broadcast { + FundingInfo::OutPoint { + outpoint: shutdown_res.channel_funding_txo + .expect("We had an unbroadcasted funding tx, so should also have had a funding outpoint"), + } + } else { + FundingInfo::Tx{ transaction } + }; pending_events.push_back((events::Event::DiscardFunding { channel_id: shutdown_res.channel_id, funding_info }, None)); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 12f1466e025..a85bb1ae4a2 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -18,7 +18,7 @@ use crate::chain::channelmonitor; use crate::chain::channelmonitor::{CLOSED_CHANNEL_UPDATE_ID, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY}; use crate::chain::transaction::OutPoint; use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider}; -use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason}; +use crate::events::{Event, FundingInfo, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason}; use crate::ln::types::{ChannelId, PaymentPreimage, PaymentSecret, PaymentHash}; use crate::ln::channel::{CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, get_holder_selected_channel_reserve_satoshis, OutboundV1Channel, InboundV1Channel, COINBASE_MATURITY, ChannelPhase}; use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, BREAKDOWN_TIMEOUT, ENABLE_GOSSIP_TICKS, DISABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA}; @@ -11209,6 +11209,48 @@ fn test_accept_inbound_channel_errors_queued() { open_channel_msg.common_fields.temporary_channel_id); } +#[test] +fn test_manual_funding_abandon() { + let mut cfg = UserConfig::default(); + cfg.channel_handshake_config.minimum_depth = 1; + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(cfg), Some(cfg)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + assert!(nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 42, None, None).is_ok()); + let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel); + let accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel); + let (temporary_channel_id, _tx, funding_outpoint) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100_000, 42); + nodes[0].node.unsafe_manual_funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), funding_outpoint).unwrap(); + check_added_monitors!(nodes[0], 0); + + let funding_created = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); + nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created); + check_added_monitors!(nodes[1], 1); + expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id()); + + let funding_signed = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id()); + let err = msgs::ErrorMessage { channel_id: funding_signed.channel_id, data: "".to_string() }; + nodes[0].node.handle_error(&nodes[1].node.get_our_node_id(), &err); + + let close_events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(close_events.len(), 2); + assert!(close_events.iter().any(|ev| matches!(ev, Event::ChannelClosed { .. }))); + assert!(close_events.iter().any(|ev| match ev { + Event::DiscardFunding { channel_id, funding_info: FundingInfo::OutPoint { outpoint } } => { + assert_eq!(*channel_id, err.channel_id); + assert_eq!(*outpoint, funding_outpoint); + true + } + _ => false, + })); +} + #[test] fn test_funding_signed_event() { let mut cfg = UserConfig::default(); diff --git a/pending_changelog/3259-no-downgrade.txt b/pending_changelog/3259-no-downgrade.txt new file mode 100644 index 00000000000..ed4193da480 --- /dev/null +++ b/pending_changelog/3259-no-downgrade.txt @@ -0,0 +1,4 @@ +# Backwards Compatibility + * Downgrading after using `ChannelManager`'s + `unsafe_manual_funding_transaction_generated` may cause deserialization of + `ChannelManager` to fail with an `Err` (#3259).