fix(p2p): reclaim connTracker lastConnect entries once their window elapses

[bdchatham]

Problem

connTracker keeps two maps. cache counts an address's live connections and is cleaned up when it reaches zero. lastConnect records when an address last connected, and is consulted only when cache[addr] == 0, to refuse a reconnect inside IncomingConnectionWindow (100ms by default).

RemoveConn drops the lastConnect entry only when the connection outlived the window:

if last, ok := rat.lastConnect[address]; ok && time.Since(last) > rat.window {
    delete(rat.lastConnect, address)
}

That condition is correct. Inside the window a reconnect still has to be refused, so the entry is still live state and deleting it would weaken the limit.

The bug is that nothing revisits it afterwards. An address whose connection died inside the window keeps its entry for the life of the process. On a public listener the set of such addresses is unbounded, and it is fed by exactly the traffic a public listener sees most: failed handshakes, port scans, protocol probes, connections dropped mid-negotiation.

Change

Sweep expired entries, at most once per window, from both AddConn and RemoveConn:

// sweepLocked drops lastConnect entries whose window has elapsed.
func (rat *connTracker) sweepLocked(now time.Time) {
	if now.Before(rat.nextSweep) {
		return
	}
	rat.nextSweep = now.Add(rat.window)
	for address, last := range rat.lastConnect {
		if now.Sub(last) > rat.window {
			delete(rat.lastConnect, address)
		}
	}
}

An entry is only consulted while it is inside the window, so anything older is dead weight. RemoveConn already handles connections that outlived the window, which leaves exactly the short-lived addresses for the sweep to reclaim.

The map is now bounded by the addresses seen within one window rather than by every address ever seen. Sweep cost is amortised: at most one pass per window, over a map that the sweep itself keeps small.

Tests

TestConnTrackerShortLivedConnsDoNotAccumulate opens and immediately closes 100k connections from distinct addresses and asserts the map does not retain them. Without the sweep it fails with "100000" is not less than "10000" — every address retained. With it, the map stays small.

TestConnTrackerSweepPreservesWindow guards the other direction: reclaiming entries must not let an address reconnect inside its window. That one passes before and after, which is the point.

The bound is asserted loosely (conns/10) because the sweep is driven by elapsed time rather than by call count.

go test ./internal/p2p/, gofmt, goimports and golangci-lint (v2.8.0) are clean.

[cursor]

PR Summary

Medium Risk
Touches P2P connection admission on every add/remove; behavior change is limited to reclaiming stale map entries after the window, with tests guarding the reconnect limit.

Overview
Fixes unbounded growth of connTracker's lastConnect map on public listeners when peers connect and disconnect inside the reconnect window (scans, failed handshakes, etc.). RemoveConn only deleted entries after the window had already passed on that path, so short-lived addresses were never revisited.

Adds sweepLocked, run from AddConn and RemoveConn at most once per window, to delete lastConnect entries older than the window. nextSweep throttles full-map passes. Reconnect refusal inside the window is unchanged.

New tests cover that 100k distinct short-lived connections do not retain every address in lastConnect, and that sweeping does not allow an immediate reconnect within the window.

^{Reviewed by Cursor Bugbot for commit 63b0fdf. Bugbot is set up for automated code reviews on this repo. Configure here.}

[github-actions]

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	✅ passed	✅ passed	✅ passed	Aug 13, 2026, 4:01 PM

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.47%. Comparing base (8bbac80) to head (63b0fdf).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3918      +/-   ##
==========================================
- Coverage   59.48%   58.47%   -1.02%     
==========================================
  Files        2325     2229      -96     
  Lines      198647   188024   -10623     
==========================================
- Hits       118160   109938    -8222     
+ Misses      69258    67699    -1559     
+ Partials    11229    10387     -842     

Flag	Coverage Δ
sei-chain-pr	78.26% <100.00%> (?)
sei-db	70.41% <ø> (ø)
sei-db-state-db	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
sei-tendermint/internal/p2p/conn_tracker.go	100.00% <100.00%> (ø)

... and 97 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[seidroid]

The sweep is correct — it only drops entries older than the window, which are exactly the entries AddConn can no longer refuse on, so the rate limit is unchanged and the lastConnect map is now bounded by roughly one window's arrivals. Both new tests, however, leave the sweep's window check unexercised and the accumulation bound timing-dependent; no blocking issues.

Findings: 0 blocking | 3 non-blocking | 2 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• No existing or new test covers the now.Sub(last) > rat.window condition inside sweepLocked: replacing that loop body with an unconditional delete for every entry keeps TestConnTrackerSweepPreservesWindow, TestConnTrackerShortLivedConnsDoNotAccumulate, TestConnTracker/Window and TestConnTracker/VeryShort all green. See the inline comments for the two cases that would close this.
• 2 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] With window = time.Hour this test never runs the sweep against a populated map, so it does not guard what its comment claims. Trace it: the first AddConn sweeps an empty map and sets nextSweep = now + 1h; the following RemoveConn and AddConn both hit now.Before(rat.nextSweep) and return early. Replacing the loop body in sweepLocked with an unconditional delete(rat.lastConnect, address) still leaves this test passing (as it does TestConnTracker/Window and /VeryShort), so the now.Sub(last) > rat.window guard is currently untested.

The case that exercises it needs an entry created after the last sweep but still inside its window when the next sweep fires — e.g. with a short window, prime nextSweep with a throwaway address, sleep until just before it elapses, AddConn/RemoveConn the address under test, then sleep past nextSweep and make one more call so the sweep runs while that entry is only a fraction of a window old, and assert the reconnect is still refused.

[seidroid]

[suggestion] This bound is a function of loop throughput rather than of the sweep: the surviving entries are those added since the last sweep, i.e. roughly window / per-iteration cost. At an estimated ~0.5–1µs per iteration that lands around 1–3k, comfortably under 10k, and -race in CI only widens the margin — but the relationship is inverted from what you want (a faster machine retains more), so the headroom shrinks precisely where the test is cheapest to run.

You can make it deterministic and much stronger at the same time by forcing the final sweep instead of sampling mid-stream: after the loop, time.Sleep(2 * time.Millisecond) then AddConn/RemoveConn one fresh address. That call sweeps (nextSweep has certainly elapsed) and every loop entry is now older than the window, so require.Len(t, ct.lastConnect, 1) holds exactly, and it still fails without the sweep.