[rpc] Add Tracking to Subscriptions Made via HTTP for Unused Filter Eviction

[onelapahead]

This adds a missing feature from Geth: evicting stale filters via a background "timeout" loop. Similar to what was done in https://github.com/okx/xlayer-erigon/pull/777 but hopefully more efficient.

Meant to help avoid accumulating Go heap for unused filters which can further hurt performance over time.

Also adds metrics for keeping track of the number of filters we have so we can confirm they do not grow indefinitely.

[onelapahead]

We've been soaking this in our nodes with success on ensuring unused filters are evicted overtime. This PR is ready for review now.

[canepat]

Thanks for this, it looks like a solid contribution.

LGTM, please rebase on main and avoid changing execution-spec-tests commit

[onelapahead]

Thanks @canepat - should be ready for review now. We'll start soaking this on a cherry picked version of v3.3.8 including this change.

[yperbasis]

Bug: Log store leak on remote filter update failure

Severity: High

In unsubscribeLogsInternal, if the remote logs filter update fails, the function returns false without calling deleteLogStore(id). The filter has already been removed from logsFilters (via removeLogsFilter),
but its associated log store in ff.logsStores is never cleaned up — it will accumulate indefinitely.

The old code always reached deleteLogStore(id) regardless of the remote update outcome. Fix:

// In unsubscribeLogsInternal, before return false in the error path:
ff.deleteLogStore(id)
return false

Also, the return value change from true → false when the remote update fails is defensible (the operation partially failed), but callers should be aware that the local filter was removed even though false is
returned.

---

Race condition: TOCTOU in eviction loop

Severity: Medium

The eviction uses a two-phase approach:

1. Phase 1: Range over the SyncMap, calling ShouldEvict() and collecting IDs
2. Phase 2: Iterate collected IDs, call unsubscribe*Internal()

Between phases, a client can call TouchSubscription() (resetting lastAccess via atomic store), but phase 2 does not re-check ShouldEvict(). An actively-polled subscription could be evicted.

The SyncMap uses sync.RWMutex — Range holds RLock, but releases it before phase 2. The atomic lastAccess update in Touch is invisible to the already-evaluated ShouldEvict.

Fix: Re-check ShouldEvict inside the phase-2 loop before unsubscribing:

for _, id := range headsToEvict {
if sub, ok := ff.headsSubs.Get(id); ok {
if !sub.ShouldEvict(timeout) {
continue // Touched since we collected it
}
// ... proceed with eviction
}
}

This doesn't eliminate the race entirely (another TOCTOU window between the re-check and unsubscribe), but it shrinks the window from O(full Range iteration) to O(single unsubscribe call), which is practical
given that the timeout is typically minutes.

---

Race condition: Double-close on concurrent unsubscribe

Severity: Low (mitigated by existing guard)

The eviction loop and a client-initiated UnsubscribeHeads can race on the same ID:

1. Eviction: Get(id) → gets sub reference
2. Client: Get(id) → gets same sub reference
3. Eviction: Delete(id) + sub.Close()
4. Client: sub.Close() again

The chan_sub.Close() has a if s.closed { return } guard under a mutex, which prevents a panic from double-closing the channel. However, both goroutines call decrementMetrics, leading to a double-decrement of
the gauge — the active subscription count would go negative.

Fix: The UnsubscribeHeads public method should handle the case where unsubscribeHeadsInternal returns false (already evicted) by skipping the decrementMetrics call — which it already does. So this is safe as
written for metrics. The Close() double-call is protected by the mutex guard. Low severity, but worth a comment.

---

Metrics: subscriptions renamed to subscriptions_active

Severity: Low (operational)

The gauge metric name changes from subscriptions to subscriptions_active, and gains a protocol label dimension. This is a breaking change for any existing Prometheus dashboards or alerting rules referencing
subscriptions{filter=...}.

Investigation shows the old activeSubscriptionsGauge was defined but never actually used in the codebase — so this is effectively creating a new metric, not breaking an existing one. The rename is fine.

