Skip to content

Commit

Permalink
fix(autonat): prohibit potentially large memory consumption
Browse files Browse the repository at this point in the history
Autonat uses `Vec` for the servers field to hold a list of `PeerId` when calling `Behaviour::add_server`, but there is no check to prevent duplicated entries of the `PeerId`, which would cause unnecessary allocation overtime with repeated calls to the function.

Resolves #3733.

Pull-Request: #3736.
  • Loading branch information
dariusc93 committed Apr 11, 2023
1 parent 06ea8ce commit 8f8c86d
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 8 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 protocols/autonat/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 0.10.2 - unreleased

- Store server `PeerId`s in `HashSet` to avoid duplicates and lower memory consumption.
See [PR 3736].

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

## 0.10.1

- Migrate from `prost` to `quick-protobuf`. This removes `protoc` dependency. See [PR 3312].
Expand Down
2 changes: 1 addition & 1 deletion protocols/autonat/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name = "libp2p-autonat"
edition = "2021"
rust-version = "1.62.0"
description = "NAT and firewall detection for libp2p"
version = "0.10.1"
version = "0.10.2"
authors = ["David Craven <david@craven.ch>", "Elena Frank <elena.frank@protonmail.com>"]
license = "MIT"
repository = "https://github.com/libp2p/rust-libp2p"
Expand Down
8 changes: 4 additions & 4 deletions protocols/autonat/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use libp2p_swarm::{
PollParameters, THandler, THandlerInEvent, THandlerOutEvent, ToSwarm,
};
use std::{
collections::{HashMap, VecDeque},
collections::{HashMap, HashSet, VecDeque},
iter,
task::{Context, Poll},
time::Duration,
Expand Down Expand Up @@ -170,7 +170,7 @@ pub struct Behaviour {
config: Config,

// Additional peers apart from the currently connected ones, that may be used for probes.
servers: Vec<PeerId>,
servers: HashSet<PeerId>,

// Assumed NAT status.
nat_status: NatStatus,
Expand Down Expand Up @@ -227,7 +227,7 @@ impl Behaviour {
inner,
schedule_probe: Delay::new(config.boot_delay),
config,
servers: Vec::new(),
servers: HashSet::new(),
ongoing_inbound: HashMap::default(),
ongoing_outbound: HashMap::default(),
connected: HashMap::default(),
Expand Down Expand Up @@ -266,7 +266,7 @@ impl Behaviour {
/// These peers are used for dial-request even if they are currently not connection, in which case a connection will be
/// establish before sending the dial-request.
pub fn add_server(&mut self, peer: PeerId, address: Option<Multiaddr>) {
self.servers.push(peer);
self.servers.insert(peer);
if let Some(addr) = address {
self.inner.add_address(&peer, addr);
}
Expand Down
4 changes: 2 additions & 2 deletions protocols/autonat/src/behaviour/as_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use libp2p_swarm::{
};
use rand::{seq::SliceRandom, thread_rng};
use std::{
collections::{HashMap, VecDeque},
collections::{HashMap, HashSet, VecDeque},
task::{Context, Poll},
time::Duration,
};
Expand Down Expand Up @@ -90,7 +90,7 @@ pub struct AsClient<'a> {
pub connected: &'a HashMap<PeerId, HashMap<ConnectionId, Option<Multiaddr>>>,
pub probe_id: &'a mut ProbeId,

pub servers: &'a Vec<PeerId>,
pub servers: &'a HashSet<PeerId>,
pub throttled_servers: &'a mut Vec<(PeerId, Instant)>,

pub nat_status: &'a mut NatStatus,
Expand Down

0 comments on commit 8f8c86d

Please sign in to comment.