Merge pull request #407 from TheBlueMatt/2019-11-396-english

#396 with a few english fixes

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -210,6 +210,20 @@ const MULTI_STATE_FLAGS: u32 = (BOTH_SIDES_SHUTDOWN_MASK | ChannelState::PeerDis
 
 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_chan_freshness_every_min.
+#[derive(PartialEq)]
+enum UpdateStatus {
+	/// Status has been gossiped.
+	Fresh,
+	/// Status has been changed.
+	DisabledMarked,
+	/// Status has been marked to be gossiped at next flush
+	DisabledStaged,
+}
+
 // TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking
 // has been completed, and then turn into a Channel to get compiler-time enforcement of things like
 // calling channel_id() before we're set up or things like get_outbound_funding_signed on an
@@ -340,6 +354,8 @@ pub(super) struct Channel {
 
 	channel_monitor: ChannelMonitor,
 
+	network_sync: UpdateStatus,
+
 	logger: Arc<Logger>,
 }
 
@@ -517,6 +533,8 @@ impl Channel {
 
 			channel_monitor: channel_monitor,
 
+			network_sync: UpdateStatus::Fresh,
+
 			logger,
 		})
 	}
@@ -734,6 +752,8 @@ impl Channel {
 
 			channel_monitor: channel_monitor,
 
+			network_sync: UpdateStatus::Fresh,
+
 			logger,
 		};
 
@@ -2993,6 +3013,26 @@ impl Channel {
 		} else { false }
 	}
 
+	pub fn to_disabled_staged(&mut self) {
+		self.network_sync = UpdateStatus::DisabledStaged;
+	}
+
+	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
+	}
+
 	/// Called by channelmanager based on chain blocks being connected.
 	/// Note that we only need to use this to detect funding_signed, anything else is handled by
 	/// the channel_monitor.
@@ -4091,6 +4131,8 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
 
 			channel_monitor,
 
+			network_sync: UpdateStatus::Fresh,
+
 			logger,
 		})
 	}

lightning/src/ln/channelmanager.rs

@@ -318,6 +318,12 @@ const ERR: () = "You need at least 32 bit pointers (well, usize, but we'll assum
 /// the "reorg path" (ie call block_disconnected() until you get to a common block and then call
 /// block_connected() to step towards your best block) upon deserialization before using the
 /// object!
+///
+/// Note that ChannelManager is responsible for tracking liveness of its channels and generating
+/// ChannelUpdate messages informing peers that the channel is temporarily disabled. To avoid
+/// spam due to quick disconnection/reconnection, updates are not sent until the channel has been
+/// offline for a full minute. In order to track this, you must call
+/// timer_chan_freshness_every_min roughly once per minute, though it doesn't have to be perfec.
 pub struct ChannelManager<'a> {
 	default_configuration: UserConfig,
 	genesis_hash: Sha256dHash,
@@ -1488,6 +1494,31 @@ impl<'a> ChannelManager<'a> {
 		events.append(&mut new_events);
 	}
 
+	/// If a peer is disconnected we mark any channels with that peer as 'disabled'.
+	/// After some time, if channels are still disabled we need to broadcast a ChannelUpdate
+	/// to inform the network about the uselessness of these channels.
+	///
+	/// This method handles all the details, and must be called roughly once per minute.
+	pub fn timer_chan_freshness_every_min(&self) {
+		let _ = self.total_consistency_lock.read().unwrap();
+		let mut channel_state_lock = self.channel_state.lock().unwrap();
+		let channel_state = channel_state_lock.borrow_parts();
+		for (_, chan) in channel_state.by_id {
+			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();
+			}
+		}
+	}
+
 	/// Indicates that the preimage for payment_hash is unknown or the received amount is incorrect
 	/// after a PaymentReceived event, failing the HTLC back to its origin and freeing resources
 	/// along the path (including in our own channel on which we received it).
@@ -2797,8 +2828,8 @@ impl<'a> ChannelMessageHandler for ChannelManager<'a> {
 				log_debug!(self, "Marking channels with {} disconnected and generating channel_updates", log_pubkey!(their_node_id));
 				channel_state.by_id.retain(|_, chan| {
 					if chan.get_their_node_id() == *their_node_id {
-						//TODO: mark channel disabled (and maybe announce such after a timeout).
 						let failed_adds = chan.remove_uncommitted_htlcs_and_mark_paused();
+						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

@@ -6256,3 +6256,63 @@ fn test_check_htlc_underpaying() {
 	}
 	nodes[1].node.get_and_clear_pending_events();
 }
+
+#[test]
+fn test_announce_disable_channels() {
+	// Create 2 channels between A and B. Disconnect B. Call timer_chan_freshness_every_min and check for generated
+	// ChannelUpdate. Reconnect B, reestablish and check there is non-generated ChannelUpdate.
+
+	let nodes = create_network(2, &[None, None]);
+
+	let short_id_1 = create_announced_chan_between_nodes(&nodes, 0, 1, LocalFeatures::new(), LocalFeatures::new()).0.contents.short_channel_id;
+	let short_id_2 = create_announced_chan_between_nodes(&nodes, 1, 0, LocalFeatures::new(), LocalFeatures::new()).0.contents.short_channel_id;
+	let short_id_3 = create_announced_chan_between_nodes(&nodes, 0, 1, LocalFeatures::new(), LocalFeatures::new()).0.contents.short_channel_id;
+
+	// Disconnect peers
+	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_chan_freshness_every_min(); // dirty -> stagged
+	nodes[0].node.timer_chan_freshness_every_min(); // staged -> fresh
+	let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(msg_events.len(), 3);
+	for e in msg_events {
+		match e {
+			MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
+				let short_id = msg.contents.short_channel_id;
+				// Check generated channel_update match list in PendingChannelUpdate
+				if short_id != short_id_1 && short_id != short_id_2 && short_id != short_id_3 {
+					panic!("Generated ChannelUpdate for wrong chan!");
+				}
+			},
+			_ => panic!("Unexpected event"),
+		}
+	}
+	// Reconnect peers
+	nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id());
+	let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
+	assert_eq!(reestablish_1.len(), 3);
+	nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id());
+	let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
+	assert_eq!(reestablish_2.len(), 3);
+
+	// Reestablish chan_1
+	nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]).unwrap();
+	handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+	nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]).unwrap();
+	handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
+	// Reestablish chan_2
+	nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[1]).unwrap();
+	handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+	nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[1]).unwrap();
+	handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
+	// Reestablish chan_3
+	nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[2]).unwrap();
+	handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+	nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[2]).unwrap();
+	handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
+
+	nodes[0].node.timer_chan_freshness_every_min();
+	let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(msg_events.len(), 0);
+}