diff --git a/lightning-rapid-gossip-sync/src/processing.rs b/lightning-rapid-gossip-sync/src/processing.rs index 9d3287969f2..cce3dc29a59 100644 --- a/lightning-rapid-gossip-sync/src/processing.rs +++ b/lightning-rapid-gossip-sync/src/processing.rs @@ -549,7 +549,7 @@ mod tests { 108, 101, 46, 99, 111, 109, 1, 187, 19, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 5, 57, 13, 3, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 0, 2, 23, 48, 62, 77, 75, 108, 209, 54, 16, 50, 202, 155, 210, 174, 185, 217, 0, 170, 77, 69, 217, 234, 216, 10, 201, - 66, 51, 116, 196, 81, 167, 37, 77, 7, 102, 0, 0, 2, 25, 48, 0, 0, 0, 1, 0, 0, 1, 0, 1, + 66, 51, 116, 196, 81, 167, 37, 77, 7, 102, 0, 0, 2, 25, 48, 0, 0, 0, 1, 0, 0, 1, 1, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 1, ]; @@ -669,7 +669,7 @@ mod tests { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 1, 0, 0, 1, 0, 255, 128, 0, 0, 0, 0, 0, 0, 1, 0, 147, 42, 23, 23, 23, 23, 23, + 0, 0, 0, 1, 0, 0, 1, 1, 255, 128, 0, 0, 0, 0, 0, 0, 0, 0, 147, 42, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index adeb67a9e6c..f45cb0ae507 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -2014,9 +2014,9 @@ impl NetworkGraph { &self, short_channel_id: u64, capacity_sats: Option, timestamp: u64, features: ChannelFeatures, node_id_1: NodeId, node_id_2: NodeId, ) -> Result<(), LightningError> { - if node_id_1 == node_id_2 { + if node_id_1 >= node_id_2 { return Err(LightningError { - err: "Channel announcement node had a channel with itself".to_owned(), + err: "node_ids in channel_announcements must be sorted".to_owned(), action: ErrorAction::IgnoreError, }); }; @@ -2123,6 +2123,13 @@ impl NetworkGraph { ) -> Result<(), LightningError> { let channels = self.channels.read().unwrap(); + if msg.node_id_1 >= msg.node_id_2 { + return Err(LightningError { + err: "node_ids in channel_announcements must be sorted".to_owned(), + action: ErrorAction::IgnoreError, + }); + } + if let Some(chan) = channels.get(&msg.short_channel_id) { if chan.capacity_sats.is_some() { // If we'd previously looked up the channel on-chain and checked the script @@ -3126,7 +3133,7 @@ pub(crate) mod tests { .handle_channel_announcement(Some(node_1_pubkey), &channel_to_itself_announcement) { Ok(_) => panic!(), - Err(e) => assert_eq!(e.err, "Channel announcement node had a channel with itself"), + Err(e) => assert_eq!(e.err, "node_ids in channel_announcements must be sorted"), }; // Test that channel announcements with the wrong chain hash are ignored (network graph is testnet, diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs index 1592bc0ccb2..f921d9e21d7 100644 --- a/lightning/src/routing/scoring.rs +++ b/lightning/src/routing/scoring.rs @@ -2705,20 +2705,28 @@ mod tests { let node_1_secret = &SecretKey::from_slice(&[39; 32]).unwrap(); let node_2_secret = &SecretKey::from_slice(&[40; 32]).unwrap(); let secp_ctx = Secp256k1::new(); + let mut node_id_1 = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_1_key)); + let mut node_id_2 = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_2_key)); + let mut node_signer_1 = &node_1_key; + let mut node_signer_2 = &node_2_key; + if node_id_1 > node_id_2 { + core::mem::swap(&mut node_id_1, &mut node_id_2); + core::mem::swap(&mut node_signer_1, &mut node_signer_2); + } let unsigned_announcement = UnsignedChannelAnnouncement { features: channelmanager::provided_channel_features(&UserConfig::default()), chain_hash: genesis_hash, short_channel_id, - node_id_1: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_1_key)), - node_id_2: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_2_key)), + node_id_1, + node_id_2, bitcoin_key_1: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_1_secret)), bitcoin_key_2: NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_2_secret)), excess_data: Vec::new(), }; let msghash = hash_to_message!(&Sha256dHash::hash(&unsigned_announcement.encode()[..])[..]); let signed_announcement = ChannelAnnouncement { - node_signature_1: secp_ctx.sign_ecdsa(&msghash, &node_1_key), - node_signature_2: secp_ctx.sign_ecdsa(&msghash, &node_2_key), + node_signature_1: secp_ctx.sign_ecdsa(&msghash, node_signer_1), + node_signature_2: secp_ctx.sign_ecdsa(&msghash, node_signer_2), bitcoin_signature_1: secp_ctx.sign_ecdsa(&msghash, &node_1_secret), bitcoin_signature_2: secp_ctx.sign_ecdsa(&msghash, &node_2_secret), contents: unsigned_announcement, @@ -2732,10 +2740,23 @@ mod tests { fn update_channel( network_graph: &mut NetworkGraph<&TestLogger>, short_channel_id: u64, node_key: SecretKey, - channel_flags: u8, htlc_maximum_msat: u64, timestamp: u32, + mut channel_flags: u8, htlc_maximum_msat: u64, timestamp: u32, ) { let genesis_hash = ChainHash::using_genesis_block(Network::Testnet); let secp_ctx = Secp256k1::new(); + let node_id = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, &node_key)); + // `add_channel` may have swapped the node order to satisfy the spec's sorted node_ids + // requirement, so override `channel_flags` bit 0 to match the actual node position. + { + let read_only = network_graph.read_only(); + if let Some(channel) = read_only.channel(short_channel_id) { + if channel.node_one == node_id { + channel_flags &= !1; + } else { + channel_flags |= 1; + } + } + } let unsigned_update = UnsignedChannelUpdate { chain_hash: genesis_hash, short_channel_id, diff --git a/lightning/src/routing/test_utils.rs b/lightning/src/routing/test_utils.rs index a433fa30c5b..daaf65367c0 100644 --- a/lightning/src/routing/test_utils.rs +++ b/lightning/src/routing/test_utils.rs @@ -36,8 +36,14 @@ pub(crate) fn channel_announcement( node_1_privkey: &SecretKey, node_2_privkey: &SecretKey, features: ChannelFeatures, short_channel_id: u64, secp_ctx: &Secp256k1, ) -> ChannelAnnouncement { - let node_id_1 = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_1_privkey)); - let node_id_2 = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_2_privkey)); + let mut node_id_1 = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_1_privkey)); + let mut node_id_2 = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_2_privkey)); + let mut signer_1 = node_1_privkey; + let mut signer_2 = node_2_privkey; + if node_id_1 > node_id_2 { + core::mem::swap(&mut node_id_1, &mut node_id_2); + core::mem::swap(&mut signer_1, &mut signer_2); + } let unsigned_announcement = UnsignedChannelAnnouncement { features, @@ -52,10 +58,10 @@ pub(crate) fn channel_announcement( let msghash = hash_to_message!(&Sha256dHash::hash(&unsigned_announcement.encode()[..])[..]); ChannelAnnouncement { - node_signature_1: secp_ctx.sign_ecdsa(&msghash, node_1_privkey), - node_signature_2: secp_ctx.sign_ecdsa(&msghash, node_2_privkey), - bitcoin_signature_1: secp_ctx.sign_ecdsa(&msghash, node_1_privkey), - bitcoin_signature_2: secp_ctx.sign_ecdsa(&msghash, node_2_privkey), + node_signature_1: secp_ctx.sign_ecdsa(&msghash, signer_1), + node_signature_2: secp_ctx.sign_ecdsa(&msghash, signer_2), + bitcoin_signature_1: secp_ctx.sign_ecdsa(&msghash, signer_1), + bitcoin_signature_2: secp_ctx.sign_ecdsa(&msghash, signer_2), contents: unsigned_announcement.clone(), } } @@ -119,9 +125,25 @@ pub(crate) fn add_or_update_node( pub(crate) fn update_channel( gossip_sync: &P2PGossipSync>>, Arc, Arc>, - secp_ctx: &Secp256k1, node_privkey: &SecretKey, update: UnsignedChannelUpdate + secp_ctx: &Secp256k1, node_privkey: &SecretKey, mut update: UnsignedChannelUpdate ) { let node_pubkey = PublicKey::from_secret_key(&secp_ctx, node_privkey); + let node_id = NodeId::from_pubkey(&node_pubkey); + + // `channel_announcement` may have swapped the node order to satisfy the spec's sorted node_ids + // requirement, so override `channel_flags` bit 0 to match the actual node position recorded in + // the network graph. + { + let network_graph = gossip_sync.network_graph().read_only(); + if let Some(channel) = network_graph.channel(update.short_channel_id) { + if channel.node_one == node_id { + update.channel_flags &= !1; + } else { + update.channel_flags |= 1; + } + } + } + let msghash = hash_to_message!(&Sha256dHash::hash(&update.encode()[..])[..]); let valid_channel_update = ChannelUpdate { signature: secp_ctx.sign_ecdsa(&msghash, node_privkey),