feat(addressbook): prune stale entries by last-seen time by martinconic · Pull Request #5513 · ethersphere/bee

martinconic · 2026-06-22T20:38:21Z

Checklist

I have read the coding guide.
My change requires a documentation update, and I have done it.
I have added tests to cover my changes.
I have filled out the description and linked the related issues.

Description

Implements address book pruning (#5491).

The address book recently gained a wrapping verifiedAddress{Address, Verified} struct (v2.8.0, #5477). This PR extends it with a last-seen timestamp and uses it to drop peers we have not seen for over a month, preventing the address book from accumulating stale, unreachable peers indefinitely.

Changes

pkg/addressbook
- Add LastSeen (unix seconds) to verifiedAddress.
- Put stamps LastSeen with the current time on every write (initial insertion and any record update).
- New Seen(overlays ...swarm.Address): read-modify-write that bumps only the timestamp. Overlays that are absent are skipped, and the write is throttled: an overlay already seen within the last 24h is left untouched, so per-sighting calls don't amplify disk IO (on mainnet, sightings would otherwise mean tens of thousands of redundant writes a day).
- Put and Seen are serialized by a mutex, so a concurrent Put is never rolled back by Seen's stale read.
- Entries last seen more than 30 days ago are pruned when the store is opened, inside the constructor, before the store is shared with any writer. Entries with LastSeen == 0 are kept for a later run. Best-effort: a pruning failure never blocks node startup.
- Inject a clock for testability.
pkg/hive: hearing about a peer we already know over gossip is a sighting in its own right, so Seen is called for every known peer before timestamp validation, whether or not the re-presented record is newer than the one we hold. Peers mint their bzz.Address once and re-gossip it unchanged for their whole uptime, so this is the path that keeps gossip-only peers fresh; the existing Put only fires when the record is genuinely newer. Failures are debug-logged and non-fatal.
pkg/topology/kademlia: connected peers are "constantly seen" without generating gossip, so the manage loop marks every currently connected peer as seen every 15 minutes. Failures are warning-logged and non-fatal.
pkg/statestore/storeadapter: migration step 10 stamps last_seen = now() onto existing entries that lack it, decoding into the current entry type (whose last_seen is omitempty) so all other fields round-trip unchanged. Without this, the first prune after upgrade would wipe the whole address book.

AI Disclosure

This PR contains code that has been generated by an LLM.
I have reviewed the AI generated code thoroughly.
I possess the technical expertise to responsibly review the code generated in this PR.

gacevicljubisa · 2026-06-24T11:09:39Z

Here, prune here runs once, at startup only, and it can lead that if we have some stable connections over bigger period of time, where we haven't updated LastSeen, it will remove those on next restart. But those are our most valuable connections.

Maybe we could use manage() method in kademlia, where we could trigger prune addressbook every 24h (those older then 30 days), but excluding all connected peers, and prune should not start before warmup is done (when node is started). Also, we could stamp last contact at the disconnect, so the prune clock starts from when the peer was actually lost.

One gap is left, what do to with the peer we only hear over hive gossip, but we didn't connect to?

acud · 2026-06-24T18:14:21Z

Here, prune here runs once, at startup only, and it can lead that if we have some stable connections over bigger period of time, where we haven't updated LastSeen, it will remove those on next restart. But those are our most valuable connections.

Maybe we could use manage() method in kademlia, where we could trigger prune addressbook every 24h (those older then 30 days), but excluding all connected peers, and prune should not start before warmup is done (when node is started). Also, we could stamp last contact at the disconnect, so the prune clock starts from when the peer was actually lost.

This is what we've converged to with @janos as a naive starting point to ship.

One gap is left, what do to with the peer we only hear over hive gossip, but we didn't connect to?

For now let's leave them in. The first line of defense is to get rid of garbage which is in the statestore. A peer which is seen over hive and is not connected to is still a potential peer you'd like to connect to. I agree that over time yes, probably you don't wanna keep those. I suggest to ship this first and see it works, then evolve into more well defined scenarios.

martinconic · 2026-07-13T09:54:22Z

Added two fixes:

Hive never refreshes LastSeen. Peers mint their bzz.Address once and re-gossip it unchanged, so CheckTimestamp rejects it as not-newer and Put never fires again — LastSeen freezes at first-seen, and gossip-only peers get pruned at 30 days. Fix: call UpdateLastSeen on ErrTimestampStale/ErrTimestampTooSoon (both imply a known peer). Other rejections don't refresh. Safe against reviving dead peers, since hive only gossips currently-connected peers.
UpdateLastSeen is a lost update, and fix 1 makes it reachable. Get→mutate→Put with no lock; hive runs a goroutine per gossip message, so it now races its own Put for the same overlay. Worst case: stale write-back desyncs the addressbook from the chequebook registry (swap-on only) until re-mint or restart. Fix: mutex on the store, taken by Put and UpdateLastSeen. Regression test fails without it.

acud · 2026-07-13T17:41:29Z

+
+	// mu serializes the read-modify-write in UpdateLastSeen against Put, so a
+	// concurrent Put is not rolled back by a stale copy of the entry.
+	mu sync.Mutex


i don't see why this is needed. this is not business critical information and i think it is fine with this hypothetical use case happening every now and then, if ever.

acud · 2026-07-13T17:47:10Z


+	// Prune addressbook entries whose overlays have not been seen recently, so
+	// the address book does not accumulate stale peers indefinitely.
+	if err := addressbook.Prune(time.Now().Add(-addressbookPruneAfter)); err != nil {


since you prune only on startup - there's no need to expose the method over an interface - just make sure that addressbook ctor runs the pruning internally.

…cted peers

sbackend123 · 2026-07-16T11:27:44Z

 			continue
 		}

+		// Hearing about a peer we already know is a sighting in its own right,


it seems Seen() now fires even when the incoming record is later rejected for being in the future (ErrTimestampInFuture) or invalid (ErrTimestampInvalid). These aren't meaningful sightings — a bogus record from a third party shouldn't refresh a legitimate peer's last-seen. The old placement after CheckTimestamp, gated on ErrTimestampStale || ErrTimestampTooSoon, was more precise.

Was moving it before CheckTimestamp intentional?

I think it was part of the addressing of my previous comments. The address still passes some validation until we reach this point. And it could be that the new address was minted and sent not out of malicious behavior. I am fine whatever you two decide about this - have it before or after does not matter from my side, originally I think it was inside the error check after the CheckTimestamp and I wanted to avoid a situation where the errors dictate whether the peer gets marked seen yes/no. Before or after is better from my perspective, I don't really mind which one.

(left side here)

acud

minus the comment about exporting Prune (which should anyway be a method, not a pure function), this looks good from my side

…uning Resolved a conflict in pkg/addressbook/addressbook_test.go: master's #5534 added mockCorruptedStore and TestGetCorruptedNilAddress next to this branch's Seen and prune tests. The two additions are independent, so both are kept.

gacevicljubisa

Nice work.

@acud suggested not have 24h update check, so at least in that case, please remove this information from the PR desc before merge.

gacevicljubisa · 2026-07-21T10:59:00Z

Nice work.

@acud suggested not have 24h update check, so at least in that case, please remove this information from the PR desc before merge.

Additionally I would still suggest 24h throtle to have it, becuse we have noticed increased disk writes when we we wrote TImestamp on every handshake... If my query is good, for this mainnet node, we would have more then 90k of seen events in a day, 99% redudant.

acud · 2026-07-21T12:48:18Z

Nice work.
@acud suggested not have 24h update check, so at least in that case, please remove this information from the PR desc before merge.

Additionally I would still suggest 24h throtle to have it, becuse we have noticed increased disk writes when we we wrote TImestamp on every handshake... If my query is good, for this mainnet node, we would have more then 90k of seen events in a day, 99% redudant.

hmm... ok.
but a sidenote - we cannot ignore a stale timestamp and potentially also a "too soon" timestamp - the issuing client might not know it has issued "too soon" and stale timestamp is misleading - we cannot treat this error as an error in this case - as you may receive an older timestamp via gossip, that's legit behavior and should count as "seen".

gacevicljubisa · 2026-07-21T13:59:40Z

Nice work.
@acud suggested not have 24h update check, so at least in that case, please remove this information from the PR desc before merge.

Additionally I would still suggest 24h throtle to have it, becuse we have noticed increased disk writes when we we wrote TImestamp on every handshake... If my query is good, for this mainnet node, we would have more then 90k of seen events in a day, 99% redudant.

hmm... ok. but a sidenote - we cannot ignore a stale timestamp and potentially also a "too soon" timestamp - the issuing client might not know it has issued "too soon" and stale timestamp is misleading - we cannot treat this error as an error in this case - as you may receive an older timestamp via gossip, that's legit behavior and should count as "seen".

Yes. I totaly agree. This metric is capturing just "stale" and "too soon", and real "seen" will be even more then this.

martinconic self-assigned this Jun 22, 2026

feat(addressbook): prune stale entries by last-seen time

1716cdb

martinconic force-pushed the feat/addressbook-pruning branch from 155323e to 1716cdb Compare June 23, 2026 07:49

martinconic marked this pull request as ready for review June 23, 2026 07:53

martinconic requested review from acud, akrem-chabchoub, gacevicljubisa, janos and sbackend123 June 23, 2026 07:54

acud reviewed Jun 24, 2026

View reviewed changes

Comment thread pkg/statestore/storeadapter/migration.go Outdated

Comment thread pkg/topology/kademlia/kademlia.go Outdated

Comment thread pkg/topology/kademlia/kademlia.go Outdated

acud and others added 2 commits June 27, 2026 18:25

chore: address PR comments

5ec0701

fix(addressbook): refresh last-seen on hive gossip and serialize updates

157d203

martinconic requested a review from acud July 13, 2026 10:55

acud requested changes Jul 13, 2026

View reviewed changes

fix(addressbook): mark peers seen on every sighting and refresh conne…

699b4d7

…cted peers

martinconic requested a review from acud July 15, 2026 15:38

sbackend123 reviewed Jul 16, 2026

View reviewed changes

acud approved these changes Jul 16, 2026

View reviewed changes

Comment thread pkg/addressbook/addressbook.go Outdated

martinconic added 2 commits July 17, 2026 12:47

refactor(addressbook): prune when the store is opened

59b94af

martinconic requested a review from sbackend123 July 17, 2026 10:43

gacevicljubisa approved these changes Jul 21, 2026

View reviewed changes

perf(addressbook): throttle last-seen writes to once per 24h

b3ba2f7

acud approved these changes Jul 23, 2026

View reviewed changes

martinconic merged commit a512398 into master Jul 23, 2026
18 of 19 checks passed

martinconic deleted the feat/addressbook-pruning branch July 23, 2026 13:22

Uh oh!

Conversation

martinconic commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Description

AI Disclosure

Uh oh!

gacevicljubisa commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acud commented Jun 24, 2026

Uh oh!

martinconic commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acud Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acud Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sbackend123 Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

acud Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

acud Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

acud left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gacevicljubisa left a comment

Choose a reason for hiding this comment

Uh oh!

gacevicljubisa commented Jul 21, 2026

Uh oh!

acud commented Jul 21, 2026

Uh oh!

gacevicljubisa commented Jul 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

martinconic commented Jun 22, 2026 •

edited

Loading

gacevicljubisa commented Jun 24, 2026 •

edited

Loading

martinconic commented Jul 13, 2026 •

edited

Loading