From ac3380e4704ed90503e39c1d9d647aed54cce5de Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 7 May 2021 20:56:10 +0000 Subject: [PATCH] Send update_channel messages to re-enable a disabled channel Currently, we only send an update_channel message after disconnecting a peer and waiting some time. We do not send a followup when the peer has been reconnected for some time. This changes that behavior to make the disconnect and reconnect channel updates symmetric, and also simplifies the state machine somewhat to make it more clear. Finally, it serializes the current announcement state so that we usually know when we need to send a new update_channel. --- lightning/src/ln/channel.rs | 73 ++++++++++++++++++---------- lightning/src/ln/channelmanager.rs | 36 +++++++++----- lightning/src/ln/functional_tests.rs | 4 +- 3 files changed, 71 insertions(+), 42 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 413dc8313..cf0fae025 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -251,15 +251,17 @@ pub const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1; /// Liveness is called to fluctuate given peer disconnecton/monitor failures/closing. /// If channel is public, network should have a liveness view announced by us on a /// best-effort, which means we may filter out some status transitions to avoid spam. -/// See further timer_tick_occurred. -#[derive(PartialEq)] -enum UpdateStatus { - /// Status has been gossiped. - Fresh, - /// Status has been changed. - DisabledMarked, - /// Status has been marked to be gossiped at next flush +/// See implementation at [`super::channelmanager::ChannelManager::timer_tick_occurred`]. +#[derive(Clone, Copy, PartialEq)] +pub(super) enum UpdateStatus { + /// 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, + /// Our channel is live again, but we haven't announced the channel as enabled yet. + EnabledStaged, + /// We've announced the channel as disabled. + Disabled, } /// An enum indicating whether the local or remote side offered a given HTLC. @@ -617,7 +619,7 @@ impl Channel { commitment_secrets: CounterpartyCommitmentSecrets::new(), - network_sync: UpdateStatus::Fresh, + network_sync: UpdateStatus::Enabled, #[cfg(any(test, feature = "fuzztarget"))] next_local_commitment_tx_fee_info_cached: Mutex::new(None), @@ -858,7 +860,7 @@ impl Channel { commitment_secrets: CounterpartyCommitmentSecrets::new(), - network_sync: UpdateStatus::Fresh, + network_sync: UpdateStatus::Enabled, #[cfg(any(test, feature = "fuzztarget"))] next_local_commitment_tx_fee_info_cached: Mutex::new(None), @@ -3495,24 +3497,12 @@ impl Channel { } else { false } } - pub fn to_disabled_staged(&mut self) { - self.network_sync = UpdateStatus::DisabledStaged; + pub fn get_update_status(&self) -> UpdateStatus { + self.network_sync } - pub fn to_disabled_marked(&mut self) { - self.network_sync = UpdateStatus::DisabledMarked; - } - - pub fn to_fresh(&mut self) { - self.network_sync = UpdateStatus::Fresh; - } - - pub fn is_disabled_staged(&self) -> bool { - self.network_sync == UpdateStatus::DisabledStaged - } - - pub fn is_disabled_marked(&self) -> bool { - self.network_sync == UpdateStatus::DisabledMarked + pub fn set_update_status(&mut self, status: UpdateStatus) { + self.network_sync = status; } fn check_get_funding_locked(&mut self, height: u32) -> Option { @@ -4375,6 +4365,31 @@ impl Readable for InboundHTLCRemovalReason { } } +impl Writeable for UpdateStatus { + fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { + // We only care about writing out the current state as it was announced, ie only either + // Enabled or Disabled. In the case of DisabledStaged, we most recently announced the + // channel as enabled, so we write 0. For EnabledStaged, we similarly write a 1. + match self { + UpdateStatus::Enabled => 0u8.write(writer)?, + UpdateStatus::DisabledStaged => 0u8.write(writer)?, + UpdateStatus::EnabledStaged => 1u8.write(writer)?, + UpdateStatus::Disabled => 1u8.write(writer)?, + } + Ok(()) + } +} + +impl Readable for UpdateStatus { + fn read(reader: &mut R) -> Result { + Ok(match ::read(reader)? { + 0 => UpdateStatus::Enabled, + 1 => UpdateStatus::Disabled, + _ => return Err(DecodeError::InvalidValue), + }) + } +} + impl Writeable for Channel { fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { // Note that we write out as if remove_uncommitted_htlcs_and_mark_paused had just been @@ -4568,6 +4583,8 @@ impl Writeable for Channel { self.counterparty_shutdown_scriptpubkey.write(writer)?; self.commitment_secrets.write(writer)?; + + self.network_sync.write(writer)?; Ok(()) } } @@ -4740,6 +4757,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel let counterparty_shutdown_scriptpubkey = Readable::read(reader)?; let commitment_secrets = Readable::read(reader)?; + let network_sync = Readable::read(reader)?; + let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes()); @@ -4814,7 +4833,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel commitment_secrets, - network_sync: UpdateStatus::Fresh, + network_sync, #[cfg(any(test, feature = "fuzztarget"))] next_local_commitment_tx_fee_info_cached: Mutex::new(None), diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a9093b567..6d1c73e59 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -45,7 +45,7 @@ use chain::transaction::{OutPoint, TransactionData}; // construct one themselves. use ln::{PaymentHash, PaymentPreimage, PaymentSecret}; pub use ln::channel::CounterpartyForwardingInfo; -use ln::channel::{Channel, ChannelError}; +use ln::channel::{Channel, ChannelError, UpdateStatus}; use ln::features::{InitFeatures, NodeFeatures}; use routing::router::{Route, RouteHop}; use ln::msgs; @@ -2127,17 +2127,28 @@ impl ChannelMana let mut channel_state_lock = self.channel_state.lock().unwrap(); let channel_state = &mut *channel_state_lock; for (_, chan) in channel_state.by_id.iter_mut() { - if chan.is_disabled_staged() && !chan.is_live() { - if let Ok(update) = self.get_channel_update(&chan) { - channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { - msg: update - }); - } - chan.to_fresh(); - } else if chan.is_disabled_staged() && chan.is_live() { - chan.to_fresh(); - } else if chan.is_disabled_marked() { - chan.to_disabled_staged(); + match chan.get_update_status() { + UpdateStatus::Enabled if !chan.is_live() => chan.set_update_status(UpdateStatus::DisabledStaged), + UpdateStatus::Disabled if chan.is_live() => chan.set_update_status(UpdateStatus::EnabledStaged), + UpdateStatus::DisabledStaged if chan.is_live() => chan.set_update_status(UpdateStatus::Enabled), + UpdateStatus::EnabledStaged if !chan.is_live() => chan.set_update_status(UpdateStatus::Disabled), + UpdateStatus::DisabledStaged if !chan.is_live() => { + if let Ok(update) = self.get_channel_update(&chan) { + channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { + msg: update + }); + } + chan.set_update_status(UpdateStatus::Disabled); + }, + UpdateStatus::EnabledStaged if chan.is_live() => { + if let Ok(update) = self.get_channel_update(&chan) { + channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { + msg: update + }); + } + chan.set_update_status(UpdateStatus::Enabled); + }, + _ => {}, } } } @@ -3916,7 +3927,6 @@ impl // on peer disconnect here, there will need to be corresponding changes in // reestablish logic. let failed_adds = chan.remove_uncommitted_htlcs_and_mark_paused(&self.logger); - chan.to_disabled_marked(); if !failed_adds.is_empty() { let chan_update = self.get_channel_update(&chan).map(|u| u.encode_with_len()).unwrap(); // Cannot add/recv HTLCs before we have a short_id so unwrap is safe failed_payments.push((chan_update, failed_adds)); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 4a94b6c6e..e4ffaa75d 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -7582,8 +7582,8 @@ fn test_announce_disable_channels() { nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false); nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false); - nodes[0].node.timer_tick_occurred(); // dirty -> stagged - nodes[0].node.timer_tick_occurred(); // staged -> fresh + nodes[0].node.timer_tick_occurred(); // enabled -> disabledstagged + nodes[0].node.timer_tick_occurred(); // disabledstaged -> disabled let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), 3); for e in msg_events { -- 2.39.5