Skip to content

Commit

Permalink
Rename a few variables and functions to better suit their role
Browse files Browse the repository at this point in the history
- UNFUNDED_CHANNEL_AGE_LIMIT_TICKS ->
   STALE_UNFUNDED_CHANNEL_AGE_LIMIT_TICKS

- DISCONNECTED_UNCONFIRMED_CHANNEL_AGE_LIMIT_TICKS ->
   DISCONNECTED_UNFUNDED_CHANNEL_AGE_LIMIT_TICKS

- struct UnconfirmedChannelContext -> struct DisconnectedUnfundedChannelContext

- unconfirmed_channel_age_ticks -> disconnected_unfunded_channel_age_ticks

- fn should_expire_unconfirmed_channel -> fn should_expire_disconnected_unfunded_channel

- struct UnfundedChannelContext -> struct StaleUnfundedChannelContext

- unfunded_channel_age_ticks -> stale_unfunded_channel_age_ticks

- fn should_expire_unfunded_channel -> fn should_expire_stale_unfunded_channel

- Also rewrote the comment describing the role of DisconnectedUnfundedChannelContext
  • Loading branch information
shaavan committed Dec 16, 2023
1 parent fed62bb commit 5a4a8bb
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 49 deletions.
64 changes: 28 additions & 36 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -877,11 +877,11 @@ pub(crate) const DISCONNECT_PEER_AWAITING_RESPONSE_TICKS: usize = 2;
/// 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 STALE_UNFUNDED_CHANNEL_AGE_LIMIT_TICKS: usize = 60;

/// The number of ticks that may elapse while we're waiting for a disconnected peer to reconnect,
/// before we try to close the associated outbound channel with them.
pub(crate) const DISCONNECTED_UNCONFIRMED_CHANNEL_AGE_LIMIT_TICKS: usize = 2;
pub(crate) const DISCONNECTED_UNFUNDED_CHANNEL_AGE_LIMIT_TICKS: usize = 2;

/// Number of blocks needed for an output from a coinbase transaction to be spendable.
pub(crate) const COINBASE_MATURITY: u32 = 100;
Expand Down Expand Up @@ -923,58 +923,50 @@ impl<'a, SP: Deref> ChannelPhase<SP> where
}
}

