Skip to content

Commit

Permalink
Add node id to remaining RoutingMessageHandler::handle_ methods
Browse files Browse the repository at this point in the history 
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<&PublicKey>` to have them learn who
sent a message, and set `None` if our own node it the originator of a
broadcast operation.
  • Loading branch information
@tnull
tnull committed Sep 4, 2024
1 parent b023eed commit dde402d
Show file tree
Hide file tree
Showing 12 changed files with 139 additions and 106 deletions.
8 changes: 5 additions & 3 deletions lightning-net-tokio/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
9 changes: 5 additions & 4 deletions lightning/src/ln/chanmon_update_fail_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
17 changes: 10 additions & 7 deletions lightning/src/ln/functional_test_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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();
}
}

Expand Down
8 changes: 5 additions & 3 deletions lightning/src/ln/functional_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
12 changes: 9 additions & 3 deletions lightning/src/ln/msgs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions lightning/src/ln/offers_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

Expand Down
24 changes: 12 additions & 12 deletions lightning/src/ln/peer_handler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -1888,22 +1888,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));
}
Expand Down Expand Up @@ -2286,13 +2286,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),
_ => {},
Expand All @@ -2301,15 +2301,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),
_ => {},
Expand Down Expand Up @@ -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);
}
}
Expand Down
9 changes: 5 additions & 4 deletions lightning/src/ln/priv_short_conf_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
25 changes: 14 additions & 11 deletions lightning/src/ln/reload_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Loading

0 comments on commit dde402d

Please sign in to comment.