network-gossip: Ensure sync event is processed on unknown peer roles (#…

…6553)

The `GossipEngine::poll_next` implementation polls both the
`notification_service` and the `sync_event_stream`.

If both polls produce valid data to be processed
(`Poll::Ready(Some(..))`), then the sync event is ignored when we
receive `NotificationEvent::NotificationStreamOpened` and the role
cannot be deduced.

This PR ensures both events are processed gracefully. While at it, I
have added a warning to the sync engine related to
`notification_service` producing `Poll::Ready(None)`.

This effectively ensures that `SyncEvents` propagate to the network
potentially fixing any state mismatch.


For more context: #6507

cc @paritytech/sdk-node

---------

Signed-off-by: Alexandru Vasile <[email protected]>

prdoc/pr_6553.prdoc

@@ -0,0 +1,13 @@
+title: Ensure sync event is processed on unknown peer roles
+
+doc:
+  - audience: Node Dev
+    description: |
+      The GossipEngine::poll_next implementation polls both the notification_service and the sync_event_stream.
+      This PR ensures both events are processed gracefully.
+
+crates:
+  - name: sc-network-gossip
+    bump: patch
+  - name: sc-network-sync
+    bump: patch

substrate/client/network-gossip/src/bridge.rs

@@ -220,18 +220,16 @@ impl<B: BlockT> Future for GossipEngine<B> {
 							},
 							NotificationEvent::NotificationStreamOpened {
 								peer, handshake, ..
-							} => {
-								let Some(role) = this.network.peer_role(peer, handshake) else {
+							} =>
+								if let Some(role) = this.network.peer_role(peer, handshake) {
+									this.state_machine.new_peer(
+										&mut this.notification_service,
+										peer,
+										role,
+									);
+								} else {
 									log::debug!(target: "gossip", "role for {peer} couldn't be determined");
-									continue
-								};
-
-								this.state_machine.new_peer(
-									&mut this.notification_service,
-									peer,
-									role,
-								);
-							},
+								},
 							NotificationEvent::NotificationStreamClosed { peer } => {
 								this.state_machine
 									.peer_disconnected(&mut this.notification_service, peer);

substrate/client/network/sync/src/engine.rs

@@ -545,7 +545,14 @@ where
 					self.process_service_command(command),
 				notification_event = self.notification_service.next_event() => match notification_event {
 					Some(event) => self.process_notification_event(event),
-					None => return,
+					None => {
+						error!(
+							target: LOG_TARGET,
+							"Terminating `SyncingEngine` because `NotificationService` has terminated.",
+						);
+
+						return;
+					}
 				},
 				response_event = self.pending_responses.select_next_some() =>
 					self.process_response_event(response_event),