Fix node announcement verification failure

src/fiber/gossip.rs

@@ -2079,13 +2079,12 @@ fn verify_node_announcement<S: GossipMessageStore>(
             )));
         }
     }
-    let message = node_announcement.message_to_sign();
-    match node_announcement.signature {
-        Some(ref signature) if signature.verify(&node_announcement.node_id, &message) => Ok(false),
-        _ => Err(Error::InvalidParameter(format!(
-            "Node announcement message signature verification failed: {:?}",
-            &node_announcement
-        ))),
+    if !node_announcement.verify() {
+        Err(Error::InvalidParameter(
+            "Node announcement message signature verification failed".to_string(),
+        ))
+    } else {
+        Ok(false)
     }
 }
 

src/fiber/tests/network.rs

@@ -513,6 +513,20 @@ async fn test_sync_node_announcement_on_startup() {
     assert!(node_info.is_some());
 }
 
+#[tokio::test]
+async fn test_sync_node_announcement_of_connected_nodes() {
+    let [node1, node2] = NetworkNode::new_n_interconnected_nodes().await;
+
+    // Wait for the broadcast message to be processed.
+    tokio::time::sleep(tokio::time::Duration::from_millis(500)).await;
+
+    let node_info = node1.get_network_graph_node(&node2.get_public_key()).await;
+    assert!(node_info.is_some());
+
+    let node_info = node2.get_network_graph_node(&node1.get_public_key()).await;
+    assert!(node_info.is_some());
+}
+
 // Test that we can sync the network graph with peers.
 // We will first create a node and announce a fake node announcement to the network.
 // Then we will create another node and connect to the first node.

src/fiber/tests/types.rs

@@ -1,14 +1,16 @@
 use crate::{
     fiber::{
+        config::AnnouncedNodeName,
         gen::{fiber as molecule_fiber, gossip},
         hash_algorithm::HashAlgorithm,
         types::{
-            secp256k1_instance, AddTlc, BroadcastMessageID, Cursor, Hash256, PaymentHopData,
-            PeeledOnionPacket, Privkey, Pubkey, TlcErr, TlcErrPacket, TlcErrorCode,
-            NO_SHARED_SECRET,
+            secp256k1_instance, AddTlc, BroadcastMessage, BroadcastMessageID, Cursor, Hash256,
+            NodeAnnouncement, PaymentHopData, PeeledOnionPacket, Privkey, Pubkey, TlcErr,
+            TlcErrPacket, TlcErrorCode, NO_SHARED_SECRET,
         },
     },
     gen_rand_channel_outpoint, gen_rand_fiber_private_key, gen_rand_fiber_public_key,
+    now_timestamp_as_millis_u64,
 };
 use fiber_sphinx::OnionSharedSecretIter;
 use secp256k1::{PublicKey, Secp256k1, SecretKey};
@@ -247,3 +249,64 @@ fn test_tlc_error_code() {
     let code = TlcErrorCode::try_from(code_int).expect("invalid code");
     assert_eq!(code, TlcErrorCode::IncorrectOrUnknownPaymentDetails);
 }