The getSubscriptionCounter and getReapedCounter functions use embedded labels in metric names (e.g., subscriptions_created_total{filter="logs",protocol="http"}). This pattern is correct — the Erigon metrics
library (diagnostics/metrics/parsing.go) explicitly parses embedded {label="value"} syntax from metric names, and this pattern is used widely (e.g., in execution/commitment/commitment.go).

---

Summary

  │                        Issue                         │ Severity │               Type                │                                                                                                           
  ├──────────────────────────────────────────────────────┼──────────┼───────────────────────────────────┤
  │ Log store leak on remote update failure              │ High     │ Bug                               │                                                                                                           
  ├──────────────────────────────────────────────────────┼──────────┼───────────────────────────────────┤
  │ TOCTOU: eviction after Touch                         │ Medium   │ Race condition                    │                                                                                                           
  ├──────────────────────────────────────────────────────┼──────────┼───────────────────────────────────┤
  │ Double-close / double-decrement potential            │ Low      │ Race condition (mostly mitigated) │                                                                                                           
  ├──────────────────────────────────────────────────────┼──────────┼───────────────────────────────────┤                                                                                                           
  │ SetProtocol not thread-safe                          │ Low      │ Data race                         │
  ├──────────────────────────────────────────────────────┼──────────┼───────────────────────────────────┤                                                                                                           
  │ Metric rename (subscriptions → subscriptions_active) │ Low      │ Operational / breaking dashboards │
  ├──────────────────────────────────────────────────────┼──────────┼───────────────────────────────────┤                                                                                                           
  │ Exported DecrementMetrics unused                     │ Nit      │ Dead code                         │
  ├──────────────────────────────────────────────────────┼──────────┼───────────────────────────────────┤                                                                                                           
  │ 0 * time.Minute                                      │ Nit      │ Style                             │
  ├──────────────────────────────────────────────────────┼──────────┼───────────────────────────────────┤                                                                                                           
  │ Verbose per-touch Trace logging                      │ Nit      │ Performance                       │
  └──────────────────────────────────────────────────────┴──────────┴───────────────────────────────────┘                                                                                                           

The log store leak is the only must-fix before merge. The TOCTOU race should ideally be addressed (re-check before evicting), but given the timeout is on the order of minutes, it's unlikely to cause real issues
in practice. The rest are minor.

[onelapahead]

Sorry @yperbasis - been awhile on this but have rebased and addressed feedback 🙏🏻

[Copilot]

Pull request overview

Adds timeout-based tracking and eviction for RPC polling filters (to prevent stale filters from accumulating), along with new metrics and CLI configuration to control eviction behavior.

Changes:

• Extend rpchelper.Sub with last-access/protocol tracking and implement it in chan_sub.
• Add a background eviction loop in rpchelper.Filters driven by a configurable timeout, plus TouchSubscription/tracking helpers and subscription lifecycle metrics.
• Wire protocol labeling and timeout tracking into JSON-RPC filter creation/polling paths, and expose a new CLI flag/config field for the timeout.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
rpc/rpchelper/subscription.go	Adds protocol + last-access tracking methods to subscriptions to support eviction and metrics.
rpc/rpchelper/metrics.go	Introduces protocol-labeled gauges and subscription lifecycle counters.
rpc/rpchelper/filters.go	Implements the timeout eviction loop, touch/track hooks, unsubscribe internals, and metric updates.
rpc/rpchelper/config.go	Adds RpcSubscriptionFiltersTimeout to configuration with default disabled (0).
rpc/jsonrpc/eth_filters.go	Hooks filter creation/polling into tracking/touch logic and labels WS subscriptions.
node/cli/flags.go	Adds rpc.subscription.filters.timeout flag for embedded node CLI.
node/cli/default_flags.go	Registers the new timeout flag in default flag set.
cmd/rpcdaemon/cli/config.go	Adds the timeout option to rpcdaemon’s Cobra flags/config.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yperbasis]

Thanks @onelapahead! Your branch is on an org-owned fork, which GitHub does not let base-repo maintainers push to, so we couldn't deliver the merge with latest main here. The work continues in #22261, which carries your commit with authorship preserved plus the conflict resolution against the refactored WebSocket subscription handlers. Closing in favor of that PR.