Swap `PublicKey` for `NodeId` in `UnsignedNodeAnnouncement`
authorAlec Chen <alecchendev@gmail.com>
Mon, 6 Feb 2023 21:33:02 +0000 (15:33 -0600)
committerAlec Chen <alecchendev@gmail.com>
Tue, 7 Feb 2023 16:52:20 +0000 (10:52 -0600)
Also swaps `PublicKey` for `NodeId` in `get_next_node_announcement`
and `InitSyncTracker` to avoid unnecessary deserialization that came
from changing `UnsignedNodeAnnouncement`.

fuzz/src/router.rs
lightning-net-tokio/src/lib.rs
lightning/src/ln/msgs.rs
lightning/src/ln/peer_handler.rs
lightning/src/routing/gossip.rs
lightning/src/routing/test_utils.rs
lightning/src/util/test_utils.rs

index 72c834e72976cbb35bad1371700d0d251146ec1c..dbf619f1ddf2105ebc3a216d76c81251d936e275 100644 (file)
@@ -184,7 +184,7 @@ pub fn do_test<Out: test_logger::Output>(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 => {
index c4a4bedad8f14f91e3efae01d1488de7428f3ee4..16680faaa58df8b2b48133e7021c54c86cbf1cce 100644 (file)
@@ -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<bool, LightningError> { Ok(false) }
                fn handle_channel_update(&self, _msg: &ChannelUpdate) -> Result<bool, LightningError> { Ok(false) }
                fn get_next_channel_announcement(&self, _starting_point: u64) -> Option<(ChannelAnnouncement, Option<ChannelUpdate>, Option<ChannelUpdate>)> { None }
-               fn get_next_node_announcement(&self, _starting_point: Option<&PublicKey>) -> Option<NodeAnnouncement> { None }
+               fn get_next_node_announcement(&self, _starting_point: Option<&NodeId>) -> Option<NodeAnnouncement> { 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(()) }
index 942bd82e93338fc73412778f898149b1595ddd99..c971df906c281fc96fd852c483def81367618ee2 100644 (file)
@@ -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 `<PublicKey as Ord>::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<NodeAnnouncement>;
+       fn get_next_node_announcement(&self, starting_point: Option<&NodeId>) -> Option<NodeAnnouncement>;
        /// 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: Read>(r: &mut R) -> Result<Self, DecodeError> {
                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,
index f593e23c8c6c0fd5054955a38db6dd13ec7effcb..3e05c8ef3009c46538cd882e92ac0ee6a7db8406 100644 (file)
@@ -71,7 +71,7 @@ impl RoutingMessageHandler for IgnoringMessageHandler {
        fn handle_channel_update(&self, _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<&PublicKey>) -> Option<msgs::NodeAnnouncement> { None }
+       fn get_next_node_announcement(&self, _starting_point: Option<&NodeId>) -> Option<msgs::NodeAnnouncement> { 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<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
                                                }
                                        },
                                        InitSyncTracker::ChannelsSyncing(_) => 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<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
                                                log_gossip!(self.logger, "Skipping broadcast message to {:?} as its outbound buffer is full", peer.their_node_id);
                                                continue;
                                        }
-                                       if peer.their_node_id.as_ref() == Some(&msg.contents.node_id) {
-                                               continue;
+                                       if let Some(their_node_id) = peer.their_node_id {
+                                               if NodeId::from_pubkey(&their_node_id) == msg.contents.node_id {
+                                                       continue;
+                                               }
                                        }
                                        if except_node.is_some() && peer.their_node_id.as_ref() == except_node {
                                                continue;
@@ -2023,7 +2025,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
                let announcement = msgs::UnsignedNodeAnnouncement {
                        features,
                        timestamp: self.last_node_announcement_serial.fetch_add(1, Ordering::AcqRel),
-                       node_id: self.node_signer.get_node_id(Recipient::Node).unwrap(),
+                       node_id: NodeId::from_pubkey(&self.node_signer.get_node_id(Recipient::Node).unwrap()),
                        rgb, alias, addresses,
                        excess_address_data: Vec::new(),
                        excess_data: Vec::new(),
index b72b49978fd8cef1bce5b3cd8a969ae847021453..a3331ab3bebdbc9fb1583ec1c1edd2d206e416f3 100644 (file)
@@ -385,10 +385,10 @@ where C::Target: chain::Access, L::Target: Logger
                None
        }
 
-       fn get_next_node_announcement(&self, starting_point: Option<&PublicKey>) -> Option<NodeAnnouncement> {
+       fn get_next_node_announcement(&self, starting_point: Option<&NodeId>) -> Option<NodeAnnouncement> {
                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<L: Deref> NetworkGraph<L> 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<L: Deref> NetworkGraph<L> 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: Fn(&mut UnsignedNodeAnnouncement)>(f: F, node_key: &SecretKey, secp_ctx: &Secp256k1<secp256k1::All>) -> 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);
index 8666a65ecbd31175f6a1b74a25bbc852f1cb3442..b737bab352441a1b2a9289cc143e078ba6384c97 100644 (file)
@@ -66,7 +66,7 @@ pub(super) fn add_or_update_node(
        gossip_sync: &P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
        secp_ctx: &Secp256k1<All>, 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,
index 6dd775ea30630e3211efe0e89fc092cfcdfbf9b1..be09c8f0065b29e1fff3eaed9c54380d11751d27 100644 (file)
@@ -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<msgs::NodeAnnouncement> {
+       fn get_next_node_announcement(&self, _starting_point: Option<&NodeId>) -> Option<msgs::NodeAnnouncement> {
                None
        }