From f610d41779133a01b856ad30c2ef11ffa8038813 Mon Sep 17 00:00:00 2001 From: shaavan Date: Tue, 14 May 2024 17:51:35 +0530 Subject: [PATCH 1/2] Introduce a new timer constant for timer_check_closing_negotiation_progress 1. This function implicitly assumed expiry after 1 timer_tick_occurred. 2. Introduce an explicit timer tick so that the TIMER_LIMIT can be set to an arbitrary number. 3. This addition is utilized in the following commit. --- lightning/src/ln/channel.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 5fbdb595b77..893b6db1662 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3601,8 +3601,15 @@ pub(super) struct Channel where SP::Target: SignerProvider { pub context: ChannelContext, #[cfg(any(dual_funding, splicing))] pub dual_funding_channel_context: Option, + /// Number of ticks before channel is force closed if closing_signed negotiation stalls. + timer_ticks: i32, } +/// The number of ticks before the channel is forced closed if +/// no progress on closing_signed negotiation is being made. +/// An unprogressed channel that exceeds this limit will be abandoned. +const UNPROGRESS_CLOSING_SIGNED_NEGOTIATION_AGE_LIMIT_TICKS: i32 = 1; + #[cfg(any(test, fuzzing))] struct CommitmentTxInfoCached { fee: u64, @@ -5714,7 +5721,7 @@ impl Channel where if self.closing_negotiation_ready() { if self.context.closing_signed_in_flight { return Err(ChannelError::Close("closing_signed negotiation failed to finish within two timer ticks".to_owned())); - } else { + } else if { self.timer_ticks -= 1 ; self.timer_ticks } <= 0 { self.context.closing_signed_in_flight = true; } } @@ -7790,6 +7797,7 @@ impl OutboundV1Channel where SP::Target: SignerProvider { context: self.context, #[cfg(any(dual_funding, splicing))] dual_funding_channel_context: None, + timer_ticks: UNPROGRESS_CLOSING_SIGNED_NEGOTIATION_AGE_LIMIT_TICKS, }; let need_channel_ready = channel.check_get_channel_ready(0).is_some(); @@ -8080,6 +8088,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { context: self.context, #[cfg(any(dual_funding, splicing))] dual_funding_channel_context: None, + timer_ticks: UNPROGRESS_CLOSING_SIGNED_NEGOTIATION_AGE_LIMIT_TICKS, }; let need_channel_ready = channel.check_get_channel_ready(0).is_some(); channel.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new()); @@ -9389,6 +9398,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch }, #[cfg(any(dual_funding, splicing))] dual_funding_channel_context: None, + timer_ticks: UNPROGRESS_CLOSING_SIGNED_NEGOTIATION_AGE_LIMIT_TICKS, }) } } From 9a344b6cd4976f91bcef5da95cc1a5b2ffc7aae3 Mon Sep 17 00:00:00 2001 From: shaavan Date: Tue, 14 May 2024 18:25:42 +0530 Subject: [PATCH 2/2] Convert timer_tick_occurred to a second-long timer 1. Previously, timer_tick_occurred was designed to be called every minute, leading to TIMER_LIMITS set in minutes. 2. However, this restricted the ability to set timer_tick length in smaller durations. 3. This commit updates timer_tick_occurred to be called every second instead of every minute. 4. All TIMER_LIMITS are adjusted accordingly to reflect this change. 5. Additionally, a test is updated to ensure successful compilation post-update. --- lightning-background-processor/src/lib.rs | 6 +++--- lightning/src/ln/channel.rs | 17 +++++++++-------- lightning/src/ln/channelmanager.rs | 12 ++++++++---- lightning/src/ln/shutdown_tests.rs | 6 ++++-- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index ad1056ea922..0f2fe5e27b3 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -93,7 +93,7 @@ pub struct BackgroundProcessor { } #[cfg(not(test))] -const FRESHNESS_TIMER: u64 = 60; +const FRESHNESS_TIMER: u64 = 1; #[cfg(test)] const FRESHNESS_TIMER: u64 = 1; @@ -933,7 +933,7 @@ mod tests { use lightning::events::{Event, PathFailure, MessageSendEventsProvider, MessageSendEvent}; use lightning::{get_event_msg, get_event}; use lightning::ln::types::{PaymentHash, ChannelId}; - use lightning::ln::channelmanager; + use lightning::ln::channelmanager::{self, TICKS_PER_MINUTE}; use lightning::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChainParameters, MIN_CLTV_EXPIRY_DELTA, PaymentId}; use lightning::ln::features::{ChannelFeatures, NodeFeatures}; use lightning::ln::functional_test_utils::*; @@ -960,7 +960,7 @@ mod tests { use lightning_rapid_gossip_sync::RapidGossipSync; use super::{BackgroundProcessor, GossipSync, FRESHNESS_TIMER}; - const EVENT_DEADLINE: u64 = 5 * FRESHNESS_TIMER; + const EVENT_DEADLINE: u64 = 5 * TICKS_PER_MINUTE as u64 * FRESHNESS_TIMER; #[derive(Clone, Hash, PartialEq, Eq)] struct TestDescriptor{} diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 893b6db1662..a1bb108bcbe 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -56,6 +56,7 @@ use crate::sync::Mutex; use crate::sign::type_resolver::ChannelSignerType; use super::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint, RevocationBasepoint}; +use super::channelmanager::TICKS_PER_MINUTE; #[cfg(test)] pub struct ChannelValueStat { @@ -968,9 +969,9 @@ pub(super) enum ChannelUpdateStatus { /// We've announced the channel as enabled and are connected to our peer. Enabled, /// Our channel is no longer live, but we haven't announced the channel as disabled yet. - DisabledStaged(u8), + DisabledStaged(u16), /// Our channel is live again, but we haven't announced the channel as enabled yet. - EnabledStaged(u8), + EnabledStaged(u16), /// We've announced the channel as disabled. Disabled, } @@ -1160,23 +1161,23 @@ pub(crate) const MIN_AFFORDABLE_HTLC_COUNT: usize = 4; /// number of ticks to allow forwarding HTLCs by nodes that have yet to receive the new /// ChannelUpdate prompted by the config update. This value was determined as follows: /// -/// * The expected interval between ticks (1 minute). +/// * The expected interval between ticks (1 second). /// * The average convergence delay of updates across the network, i.e., ~300 seconds on average /// for a node to see an update as seen on ``. /// * `EXPIRE_PREV_CONFIG_TICKS` = convergence_delay / tick_interval -pub(crate) const EXPIRE_PREV_CONFIG_TICKS: usize = 5; +pub(crate) const EXPIRE_PREV_CONFIG_TICKS: usize = 5 * TICKS_PER_MINUTE as usize; /// The number of ticks that may elapse while we're waiting for a response to a /// [`msgs::RevokeAndACK`] or [`msgs::ChannelReestablish`] message before we attempt to disconnect /// them. /// /// See [`ChannelContext::sent_message_awaiting_response`] for more information. -pub(crate) const DISCONNECT_PEER_AWAITING_RESPONSE_TICKS: usize = 2; +pub(crate) const DISCONNECT_PEER_AWAITING_RESPONSE_TICKS: usize = 2 * TICKS_PER_MINUTE as usize; /// The number of ticks that may elapse while we're waiting for an unfunded outbound/inbound channel /// to be promoted to a [`Channel`] since the unfunded channel was created. An unfunded channel /// exceeding this age limit will be force-closed and purged from memory. -pub(crate) const UNFUNDED_CHANNEL_AGE_LIMIT_TICKS: usize = 60; +pub(crate) const UNFUNDED_CHANNEL_AGE_LIMIT_TICKS: usize = 60 * TICKS_PER_MINUTE as usize; /// Number of blocks needed for an output from a coinbase transaction to be spendable. pub(crate) const COINBASE_MATURITY: u32 = 100; @@ -3608,7 +3609,7 @@ pub(super) struct Channel where SP::Target: SignerProvider { /// The number of ticks before the channel is forced closed if /// no progress on closing_signed negotiation is being made. /// An unprogressed channel that exceeds this limit will be abandoned. -const UNPROGRESS_CLOSING_SIGNED_NEGOTIATION_AGE_LIMIT_TICKS: i32 = 1; +pub(crate) const UNPROGRESS_CLOSING_SIGNED_NEGOTIATION_AGE_LIMIT_TICKS: i32 = 1 * TICKS_PER_MINUTE as i32; #[cfg(any(test, fuzzing))] struct CommitmentTxInfoCached { @@ -5716,7 +5717,7 @@ impl Channel where /// Checks if the closing_signed negotiation is making appropriate progress, possibly returning /// an Err if no progress is being made and the channel should be force-closed instead. - /// Should be called on a one-minute timer. + /// Should be called on a one-second timer. pub fn timer_check_closing_negotiation_progress(&mut self) -> Result<(), ChannelError> { if self.closing_negotiation_ready() { if self.context.closing_signed_in_flight { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d2b7166a503..387f62f9359 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -957,7 +957,7 @@ pub(super) struct InboundChannelRequest { /// The number of ticks that may elapse while we're waiting for an unaccepted inbound channel to be /// accepted. An unaccepted channel that exceeds this limit will be abandoned. -const UNACCEPTED_INBOUND_CHANNEL_AGE_LIMIT_TICKS: i32 = 2; +const UNACCEPTED_INBOUND_CHANNEL_AGE_LIMIT_TICKS: i32 = 2 * TICKS_PER_MINUTE as i32; /// Stores a PaymentSecret and any other data we may need to validate an inbound payment is /// actually ours and not some duplicate HTLC sent to us by a node along the route. @@ -2259,16 +2259,20 @@ const CHECK_CLTV_EXPIRY_SANITY: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRA #[allow(dead_code)] const CHECK_CLTV_EXPIRY_SANITY_2: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER; +/// Constant that keeps track of the number of times [`ChannelManager::timer_tick_occurred`] +/// is called every minute. +pub const TICKS_PER_MINUTE: u8 = 60; + /// The number of ticks of [`ChannelManager::timer_tick_occurred`] until expiry of incomplete MPPs -pub(crate) const MPP_TIMEOUT_TICKS: u8 = 3; +pub(crate) const MPP_TIMEOUT_TICKS: u8 = 3 * TICKS_PER_MINUTE; /// The number of ticks of [`ChannelManager::timer_tick_occurred`] where a peer is disconnected /// until we mark the channel disabled and gossip the update. -pub(crate) const DISABLE_GOSSIP_TICKS: u8 = 10; +pub(crate) const DISABLE_GOSSIP_TICKS: u16 = 10 * TICKS_PER_MINUTE as u16; /// The number of ticks of [`ChannelManager::timer_tick_occurred`] where a peer is connected until /// we mark the channel enabled and gossip the update. -pub(crate) const ENABLE_GOSSIP_TICKS: u8 = 5; +pub(crate) const ENABLE_GOSSIP_TICKS: u16 = 5 * TICKS_PER_MINUTE as u16; /// The maximum number of unfunded channels we can have per-peer before we start rejecting new /// (inbound) ones. The number of peers with unfunded channels is limited separately in diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index 12421ce9c3f..0ce40ba9f1c 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -10,6 +10,7 @@ //! Tests of our shutdown and closing_signed negotiation logic as well as some assorted force-close //! handling tests. +use crate::ln::channel::UNPROGRESS_CLOSING_SIGNED_NEGOTIATION_AGE_LIMIT_TICKS; use crate::sign::{EntropySource, SignerProvider}; use crate::chain::ChannelMonitorUpdateStatus; use crate::chain::transaction::OutPoint; @@ -1161,8 +1162,9 @@ fn do_test_closing_signed_reinit_timeout(timeout_step: TimeoutStep) { assert_eq!(nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1); } - nodes[1].node.timer_tick_occurred(); - nodes[1].node.timer_tick_occurred(); + for _ in 0..UNPROGRESS_CLOSING_SIGNED_NEGOTIATION_AGE_LIMIT_TICKS + 1 { + nodes[1].node.timer_tick_occurred(); + } let txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); assert_eq!(txn.len(), 1);