Fix starting requests on shutting down connections

bin/full-node/src/run/network_service.rs

@@ -555,7 +555,7 @@ impl NetworkService {
 
         // The call to `send_block_announce` below panics if we have no active connection.
         // TODO: not the correct check; must make sure that we have a substream open
-        if !guarded.network.has_established_connection(target) {
+        if !guarded.network.can_start_requests(target) {
             return Err(QueueNotificationError::NoConnection);
         }
 
@@ -624,7 +624,7 @@ impl NetworkService {
             let mut guarded = self.inner.guarded.lock().await;
 
             // The call to `start_blocks_request` below panics if we have no active connection.
-            if !guarded.network.has_established_connection(&target) {
+            if !guarded.network.can_start_requests(&target) {
                 return Err(BlocksRequestError::NoConnection);
             }
 

bin/light-base/src/network_service.rs

@@ -385,7 +385,7 @@ impl<TPlat: Platform> NetworkService<TPlat> {
             let mut guarded = self.shared.guarded.lock().await;
 
             // The call to `start_blocks_request` below panics if we have no active connection.
-            if !guarded.network.has_established_connection(&target) {
+            if !guarded.network.can_start_requests(&target) {
                 return Err(BlocksRequestError::NoConnection);
             }
 
@@ -490,7 +490,7 @@ impl<TPlat: Platform> NetworkService<TPlat> {
 
             // The call to `start_grandpa_warp_sync_request` below panics if we have no
             // active connection.
-            if !guarded.network.has_established_connection(&target) {
+            if !guarded.network.can_start_requests(&target) {
                 return Err(GrandpaWarpSyncRequestError::NoConnection);
             }
 
@@ -593,7 +593,7 @@ impl<TPlat: Platform> NetworkService<TPlat> {
 
             // The call to `start_storage_proof_request` below panics if we have no active
             // connection.
-            if !guarded.network.has_established_connection(&target) {
+            if !guarded.network.can_start_requests(&target) {
                 return Err(StorageProofRequestError::NoConnection);
             }
 
@@ -663,7 +663,7 @@ impl<TPlat: Platform> NetworkService<TPlat> {
             let mut guarded = self.shared.guarded.lock().await;
 
             // The call to `start_call_proof_request` below panics if we have no active connection.
-            if !guarded.network.has_established_connection(&target) {
+            if !guarded.network.can_start_requests(&target) {
                 return Err(CallProofRequestError::NoConnection);
             }
 
@@ -772,7 +772,7 @@ impl<TPlat: Platform> NetworkService<TPlat> {
 
         // The call to `send_block_announce` below panics if we have no active connection.
         // TODO: not the correct check; must make sure that we have a substream open
-        if !guarded.network.has_established_connection(target) {
+        if !guarded.network.can_start_requests(target) {
             return Err(QueueNotificationError::NoConnection);
         }
 

bin/wasm-node/CHANGELOG.md

@@ -8,6 +8,7 @@
 
 ### Fixed
 
+- Fix smoldot trying to send requests to peers whose connection is shutting down, leading to a panic. ([#2847](https://github.com/paritytech/smoldot/pull/2847))
 - Fix the responses to libp2p identify requests being wrongly empty. ([#2840](https://github.com/paritytech/smoldot/pull/2840))
 - Fix some Merkle proofs and SCALE-encoded structures being accepted as correct when they are actually invalid. This is a very minor fix that can presumably not be used as an attack vector. ([#2819](https://github.com/paritytech/smoldot/pull/2819))
 

src/libp2p/peers.rs

@@ -1386,7 +1386,8 @@ where
     /// # Panic
     ///
     /// Panics if `protocol_index` isn't a valid index in [`Config::request_response_protocols`].
-    /// Panics if there is no open connection with the target.
+    /// Panics if there is no open connection with the target or if all connections are shutting
+    /// down. Use [`Peers::can_start_requests`] to check if this is the case.
     ///
     #[track_caller]
     pub fn start_request(
@@ -1409,6 +1410,21 @@ where
         ))
     }
 
+    /// Returns `true` if if it possible to send requests (i.e. through [`Peers::start_request`])
+    /// to the given peer.
+    ///
+    /// If `false` is returned, then starting a request will panic.
+    ///
+    /// In other words, returns `true` if there exists an established connection non-shutting-down
+    /// connection with the given peer.
+    pub fn can_start_requests(&self, peer_id: &PeerId) -> bool {
+        self.established_peer_connections(peer_id).any(|c| {
+            let state = self.connection_state(c);
+            debug_assert!(state.established); // Guaranteed by `established_peer_connections`.
+            !state.shutting_down
+        })
+    }
+
     /// Responds to a previously-emitted [`Event::RequestIn`].
     ///
     /// # Panic

src/network/service.rs

@@ -2298,10 +2298,16 @@ where
         None
     }
 
-    /// Returns `true` if there exists an established connection with the given peer.
-    // TODO: revisit this API w.r.t. shutdowns
-    pub fn has_established_connection(&self, peer_id: &PeerId) -> bool {
-        self.inner.established_peer_connections(peer_id).count() != 0
+    /// Returns `true` if if it possible to send requests (i.e. through
+    /// [`ChainNetwork::start_grandpa_warp_sync_request`],
+    /// [`ChainNetwork::start_blocks_request`], etc.) to the given peer.
+    ///
+    /// If `false` is returned, starting a request will panic.
+    ///
+    /// In other words, returns `true` if there exists an established connection non-shutting-down
+    /// connection with the given peer.
+    pub fn can_start_requests(&self, peer_id: &PeerId) -> bool {
+        self.inner.can_start_requests(peer_id)
     }
 
     /// Returns an iterator to the list of [`PeerId`]s that we have an established connection