Skip to content

feat(pstore): cap unconnected addrs per peer#3486

Open
lidel wants to merge 3 commits intomasterfrom
feat/pstoremem-per-peer-addr-cap
Open

feat(pstore): cap unconnected addrs per peer#3486
lidel wants to merge 3 commits intomasterfrom
feat/pstoremem-per-peer-addr-cap

Conversation

@lidel
Copy link
Copy Markdown
Member

@lidel lidel commented Apr 14, 2026

Note

This is another fix that aims to reduce stale addr accumulation (defensive, on Consumer side).

Problem

Third-party peers (e.g. via DHT gossip) can flood the peerstore with stale or misconfigured addresses for a single peer id. Real-world delegated-routing responses top out around 26 addrs per well-connected node, but a misbehaving peer can push hundreds before the global peerstore cap kicks in, crowding out good addrs during dial selection.

This PR

Bound the stored unconnected addrs per peer to 64 by default and evict the unconnected entry with the nearest expiry when the cap is reached. This naturally sheds short-TTL gossip (TempAddrTTL) before longer-lived identify-backed entries (RecentlyConnectedAddrTTL) without tracking provenance. Addrs held by a live connection are never counted toward the cap and never evicted.

  • WithMaxAddressesPerPeer(n) lets callers tune the cap.
  • Enforcement lives in addAddrsUnlocked and SetAddrs, the two write paths that can grow the per-peer set.
  • Shared-suite SetNegativeTTLClears adds 100 addrs, above the new default; the inmem test factory raises the cap to 10000 so the shared suite keeps exercising general behavior rather than the cap path.

Open Questions

  • Is this the right approach?
  • Is 64 the right implicit default limit?

Third-party peers (e.g. via DHT gossip) can flood the peerstore with
stale or misconfigured addresses for a single peer id. Real-world
delegated-routing responses top out around 26 addrs per well-connected
node, but a misbehaving peer can push hundreds before the global
peerstore cap kicks in, crowding out good addrs during dial selection.

Bound the stored unconnected addrs per peer to 64 by default and evict
the unconnected entry with the nearest expiry when the cap is reached.
This naturally sheds short-TTL gossip (TempAddrTTL) before longer-lived
identify-backed entries (RecentlyConnectedAddrTTL) without tracking
provenance. Addrs held by a live connection are never counted toward
the cap and never evicted.

- `WithMaxAddressesPerPeer(n)` lets callers tune the cap.
- Enforcement lives in `addAddrsUnlocked` and `SetAddrs`, the two
  write paths that can grow the per-peer set.
- Shared-suite `SetNegativeTTLClears` adds 100 addrs, above the new
  default; the inmem test factory raises the cap to 10000 so the
  shared suite keeps exercising general behavior rather than the
  cap path.
@lidel lidel changed the title feat(pstoremem): cap unconnected addrs per peer feat(pstore): cap unconnected addrs per peer Apr 15, 2026
Mirror the per-peer address cap from pstoremem in the datastore-backed
addr book so both backends bound peerstore pollution from sources like
DHT gossip. Adds Options.MaxAddrsPerPeer (default 64); when full, the
unconnected entry with the nearest expiry is evicted before the new
entry is inserted. Addresses held by a live connection
(TTL >= ConnectedAddrTTL) are not counted and never evicted.

A non-positive cap now disables the check in both backends, so
WithMaxAddressesPerPeer(0) and Options.MaxAddrsPerPeer = 0 opt out
cleanly. Shared suite fixtures use that switch instead of an arbitrary
high number.
@lidel lidel requested a review from MarcoPolo April 15, 2026 00:10
@lidel lidel marked this pull request as ready for review April 15, 2026 00:29
Comment thread p2p/host/peerstore/pstoreds/addr_book.go
Comment thread p2p/host/peerstore/pstoreds/addr_book.go Outdated
Co-authored-by: Marco Munizaga <git@marcopolo.io>
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