pub(super) struct UnconfirmedChannelContext {
/// A counter tracking how many ticks have elapsed since this unaccepted channel was
/// created. If this unaccepted channel reaches peer has yet to respond after reaching
/// `UNACCEPTED_CHANNEL_AGE_LIMIT_TICKS`, it will be force-closed and purged from memory.
///
/// This is so that we don't keep outbound request around which have not been accepted
/// in a timely manner
///
/// A counter tracking how many ticks have elapsed since this the peer associated to this
/// unconfimed outbound channel has been disconnected. If this unconfirmed channel associated
/// peer is still disconnected after reaching `DISCONNECTED_UNCONFIRMED_CHANNEL_AGE_LIMIT_TICKS`,
/// it will be force-closed and purged from memory.
///
/// This is so that we don't keep the outbound request around for long whose associated peer
/// has disconnected in middle of channel creation handshake, and has not connected back yet.
unconfirmed_channel_age_ticks: usize,
pub(super) struct DisconnectedUnfundedChannelContext {
/// A counter tracking how many ticks have elapsed since the peer associated with
/// this unconfirmed outbound channel has been disconnected. If the peer is still
/// disconnected after reaching `DISCONNECTED_UNFUNDED_CHANNEL_AGE_LIMIT_TICKS`
/// the outbound channel will be force-closed and purged from memory. This is so that
/// we don't keep the outbound request around for long whose associated peer has
/// disconnected in the middle of the channel creation handshake and has not yet connected back.
disconnected_unfunded_channel_age_ticks: usize,
}

impl UnconfirmedChannelContext {
impl DisconnectedUnfundedChannelContext {
/// Determines whether we should force-close and purge this unfunded channel from memory due to it
/// having reached the unfunded channel age limit.
///
/// This should be called on every [`super::channelmanager::ChannelManager::timer_tick_occurred`].
pub fn should_expire_unconfirmed_channel(&mut self, peer_connected: bool) -> bool {
pub fn should_expire_disconnected_unfunded_channel(&mut self, peer_connected: bool) -> bool {
if peer_connected {
self.unconfirmed_channel_age_ticks = 0;
self.disconnected_unfunded_channel_age_ticks = 0;
return false;
}
self.unconfirmed_channel_age_ticks += 1;
self.unconfirmed_channel_age_ticks >= DISCONNECTED_UNCONFIRMED_CHANNEL_AGE_LIMIT_TICKS
self.disconnected_unfunded_channel_age_ticks += 1;
self.disconnected_unfunded_channel_age_ticks >= DISCONNECTED_UNFUNDED_CHANNEL_AGE_LIMIT_TICKS
}
}

/// Contains all state common to unfunded inbound/outbound channels.
pub(super) struct UnfundedChannelContext {
pub(super) struct StaleUnfundedChannelContext {
/// A counter tracking how many ticks have elapsed since this unfunded channel was
/// created. If this unfunded channel reaches peer has yet to respond after reaching
/// `UNFUNDED_CHANNEL_AGE_LIMIT_TICKS`, it will be force-closed and purged from memory.
/// `STALE_UNFUNDED_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.
unfunded_channel_age_ticks: usize,
stale_unfunded_channel_age_ticks: usize,
}

impl UnfundedChannelContext {
impl StaleUnfundedChannelContext {
/// Determines whether we should force-close and purge this unfunded channel from memory due to it
/// having reached the unfunded channel age limit.
///
/// This should be called on every [`super::channelmanager::ChannelManager::timer_tick_occurred`].
pub fn should_expire_unfunded_channel(&mut self) -> bool {
self.unfunded_channel_age_ticks += 1;
self.unfunded_channel_age_ticks >= UNFUNDED_CHANNEL_AGE_LIMIT_TICKS
pub fn should_expire_stale_unfunded_channel(&mut self) -> bool {
self.stale_unfunded_channel_age_ticks += 1;
self.stale_unfunded_channel_age_ticks >= STALE_UNFUNDED_CHANNEL_AGE_LIMIT_TICKS
}
}

Expand Down Expand Up @@ -6111,8 +6103,8 @@ impl<SP: Deref> Channel<SP> where
/// A not-yet-funded outbound (from holder) channel using V1 channel establishment.
pub(super) struct OutboundV1Channel<SP: Deref> where SP::Target: SignerProvider {
pub context: ChannelContext<SP>,
pub unfunded_context: UnfundedChannelContext,
pub unconfirmed_context: UnconfirmedChannelContext,
pub stale_unfunded_context: StaleUnfundedChannelContext,
pub disconnected_unfunded_context: DisconnectedUnfundedChannelContext,
}

impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
Expand Down Expand Up @@ -6317,8 +6309,8 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {

blocked_monitor_updates: Vec::new(),
},
unfunded_context: UnfundedChannelContext { unfunded_channel_age_ticks: 0 },
unconfirmed_context: UnconfirmedChannelContext { unconfirmed_channel_age_ticks: 0 }
stale_unfunded_context: StaleUnfundedChannelContext { stale_unfunded_channel_age_ticks: 0 },
disconnected_unfunded_context: DisconnectedUnfundedChannelContext { disconnected_unfunded_channel_age_ticks: 0 }
})
}

Expand Down Expand Up @@ -6759,7 +6751,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
/// A not-yet-funded inbound (from counterparty) channel using V1 channel establishment.
pub(super) struct InboundV1Channel<SP: Deref> where SP::Target: SignerProvider {
pub context: ChannelContext<SP>,
pub unfunded_context: UnfundedChannelContext,
pub stale_unfunded_context: StaleUnfundedChannelContext,
}

impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
Expand Down Expand Up @@ -7104,7 +7096,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {

blocked_monitor_updates: Vec::new(),
},
unfunded_context: UnfundedChannelContext { unfunded_channel_age_ticks: 0 }
stale_unfunded_context: StaleUnfundedChannelContext { stale_unfunded_channel_age_ticks: 0 }
};

Ok(chan)
Expand Down
20 changes: 10 additions & 10 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use crate::events::{Event, EventHandler, EventsProvider, MessageSendEvent, Messa
// Since this struct is returned in `list_channels` methods, expose it here in case users want to
// construct one themselves.
use crate::ln::{inbound_payment, ChannelId, PaymentHash, PaymentPreimage, PaymentSecret};
use crate::ln::channel::{Channel, ChannelPhase, ChannelContext, ChannelError, ChannelUpdateStatus, ShutdownResult, UnconfirmedChannelContext, UnfundedChannelContext, UpdateFulfillCommitFetch, OutboundV1Channel, InboundV1Channel, WithChannelContext};
use crate::ln::channel::{Channel, ChannelPhase, ChannelContext, ChannelError, ChannelUpdateStatus, ShutdownResult, DisconnectedUnfundedChannelContext, StaleUnfundedChannelContext, UpdateFulfillCommitFetch, OutboundV1Channel, InboundV1Channel, WithChannelContext};
use crate::ln::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
#[cfg(any(feature = "_test_utils", test))]
use crate::ln::features::Bolt11InvoiceFeatures;
Expand Down Expand Up @@ -4831,23 +4831,23 @@ where
let mut process_unfunded_channel_tick = |
chan_id: &ChannelId,
context: &mut ChannelContext<SP>,
unfunded_context: &mut UnfundedChannelContext,
unconfirmed_context: &mut Option<&mut UnconfirmedChannelContext>,
stale_unfunded_context: &mut StaleUnfundedChannelContext,
disconnected_unfunded_context: &mut Option<&mut DisconnectedUnfundedChannelContext>,
pending_msg_events: &mut Vec<MessageSendEvent>,
counterparty_node_id: PublicKey,
is_connected: bool
| {
context.maybe_expire_prev_config();
let should_expire_unconfirmed_channel = match unconfirmed_context {
Some(unconfirmed_context) => unconfirmed_context.should_expire_unconfirmed_channel(is_connected),
let should_expire_disconnected_unfunded_channel = match disconnected_unfunded_context {
Some(disconnected_unfunded_context) => disconnected_unfunded_context.should_expire_disconnected_unfunded_channel(is_connected),
None => false,
};
let should_expire_unfunded_channel = unfunded_context.should_expire_unfunded_channel();
if should_expire_unconfirmed_channel || should_expire_unfunded_channel {
let should_expire_stale_unfunded_channel = stale_unfunded_context.should_expire_stale_unfunded_channel();
if should_expire_disconnected_unfunded_channel || should_expire_stale_unfunded_channel {
let logger = WithChannelContext::from(&self.logger, context);
log_error!(logger, "Force-closing pending channel with ID {} for not establishing in a timely manner", chan_id);
update_maps_on_chan_removal!(self, &context);
if should_expire_unconfirmed_channel {
if should_expire_disconnected_unfunded_channel {
self.issue_channel_close_events(&context, ClosureReason::DisconnectedPeer);
} else {
self.issue_channel_close_events(&context, ClosureReason::HolderForceClosed);
Expand Down Expand Up @@ -4951,11 +4951,11 @@ where
true
},
ChannelPhase::UnfundedInboundV1(chan) => {
process_unfunded_channel_tick(chan_id, &mut chan.context, &mut chan.unfunded_context, &mut None,
process_unfunded_channel_tick(chan_id, &mut chan.context, &mut chan.stale_unfunded_context, &mut None,
pending_msg_events, counterparty_node_id, peer_connected)
},
ChannelPhase::UnfundedOutboundV1(chan) => {
process_unfunded_channel_tick(chan_id, &mut chan.context, &mut chan.unfunded_context, &mut Some(&mut chan.unconfirmed_context),
process_unfunded_channel_tick(chan_id, &mut chan.context, &mut chan.stale_unfunded_context, &mut Some(&mut chan.disconnected_unfunded_context),
pending_msg_events, counterparty_node_id, peer_connected)
},
}
Expand Down
6 changes: 3 additions & 3 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ use crate::sync::{Arc, Mutex, RwLock};
use crate::ln::functional_test_utils::*;
use crate::ln::chan_utils::CommitmentTransaction;

use super::channel::UNFUNDED_CHANNEL_AGE_LIMIT_TICKS;
use super::channel::STALE_UNFUNDED_CHANNEL_AGE_LIMIT_TICKS;

#[test]
fn test_insane_channel_opens() {
Expand Down Expand Up @@ -10276,7 +10276,7 @@ fn test_remove_expired_outbound_unfunded_channels() {
check_outbound_channel_existence(true);

// Channel should exist with 1 timer tick less than required.
for _ in 0..UNFUNDED_CHANNEL_AGE_LIMIT_TICKS - 1 {
for _ in 0..STALE_UNFUNDED_CHANNEL_AGE_LIMIT_TICKS - 1 {
nodes[0].node.timer_tick_occurred();
check_outbound_channel_existence(true)
}
Expand Down Expand Up @@ -10327,7 +10327,7 @@ fn test_remove_expired_inbound_unfunded_channels() {
check_inbound_channel_existence(true);

// Channel should exist with 1 timer tick less than required.
for _ in 0..UNFUNDED_CHANNEL_AGE_LIMIT_TICKS - 1 {
for _ in 0..STALE_UNFUNDED_CHANNEL_AGE_LIMIT_TICKS - 1 {
nodes[1].node.timer_tick_occurred();
check_inbound_channel_existence(true)
}
Expand Down

0 comments on commit 5a4a8bb

Please sign in to comment.