Skip to content

Conversation

@Galoretka
Copy link

This change enforces the documented contract that connections matching bypass rules “won’t be counted toward limits.” Previously, while bypassed peers skipped limit checks in handle_established_*, established counters were still incremented unconditionally in on_swarm_event() upon FromSwarm::ConnectionEstablished, causing bypassed connections to inflate total and per-peer counts. This contradicted both the crate-level documentation and the 0.6.0 changelog entry introducing bypass semantics. The fix guards established counter updates behind !is_bypassed(&peer_id), ensuring bypassed connections are neither checked nor counted. Additionally, the per-peer removal path on ConnectionClosed no longer creates empty entries with .or_default() and removes the map entry when empty, preventing stale allocations, which is particularly relevant now that bypassed connections are not inserted into the per-peer map.

@elenaf9
Copy link
Member

elenaf9 commented Nov 26, 2025

We already check if a peer is bypassed and early return before checking the per-peer limits:

fn handle_established_inbound_connection(
&mut self,
connection_id: ConnectionId,
peer: PeerId,
_: &Multiaddr,
_: &Multiaddr,
) -> Result<THandler<Self>, ConnectionDenied> {
self.pending_inbound_connections.remove(&connection_id);
if self.is_bypassed(&peer) {
return Ok(dummy::ConnectionHandler);
}

fn handle_established_outbound_connection(
&mut self,
connection_id: ConnectionId,
peer: PeerId,
_: &Multiaddr,
_: Endpoint,
_: PortUse,
) -> Result<THandler<Self>, ConnectionDenied> {
self.pending_outbound_connections.remove(&connection_id);
if self.is_bypassed(&peer) {
return Ok(dummy::ConnectionHandler);
}

And the total connection count is calculated with established_inbound_connections and established_outbound_connections, not with established_per_peer.

@elenaf9 elenaf9 closed this Nov 26, 2025
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