fix(allowblocklist): correctly remove Peer from sets

Previously, we reinserted the `Peer` in the "undo" function which is obviously wrong. This patch fixes the behaviour to be correct and adds two regression tests.

Pull-Request: #3789.

Cargo.lock

@@ -2269,7 +2269,7 @@ dependencies = [
 
 [[package]]
 name = "libp2p-allow-block-list"
-version = "0.1.0"
+version = "0.1.1"
 dependencies = [
  "async-std",
  "libp2p-core",

misc/allow-block-list/CHANGELOG.md

@@ -1,3 +1,10 @@
+## 0.1.1 - unreleased
+
+- Correctly unblock and disallow peer in `unblock_peer` and `disallow_peer` functions.
+  See [PR 3789].
+
+[PR 3789]: https://github.com/libp2p/rust-libp2p/pull/3789
+
 ## 0.1.0
 
 - Initial release.

misc/allow-block-list/Cargo.toml

@@ -3,7 +3,7 @@ name = "libp2p-allow-block-list"
 edition = "2021"
 rust-version = "1.62.0"
 description = "Allow/block list connection management for libp2p."
-version = "0.1.0"
+version = "0.1.1"
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"
 keywords = ["peer-to-peer", "libp2p", "networking"]

misc/allow-block-list/src/lib.rs

@@ -105,7 +105,7 @@ impl Behaviour<AllowedPeers> {
     ///
     /// All active connections to this peer will be closed immediately.
     pub fn disallow_peer(&mut self, peer: PeerId) {
-        self.state.peers.insert(peer);
+        self.state.peers.remove(&peer);
         self.close_connections.push_back(peer);
         if let Some(waker) = self.waker.take() {
             waker.wake()
@@ -127,7 +127,7 @@ impl Behaviour<BlockedPeers> {
 
     /// Unblock connections to a given peer.
     pub fn unblock_peer(&mut self, peer: PeerId) {
-        self.state.peers.insert(peer);
+        self.state.peers.remove(&peer);
         if let Some(waker) = self.waker.take() {
             waker.wake()
         }
@@ -297,6 +297,24 @@ mod tests {
         assert!(cause.downcast::<Blocked>().is_ok());
     }
 
+    #[async_std::test]
+    async fn can_dial_unblocked_peer() {
+        let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<BlockedPeers>::new());
+        let mut listener = Swarm::new_ephemeral(|_| Behaviour::<BlockedPeers>::new());
+        listener.listen().await;
+
+        dialer
+            .behaviour_mut()
+            .list
+            .block_peer(*listener.local_peer_id());
+        dialer
+            .behaviour_mut()
+            .list
+            .unblock_peer(*listener.local_peer_id());
+
+        dial(&mut dialer, &listener).unwrap();
+    }
+
     #[async_std::test]
     async fn blocked_peer_cannot_dial_us() {
         let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<BlockedPeers>::new());
@@ -362,6 +380,27 @@ mod tests {
         assert!(dial(&mut dialer, &listener).is_ok());
     }
 
+    #[async_std::test]
+    async fn cannot_dial_disallowed_peer() {
+        let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<AllowedPeers>::new());
+        let mut listener = Swarm::new_ephemeral(|_| Behaviour::<AllowedPeers>::new());
+        listener.listen().await;
+
+        dialer
+            .behaviour_mut()
+            .list
+            .allow_peer(*listener.local_peer_id());
+        dialer
+            .behaviour_mut()
+            .list
+            .disallow_peer(*listener.local_peer_id());
+
+        let DialError::Denied { cause } = dial(&mut dialer, &listener).unwrap_err() else {
+            panic!("unexpected dial error")
+        };
+        assert!(cause.downcast::<NotAllowed>().is_ok());
+    }
+
     #[async_std::test]
     async fn not_allowed_peer_cannot_dial_us() {
         let mut dialer = Swarm::new_ephemeral(|_| Behaviour::<AllowedPeers>::new());