Merge pull request #2016 from alecchendev/2023-02-gossip-msg-pubkey-to-nodeid

Swap gossip message `PublicKey` for `NodeId`

Author: TheBlueMatt

fuzz/src/router.rs

@@ -149,6 +149,15 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 		}
 	}
 
+	macro_rules! get_pubkey_from_node_id {
+		($node_id: expr ) => {
+			match PublicKey::from_slice($node_id.as_slice()) {
+				Ok(pk) => pk,
+				Err(_) => return,
+			}
+		}
+	}
+
 	macro_rules! get_pubkey {
 		() => {
 			match PublicKey::from_slice(get_slice!(33)) {
@@ -175,19 +184,19 @@ 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 => {
 				let msg = decode_msg_with_len16!(msgs::UnsignedChannelAnnouncement, 32+8+33*4);
-				node_pks.insert(msg.node_id_1);
-				node_pks.insert(msg.node_id_2);
+				node_pks.insert(get_pubkey_from_node_id!(msg.node_id_1));
+				node_pks.insert(get_pubkey_from_node_id!(msg.node_id_2));
 				let _ = net_graph.update_channel_from_unsigned_announcement::<&FuzzChainSource>(&msg, &None);
 			},
 			2 => {
 				let msg = decode_msg_with_len16!(msgs::UnsignedChannelAnnouncement, 32+8+33*4);
-				node_pks.insert(msg.node_id_1);
-				node_pks.insert(msg.node_id_2);
+				node_pks.insert(get_pubkey_from_node_id!(msg.node_id_1));
+				node_pks.insert(get_pubkey_from_node_id!(msg.node_id_2));
 				let _ = net_graph.update_channel_from_unsigned_announcement(&msg, &Some(&FuzzChainSource { input: Arc::clone(&input) }));
 			},
 			3 => {

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<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(()) }

lightning/src/ln/chan_utils.rs

@@ -23,6 +23,7 @@ use bitcoin::hash_types::{Txid, PubkeyHash};
 
 use crate::ln::{PaymentHash, PaymentPreimage};
 use crate::ln::msgs::DecodeError;
+use crate::routing::gossip::NodeId;
 use crate::util::ser::{Readable, Writeable, Writer};
 use crate::util::transaction_utils;
 
@@ -660,13 +661,17 @@ pub fn make_funding_redeemscript(broadcaster: &PublicKey, countersignatory: &Pub
 	let broadcaster_funding_key = broadcaster.serialize();
 	let countersignatory_funding_key = countersignatory.serialize();
 
+	make_funding_redeemscript_from_slices(&broadcaster_funding_key, &countersignatory_funding_key)
+}
+
+pub(crate) fn make_funding_redeemscript_from_slices(broadcaster_funding_key: &[u8], countersignatory_funding_key: &[u8]) -> Script {
 	let builder = Builder::new().push_opcode(opcodes::all::OP_PUSHNUM_2);
 	if broadcaster_funding_key[..] < countersignatory_funding_key[..] {
-		builder.push_slice(&broadcaster_funding_key)
-			.push_slice(&countersignatory_funding_key)
+		builder.push_slice(broadcaster_funding_key)
+			.push_slice(countersignatory_funding_key)
 	} else {
-		builder.push_slice(&countersignatory_funding_key)
-			.push_slice(&broadcaster_funding_key)
+		builder.push_slice(countersignatory_funding_key)
+			.push_slice(broadcaster_funding_key)
 	}.push_opcode(opcodes::all::OP_PUSHNUM_2).push_opcode(opcodes::all::OP_CHECKMULTISIG).into_script()
 }
 

lightning/src/ln/channel.rs

@@ -36,6 +36,7 @@ use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBounde
 use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS};
 use crate::chain::transaction::{OutPoint, TransactionData};
 use crate::chain::keysinterface::{WriteableEcdsaChannelSigner, EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient};
+use crate::routing::gossip::NodeId;
 use crate::util::events::ClosureReason;
 use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
 use crate::util::logger::Logger;
@@ -5416,18 +5417,19 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 			return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel is not currently usable".to_owned()));
 		}
 
-		let node_id = node_signer.get_node_id(Recipient::Node)
-			.map_err(|_| ChannelError::Ignore("Failed to retrieve own public key".to_owned()))?;
-		let were_node_one = node_id.serialize()[..] < self.counterparty_node_id.serialize()[..];
+		let node_id = NodeId::from_pubkey(&node_signer.get_node_id(Recipient::Node)
+			.map_err(|_| ChannelError::Ignore("Failed to retrieve own public key".to_owned()))?);
+		let counterparty_node_id = NodeId::from_pubkey(&self.get_counterparty_node_id());
+		let were_node_one = node_id.as_slice() < counterparty_node_id.as_slice();
 
 		let msg = msgs::UnsignedChannelAnnouncement {
 			features: channelmanager::provided_channel_features(&user_config),
 			chain_hash,
 			short_channel_id: self.get_short_channel_id().unwrap(),
-			node_id_1: if were_node_one { node_id } else { self.get_counterparty_node_id() },
-			node_id_2: if were_node_one { self.get_counterparty_node_id() } else { node_id },
-			bitcoin_key_1: if were_node_one { self.get_holder_pubkeys().funding_pubkey } else { self.counterparty_funding_pubkey().clone() },
-			bitcoin_key_2: if were_node_one { self.counterparty_funding_pubkey().clone() } else { self.get_holder_pubkeys().funding_pubkey },
+			node_id_1: if were_node_one { node_id } else { counterparty_node_id },
+			node_id_2: if were_node_one { counterparty_node_id } else { node_id },
+			bitcoin_key_1: NodeId::from_pubkey(if were_node_one { &self.get_holder_pubkeys().funding_pubkey } else { self.counterparty_funding_pubkey() }),
+			bitcoin_key_2: NodeId::from_pubkey(if were_node_one { self.counterparty_funding_pubkey() } else { &self.get_holder_pubkeys().funding_pubkey }),
 			excess_data: Vec::new(),
 		};
 
@@ -5497,8 +5499,8 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 		&self, node_signer: &NS, announcement: msgs::UnsignedChannelAnnouncement
 	) -> Result<msgs::ChannelAnnouncement, ChannelError> where NS::Target: NodeSigner {
 		if let Some((their_node_sig, their_bitcoin_sig)) = self.announcement_sigs {
-			let our_node_key = node_signer.get_node_id(Recipient::Node)
-				.map_err(|_| ChannelError::Ignore("Signer failed to retrieve own public key".to_owned()))?;
+			let our_node_key = NodeId::from_pubkey(&node_signer.get_node_id(Recipient::Node)
+				.map_err(|_| ChannelError::Ignore("Signer failed to retrieve own public key".to_owned()))?);
 			let were_node_one = announcement.node_id_1 == our_node_key;
 
 			let our_node_sig = node_signer.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelAnnouncement(&announcement))

lightning/src/ln/msgs.rs

@@ -46,6 +46,8 @@ use crate::util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer
 
 use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
 
+use crate::routing::gossip::NodeId;
+
 /// 21 million * 10^8 * 1000
 pub(crate) const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;
 
@@ -663,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.
@@ -698,13 +700,13 @@ pub struct UnsignedChannelAnnouncement {
 	/// The short channel ID
 	pub short_channel_id: u64,
 	/// One of the two `node_id`s which are endpoints of this channel
-	pub node_id_1: PublicKey,
+	pub node_id_1: NodeId,
 	/// The other of the two `node_id`s which are endpoints of this channel
-	pub node_id_2: PublicKey,
+	pub node_id_2: NodeId,
 	/// The funding key for the first node
-	pub bitcoin_key_1: PublicKey,
+	pub bitcoin_key_1: NodeId,
 	/// The funding key for the second node
-	pub bitcoin_key_2: PublicKey,
+	pub bitcoin_key_2: NodeId,
 	pub(crate) excess_data: Vec<u8>,
 }
 /// A [`channel_announcement`] message to be sent to or received from a peer.
@@ -1055,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.
@@ -1843,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)?;
@@ -2053,6 +2055,7 @@ mod tests {
 	use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
 	use crate::ln::msgs;
 	use crate::ln::msgs::{FinalOnionHopData, OptionalField, OnionErrorPacket, OnionHopDataFormat};
+	use crate::routing::gossip::NodeId;
 	use crate::util::ser::{Writeable, Readable, Hostname};
 
 	use bitcoin::hashes::hex::FromHex;
@@ -2160,10 +2163,10 @@ mod tests {
 			features,
 			chain_hash: BlockHash::from_hex("6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000").unwrap(),
 			short_channel_id: 2316138423780173,
-			node_id_1: pubkey_1,
-			node_id_2: pubkey_2,
-			bitcoin_key_1: pubkey_3,
-			bitcoin_key_2: pubkey_4,
+			node_id_1: NodeId::from_pubkey(&pubkey_1),
+			node_id_2: NodeId::from_pubkey(&pubkey_2),
+			bitcoin_key_1: NodeId::from_pubkey(&pubkey_3),
+			bitcoin_key_2: NodeId::from_pubkey(&pubkey_4),
 			excess_data: if excess_data { vec![10, 0, 0, 20, 0, 0, 30, 0, 0, 40] } else { Vec::new() },
 		};
 		let channel_announcement = msgs::ChannelAnnouncement {
@@ -2245,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,

lightning/src/ln/peer_handler.rs

@@ -27,7 +27,7 @@ use crate::ln::peer_channel_encryptor::{PeerChannelEncryptor,NextNoiseStep};
 use crate::ln::wire;
 use crate::ln::wire::Encode;
 use crate::onion_message::{CustomOnionMessageContents, CustomOnionMessageHandler, SimpleArcOnionMessenger, SimpleRefOnionMessenger};
-use crate::routing::gossip::{NetworkGraph, P2PGossipSync};
+use crate::routing::gossip::{NetworkGraph, P2PGossipSync, NodeId};
 use crate::util::atomic_counter::AtomicCounter;
 use crate::util::events::{MessageSendEvent, MessageSendEventsProvider, OnionMessageProvider};
 use crate::util::logger::Logger;
@@ -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,15 +434,15 @@ 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;
 			}
 		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 {
@@ -1467,9 +1467,11 @@ 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_1) ||
-					   peer.their_node_id.as_ref() == Some(&msg.contents.node_id_2) {
-						continue;
+					if let Some(their_node_id) = peer.their_node_id {
+						let their_node_id = NodeId::from_pubkey(&their_node_id);
+						if their_node_id == msg.contents.node_id_1 || their_node_id == msg.contents.node_id_2 {
+							continue;
+						}
 					}
 					if except_node.is_some() && peer.their_node_id.as_ref() == except_node {
 						continue;
@@ -1491,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;
@@ -2021,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(),