+
+#[test]
+fn test_create_and_verify_node_announcement() {
+    let privkey = gen_rand_fiber_private_key();
+    let node_announcement = NodeAnnouncement::new(
+        AnnouncedNodeName::from_str("node1").expect("valid name"),
+        vec![],
+        &privkey,
+        now_timestamp_as_millis_u64(),
+        0,
+    );
+    assert!(
+        node_announcement.verify(),
+        "Node announcement message signature verification failed: {:?}",
+        &node_announcement
+    );
+}
+
+#[test]
+fn test_serde_node_announcement() {
+    let privkey = gen_rand_fiber_private_key();
+    let node_announcement = NodeAnnouncement::new(
+        AnnouncedNodeName::from_str("node1").expect("valid name"),
+        vec![],
+        &privkey,
+        now_timestamp_as_millis_u64(),
+        0,
+    );
+    assert!(
+        node_announcement.verify(),
+        "Node announcement verification failed: {:?}",
+        &node_announcement
+    );
+    let serialized = bincode::serialize(&node_announcement).expect("serialize");
+    let deserialized: NodeAnnouncement = bincode::deserialize(&serialized).expect("deserialize");
+    assert_eq!(node_announcement, deserialized);
+    assert!(
+        deserialized.verify(),
+        "Node announcement verification failed: {:?}",
+        &deserialized
+    );
+}
+
+// There was a bug in the node announcement verification logic which uses local udt whitelist to
+// verify the signature. This bug causes different nodes to have different results on signature verification.
+// We add a few hard coded node announcements with different udt_cfg_infos to ensure the verification logic is correct.
+#[test]
+fn test_verify_hard_coded_node_announcement() {
+    for s in [
+        "000000000146000000000000003044022015c1b36c0f5d08cbcb7ac77939506495cbe6dbd4bdd6076de54e8cabe707f894022003c4e84c69e88906499e00e0bc9e601c727bead819233c0e1a2f66c59385f11f0000000000000000e67330889401000002a64b8993f33b2ebd37a4de1c9441f491291a4e779da8e519bcfb7c1f3f56c9c0200000000000000066696265722d310000000000000000000000000000000000000000000000000001000000000000002d00000000000000047f000001062098a503221220c9cf006bbaa881b6962c3a61f4dc7100aaedd875253d1bbb78408e2be7f5c93f420000000000000030783963306138666666323461376265333339623932303838373330633264633766616336646663626466306137333737346436643264366232393532336661356200e40b540200000002000000000000000a0000000000000053494d504c455f554454420000000000000030786531653335346436643634336164343237323464343039363765333334393834353334653033363734303563356165343261396437643633643737646634313905000000000000006461746131040000000000000030782e2a01e803000000000000000000000000000001000000000000000400000000000000636f6465420000000000000030786638393762666335313736366565396364623262393237396536336338616264626134623335623665653764646535666564396230613561343163393564633408000000040000000000000058554454420000000000000030783530626438643636383062386239636639386237336633633038666166386232613231393134333131393534313138616436363039626536653738613162393505000000000000006461746131040000000000000030782e2a01e803000000000000000000000000000001000000000000000400000000000000636f6465420000000000000030786638393762666335313736366565396364623262393237396536336338616264626134623335623665653764646535666564396230613561343163393564633409000000",
+        "000000000146000000000000003044022052158ADBFCEA30AEAF89CA00200DF0CC3D1E593EE635DB7FBFF01A28A34D07CF022065FD67E565540A1EFE99B939D2F084CBACEFCF15F2B17AA52CFFEE0614819C7B00000000000000004F3B0F889401000003781A50829680593CD47EDCCB646E62625212CF9AEA83EF4BE421A2B2C08872102000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000002D0000000000000004DDBB3DA2064734A50322122042F6793087F4481CEA9768D563FD8EBE6FEDA11E6A409CD84E92094CA69CE955420000000000000030783130363339653038393535303262353638386136626538636636393436306437363534316266613438323136323964383664363262613061616533663936303600E40B54020000000100000000000000040000000000000052555344420000000000000030783131343237353561303434626632656533353863626139663264613138376365393238633931636434646338363932646564303333376566613637376432316104000000000000007479706542000000000000003078383738666363366631663038643438653837626231633362336435303833663233663861333963356435633736346632353362353562393938353236343339620100CA9A3B00000000000000000000000001000000000000000400000000000000636F6465420000000000000030786564376436356239616433643939363537653337633432383564353835666561386135666361663538313635643534646163663930323433663931313534386200000000"
+    ] {
+        let bytes = hex::decode(s).expect("decode");
+
+        let node_announcement = match bincode::deserialize(&bytes).expect("deserialize") {
+            BroadcastMessage::NodeAnnouncement(node_announcement) => node_announcement,
+            _ => panic!("deserialize failed"),
+        };
+        assert!(node_announcement.verify())
+    }
+}

src/fiber/types.rs

