Relay/store channel/node announces w/ unknown req'd feature bits

This change was made in the flat features BOLT PR, as if a channel
requires some unknown feature bits we should still rumor it, we just
shouldn't route through it.

Author: TheBlueMatt

lightning/src/ln/msgs.rs

@@ -179,6 +179,23 @@ impl<T: FeatureContext> Features<T> {
 			( idx != 0 && byte != 0 ) || ( idx == 0 && (byte & 0xc4) != 0 )
 		})
 	}
+
+	#[cfg(test)]
+	pub(crate) fn set_require_unknown_bits(&mut self) {
+		let newlen = cmp::max(2, self.flags.len());
+		self.flags.resize(newlen, 0u8);
+		self.flags[1] |= 0x40;
+	}
+
+	#[cfg(test)]
+	pub(crate) fn clear_require_unknown_bits(&mut self) {
+		let newlen = cmp::max(2, self.flags.len());
+		self.flags.resize(newlen, 0u8);
+		self.flags[1] &= !0x40;
+		if self.flags.len() == 2 && self.flags[1] == 0 {
+			self.flags.resize(1, 0u8);
+		}
+	}
 }
 
 impl<T: FeatureContextInitNode> Features<T> {
@@ -1249,13 +1266,7 @@ impl Writeable for UnsignedChannelAnnouncement {
 impl<R: Read> Readable<R> for UnsignedChannelAnnouncement {
 	fn read(r: &mut R) -> Result<Self, DecodeError> {
 		Ok(Self {
-			features: {
-				let f: Features<FeatureContextChannel> = Readable::read(r)?;
-				if f.requires_unknown_bits() {
-					return Err(DecodeError::UnknownRequiredFeature);
-				}
-				f
-			},
+			features: Readable::read(r)?,
 			chain_hash: Readable::read(r)?,
 			short_channel_id: Readable::read(r)?,
 			node_id_1: Readable::read(r)?,
@@ -1383,9 +1394,6 @@ impl Writeable for UnsignedNodeAnnouncement {
 impl<R: Read> Readable<R> for UnsignedNodeAnnouncement {
 	fn read(r: &mut R) -> Result<Self, DecodeError> {
 		let features: Features<FeatureContextNode> = Readable::read(r)?;
-		if features.requires_unknown_bits() {
-			return Err(DecodeError::UnknownRequiredFeature);
-		}
 		let timestamp: u32 = Readable::read(r)?;
 		let node_id: PublicKey = Readable::read(r)?;
 		let mut rgb = [0; 3];

lightning/src/ln/router.rs

@@ -414,10 +414,6 @@ impl RoutingMessageHandler for Router {
 		let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.contents.encode()[..])[..]);
 		secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &msg.contents.node_id);
 
-		if msg.contents.features.requires_unknown_bits() {
-			panic!("Unknown-required-features NodeAnnouncements should never deserialize!");
-		}
-
 		let mut network = self.network_map.write().unwrap();
 		match network.nodes.get_mut(&msg.contents.node_id) {
 			None => Err(LightningError{err: "No existing channels for node_announcement", action: ErrorAction::IgnoreError}),
@@ -432,7 +428,7 @@ impl RoutingMessageHandler for Router {
 				node.alias = msg.contents.alias;
 				node.addresses = msg.contents.addresses.clone();
 
-				let should_relay = msg.contents.excess_data.is_empty() && msg.contents.excess_address_data.is_empty() && !msg.contents.features.supports_unknown_bits();
+				let should_relay = msg.contents.excess_data.is_empty() && msg.contents.excess_address_data.is_empty();
 				node.announcement_message = if should_relay { Some(msg.clone()) } else { None };
 				Ok(should_relay)
 			}
@@ -450,10 +446,6 @@ impl RoutingMessageHandler for Router {
 		secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_1, &msg.contents.bitcoin_key_1);
 		secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_2, &msg.contents.bitcoin_key_2);
 
-		if msg.contents.features.requires_unknown_bits() {
-			panic!("Unknown-required-features ChannelAnnouncements should never deserialize!");
-		}
-
 		let checked_utxo = match self.chain_monitor.get_chain_utxo(msg.contents.chain_hash, msg.contents.short_channel_id) {
 			Ok((script_pubkey, _value)) => {
 				let expected_script = Builder::new().push_opcode(opcodes::all::OP_PUSHNUM_2)
@@ -483,7 +475,7 @@ impl RoutingMessageHandler for Router {
 		let mut network_lock = self.network_map.write().unwrap();
 		let network = network_lock.borrow_parts();
 
-		let should_relay = msg.contents.excess_data.is_empty() && !msg.contents.features.supports_unknown_bits();
+		let should_relay = msg.contents.excess_data.is_empty();
 
 		let chan_info = ChannelInfo {
 				features: msg.contents.features.clone(),
@@ -941,17 +933,19 @@ impl Router {
 
 				for chan_id in $node.channels.iter() {
 					let chan = network.channels.get(chan_id).unwrap();
-					if chan.one_to_two.src_node_id == *$node_id {
-						// ie $node is one, ie next hop in A* is two, via the two_to_one channel
-						if first_hops.is_none() || chan.two_to_one.src_node_id != network.our_node_id {
-							if chan.two_to_one.enabled {
-								add_entry!(chan_id, chan.one_to_two.src_node_id, chan.two_to_one, $fee_to_target_msat);
+					if !chan.features.requires_unknown_bits() {
+						if chan.one_to_two.src_node_id == *$node_id {
+							// ie $node is one, ie next hop in A* is two, via the two_to_one channel
+							if first_hops.is_none() || chan.two_to_one.src_node_id != network.our_node_id {
+								if chan.two_to_one.enabled {
+									add_entry!(chan_id, chan.one_to_two.src_node_id, chan.two_to_one, $fee_to_target_msat);
+								}
 							}
-						}
-					} else {
-						if first_hops.is_none() || chan.one_to_two.src_node_id != network.our_node_id {
-							if chan.one_to_two.enabled {
-								add_entry!(chan_id, chan.two_to_one.src_node_id, chan.one_to_two, $fee_to_target_msat);
+						} else {
+							if first_hops.is_none() || chan.one_to_two.src_node_id != network.our_node_id {
+								if chan.one_to_two.enabled {
+									add_entry!(chan_id, chan.two_to_one.src_node_id, chan.one_to_two, $fee_to_target_msat);
+								}
 							}
 						}
 					}
@@ -1015,7 +1009,7 @@ mod tests {
 	use chain::chaininterface;
 	use ln::channelmanager;
 	use ln::router::{Router,NodeInfo,NetworkMap,ChannelInfo,DirectionalChannelInfo,RouteHint};
-	use ln::msgs::{Features, FeatureContextChannel, FeatureContextNode};
+	use ln::msgs::{Features, FeatureContextChannel, FeatureContextNode, LightningError, ErrorAction};
 	use util::test_utils;
 	use util::test_utils::TestVecWriter;
 	use util::logger::Logger;
@@ -1441,6 +1435,49 @@ mod tests {
 			assert_eq!(route.hops[1].cltv_expiry_delta, 42);
 		}
 
+		{ // Disable 4 and 12 by requiring unknown feature bits
+			let mut network = router.network_map.write().unwrap();
+			network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.set_require_unknown_bits();
+			network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.set_require_unknown_bits();
+		}
+
+		{ // If all the channels require some features we don't understand, route should fail
+			if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = router.get_route(&node3, None, &Vec::new(), 100, 42) {
+				assert_eq!(err, "Failed to find a path to the given destination");
+			} else { panic!(); }
+		}
+
+		{ // If we specify a channel to node8, that overrides our local channel view and that gets used
+			let our_chans = vec![channelmanager::ChannelDetails {
+				channel_id: [0; 32],
+				short_channel_id: Some(42),
+				remote_network_id: node8.clone(),
+				channel_value_satoshis: 0,
+				user_id: 0,
+				outbound_capacity_msat: 0,
+				inbound_capacity_msat: 0,
+				is_live: true,
+			}];
+			let route = router.get_route(&node3, Some(&our_chans), &Vec::new(), 100, 42).unwrap();
+			assert_eq!(route.hops.len(), 2);
+
+			assert_eq!(route.hops[0].pubkey, node8);
+			assert_eq!(route.hops[0].short_channel_id, 42);
+			assert_eq!(route.hops[0].fee_msat, 200);
+			assert_eq!(route.hops[0].cltv_expiry_delta, (13 << 8) | 1);
+
+			assert_eq!(route.hops[1].pubkey, node3);
+			assert_eq!(route.hops[1].short_channel_id, 13);
+			assert_eq!(route.hops[1].fee_msat, 100);
+			assert_eq!(route.hops[1].cltv_expiry_delta, 42);
+		}
+
+		{ // Re-enable 4 and 12 by wiping the unknown feature bits
+			let mut network = router.network_map.write().unwrap();
+			network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.clear_require_unknown_bits();
+			network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.clear_require_unknown_bits();
+		}
+
 		{ // Route to 1 via 2 and 3 because our channel to 1 is disabled
 			let route = router.get_route(&node1, None, &Vec::new(), 100, 42).unwrap();
 			assert_eq!(route.hops.len(), 3);