diff --git a/sei-tendermint/internal/p2p/conn_tracker.go b/sei-tendermint/internal/p2p/conn_tracker.go index 340807d3ff..0c57668b70 100644 --- a/sei-tendermint/internal/p2p/conn_tracker.go +++ b/sei-tendermint/internal/p2p/conn_tracker.go @@ -13,6 +13,7 @@ type connTracker struct { mutex sync.RWMutex max uint window time.Duration + nextSweep time.Time } func newConnTracker(max uint, window time.Duration) *connTracker { @@ -30,10 +31,29 @@ func (rat *connTracker) Len() int { return len(rat.cache) } +// sweepLocked drops lastConnect entries whose window has elapsed. +func (rat *connTracker) sweepLocked(now time.Time) { + // At most once per window. An entry is only consulted while it is inside the + // window, so anything older is dead weight. RemoveConn drops an entry itself + // when the connection outlived the window, which leaves exactly the addresses + // whose connections died inside it: without this sweep those stay for the life + // of the process, and on a public listener that set is unbounded. + 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) + } + } +} + func (rat *connTracker) AddConn(addrPort netip.AddrPort) error { address := addrPort.Addr() rat.mutex.Lock() defer rat.mutex.Unlock() + rat.sweepLocked(time.Now()) if num := rat.cache[address]; num >= rat.max { return fmt.Errorf("%q has %d connections [max=%d]", address, num, rat.max) @@ -56,6 +76,7 @@ func (rat *connTracker) RemoveConn(addrPort netip.AddrPort) { address := addrPort.Addr() rat.mutex.Lock() defer rat.mutex.Unlock() + rat.sweepLocked(time.Now()) if num := rat.cache[address]; num > 0 { rat.cache[address]-- diff --git a/sei-tendermint/internal/p2p/conn_tracker_test.go b/sei-tendermint/internal/p2p/conn_tracker_test.go index 6490c817b9..e8d16cc716 100644 --- a/sei-tendermint/internal/p2p/conn_tracker_test.go +++ b/sei-tendermint/internal/p2p/conn_tracker_test.go @@ -83,3 +83,32 @@ func TestConnTracker(t *testing.T) { }) } + +// A connection that dies inside the window keeps its lastConnect entry, because +// the window has not elapsed and a reconnect still has to be refused. Nothing +// revisited those entries afterwards, so on a public listener every address whose +// connection was short-lived stayed in the map for the life of the process. +func TestConnTrackerShortLivedConnsDoNotAccumulate(t *testing.T) { + const conns = 100_000 + + ct := newConnTracker(10, time.Millisecond) + for range conns { + ip := randLocalAddr() + require.NoError(t, ct.AddConn(ip)) + ct.RemoveConn(ip) + } + + // Bounded by the addresses seen within one window rather than by every address + // seen. The margin is wide because the sweep is driven by elapsed time. + require.Less(t, len(ct.lastConnect), conns/10) +} + +// Reclaiming entries must not let an address reconnect inside its window. +func TestConnTrackerSweepPreservesWindow(t *testing.T) { + ct := newConnTracker(10, time.Hour) + ip := randLocalAddr() + + require.NoError(t, ct.AddConn(ip)) + ct.RemoveConn(ip) + require.Error(t, ct.AddConn(ip)) +}