Skip to content

Commit

Permalink
fix(allowblocklist): correctly remove Peer from sets
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
thomaseizinger authored Apr 19, 2023
1 parent 96288b8 commit 5df321d
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 4 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions misc/allow-block-list/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion misc/allow-block-list/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
43 changes: 41 additions & 2 deletions misc/allow-block-list/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
}
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down

0 comments on commit 5df321d

Please # to comment.