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.

Author: TheBlueMatt

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 [`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),

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<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));

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 {