Skip to content

Remove NodeAnnouncementInfo addresses #2102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 21, 2023
Merged

Remove NodeAnnouncementInfo addresses #2102

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 60 additions & 28 deletions lightning/src/routing/gossip.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1046,23 +1046,52 @@ pub struct NodeAnnouncementInfo {
/// May be invalid or malicious (eg control chars),
/// should not be exposed to the user.
pub alias: NodeAlias,
/// Internet-level addresses via which one can connect to the node
pub addresses: Vec<NetAddress>,
/// An initial announcement of the node
/// Mostly redundant with the data we store in fields explicitly.
/// Everything else is useful only for sending out for initial routing sync.
/// Not stored if contains excess data to prevent DoS.
pub announcement_message: Option<NodeAnnouncement>
}

impl_writeable_tlv_based!(NodeAnnouncementInfo, {
(0, features, required),
(2, last_update, required),
(4, rgb, required),
(6, alias, required),
(8, announcement_message, option),
(10, addresses, vec_type),
});
impl NodeAnnouncementInfo {
/// Internet-level addresses via which one can connect to the node
pub fn addresses(&self) -> &[NetAddress] {
self.announcement_message.as_ref()
.map(|msg| msg.contents.addresses.as_slice())
.unwrap_or_default()
}
}

impl Writeable for NodeAnnouncementInfo {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
let empty_addresses = Vec::<NetAddress>::new();
write_tlv_fields!(writer, {
(0, self.features, required),
(2, self.last_update, required),
(4, self.rgb, required),
(6, self.alias, required),
(8, self.announcement_message, option),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to write an empty vec for the old field, since currently old versions will fail to read this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, looking back at this this isn't true - vec_type semantics are super unclear - they are allowed to have a missing TLV (just implies an empty vec), but we always write something even if the vec is empty. It doesnt matter much tho.

(10, empty_addresses, vec_type), // Versions prior to 0.0.115 require this field
});
Ok(())
}
}

impl Readable for NodeAnnouncementInfo {
fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
_init_and_read_tlv_fields!(reader, {
(0, features, required),
(2, last_update, required),
(4, rgb, required),
(6, alias, required),
(8, announcement_message, option),
(10, _addresses, vec_type), // deprecated, not used anymore
});
let _: Option<Vec<NetAddress>> = _addresses;
Ok(Self { features: features.0.unwrap(), last_update: last_update.0.unwrap(), rgb: rgb.0.unwrap(),
alias: alias.0.unwrap(), announcement_message })
}
}

/// A user-defined name for a node, which may be used when displaying the node in a graph.
///
Expand Down Expand Up @@ -1133,7 +1162,7 @@ impl Writeable for NodeInfo {
}
}

// A wrapper allowing for the optional deseralization of `NodeAnnouncementInfo`. Utilizing this is
// A wrapper allowing for the optional deserialization of `NodeAnnouncementInfo`. Utilizing this is
// necessary to maintain compatibility with previous serializations of `NetAddress` that have an
// invalid hostname set. We ignore and eat all errors until we are either able to read a
// `NodeAnnouncementInfo` or hit a `ShortRead`, i.e., read the TLV field to the end.
Expand Down Expand Up @@ -1362,7 +1391,6 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
last_update: msg.timestamp,
rgb: msg.rgb,
alias: NodeAlias(msg.alias),
addresses: msg.addresses.clone(),
announcement_message: if should_relay { full_msg.cloned() } else { None },
});

Expand Down Expand Up @@ -1916,12 +1944,8 @@ impl ReadOnlyNetworkGraph<'_> {
/// Returns None if the requested node is completely unknown,
/// or if node announcement for the node was never received.
pub fn get_addresses(&self, pubkey: &PublicKey) -> Option<Vec<NetAddress>> {
if let Some(node) = self.nodes.get(&NodeId::from_pubkey(&pubkey)) {
if let Some(node_info) = node.announcement_info.as_ref() {
return Some(node_info.addresses.clone())
}
}
None
self.nodes.get(&NodeId::from_pubkey(&pubkey))
.and_then(|node| node.announcement_info.as_ref().map(|ann| ann.addresses().to_vec()))
}
}

Expand All @@ -1938,7 +1962,7 @@ pub(crate) mod tests {
ReplyChannelRange, QueryChannelRange, QueryShortChannelIds, MAX_VALUE_MSAT};
use crate::util::config::UserConfig;
use crate::util::test_utils;
use crate::util::ser::{ReadableArgs, Writeable};
use crate::util::ser::{ReadableArgs, Readable, Writeable};
use crate::util::events::{MessageSendEvent, MessageSendEventsProvider};
use crate::util::scid_utils::scid_from_parts;

