From: Matt Corallo <git@bluematt.me>
Date: Thu, 5 Mar 2020 18:39:53 +0000 (-0500)
Subject: Allow more than one address per type in node_announcement messages
X-Git-Tag: v0.0.12~108^2~3
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=107da97cd03803fdd1f61501b05e96260ed19016;p=rust-lightning

Allow more than one address per type in node_announcement messages

lnd has been blatantly ignoring this line in the spec forever, so
its somewhat of a lost cause trying to enforce it.
---

diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs
index b0d7b6031..b1a766ba7 100644
--- a/fuzz/src/router.rs
+++ b/fuzz/src/router.rs
@@ -124,7 +124,6 @@ pub fn do_test(data: &[u8]) {
 					msgs::DecodeError::UnknownVersion => return,
 					msgs::DecodeError::UnknownRequiredFeature => return,
 					msgs::DecodeError::InvalidValue => return,
-					msgs::DecodeError::ExtraAddressesPerType => return,
 					msgs::DecodeError::BadLengthDescriptor => return,
 					msgs::DecodeError::ShortRead => panic!("We picked the length..."),
 					msgs::DecodeError::Io(e) => panic!(format!("{}", e)),
diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs
index 8d23ac9a8..be5be241e 100644
--- a/lightning/src/ln/msgs.rs
+++ b/lightning/src/ln/msgs.rs
@@ -47,8 +47,6 @@ pub enum DecodeError {
 	InvalidValue,
 	/// Buffer too short
 	ShortRead,
-	/// node_announcement included more than one address of a given type!
-	ExtraAddressesPerType,
 	/// A length descriptor in the packet didn't describe the later data correctly
 	BadLengthDescriptor,
 	/// Error from std::io
@@ -677,7 +675,6 @@ impl Error for DecodeError {
 			DecodeError::UnknownRequiredFeature => "Unknown required feature preventing decode",
 			DecodeError::InvalidValue => "Nonsense bytes didn't map to the type they were interpreted as",
 			DecodeError::ShortRead => "Packet extended beyond the provided bytes",
-			DecodeError::ExtraAddressesPerType => "More than one address of a single type",
 			DecodeError::BadLengthDescriptor => "A length descriptor in the packet didn't describe the later data correctly",
 			DecodeError::Io(ref e) => e.description(),
 		}
@@ -1209,8 +1206,7 @@ impl Writeable for UnsignedNodeAnnouncement {
 		self.alias.write(w)?;
 
 		let mut addrs_to_encode = self.addresses.clone();
-		addrs_to_encode.sort_unstable_by(|a, b| { a.get_id().cmp(&b.get_id()) });
-		addrs_to_encode.dedup_by(|a, b| { a.get_id() == b.get_id() });
+		addrs_to_encode.sort_by(|a, b| { a.get_id().cmp(&b.get_id()) });
 		let mut addr_len = 0;
 		for addr in &addrs_to_encode {
 			addr_len += 1 + addr.len();
@@ -1235,7 +1231,8 @@ impl Readable for UnsignedNodeAnnouncement {
 		let alias: [u8; 32] = Readable::read(r)?;
 
 		let addr_len: u16 = Readable::read(r)?;
-		let mut addresses: Vec<NetAddress> = Vec::with_capacity(4);
+		let mut addresses: Vec<NetAddress> = Vec::new();
+		let mut highest_addr_type = 0;
 		let mut addr_readpos = 0;
 		let mut excess = false;
 		let mut excess_byte = 0;
@@ -1243,28 +1240,11 @@ impl Readable for UnsignedNodeAnnouncement {
 			if addr_len <= addr_readpos { break; }
 			match Readable::read(r) {
 				Ok(Ok(addr)) => {
-					match addr {
-						NetAddress::IPv4 { .. } => {
-							if addresses.len() > 0 {
-								return Err(DecodeError::ExtraAddressesPerType);
-							}
-						},
-						NetAddress::IPv6 { .. } => {
-							if addresses.len() > 1 || (addresses.len() == 1 && addresses[0].get_id() != 1) {
-								return Err(DecodeError::ExtraAddressesPerType);
-							}
-						},
-						NetAddress::OnionV2 { .. } => {
-							if addresses.len() > 2 || (addresses.len() > 0 && addresses.last().unwrap().get_id() > 2) {
-								return Err(DecodeError::ExtraAddressesPerType);
-							}
-						},
-						NetAddress::OnionV3 { .. } => {
-							if addresses.len() > 3 || (addresses.len() > 0 && addresses.last().unwrap().get_id() > 3) {
-								return Err(DecodeError::ExtraAddressesPerType);
-							}
-						},
+					if addr.get_id() < highest_addr_type {
+						// Addresses must be sorted in increasing order
+						return Err(DecodeError::InvalidValue);
 					}
+					highest_addr_type = addr.get_id();
 					if addr_len < addr_readpos + 1 + addr.len() {
 						return Err(DecodeError::BadLengthDescriptor);
 					}
diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs
index 60df3d801..6a909e5f9 100644
--- a/lightning/src/ln/peer_handler.rs
+++ b/lightning/src/ln/peer_handler.rs
@@ -586,10 +586,6 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
 														log_debug!(self, "Deserialization failed due to shortness of message");
 														return Err(PeerHandleError { no_connection_possible: false });
 													}
-													msgs::DecodeError::ExtraAddressesPerType => {
-														log_debug!(self, "Error decoding message, ignoring due to lnd spec incompatibility. See https://github.com/lightningnetwork/lnd/issues/1407");
-														continue;
-													}
 													msgs::DecodeError::BadLengthDescriptor => return Err(PeerHandleError { no_connection_possible: false }),
 													msgs::DecodeError::Io(_) => return Err(PeerHandleError { no_connection_possible: false }),
 												}