Skip to content

fix(relay): enforce per-peer reservation limit at boundary#6365

Open
failuresmith wants to merge 2 commits intolibp2p:masterfrom
failuresmith:codex/fix-relay-per-peer-reservation-boundary
Open

fix(relay): enforce per-peer reservation limit at boundary#6365
failuresmith wants to merge 2 commits intolibp2p:masterfrom
failuresmith:codex/fix-relay-per-peer-reservation-boundary

Conversation

@failuresmith
Copy link
Copy Markdown

Summary

  • change the per-peer reservation guard to reject at the configured boundary
  • add a regression test covering two connections that share the same PeerId

Root cause

The relay admission check used > instead of >= when evaluating max_reservations_per_peer, which allowed one extra reservation when a peer was already exactly at the configured limit.

Validation

  • cargo fmt --check
  • cargo test -p libp2p-relay

Comment on lines +354 to +361
let relay_addr = Multiaddr::empty().with(Protocol::Memory(rand::random::<u64>()));
let mut relay = build_relay_with_config(relay::Config {
max_reservations: 10,
max_reservations_per_peer: 1,
reservation_duration: Duration::from_secs(60),
..relay::Config::default()
});
let relay_peer_id = *relay.local_peer_id();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The test is very much appreciated, thanks!

The test right now creates two clients with the same peer ID, because the client behavior can have only one reservation per connection at a time.
I think we can simplify this quite a lot by instead adding two listen addresses for the relay and have one client try to use both addresses (which internally will then create two connections between client and relay).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants