Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(swarm): report outcome of handling SwarmEvent #3865

Merged
merged 6 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

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

4 changes: 4 additions & 0 deletions swarm/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
- Raise MSRV to 1.65.
See [PR 3715].

- Return a bool from `ExternalAddresses::on_swarm_event` and `ListenAddresses::on_swarm_event` indicating whether any state was changed.
See [PR 3865].

[PR 3715]: https://github.com/libp2p/rust-libp2p/pull/3715
[PR 3865]: https://github.com/libp2p/rust-libp2p/pull/3865

## 0.42.2

Expand Down
1 change: 1 addition & 0 deletions swarm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ libp2p-swarm-test = { workspace = true }
libp2p-yamux = { workspace = true }
quickcheck = { workspace = true }
void = "1"
once_cell = "1.17.1"

[[test]]
name = "swarm_derive"
Expand Down
52 changes: 49 additions & 3 deletions swarm/src/behaviour/external_addresses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,67 @@ impl ExternalAddresses {
}

/// Feed a [`FromSwarm`] event to this struct.
///
/// Returns whether the event changed our set of external addresses.
#[allow(deprecated)]
pub fn on_swarm_event<THandler>(&mut self, event: &FromSwarm<THandler>)
pub fn on_swarm_event<THandler>(&mut self, event: &FromSwarm<THandler>) -> bool
where
THandler: IntoConnectionHandler,
{
match event {
FromSwarm::NewExternalAddr(NewExternalAddr { addr, .. }) => {
if self.addresses.len() < self.limit {
self.addresses.insert((*addr).clone());
return self.addresses.insert((*addr).clone());
}
}
FromSwarm::ExpiredExternalAddr(ExpiredExternalAddr { addr, .. }) => {
self.addresses.remove(addr);
return self.addresses.remove(addr)
}
_ => {}
}

false
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::dummy;
use libp2p_core::multiaddr::Protocol;
use once_cell::sync::Lazy;

#[test]
fn new_external_addr_returns_correct_changed_value() {
let mut addresses = ExternalAddresses::default();

let changed = addresses.on_swarm_event(&new_external_addr());
assert!(changed);

let changed = addresses.on_swarm_event(&new_external_addr());
assert!(!changed)
}

#[test]
fn expired_external_addr_returns_correct_changed_value() {
let mut addresses = ExternalAddresses::default();
addresses.on_swarm_event(&new_external_addr());

let changed = addresses.on_swarm_event(&expired_external_addr());
assert!(changed);

let changed = addresses.on_swarm_event(&expired_external_addr());
assert!(!changed)
}

fn new_external_addr() -> FromSwarm<'static, dummy::ConnectionHandler> {
FromSwarm::NewExternalAddr(NewExternalAddr { addr: &MEMORY_ADDR })
}

fn expired_external_addr() -> FromSwarm<'static, dummy::ConnectionHandler> {
FromSwarm::ExpiredExternalAddr(ExpiredExternalAddr { addr: &MEMORY_ADDR })
}

static MEMORY_ADDR: Lazy<Multiaddr> =
Lazy::new(|| Multiaddr::empty().with(Protocol::Memory(1000)));
Comment on lines +96 to +97
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the value of a static over a const here? Why does it warrant introducing once_cell as a dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiaddr::empty and Multiaddr::with are unfortunately not const.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. I missed that.

I am hesitant to introduce a dependency just for the sake of more succinct test code. That said, once_cell seems to be everywhere, thus not really an additional dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is on the pathway to being stabilized and part of std: rust-lang/rust#105587

}
58 changes: 54 additions & 4 deletions swarm/src/behaviour/listen_addresses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,69 @@ impl ListenAddresses {
}

/// Feed a [`FromSwarm`] event to this struct.
///
/// Returns whether the event changed our set of listen addresses.
#[allow(deprecated)]
pub fn on_swarm_event<THandler>(&mut self, event: &FromSwarm<THandler>)
pub fn on_swarm_event<THandler>(&mut self, event: &FromSwarm<THandler>) -> bool
where
THandler: IntoConnectionHandler,
{
match event {
FromSwarm::NewListenAddr(NewListenAddr { addr, .. }) => {
self.addresses.insert((*addr).clone());
self.addresses.insert((*addr).clone())
}
FromSwarm::ExpiredListenAddr(ExpiredListenAddr { addr, .. }) => {
self.addresses.remove(addr);
self.addresses.remove(addr)
}
_ => {}
_ => false,
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::dummy;
use libp2p_core::multiaddr::Protocol;
use once_cell::sync::Lazy;

#[test]
fn new_listen_addr_returns_correct_changed_value() {
let mut addresses = ListenAddresses::default();

let changed = addresses.on_swarm_event(&new_listen_addr());
assert!(changed);

let changed = addresses.on_swarm_event(&new_listen_addr());
assert!(!changed)
}

#[test]
fn expired_listen_addr_returns_correct_changed_value() {
let mut addresses = ListenAddresses::default();
addresses.on_swarm_event(&new_listen_addr());

let changed = addresses.on_swarm_event(&expired_listen_addr());
assert!(changed);

let changed = addresses.on_swarm_event(&expired_listen_addr());
assert!(!changed)
}

fn new_listen_addr() -> FromSwarm<'static, dummy::ConnectionHandler> {
FromSwarm::NewListenAddr(NewListenAddr {
listener_id: Default::default(),
addr: &MEMORY_ADDR,
})
}

fn expired_listen_addr() -> FromSwarm<'static, dummy::ConnectionHandler> {
FromSwarm::ExpiredListenAddr(ExpiredListenAddr {
listener_id: Default::default(),
addr: &MEMORY_ADDR,
})
}

static MEMORY_ADDR: Lazy<Multiaddr> =
Lazy::new(|| Multiaddr::empty().with(Protocol::Memory(1000)));
}