Add node id to remaining RoutingMessageHandler::handle_ methods

tnull · tnull · commit f68b8b68346e · 2024-09-11T18:01:33.000+02:00
Previously, some `RoutingMessageHandler::handle_` methods (in particular
the ones handling node and channel announcements, as well as channel
updates, omitted the `their_node_id` argument. This didn't allow
implementors to discern *who* sent a particular method.

Here, we add `their_node_id: Option&lt;&amp;PublicKey&gt;` to have them learn who
sent a message, and set `None` if our own node it the originator of a
broadcast operation.
diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs
@@ -660,16 +660,18 @@ mod tests {
 	}
 	impl RoutingMessageHandler for MsgHandler {
 		fn handle_node_announcement(
-			&self, _msg: &NodeAnnouncement,
+			&self, _their_node_id: Option<&PublicKey>, _msg: &NodeAnnouncement,
 		) -> Result<bool, LightningError> {
 			Ok(false)
 		}
 		fn handle_channel_announcement(
-			&self, _msg: &ChannelAnnouncement,
+			&self, _their_node_id: Option<&PublicKey>, _msg: &ChannelAnnouncement,
 		) -> Result<bool, LightningError> {
 			Ok(false)
 		}
-		fn handle_channel_update(&self, _msg: &ChannelUpdate) -> Result<bool, LightningError> {
+		fn handle_channel_update(
+			&self, _their_node_id: Option<&PublicKey>, _msg: &ChannelUpdate,
+		) -> Result<bool, LightningError> {
 			Ok(false)
 		}
 		fn get_next_channel_announcement(
diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs
@@ -1945,10 +1945,11 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
 		let (channel_ready, channel_id) = create_chan_between_nodes_with_value_confirm_second(&nodes[0], &nodes[1]);
 		(channel_id, create_chan_between_nodes_with_value_b(&nodes[1], &nodes[0], &channel_ready))
 	};
-	for node in nodes.iter() {
-		assert!(node.gossip_sync.handle_channel_announcement(&announcement).unwrap());
-		node.gossip_sync.handle_channel_update(&as_update).unwrap();
-		node.gossip_sync.handle_channel_update(&bs_update).unwrap();
+	for (i, node) in nodes.iter().enumerate() {
+		let counterparty_node_id = nodes[(i + 1) % 2].node.get_our_node_id();
+		assert!(node.gossip_sync.handle_channel_announcement(Some(&counterparty_node_id), &announcement).unwrap());
+		node.gossip_sync.handle_channel_update(Some(&counterparty_node_id), &as_update).unwrap();
+		node.gossip_sync.handle_channel_update(Some(&counterparty_node_id), &bs_update).unwrap();
 	}
 
 	if !restore_b_before_lock {
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
@@ -1501,14 +1501,16 @@ pub fn create_unannounced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &
 
 pub fn update_nodes_with_chan_announce<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize, ann: &msgs::ChannelAnnouncement, upd_1: &msgs::ChannelUpdate, upd_2: &msgs::ChannelUpdate) {
 	for node in nodes {
-		assert!(node.gossip_sync.handle_channel_announcement(ann).unwrap());
-		node.gossip_sync.handle_channel_update(upd_1).unwrap();
-		node.gossip_sync.handle_channel_update(upd_2).unwrap();
+		let node_id_a = nodes[a].node.get_our_node_id();
+		let node_id_b = nodes[b].node.get_our_node_id();
+		assert!(node.gossip_sync.handle_channel_announcement(None, ann).unwrap());
+		node.gossip_sync.handle_channel_update(Some(&node_id_a), upd_1).unwrap();
+		node.gossip_sync.handle_channel_update(Some(&node_id_b), upd_2).unwrap();
 
 		// Note that channel_updates are also delivered to ChannelManagers to ensure we have
 		// forwarding info for local channels even if its not accepted in the network graph.
-		node.node.handle_channel_update(&nodes[a].node.get_our_node_id(), &upd_1);
-		node.node.handle_channel_update(&nodes[b].node.get_our_node_id(), &upd_2);
+		node.node.handle_channel_update(&node_id_a, &upd_1);
+		node.node.handle_channel_update(&node_id_b, &upd_2);
 	}
 }
 
@@ -3512,9 +3514,10 @@ pub fn handle_announce_close_broadcast_events<'a, 'b, 'c>(nodes: &Vec<Node<'a, '
 	if dummy_connected {
 		disconnect_dummy_node(&nodes[b]);
 	}
+	let node_id_a = nodes[a].node.get_our_node_id();
 	for node in nodes {
-		node.gossip_sync.handle_channel_update(&as_update).unwrap();
-		node.gossip_sync.handle_channel_update(&bs_update).unwrap();
+		node.gossip_sync.handle_channel_update(Some(&node_id_a), &as_update).unwrap();
+		node.gossip_sync.handle_channel_update(Some(&node_id_a), &bs_update).unwrap();
 	}
 }
 
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
@@ -2461,14 +2461,16 @@ fn channel_monitor_network_test() {
 		check_preimage_claim(&nodes[4], &node_txn);
 		(close_chan_update_1, close_chan_update_2)
 	};
-	nodes[3].gossip_sync.handle_channel_update(&close_chan_update_2).unwrap();
-	nodes[4].gossip_sync.handle_channel_update(&close_chan_update_1).unwrap();
+	let node_id_4 = nodes[4].node.get_our_node_id();
+	let node_id_3 = nodes[3].node.get_our_node_id();
+	nodes[3].gossip_sync.handle_channel_update(Some(&node_id_4), &close_chan_update_2).unwrap();
+	nodes[4].gossip_sync.handle_channel_update(Some(&node_id_3), &close_chan_update_1).unwrap();
 	assert_eq!(nodes[3].node.list_channels().len(), 0);
 	assert_eq!(nodes[4].node.list_channels().len(), 0);
 
 	assert_eq!(nodes[3].chain_monitor.chain_monitor.watch_channel(OutPoint { txid: chan_3.3.compute_txid(), index: 0 }, chan_3_mon),
 		Ok(ChannelMonitorUpdateStatus::Completed));
-	check_closed_event!(nodes[3], 1, ClosureReason::HTLCsTimedOut, [nodes[4].node.get_our_node_id()], 100000);
+	check_closed_event!(nodes[3], 1, ClosureReason::HTLCsTimedOut, [node_id_4], 100000);
 }
 
 #[test]
diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs
@@ -1617,13 +1617,19 @@ pub trait ChannelMessageHandler : MessageSendEventsProvider {
 pub trait RoutingMessageHandler : MessageSendEventsProvider {
 	/// Handle an incoming `node_announcement` message, returning `true` if it should be forwarded on,
 	/// `false` or returning an `Err` otherwise.
-	fn handle_node_announcement(&self, msg: &NodeAnnouncement) -> Result<bool, LightningError>;
+	///
+	/// If `their_node_id` is `None`, the message was generated by our own local node.
+	fn handle_node_announcement(&self, their_node_id: Option<&PublicKey>, msg: &NodeAnnouncement) -> Result<bool, LightningError>;
 	/// Handle a `channel_announcement` message, returning `true` if it should be forwarded on, `false`
 	/// or returning an `Err` otherwise.
-	fn handle_channel_announcement(&self, msg: &ChannelAnnouncement) -> Result<bool, LightningError>;
+	///
+	/// If `their_node_id` is `None`, the message was generated by our own local node.
+	fn handle_channel_announcement(&self, their_node_id: Option<&PublicKey>, msg: &ChannelAnnouncement) -> Result<bool, LightningError>;
 	/// Handle an incoming `channel_update` message, returning true if it should be forwarded on,
 	/// `false` or returning an `Err` otherwise.
-	fn handle_channel_update(&self, msg: &ChannelUpdate) -> Result<bool, LightningError>;
+	///
+	/// If `their_node_id` is `None`, the message was generated by our own local node.
+	fn handle_channel_update(&self, their_node_id: Option<&PublicKey>, msg: &ChannelUpdate) -> Result<bool, LightningError>;
 	/// Gets channel announcements and updates required to dump our routing table to a remote node,
 	/// starting at the `short_channel_id` indicated by `starting_point` and including announcements
 	/// for a single channel.
diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs
@@ -134,9 +134,10 @@ fn announce_node_address<'a, 'b, 'c>(
 		contents: announcement
 	};
 
-	node.gossip_sync.handle_node_announcement(&msg).unwrap();
+	let node_pubkey = node.node.get_our_node_id();
+	node.gossip_sync.handle_node_announcement(None, &msg).unwrap();
 	for peer in peers {
-		peer.gossip_sync.handle_node_announcement(&msg).unwrap();
+		peer.gossip_sync.handle_node_announcement(Some(&node_pubkey), &msg).unwrap();
 	}
 }
 
diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs
@@ -111,9 +111,9 @@ impl MessageSendEventsProvider for IgnoringMessageHandler {
 	fn get_and_clear_pending_msg_events(&self) -> Vec<MessageSendEvent> { Vec::new() }
 }
 impl RoutingMessageHandler for IgnoringMessageHandler {
-	fn handle_node_announcement(&self, _msg: &msgs::NodeAnnouncement) -> Result<bool, LightningError> { Ok(false) }
-	fn handle_channel_announcement(&self, _msg: &msgs::ChannelAnnouncement) -> Result<bool, LightningError> { Ok(false) }
-	fn handle_channel_update(&self, _msg: &msgs::ChannelUpdate) -> Result<bool, LightningError> { Ok(false) }
+	fn handle_node_announcement(&self, _their_node_id: Option<&PublicKey>, _msg: &msgs::NodeAnnouncement) -> Result<bool, LightningError> { Ok(false) }
+	fn handle_channel_announcement(&self, _their_node_id: Option<&PublicKey>, _msg: &msgs::ChannelAnnouncement) -> Result<bool, LightningError> { Ok(false) }
+	fn handle_channel_update(&self, _their_node_id: Option<&PublicKey>, _msg: &msgs::ChannelUpdate) -> Result<bool, LightningError> { Ok(false) }
 	fn get_next_channel_announcement(&self, _starting_point: u64) ->
 		Option<(msgs::ChannelAnnouncement, Option<msgs::ChannelUpdate>, Option<msgs::ChannelUpdate>)> { None }
 	fn get_next_node_announcement(&self, _starting_point: Option<&NodeId>) -> Option<msgs::NodeAnnouncement> { None }
@@ -1887,22 +1887,22 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 				self.message_handler.chan_handler.handle_announcement_signatures(&their_node_id, &msg);
 			},
 			wire::Message::ChannelAnnouncement(msg) => {
-				if self.message_handler.route_handler.handle_channel_announcement(&msg)
+				if self.message_handler.route_handler.handle_channel_announcement(Some(their_node_id), &msg)
 						.map_err(|e| -> MessageHandlingError { e.into() })? {
 					should_forward = Some(wire::Message::ChannelAnnouncement(msg));
 				}
 				self.update_gossip_backlogged();
 			},
 			wire::Message::NodeAnnouncement(msg) => {
-				if self.message_handler.route_handler.handle_node_announcement(&msg)
+				if self.message_handler.route_handler.handle_node_announcement(Some(their_node_id), &msg)
 						.map_err(|e| -> MessageHandlingError { e.into() })? {
 					should_forward = Some(wire::Message::NodeAnnouncement(msg));
 				}
 				self.update_gossip_backlogged();
 			},
 			wire::Message::ChannelUpdate(msg) => {
-				self.message_handler.chan_handler.handle_channel_update(&their_node_id, &msg);
-				if self.message_handler.route_handler.handle_channel_update(&msg)
+				self.message_handler.chan_handler.handle_channel_update(their_node_id, &msg);
+				if self.message_handler.route_handler.handle_channel_update(Some(their_node_id), &msg)
 						.map_err(|e| -> MessageHandlingError { e.into() })? {
 					should_forward = Some(wire::Message::ChannelUpdate(msg));
 				}
@@ -2285,13 +2285,13 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 						},
 						MessageSendEvent::BroadcastChannelAnnouncement { msg, update_msg } => {
 							log_debug!(self.logger, "Handling BroadcastChannelAnnouncement event in peer_handler for short channel id {}", msg.contents.short_channel_id);
-							match self.message_handler.route_handler.handle_channel_announcement(&msg) {
+							match self.message_handler.route_handler.handle_channel_announcement(None, &msg) {
 								Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
 									self.forward_broadcast_msg(peers, &wire::Message::ChannelAnnouncement(msg), None),
 								_ => {},
 							}
 							if let Some(msg) = update_msg {
-								match self.message_handler.route_handler.handle_channel_update(&msg) {
+								match self.message_handler.route_handler.handle_channel_update(None, &msg) {
 									Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
 										self.forward_broadcast_msg(peers, &wire::Message::ChannelUpdate(msg), None),
 									_ => {},
@@ -2300,15 +2300,15 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 						},
 						MessageSendEvent::BroadcastChannelUpdate { msg } => {
 							log_debug!(self.logger, "Handling BroadcastChannelUpdate event in peer_handler for contents {:?}", msg.contents);
-							match self.message_handler.route_handler.handle_channel_update(&msg) {
+							match self.message_handler.route_handler.handle_channel_update(None, &msg) {
 								Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
 									self.forward_broadcast_msg(peers, &wire::Message::ChannelUpdate(msg), None),
 								_ => {},
 							}
 						},
 						MessageSendEvent::BroadcastNodeAnnouncement { msg } => {
 							log_debug!(self.logger, "Handling BroadcastNodeAnnouncement event in peer_handler for node {}", msg.contents.node_id);
-							match self.message_handler.route_handler.handle_node_announcement(&msg) {
+							match self.message_handler.route_handler.handle_node_announcement(None, &msg) {
 								Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
 									self.forward_broadcast_msg(peers, &wire::Message::NodeAnnouncement(msg), None),
 								_ => {},
@@ -2674,7 +2674,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 		};
 
 		log_debug!(self.logger, "Broadcasting NodeAnnouncement after passing it to our own RoutingMessageHandler.");
-		let _ = self.message_handler.route_handler.handle_node_announcement(&msg);
+		let _ = self.message_handler.route_handler.handle_node_announcement(None, &msg);
 		self.forward_broadcast_msg(&*self.peers.read().unwrap(), &wire::Message::NodeAnnouncement(msg), None);
 	}
 }
diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs
@@ -207,10 +207,11 @@ fn do_test_1_conf_open(connect_style: ConnectStyle) {
 	} else { panic!("Unexpected event"); };
 	assert_eq!(announcement, bs_announcement);
 
-	for node in nodes {
-		assert!(node.gossip_sync.handle_channel_announcement(&announcement).unwrap());
-		node.gossip_sync.handle_channel_update(&as_update).unwrap();
-		node.gossip_sync.handle_channel_update(&bs_update).unwrap();
+	for (i, node) in nodes.iter().enumerate() {
+		let counterparty_node_id = nodes[(i + 1) % 2].node.get_our_node_id();
+		assert!(node.gossip_sync.handle_channel_announcement(Some(&counterparty_node_id), &announcement).unwrap());
+		node.gossip_sync.handle_channel_update(Some(&counterparty_node_id), &as_update).unwrap();
+		node.gossip_sync.handle_channel_update(Some(&counterparty_node_id), &bs_update).unwrap();
 	}
 }
 #[test]
diff --git a/lightning/src/ln/reload_tests.rs b/lightning/src/ln/reload_tests.rs
@@ -165,9 +165,10 @@ fn test_funding_peer_disconnect() {
 	};
 
 	// Provide the channel announcement and public updates to the network graph
-	nodes[0].gossip_sync.handle_channel_announcement(&chan_announcement).unwrap();
-	nodes[0].gossip_sync.handle_channel_update(&bs_update).unwrap();
-	nodes[0].gossip_sync.handle_channel_update(&as_update).unwrap();
+	let node_1_pubkey = nodes[1].node.get_our_node_id();
+	nodes[0].gossip_sync.handle_channel_announcement(Some(&node_1_pubkey), &chan_announcement).unwrap();
+	nodes[0].gossip_sync.handle_channel_update(Some(&node_1_pubkey), &bs_update).unwrap();
+	nodes[0].gossip_sync.handle_channel_update(Some(&node_1_pubkey), &as_update).unwrap();
 
 	let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
 	let payment_preimage = send_along_route(&nodes[0], route, &[&nodes[1]], 1000000).0;
@@ -219,10 +220,11 @@ fn test_no_txn_manager_serialize_deserialize() {
 
 	let (channel_ready, _) = create_chan_between_nodes_with_value_confirm(&nodes[0], &nodes[1], &tx);
 	let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &channel_ready);
-	for node in nodes.iter() {
-		assert!(node.gossip_sync.handle_channel_announcement(&announcement).unwrap());
-		node.gossip_sync.handle_channel_update(&as_update).unwrap();
-		node.gossip_sync.handle_channel_update(&bs_update).unwrap();
+	for (i, node) in nodes.iter().enumerate() {
+		let counterparty_node_id = nodes[(i + 1) % 2].node.get_our_node_id();
+		assert!(node.gossip_sync.handle_channel_announcement(Some(&counterparty_node_id), &announcement).unwrap());
+		node.gossip_sync.handle_channel_update(Some(&counterparty_node_id), &as_update).unwrap();
+		node.gossip_sync.handle_channel_update(Some(&counterparty_node_id), &bs_update).unwrap();
 	}
 
 	send_payment(&nodes[0], &[&nodes[1]], 1000000);
@@ -309,10 +311,11 @@ fn test_manager_serialize_deserialize_events() {
 
 	let (channel_ready, _) = create_chan_between_nodes_with_value_confirm(&nodes[0], &nodes[1], &tx);
 	let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &channel_ready);
-	for node in nodes.iter() {
-		assert!(node.gossip_sync.handle_channel_announcement(&announcement).unwrap());
-		node.gossip_sync.handle_channel_update(&as_update).unwrap();
-		node.gossip_sync.handle_channel_update(&bs_update).unwrap();
+	for (i, node) in nodes.iter().enumerate() {
+		let counterparty_node_id = nodes[(i + 1) % 2].node.get_our_node_id();
+		assert!(node.gossip_sync.handle_channel_announcement(Some(&counterparty_node_id), &announcement).unwrap());
+		node.gossip_sync.handle_channel_update(Some(&counterparty_node_id), &as_update).unwrap();
+		node.gossip_sync.handle_channel_update(Some(&counterparty_node_id), &bs_update).unwrap();
 	}
 
 	send_payment(&nodes[0], &[&nodes[1]], 1000000);
diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs
diff --git a/lightning/src/routing/test_utils.rs b/lightning/src/routing/test_utils.rs
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs

Original file line number	Diff line number	Diff line change
`@@ -660,16 +660,18 @@ mod tests {`
`660`	`660`	`}`
`661`	`661`	`impl RoutingMessageHandler for MsgHandler {`
`662`	`662`	`fn handle_node_announcement(`
`663`		`- &self, _msg: &NodeAnnouncement,`
	`663`	`+ &self, _their_node_id: Option<&PublicKey>, _msg: &NodeAnnouncement,`
`664`	`664`	`) -> Result<bool, LightningError> {`
`665`	`665`	`Ok(false)`
`666`	`666`	`}`
`667`	`667`	`fn handle_channel_announcement(`
`668`		`- &self, _msg: &ChannelAnnouncement,`
	`668`	`+ &self, _their_node_id: Option<&PublicKey>, _msg: &ChannelAnnouncement,`
`669`	`669`	`) -> Result<bool, LightningError> {`
`670`	`670`	`Ok(false)`
`671`	`671`	`}`
`672`		`- fn handle_channel_update(&self, _msg: &ChannelUpdate) -> Result<bool, LightningError> {`
	`672`	`+ fn handle_channel_update(`
	`673`	`+ &self, _their_node_id: Option<&PublicKey>, _msg: &ChannelUpdate,`
	`674`	`+ ) -> Result<bool, LightningError> {`
`673`	`675`	`Ok(false)`
`674`	`676`	`}`
`675`	`677`	`fn get_next_channel_announcement(`
Original file line number	Diff line number	Diff line change
`@@ -134,9 +134,10 @@ fn announce_node_address<'a, 'b, 'c>(`
`134`	`134`	`contents: announcement`
`135`	`135`	`};`
`136`	`136`
`137`		`- node.gossip_sync.handle_node_announcement(&msg).unwrap();`
	`137`	`+ let node_pubkey = node.node.get_our_node_id();`
	`138`	`+ node.gossip_sync.handle_node_announcement(None, &msg).unwrap();`
`138`	`139`	`for peer in peers {`
`139`		`- peer.gossip_sync.handle_node_announcement(&msg).unwrap();`
	`140`	`+ peer.gossip_sync.handle_node_announcement(Some(&node_pubkey), &msg).unwrap();`
`140`	`141`	`}`
`141`	`142`	`}`
`142`	`143`