Expand Down Expand Up @@ -3244,26 +3268,25 @@ pub(crate) mod tests {

#[test]
fn node_info_is_readable() {
use std::convert::TryFrom;

// 1. Check we can read a valid NodeAnnouncementInfo and fail on an invalid one
let valid_netaddr = crate::ln::msgs::NetAddress::Hostname { hostname: crate::util::ser::Hostname::try_from("A".to_string()).unwrap(), port: 1234 };
let announcement_message = hex::decode("d977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a000122013413a7031b84c5567b126440995d3ed5aaba0565d71e1834604819ff9c17f5e9d5dd078f2020201010101010101010101010101010101010101010101010101010101010101010000701fffefdfc2607").unwrap();
let announcement_message = NodeAnnouncement::read(&mut announcement_message.as_slice()).unwrap();
let valid_node_ann_info = NodeAnnouncementInfo {
features: channelmanager::provided_node_features(&UserConfig::default()),
last_update: 0,
rgb: [0u8; 3],
alias: NodeAlias([0u8; 32]),
addresses: vec![valid_netaddr],
announcement_message: None,
announcement_message: Some(announcement_message)
};

let mut encoded_valid_node_ann_info = Vec::new();
assert!(valid_node_ann_info.write(&mut encoded_valid_node_ann_info).is_ok());
let read_valid_node_ann_info: NodeAnnouncementInfo = crate::util::ser::Readable::read(&mut encoded_valid_node_ann_info.as_slice()).unwrap();
let read_valid_node_ann_info = NodeAnnouncementInfo::read(&mut encoded_valid_node_ann_info.as_slice()).unwrap();
assert_eq!(read_valid_node_ann_info, valid_node_ann_info);
assert_eq!(read_valid_node_ann_info.addresses().len(), 1);

let encoded_invalid_node_ann_info = hex::decode("3f0009000788a000080a51a20204000000000403000000062000000000000000000000000000000000000000000000000000000000000000000a0505014004d2").unwrap();
let read_invalid_node_ann_info_res: Result<NodeAnnouncementInfo, crate::ln::msgs::DecodeError> = crate::util::ser::Readable::read(&mut encoded_invalid_node_ann_info.as_slice());
let read_invalid_node_ann_info_res = NodeAnnouncementInfo::read(&mut encoded_invalid_node_ann_info.as_slice());
assert!(read_invalid_node_ann_info_res.is_err());

// 2. Check we can read a NodeInfo anyways, but set the NodeAnnouncementInfo to None if invalid
Expand All @@ -3274,13 +3297,22 @@ pub(crate) mod tests {

let mut encoded_valid_node_info = Vec::new();
assert!(valid_node_info.write(&mut encoded_valid_node_info).is_ok());
let read_valid_node_info: NodeInfo = crate::util::ser::Readable::read(&mut encoded_valid_node_info.as_slice()).unwrap();
let read_valid_node_info = NodeInfo::read(&mut encoded_valid_node_info.as_slice()).unwrap();
assert_eq!(read_valid_node_info, valid_node_info);

let encoded_invalid_node_info_hex = hex::decode("4402403f0009000788a000080a51a20204000000000403000000062000000000000000000000000000000000000000000000000000000000000000000a0505014004d20400").unwrap();
let read_invalid_node_info: NodeInfo = crate::util::ser::Readable::read(&mut encoded_invalid_node_info_hex.as_slice()).unwrap();
let read_invalid_node_info = NodeInfo::read(&mut encoded_invalid_node_info_hex.as_slice()).unwrap();
assert_eq!(read_invalid_node_info.announcement_info, None);
}

#[test]
fn test_node_info_keeps_compatibility() {
let old_ann_info_with_addresses = hex::decode("3f0009000708a000080a51220204000000000403000000062000000000000000000000000000000000000000000000000000000000000000000a0505014104d2").unwrap();
let ann_info_with_addresses = NodeAnnouncementInfo::read(&mut old_ann_info_with_addresses.as_slice())
.expect("to be able to read an old NodeAnnouncementInfo with addresses");
// This serialized info has an address field but no announcement_message, therefore the addresses returned by our function will still be empty
assert!(ann_info_with_addresses.addresses().is_empty());
}
}

#[cfg(all(test, feature = "_bench_unstable"))]
Expand Down