@@ -1808,7 +1808,7 @@ impl NodeAnnouncement {
             chain_hash: self.chain_hash,
             addresses: self.addresses.clone(),
             auto_accept_min_ckb_funding_amount: self.auto_accept_min_ckb_funding_amount,
-            udt_cfg_infos: get_udt_whitelist(),
+            udt_cfg_infos: self.udt_cfg_infos.clone(),
         };
         deterministically_hash(&molecule_gossip::NodeAnnouncement::from(
             unsigned_announcement,
@@ -1825,6 +1825,14 @@ impl NodeAnnouncement {
             BroadcastMessageID::NodeAnnouncement(self.node_id),
         )
     }
+
+    pub fn verify(&self) -> bool {
+        let message = self.message_to_sign();
+        match self.signature {
+            Some(ref signature) => signature.verify(&self.node_id, &message),
+            _ => false,
+        }
+    }
 }
 
 impl From<UdtCellDep> for molecule_fiber::UdtCellDep {

src/store/store.rs

@@ -161,12 +161,12 @@ pub trait StoreKeyValue {
     fn value(&self) -> Vec<u8>;
 }
 
-fn serialize_to_vec<T: ?Sized + Serialize>(value: &T, field_name: &str) -> Vec<u8> {
+pub(crate) fn serialize_to_vec<T: ?Sized + Serialize>(value: &T, field_name: &str) -> Vec<u8> {
     bincode::serialize(value)
         .unwrap_or_else(|e| panic!("serialization of {} failed: {}", field_name, e))
 }
 
-fn deserialize_from<'a, T>(slice: &'a [u8], field_name: &str) -> T
+pub(crate) fn deserialize_from<'a, T>(slice: &'a [u8], field_name: &str) -> T
 where
     T: serde::Deserialize<'a>,
 {
@@ -670,6 +670,11 @@ impl GossipMessageStore for Store {
     }
 
     fn save_node_announcement(&self, node_announcement: crate::fiber::types::NodeAnnouncement) {
+        debug_assert!(
+            node_announcement.verify(),
+            "Node announcement must be verified: {:?}",
+            node_announcement
+        );
         let mut batch = self.batch();
         let message_id = BroadcastMessageID::NodeAnnouncement(node_announcement.node_id.clone());
 

src/store/tests/store.rs

@@ -9,10 +9,13 @@ use crate::fiber::history::TimedResult;
 use crate::fiber::network::SendPaymentData;
 use crate::fiber::tests::test_utils::*;
 use crate::fiber::types::*;
+use crate::gen_rand_fiber_private_key;
 use crate::gen_rand_fiber_public_key;
 use crate::gen_rand_sha256_hash;
 use crate::invoice::*;
 use crate::now_timestamp_as_millis_u64;
+use crate::store::store::deserialize_from;
+use crate::store::store::serialize_to_vec;
 use crate::store::Store;
 use crate::watchtower::*;
 use ckb_hash::new_blake2b;
@@ -589,3 +592,28 @@ fn test_store_payment_history() {
     sort_results(&mut r2);
     assert_eq!(r1, r2);
 }
+
+#[test]
+fn test_serde_node_announcement_as_broadcast_message() {
+    let privkey = gen_rand_fiber_private_key();
+    let node_announcement = NodeAnnouncement::new(
+        AnnouncedNodeName::from_str("node1").expect("valid name"),
+        vec![],
+        &privkey,
+        now_timestamp_as_millis_u64(),
+        0,
+    );
+    assert!(
+        node_announcement.verify(),
+        "Node announcement verification failed: {:?}",
+        &node_announcement
+    );
+    let broadcast_message = BroadcastMessage::NodeAnnouncement(node_announcement.clone());
+    let serialized = serialize_to_vec(&broadcast_message, "BroadcastMessage");
+    dbg!("serialized", hex::encode(&serialized));
+    let deserialized: BroadcastMessage = deserialize_from(serialized.as_ref(), "BroadcastMessage");
+    assert_eq!(
+        BroadcastMessage::NodeAnnouncement(node_announcement),
+        deserialized
+    );
+}