Fix panic in networking state machine (#1395)

* Fix panic in networking state machine * PR link
smol-dot · Nov 23, 2023 · 15dfd43 · 15dfd43
1 parent 8967915
commit 15dfd43
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 22 deletions.
diff --git a/lib/src/network/service.rs b/lib/src/network/service.rs
@@ -712,7 +712,7 @@ where
             )
             .any(|(_, connection_id)| {
                 let state = self.inner.connection_state(*connection_id);
-                !state.shutting_down
+                state.established && !state.shutting_down
             })
         {
             if self
@@ -745,7 +745,18 @@ where
                     .insert((peer_index, chain_id, kind));
                 debug_assert!(_was_inserted);
             }
-        } else {
+        }
+
+        if !self
+            .connections_by_peer_id
+            .range(
+                (peer_index, ConnectionId::min_value())..=(peer_index, ConnectionId::max_value()),
+            )
+            .any(|(_, connection_id)| {
+                let state = self.inner.connection_state(*connection_id);
+                !state.shutting_down
+            })
+        {
             // Note that that `PeerId` might already be desired towards a different chain, in
             // which case it is already present in `unconnected_desired`.
             self.unconnected_desired.insert(peer_index);
@@ -1265,7 +1276,8 @@ where
                             )
                             .count()
                             != 0
-                            && !self
+                        {
+                            if !self
                                 .connections_by_peer_id
                                 .range(
                                     (peer_index, ConnectionId::min_value())
@@ -1275,25 +1287,38 @@ where
                                     let state = self.inner.connection_state(*connection_id);
                                     !state.shutting_down
                                 })
-                        {
-                            self.unconnected_desired.insert(peer_index);
-                            for (_, _, chain_index) in self.gossip_desired_peers.range(
-                                (
-                                    peer_index,
-                                    GossipKind::ConsensusTransactions,
-                                    usize::min_value(),
+                            {
+                                self.unconnected_desired.insert(peer_index);
+                            }
+                            if !self
+                                .connections_by_peer_id
+                                .range(
+                                    (peer_index, ConnectionId::min_value())
+                                        ..=(peer_index, ConnectionId::max_value()),
                                 )
-                                    ..=(
+                                .any(|(_, connection_id)| {
+                                    let state = self.inner.connection_state(*connection_id);
+                                    state.established && !state.shutting_down
+                                })
+                            {
+                                for (_, _, chain_index) in self.gossip_desired_peers.range(
+                                    (
                                         peer_index,
                                         GossipKind::ConsensusTransactions,
-                                        usize::max_value(),
-                                    ),
-                            ) {
-                                self.connected_unopened_gossip_desired.remove(&(
-                                    peer_index,
-                                    ChainId(*chain_index),
-                                    GossipKind::ConsensusTransactions,
-                                ));
+                                        usize::min_value(),
+                                    )
+                                        ..=(
+                                            peer_index,
+                                            GossipKind::ConsensusTransactions,
+                                            usize::max_value(),
+                                        ),
+                                ) {
+                                    self.connected_unopened_gossip_desired.remove(&(
+                                        peer_index,
+                                        ChainId(*chain_index),
+                                        GossipKind::ConsensusTransactions,
+                                    ));
+                                }
                             }
                         }
                     }
@@ -2141,15 +2166,15 @@ where
                                 chain_index,
                                 GossipKind::ConsensusTransactions,
                                 peer_index,
-                            )) && !self
+                            )) && self
                                 .connections_by_peer_id
                                 .range(
                                     (peer_index, ConnectionId::min_value())
                                         ..=(peer_index, ConnectionId::max_value()),
                                 )
                                 .any(|(_, connection_id)| {
                                     let state = self.inner.connection_state(*connection_id);
-                                    !state.shutting_down
+                                    state.established && !state.shutting_down
                                 })
                             {
                                 debug_assert!(self
@@ -2159,7 +2184,7 @@ where
                                             NotificationsProtocol::BlockAnnounces { chain_index },
                                             peer_index,
                                             SubstreamDirection::Out,
-                                            NotificationsSubstreamState::Open,
+                                            NotificationsSubstreamState::Pending,
                                             SubstreamId::min_value(),
                                         )
                                             ..=(

diff --git a/wasm-node/CHANGELOG.md b/wasm-node/CHANGELOG.md
@@ -12,6 +12,7 @@
 - Fix panic when the runtime of a chain provides consensus information that is inconsistent with the information found in the finalized block. ([#1317](https://github.com/smol-dot/smoldot/pull/1317))
 - Incoming notification substreams are now properly when accepted when a peer doesn't have a slot or gets a slot later on. ([#1369](https://github.com/smol-dot/smoldot/pull/1369))
 - Fix panic when `chainHead_unstable_follow` is called too many times. ([#1392](https://github.com/smol-dot/smoldot/pull/1392))
+- Fix panic when opening a gossiping link to a peer that we were previously connected to. ([#1395](https://github.com/smol-dot/smoldot/pull/1395))
 
 ## 2.0.10 - 2023-11-17