Merge pull request #1701 from TheBlueMatt/2022-09-feature-or

Fetch InitFeatures from both Channel and Routing Message Handlers

Author: TheBlueMatt

lightning-net-tokio/src/lib.rs

@@ -582,6 +582,7 @@ mod tests {
 		fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: ReplyShortChannelIdsEnd) -> Result<(), LightningError> { Ok(()) }
 		fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: QueryChannelRange) -> Result<(), LightningError> { Ok(()) }
 		fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: QueryShortChannelIds) -> Result<(), LightningError> { Ok(()) }
+		fn provided_init_features(&self, _their_node_id: &PublicKey) -> InitFeatures { InitFeatures::known() }
 	}
 	impl ChannelMessageHandler for MsgHandler {
 		fn handle_open_channel(&self, _their_node_id: &PublicKey, _their_features: InitFeatures, _msg: &OpenChannel) {}
@@ -614,6 +615,7 @@ mod tests {
 		fn handle_channel_reestablish(&self, _their_node_id: &PublicKey, _msg: &ChannelReestablish) {}
 		fn handle_error(&self, _their_node_id: &PublicKey, _msg: &ErrorMessage) {}
 		fn provided_node_features(&self) -> NodeFeatures { NodeFeatures::known() }
+		fn provided_init_features(&self, _their_node_id: &PublicKey) -> InitFeatures { InitFeatures::known() }
 	}
 	impl MessageSendEventsProvider for MsgHandler {
 		fn get_and_clear_pending_msg_events(&self) -> Vec<MessageSendEvent> {

lightning/src/ln/channelmanager.rs

@@ -6124,6 +6124,10 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
 	fn provided_node_features(&self) -> NodeFeatures {
 		NodeFeatures::known()
 	}
+
+	fn provided_init_features(&self, _their_init_features: &PublicKey) -> InitFeatures {
+		InitFeatures::known_channel_features()
+	}
 }
 
 const SERIALIZATION_VERSION: u8 = 1;

lightning/src/ln/features.rs

@@ -163,6 +163,8 @@ mod sealed {
 			,
 		],
 		optional_features: [
+			// Note that if new "non-channel-related" flags are added here they should be
+			// explicitly cleared in InitFeatures::known_channel_features.
 			// Byte 0
 			DataLossProtect | InitialRoutingSync | UpfrontShutdownScript | GossipQueries,
 			// Byte 1
@@ -545,6 +547,14 @@ impl InitFeatures {
 	pub(crate) fn to_context<C: sealed::Context>(&self) -> Features<C> {
 		self.to_context_internal()
 	}
+
+	/// Returns the set of known init features that are related to channels. At least some of
+	/// these features are likely required for peers to talk to us.
+	pub fn known_channel_features() -> InitFeatures {
+		Self::known()
+			.clear_initial_routing_sync()
+			.clear_gossip_queries()
+	}
 }
 
 impl InvoiceFeatures {
@@ -763,20 +773,17 @@ impl<T: sealed::UpfrontShutdownScript> Features<T> {
 
 
 impl<T: sealed::GossipQueries> Features<T> {
-	#[cfg(test)]
 	pub(crate) fn clear_gossip_queries(mut self) -> Self {
 		<T as sealed::GossipQueries>::clear_bits(&mut self.flags);
 		self
 	}
 }
 
 impl<T: sealed::InitialRoutingSync> Features<T> {
-	// We are no longer setting initial_routing_sync now that gossip_queries
-	// is enabled. This feature is ignored by a peer when gossip_queries has
-	// been negotiated.
-	#[cfg(test)]
-	pub(crate) fn clear_initial_routing_sync(&mut self) {
-		<T as sealed::InitialRoutingSync>::clear_bits(&mut self.flags)
+	// Note that initial_routing_sync is ignored if gossip_queries is set.
+	pub(crate) fn clear_initial_routing_sync(mut self) -> Self {
+		<T as sealed::InitialRoutingSync>::clear_bits(&mut self.flags);
+		self
 	}
 }
 
@@ -915,7 +922,7 @@ mod tests {
 
 		let mut init_features = InitFeatures::known();
 		assert!(init_features.initial_routing_sync());
-		init_features.clear_initial_routing_sync();
+		init_features = init_features.clear_initial_routing_sync();
 		assert!(!init_features.initial_routing_sync());
 	}
 

lightning/src/ln/msgs.rs

@@ -902,6 +902,13 @@ pub trait ChannelMessageHandler : MessageSendEventsProvider {
 	/// queried similarly and their feature flags are OR'd together to form the [`NodeFeatures`]
 	/// which are broadcasted in our node_announcement message.
 	fn provided_node_features(&self) -> NodeFeatures;
+
+	/// Gets the init feature flags which should be sent to the given peer. All available handlers
+	/// are queried similarly and their feature flags are OR'd together to form the [`InitFeatures`]
+	/// which are sent in our [`Init`] message.
+	///
+	/// Note that this method is called before [`Self::peer_connected`].
+	fn provided_init_features(&self, their_node_id: &PublicKey) -> InitFeatures;
 }
 
 /// A trait to describe an object which can receive routing messages.
@@ -949,6 +956,14 @@ pub trait RoutingMessageHandler : MessageSendEventsProvider {
 	/// Handles when a peer asks us to send routing gossip messages for a
 	/// list of short_channel_ids.
 	fn handle_query_short_channel_ids(&self, their_node_id: &PublicKey, msg: QueryShortChannelIds) -> Result<(), LightningError>;
+
+	// Handler information:
+	/// Gets the init feature flags which should be sent to the given peer. All available handlers
+	/// are queried similarly and their feature flags are OR'd together to form the [`InitFeatures`]
+	/// which are sent in our [`Init`] message.
+	///
+	/// Note that this method is called before [`Self::peer_connected`].
+	fn provided_init_features(&self, their_node_id: &PublicKey) -> InitFeatures;
 }
 
 /// A trait to describe an object that can receive onion messages.

lightning/src/ln/peer_handler.rs

@@ -77,6 +77,9 @@ impl RoutingMessageHandler for IgnoringMessageHandler {
 	fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyShortChannelIdsEnd) -> Result<(), LightningError> { Ok(()) }
 	fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: msgs::QueryChannelRange) -> Result<(), LightningError> { Ok(()) }
 	fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: msgs::QueryShortChannelIds) -> Result<(), LightningError> { Ok(()) }
+	fn provided_init_features(&self, _their_node_id: &PublicKey) -> InitFeatures {
+		InitFeatures::empty()
+	}
 }
 impl OnionMessageProvider for IgnoringMessageHandler {
 	fn next_onion_message_for_peer(&self, _peer_node_id: PublicKey) -> Option<msgs::OnionMessage> { None }
@@ -203,6 +206,11 @@ impl ChannelMessageHandler for ErroringMessageHandler {
 	fn peer_connected(&self, _their_node_id: &PublicKey, _msg: &msgs::Init) {}
 	fn handle_error(&self, _their_node_id: &PublicKey, _msg: &msgs::ErrorMessage) {}
 	fn provided_node_features(&self) -> NodeFeatures { NodeFeatures::empty() }
+	fn provided_init_features(&self, _their_node_id: &PublicKey) -> InitFeatures {
+		// Use our known channel feature set as peers may otherwise not be willing to talk to us at
+		// all.
+		InitFeatures::known_channel_features()
+	}
 }
 impl Deref for ErroringMessageHandler {
 	type Target = ErroringMessageHandler;
@@ -1052,7 +1060,8 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 
 								peer.their_node_id = Some(their_node_id);
 								insert_node_id!();
-								let features = InitFeatures::known();
+								let features = self.message_handler.chan_handler.provided_init_features(&their_node_id)
+									.or(self.message_handler.route_handler.provided_init_features(&their_node_id));
 								let resp = msgs::Init { features, remote_network_address: filter_addresses(peer.their_net_address.clone()) };
 								self.enqueue_message(peer, &resp);
 								peer.awaiting_pong_timer_tick_intervals = 0;
@@ -1064,7 +1073,8 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 								peer.pending_read_is_header = true;
 								peer.their_node_id = Some(their_node_id);
 								insert_node_id!();
-								let features = InitFeatures::known();
+								let features = self.message_handler.chan_handler.provided_init_features(&their_node_id)
+									.or(self.message_handler.route_handler.provided_init_features(&their_node_id));
 								let resp = msgs::Init { features, remote_network_address: filter_addresses(peer.their_net_address.clone()) };
 								self.enqueue_message(peer, &resp);
 								peer.awaiting_pong_timer_tick_intervals = 0;

lightning/src/routing/gossip.rs

@@ -22,7 +22,7 @@ use bitcoin::hash_types::BlockHash;
 use chain;
 use chain::Access;
 use ln::chan_utils::make_funding_redeemscript;
-use ln::features::{ChannelFeatures, NodeFeatures};
+use ln::features::{ChannelFeatures, NodeFeatures, InitFeatures};
 use ln::msgs::{DecodeError, ErrorAction, Init, LightningError, RoutingMessageHandler, NetAddress, MAX_VALUE_MSAT};
 use ln::msgs::{ChannelAnnouncement, ChannelUpdate, NodeAnnouncement, GossipTimestampFilter};
 use ln::msgs::{QueryChannelRange, ReplyChannelRange, QueryShortChannelIds, ReplyShortChannelIdsEnd};
@@ -570,6 +570,12 @@ where C::Target: chain::Access, L::Target: Logger
 			action: ErrorAction::IgnoreError,
 		})
 	}
+
+	fn provided_init_features(&self, _their_node_id: &PublicKey) -> InitFeatures {
+		let mut features = InitFeatures::empty();
+		features.set_gossip_queries_optional();
+		features
+	}
 }
 
 impl<G: Deref<Target=NetworkGraph<L>>, C: Deref, L: Deref> MessageSendEventsProvider for P2PGossipSync<G, C, L>

lightning/src/util/test_utils.rs

@@ -360,6 +360,9 @@ impl msgs::ChannelMessageHandler for TestChannelMessageHandler {
 	fn provided_node_features(&self) -> NodeFeatures {
 		NodeFeatures::empty()
 	}
+	fn provided_init_features(&self, _their_init_features: &PublicKey) -> InitFeatures {
+		InitFeatures::known_channel_features()
+	}
 }
 
 impl events::MessageSendEventsProvider for TestChannelMessageHandler {
@@ -507,6 +510,12 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler {
 	fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: msgs::QueryShortChannelIds) -> Result<(), msgs::LightningError> {
 		Ok(())
 	}
+
+	fn provided_init_features(&self, _their_init_features: &PublicKey) -> InitFeatures {
+		let mut features = InitFeatures::empty();
+		features.set_gossip_queries_optional();
+		features
+	}
 }
 
 impl events::MessageSendEventsProvider for TestRoutingMessageHandler {