From: Alec Chen Date: Mon, 6 Feb 2023 21:33:02 +0000 (-0600) Subject: Swap `PublicKey` for `NodeId` in `UnsignedNodeAnnouncement` X-Git-Tag: v0.0.114-beta~27^2 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=b156371d45cb3f25131468b82352902a66024933;p=rust-lightning Swap `PublicKey` for `NodeId` in `UnsignedNodeAnnouncement` Also swaps `PublicKey` for `NodeId` in `get_next_node_announcement` and `InitSyncTracker` to avoid unnecessary deserialization that came from changing `UnsignedNodeAnnouncement`. --- diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 72c834e72..dbf619f1d 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -184,7 +184,7 @@ pub fn do_test(data: &[u8], out: Out) { return; } let msg = decode_msg_with_len16!(msgs::UnsignedNodeAnnouncement, 288); - node_pks.insert(msg.node_id); + node_pks.insert(get_pubkey_from_node_id!(msg.node_id)); let _ = net_graph.update_node_from_unsigned_announcement(&msg); }, 1 => { diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs index c4a4bedad..16680faaa 100644 --- a/lightning-net-tokio/src/lib.rs +++ b/lightning-net-tokio/src/lib.rs @@ -584,6 +584,7 @@ mod tests { use lightning::ln::msgs::*; use lightning::ln::peer_handler::{MessageHandler, PeerManager}; use lightning::ln::features::NodeFeatures; + use lightning::routing::gossip::NodeId; use lightning::util::events::*; use lightning::util::test_utils::TestNodeSigner; use bitcoin::secp256k1::{Secp256k1, SecretKey, PublicKey}; @@ -614,7 +615,7 @@ mod tests { fn handle_channel_announcement(&self, _msg: &ChannelAnnouncement) -> Result { Ok(false) } fn handle_channel_update(&self, _msg: &ChannelUpdate) -> Result { Ok(false) } fn get_next_channel_announcement(&self, _starting_point: u64) -> Option<(ChannelAnnouncement, Option, Option)> { None } - fn get_next_node_announcement(&self, _starting_point: Option<&PublicKey>) -> Option { None } + fn get_next_node_announcement(&self, _starting_point: Option<&NodeId>) -> Option { None } fn peer_connected(&self, _their_node_id: &PublicKey, _init_msg: &Init) -> Result<(), ()> { Ok(()) } fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: ReplyChannelRange) -> Result<(), LightningError> { Ok(()) } fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: ReplyShortChannelIdsEnd) -> Result<(), LightningError> { Ok(()) } diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 942bd82e9..c971df906 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -665,7 +665,7 @@ pub struct UnsignedNodeAnnouncement { pub timestamp: u32, /// The `node_id` this announcement originated from (don't rebroadcast the `node_announcement` back /// to this node). - pub node_id: PublicKey, + pub node_id: NodeId, /// An RGB color for UI purposes pub rgb: [u8; 3], /// An alias, for UI purposes. @@ -1057,7 +1057,7 @@ pub trait RoutingMessageHandler : MessageSendEventsProvider { /// the node *after* the provided pubkey and including up to one announcement immediately /// higher (as defined by `::cmp`) than `starting_point`. /// If `None` is provided for `starting_point`, we start at the first node. - fn get_next_node_announcement(&self, starting_point: Option<&PublicKey>) -> Option; + fn get_next_node_announcement(&self, starting_point: Option<&NodeId>) -> Option; /// Called when a connection is established with a peer. This can be used to /// perform routing table synchronization using a strategy defined by the /// implementor. @@ -1845,7 +1845,7 @@ impl Readable for UnsignedNodeAnnouncement { fn read(r: &mut R) -> Result { let features: NodeFeatures = Readable::read(r)?; let timestamp: u32 = Readable::read(r)?; - let node_id: PublicKey = Readable::read(r)?; + let node_id: NodeId = Readable::read(r)?; let mut rgb = [0; 3]; r.read_exact(&mut rgb)?; let alias: [u8; 32] = Readable::read(r)?; @@ -2248,7 +2248,7 @@ mod tests { let unsigned_node_announcement = msgs::UnsignedNodeAnnouncement { features, timestamp: 20190119, - node_id: pubkey_1, + node_id: NodeId::from_pubkey(&pubkey_1), rgb: [32; 3], alias: [16;32], addresses, diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index f593e23c8..3e05c8ef3 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -71,7 +71,7 @@ impl RoutingMessageHandler for IgnoringMessageHandler { fn handle_channel_update(&self, _msg: &msgs::ChannelUpdate) -> Result { Ok(false) } fn get_next_channel_announcement(&self, _starting_point: u64) -> Option<(msgs::ChannelAnnouncement, Option, Option)> { None } - fn get_next_node_announcement(&self, _starting_point: Option<&PublicKey>) -> Option { None } + fn get_next_node_announcement(&self, _starting_point: Option<&NodeId>) -> Option { None } fn peer_connected(&self, _their_node_id: &PublicKey, _init: &msgs::Init) -> Result<(), ()> { Ok(()) } fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyChannelRange) -> Result<(), LightningError> { Ok(()) } fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyShortChannelIdsEnd) -> Result<(), LightningError> { Ok(()) } @@ -345,7 +345,7 @@ impl error::Error for PeerHandleError { enum InitSyncTracker{ NoSyncRequested, ChannelsSyncing(u64), - NodesSyncing(PublicKey), + NodesSyncing(NodeId), } /// The ratio between buffer sizes at which we stop sending initial sync messages vs when we stop @@ -434,7 +434,7 @@ impl Peer { } /// Similar to the above, but for node announcements indexed by node_id. - fn should_forward_node_announcement(&self, node_id: PublicKey) -> bool { + fn should_forward_node_announcement(&self, node_id: NodeId) -> bool { if self.their_features.as_ref().unwrap().supports_gossip_queries() && !self.sent_gossip_timestamp_filter { return false; @@ -442,7 +442,7 @@ impl Peer { match self.sync_status { InitSyncTracker::NoSyncRequested => true, InitSyncTracker::ChannelsSyncing(_) => false, - InitSyncTracker::NodesSyncing(pk) => pk < node_id, + InitSyncTracker::NodesSyncing(sync_node_id) => sync_node_id.as_slice() < node_id.as_slice(), } } @@ -889,8 +889,8 @@ impl unreachable!(), - InitSyncTracker::NodesSyncing(key) => { - if let Some(msg) = self.message_handler.route_handler.get_next_node_announcement(Some(&key)) { + InitSyncTracker::NodesSyncing(sync_node_id) => { + if let Some(msg) = self.message_handler.route_handler.get_next_node_announcement(Some(&sync_node_id)) { self.enqueue_message(peer, &msg); peer.sync_status = InitSyncTracker::NodesSyncing(msg.contents.node_id); } else { @@ -1493,8 +1493,10 @@ impl) -> Option { + fn get_next_node_announcement(&self, starting_point: Option<&NodeId>) -> Option { let nodes = self.network_graph.nodes.read().unwrap(); - let iter = if let Some(pubkey) = starting_point { - nodes.range((Bound::Excluded(NodeId::from_pubkey(pubkey)), Bound::Unbounded)) + let iter = if let Some(node_id) = starting_point { + nodes.range((Bound::Excluded(node_id), Bound::Unbounded)) } else { nodes.range(..) }; @@ -1286,7 +1286,7 @@ impl NetworkGraph where L::Target: Logger { /// routing messages from a source using a protocol other than the lightning P2P protocol. pub fn update_node_from_announcement(&self, msg: &msgs::NodeAnnouncement) -> Result<(), LightningError> { let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.contents.encode()[..])[..]); - secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &msg.contents.node_id, "node_announcement"); + secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &get_pubkey_from_node_id!(msg.contents.node_id, "node_announcement"), "node_announcement"); self.update_node_from_announcement_intern(&msg.contents, Some(&msg)) } @@ -1299,7 +1299,7 @@ impl NetworkGraph where L::Target: Logger { } fn update_node_from_announcement_intern(&self, msg: &msgs::UnsignedNodeAnnouncement, full_msg: Option<&msgs::NodeAnnouncement>) -> Result<(), LightningError> { - match self.nodes.write().unwrap().get_mut(&NodeId::from_pubkey(&msg.node_id)) { + match self.nodes.write().unwrap().get_mut(&msg.node_id) { None => Err(LightningError{err: "No existing channels for node_announcement".to_owned(), action: ErrorAction::IgnoreError}), Some(node) => { if let Some(node_info) = node.announcement_info.as_ref() { @@ -1971,11 +1971,11 @@ mod tests { } fn get_signed_node_announcement(f: F, node_key: &SecretKey, secp_ctx: &Secp256k1) -> NodeAnnouncement { - let node_id = PublicKey::from_secret_key(&secp_ctx, node_key); + let node_id = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_key)); let mut unsigned_announcement = UnsignedNodeAnnouncement { features: channelmanager::provided_node_features(&UserConfig::default()), timestamp: 100, - node_id: node_id, + node_id, rgb: [0; 3], alias: [0; 32], addresses: Vec::new(), @@ -2652,7 +2652,7 @@ mod tests { let (secp_ctx, gossip_sync) = create_gossip_sync(&network_graph); let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap(); let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap(); - let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_1_privkey); + let node_id_1 = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_1_privkey)); // No nodes yet. let next_announcements = gossip_sync.get_next_node_announcement(None); diff --git a/lightning/src/routing/test_utils.rs b/lightning/src/routing/test_utils.rs index 8666a65ec..b737bab35 100644 --- a/lightning/src/routing/test_utils.rs +++ b/lightning/src/routing/test_utils.rs @@ -66,7 +66,7 @@ pub(super) fn add_or_update_node( gossip_sync: &P2PGossipSync>>, Arc, Arc>, secp_ctx: &Secp256k1, node_privkey: &SecretKey, features: NodeFeatures, timestamp: u32 ) { - let node_id = PublicKey::from_secret_key(&secp_ctx, node_privkey); + let node_id = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_privkey)); let unsigned_announcement = UnsignedNodeAnnouncement { features, timestamp, diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 6dd775ea3..be09c8f00 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -511,7 +511,7 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler { Some((chan_ann, Some(chan_upd_1), Some(chan_upd_2))) } - fn get_next_node_announcement(&self, _starting_point: Option<&PublicKey>) -> Option { + fn get_next_node_announcement(&self, _starting_point: Option<&NodeId>) -> Option { None }