diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index db286c459c4..87ad6a532d9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -590,6 +590,11 @@ pub(crate) const EXPIRE_PREV_CONFIG_TICKS: usize = 5; /// See [`ChannelContext::sent_message_awaiting_response`] for more information. pub(crate) const DISCONNECT_PEER_AWAITING_RESPONSE_TICKS: usize = 2; +/// The number of ticks that may elapse while we're waiting for a pending outbound/inbound channel +/// to be promoted to a [`Channel`] since the pending channel was created. A pending channel +/// exceeding this age limit will be force closed and purged from memory. +pub(crate) const PENDING_CHANNEL_AGE_LIMIT_TICKS: usize = 60; + struct PendingChannelMonitorUpdate { update: ChannelMonitorUpdate, } @@ -598,6 +603,28 @@ impl_writeable_tlv_based!(PendingChannelMonitorUpdate, { (0, update, required), }); +/// Contains all state common to pending (unfunded) inbound/outbound channels. +pub(super) struct PendingChannelContext { + /// A counter tracking how many ticks have elapsed since this pending channel was + /// created. If this pending channel reaches peer has yet to respond after reaching + /// `PENDING_CHANNEL_AGE_LIMIT_TICKS`, it will be force-closed and purged from memory. + /// + /// This is so that we don't keep channels around that haven't progressed to a funded state + /// in a timely manner. + pending_channel_age_ticks: usize, +} + +impl PendingChannelContext { + /// Determines whether we should force-close and purge this pending channel from memory due to it + /// having reached the pending channel age limit. + /// + /// This should be called on every [`super::channelmanager::ChannelManager::timer_tick_occurred`]. + pub fn should_expire_pending_channel(&mut self) -> bool { + self.pending_channel_age_ticks += 1; + self.pending_channel_age_ticks >= PENDING_CHANNEL_AGE_LIMIT_TICKS + } +} + /// Contains everything about the channel including state, and various flags. pub(super) struct ChannelContext { config: LegacyChannelConfig, @@ -5462,6 +5489,7 @@ impl Channel { /// A not-yet-funded outbound (from holder) channel using V1 channel establishment. pub(super) struct OutboundV1Channel { pub context: ChannelContext, + pub pending_context: PendingChannelContext, } impl OutboundV1Channel { @@ -5658,7 +5686,8 @@ impl OutboundV1Channel { channel_keys_id, blocked_monitor_updates: Vec::new(), - } + }, + pending_context: PendingChannelContext { pending_channel_age_ticks: 0 } }) } @@ -5957,6 +5986,7 @@ impl OutboundV1Channel { /// A not-yet-funded inbound (from counterparty) channel using V1 channel establishment. pub(super) struct InboundV1Channel { pub context: ChannelContext, + pub pending_context: PendingChannelContext, } impl InboundV1Channel { @@ -6284,7 +6314,8 @@ impl InboundV1Channel { channel_keys_id, blocked_monitor_updates: Vec::new(), - } + }, + pending_context: PendingChannelContext { pending_channel_age_ticks: 0 } }; Ok(chan) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f1d1e8e6f65..a38c52e22c5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4326,6 +4326,7 @@ where /// * Expiring a channel's previous [`ChannelConfig`] if necessary to only allow forwarding HTLCs /// with the current [`ChannelConfig`]. /// * Removing peers which have disconnected but and no longer have any channels. + /// * Force-closing and removing channels which have not completed establishment in a timely manner. /// /// Note that this may cause reentrancy through [`chain::Watch::update_channel`] calls or feerate /// estimate fetches. @@ -4414,6 +4415,28 @@ where true }); + + let force_close_expired_pending_channel = |chan_id: &[u8; 32], chan_context: &mut ChannelContext<::Signer>| { + log_error!(self.logger, "Force-closing pending outbound channel {} for not establishing in a timely manner", log_bytes!(&chan_id[..])); + self.issue_channel_close_events(&chan_context, ClosureReason::HolderForceClosed); + self.finish_force_close_channel(chan_context.force_shutdown(false)); + false + }; + peer_state.outbound_v1_channel_by_id.retain(|chan_id, chan| { + if chan.pending_context.should_expire_pending_channel() { + force_close_expired_pending_channel(chan_id, &mut chan.context) + } else { + true + } + }); + peer_state.inbound_v1_channel_by_id.retain(|chan_id, chan| { + if chan.pending_context.should_expire_pending_channel() { + force_close_expired_pending_channel(chan_id, &mut chan.context) + } else { + true + } + }); + if peer_state.ok_to_remove(true) { pending_peers_awaiting_removal.push(counterparty_node_id); } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 0e35f35f0c8..6e5d1b50789 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -61,6 +61,8 @@ use crate::sync::{Arc, Mutex}; use crate::ln::functional_test_utils::*; use crate::ln::chan_utils::CommitmentTransaction; +use super::channel::PENDING_CHANNEL_AGE_LIMIT_TICKS; + #[test] fn test_insane_channel_opens() { // Stand up a network of 2 nodes @@ -10017,3 +10019,89 @@ fn test_disconnects_peer_awaiting_response_ticks() { } } } + +#[test] +fn test_remove_expired_outbound_pending_channels() { + 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, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let temp_channel_id = nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 42, None).unwrap(); + let open_channel_message = 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_message); + let accept_channel_message = 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_message); + + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::FundingGenerationReady { .. } => (), + _ => panic!("Unexpected event"), + }; + + // Asserts the outbound channel has been removed from a nodes[0]'s peer state map. + let check_outbound_channel_existence = |should_exist: bool| { + let per_peer_state = nodes[0].node.per_peer_state.read().unwrap(); + let chan_lock = per_peer_state.get(&nodes[1].node.get_our_node_id()).unwrap().lock().unwrap(); + assert_eq!(chan_lock.outbound_v1_channel_by_id.contains_key(&temp_channel_id), should_exist); + }; + + // Channel should exist without any timer ticks. + check_outbound_channel_existence(true); + + // Channel should exist with 1 timer tick less than required. + for _ in 0..PENDING_CHANNEL_AGE_LIMIT_TICKS - 1 { + nodes[0].node.timer_tick_occurred(); + check_outbound_channel_existence(true) + } + + // Remove channel after reaching the required ticks. + nodes[0].node.timer_tick_occurred(); + check_outbound_channel_existence(false); + + check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed); +} + +#[test] +fn test_remove_expired_inbound_pending_channels() { + 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, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let temp_channel_id = nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 42, None).unwrap(); + let open_channel_message = 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_message); + let accept_channel_message = 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_message); + + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::FundingGenerationReady { .. } => (), + _ => panic!("Unexpected event"), + }; + + // Asserts the inbound channel has been removed from a nodes[1]'s peer state map. + let check_inbound_channel_existence = |should_exist: bool| { + let per_peer_state = nodes[1].node.per_peer_state.read().unwrap(); + let chan_lock = per_peer_state.get(&nodes[0].node.get_our_node_id()).unwrap().lock().unwrap(); + assert_eq!(chan_lock.inbound_v1_channel_by_id.contains_key(&temp_channel_id), should_exist); + }; + + // Channel should exist without any timer ticks. + check_inbound_channel_existence(true); + + // Channel should exist with 1 timer tick less than required. + for _ in 0..PENDING_CHANNEL_AGE_LIMIT_TICKS - 1 { + nodes[1].node.timer_tick_occurred(); + check_inbound_channel_existence(true) + } + + // Remove channel after reaching the required ticks. + nodes[1].node.timer_tick_occurred(); + check_inbound_channel_existence(false); + + check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed); +}