Send update_channel messages to re-enable a disabled channel
authorMatt Corallo <git@bluematt.me>
Fri, 7 May 2021 20:56:10 +0000 (20:56 +0000)
committerMatt Corallo <git@bluematt.me>
Thu, 13 May 2021 20:53:53 +0000 (20:53 +0000)
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
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs

index 413dc8313a6c5d18c73b92f74c5ee9d685c44dda..cf0fae02581e3afeba2f3f2cab07e7db943ead03 100644 (file)
@@ -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<Signer: Sign> Channel<Signer> {
 
                        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<Signer: Sign> Channel<Signer> {
 
                        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<Signer: Sign> Channel<Signer> {
                } 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<msgs::FundingLocked> {
@@ -4375,6 +4365,31 @@ impl Readable for InboundHTLCRemovalReason {
        }
 }
 
+impl Writeable for UpdateStatus {
+       fn write<W: Writer>(&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<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+               Ok(match <u8 as Readable>::read(reader)? {
+                       0 => UpdateStatus::Enabled,
+                       1 => UpdateStatus::Disabled,
+                       _ => return Err(DecodeError::InvalidValue),
+               })
+       }
+}
+
 impl<Signer: Sign> Writeable for Channel<Signer> {
        fn write<W: Writer>(&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<Signer: Sign> Writeable for Channel<Signer> {
                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<Signer>
                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<Signer>
 
                        commitment_secrets,
 
-                       network_sync: UpdateStatus::Fresh,
+                       network_sync,
 
                        #[cfg(any(test, feature = "fuzztarget"))]
                        next_local_commitment_tx_fee_info_cached: Mutex::new(None),
index a9093b567120dbb4b82d0d00c9b04feef3fd8966..6d1c73e59826ea5ae46b9e4f656f347c3c40d56c 100644 (file)
@@ -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<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
                                                // 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));
index 4a94b6c6e4d2781f2efaef2138c846fa9d2f8fb8..e4ffaa75da0dea64f5b55f1256aa24fa9c3d5874 100644 (file)
@@ -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 {