Skip to content

Commit e60d06f

Browse files
committed
Replace PublicKey with [u8; 33] in NetworkGraph
1 parent 7aa2cac commit e60d06f

File tree

8 files changed

+130
-131
lines changed

8 files changed

+130
-131
lines changed

lightning-invoice/src/de.rs

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use std::convert::TryInto;
12
use std::error;
23
use std::fmt;
34
use std::fmt::{Display, Formatter};
@@ -600,7 +601,7 @@ impl FromBase32 for PrivateRoute {
600601
channel_id.copy_from_slice(&hop_bytes[33..41]);
601602

602603
let hop = RouteHintHop {
603-
src_node_id: PublicKey::from_slice(&hop_bytes[0..33])?,
604+
src_node_id: hop_bytes[0..33].try_into().map_err(|_| ParseError::InvalidSliceLength("PrivateRoute::from_base32()".into()))?,
604605
short_channel_id: parse_int_be(&channel_id, 256).expect("short chan ID slice too big?"),
605606
fees: RoutingFees {
606607
base_msat: parse_int_be(&hop_bytes[41..45], 256).expect("slice too big?"),
@@ -963,13 +964,11 @@ mod test {
963964

964965
let mut expected = Vec::<RouteHintHop>::new();
965966
expected.push(RouteHintHop {
966-
src_node_id: PublicKey::from_slice(
967-
&[
968-
0x02u8, 0x9e, 0x03, 0xa9, 0x01, 0xb8, 0x55, 0x34, 0xff, 0x1e, 0x92, 0xc4, 0x3c,
969-
0x74, 0x43, 0x1f, 0x7c, 0xe7, 0x20, 0x46, 0x06, 0x0f, 0xcf, 0x7a, 0x95, 0xc3,
970-
0x7e, 0x14, 0x8f, 0x78, 0xc7, 0x72, 0x55
971-
][..]
972-
).unwrap(),
967+
src_node_id: [
968+
0x02u8, 0x9e, 0x03, 0xa9, 0x01, 0xb8, 0x55, 0x34, 0xff, 0x1e, 0x92, 0xc4, 0x3c,
969+
0x74, 0x43, 0x1f, 0x7c, 0xe7, 0x20, 0x46, 0x06, 0x0f, 0xcf, 0x7a, 0x95, 0xc3,
970+
0x7e, 0x14, 0x8f, 0x78, 0xc7, 0x72, 0x55
971+
],
973972
short_channel_id: parse_int_be(&[0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08], 256).expect("short chan ID slice too big?"),
974973
fees: RoutingFees {
975974
base_msat: 1,
@@ -980,13 +979,11 @@ mod test {
980979
htlc_maximum_msat: None
981980
});
982981
expected.push(RouteHintHop {
983-
src_node_id: PublicKey::from_slice(
984-
&[
985-
0x03u8, 0x9e, 0x03, 0xa9, 0x01, 0xb8, 0x55, 0x34, 0xff, 0x1e, 0x92, 0xc4, 0x3c,
986-
0x74, 0x43, 0x1f, 0x7c, 0xe7, 0x20, 0x46, 0x06, 0x0f, 0xcf, 0x7a, 0x95, 0xc3,
987-
0x7e, 0x14, 0x8f, 0x78, 0xc7, 0x72, 0x55
988-
][..]
989-
).unwrap(),
982+
src_node_id: [
983+
0x03u8, 0x9e, 0x03, 0xa9, 0x01, 0xb8, 0x55, 0x34, 0xff, 0x1e, 0x92, 0xc4, 0x3c,
984+
0x74, 0x43, 0x1f, 0x7c, 0xe7, 0x20, 0x46, 0x06, 0x0f, 0xcf, 0x7a, 0x95, 0xc3,
985+
0x7e, 0x14, 0x8f, 0x78, 0xc7, 0x72, 0x55
986+
],
990987
short_channel_id: parse_int_be(&[0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a], 256).expect("short chan ID slice too big?"),
991988
fees: RoutingFees {
992989
base_msat: 2,

lightning-invoice/src/lib.rs

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,7 +1728,6 @@ mod test {
17281728
use ::*;
17291729
use lightning::routing::router::RouteHintHop;
17301730
use std::iter::FromIterator;
1731-
use secp256k1::key::PublicKey;
17321731

17331732
let builder = InvoiceBuilder::new(Currency::Bitcoin)
17341733
.payment_hash(sha256::Hash::from_slice(&[0;32][..]).unwrap())
@@ -1745,13 +1744,11 @@ mod test {
17451744
assert_eq!(long_desc_res, Err(CreationError::DescriptionTooLong));
17461745

17471746
let route_hop = RouteHintHop {
1748-
src_node_id: PublicKey::from_slice(
1749-
&[
1750-
0x03, 0x9e, 0x03, 0xa9, 0x01, 0xb8, 0x55, 0x34, 0xff, 0x1e, 0x92, 0xc4,
1751-
0x3c, 0x74, 0x43, 0x1f, 0x7c, 0xe7, 0x20, 0x46, 0x06, 0x0f, 0xcf, 0x7a,
1752-
0x95, 0xc3, 0x7e, 0x14, 0x8f, 0x78, 0xc7, 0x72, 0x55
1753-
][..]
1754-
).unwrap(),
1747+
src_node_id: [
1748+
0x03, 0x9e, 0x03, 0xa9, 0x01, 0xb8, 0x55, 0x34, 0xff, 0x1e, 0x92, 0xc4,
1749+
0x3c, 0x74, 0x43, 0x1f, 0x7c, 0xe7, 0x20, 0x46, 0x06, 0x0f, 0xcf, 0x7a,
1750+
0x95, 0xc3, 0x7e, 0x14, 0x8f, 0x78, 0xc7, 0x72, 0x55
1751+
],
17551752
short_channel_id: 0,
17561753
fees: RoutingFees {
17571754
base_msat: 0,
@@ -1798,7 +1795,7 @@ mod test {
17981795

17991796
let route_1 = RouteHint(vec![
18001797
RouteHintHop {
1801-
src_node_id: public_key.clone(),
1798+
src_node_id: public_key.serialize(),
18021799
short_channel_id: de::parse_int_be(&[123; 8], 256).expect("short chan ID slice too big?"),
18031800
fees: RoutingFees {
18041801
base_msat: 2,
@@ -1809,7 +1806,7 @@ mod test {
18091806
htlc_maximum_msat: None,
18101807
},
18111808
RouteHintHop {
1812-
src_node_id: public_key.clone(),
1809+
src_node_id: public_key.serialize(),
18131810
short_channel_id: de::parse_int_be(&[42; 8], 256).expect("short chan ID slice too big?"),
18141811
fees: RoutingFees {
18151812
base_msat: 3,
@@ -1823,7 +1820,7 @@ mod test {
18231820

18241821
let route_2 = RouteHint(vec![
18251822
RouteHintHop {
1826-
src_node_id: public_key.clone(),
1823+
src_node_id: public_key.serialize(),
18271824
short_channel_id: 0,
18281825
fees: RoutingFees {
18291826
base_msat: 4,
@@ -1834,7 +1831,7 @@ mod test {
18341831
htlc_maximum_msat: None,
18351832
},
18361833
RouteHintHop {
1837-
src_node_id: public_key.clone(),
1834+
src_node_id: public_key.serialize(),
18381835
short_channel_id: de::parse_int_be(&[1; 8], 256).expect("short chan ID slice too big?"),
18391836
fees: RoutingFees {
18401837
base_msat: 5,

lightning-invoice/src/ser.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ impl ToBase32 for PrivateRoute {
361361
let mut converter = BytesToBase32::new(writer);
362362

363363
for hop in (self.0).0.iter() {
364-
converter.append(&hop.src_node_id.serialize()[..])?;
364+
converter.append(&hop.src_node_id[..])?;
365365
let short_channel_id = try_stretch(
366366
encode_int_be_base256(hop.short_channel_id),
367367
8

lightning-invoice/src/utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ where
4141
None => continue,
4242
};
4343
route_hints.push(RouteHint(vec![RouteHintHop {
44-
src_node_id: channel.counterparty.node_id,
44+
src_node_id: channel.counterparty.node_id.serialize(),
4545
short_channel_id,
4646
fees: RoutingFees {
4747
base_msat: forwarding_info.fee_base_msat,

lightning-invoice/tests/ser_de.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ use lightning::ln::PaymentSecret;
1212
use lightning::routing::router::{RouteHint, RouteHintHop};
1313
use lightning::routing::network_graph::RoutingFees;
1414
use lightning_invoice::*;
15-
use secp256k1::PublicKey;
1615
use secp256k1::recovery::{RecoverableSignature, RecoveryId};
1716
use std::collections::HashSet;
1817
use std::time::{Duration, UNIX_EPOCH};
1918
use std::str::FromStr;
19+
use std::convert::TryInto;
2020

2121
fn get_test_tuples() -> Vec<(String, SignedRawInvoice, bool, bool)> {
2222
vec![
@@ -139,17 +139,13 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, bool, bool)> {
139139
).unwrap())
140140
.fallback(Fallback::PubKeyHash([4, 182, 31, 125, 193, 234, 13, 201, 148, 36, 70, 76, 196, 6, 77, 197, 100, 217, 30, 137]))
141141
.private_route(RouteHint(vec![RouteHintHop {
142-
src_node_id: PublicKey::from_slice(&hex::decode(
143-
"029e03a901b85534ff1e92c43c74431f7ce72046060fcf7a95c37e148f78c77255"
144-
).unwrap()).unwrap(),
142+
src_node_id: hex::decode("029e03a901b85534ff1e92c43c74431f7ce72046060fcf7a95c37e148f78c77255").unwrap().try_into().unwrap(),
145143
short_channel_id: (66051 << 40) | (263430 << 16) | 1800,
146144
fees: RoutingFees { base_msat: 1, proportional_millionths: 20 },
147145
cltv_expiry_delta: 3,
148146
htlc_maximum_msat: None, htlc_minimum_msat: None,
149147
}, RouteHintHop {
150-
src_node_id: PublicKey::from_slice(&hex::decode(
151-
"039e03a901b85534ff1e92c43c74431f7ce72046060fcf7a95c37e148f78c77255"
152-
).unwrap()).unwrap(),
148+
src_node_id: hex::decode("039e03a901b85534ff1e92c43c74431f7ce72046060fcf7a95c37e148f78c77255").unwrap().try_into().unwrap(),
153149
short_channel_id: (197637 << 40) | (395016 << 16) | 2314,
154150
fees: RoutingFees { base_msat: 2, proportional_millionths: 30 },
155151
cltv_expiry_delta: 4,
@@ -249,9 +245,7 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, bool, bool)> {
249245
.min_final_cltv_expiry(10)
250246
.description("Blockstream Store: 88.85 USD for Blockstream Ledger Nano S x 1, \"Back In My Day\" Sticker x 2, \"I Got Lightning Working\" Sticker x 2 and 1 more items".to_owned())
251247
.private_route(RouteHint(vec![RouteHintHop {
252-
src_node_id: PublicKey::from_slice(&hex::decode(
253-
"03d06758583bb5154774a6eb221b1276c9e82d65bbaceca806d90e20c108f4b1c7"
254-
).unwrap()).unwrap(),
248+
src_node_id: hex::decode("03d06758583bb5154774a6eb221b1276c9e82d65bbaceca806d90e20c108f4b1c7").unwrap().try_into().unwrap(),
255249
short_channel_id: (589390 << 40) | (3312 << 16) | 1,
256250
fees: RoutingFees { base_msat: 1000, proportional_millionths: 2500 },
257251
cltv_expiry_delta: 40,

lightning/src/ln/functional_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7801,7 +7801,7 @@ fn test_priv_forwarding_rejection() {
78017801
&nodes[0].net_graph_msg_handler.network_graph,
78027802
&nodes[2].node.get_our_node_id(), Some(InvoiceFeatures::known()), None,
78037803
&[&RouteHint(vec![RouteHintHop {
7804-
src_node_id: nodes[1].node.get_our_node_id(),
7804+
src_node_id: nodes[1].node.get_our_node_id().serialize(),
78057805
short_channel_id: nodes[2].node.list_channels()[0].short_channel_id.unwrap(),
78067806
fees: RoutingFees { base_msat: 1000, proportional_millionths: 0 },
78077807
cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA,

lightning/src/routing/network_graph.rs

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,15 @@ const MAX_EXCESS_BYTES_FOR_RELAY: usize = 1024;
5050
/// This value ensures a reply fits within the 65k payload limit and is consistent with other implementations.
5151
const MAX_SCIDS_PER_REPLY: usize = 8000;
5252

53+
/// Represents the compressed public key of a node
54+
pub type NodeId = [u8; 33];
55+
5356
/// Represents the network as nodes and channels between them
5457
pub struct NetworkGraph {
5558
genesis_hash: BlockHash,
5659
// Lock order: channels -> nodes
5760
channels: RwLock<BTreeMap<u64, ChannelInfo>>,
58-
nodes: RwLock<BTreeMap<PublicKey, NodeInfo>>,
61+
nodes: RwLock<BTreeMap<NodeId, NodeInfo>>,
5962
}
6063

6164
impl Clone for NetworkGraph {
@@ -73,7 +76,7 @@ impl Clone for NetworkGraph {
7376
/// A read-only view of [`NetworkGraph`].
7477
pub struct ReadOnlyNetworkGraph<'a> {
7578
channels: RwLockReadGuard<'a, BTreeMap<u64, ChannelInfo>>,
76-
nodes: RwLockReadGuard<'a, BTreeMap<PublicKey, NodeInfo>>,
79+
nodes: RwLockReadGuard<'a, BTreeMap<NodeId, NodeInfo>>,
7780
}
7881

7982
/// Update to the [`NetworkGraph`] based on payment failure information conveyed via the Onion
@@ -208,8 +211,8 @@ where C::Target: chain::Access, L::Target: Logger
208211
},
209212
NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
210213
let action = if is_permanent { "Removing" } else { "Disabling" };
211-
log_debug!(self.logger, "{} node graph entry for {} due to a payment failure.", action, node_id);
212-
self.network_graph.fail_node(node_id, is_permanent);
214+
log_debug!(self.logger, "{} node graph entry for {:?} due to a payment failure.", action, node_id);
215+
self.network_graph.fail_node(&node_id.serialize(), is_permanent);
213216
},
214217
}
215218
}
@@ -277,11 +280,11 @@ where C::Target: chain::Access, L::Target: Logger
277280
let mut result = Vec::with_capacity(batch_amount as usize);
278281
let nodes = self.network_graph.nodes.read().unwrap();
279282
let mut iter = if let Some(pubkey) = starting_point {
280-
let mut iter = nodes.range((*pubkey)..);
283+
let mut iter = nodes.range((pubkey.serialize())..);
281284
iter.next();
282285
iter
283286
} else {
284-
nodes.range(..)
287+
nodes.range::<NodeId, _>(..)
285288
};
286289
while result.len() < batch_amount as usize {
287290
if let Some((_, ref node)) = iter.next() {
@@ -314,7 +317,7 @@ where C::Target: chain::Access, L::Target: Logger
314317
}
315318

316319
// Check if we need to perform a full synchronization with this peer
317-
if !self.should_request_full_sync(their_node_id) {
320+
if !self.should_request_full_sync(&their_node_id) {
318321
return ();
319322
}
320323

@@ -551,11 +554,11 @@ pub struct ChannelInfo {
551554
/// Protocol features of a channel communicated during its announcement
552555
pub features: ChannelFeatures,
553556
/// Source node of the first direction of a channel
554-
pub node_one: PublicKey,
557+
pub node_one: NodeId,
555558
/// Details about the first direction of a channel
556559
pub one_to_two: Option<DirectionalChannelInfo>,
557560
/// Source node of the second direction of a channel
558-
pub node_two: PublicKey,
561+
pub node_two: NodeId,
559562
/// Details about the second direction of a channel
560563
pub two_to_one: Option<DirectionalChannelInfo>,
561564
/// The channel capacity as seen on-chain, if chain lookup is available.
@@ -570,7 +573,7 @@ pub struct ChannelInfo {
570573
impl fmt::Display for ChannelInfo {
571574
fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
572575
write!(f, "features: {}, node_one: {}, one_to_two: {:?}, node_two: {}, two_to_one: {:?}",
573-
log_bytes!(self.features.encode()), log_pubkey!(self.node_one), self.one_to_two, log_pubkey!(self.node_two), self.two_to_one)?;
576+
log_bytes!(self.features.encode()), log_bytes!(self.node_one), self.one_to_two, log_bytes!(self.node_two), self.two_to_one)?;
574577
Ok(())
575578
}
576579
}
@@ -725,7 +728,7 @@ impl fmt::Display for NetworkGraph {
725728
}
726729
writeln!(f, "[Nodes]")?;
727730
for (key, val) in self.nodes.read().unwrap().iter() {
728-
writeln!(f, " {}: {}", log_pubkey!(key), val)?;
731+
writeln!(f, " {}: {}", log_bytes!(*key), val)?;
729732
}
730733
Ok(())
731734
}
@@ -780,7 +783,7 @@ impl NetworkGraph {
780783
}
781784

782785
fn update_node_from_announcement_intern(&self, msg: &msgs::UnsignedNodeAnnouncement, full_msg: Option<&msgs::NodeAnnouncement>) -> Result<(), LightningError> {
783-
match self.nodes.write().unwrap().get_mut(&msg.node_id) {
786+
match self.nodes.write().unwrap().get_mut(&msg.node_id.serialize()) {
784787
None => Err(LightningError{err: "No existing channels for node_announcement".to_owned(), action: ErrorAction::IgnoreError}),
785788
Some(node) => {
786789
if let Some(node_info) = node.announcement_info.as_ref() {
@@ -886,9 +889,9 @@ impl NetworkGraph {
886889

887890
let chan_info = ChannelInfo {
888891
features: msg.features.clone(),
889-
node_one: msg.node_id_1.clone(),
892+
node_one: msg.node_id_1.serialize(),
890893
one_to_two: None,
891-
node_two: msg.node_id_2.clone(),
894+
node_two: msg.node_id_2.serialize(),
892895
two_to_one: None,
893896
capacity_sats: utxo_value,
894897
announcement_message: if msg.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY
@@ -939,8 +942,8 @@ impl NetworkGraph {
939942
};
940943
}
941944

942-
add_channel_to_node!(msg.node_id_1);
943-
add_channel_to_node!(msg.node_id_2);
945+
add_channel_to_node!(msg.node_id_1.serialize());
946+
add_channel_to_node!(msg.node_id_2.serialize());
944947

945948
Ok(())
946949
}
@@ -969,7 +972,7 @@ impl NetworkGraph {
969972
}
970973

971974
/// Marks a node in the graph as failed.
972-
pub fn fail_node(&self, _node_id: &PublicKey, is_permanent: bool) {
975+
pub fn fail_node(&self, _node_id: &NodeId, is_permanent: bool) {
973976
if is_permanent {
974977
// TODO: Wholly remove the node
975978
} else {
@@ -1050,13 +1053,19 @@ impl NetworkGraph {
10501053
if msg.flags & 1 == 1 {
10511054
dest_node_id = channel.node_one.clone();
10521055
if let Some((sig, ctx)) = sig_info {
1053-
secp_verify_sig!(ctx, &msg_hash, &sig, &channel.node_two);
1056+
secp_verify_sig!(ctx, &msg_hash, &sig, &PublicKey::from_slice(&channel.node_two).map_err(|_| LightningError{
1057+
err: "Couldn't parse source node pubkey".to_owned(),
1058+
action: ErrorAction::IgnoreAndLog(Level::Debug)
1059+
})?);
10541060
}
10551061
maybe_update_channel_info!(channel.two_to_one, channel.node_two);
10561062
} else {
10571063
dest_node_id = channel.node_two.clone();
10581064
if let Some((sig, ctx)) = sig_info {
1059-
secp_verify_sig!(ctx, &msg_hash, &sig, &channel.node_one);
1065+
secp_verify_sig!(ctx, &msg_hash, &sig, &PublicKey::from_slice(&channel.node_one).map_err(|_| LightningError{
1066+
err: "Couldn't parse destination node pubkey".to_owned(),
1067+
action: ErrorAction::IgnoreAndLog(Level::Debug)
1068+
})?);
10601069
}
10611070
maybe_update_channel_info!(channel.one_to_two, channel.node_one);
10621071
}
@@ -1104,7 +1113,7 @@ impl NetworkGraph {
11041113
Ok(())
11051114
}
11061115

1107-
fn remove_channel_in_nodes(nodes: &mut BTreeMap<PublicKey, NodeInfo>, chan: &ChannelInfo, short_channel_id: u64) {
1116+
fn remove_channel_in_nodes(nodes: &mut BTreeMap<NodeId, NodeInfo>, chan: &ChannelInfo, short_channel_id: u64) {
11081117
macro_rules! remove_from_node {
11091118
($node_id: expr) => {
11101119
if let BtreeEntry::Occupied(mut entry) = nodes.entry($node_id) {
@@ -1136,7 +1145,7 @@ impl ReadOnlyNetworkGraph<'_> {
11361145
/// Returns all known nodes' public keys along with announced node info.
11371146
///
11381147
/// (C-not exported) because we have no mapping for `BTreeMap`s
1139-
pub fn nodes(&self) -> &BTreeMap<PublicKey, NodeInfo> {
1148+
pub fn nodes(&self) -> &BTreeMap<NodeId, NodeInfo> {
11401149
&*self.nodes
11411150
}
11421151

@@ -1146,7 +1155,7 @@ impl ReadOnlyNetworkGraph<'_> {
11461155
///
11471156
/// (C-not exported) as there is no practical way to track lifetimes of returned values.
11481157
pub fn get_addresses(&self, pubkey: &PublicKey) -> Option<&Vec<NetAddress>> {
1149-
if let Some(node) = self.nodes.get(pubkey) {
1158+
if let Some(node) = self.nodes.get(&pubkey.serialize()) {
11501159
if let Some(node_info) = node.announcement_info.as_ref() {
11511160
return Some(&node_info.addresses)
11521161
}

0 commit comments

Comments
 (0)