From a9b18e937103c69d2c54ffca09ea87919c7920b9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 1 Jul 2025 02:16:44 +0000 Subject: [PATCH] Include node details when the node is new In cases where we only receive node details for the first time after the last-seen timestamp, we were mistakenly not including the node info at all in the snapshot. This was the most egregious for initial syncs, where we'd include no node info at all. Luckily the fix is simple, just include a full node info sync in such cases. Fixes #100 --- src/serialization.rs | 5 ++- src/tests/mod.rs | 89 +++++++++++++++++++++++++++++++------------- 2 files changed, 68 insertions(+), 26 deletions(-) diff --git a/src/serialization.rs b/src/serialization.rs index cb38545c..ab2ae816 100644 --- a/src/serialization.rs +++ b/src/serialization.rs @@ -240,7 +240,10 @@ pub(super) fn serialize_delta_set(channel_delta_set: DeltaSet, node_delta_set: N } Some((id, delta)) } else { - None + // If the node details didn't exist before the last-update timestamp, send a full + // update. + delta.strategy = Some(NodeSerializationStrategy::Full); + Some((id, delta)) } }).collect(); diff --git a/src/tests/mod.rs b/src/tests/mod.rs index cc231cac..b23fd55c 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -75,14 +75,12 @@ fn generate_node_announcement(private_key: Option) -> NodeAnnouncemen } -fn generate_channel_announcement(short_channel_id: u64) -> ChannelAnnouncement { +fn generate_channel_announcement_between_nodes(short_channel_id: u64, random_private_key_1: &SecretKey, random_private_key_2: &SecretKey) -> ChannelAnnouncement { let secp_context = Secp256k1::new(); - let random_private_key_1 = SecretKey::from_slice(&[1; 32]).unwrap(); let random_public_key_1 = random_private_key_1.public_key(&secp_context); let node_id_1 = NodeId::from_pubkey(&random_public_key_1); - let random_private_key_2 = SecretKey::from_slice(&[2; 32]).unwrap(); let random_public_key_2 = random_private_key_2.public_key(&secp_context); let node_id_2 = NodeId::from_pubkey(&random_public_key_2); @@ -98,8 +96,8 @@ fn generate_channel_announcement(short_channel_id: u64) -> ChannelAnnouncement { }; let msg_hash = bitcoin::secp256k1::Message::from_slice(&Sha256dHash::hash(&announcement.encode()[..])[..]).unwrap(); - let node_signature_1 = secp_context.sign_ecdsa(&msg_hash, &random_private_key_1); - let node_signature_2 = secp_context.sign_ecdsa(&msg_hash, &random_private_key_2); + let node_signature_1 = secp_context.sign_ecdsa(&msg_hash, random_private_key_1); + let node_signature_2 = secp_context.sign_ecdsa(&msg_hash, random_private_key_2); ChannelAnnouncement { node_signature_1, @@ -110,6 +108,12 @@ fn generate_channel_announcement(short_channel_id: u64) -> ChannelAnnouncement { } } +fn generate_channel_announcement(short_channel_id: u64) -> ChannelAnnouncement { + let random_private_key_1 = SecretKey::from_slice(&[1; 32]).unwrap(); + let random_private_key_2 = SecretKey::from_slice(&[2; 32]).unwrap(); + generate_channel_announcement_between_nodes(short_channel_id, &random_private_key_1, &random_private_key_2) +} + fn generate_update(scid: u64, direction: bool, timestamp: u32, expiry_delta: u16, min_msat: u64, max_msat: u64, base_msat: u32, fee_rate: u32) -> ChannelUpdate { let flag_mask = if direction { 1 } else { 0 }; ChannelUpdate { @@ -360,30 +364,59 @@ async fn test_node_announcement_delta_detection() { let timestamp = current_time() - 10; { // seed the db + let third_node = SecretKey::from_slice(&[3; 32]).unwrap(); + let fourth_node = SecretKey::from_slice(&[4; 32]).unwrap(); { // necessary for the node announcements to be considered relevant let announcement = generate_channel_announcement(1); - let update_1 = generate_update(1, false, timestamp, 0, 0, 0, 6, 0); - let update_2 = generate_update(1, true, timestamp, 0, 0, 0, 6, 0); + let update_1 = generate_update(1, false, timestamp - 10, 0, 0, 0, 6, 0); + let update_2 = generate_update(1, true, timestamp - 10, 0, 0, 0, 6, 0); network_graph_arc.update_channel_from_announcement_no_lookup(&announcement).unwrap(); network_graph_arc.update_channel_unsigned(&update_1.contents).unwrap(); network_graph_arc.update_channel_unsigned(&update_2.contents).unwrap(); - receiver.send(GossipMessage::ChannelAnnouncement(announcement, 100, Some(timestamp))).await.unwrap(); - receiver.send(GossipMessage::ChannelUpdate(update_1, Some(timestamp))).await.unwrap(); - receiver.send(GossipMessage::ChannelUpdate(update_2, Some(timestamp))).await.unwrap(); + receiver.send(GossipMessage::ChannelAnnouncement(announcement, 100, Some(timestamp - 10))).await.unwrap(); + receiver.send(GossipMessage::ChannelUpdate(update_1, Some(timestamp - 10))).await.unwrap(); + receiver.send(GossipMessage::ChannelUpdate(update_2, Some(timestamp - 10))).await.unwrap(); } - let mut announcement = generate_node_announcement(None); - announcement.contents.timestamp = timestamp - 10; - network_graph_arc.update_node_from_unsigned_announcement(&announcement.contents).unwrap(); - receiver.send(GossipMessage::NodeAnnouncement(announcement.clone(), Some(announcement.contents.timestamp))).await.unwrap(); - announcement.contents.timestamp = timestamp - 8; - network_graph_arc.update_node_from_unsigned_announcement(&announcement.contents).unwrap(); - receiver.send(GossipMessage::NodeAnnouncement(announcement.clone(), Some(announcement.contents.timestamp))).await.unwrap(); + { // necessary for the second node announcements to be considered relevant + let announcement = generate_channel_announcement_between_nodes(2, &third_node, &fourth_node); + let update_1 = generate_update(2, false, timestamp - 10, 0, 0, 0, 6, 0); + let update_2 = generate_update(2, true, timestamp - 10, 0, 0, 0, 6, 0); + + network_graph_arc.update_channel_from_announcement_no_lookup(&announcement).unwrap(); + network_graph_arc.update_channel_unsigned(&update_1.contents).unwrap(); + network_graph_arc.update_channel_unsigned(&update_2.contents).unwrap(); + + receiver.send(GossipMessage::ChannelAnnouncement(announcement, 100, Some(timestamp - 10))).await.unwrap(); + receiver.send(GossipMessage::ChannelUpdate(update_1, Some(timestamp - 10))).await.unwrap(); + receiver.send(GossipMessage::ChannelUpdate(update_2, Some(timestamp - 10))).await.unwrap(); + } + + { + // Add some node announcements from before the last sync for node 1. + let mut announcement = generate_node_announcement(None); + announcement.contents.timestamp = timestamp - 10; + network_graph_arc.update_node_from_unsigned_announcement(&announcement.contents).unwrap(); + receiver.send(GossipMessage::NodeAnnouncement(announcement.clone(), Some(announcement.contents.timestamp))).await.unwrap(); + announcement.contents.timestamp = timestamp - 8; + network_graph_arc.update_node_from_unsigned_announcement(&announcement.contents).unwrap(); + receiver.send(GossipMessage::NodeAnnouncement(announcement.clone(), Some(announcement.contents.timestamp))).await.unwrap(); + } + + { + // Add a node announcement from before the last sync for node 4. + let mut announcement = generate_node_announcement(Some(fourth_node.clone())); + announcement.contents.timestamp = timestamp - 10; + network_graph_arc.update_node_from_unsigned_announcement(&announcement.contents).unwrap(); + receiver.send(GossipMessage::NodeAnnouncement(announcement.clone(), Some(announcement.contents.timestamp))).await.unwrap(); + } { + // Add current announcement for node two, which should be included in its entirety as + // its new since the last sync time let mut current_announcement = generate_node_announcement(Some(SecretKey::from_slice(&[2; 32]).unwrap())); current_announcement.contents.features = NodeFeatures::from_be_bytes(vec![23, 48]); current_announcement.contents.timestamp = timestamp; @@ -392,19 +425,22 @@ async fn test_node_announcement_delta_detection() { } { - let mut current_announcement = generate_node_announcement(Some(SecretKey::from_slice(&[3; 32]).unwrap())); + // Note that we do *not* add the node announcement for node three to the network graph, + // simulating its channels having timed out, and thus the node announcement (which is + // new) will not be included. + let mut current_announcement = generate_node_announcement(Some(third_node)); current_announcement.contents.features = NodeFeatures::from_be_bytes(vec![22, 49]); current_announcement.contents.timestamp = timestamp; receiver.send(GossipMessage::NodeAnnouncement(current_announcement, Some(timestamp))).await.unwrap(); } { - // modify announcement to contain a bunch of addresses + // modify announcement of node 1 to contain a bunch of addresses + let mut announcement = generate_node_announcement(None); announcement.contents.addresses.push(SocketAddress::Hostname { hostname: "google.com".to_string().try_into().unwrap(), port: 443, }); - announcement.contents.features = NodeFeatures::from_be_bytes(vec![23, 48]); announcement.contents.addresses.push(SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: 9635 }); announcement.contents.addresses.push(SocketAddress::TcpIpV6 { addr: [1; 16], port: 1337 }); announcement.contents.addresses.push(SocketAddress::OnionV2([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12])); @@ -415,9 +451,9 @@ async fn test_node_announcement_delta_detection() { port: 4, }); announcement.contents.timestamp = timestamp; + network_graph_arc.update_node_from_unsigned_announcement(&announcement.contents).unwrap(); + receiver.send(GossipMessage::NodeAnnouncement(announcement, Some(timestamp))).await.unwrap(); } - network_graph_arc.update_node_from_unsigned_announcement(&announcement.contents).unwrap(); - receiver.send(GossipMessage::NodeAnnouncement(announcement, Some(timestamp))).await.unwrap(); drop(receiver); persister.persist_gossip().await; @@ -431,11 +467,14 @@ async fn test_node_announcement_delta_detection() { let serialization = serialize_delta(&delta, 2, logger.clone()); clean_test_db().await; - assert_eq!(serialization.message_count, 3); + // All channel updates completed prior to the last sync timestamp, so none are included. + assert_eq!(serialization.message_count, 0); + // We should have updated addresses for node 1, full announcements for node 2 and 3, and no + // announcement for node 4. assert_eq!(serialization.node_announcement_count, 2); - assert_eq!(serialization.node_update_count, 1); + assert_eq!(serialization.node_update_count, 2); assert_eq!(serialization.node_feature_update_count, 1); - assert_eq!(serialization.node_address_update_count, 1); + assert_eq!(serialization.node_address_update_count, 2); } /// If a channel has only seen updates in one direction, it should not be announced