From 6d67fb1c8bb203dfc0c926f72f1fde4eef349f24 Mon Sep 17 00:00:00 2001 From: hfuss Date: Thu, 8 Jan 2026 09:39:13 -0500 Subject: [PATCH 01/11] [rpc] Add Tracking to Subscriptions Made via HTTP for Unused Filter Eviction Signed-off-by: hfuss self-review fixes Signed-off-by: hfuss --- cmd/rpcdaemon/cli/config.go | 1 + node/cli/default_flags.go | 1 + node/cli/flags.go | 6 + rpc/jsonrpc/eth_filters.go | 52 +++--- rpc/rpchelper/config.go | 17 +- rpc/rpchelper/filters.go | 319 ++++++++++++++++++++++++++++++++-- rpc/rpchelper/metrics.go | 34 +++- rpc/rpchelper/subscription.go | 67 ++++++- 8 files changed, 454 insertions(+), 43 deletions(-) diff --git a/cmd/rpcdaemon/cli/config.go b/cmd/rpcdaemon/cli/config.go index eafe14097e7..e692f42574f 100644 --- a/cmd/rpcdaemon/cli/config.go +++ b/cmd/rpcdaemon/cli/config.go @@ -181,6 +181,7 @@ func RootCommand() (*cobra.Command, *httpcfg.HttpCfg) { rootCmd.PersistentFlags().IntVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersMaxTxs, "rpc.subscription.filters.maxtxs", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersMaxTxs, "Maximum number of transactions to store per subscription.") rootCmd.PersistentFlags().IntVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersMaxAddresses, "rpc.subscription.filters.maxaddresses", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersMaxAddresses, "Maximum number of addresses per subscription to filter logs by.") rootCmd.PersistentFlags().IntVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersMaxTopics, "rpc.subscription.filters.maxtopics", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersMaxTopics, "Maximum number of topics per subscription to filter logs by.") + rootCmd.PersistentFlags().DurationVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersTimeout, "rpc.subscription.filters.timeout", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersTimeout, "Timeout before idle filters are evicted. Defaults to 0 to disable eviction.") rootCmd.PersistentFlags().IntVar(&cfg.BlockRangeLimit, utils.RpcBlockRangeLimit.Name, utils.RpcBlockRangeLimit.Value, utils.RpcBlockRangeLimit.Usage) rootCmd.PersistentFlags().IntVar(&cfg.GetLogsMaxResults, utils.RpcGetLogsMaxResults.Name, utils.RpcGetLogsMaxResults.Value, utils.RpcGetLogsMaxResults.Usage) rootCmd.PersistentFlags().IntVar(&cfg.BatchLimit, utils.RpcBatchLimit.Name, utils.RpcBatchLimit.Value, utils.RpcBatchLimit.Usage) diff --git a/node/cli/default_flags.go b/node/cli/default_flags.go index 46c914537f2..25af40f859e 100644 --- a/node/cli/default_flags.go +++ b/node/cli/default_flags.go @@ -121,6 +121,7 @@ var DefaultFlags = []cli.Flag{ &RpcSubscriptionFiltersMaxTxsFlag, &RpcSubscriptionFiltersMaxAddressesFlag, &RpcSubscriptionFiltersMaxTopicsFlag, + &RpcSubscriptionFiltersTimeoutFlag, &utils.SnapKeepBlocksFlag, &utils.SnapStopFlag, diff --git a/node/cli/flags.go b/node/cli/flags.go index 1302e6c545c..a174a29d769 100644 --- a/node/cli/flags.go +++ b/node/cli/flags.go @@ -206,6 +206,11 @@ var ( Usage: "Maximum number of topics per subscription to filter logs by.", Value: rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersMaxTopics, } + RpcSubscriptionFiltersTimeoutFlag = cli.DurationFlag{ + Name: "rpc.subscription.filters.timeout", + Usage: "Timeout before idle filters are evicted. Set to 0 to disable eviction.", + Value: rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersTimeout, + } ) // BuildEthConfig applies all CLI flags to the ethconfig.Config. This is the single @@ -466,6 +471,7 @@ func setEmbeddedRpcDaemon(ctx *cli.Context, cfg *nodecfg.Config, logger log.Logg RpcSubscriptionFiltersMaxTxs: ctx.Int(RpcSubscriptionFiltersMaxTxsFlag.Name), RpcSubscriptionFiltersMaxAddresses: ctx.Int(RpcSubscriptionFiltersMaxAddressesFlag.Name), RpcSubscriptionFiltersMaxTopics: ctx.Int(RpcSubscriptionFiltersMaxTopicsFlag.Name), + RpcSubscriptionFiltersTimeout: ctx.Duration(RpcSubscriptionFiltersTimeoutFlag.Name), }, Gascap: ctx.Uint64(utils.RpcGasCapFlag.Name), BlockRangeLimit: ctx.Int(utils.RpcBlockRangeLimit.Name), diff --git a/rpc/jsonrpc/eth_filters.go b/rpc/jsonrpc/eth_filters.go index eb3c926357e..cd5a63e234a 100644 --- a/rpc/jsonrpc/eth_filters.go +++ b/rpc/jsonrpc/eth_filters.go @@ -35,6 +35,7 @@ func (api *APIImpl) NewPendingTransactionFilter(_ context.Context) (string, erro return "", rpc.ErrNotificationsUnsupported } txsCh, id := api.filters.SubscribePendingTxs(32) + api.filters.TrackSubscription(rpchelper.SubscriptionID(id), rpchelper.FilterTypePendingTxs, rpchelper.ProtocolHTTP) go func() { for txs := range txsCh { api.filters.AddPendingTxs(id, txs) @@ -49,6 +50,7 @@ func (api *APIImpl) NewBlockFilter(_ context.Context) (string, error) { return "", rpc.ErrNotificationsUnsupported } ch, id := api.filters.SubscribeNewHeads(32) + api.filters.TrackSubscription(rpchelper.SubscriptionID(id), rpchelper.FilterTypeHeads, rpchelper.ProtocolHTTP) go func() { for block := range ch { api.filters.AddPendingBlock(id, block) @@ -63,6 +65,7 @@ func (api *APIImpl) NewFilter(_ context.Context, crit filters.FilterCriteria) (s return "", rpc.ErrNotificationsUnsupported } logs, id := api.filters.SubscribeLogs(256, crit) + api.filters.TrackSubscription(rpchelper.SubscriptionID(id), rpchelper.FilterTypeLogs, rpchelper.ProtocolHTTP) go func() { for lg := range logs { api.filters.AddLogs(id, lg) @@ -100,37 +103,38 @@ func (api *APIImpl) GetFilterChanges(_ context.Context, index string) ([]any, er stub := make([]any, 0) // remove 0x cutIndex := strings.TrimPrefix(index, "0x") - // Validate the id exists for any subscription type first to distinguish "never created" from "no changes yet". - exists := api.filters.HasHeadsSubscription(rpchelper.HeadsSubID(cutIndex)) || - api.filters.HasPendingTxsSubscription(rpchelper.PendingTxsSubID(cutIndex)) || - api.filters.HasSubscription(rpchelper.LogsSubID(cutIndex)) - if !exists { - return nil, rpc.ErrFilterNotFound - } - - // Identify the subscription type by probing each store; if none have data yet, return empty slice - if blocks, ok := api.filters.ReadPendingBlocks(rpchelper.HeadsSubID(cutIndex)); ok { - for _, v := range blocks { - stub = append(stub, v.Hash()) + // Identify which subscription type the id belongs to, touch it to reset the + // eviction deadline, then probe the store. Touch must happen on every poll + // — not only when there is buffered data — otherwise an actively-polling + // client whose chain is quiet would be evicted. + switch { + case api.filters.HasHeadsSubscription(rpchelper.HeadsSubID(cutIndex)): + api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeHeads) + if blocks, ok := api.filters.ReadPendingBlocks(rpchelper.HeadsSubID(cutIndex)); ok { + for _, v := range blocks { + stub = append(stub, v.Hash()) + } } return stub, nil - } - if txs, ok := api.filters.ReadPendingTxs(rpchelper.PendingTxsSubID(cutIndex)); ok { - if len(txs) > 0 { + case api.filters.HasPendingTxsSubscription(rpchelper.PendingTxsSubID(cutIndex)): + api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypePendingTxs) + if txs, ok := api.filters.ReadPendingTxs(rpchelper.PendingTxsSubID(cutIndex)); ok && len(txs) > 0 { for _, txn := range txs[0] { stub = append(stub, txn.Hash()) } - return stub, nil } return stub, nil - } - if logs, ok := api.filters.ReadLogs(rpchelper.LogsSubID(cutIndex)); ok { - for _, v := range logs { - stub = append(stub, v) + case api.filters.HasSubscription(rpchelper.LogsSubID(cutIndex)): + api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeLogs) + if logs, ok := api.filters.ReadLogs(rpchelper.LogsSubID(cutIndex)); ok { + for _, v := range logs { + stub = append(stub, v) + } } return stub, nil + default: + return nil, rpc.ErrFilterNotFound } - return []any{}, nil } // GetFilterLogs implements eth_getFilterLogs. @@ -144,6 +148,8 @@ func (api *APIImpl) GetFilterLogs(_ context.Context, index string) ([]*types.Log if found := api.filters.HasSubscription(rpchelper.LogsSubID(cutIndex)); !found { return nil, rpc.ErrFilterNotFound } + // Reset the filter deadline since it was just accessed + api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeLogs) if logs, ok := api.filters.ReadLogs(rpchelper.LogsSubID(cutIndex)); ok { return logs, nil } @@ -165,6 +171,7 @@ func (api *APIImpl) NewHeads(ctx context.Context) (*rpc.Subscription, error) { go func() { defer dbg.LogPanic() headers, id := api.filters.SubscribeNewHeads(32) + api.filters.SetSubscriptionProtocol(rpchelper.SubscriptionID(id), rpchelper.FilterTypeHeads, rpchelper.ProtocolWS) defer api.filters.UnsubscribeHeads(id) for { select { @@ -203,6 +210,7 @@ func (api *APIImpl) NewPendingTransactions(ctx context.Context, fullTx *bool) (* go func() { defer dbg.LogPanic() txsCh, id := api.filters.SubscribePendingTxs(256) + api.filters.SetSubscriptionProtocol(rpchelper.SubscriptionID(id), rpchelper.FilterTypePendingTxs, rpchelper.ProtocolWS) defer api.filters.UnsubscribePendingTxs(id) for { @@ -250,6 +258,7 @@ func (api *APIImpl) NewPendingTransactionsWithBody(ctx context.Context) (*rpc.Su go func() { defer dbg.LogPanic() txsCh, id := api.filters.SubscribePendingTxs(512) + api.filters.SetSubscriptionProtocol(rpchelper.SubscriptionID(id), rpchelper.FilterTypePendingTxs, rpchelper.ProtocolWS) defer api.filters.UnsubscribePendingTxs(id) for { @@ -291,6 +300,7 @@ func (api *APIImpl) Logs(ctx context.Context, crit filters.FilterCriteria) (*rpc go func() { defer dbg.LogPanic() logs, id := api.filters.SubscribeLogs(api.SubscribeLogsChannelSize, crit) + api.filters.SetSubscriptionProtocol(rpchelper.SubscriptionID(id), rpchelper.FilterTypeLogs, rpchelper.ProtocolWS) defer api.filters.UnsubscribeLogs(id) for { diff --git a/rpc/rpchelper/config.go b/rpc/rpchelper/config.go index ff85a580874..ea078a30a26 100644 --- a/rpc/rpchelper/config.go +++ b/rpc/rpchelper/config.go @@ -16,15 +16,21 @@ package rpchelper +import "time" + +// DefaultFilterTimeout is the default eviction timeout; 0 disables eviction. +const DefaultFilterTimeout time.Duration = 0 + // FiltersConfig defines the configuration settings for RPC subscription filters. // Each field represents a limit on the number of respective items that can be stored per subscription. // A value of 0 disables the limit (no cap). Oldest items are evicted first (FIFO) when the limit is reached. type FiltersConfig struct { - RpcSubscriptionFiltersMaxLogs int // Maximum number of logs to store per subscription. Default: 10000 - RpcSubscriptionFiltersMaxHeaders int // Maximum number of block headers to store per subscription. Default: 10000 - RpcSubscriptionFiltersMaxTxs int // Maximum number of transactions to store per subscription. Default: 10000 - RpcSubscriptionFiltersMaxAddresses int // Maximum number of addresses per subscription to filter logs by. Default: 0 (no limit) - RpcSubscriptionFiltersMaxTopics int // Maximum number of topics per subscription to filter logs by. Default: 0 (no limit) + RpcSubscriptionFiltersMaxLogs int // Maximum number of logs to store per subscription. Default: 10000 + RpcSubscriptionFiltersMaxHeaders int // Maximum number of block headers to store per subscription. Default: 10000 + RpcSubscriptionFiltersMaxTxs int // Maximum number of transactions to store per subscription. Default: 10000 + RpcSubscriptionFiltersMaxAddresses int // Maximum number of addresses per subscription to filter logs by. Default: 0 (no limit) + RpcSubscriptionFiltersMaxTopics int // Maximum number of topics per subscription to filter logs by. Default: 0 (no limit) + RpcSubscriptionFiltersTimeout time.Duration // Timeout before idle filters are evicted. Default: 0 (no eviction) } // DefaultFiltersConfig defines the default settings for filter configurations. @@ -37,4 +43,5 @@ var DefaultFiltersConfig = FiltersConfig{ RpcSubscriptionFiltersMaxTxs: 10000, RpcSubscriptionFiltersMaxAddresses: 0, // no limit RpcSubscriptionFiltersMaxTopics: 0, // no limit + RpcSubscriptionFiltersTimeout: DefaultFilterTimeout, } diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index 101d2e6277a..7a8b461d3b6 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -47,6 +47,15 @@ import ( "github.com/erigontech/erigon/txnprovider/txpool" ) +// FilterType represents the type of a filter subscription for metrics tracking. +type FilterType string + +const ( + FilterTypeLogs FilterType = "logs" + FilterTypeHeads FilterType = "heads" + FilterTypePendingTxs FilterType = "pendingTxs" +) + // Filters holds the state for managing subscriptions to various Ethereum events. // It allows for the subscription and management of events such as new blocks, pending transactions, // logs, and other Ethereum-related activities. @@ -104,6 +113,16 @@ func New(ctx context.Context, config FiltersConfig, ethBackend ApiBackend, txPoo events: events, } + // Start the timeout loop for filter eviction if timeout is configured + if config.RpcSubscriptionFiltersTimeout > 0 { + logger.Info("[rpc] [filters] starting timeout loop for idle filter eviction", + "timeout", config.RpcSubscriptionFiltersTimeout, + "checkInterval", config.RpcSubscriptionFiltersTimeout/2) + go ff.timeoutLoop(ctx, config.RpcSubscriptionFiltersTimeout) + } else { + logger.Info("[rpc] [filters] timeout-based filter eviction disabled") + } + go func() { if ethBackend == nil { return @@ -286,6 +305,210 @@ func (ff *Filters) LastPendingBlock() *types.Block { return ff.pendingBlock } +// timeoutLoop runs periodically and evicts subscriptions that have not been polled within the timeout duration. +// This prevents unbounded accumulation of idle subscriptions and matches geth's behavior. +func (ff *Filters) timeoutLoop(ctx context.Context, timeout time.Duration) { + // Check more frequently than the timeout to ensure timely eviction. + // Floor at 1s so a sub-2s timeout doesn't produce a zero or sub-second + // interval (time.NewTicker panics on non-positive durations and a tight + // loop would burn CPU for no practical benefit). + checkInterval := timeout / 2 + if checkInterval < time.Second { + checkInterval = time.Second + } + ticker := time.NewTicker(checkInterval) + defer ticker.Stop() + + ff.logger.Debug("[rpc] [filters] timeout loop started", "timeout", timeout, "checkInterval", checkInterval) + + for { + select { + case <-ctx.Done(): + ff.logger.Debug("[rpc] [filters] timeout loop stopping due to context cancellation") + return + case <-ticker.C: + ff.logger.Trace("[rpc] [filters] running eviction check") + ff.evictStaleSubscriptions(timeout) + } + } +} + +// evictStaleFilters removes filters that haven't been accessed within the timeout duration. +// It iterates the existing subscription maps and checks each subscription's embedded tracking. +func (ff *Filters) evictStaleSubscriptions(timeout time.Duration) { + var headsChecked, txsChecked, logsChecked int + var headsEvicted, txsEvicted, logsEvicted int + + // Collect expired heads subscriptions + var headsToEvict []HeadsSubID + ff.headsSubs.Range(func(id HeadsSubID, sub Sub[*types.Header]) error { + headsChecked++ + if sub.ShouldEvict(timeout) { + ff.logger.Trace("[rpc] [filters] heads filter marked for eviction", "id", id, "protocol", sub.Protocol()) + headsToEvict = append(headsToEvict, id) + } + return nil + }) + for _, id := range headsToEvict { + if sub, ok := ff.headsSubs.Get(id); ok { + // Re-check ShouldEvict: a client may have called TouchSubscription between phase 1 and phase 2. + // This shrinks the TOCTOU window but does not eliminate it; an unavoidable race remains + // between this check and unsubscribeHeadsInternal, but the timeout is on the order of minutes + // so the practical impact is bounded. + if !sub.ShouldEvict(timeout) { + continue + } + protocol := sub.Protocol() + if ff.unsubscribeHeadsInternal(id) { + headsEvicted++ + ff.logger.Info("[rpc] [filters] evicted idle heads filter", "id", id, "protocol", protocol, "timeout", timeout) + ff.decrementMetrics(FilterTypeHeads, protocol) + getReapedCounter(string(FilterTypeHeads)).Inc() + } + } + } + + // Collect expired pending txs subscriptions + var txsToEvict []PendingTxsSubID + ff.pendingTxsSubs.Range(func(id PendingTxsSubID, sub Sub[[]types.Transaction]) error { + txsChecked++ + if sub.ShouldEvict(timeout) { + ff.logger.Trace("[rpc] [filters] pending txs filter marked for eviction", "id", id, "protocol", sub.Protocol()) + txsToEvict = append(txsToEvict, id) + } + return nil + }) + for _, id := range txsToEvict { + if sub, ok := ff.pendingTxsSubs.Get(id); ok { + if !sub.ShouldEvict(timeout) { + continue + } + protocol := sub.Protocol() + if ff.unsubscribePendingTxsInternal(id) { + txsEvicted++ + ff.logger.Info("[rpc] [filters] evicted idle pending txs filter", "id", id, "protocol", protocol, "timeout", timeout) + ff.decrementMetrics(FilterTypePendingTxs, protocol) + getReapedCounter(string(FilterTypePendingTxs)).Inc() + } + } + } + + // Collect expired logs filter subscriptions + var logsToEvict []LogsSubID + ff.logsSubs.logsFilters.Range(func(id LogsSubID, filter *LogsFilter) error { + logsChecked++ + if filter.sender != nil && filter.sender.ShouldEvict(timeout) { + ff.logger.Trace("[rpc] [filters] logs filter marked for eviction", "id", id, "protocol", filter.sender.Protocol()) + logsToEvict = append(logsToEvict, id) + } + return nil + }) + for _, id := range logsToEvict { + if filter, ok := ff.logsSubs.logsFilters.Get(id); ok && filter.sender != nil { + if !filter.sender.ShouldEvict(timeout) { + continue + } + protocol := filter.sender.Protocol() + if ff.unsubscribeLogsInternal(id) { + logsEvicted++ + ff.logger.Info("[rpc] [filters] evicted idle logs filter", "id", id, "protocol", protocol, "timeout", timeout) + ff.decrementMetrics(FilterTypeLogs, protocol) + getReapedCounter(string(FilterTypeLogs)).Inc() + } + } + } + + // Log summary at DEBUG level + totalEvicted := headsEvicted + txsEvicted + logsEvicted + if totalEvicted > 0 { + ff.logger.Debug("[rpc] [filters] eviction cycle complete", + "headsChecked", headsChecked, "headsEvicted", headsEvicted, + "txsChecked", txsChecked, "txsEvicted", txsEvicted, + "logsChecked", logsChecked, "logsEvicted", logsEvicted) + } else { + ff.logger.Trace("[rpc] [filters] eviction cycle complete, no stale filters", + "headsChecked", headsChecked, "txsChecked", txsChecked, "logsChecked", logsChecked) + } +} + +// TouchSubscription resets the deadline for the given filter, preventing it from being evicted. +// This should be called whenever a filter is accessed (e.g., via GetFilterChanges). +// Called on every poll, so it must stay cheap — no logging on the hot path. +func (ff *Filters) TouchSubscription(id SubscriptionID, ft FilterType) { + switch ft { + case FilterTypeHeads: + if sub, ok := ff.headsSubs.Get(HeadsSubID(id)); ok { + sub.Touch() + } + case FilterTypePendingTxs: + if sub, ok := ff.pendingTxsSubs.Get(PendingTxsSubID(id)); ok { + sub.Touch() + } + case FilterTypeLogs: + if filter, ok := ff.logsSubs.logsFilters.Get(LogsSubID(id)); ok && filter.sender != nil { + filter.sender.Touch() + } + } +} + +// TrackSubscription enables timeout tracking and metrics for a subscription. +// This should be called for HTTP polling filters. For WebSocket subscriptions, call +// SetSubscriptionProtocol directly without enabling tracking as they are "evicted" once +// connections are closed. +func (ff *Filters) TrackSubscription(id SubscriptionID, ft FilterType, protocol string) { + // Only increment metrics when the subscription is actually found and tagged. + // Incrementing on a missing lookup would leak the gauge — nothing later + // decrements it because there is no subscription to unsubscribe. + switch ft { + case FilterTypeHeads: + if sub, ok := ff.headsSubs.Get(HeadsSubID(id)); ok { + sub.SetProtocol(protocol) + sub.EnableTimeout() + ff.logger.Debug("[rpc] [filters] registered heads subscription with timeout tracking", "id", id, "protocol", protocol) + ff.incrementMetrics(ft, protocol) + } + case FilterTypePendingTxs: + if sub, ok := ff.pendingTxsSubs.Get(PendingTxsSubID(id)); ok { + sub.SetProtocol(protocol) + sub.EnableTimeout() + ff.logger.Debug("[rpc] [filters] registered pending txs subscription with timeout tracking", "id", id, "protocol", protocol) + ff.incrementMetrics(ft, protocol) + } + case FilterTypeLogs: + if filter, ok := ff.logsSubs.logsFilters.Get(LogsSubID(id)); ok && filter.sender != nil { + filter.sender.SetProtocol(protocol) + filter.sender.EnableTimeout() + ff.logger.Debug("[rpc] [filters] registered logs subscription with timeout tracking", "id", id, "protocol", protocol) + ff.incrementMetrics(ft, protocol) + } + } +} + +// SetSubscriptionProtocol sets the protocol for metrics tracking without enabling timeout. +// Use this for WebSocket subscriptions that don't need eviction but need metrics. +func (ff *Filters) SetSubscriptionProtocol(id SubscriptionID, ft FilterType, protocol string) { + switch ft { + case FilterTypeHeads: + if sub, ok := ff.headsSubs.Get(HeadsSubID(id)); ok { + sub.SetProtocol(protocol) + ff.logger.Debug("[rpc] [filters] registered heads subscription (no timeout)", "id", id, "protocol", protocol) + ff.incrementMetrics(ft, protocol) + } + case FilterTypePendingTxs: + if sub, ok := ff.pendingTxsSubs.Get(PendingTxsSubID(id)); ok { + sub.SetProtocol(protocol) + ff.logger.Debug("[rpc] [filters] registered pending txs subscription (no timeout)", "id", id, "protocol", protocol) + ff.incrementMetrics(ft, protocol) + } + case FilterTypeLogs: + if filter, ok := ff.logsSubs.logsFilters.Get(LogsSubID(id)); ok && filter.sender != nil { + filter.sender.SetProtocol(protocol) + ff.logger.Debug("[rpc] [filters] registered logs subscription (no timeout)", "id", id, "protocol", protocol) + ff.incrementMetrics(ft, protocol) + } + } +} + // subscribeToPendingTransactions subscribes to pending transactions using the given transaction pool client. // It listens for new transactions and processes them as they arrive. func (ff *Filters) subscribeToPendingTransactions(ctx context.Context, txPool txpoolproto.TxpoolClient) error { @@ -412,11 +635,28 @@ func (ff *Filters) SubscribeNewHeads(size int) (<-chan *types.Header, HeadsSubID // UnsubscribeHeads unsubscribes from new block headers using the given subscription ID. // It returns true if the unsubscription was successful, otherwise false. func (ff *Filters) UnsubscribeHeads(id HeadsSubID) bool { - ch, ok := ff.headsSubs.Get(id) + sub, ok := ff.headsSubs.Get(id) if !ok { + ff.logger.Debug("[rpc] [filters] unsubscribe heads filter not found", "id", id) return false } - ch.Close() + protocol := sub.Protocol() + if !ff.unsubscribeHeadsInternal(id) { + return false + } + ff.logger.Debug("[rpc] [filters] unsubscribed heads filter", "id", id, "protocol", protocol) + ff.decrementMetrics(FilterTypeHeads, protocol) + return true +} + +// unsubscribeHeadsInternal performs the actual unsubscription without updating metrics. +// Used by eviction to avoid double-counting. +func (ff *Filters) unsubscribeHeadsInternal(id HeadsSubID) bool { + sub, ok := ff.headsSubs.Get(id) + if !ok { + return false + } + sub.Close() if _, ok = ff.headsSubs.Delete(id); !ok { return false } @@ -478,11 +718,28 @@ func (ff *Filters) SubscribePendingTxs(size int) (<-chan []types.Transaction, Pe // UnsubscribePendingTxs unsubscribes from pending transactions using the given subscription ID. // It returns true if the unsubscription was successful, otherwise false. func (ff *Filters) UnsubscribePendingTxs(id PendingTxsSubID) bool { - ch, ok := ff.pendingTxsSubs.Get(id) + sub, ok := ff.pendingTxsSubs.Get(id) if !ok { + ff.logger.Debug("[rpc] [filters] unsubscribe pending txs filter not found", "id", id) return false } - ch.Close() + protocol := sub.Protocol() + if !ff.unsubscribePendingTxsInternal(id) { + return false + } + ff.logger.Debug("[rpc] [filters] unsubscribed pending txs filter", "id", id, "protocol", protocol) + ff.decrementMetrics(FilterTypePendingTxs, protocol) + return true +} + +// unsubscribePendingTxsInternal performs the actual unsubscription without updating metrics. +// Used by eviction to avoid double-counting. +func (ff *Filters) unsubscribePendingTxsInternal(id PendingTxsSubID) bool { + sub, ok := ff.pendingTxsSubs.Get(id) + if !ok { + return false + } + sub.Close() if _, ok = ff.pendingTxsSubs.Delete(id); !ok { return false } @@ -635,7 +892,30 @@ func (ff *Filters) HasPendingTxsSubscription(id PendingTxsSubID) bool { // UnsubscribeLogs unsubscribes from logs using the given subscription ID. // It returns true if the unsubscription was successful, otherwise false. func (ff *Filters) UnsubscribeLogs(id LogsSubID) bool { + // Get protocol before unsubscribing + var protocol string + filter, ok := ff.logsSubs.logsFilters.Get(id) + if ok && filter.sender != nil { + protocol = filter.sender.Protocol() + } + if !ok { + ff.logger.Debug("[rpc] [filters] unsubscribe logs filter not found", "id", id) + } + if !ff.unsubscribeLogsInternal(id) { + return false + } + ff.logger.Debug("[rpc] [filters] unsubscribed logs filter", "id", id, "protocol", protocol) + ff.decrementMetrics(FilterTypeLogs, protocol) + return true +} + +// unsubscribeLogsInternal performs the actual unsubscription without updating metrics. +// Used by eviction to avoid double-counting. +func (ff *Filters) unsubscribeLogsInternal(id LogsSubID) bool { isDeleted := ff.logsSubs.removeLogsFilter(id) + if !isDeleted { + return false + } // if any filters in the aggregate need all addresses or all topics then the request to the central // log subscription needs to honour this lfr := ff.logsSubs.createFilterRequest() @@ -648,17 +928,17 @@ func (ff *Filters) UnsubscribeLogs(id LogsSubID) bool { for topic := range topics { lfr.Topics = append(lfr.Topics, gointerfaces.ConvertHashToH256(topic)) } - loaded := ff.loadLogsRequester() - if loaded != nil { + // The local filter has been removed; the log store and metric must be released + // regardless of whether the remote-filter update succeeds. A failed remote update + // only means the upstream will keep sending logs we'll now discard — it does not + // reanimate the local subscription. + if loaded := ff.loadLogsRequester(); loaded != nil { if err := loaded.(func(*remoteproto.LogsFilterRequest) error)(lfr); err != nil { ff.logger.Warn("Could not update remote logs filter", "err", err) - return isDeleted || ff.logsSubs.removeLogsFilter(id) } } - ff.deleteLogStore(id) - - return isDeleted + return true } // deleteLogStore deletes the log store associated with the given subscription ID. @@ -923,3 +1203,22 @@ func (ff *Filters) WithTemporalOverlay(tx kv.TemporalTx) kv.TemporalTx { } return tx } + +func (ff *Filters) incrementMetrics(ft FilterType, protocol string) { + activeSubscriptionsGauge.With(prometheus.Labels{ + filterLabelName: string(ft), + protocolLabelName: protocol, + }).Inc() + getSubscriptionCounter("created", string(ft), protocol).Inc() +} + +func (ff *Filters) decrementMetrics(ft FilterType, protocol string) { + if protocol == "" { + return // Not tracked + } + activeSubscriptionsGauge.With(prometheus.Labels{ + filterLabelName: string(ft), + protocolLabelName: protocol, + }).Dec() + getSubscriptionCounter("unsubscribed", string(ft), protocol).Inc() +} diff --git a/rpc/rpchelper/metrics.go b/rpc/rpchelper/metrics.go index a91dd46609a..523821a2ae4 100644 --- a/rpc/rpchelper/metrics.go +++ b/rpc/rpchelper/metrics.go @@ -21,15 +21,43 @@ import ( ) const ( - filterLabelName = "filter" - clientLabelName = "client" + filterLabelName = "filter" + clientLabelName = "client" + protocolLabelName = "protocol" +) + +// Protocol types for metrics labeling +const ( + ProtocolHTTP = "http" + ProtocolWS = "ws" ) var ( - activeSubscriptionsGauge = metrics.GetOrCreateGaugeVec("subscriptions", []string{filterLabelName}, "Current number of subscriptions") + // activeSubscriptionsGauge tracks current active subscriptions by filter type and protocol + activeSubscriptionsGauge = metrics.GetOrCreateGaugeVec( + "subscriptions_active", + []string{filterLabelName, protocolLabelName}, + "Current number of active subscriptions", + ) + activeSubscriptionsLogsAllAddressesGauge = metrics.GetOrCreateGauge("subscriptions_logs_all_addresses") activeSubscriptionsLogsAllTopicsGauge = metrics.GetOrCreateGauge("subscriptions_logs_all_topics") activeSubscriptionsLogsAddressesGauge = metrics.GetOrCreateGauge("subscriptions_logs_addresses") activeSubscriptionsLogsTopicsGauge = metrics.GetOrCreateGauge("subscriptions_logs_topics") activeSubscriptionsLogsClientGauge = metrics.GetOrCreateGaugeVec("subscriptions_logs_client", []string{clientLabelName}, "Current number of subscriptions by client") ) + +// getSubscriptionCounter returns a counter for subscription lifecycle events. +// pattern: subscriptions_{event}_total{{filter="{filterType}",protocol="{protocol}"}} +func getSubscriptionCounter(event, filterType, protocol string) metrics.Counter { + return metrics.GetOrCreateCounter( + `subscriptions_` + event + `_total{filter="` + filterType + `",protocol="` + protocol + `"}`, + ) +} + +// getReapedCounter returns a counter for reaped (timeout-evicted) subscriptions. +func getReapedCounter(filterType string) metrics.Counter { + return metrics.GetOrCreateCounter( + `subscriptions_reaped_total{filter="` + filterType + `"}`, + ) +} diff --git a/rpc/rpchelper/subscription.go b/rpc/rpchelper/subscription.go index 25ec40722d1..c2a4f1f5f4b 100644 --- a/rpc/rpchelper/subscription.go +++ b/rpc/rpchelper/subscription.go @@ -18,18 +18,33 @@ package rpchelper import ( "sync" + "sync/atomic" + "time" ) // a simple interface for subscriptions for rpc helper type Sub[T any] interface { Send(T) Close() + // Tracking methods for timeout-based eviction and metrics + Touch() // Reset last access time (for HTTP polling filters) + SetProtocol(protocol string) // Set protocol label for metrics + EnableTimeout() // Enable timeout tracking (HTTP only) + ShouldEvict(timeout time.Duration) bool // Check if subscription should be evicted + Protocol() string // Get protocol label for metrics } type chan_sub[T any] struct { - lock sync.Mutex // protects all fileds of this struct + lock sync.Mutex // protects ch and closed fields ch chan T closed bool + + // protocol is published once via SetProtocol and read concurrently by the eviction loop. + // atomic.Pointer keeps the write/read ordered without a lock. + protocol atomic.Pointer[string] + // lastAccess tracks last poll time for timeout eviction (atomic for concurrent Touch/ShouldEvict) + // lastAccess == 0 means no timeout tracking (WebSocket subscriptions) + lastAccess atomic.Int64 } // newChanSub - buffered channel @@ -37,9 +52,9 @@ func newChanSub[T any](size int) *chan_sub[T] { if size < 8 { // set min size to 8 size = 8 } - o := &chan_sub[T]{} - o.ch = make(chan T, size) - return o + return &chan_sub[T]{ + ch: make(chan T, size), + } } func (s *chan_sub[T]) Send(x T) { s.lock.Lock() @@ -52,6 +67,12 @@ func (s *chan_sub[T]) Send(x T) { default: // the sub is overloaded, dispose message } } + +// Close is safe to call concurrently and idempotent — the eviction loop and a +// client-initiated unsubscribe can both reach Close for the same subscription. +// The s.closed guard prevents the double channel close panic; the +// unsubscribe*Internal funcs guard the matching metrics decrement via the +// SyncMap.Delete result so the gauge is decremented at most once. func (s *chan_sub[T]) Close() { s.lock.Lock() defer s.lock.Unlock() @@ -61,3 +82,41 @@ func (s *chan_sub[T]) Close() { s.closed = true close(s.ch) } + +// SetProtocol sets the protocol label for metrics tracking. +// Safe to call concurrently with Protocol(); intended to be called once at subscription setup. +func (s *chan_sub[T]) SetProtocol(protocol string) { + s.protocol.Store(&protocol) +} + +// EnableTimeout enables timeout tracking for this subscription (HTTP polling filters). +// For WebSocket subscriptions, don't call this - they will never be evicted. +func (s *chan_sub[T]) EnableTimeout() { + s.lastAccess.Store(time.Now().UnixNano()) +} + +// Touch resets the last access time, preventing timeout eviction. +// Only effective if EnableTimeout was called. +func (s *chan_sub[T]) Touch() { + if s.lastAccess.Load() != 0 { + s.lastAccess.Store(time.Now().UnixNano()) + } +} + +// ShouldEvict returns true if the subscription has not been accessed within the timeout. +// Returns false if tracking is not enabled (lastAccess == 0). +func (s *chan_sub[T]) ShouldEvict(timeout time.Duration) bool { + last := s.lastAccess.Load() + if last == 0 { + return false // No tracking, never evict (WebSocket) + } + return time.Since(time.Unix(0, last)) > timeout +} + +// Protocol returns the protocol label for metrics. Returns "" if SetProtocol was never called. +func (s *chan_sub[T]) Protocol() string { + if p := s.protocol.Load(); p != nil { + return *p + } + return "" +} From 92c3247ca4ed17bb3ec48f0fe6cbb5b0f35f678d Mon Sep 17 00:00:00 2001 From: yperbasis Date: Mon, 6 Jul 2026 11:34:21 +0200 Subject: [PATCH 02/11] rpc/jsonrpc: return all buffered pending-tx batches from eth_getFilterChanges ReadPendingTxs consumes every batch accumulated since the last poll, but GetFilterChanges only reported txs[0], silently dropping the hashes of any later batches. --- rpc/jsonrpc/eth_filters.go | 8 +++++--- rpc/jsonrpc/eth_filters_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/rpc/jsonrpc/eth_filters.go b/rpc/jsonrpc/eth_filters.go index 4c8641997f1..0fa74aa4971 100644 --- a/rpc/jsonrpc/eth_filters.go +++ b/rpc/jsonrpc/eth_filters.go @@ -119,9 +119,11 @@ func (api *APIImpl) GetFilterChanges(_ context.Context, index string) ([]any, er return stub, nil case api.filters.HasPendingTxsSubscription(rpchelper.PendingTxsSubID(cutIndex)): api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypePendingTxs) - if txs, ok := api.filters.ReadPendingTxs(rpchelper.PendingTxsSubID(cutIndex)); ok && len(txs) > 0 { - for _, txn := range txs[0] { - stub = append(stub, txn.Hash()) + if txs, ok := api.filters.ReadPendingTxs(rpchelper.PendingTxsSubID(cutIndex)); ok { + for _, batch := range txs { + for _, txn := range batch { + stub = append(stub, txn.Hash()) + } } } return stub, nil diff --git a/rpc/jsonrpc/eth_filters_test.go b/rpc/jsonrpc/eth_filters_test.go index 950b14c6376..0a80e8ee872 100644 --- a/rpc/jsonrpc/eth_filters_test.go +++ b/rpc/jsonrpc/eth_filters_test.go @@ -18,6 +18,7 @@ package jsonrpc import ( "math/rand" + "strings" "sync" "testing" "time" @@ -243,6 +244,38 @@ func TestNewPendingTransactionIncludesFrom(t *testing.T) { require.Equal(t, m.Address, rpcTx.From) } +func TestPendingTxsFilterChangesReturnsAllBatches(t *testing.T) { + m := execmoduletester.New(t) + ctx, conn := rpcdaemontest.CreateTestGrpcConn(t, m) + mining := txpoolproto.NewMiningClient(conn) + ff := rpchelper.New(ctx, rpchelper.DefaultFiltersConfig, nil, nil, mining, func() {}, m.Log, nil) + stateCache := kvcache.New(kvcache.DefaultCoherentConfig) + api := newEthApiForTest(newBaseApiWithFiltersForTest(ff, stateCache, m), m.DB, nil, nil) + + ptf, err := api.NewPendingTransactionFilter(ctx) + require.NoError(t, err) + id := rpchelper.PendingTxsSubID(strings.TrimPrefix(ptf, "0x")) + + signer := types.LatestSignerForChainID(m.ChainConfig.ChainID) + makeTx := func(nonce uint64) types.Transaction { + tx, err := types.SignTx( + types.NewTransaction(nonce, m.Address, uint256.NewInt(1), params.TxGas, uint256.NewInt(1), nil), + *signer, + m.Key, + ) + require.NoError(t, err) + return tx + } + tx0, tx1, tx2 := makeTx(0), makeTx(1), makeTx(2) + + ff.AddPendingTxs(id, []types.Transaction{tx0, tx1}) + ff.AddPendingTxs(id, []types.Transaction{tx2}) + + changes, err := api.GetFilterChanges(ctx, ptf) + require.NoError(t, err) + require.Equal(t, []any{tx0.Hash(), tx1.Hash(), tx2.Hash()}, changes) +} + func TestGetFilterChangesReturnsFilterNotFoundForUnknownID(t *testing.T) { if testing.Short() { t.Skip("slow test") From a05032f1ff283d3af55d1c1205542a2a9b13f642 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Mon, 6 Jul 2026 11:38:50 +0200 Subject: [PATCH 03/11] rpc/rpchelper: close idle subscriptions atomically with the staleness check Replace ShouldEvict + re-check with CloseIfIdle, which decides staleness and closes under the same lock Touch uses, so a subscription polled within the timeout can never be evicted; this removes the TOCTOU race the old two-phase eviction acknowledged as unavoidable. lastAccess and protocol move from atomics to the existing chan_sub mutex. Also compute the eviction check interval once in New so the startup log matches the actual ticker (it previously logged timeout/2 while the loop floored to 1s), fix the evictStaleSubscriptions doc-comment name, and add eviction/touch coverage tests. Addresses Copilot review comments from #18591. --- rpc/rpchelper/filters.go | 107 +++++++++---------- rpc/rpchelper/filters_eviction_test.go | 138 +++++++++++++++++++++++++ rpc/rpchelper/subscription.go | 70 +++++++------ 3 files changed, 222 insertions(+), 93 deletions(-) create mode 100644 rpc/rpchelper/filters_eviction_test.go diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index 7a8b461d3b6..e9424791bd9 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -115,10 +115,17 @@ func New(ctx context.Context, config FiltersConfig, ethBackend ApiBackend, txPoo // Start the timeout loop for filter eviction if timeout is configured if config.RpcSubscriptionFiltersTimeout > 0 { + // Check more frequently than the timeout to ensure timely eviction, with a + // 1s floor: time.NewTicker panics on non-positive intervals and a + // sub-second sweep would burn CPU for no practical benefit. + checkInterval := config.RpcSubscriptionFiltersTimeout / 2 + if checkInterval < time.Second { + checkInterval = time.Second + } logger.Info("[rpc] [filters] starting timeout loop for idle filter eviction", "timeout", config.RpcSubscriptionFiltersTimeout, - "checkInterval", config.RpcSubscriptionFiltersTimeout/2) - go ff.timeoutLoop(ctx, config.RpcSubscriptionFiltersTimeout) + "checkInterval", checkInterval) + go ff.timeoutLoop(ctx, config.RpcSubscriptionFiltersTimeout, checkInterval) } else { logger.Info("[rpc] [filters] timeout-based filter eviction disabled") } @@ -307,20 +314,10 @@ func (ff *Filters) LastPendingBlock() *types.Block { // timeoutLoop runs periodically and evicts subscriptions that have not been polled within the timeout duration. // This prevents unbounded accumulation of idle subscriptions and matches geth's behavior. -func (ff *Filters) timeoutLoop(ctx context.Context, timeout time.Duration) { - // Check more frequently than the timeout to ensure timely eviction. - // Floor at 1s so a sub-2s timeout doesn't produce a zero or sub-second - // interval (time.NewTicker panics on non-positive durations and a tight - // loop would burn CPU for no practical benefit). - checkInterval := timeout / 2 - if checkInterval < time.Second { - checkInterval = time.Second - } +func (ff *Filters) timeoutLoop(ctx context.Context, timeout, checkInterval time.Duration) { ticker := time.NewTicker(checkInterval) defer ticker.Stop() - ff.logger.Debug("[rpc] [filters] timeout loop started", "timeout", timeout, "checkInterval", checkInterval) - for { select { case <-ctx.Done(): @@ -333,88 +330,78 @@ func (ff *Filters) timeoutLoop(ctx context.Context, timeout time.Duration) { } } -// evictStaleFilters removes filters that haven't been accessed within the timeout duration. +// evictStaleSubscriptions removes filters that haven't been accessed within the timeout duration. // It iterates the existing subscription maps and checks each subscription's embedded tracking. +// CloseIfIdle decides staleness and closes in one atomic step, so a concurrent Touch either +// lands before it (the subscription survives) or finds the subscription already closed; +// the map delete and metrics follow after the Range to avoid mutating during iteration. func (ff *Filters) evictStaleSubscriptions(timeout time.Duration) { var headsChecked, txsChecked, logsChecked int var headsEvicted, txsEvicted, logsEvicted int - // Collect expired heads subscriptions var headsToEvict []HeadsSubID ff.headsSubs.Range(func(id HeadsSubID, sub Sub[*types.Header]) error { headsChecked++ - if sub.ShouldEvict(timeout) { - ff.logger.Trace("[rpc] [filters] heads filter marked for eviction", "id", id, "protocol", sub.Protocol()) + if sub.CloseIfIdle(timeout) { headsToEvict = append(headsToEvict, id) } return nil }) for _, id := range headsToEvict { - if sub, ok := ff.headsSubs.Get(id); ok { - // Re-check ShouldEvict: a client may have called TouchSubscription between phase 1 and phase 2. - // This shrinks the TOCTOU window but does not eliminate it; an unavoidable race remains - // between this check and unsubscribeHeadsInternal, but the timeout is on the order of minutes - // so the practical impact is bounded. - if !sub.ShouldEvict(timeout) { - continue - } - protocol := sub.Protocol() - if ff.unsubscribeHeadsInternal(id) { - headsEvicted++ - ff.logger.Info("[rpc] [filters] evicted idle heads filter", "id", id, "protocol", protocol, "timeout", timeout) - ff.decrementMetrics(FilterTypeHeads, protocol) - getReapedCounter(string(FilterTypeHeads)).Inc() - } + sub, ok := ff.headsSubs.Get(id) + if !ok { + continue + } + protocol := sub.Protocol() + if ff.unsubscribeHeadsInternal(id) { + headsEvicted++ + ff.logger.Info("[rpc] [filters] evicted idle heads filter", "id", id, "protocol", protocol, "timeout", timeout) + ff.decrementMetrics(FilterTypeHeads, protocol) + getReapedCounter(string(FilterTypeHeads)).Inc() } } - // Collect expired pending txs subscriptions var txsToEvict []PendingTxsSubID ff.pendingTxsSubs.Range(func(id PendingTxsSubID, sub Sub[[]types.Transaction]) error { txsChecked++ - if sub.ShouldEvict(timeout) { - ff.logger.Trace("[rpc] [filters] pending txs filter marked for eviction", "id", id, "protocol", sub.Protocol()) + if sub.CloseIfIdle(timeout) { txsToEvict = append(txsToEvict, id) } return nil }) for _, id := range txsToEvict { - if sub, ok := ff.pendingTxsSubs.Get(id); ok { - if !sub.ShouldEvict(timeout) { - continue - } - protocol := sub.Protocol() - if ff.unsubscribePendingTxsInternal(id) { - txsEvicted++ - ff.logger.Info("[rpc] [filters] evicted idle pending txs filter", "id", id, "protocol", protocol, "timeout", timeout) - ff.decrementMetrics(FilterTypePendingTxs, protocol) - getReapedCounter(string(FilterTypePendingTxs)).Inc() - } + sub, ok := ff.pendingTxsSubs.Get(id) + if !ok { + continue + } + protocol := sub.Protocol() + if ff.unsubscribePendingTxsInternal(id) { + txsEvicted++ + ff.logger.Info("[rpc] [filters] evicted idle pending txs filter", "id", id, "protocol", protocol, "timeout", timeout) + ff.decrementMetrics(FilterTypePendingTxs, protocol) + getReapedCounter(string(FilterTypePendingTxs)).Inc() } } - // Collect expired logs filter subscriptions var logsToEvict []LogsSubID ff.logsSubs.logsFilters.Range(func(id LogsSubID, filter *LogsFilter) error { logsChecked++ - if filter.sender != nil && filter.sender.ShouldEvict(timeout) { - ff.logger.Trace("[rpc] [filters] logs filter marked for eviction", "id", id, "protocol", filter.sender.Protocol()) + if filter.sender != nil && filter.sender.CloseIfIdle(timeout) { logsToEvict = append(logsToEvict, id) } return nil }) for _, id := range logsToEvict { - if filter, ok := ff.logsSubs.logsFilters.Get(id); ok && filter.sender != nil { - if !filter.sender.ShouldEvict(timeout) { - continue - } - protocol := filter.sender.Protocol() - if ff.unsubscribeLogsInternal(id) { - logsEvicted++ - ff.logger.Info("[rpc] [filters] evicted idle logs filter", "id", id, "protocol", protocol, "timeout", timeout) - ff.decrementMetrics(FilterTypeLogs, protocol) - getReapedCounter(string(FilterTypeLogs)).Inc() - } + filter, ok := ff.logsSubs.logsFilters.Get(id) + if !ok || filter.sender == nil { + continue + } + protocol := filter.sender.Protocol() + if ff.unsubscribeLogsInternal(id) { + logsEvicted++ + ff.logger.Info("[rpc] [filters] evicted idle logs filter", "id", id, "protocol", protocol, "timeout", timeout) + ff.decrementMetrics(FilterTypeLogs, protocol) + getReapedCounter(string(FilterTypeLogs)).Inc() } } diff --git a/rpc/rpchelper/filters_eviction_test.go b/rpc/rpchelper/filters_eviction_test.go new file mode 100644 index 00000000000..a2c1cf15e2d --- /dev/null +++ b/rpc/rpchelper/filters_eviction_test.go @@ -0,0 +1,138 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package rpchelper + +import ( + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common/log/v3" + "github.com/erigontech/erigon/rpc/filters" +) + +func newTestFilters(t *testing.T) *Filters { + return New(t.Context(), DefaultFiltersConfig, nil, nil, nil, func() {}, log.New(), nil) +} + +func backdateSub[T any](t *testing.T, sub Sub[T], age time.Duration) { + t.Helper() + cs, ok := sub.(*chan_sub[T]) + require.True(t, ok) + cs.lock.Lock() + cs.lastAccess = time.Now().Add(-age) + cs.lock.Unlock() +} + +func TestEvictStaleSubscriptionsRemovesIdleFilters(t *testing.T) { + f := newTestFilters(t) + + headsCh, headsID := f.SubscribeNewHeads(8) + f.TrackSubscription(SubscriptionID(headsID), FilterTypeHeads, ProtocolHTTP) + txsCh, txsID := f.SubscribePendingTxs(8) + f.TrackSubscription(SubscriptionID(txsID), FilterTypePendingTxs, ProtocolHTTP) + logsCh, logsID := f.SubscribeLogs(8, filters.FilterCriteria{}) + f.TrackSubscription(SubscriptionID(logsID), FilterTypeLogs, ProtocolHTTP) + + headsSub, ok := f.headsSubs.Get(headsID) + require.True(t, ok) + backdateSub(t, headsSub, 2*time.Hour) + txsSub, ok := f.pendingTxsSubs.Get(txsID) + require.True(t, ok) + backdateSub(t, txsSub, 2*time.Hour) + logsFilter, ok := f.logsSubs.logsFilters.Get(logsID) + require.True(t, ok) + backdateSub(t, logsFilter.sender, 2*time.Hour) + + f.evictStaleSubscriptions(time.Hour) + + require.False(t, f.HasHeadsSubscription(headsID)) + require.False(t, f.HasPendingTxsSubscription(txsID)) + require.False(t, f.HasSubscription(logsID)) + + _, open := <-headsCh + require.False(t, open) + _, open = <-txsCh + require.False(t, open) + _, open = <-logsCh + require.False(t, open) +} + +func TestTouchSubscriptionPreventsEviction(t *testing.T) { + f := newTestFilters(t) + + _, touchedID := f.SubscribePendingTxs(8) + f.TrackSubscription(SubscriptionID(touchedID), FilterTypePendingTxs, ProtocolHTTP) + _, idleID := f.SubscribePendingTxs(8) + f.TrackSubscription(SubscriptionID(idleID), FilterTypePendingTxs, ProtocolHTTP) + + for _, id := range []PendingTxsSubID{touchedID, idleID} { + sub, ok := f.pendingTxsSubs.Get(id) + require.True(t, ok) + backdateSub(t, sub, 2*time.Hour) + } + + f.TouchSubscription(SubscriptionID(touchedID), FilterTypePendingTxs) + f.evictStaleSubscriptions(time.Hour) + + require.True(t, f.HasPendingTxsSubscription(touchedID)) + require.False(t, f.HasPendingTxsSubscription(idleID)) +} + +func TestEvictStaleSubscriptionsSkipsWebSocketSubscriptions(t *testing.T) { + f := newTestFilters(t) + + _, id := f.SubscribeNewHeads(8) + f.SetSubscriptionProtocol(SubscriptionID(id), FilterTypeHeads, ProtocolWS) + + f.evictStaleSubscriptions(time.Nanosecond) + + require.True(t, f.HasHeadsSubscription(id)) +} + +func TestConcurrentTouchAndEviction(t *testing.T) { + f := newTestFilters(t) + + _, id := f.SubscribePendingTxs(8) + f.TrackSubscription(SubscriptionID(id), FilterTypePendingTxs, ProtocolHTTP) + + stop := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for { + select { + case <-stop: + return + default: + f.TouchSubscription(SubscriptionID(id), FilterTypePendingTxs) + } + } + }() + + deadline := time.Now().Add(200 * time.Millisecond) + for time.Now().Before(deadline) { + f.evictStaleSubscriptions(50 * time.Millisecond) + } + close(stop) + wg.Wait() + + require.True(t, f.HasPendingTxsSubscription(id)) +} diff --git a/rpc/rpchelper/subscription.go b/rpc/rpchelper/subscription.go index c2a4f1f5f4b..a609febee44 100644 --- a/rpc/rpchelper/subscription.go +++ b/rpc/rpchelper/subscription.go @@ -18,7 +18,6 @@ package rpchelper import ( "sync" - "sync/atomic" "time" ) @@ -30,21 +29,18 @@ type Sub[T any] interface { Touch() // Reset last access time (for HTTP polling filters) SetProtocol(protocol string) // Set protocol label for metrics EnableTimeout() // Enable timeout tracking (HTTP only) - ShouldEvict(timeout time.Duration) bool // Check if subscription should be evicted + CloseIfIdle(timeout time.Duration) bool // Close and report true when idle longer than timeout Protocol() string // Get protocol label for metrics } type chan_sub[T any] struct { - lock sync.Mutex // protects ch and closed fields - ch chan T - closed bool - - // protocol is published once via SetProtocol and read concurrently by the eviction loop. - // atomic.Pointer keeps the write/read ordered without a lock. - protocol atomic.Pointer[string] - // lastAccess tracks last poll time for timeout eviction (atomic for concurrent Touch/ShouldEvict) - // lastAccess == 0 means no timeout tracking (WebSocket subscriptions) - lastAccess atomic.Int64 + lock sync.Mutex // protects all fields of this struct + ch chan T + closed bool + protocol string + // lastAccess is the last poll time for timeout eviction; zero means no + // timeout tracking (push subscriptions die with their connection instead). + lastAccess time.Time } // newChanSub - buffered channel @@ -68,14 +64,14 @@ func (s *chan_sub[T]) Send(x T) { } } -// Close is safe to call concurrently and idempotent — the eviction loop and a -// client-initiated unsubscribe can both reach Close for the same subscription. -// The s.closed guard prevents the double channel close panic; the -// unsubscribe*Internal funcs guard the matching metrics decrement via the -// SyncMap.Delete result so the gauge is decremented at most once. +// Close is idempotent and safe to call concurrently with Send and CloseIfIdle. func (s *chan_sub[T]) Close() { s.lock.Lock() defer s.lock.Unlock() + s.closeLocked() +} + +func (s *chan_sub[T]) closeLocked() { if s.closed { return } @@ -84,39 +80,47 @@ func (s *chan_sub[T]) Close() { } // SetProtocol sets the protocol label for metrics tracking. -// Safe to call concurrently with Protocol(); intended to be called once at subscription setup. func (s *chan_sub[T]) SetProtocol(protocol string) { - s.protocol.Store(&protocol) + s.lock.Lock() + defer s.lock.Unlock() + s.protocol = protocol } // EnableTimeout enables timeout tracking for this subscription (HTTP polling filters). // For WebSocket subscriptions, don't call this - they will never be evicted. func (s *chan_sub[T]) EnableTimeout() { - s.lastAccess.Store(time.Now().UnixNano()) + s.lock.Lock() + defer s.lock.Unlock() + s.lastAccess = time.Now() } // Touch resets the last access time, preventing timeout eviction. // Only effective if EnableTimeout was called. func (s *chan_sub[T]) Touch() { - if s.lastAccess.Load() != 0 { - s.lastAccess.Store(time.Now().UnixNano()) + s.lock.Lock() + defer s.lock.Unlock() + if !s.lastAccess.IsZero() && !s.closed { + s.lastAccess = time.Now() } } -// ShouldEvict returns true if the subscription has not been accessed within the timeout. -// Returns false if tracking is not enabled (lastAccess == 0). -func (s *chan_sub[T]) ShouldEvict(timeout time.Duration) bool { - last := s.lastAccess.Load() - if last == 0 { - return false // No tracking, never evict (WebSocket) +// CloseIfIdle closes the subscription and reports true when timeout tracking is +// enabled and the last access is older than timeout. Deciding and closing under +// the same lock Touch uses guarantees a subscription touched within the timeout +// is never evicted. +func (s *chan_sub[T]) CloseIfIdle(timeout time.Duration) bool { + s.lock.Lock() + defer s.lock.Unlock() + if s.closed || s.lastAccess.IsZero() || time.Since(s.lastAccess) <= timeout { + return false } - return time.Since(time.Unix(0, last)) > timeout + s.closeLocked() + return true } // Protocol returns the protocol label for metrics. Returns "" if SetProtocol was never called. func (s *chan_sub[T]) Protocol() string { - if p := s.protocol.Load(); p != nil { - return *p - } - return "" + s.lock.Lock() + defer s.lock.Unlock() + return s.protocol } From 051e47c5859bfe537ed1329c471d9b181080b7a5 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Mon, 6 Jul 2026 11:59:15 +0200 Subject: [PATCH 04/11] rpc/rpchelper: batch the remote logs-filter update during eviction Each evicted logs filter rebuilt the aggregate request and issued its own remote update; one update at the end of the eviction cycle covers them all. Client-driven UnsubscribeLogs keeps its per-call update. --- rpc/rpchelper/filters.go | 42 +++++++++++++++----------- rpc/rpchelper/filters_eviction_test.go | 30 ++++++++++++++++++ 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index e9424791bd9..a17dfbb13e5 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -397,12 +397,18 @@ func (ff *Filters) evictStaleSubscriptions(timeout time.Duration) { continue } protocol := filter.sender.Protocol() - if ff.unsubscribeLogsInternal(id) { - logsEvicted++ - ff.logger.Info("[rpc] [filters] evicted idle logs filter", "id", id, "protocol", protocol, "timeout", timeout) - ff.decrementMetrics(FilterTypeLogs, protocol) - getReapedCounter(string(FilterTypeLogs)).Inc() + if !ff.logsSubs.removeLogsFilter(id) { + continue } + ff.deleteLogStore(id) + logsEvicted++ + ff.logger.Info("[rpc] [filters] evicted idle logs filter", "id", id, "protocol", protocol, "timeout", timeout) + ff.decrementMetrics(FilterTypeLogs, protocol) + getReapedCounter(string(FilterTypeLogs)).Inc() + } + // One remote update covers every logs filter removed this cycle. + if logsEvicted > 0 { + ff.updateRemoteLogsFilter() } // Log summary at DEBUG level @@ -899,33 +905,35 @@ func (ff *Filters) UnsubscribeLogs(id LogsSubID) bool { // unsubscribeLogsInternal performs the actual unsubscription without updating metrics. // Used by eviction to avoid double-counting. func (ff *Filters) unsubscribeLogsInternal(id LogsSubID) bool { - isDeleted := ff.logsSubs.removeLogsFilter(id) - if !isDeleted { + if !ff.logsSubs.removeLogsFilter(id) { return false } - // if any filters in the aggregate need all addresses or all topics then the request to the central - // log subscription needs to honour this - lfr := ff.logsSubs.createFilterRequest() + // The local filter has been removed; the log store and metric must be released + // regardless of whether the remote-filter update succeeds. A failed remote update + // only means the upstream will keep sending logs we'll now discard — it does not + // reanimate the local subscription. + ff.updateRemoteLogsFilter() + ff.deleteLogStore(id) + return true +} +// updateRemoteLogsFilter pushes the aggregated filter state to the remote log source. +// If any filters in the aggregate need all addresses or all topics then the request to +// the central log subscription needs to honour this. +func (ff *Filters) updateRemoteLogsFilter() { + lfr := ff.logsSubs.createFilterRequest() addresses, topics := ff.logsSubs.getAggMaps() - for addr := range addresses { lfr.Addresses = append(lfr.Addresses, gointerfaces.ConvertAddressToH160(addr)) } for topic := range topics { lfr.Topics = append(lfr.Topics, gointerfaces.ConvertHashToH256(topic)) } - // The local filter has been removed; the log store and metric must be released - // regardless of whether the remote-filter update succeeds. A failed remote update - // only means the upstream will keep sending logs we'll now discard — it does not - // reanimate the local subscription. if loaded := ff.loadLogsRequester(); loaded != nil { if err := loaded.(func(*remoteproto.LogsFilterRequest) error)(lfr); err != nil { ff.logger.Warn("Could not update remote logs filter", "err", err) } } - ff.deleteLogStore(id) - return true } // deleteLogStore deletes the log store associated with the given subscription ID. diff --git a/rpc/rpchelper/filters_eviction_test.go b/rpc/rpchelper/filters_eviction_test.go index a2c1cf15e2d..e5107a1f405 100644 --- a/rpc/rpchelper/filters_eviction_test.go +++ b/rpc/rpchelper/filters_eviction_test.go @@ -18,12 +18,14 @@ package rpchelper import ( "sync" + "sync/atomic" "testing" "time" "github.com/stretchr/testify/require" "github.com/erigontech/erigon/common/log/v3" + "github.com/erigontech/erigon/node/gointerfaces/remoteproto" "github.com/erigontech/erigon/rpc/filters" ) @@ -106,6 +108,34 @@ func TestEvictStaleSubscriptionsSkipsWebSocketSubscriptions(t *testing.T) { require.True(t, f.HasHeadsSubscription(id)) } +func TestLogsEvictionBatchesRemoteFilterUpdate(t *testing.T) { + f := newTestFilters(t) + + var updates atomic.Int32 + f.logsRequestor.Store(func(*remoteproto.LogsFilterRequest) error { + updates.Add(1) + return nil + }) + + ids := make([]LogsSubID, 0, 3) + for range 3 { + _, id := f.SubscribeLogs(8, filters.FilterCriteria{}) + f.TrackSubscription(SubscriptionID(id), FilterTypeLogs, ProtocolHTTP) + lf, ok := f.logsSubs.logsFilters.Get(id) + require.True(t, ok) + backdateSub(t, lf.sender, 2*time.Hour) + ids = append(ids, id) + } + + updates.Store(0) // SubscribeLogs issues its own updates; count only eviction's + f.evictStaleSubscriptions(time.Hour) + + for _, id := range ids { + require.False(t, f.HasSubscription(id)) + } + require.EqualValues(t, 1, updates.Load()) +} + func TestConcurrentTouchAndEviction(t *testing.T) { f := newTestFilters(t) From 823d11addca2877b80c4403abbb736ca83dd072b Mon Sep 17 00:00:00 2001 From: yperbasis Date: Mon, 6 Jul 2026 12:31:26 +0200 Subject: [PATCH 05/11] rpc/rpchelper, rpc/jsonrpc: harden filter stores, dedup tracking and eviction plumbing Drop late writes to the per-filter stores: the forwarding goroutine can drain channel-buffered items after unsubscribe/eviction has deleted the store, and the recreated entry is unreachable (reads are gated on the subscription existing), so it leaked forever. The Add* functions now check subscription existence under the store lock and delete instead of storing when it is gone. Deduplicate the tracking/eviction plumbing: one trackedSub resolver (via a new SubTracker interface) behind Touch/Track/SetSubscriptionProtocol, one generic evictIdleSubs for heads and pending txs, and one generic unsubscribeSubInternal for the channel-sub teardown. Make each eth_getFilterChanges poll cost a single subscription-map lookup: TouchSubscription reports existence, so the type dispatch probes it directly instead of Has* + Touch + re-Get. Metrics helpers switch to WithLabelValues, avoiding a per-call label-map allocation. --- rpc/jsonrpc/eth_filters.go | 20 +- rpc/rpchelper/filters.go | 260 +++++++++++-------------- rpc/rpchelper/filters_eviction_test.go | 30 +++ rpc/rpchelper/filters_test.go | 6 +- rpc/rpchelper/subscription.go | 15 +- 5 files changed, 165 insertions(+), 166 deletions(-) diff --git a/rpc/jsonrpc/eth_filters.go b/rpc/jsonrpc/eth_filters.go index 0fa74aa4971..450cffb8489 100644 --- a/rpc/jsonrpc/eth_filters.go +++ b/rpc/jsonrpc/eth_filters.go @@ -104,21 +104,18 @@ func (api *APIImpl) GetFilterChanges(_ context.Context, index string) ([]any, er stub := make([]any, 0) // remove 0x cutIndex := strings.TrimPrefix(index, "0x") - // Identify which subscription type the id belongs to, touch it to reset the - // eviction deadline, then probe the store. Touch must happen on every poll - // — not only when there is buffered data — otherwise an actively-polling - // client whose chain is quiet would be evicted. + // The Touch probe identifies the id's type and resets the eviction deadline on + // every poll — even when no data is buffered — so an actively-polling client on + // a quiet chain is never evicted. switch { - case api.filters.HasHeadsSubscription(rpchelper.HeadsSubID(cutIndex)): - api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeHeads) + case api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeHeads): if blocks, ok := api.filters.ReadPendingBlocks(rpchelper.HeadsSubID(cutIndex)); ok { for _, v := range blocks { stub = append(stub, v.Hash()) } } return stub, nil - case api.filters.HasPendingTxsSubscription(rpchelper.PendingTxsSubID(cutIndex)): - api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypePendingTxs) + case api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypePendingTxs): if txs, ok := api.filters.ReadPendingTxs(rpchelper.PendingTxsSubID(cutIndex)); ok { for _, batch := range txs { for _, txn := range batch { @@ -127,8 +124,7 @@ func (api *APIImpl) GetFilterChanges(_ context.Context, index string) ([]any, er } } return stub, nil - case api.filters.HasSubscription(rpchelper.LogsSubID(cutIndex)): - api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeLogs) + case api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeLogs): if logs, ok := api.filters.ReadLogs(rpchelper.LogsSubID(cutIndex)); ok { for _, v := range logs { stub = append(stub, v) @@ -148,11 +144,9 @@ func (api *APIImpl) GetFilterLogs(_ context.Context, index string) ([]*types.Log return nil, rpc.ErrNotificationsUnsupported } cutIndex := strings.TrimPrefix(index, "0x") - if found := api.filters.HasSubscription(rpchelper.LogsSubID(cutIndex)); !found { + if !api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeLogs) { return nil, rpc.ErrFilterNotFound } - // Reset the filter deadline since it was just accessed - api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeLogs) if logs, ok := api.filters.ReadLogs(rpchelper.LogsSubID(cutIndex)); ok { return logs, nil } diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index a17dfbb13e5..904f9747da5 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -331,67 +331,66 @@ func (ff *Filters) timeoutLoop(ctx context.Context, timeout, checkInterval time. } // evictStaleSubscriptions removes filters that haven't been accessed within the timeout duration. -// It iterates the existing subscription maps and checks each subscription's embedded tracking. -// CloseIfIdle decides staleness and closes in one atomic step, so a concurrent Touch either -// lands before it (the subscription survives) or finds the subscription already closed; -// the map delete and metrics follow after the Range to avoid mutating during iteration. func (ff *Filters) evictStaleSubscriptions(timeout time.Duration) { - var headsChecked, txsChecked, logsChecked int - var headsEvicted, txsEvicted, logsEvicted int + headsChecked, headsEvicted := evictIdleSubs(ff, ff.headsSubs, FilterTypeHeads, timeout, ff.unsubscribeHeadsInternal) + txsChecked, txsEvicted := evictIdleSubs(ff, ff.pendingTxsSubs, FilterTypePendingTxs, timeout, ff.unsubscribePendingTxsInternal) + logsChecked, logsEvicted := ff.evictIdleLogsSubs(timeout) - var headsToEvict []HeadsSubID - ff.headsSubs.Range(func(id HeadsSubID, sub Sub[*types.Header]) error { - headsChecked++ - if sub.CloseIfIdle(timeout) { - headsToEvict = append(headsToEvict, id) - } - return nil - }) - for _, id := range headsToEvict { - sub, ok := ff.headsSubs.Get(id) - if !ok { - continue - } - protocol := sub.Protocol() - if ff.unsubscribeHeadsInternal(id) { - headsEvicted++ - ff.logger.Info("[rpc] [filters] evicted idle heads filter", "id", id, "protocol", protocol, "timeout", timeout) - ff.decrementMetrics(FilterTypeHeads, protocol) - getReapedCounter(string(FilterTypeHeads)).Inc() - } + // Log summary at DEBUG level + totalEvicted := headsEvicted + txsEvicted + logsEvicted + if totalEvicted > 0 { + ff.logger.Debug("[rpc] [filters] eviction cycle complete", + "headsChecked", headsChecked, "headsEvicted", headsEvicted, + "txsChecked", txsChecked, "txsEvicted", txsEvicted, + "logsChecked", logsChecked, "logsEvicted", logsEvicted) + } else { + ff.logger.Trace("[rpc] [filters] eviction cycle complete, no stale filters", + "headsChecked", headsChecked, "txsChecked", txsChecked, "logsChecked", logsChecked) } +} - var txsToEvict []PendingTxsSubID - ff.pendingTxsSubs.Range(func(id PendingTxsSubID, sub Sub[[]types.Transaction]) error { - txsChecked++ +// evictIdleSubs closes and removes the subscriptions in subs that have been idle +// longer than timeout. CloseIfIdle decides staleness and closes in one atomic step, +// so a concurrent Touch either lands before it (the subscription survives) or finds +// the subscription already closed; the map delete and metrics follow after the +// Range to avoid mutating the map during iteration. +func evictIdleSubs[K comparable, T any](ff *Filters, subs *concurrent.SyncMap[K, Sub[T]], ft FilterType, timeout time.Duration, unsubscribe func(K) bool) (checked, evicted int) { + var toEvict []K + subs.Range(func(id K, sub Sub[T]) error { + checked++ if sub.CloseIfIdle(timeout) { - txsToEvict = append(txsToEvict, id) + toEvict = append(toEvict, id) } return nil }) - for _, id := range txsToEvict { - sub, ok := ff.pendingTxsSubs.Get(id) + for _, id := range toEvict { + sub, ok := subs.Get(id) if !ok { continue } protocol := sub.Protocol() - if ff.unsubscribePendingTxsInternal(id) { - txsEvicted++ - ff.logger.Info("[rpc] [filters] evicted idle pending txs filter", "id", id, "protocol", protocol, "timeout", timeout) - ff.decrementMetrics(FilterTypePendingTxs, protocol) - getReapedCounter(string(FilterTypePendingTxs)).Inc() + if unsubscribe(id) { + evicted++ + ff.logger.Info("[rpc] [filters] evicted idle filter", "type", ft, "id", id, "protocol", protocol, "timeout", timeout) + ff.decrementMetrics(ft, protocol) + getReapedCounter(string(ft)).Inc() } } + return checked, evicted +} - var logsToEvict []LogsSubID +// evictIdleLogsSubs mirrors evictIdleSubs for logs filters, which live behind the +// aggregator and share one remote-filter update per eviction cycle. +func (ff *Filters) evictIdleLogsSubs(timeout time.Duration) (checked, evicted int) { + var toEvict []LogsSubID ff.logsSubs.logsFilters.Range(func(id LogsSubID, filter *LogsFilter) error { - logsChecked++ + checked++ if filter.sender != nil && filter.sender.CloseIfIdle(timeout) { - logsToEvict = append(logsToEvict, id) + toEvict = append(toEvict, id) } return nil }) - for _, id := range logsToEvict { + for _, id := range toEvict { filter, ok := ff.logsSubs.logsFilters.Get(id) if !ok || filter.sender == nil { continue @@ -401,105 +400,72 @@ func (ff *Filters) evictStaleSubscriptions(timeout time.Duration) { continue } ff.deleteLogStore(id) - logsEvicted++ - ff.logger.Info("[rpc] [filters] evicted idle logs filter", "id", id, "protocol", protocol, "timeout", timeout) + evicted++ + ff.logger.Info("[rpc] [filters] evicted idle filter", "type", FilterTypeLogs, "id", id, "protocol", protocol, "timeout", timeout) ff.decrementMetrics(FilterTypeLogs, protocol) getReapedCounter(string(FilterTypeLogs)).Inc() } - // One remote update covers every logs filter removed this cycle. - if logsEvicted > 0 { + if evicted > 0 { ff.updateRemoteLogsFilter() } - - // Log summary at DEBUG level - totalEvicted := headsEvicted + txsEvicted + logsEvicted - if totalEvicted > 0 { - ff.logger.Debug("[rpc] [filters] eviction cycle complete", - "headsChecked", headsChecked, "headsEvicted", headsEvicted, - "txsChecked", txsChecked, "txsEvicted", txsEvicted, - "logsChecked", logsChecked, "logsEvicted", logsEvicted) - } else { - ff.logger.Trace("[rpc] [filters] eviction cycle complete, no stale filters", - "headsChecked", headsChecked, "txsChecked", txsChecked, "logsChecked", logsChecked) - } + return checked, evicted } -// TouchSubscription resets the deadline for the given filter, preventing it from being evicted. -// This should be called whenever a filter is accessed (e.g., via GetFilterChanges). -// Called on every poll, so it must stay cheap — no logging on the hot path. -func (ff *Filters) TouchSubscription(id SubscriptionID, ft FilterType) { +// trackedSub resolves the tracking surface of the subscription with the given ID and type. +func (ff *Filters) trackedSub(id SubscriptionID, ft FilterType) (SubTracker, bool) { switch ft { case FilterTypeHeads: if sub, ok := ff.headsSubs.Get(HeadsSubID(id)); ok { - sub.Touch() + return sub, true } case FilterTypePendingTxs: if sub, ok := ff.pendingTxsSubs.Get(PendingTxsSubID(id)); ok { - sub.Touch() + return sub, true } case FilterTypeLogs: if filter, ok := ff.logsSubs.logsFilters.Get(LogsSubID(id)); ok && filter.sender != nil { - filter.sender.Touch() + return filter.sender, true } } + return nil, false +} + +// TouchSubscription resets the eviction deadline for the given filter and reports +// whether the subscription exists. Called on every poll, so it must stay cheap — +// no logging on the hot path. +func (ff *Filters) TouchSubscription(id SubscriptionID, ft FilterType) bool { + sub, ok := ff.trackedSub(id, ft) + if !ok { + return false + } + sub.Touch() + return true } // TrackSubscription enables timeout tracking and metrics for a subscription. // This should be called for HTTP polling filters. For WebSocket subscriptions, call -// SetSubscriptionProtocol directly without enabling tracking as they are "evicted" once -// connections are closed. +// SetSubscriptionProtocol instead as they are "evicted" once connections are closed. func (ff *Filters) TrackSubscription(id SubscriptionID, ft FilterType, protocol string) { - // Only increment metrics when the subscription is actually found and tagged. - // Incrementing on a missing lookup would leak the gauge — nothing later - // decrements it because there is no subscription to unsubscribe. - switch ft { - case FilterTypeHeads: - if sub, ok := ff.headsSubs.Get(HeadsSubID(id)); ok { - sub.SetProtocol(protocol) - sub.EnableTimeout() - ff.logger.Debug("[rpc] [filters] registered heads subscription with timeout tracking", "id", id, "protocol", protocol) - ff.incrementMetrics(ft, protocol) - } - case FilterTypePendingTxs: - if sub, ok := ff.pendingTxsSubs.Get(PendingTxsSubID(id)); ok { - sub.SetProtocol(protocol) - sub.EnableTimeout() - ff.logger.Debug("[rpc] [filters] registered pending txs subscription with timeout tracking", "id", id, "protocol", protocol) - ff.incrementMetrics(ft, protocol) - } - case FilterTypeLogs: - if filter, ok := ff.logsSubs.logsFilters.Get(LogsSubID(id)); ok && filter.sender != nil { - filter.sender.SetProtocol(protocol) - filter.sender.EnableTimeout() - ff.logger.Debug("[rpc] [filters] registered logs subscription with timeout tracking", "id", id, "protocol", protocol) - ff.incrementMetrics(ft, protocol) - } - } + ff.registerSubscription(id, ft, protocol, true) } // SetSubscriptionProtocol sets the protocol for metrics tracking without enabling timeout. -// Use this for WebSocket subscriptions that don't need eviction but need metrics. func (ff *Filters) SetSubscriptionProtocol(id SubscriptionID, ft FilterType, protocol string) { - switch ft { - case FilterTypeHeads: - if sub, ok := ff.headsSubs.Get(HeadsSubID(id)); ok { - sub.SetProtocol(protocol) - ff.logger.Debug("[rpc] [filters] registered heads subscription (no timeout)", "id", id, "protocol", protocol) - ff.incrementMetrics(ft, protocol) - } - case FilterTypePendingTxs: - if sub, ok := ff.pendingTxsSubs.Get(PendingTxsSubID(id)); ok { - sub.SetProtocol(protocol) - ff.logger.Debug("[rpc] [filters] registered pending txs subscription (no timeout)", "id", id, "protocol", protocol) - ff.incrementMetrics(ft, protocol) - } - case FilterTypeLogs: - if filter, ok := ff.logsSubs.logsFilters.Get(LogsSubID(id)); ok && filter.sender != nil { - filter.sender.SetProtocol(protocol) - ff.logger.Debug("[rpc] [filters] registered logs subscription (no timeout)", "id", id, "protocol", protocol) - ff.incrementMetrics(ft, protocol) - } + ff.registerSubscription(id, ft, protocol, false) +} + +func (ff *Filters) registerSubscription(id SubscriptionID, ft FilterType, protocol string, enableTimeout bool) { + sub, ok := ff.trackedSub(id, ft) + if !ok { + // Skip metrics for a missing subscription — nothing would ever decrement them. + return + } + sub.SetProtocol(protocol) + if enableTimeout { + sub.EnableTimeout() } + ff.logger.Debug("[rpc] [filters] registered subscription", "type", ft, "id", id, "protocol", protocol, "evictable", enableTimeout) + ff.incrementMetrics(ft, protocol) } // subscribeToPendingTransactions subscribes to pending transactions using the given transaction pool client. @@ -642,18 +608,23 @@ func (ff *Filters) UnsubscribeHeads(id HeadsSubID) bool { return true } -// unsubscribeHeadsInternal performs the actual unsubscription without updating metrics. -// Used by eviction to avoid double-counting. func (ff *Filters) unsubscribeHeadsInternal(id HeadsSubID) bool { - sub, ok := ff.headsSubs.Get(id) + return unsubscribeSubInternal(ff.headsSubs, ff.pendingHeadsStores, id) +} + +// unsubscribeSubInternal tears down a subscription and its buffered store without +// updating metrics: eviction and client unsubscribe share it, and the caller owns +// the single decrement, gated on the map delete succeeding. +func unsubscribeSubInternal[K comparable, T, S any](subs *concurrent.SyncMap[K, Sub[T]], stores *concurrent.SyncMap[K, S], id K) bool { + sub, ok := subs.Get(id) if !ok { return false } sub.Close() - if _, ok = ff.headsSubs.Delete(id); !ok { + if _, ok = subs.Delete(id); !ok { return false } - ff.pendingHeadsStores.Delete(id) + stores.Delete(id) return true } @@ -725,19 +696,8 @@ func (ff *Filters) UnsubscribePendingTxs(id PendingTxsSubID) bool { return true } -// unsubscribePendingTxsInternal performs the actual unsubscription without updating metrics. -// Used by eviction to avoid double-counting. func (ff *Filters) unsubscribePendingTxsInternal(id PendingTxsSubID) bool { - sub, ok := ff.pendingTxsSubs.Get(id) - if !ok { - return false - } - sub.Close() - if _, ok = ff.pendingTxsSubs.Delete(id); !ok { - return false - } - ff.pendingTxsStores.Delete(id) - return true + return unsubscribeSubInternal(ff.pendingTxsSubs, ff.pendingTxsStores, id) } // SubscribeReceipts subscribes to transaction receipts and returns a channel to receive the receipts @@ -902,8 +862,8 @@ func (ff *Filters) UnsubscribeLogs(id LogsSubID) bool { return true } -// unsubscribeLogsInternal performs the actual unsubscription without updating metrics. -// Used by eviction to avoid double-counting. +// unsubscribeLogsInternal is the logs counterpart of unsubscribeSubInternal (metrics +// stay with the caller); logs additionally push the shrunken aggregate to the remote. func (ff *Filters) unsubscribeLogsInternal(id LogsSubID) bool { if !ff.logsSubs.removeLogsFilter(id) { return false @@ -1051,7 +1011,15 @@ func (ff *Filters) OnNewLogs(reply *remoteproto.SubscribeLogsReply) { // AddLogs adds logs to the store associated with the given subscription ID. func (ff *Filters) AddLogs(id LogsSubID, log *types.Log) { - ff.logsStores.DoAndStore(id, func(st []*types.Log, ok bool) []*types.Log { + ff.logsStores.Do(id, func(st []*types.Log, ok bool) ([]*types.Log, bool) { + // Drop (and clear) the entry when the subscription is gone: reads are gated + // on the subscription's existence, so a late write from the forwarding + // goroutine draining a closed channel would orphan the entry forever. + // Checking under the store lock makes this race-free against the + // remove-subscription-then-delete-store teardown ordering. + if !ff.logsSubs.hasLogsFilter(id) { + return nil, false + } if !ok { st = make([]*types.Log, 0) } @@ -1068,7 +1036,7 @@ func (ff *Filters) AddLogs(id LogsSubID, log *types.Log) { // Append the new log st = append(st, log) - return st + return st, true }) } @@ -1080,7 +1048,11 @@ func (ff *Filters) ReadLogs(id LogsSubID) ([]*types.Log, bool) { // AddPendingBlock adds a pending block header to the store associated with the given subscription ID. func (ff *Filters) AddPendingBlock(id HeadsSubID, block *types.Header) { - ff.pendingHeadsStores.DoAndStore(id, func(st []*types.Header, ok bool) []*types.Header { + ff.pendingHeadsStores.Do(id, func(st []*types.Header, ok bool) ([]*types.Header, bool) { + // Same orphaned-store guard as AddLogs. + if _, exists := ff.headsSubs.Get(id); !exists { + return nil, false + } if !ok { st = make([]*types.Header, 0) } @@ -1097,7 +1069,7 @@ func (ff *Filters) AddPendingBlock(id HeadsSubID, block *types.Header) { // Append the new header st = append(st, block) - return st + return st, true }) } @@ -1109,7 +1081,11 @@ func (ff *Filters) ReadPendingBlocks(id HeadsSubID) ([]*types.Header, bool) { // AddPendingTxs adds pending transactions to the store associated with the given subscription ID. func (ff *Filters) AddPendingTxs(id PendingTxsSubID, txs []types.Transaction) { - ff.pendingTxsStores.DoAndStore(id, func(st [][]types.Transaction, ok bool) [][]types.Transaction { + ff.pendingTxsStores.Do(id, func(st [][]types.Transaction, ok bool) ([][]types.Transaction, bool) { + // Same orphaned-store guard as AddLogs. + if _, exists := ff.pendingTxsSubs.Get(id); !exists { + return nil, false + } if !ok { st = make([][]types.Transaction, 0) } @@ -1142,7 +1118,7 @@ func (ff *Filters) AddPendingTxs(id PendingTxsSubID, txs []types.Transaction) { // Append the new transactions as a new batch st = append(st, txs) - return st + return st, true }) } @@ -1200,10 +1176,7 @@ func (ff *Filters) WithTemporalOverlay(tx kv.TemporalTx) kv.TemporalTx { } func (ff *Filters) incrementMetrics(ft FilterType, protocol string) { - activeSubscriptionsGauge.With(prometheus.Labels{ - filterLabelName: string(ft), - protocolLabelName: protocol, - }).Inc() + activeSubscriptionsGauge.WithLabelValues(string(ft), protocol).Inc() getSubscriptionCounter("created", string(ft), protocol).Inc() } @@ -1211,9 +1184,6 @@ func (ff *Filters) decrementMetrics(ft FilterType, protocol string) { if protocol == "" { return // Not tracked } - activeSubscriptionsGauge.With(prometheus.Labels{ - filterLabelName: string(ft), - protocolLabelName: protocol, - }).Dec() + activeSubscriptionsGauge.WithLabelValues(string(ft), protocol).Dec() getSubscriptionCounter("unsubscribed", string(ft), protocol).Inc() } diff --git a/rpc/rpchelper/filters_eviction_test.go b/rpc/rpchelper/filters_eviction_test.go index e5107a1f405..1a8d01cc3cf 100644 --- a/rpc/rpchelper/filters_eviction_test.go +++ b/rpc/rpchelper/filters_eviction_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/require" "github.com/erigontech/erigon/common/log/v3" + "github.com/erigontech/erigon/execution/types" "github.com/erigontech/erigon/node/gointerfaces/remoteproto" "github.com/erigontech/erigon/rpc/filters" ) @@ -108,6 +109,35 @@ func TestEvictStaleSubscriptionsSkipsWebSocketSubscriptions(t *testing.T) { require.True(t, f.HasHeadsSubscription(id)) } +// A forwarding goroutine can drain channel-buffered items after its subscription is +// unsubscribed or evicted; such late writes must not recreate the per-filter store, +// which would be unreachable (reads are gated on the subscription) and leak forever. +func TestAddAfterUnsubscribeDoesNotOrphanStore(t *testing.T) { + f := newTestFilters(t) + + t.Run("heads", func(t *testing.T) { + _, id := f.SubscribeNewHeads(8) + require.True(t, f.UnsubscribeHeads(id)) + f.AddPendingBlock(id, &types.Header{}) + _, ok := f.pendingHeadsStores.Get(id) + require.False(t, ok) + }) + t.Run("pendingTxs", func(t *testing.T) { + _, id := f.SubscribePendingTxs(8) + require.True(t, f.UnsubscribePendingTxs(id)) + f.AddPendingTxs(id, []types.Transaction{}) + _, ok := f.pendingTxsStores.Get(id) + require.False(t, ok) + }) + t.Run("logs", func(t *testing.T) { + _, id := f.SubscribeLogs(8, filters.FilterCriteria{}) + require.True(t, f.UnsubscribeLogs(id)) + f.AddLogs(id, &types.Log{}) + _, ok := f.logsStores.Get(id) + require.False(t, ok) + }) +} + func TestLogsEvictionBatchesRemoteFilterUpdate(t *testing.T) { f := newTestFilters(t) diff --git a/rpc/rpchelper/filters_test.go b/rpc/rpchelper/filters_test.go index cf1ae0f8f98..08f0d77165e 100644 --- a/rpc/rpchelper/filters_test.go +++ b/rpc/rpchelper/filters_test.go @@ -450,7 +450,7 @@ func TestFilters_AddLogs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { config := FiltersConfig{RpcSubscriptionFiltersMaxLogs: tt.maxLogs} f := New(t.Context(), config, nil, nil, nil, func() {}, log.New(), nil) - logID := LogsSubID("test-log") + _, logID := f.SubscribeLogs(8, filters.FilterCriteria{}) logEntry := &types.Log{Address: common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87")} for i := 0; i < tt.numToAdd; i++ { @@ -484,7 +484,7 @@ func TestFilters_AddPendingBlocks(t *testing.T) { t.Run(tt.name, func(t *testing.T) { config := FiltersConfig{RpcSubscriptionFiltersMaxHeaders: tt.maxHeaders} f := New(t.Context(), config, nil, nil, nil, func() {}, log.New(), nil) - blockID := HeadsSubID("test-block") + _, blockID := f.SubscribeNewHeads(8) header := &types.Header{} for i := 0; i < tt.numToAdd; i++ { @@ -519,7 +519,7 @@ func TestFilters_AddPendingTxs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { config := FiltersConfig{RpcSubscriptionFiltersMaxTxs: tt.maxTxs} f := New(t.Context(), config, nil, nil, nil, func() {}, log.New(), nil) - txID := PendingTxsSubID("test-tx") + _, txID := f.SubscribePendingTxs(8) var txn types.Transaction = types.NewTransaction(0, common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87"), uint256.NewInt(10), 50000, uint256.NewInt(10), nil) txn, _ = txn.WithSignature(*types.LatestSignerForChainID(nil), common.Hex2Bytes("9bea4c4daac7c7c52e093e6a4c35dbbcf8856f1af7b059ba20253e70848d094f8a8fae537ce25ed8cb5af9adac3f141af69bd515bd2ba031522df09b97dd72b100")) diff --git a/rpc/rpchelper/subscription.go b/rpc/rpchelper/subscription.go index a609febee44..02863f91071 100644 --- a/rpc/rpchelper/subscription.go +++ b/rpc/rpchelper/subscription.go @@ -25,12 +25,17 @@ import ( type Sub[T any] interface { Send(T) Close() - // Tracking methods for timeout-based eviction and metrics - Touch() // Reset last access time (for HTTP polling filters) - SetProtocol(protocol string) // Set protocol label for metrics - EnableTimeout() // Enable timeout tracking (HTTP only) CloseIfIdle(timeout time.Duration) bool // Close and report true when idle longer than timeout - Protocol() string // Get protocol label for metrics + SubTracker +} + +// SubTracker is the element-type-independent tracking surface of a subscription, +// used for timeout-based eviction and metrics. +type SubTracker interface { + Touch() // Reset last access time (for HTTP polling filters) + SetProtocol(protocol string) // Set protocol label for metrics + EnableTimeout() // Enable timeout tracking (HTTP only) + Protocol() string // Get protocol label for metrics } type chan_sub[T any] struct { From 60e6af125eda476783141bfff35218c783982094 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Mon, 6 Jul 2026 12:45:59 +0200 Subject: [PATCH 06/11] rpc/rpchelper: fix metric doc pattern braces, yield in concurrency test loops --- rpc/rpchelper/filters_eviction_test.go | 3 +++ rpc/rpchelper/metrics.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/rpc/rpchelper/filters_eviction_test.go b/rpc/rpchelper/filters_eviction_test.go index 1a8d01cc3cf..26be5ffe24e 100644 --- a/rpc/rpchelper/filters_eviction_test.go +++ b/rpc/rpchelper/filters_eviction_test.go @@ -17,6 +17,7 @@ package rpchelper import ( + "runtime" "sync" "sync/atomic" "testing" @@ -183,6 +184,7 @@ func TestConcurrentTouchAndEviction(t *testing.T) { return default: f.TouchSubscription(SubscriptionID(id), FilterTypePendingTxs) + runtime.Gosched() } } }() @@ -190,6 +192,7 @@ func TestConcurrentTouchAndEviction(t *testing.T) { deadline := time.Now().Add(200 * time.Millisecond) for time.Now().Before(deadline) { f.evictStaleSubscriptions(50 * time.Millisecond) + runtime.Gosched() } close(stop) wg.Wait() diff --git a/rpc/rpchelper/metrics.go b/rpc/rpchelper/metrics.go index 523821a2ae4..d425e02aeb6 100644 --- a/rpc/rpchelper/metrics.go +++ b/rpc/rpchelper/metrics.go @@ -48,7 +48,7 @@ var ( ) // getSubscriptionCounter returns a counter for subscription lifecycle events. -// pattern: subscriptions_{event}_total{{filter="{filterType}",protocol="{protocol}"}} +// pattern: subscriptions__total{filter="",protocol=""} func getSubscriptionCounter(event, filterType, protocol string) metrics.Counter { return metrics.GetOrCreateCounter( `subscriptions_` + event + `_total{filter="` + filterType + `",protocol="` + protocol + `"}`, From eadb77394d65d533ecb600117c192fe5cc918276 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Mon, 6 Jul 2026 12:56:44 +0200 Subject: [PATCH 07/11] rpc/rpchelper: evict idle filters after 5 minutes by default, matching geth --- cmd/rpcdaemon/cli/config.go | 2 +- rpc/rpchelper/config.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/rpcdaemon/cli/config.go b/cmd/rpcdaemon/cli/config.go index fead5e85ff0..373694869da 100644 --- a/cmd/rpcdaemon/cli/config.go +++ b/cmd/rpcdaemon/cli/config.go @@ -181,7 +181,7 @@ func RootCommand() (*cobra.Command, *httpcfg.HttpCfg) { rootCmd.PersistentFlags().IntVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersMaxTxs, "rpc.subscription.filters.maxtxs", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersMaxTxs, "Maximum number of transactions to store per subscription.") rootCmd.PersistentFlags().IntVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersMaxAddresses, "rpc.subscription.filters.maxaddresses", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersMaxAddresses, "Maximum number of addresses per subscription to filter logs by.") rootCmd.PersistentFlags().IntVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersMaxTopics, "rpc.subscription.filters.maxtopics", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersMaxTopics, "Maximum number of topics per subscription to filter logs by.") - rootCmd.PersistentFlags().DurationVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersTimeout, "rpc.subscription.filters.timeout", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersTimeout, "Timeout before idle filters are evicted. Defaults to 0 to disable eviction.") + rootCmd.PersistentFlags().DurationVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersTimeout, "rpc.subscription.filters.timeout", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersTimeout, "Timeout before idle filters are evicted. Set to 0 to disable eviction.") rootCmd.PersistentFlags().IntVar(&cfg.BlockRangeLimit, utils.RpcBlockRangeLimit.Name, utils.RpcBlockRangeLimit.Value, utils.RpcBlockRangeLimit.Usage) rootCmd.PersistentFlags().IntVar(&cfg.GetLogsMaxResults, utils.RpcGetLogsMaxResults.Name, utils.RpcGetLogsMaxResults.Value, utils.RpcGetLogsMaxResults.Usage) rootCmd.PersistentFlags().IntVar(&cfg.BatchLimit, utils.RpcBatchLimit.Name, utils.RpcBatchLimit.Value, utils.RpcBatchLimit.Usage) diff --git a/rpc/rpchelper/config.go b/rpc/rpchelper/config.go index ea078a30a26..eacf35fe107 100644 --- a/rpc/rpchelper/config.go +++ b/rpc/rpchelper/config.go @@ -18,8 +18,8 @@ package rpchelper import "time" -// DefaultFilterTimeout is the default eviction timeout; 0 disables eviction. -const DefaultFilterTimeout time.Duration = 0 +// DefaultFilterTimeout matches geth's deadline for evicting idle filters; 0 disables eviction. +const DefaultFilterTimeout = 5 * time.Minute // FiltersConfig defines the configuration settings for RPC subscription filters. // Each field represents a limit on the number of respective items that can be stored per subscription. @@ -30,7 +30,7 @@ type FiltersConfig struct { RpcSubscriptionFiltersMaxTxs int // Maximum number of transactions to store per subscription. Default: 10000 RpcSubscriptionFiltersMaxAddresses int // Maximum number of addresses per subscription to filter logs by. Default: 0 (no limit) RpcSubscriptionFiltersMaxTopics int // Maximum number of topics per subscription to filter logs by. Default: 0 (no limit) - RpcSubscriptionFiltersTimeout time.Duration // Timeout before idle filters are evicted. Default: 0 (no eviction) + RpcSubscriptionFiltersTimeout time.Duration // Timeout before idle filters are evicted. Default: 5m; 0 disables eviction } // DefaultFiltersConfig defines the default settings for filter configurations. From 8005c87d13ceca9790a39c3f450a8663622f1321 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Mon, 6 Jul 2026 12:56:44 +0200 Subject: [PATCH 08/11] rpc/filters: remove commented-out geth PublicFilterAPI port Dead since its import; erigon's filter machinery lives in rpc/rpchelper. Keeps the criteria types and FilterCriteria JSON decoding, and fixes the ReceiptsFilterCriteria doc-comment name. --- rpc/filters/api.go | 433 +-------------------------------------------- 1 file changed, 1 insertion(+), 432 deletions(-) diff --git a/rpc/filters/api.go b/rpc/filters/api.go index f15d54cf02e..5ce708dd59c 100644 --- a/rpc/filters/api.go +++ b/rpc/filters/api.go @@ -48,444 +48,13 @@ func DefaultLogFilterOptions() LogFilterOptions { } } -// TransactionReceiptsFilter defines criteria for transaction receipts subscription. +// ReceiptsFilterCriteria defines criteria for transaction receipts subscription. // If TransactionHashes is nil or empty, receipts for all transactions included in new blocks will be delivered. // Otherwise, only receipts for the specified transactions will be delivered. type ReceiptsFilterCriteria struct { TransactionHashes []common.Hash `json:"transactionHashes"` } -/* -// filter is a helper struct that holds meta information over the filter type -// and associated subscription in the event system. -type filter struct { - typ Type - deadline *time.Timer // filter is inactiv when deadline triggers - hashes []common.Hash - crit FilterCriteria - logs []*types.Log - s *Subscription // associated subscription in event system -} - -// PublicFilterAPI offers support to create and manage filters. This will allow external clients to retrieve various -// information related to the Ethereum protocol such as blocks, transactions and logs. -type PublicFilterAPI struct { - backend Backend - quit chan struct{} - chainDb ethdb.Database - events *EventSystem - filtersMu sync.Mutex - filters map[rpc.ID]*filter - timeout time.Duration -} - -// NewPublicFilterAPI returns a new PublicFilterAPI instance. -func NewPublicFilterAPI(backend Backend, timeout time.Duration) *PublicFilterAPI { - api := &PublicFilterAPI{ - backend: backend, - quit: make(chan struct{}, 1), - chainDb: backend.ChainDb(), - events: NewEventSystem(backend), - filters: make(map[rpc.ID]*filter), - timeout: timeout, - } - go api.timeoutLoop(timeout) - - return api -} - -// timeoutLoop runs every 5 minutes and deletes filters that have not been recently used. -// Tt is started when the api is created. -func (api *PublicFilterAPI) timeoutLoop(timeout time.Duration) { - var toUninstall []*Subscription - ticker := time.NewTicker(timeout) - defer ticker.Stop() - for { - select { - case <-ticker.C: - //nothing to do - case <-api.quit: - return - } - - api.filtersMu.Lock() - for id, f := range api.filters { - select { - case <-f.deadline.C: - toUninstall = append(toUninstall, f.s) - delete(api.filters, id) - default: - continue - } - } - api.filtersMu.Unlock() - - // Unsubscribes are processed outside the lock to avoid the following scenario: - // event loop attempts broadcasting events to still active filters while - // Unsubscribe is waiting for it to process the uninstall request. - for _, s := range toUninstall { - s.Unsubscribe() - } - toUninstall = nil - } -} - -func (api *PublicFilterAPI) Close() { - close(api.quit) -} - -// NewPendingTransactionFilter creates a filter that fetches pending transaction hashes -// as transactions enter the pending state. -// -// It is part of the filter package because this filter can be used through the -// `eth_getFilterChanges` polling method that is also used for log filters. -// -// https://eth.wiki/json-rpc/API#eth_newpendingtransactionfilter -func (api *PublicFilterAPI) NewPendingTransactionFilter() rpc.ID { - var ( - pendingTxs = make(chan []common.Hash) - pendingTxSub = api.events.SubscribePendingTxs(pendingTxs) - ) - - api.filtersMu.Lock() - api.filters[pendingTxSub.ID] = &filter{typ: PendingTransactionsSubscription, deadline: time.NewTimer(api.timeout), hashes: make([]common.Hash, 0), s: pendingTxSub} - api.filtersMu.Unlock() - - go func() { - for { - select { - case ph := <-pendingTxs: - api.filtersMu.Lock() - if f, found := api.filters[pendingTxSub.ID]; found { - f.hashes = append(f.hashes, ph...) - } - api.filtersMu.Unlock() - case <-pendingTxSub.Err(): - api.filtersMu.Lock() - delete(api.filters, pendingTxSub.ID) - api.filtersMu.Unlock() - return - case <-api.quit: - return - } - } - }() - - return pendingTxSub.ID -} - -// NewPendingTransactions creates a subscription that is triggered each time a transaction -// enters the transaction pool and was signed from one of the transactions this nodes manages. -func (api *PublicFilterAPI) NewPendingTransactions(ctx context.Context) (*rpc.Subscription, error) { - notifier, supported := rpc.NotifierFromContext(ctx) - if !supported { - return &rpc.Subscription{}, rpc.ErrNotificationsUnsupported - } - - rpcSub := notifier.CreateSubscription() - - go func() { - txHashes := make(chan []common.Hash, 128) - pendingTxSub := api.events.SubscribePendingTxs(txHashes) - - for { - select { - case hashes := <-txHashes: - // To keep the original behaviour, send a single txn hash in one notification. - // TODO(rjl493456442) Send a batch of txn hashes in one notification - for _, h := range hashes { - notifier.Notify(rpcSub.ID, h) - } - case <-rpcSub.Err(): - pendingTxSub.Unsubscribe() - return - case <-notifier.Closed(): - pendingTxSub.Unsubscribe() - return - case <-api.quit: - return - } - } - }() - - return rpcSub, nil -} - -// NewBlockFilter creates a filter that fetches blocks that are imported into the chain. -// It is part of the filter package since polling goes with eth_getFilterChanges. -// -// https://eth.wiki/json-rpc/API#eth_newblockfilter -func (api *PublicFilterAPI) NewBlockFilter() rpc.ID { - var ( - headers = make(chan *types.Header) - headerSub = api.events.SubscribeNewHeads(headers) - ) - - api.filtersMu.Lock() - api.filters[headerSub.ID] = &filter{typ: BlocksSubscription, deadline: time.NewTimer(api.timeout), hashes: make([]common.Hash, 0), s: headerSub} - api.filtersMu.Unlock() - - go func() { - for { - select { - case h := <-headers: - api.filtersMu.Lock() - if f, found := api.filters[headerSub.ID]; found { - f.hashes = append(f.hashes, h.Hash()) - } - api.filtersMu.Unlock() - case <-headerSub.Err(): - api.filtersMu.Lock() - delete(api.filters, headerSub.ID) - api.filtersMu.Unlock() - return - case <-api.quit: - return - } - } - }() - - return headerSub.ID -} - -// NewHeads send a notification each time a new (header) block is appended to the chain. -func (api *PublicFilterAPI) NewHeads(ctx context.Context) (*rpc.Subscription, error) { - notifier, supported := rpc.NotifierFromContext(ctx) - if !supported { - return &rpc.Subscription{}, rpc.ErrNotificationsUnsupported - } - - rpcSub := notifier.CreateSubscription() - - go func() { - headers := make(chan *types.Header) - headersSub := api.events.SubscribeNewHeads(headers) - - for { - select { - case h := <-headers: - notifier.Notify(rpcSub.ID, h) - case <-rpcSub.Err(): - headersSub.Unsubscribe() - return - case <-notifier.Closed(): - headersSub.Unsubscribe() - return - case <-api.quit: - return - } - } - }() - - return rpcSub, nil -} - -// Logs creates a subscription that fires for all new log that match the given filter criteria. -func (api *PublicFilterAPI) Logs(ctx context.Context, crit FilterCriteria) (*rpc.Subscription, error) { - notifier, supported := rpc.NotifierFromContext(ctx) - if !supported { - return &rpc.Subscription{}, rpc.ErrNotificationsUnsupported - } - - var ( - rpcSub = notifier.CreateSubscription() - matchedLogs = make(chan []*types.Log) - ) - - logsSub, err := api.events.SubscribeLogs(bind.FilterQuery(crit), matchedLogs) - if err != nil { - return nil, err - } - - go func() { - - for { - select { - case logs := <-matchedLogs: - for _, log := range logs { - log := log - notifier.Notify(rpcSub.ID, &log) - } - case <-rpcSub.Err(): // client send an unsubscribe request - logsSub.Unsubscribe() - return - case <-notifier.Closed(): // connection dropped - logsSub.Unsubscribe() - return - case <-api.quit: - return - } - } - }() - - return rpcSub, nil -} -// NewFilter creates a new filter and returns the filter id. It can be -// used to retrieve logs when the state changes. This method cannot be -// used to fetch logs that are already stored in the state. -// -// Default criteria for the from and to block are "latest". -// Using "latest" as block number will return logs for mined blocks. -// Using "pending" as block number returns logs for not yet mined (pending) blocks. -// In case logs are removed (chain reorg) previously returned logs are returned -// again but with the removed property set to true. -// -// In case "fromBlock" > "toBlock" an error is returned. -// -// https://eth.wiki/json-rpc/API#eth_newfilter -func (api *PublicFilterAPI) NewFilter(crit FilterCriteria) (rpc.ID, error) { - logs := make(chan []*types.Log) - logsSub, err := api.events.SubscribeLogs(bind.FilterQuery(crit), logs) - if err != nil { - return rpc.ID(""), err - } - - api.filtersMu.Lock() - api.filters[logsSub.ID] = &filter{typ: LogsSubscription, crit: crit, deadline: time.NewTimer(api.timeout), logs: make([]*types.Log, 0), s: logsSub} - api.filtersMu.Unlock() - - go func() { - for { - select { - case l := <-logs: - api.filtersMu.Lock() - if f, found := api.filters[logsSub.ID]; found { - f.logs = append(f.logs, l...) - } - api.filtersMu.Unlock() - case <-logsSub.Err(): - api.filtersMu.Lock() - delete(api.filters, logsSub.ID) - api.filtersMu.Unlock() - return - case <-api.quit: - return - } - } - }() - - return logsSub.ID, nil -} - -// GetLogs returns logs matching the given argument that are stored within the state. -// -// https://eth.wiki/json-rpc/API#eth_getlogs -func (api *PublicFilterAPI) GetLogs(ctx context.Context, crit FilterCriteria) ([]*types.Log, error) { - var filter *Filter - if crit.BlockHash != nil { - // Block filter requested, construct a single-shot filter - filter = NewBlockFilter(api.backend, *crit.BlockHash, crit.Addresses, crit.Topics) - } else { - // Convert the RPC block numbers into internal representations - begin := rpc.LatestBlockNumber.Int64() - if crit.FromBlock != nil { - begin = crit.FromBlock.Int64() - } - end := rpc.LatestBlockNumber.Int64() - if crit.ToBlock != nil { - end = crit.ToBlock.Int64() - } - // Construct the range filter - filter = NewRangeFilter(api.backend, begin, end, crit.Addresses, crit.Topics) - } - // Run the filter and return all the logs - logs, err := filter.Logs(ctx) - if err != nil { - return nil, err - } - return returnLogs(logs), err -} - -// UninstallFilter removes the filter with the given filter id. -// -// https://eth.wiki/json-rpc/API#eth_uninstallfilter -func (api *PublicFilterAPI) UninstallFilter(id rpc.ID) bool { - api.filtersMu.Lock() - f, found := api.filters[id] - if found { - delete(api.filters, id) - } - api.filtersMu.Unlock() - if found { - f.s.Unsubscribe() - } - - return found -} - -// GetFilterLogs returns the logs for the filter with the given id. -// If the filter could not be found an empty array of logs is returned. -// -// https://eth.wiki/json-rpc/API#eth_getfilterlogs -func (api *PublicFilterAPI) GetFilterLogs(ctx context.Context, id rpc.ID) ([]*types.Log, error) { - api.filtersMu.Lock() - f, found := api.filters[id] - api.filtersMu.Unlock() - - if !found || f.typ != LogsSubscription { - return nil, errors.New("filter not found") - } - - var filter *Filter - if f.crit.BlockHash != nil { - // Block filter requested, construct a single-shot filter - filter = NewBlockFilter(api.backend, *f.crit.BlockHash, f.crit.Addresses, f.crit.Topics) - } else { - // Convert the RPC block numbers into internal representations - begin := rpc.LatestBlockNumber.Int64() - if f.crit.FromBlock != nil { - begin = f.crit.FromBlock.Int64() - } - end := rpc.LatestBlockNumber.Int64() - if f.crit.ToBlock != nil { - end = f.crit.ToBlock.Int64() - } - // Construct the range filter - filter = NewRangeFilter(api.backend, begin, end, f.crit.Addresses, f.crit.Topics) - } - // Run the filter and return all the logs - logs, err := filter.Logs(ctx) - if err != nil { - return nil, err - } - return returnLogs(logs), nil -} - -// GetFilterChanges returns the logs for the filter with the given id since -// last time it was called. This can be used for polling. -// -// For pending transaction and block filters the result is []common.Hash. -// (pending)Log filters return []Log. -// -// https://eth.wiki/json-rpc/API#eth_getfilterchanges -func (api *PublicFilterAPI) GetFilterChanges(id rpc.ID) (interface{}, error) { - api.filtersMu.Lock() - defer api.filtersMu.Unlock() - - if f, found := api.filters[id]; found { - if !f.deadline.Stop() { - // timer expired but filter is not yet removed in timeout loop - // receive timer value and reset timer - <-f.deadline.C - } - f.deadline.Reset(api.timeout) - - switch f.typ { - case PendingTransactionsSubscription, BlocksSubscription: - hashes := f.hashes - f.hashes = nil - return returnHashes(hashes), nil - case LogsSubscription, MinedAndPendingLogsSubscription: - logs := f.logs - f.logs = nil - return returnLogs(logs), nil - } - } - - return []interface{}{}, errors.New("filter not found") -} -*/ - // UnmarshalJSON sets *args fields with given data. func (args *FilterCriteria) UnmarshalJSON(data []byte) error { type input struct { From 91f39a8d4f4eb8aa89315a6d576cede012ae97dc Mon Sep 17 00:00:00 2001 From: yperbasis Date: Mon, 6 Jul 2026 13:02:28 +0200 Subject: [PATCH 09/11] node/cli, cmd/rpcdaemon: state the 5m eviction default in flag help; ChangeLog entry --- ChangeLog.md | 16 ++++++++++++++++ cmd/rpcdaemon/cli/config.go | 2 +- node/cli/flags.go | 2 +- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 77018d7012f..4bfeff3bda9 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -36,11 +36,27 @@ Aligns Erigon with the `eth_simulateV1` error code specification ([NethermindEth - If your tooling matches on error code `-32602` to detect base-fee-too-low conditions in `eth_simulateV1` responses, update it to match `-38012` instead. +--- + +#### JSON-RPC: idle polling filters are evicted after 5 minutes + +Filters created with `eth_newFilter`, `eth_newBlockFilter`, and `eth_newPendingTransactionFilter` are now evicted when not polled for 5 minutes, matching geth's stale-filter deadline. Previously they lived — and kept buffering data — until `eth_uninstallFilter` or a restart. + +**What changed:** + +| Aspect | Before | After | +|---|---|---| +| Idle polling filter | kept until uninstalled or restart | evicted after 5 minutes without a poll | +| `eth_getFilterChanges` / `eth_getFilterLogs` on an evicted id | — | `filter not found` | + +**Migration:** poll more often than the timeout, or recreate the filter when `filter not found` is returned (as with geth). Tune with `--rpc.subscription.filters.timeout`; set it to 0 to restore the previous keep-forever behavior. (#22261 by @onelapahead) + ### Added #### CLI & Operations - `--prune.distance.blocks` now accepts readable policy names — `keep-post-merge` and `keep-all` — instead of the raw `MaxUint64`-based magic numbers (`18446744073709551615` / `18446744073709551614`); `--prune.distance` likewise accepts `keep-all`. Numeric values still work (#22119) — by @yperbasis +- `--rpc.subscription.filters.timeout` — deadline for evicting idle RPC polling filters (default 5m; 0 disables). New `subscriptions_active` gauge and `subscriptions_created_total` / `subscriptions_unsubscribed_total` / `subscriptions_reaped_total` counters track the filter lifecycle (#22261) — by @onelapahead --- diff --git a/cmd/rpcdaemon/cli/config.go b/cmd/rpcdaemon/cli/config.go index 373694869da..5118aa21733 100644 --- a/cmd/rpcdaemon/cli/config.go +++ b/cmd/rpcdaemon/cli/config.go @@ -181,7 +181,7 @@ func RootCommand() (*cobra.Command, *httpcfg.HttpCfg) { rootCmd.PersistentFlags().IntVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersMaxTxs, "rpc.subscription.filters.maxtxs", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersMaxTxs, "Maximum number of transactions to store per subscription.") rootCmd.PersistentFlags().IntVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersMaxAddresses, "rpc.subscription.filters.maxaddresses", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersMaxAddresses, "Maximum number of addresses per subscription to filter logs by.") rootCmd.PersistentFlags().IntVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersMaxTopics, "rpc.subscription.filters.maxtopics", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersMaxTopics, "Maximum number of topics per subscription to filter logs by.") - rootCmd.PersistentFlags().DurationVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersTimeout, "rpc.subscription.filters.timeout", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersTimeout, "Timeout before idle filters are evicted. Set to 0 to disable eviction.") + rootCmd.PersistentFlags().DurationVar(&cfg.RpcFiltersConfig.RpcSubscriptionFiltersTimeout, "rpc.subscription.filters.timeout", rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersTimeout, "Timeout before idle filters are evicted. Defaults to 5m; set to 0 to disable eviction.") rootCmd.PersistentFlags().IntVar(&cfg.BlockRangeLimit, utils.RpcBlockRangeLimit.Name, utils.RpcBlockRangeLimit.Value, utils.RpcBlockRangeLimit.Usage) rootCmd.PersistentFlags().IntVar(&cfg.GetLogsMaxResults, utils.RpcGetLogsMaxResults.Name, utils.RpcGetLogsMaxResults.Value, utils.RpcGetLogsMaxResults.Usage) rootCmd.PersistentFlags().IntVar(&cfg.BatchLimit, utils.RpcBatchLimit.Name, utils.RpcBatchLimit.Value, utils.RpcBatchLimit.Usage) diff --git a/node/cli/flags.go b/node/cli/flags.go index 9f963416879..66a5f706495 100644 --- a/node/cli/flags.go +++ b/node/cli/flags.go @@ -209,7 +209,7 @@ var ( } RpcSubscriptionFiltersTimeoutFlag = cli.DurationFlag{ Name: "rpc.subscription.filters.timeout", - Usage: "Timeout before idle filters are evicted. Set to 0 to disable eviction.", + Usage: "Timeout before idle filters are evicted. Defaults to 5m; set to 0 to disable eviction.", Value: rpchelper.DefaultFiltersConfig.RpcSubscriptionFiltersTimeout, } ) From 65ebd007527d6f53bfd7746a238309ea0fbffb46 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Mon, 6 Jul 2026 16:15:38 +0200 Subject: [PATCH 10/11] rpc, diagnostics/metrics: review fixes for evictable polling filters - Track pollable subscriptions in a single id->type registry: Subscribe* now takes the protocol at creation (http = evictable polling filter, ws = connection-scoped, empty = internal), so tracking cannot be forgotten; TouchSubscription resolves the filter type in one map lookup; one eviction loop replaces evictIdleSubs/evictIdleLogsSubs and captures the protocol during the Range. - Add metrics.CounterVec (mirroring GaugeVec) and cache the subscription lifecycle counters as package vars instead of building a label string and hitting the global registry on every event. - hasLogsFilter: RLock instead of exclusive Lock; it runs per delivered log inside AddLogs' orphan guard. - Drop the now test-only Has*Subscription methods; reshape UnsubscribeLogs to the early-return shape of its siblings. --- diagnostics/metrics/countervec.go | 75 +++++++ diagnostics/metrics/register.go | 20 ++ diagnostics/metrics/set.go | 40 ++++ rpc/jsonrpc/eth_filters.go | 41 ++-- rpc/jsonrpc/eth_filters_test.go | 2 +- rpc/jsonrpc/eth_subscribe_test.go | 2 +- rpc/jsonrpc/send_transaction_test.go | 4 +- rpc/rpchelper/filters.go | 266 ++++++++++--------------- rpc/rpchelper/filters_deadlock_test.go | 2 +- rpc/rpchelper/filters_eviction_test.go | 71 ++++--- rpc/rpchelper/filters_test.go | 30 +-- rpc/rpchelper/logsfilter.go | 4 +- rpc/rpchelper/metrics.go | 31 +-- rpc/rpchelper/subscription.go | 58 +++--- 14 files changed, 350 insertions(+), 296 deletions(-) create mode 100644 diagnostics/metrics/countervec.go diff --git a/diagnostics/metrics/countervec.go b/diagnostics/metrics/countervec.go new file mode 100644 index 00000000000..b3467be0322 --- /dev/null +++ b/diagnostics/metrics/countervec.go @@ -0,0 +1,75 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package metrics + +import "github.com/prometheus/client_golang/prometheus" + +// Since we wrapped prometheus.Counter into a custom interface, we need to wrap prometheus.CounterVec into a custom struct +type CounterVec struct { + *prometheus.CounterVec +} + +func (cv *CounterVec) Collect(ch chan<- prometheus.Metric) { + cv.MetricVec.Collect(ch) +} + +func (cv *CounterVec) CurryWith(labels prometheus.Labels) (*CounterVec, error) { + cv2, err := cv.CounterVec.CurryWith(labels) + return &CounterVec{cv2}, err +} + +func (cv *CounterVec) Delete(labels prometheus.Labels) bool { + return cv.CounterVec.MetricVec.Delete(labels) +} + +func (cv *CounterVec) DeleteLabelValues(lvs ...string) bool { + return cv.CounterVec.MetricVec.DeleteLabelValues(lvs...) +} + +func (cv *CounterVec) DeletePartialMatch(labels prometheus.Labels) int { + return cv.CounterVec.MetricVec.DeletePartialMatch(labels) +} + +func (cv *CounterVec) Describe(ch chan<- *prometheus.Desc) { + cv.CounterVec.MetricVec.Describe(ch) +} + +func (cv *CounterVec) GetMetricWith(labels prometheus.Labels) (Counter, error) { + c, err := cv.CounterVec.GetMetricWith(labels) + return &counter{c}, err +} + +func (cv *CounterVec) GetMetricWithLabelValues(lvs ...string) (Counter, error) { + c, err := cv.CounterVec.GetMetricWithLabelValues(lvs...) + return &counter{c}, err +} + +func (cv *CounterVec) MustCurryWith(labels prometheus.Labels) *CounterVec { + return &CounterVec{cv.CounterVec.MustCurryWith(labels)} +} + +func (cv *CounterVec) Reset() { + cv.CounterVec.MetricVec.Reset() +} + +func (cv *CounterVec) With(labels prometheus.Labels) Counter { + return &counter{cv.CounterVec.With(labels)} +} + +func (cv *CounterVec) WithLabelValues(lvs ...string) Counter { + return &counter{cv.CounterVec.WithLabelValues(lvs...)} +} diff --git a/diagnostics/metrics/register.go b/diagnostics/metrics/register.go index 51464c32f3d..cf65fe22e16 100644 --- a/diagnostics/metrics/register.go +++ b/diagnostics/metrics/register.go @@ -63,6 +63,26 @@ func GetOrCreateCounter(name string) Counter { return &counter{c} } +// GetOrCreateCounterVec returns registered CounterVec with the given name +// or creates a new CounterVec if the registry doesn't contain a CounterVec with +// the given name and labels. +// +// name must be a valid Prometheus-compatible metric with possible labels. +// labels are the names of the dimensions associated with the counter vector. +// For instance, +// +// - foo, with labels []string{"bar", "baz"} +// +// The returned CounterVec is safe to use from concurrent goroutines. +func GetOrCreateCounterVec(name string, labels []string, help ...string) *CounterVec { + cv, err := defaultSet.GetOrCreateCounterVec(name, labels, help...) + if err != nil { + panic(fmt.Errorf("could not get or create new countervec: %w", err)) + } + + return &CounterVec{cv} +} + // NewGauge registers and returns gauge with the given name. // // name must be valid Prometheus-compatible metric with possible labels. diff --git a/diagnostics/metrics/set.go b/diagnostics/metrics/set.go index fbd9ac8d739..09c98a85d48 100644 --- a/diagnostics/metrics/set.go +++ b/diagnostics/metrics/set.go @@ -211,6 +211,46 @@ func (s *Set) GetOrCreateCounter(name string, help ...string) (prometheus.Counte }) } +// GetOrCreateCounterVec returns registered CounterVec in s with the given name +// or creates new CounterVec if s doesn't contain CounterVec with the given name. +// +// name must be valid Prometheus-compatible metric with possible labels. +// For instance, +// +// - foo +// - foo{bar="baz"} +// - foo{bar="baz",aaa="b"} +// +// labels are the labels associated with the CounterVec. +// +// The returned CounterVec is safe to use from concurrent goroutines. +func (s *Set) GetOrCreateCounterVec(name string, labels []string, help ...string) (*prometheus.CounterVec, error) { + return getOrCreateVec(s, name, func() (*prometheus.CounterVec, error) { + return newCounterVec(name, labels, help...) + }) +} + +// newCounterVec creates a new Prometheus CounterVec. +func newCounterVec(name string, labels []string, help ...string) (*prometheus.CounterVec, error) { + name, constLabels, err := parseMetric(name) + if err != nil { + return nil, err + } + + helpStr := "counter metric" + if len(help) > 0 { + helpStr = strings.Join(help, ", ") + } + + cv := prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: name, + Help: helpStr, + ConstLabels: constLabels, + }, labels) + + return cv, nil +} + // NewGauge registers and returns gauge with the given name. // // name must be valid Prometheus-compatible metric with possible labels. diff --git a/rpc/jsonrpc/eth_filters.go b/rpc/jsonrpc/eth_filters.go index 450cffb8489..03e19fa313c 100644 --- a/rpc/jsonrpc/eth_filters.go +++ b/rpc/jsonrpc/eth_filters.go @@ -35,8 +35,7 @@ func (api *APIImpl) NewPendingTransactionFilter(_ context.Context) (string, erro if api.filters == nil { return "", rpc.ErrNotificationsUnsupported } - txsCh, id := api.filters.SubscribePendingTxs(32) - api.filters.TrackSubscription(rpchelper.SubscriptionID(id), rpchelper.FilterTypePendingTxs, rpchelper.ProtocolHTTP) + txsCh, id := api.filters.SubscribePendingTxs(32, rpchelper.ProtocolHTTP) go func() { for txs := range txsCh { api.filters.AddPendingTxs(id, txs) @@ -50,8 +49,7 @@ func (api *APIImpl) NewBlockFilter(_ context.Context) (string, error) { if api.filters == nil { return "", rpc.ErrNotificationsUnsupported } - ch, id := api.filters.SubscribeNewHeads(32) - api.filters.TrackSubscription(rpchelper.SubscriptionID(id), rpchelper.FilterTypeHeads, rpchelper.ProtocolHTTP) + ch, id := api.filters.SubscribeNewHeads(32, rpchelper.ProtocolHTTP) go func() { for block := range ch { api.filters.AddPendingBlock(id, block) @@ -65,8 +63,7 @@ func (api *APIImpl) NewFilter(_ context.Context, crit filters.FilterCriteria) (s if api.filters == nil { return "", rpc.ErrNotificationsUnsupported } - logs, id := api.filters.SubscribeLogs(256, crit) - api.filters.TrackSubscription(rpchelper.SubscriptionID(id), rpchelper.FilterTypeLogs, rpchelper.ProtocolHTTP) + logs, id := api.filters.SubscribeLogs(256, crit, rpchelper.ProtocolHTTP) go func() { for lg := range logs { api.filters.AddLogs(id, lg) @@ -104,18 +101,18 @@ func (api *APIImpl) GetFilterChanges(_ context.Context, index string) ([]any, er stub := make([]any, 0) // remove 0x cutIndex := strings.TrimPrefix(index, "0x") - // The Touch probe identifies the id's type and resets the eviction deadline on - // every poll — even when no data is buffered — so an actively-polling client on - // a quiet chain is never evicted. - switch { - case api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeHeads): + ft, ok := api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex)) + if !ok { + return nil, rpc.ErrFilterNotFound + } + switch ft { + case rpchelper.FilterTypeHeads: if blocks, ok := api.filters.ReadPendingBlocks(rpchelper.HeadsSubID(cutIndex)); ok { for _, v := range blocks { stub = append(stub, v.Hash()) } } - return stub, nil - case api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypePendingTxs): + case rpchelper.FilterTypePendingTxs: if txs, ok := api.filters.ReadPendingTxs(rpchelper.PendingTxsSubID(cutIndex)); ok { for _, batch := range txs { for _, txn := range batch { @@ -123,17 +120,14 @@ func (api *APIImpl) GetFilterChanges(_ context.Context, index string) ([]any, er } } } - return stub, nil - case api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeLogs): + case rpchelper.FilterTypeLogs: if logs, ok := api.filters.ReadLogs(rpchelper.LogsSubID(cutIndex)); ok { for _, v := range logs { stub = append(stub, v) } } - return stub, nil - default: - return nil, rpc.ErrFilterNotFound } + return stub, nil } // GetFilterLogs implements eth_getFilterLogs. @@ -144,7 +138,7 @@ func (api *APIImpl) GetFilterLogs(_ context.Context, index string) ([]*types.Log return nil, rpc.ErrNotificationsUnsupported } cutIndex := strings.TrimPrefix(index, "0x") - if !api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex), rpchelper.FilterTypeLogs) { + if ft, ok := api.filters.TouchSubscription(rpchelper.SubscriptionID(cutIndex)); !ok || ft != rpchelper.FilterTypeLogs { return nil, rpc.ErrFilterNotFound } if logs, ok := api.filters.ReadLogs(rpchelper.LogsSubID(cutIndex)); ok { @@ -200,8 +194,7 @@ func subscribeRPC[T any](ctx context.Context, apiFilters *rpchelper.Filters, sub func (api *APIImpl) NewHeads(ctx context.Context) (*rpc.Subscription, error) { return subscribeRPC(ctx, api.filters, func() (<-chan *types.Header, func()) { - headers, id := api.filters.SubscribeNewHeads(32) - api.filters.SetSubscriptionProtocol(rpchelper.SubscriptionID(id), rpchelper.FilterTypeHeads, rpchelper.ProtocolWS) + headers, id := api.filters.SubscribeNewHeads(32, rpchelper.ProtocolWS) return headers, func() { api.filters.UnsubscribeHeads(id) } }, func(emit func(payload any), h *types.Header) { @@ -225,8 +218,7 @@ func (api *APIImpl) NewPendingTransactionsWithBody(ctx context.Context) (*rpc.Su func (api *APIImpl) subscribePendingTransactions(ctx context.Context, chanSize int, fullTx bool) (*rpc.Subscription, error) { return subscribeRPC(ctx, api.filters, func() (<-chan []types.Transaction, func()) { - txsCh, id := api.filters.SubscribePendingTxs(chanSize) - api.filters.SetSubscriptionProtocol(rpchelper.SubscriptionID(id), rpchelper.FilterTypePendingTxs, rpchelper.ProtocolWS) + txsCh, id := api.filters.SubscribePendingTxs(chanSize, rpchelper.ProtocolWS) return txsCh, func() { api.filters.UnsubscribePendingTxs(id) } }, func(emit func(payload any), txs []types.Transaction) { @@ -247,8 +239,7 @@ func (api *APIImpl) subscribePendingTransactions(ctx context.Context, chanSize i func (api *APIImpl) Logs(ctx context.Context, crit filters.FilterCriteria) (*rpc.Subscription, error) { return subscribeRPC(ctx, api.filters, func() (<-chan *types.Log, func()) { - logs, id := api.filters.SubscribeLogs(api.SubscribeLogsChannelSize, crit) - api.filters.SetSubscriptionProtocol(rpchelper.SubscriptionID(id), rpchelper.FilterTypeLogs, rpchelper.ProtocolWS) + logs, id := api.filters.SubscribeLogs(api.SubscribeLogsChannelSize, crit, rpchelper.ProtocolWS) return logs, func() { api.filters.UnsubscribeLogs(id) } }, func(emit func(payload any), h *types.Log) { diff --git a/rpc/jsonrpc/eth_filters_test.go b/rpc/jsonrpc/eth_filters_test.go index 0a80e8ee872..fdcf81c9bca 100644 --- a/rpc/jsonrpc/eth_filters_test.go +++ b/rpc/jsonrpc/eth_filters_test.go @@ -144,7 +144,7 @@ func TestLogsSubscribeAndUnsubscribe_WithoutConcurrentMapIssue(t *testing.T) { for i := 0; i < 1000; i++ { wg.Add(1) go func(idx int) { - _, id := ff.SubscribeLogs(32, crit) + _, id := ff.SubscribeLogs(32, crit, "") defer func() { time.Sleep(100 * time.Nanosecond) ff.UnsubscribeLogs(id) diff --git a/rpc/jsonrpc/eth_subscribe_test.go b/rpc/jsonrpc/eth_subscribe_test.go index 45f38bf8ffb..efef8a8cfff 100644 --- a/rpc/jsonrpc/eth_subscribe_test.go +++ b/rpc/jsonrpc/eth_subscribe_test.go @@ -62,7 +62,7 @@ func TestEthSubscribe(t *testing.T) { } ff := rpchelper.New(ctx, rpchelper.DefaultFiltersConfig, backend, nil, nil, onNewSnapshot, m.Log, nil) subscriptionReadyWg.Wait() // This is needed *before* inserting the blocks, which sends NEW_HEADER events - newHeads, id := ff.SubscribeNewHeads(16) + newHeads, id := ff.SubscribeNewHeads(16, "") defer ff.UnsubscribeHeads(id) highestSeenHeader := chain.TopBlock.NumberU64() err = m.InsertChain(chain) diff --git a/rpc/jsonrpc/send_transaction_test.go b/rpc/jsonrpc/send_transaction_test.go index 140798b3a1b..eb88075f5a7 100644 --- a/rpc/jsonrpc/send_transaction_test.go +++ b/rpc/jsonrpc/send_transaction_test.go @@ -59,7 +59,7 @@ func TestSendRawTransaction(t *testing.T) { buf := bytes.NewBuffer(nil) err = txn.MarshalBinary(buf) require.NoError(err) - txsCh, id := ff.SubscribePendingTxs(1) + txsCh, id := ff.SubscribePendingTxs(1, "") defer ff.UnsubscribePendingTxs(id) txHash, err := api.SendRawTransaction(ctx, buf.Bytes()) require.NoError(err) @@ -102,7 +102,7 @@ func TestSendRawTransactionUnprotected(t *testing.T) { buf := bytes.NewBuffer(nil) err = txn.MarshalBinary(buf) require.NoError(err) - txsCh, id := ff.SubscribePendingTxs(1) + txsCh, id := ff.SubscribePendingTxs(1, "") defer ff.UnsubscribePendingTxs(id) txHash, err := api.SendRawTransaction(ctx, buf.Bytes()) require.NoError(err) diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index 904f9747da5..6a2f94b8cdc 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -56,6 +56,14 @@ const ( FilterTypePendingTxs FilterType = "pendingTxs" ) +// trackedSub indexes a pollable subscription for O(1) id lookup: polls resolve the +// filter type and reset the eviction deadline in one step, and the eviction loop +// iterates all pollable subscriptions regardless of element type. +type trackedSub struct { + ft FilterType + tracker SubTracker +} + // Filters holds the state for managing subscriptions to various Ethereum events. // It allows for the subscription and management of events such as new blocks, pending transactions, // logs, and other Ethereum-related activities. @@ -78,6 +86,7 @@ type Filters struct { logsStores *concurrent.SyncMap[LogsSubID, []*types.Log] pendingHeadsStores *concurrent.SyncMap[HeadsSubID, []*types.Header] pendingTxsStores *concurrent.SyncMap[PendingTxsSubID, [][]types.Transaction] + trackedSubs *concurrent.SyncMap[SubscriptionID, trackedSub] logger log.Logger // latestSD is the local fallback for the most recent SharedDomains. @@ -108,6 +117,7 @@ func New(ctx context.Context, config FiltersConfig, ethBackend ApiBackend, txPoo logsStores: concurrent.NewSyncMap[LogsSubID, []*types.Log](), pendingHeadsStores: concurrent.NewSyncMap[HeadsSubID, []*types.Header](), pendingTxsStores: concurrent.NewSyncMap[PendingTxsSubID, [][]types.Transaction](), + trackedSubs: concurrent.NewSyncMap[SubscriptionID, trackedSub](), logger: logger, config: config, events: events, @@ -330,142 +340,75 @@ func (ff *Filters) timeoutLoop(ctx context.Context, timeout, checkInterval time. } } -// evictStaleSubscriptions removes filters that haven't been accessed within the timeout duration. +// evictStaleSubscriptions removes filters that haven't been accessed within the timeout +// duration. CloseIfIdle decides staleness and closes in one atomic step, so a concurrent +// Touch either lands before it (the subscription survives) or finds the subscription +// already closed; teardown and metrics follow after the Range to avoid mutating the +// maps during iteration. Evicted logs filters share one remote-filter update per cycle. func (ff *Filters) evictStaleSubscriptions(timeout time.Duration) { - headsChecked, headsEvicted := evictIdleSubs(ff, ff.headsSubs, FilterTypeHeads, timeout, ff.unsubscribeHeadsInternal) - txsChecked, txsEvicted := evictIdleSubs(ff, ff.pendingTxsSubs, FilterTypePendingTxs, timeout, ff.unsubscribePendingTxsInternal) - logsChecked, logsEvicted := ff.evictIdleLogsSubs(timeout) - - // Log summary at DEBUG level - totalEvicted := headsEvicted + txsEvicted + logsEvicted - if totalEvicted > 0 { - ff.logger.Debug("[rpc] [filters] eviction cycle complete", - "headsChecked", headsChecked, "headsEvicted", headsEvicted, - "txsChecked", txsChecked, "txsEvicted", txsEvicted, - "logsChecked", logsChecked, "logsEvicted", logsEvicted) - } else { - ff.logger.Trace("[rpc] [filters] eviction cycle complete, no stale filters", - "headsChecked", headsChecked, "txsChecked", txsChecked, "logsChecked", logsChecked) + type victim struct { + id SubscriptionID + ft FilterType + protocol SubProtocol } -} - -// evictIdleSubs closes and removes the subscriptions in subs that have been idle -// longer than timeout. CloseIfIdle decides staleness and closes in one atomic step, -// so a concurrent Touch either lands before it (the subscription survives) or finds -// the subscription already closed; the map delete and metrics follow after the -// Range to avoid mutating the map during iteration. -func evictIdleSubs[K comparable, T any](ff *Filters, subs *concurrent.SyncMap[K, Sub[T]], ft FilterType, timeout time.Duration, unsubscribe func(K) bool) (checked, evicted int) { - var toEvict []K - subs.Range(func(id K, sub Sub[T]) error { + checked := 0 + var victims []victim + ff.trackedSubs.Range(func(id SubscriptionID, sub trackedSub) error { checked++ - if sub.CloseIfIdle(timeout) { - toEvict = append(toEvict, id) + if sub.tracker.CloseIfIdle(timeout) { + victims = append(victims, victim{id, sub.ft, sub.tracker.Protocol()}) } return nil }) - for _, id := range toEvict { - sub, ok := subs.Get(id) - if !ok { - continue - } - protocol := sub.Protocol() - if unsubscribe(id) { - evicted++ - ff.logger.Info("[rpc] [filters] evicted idle filter", "type", ft, "id", id, "protocol", protocol, "timeout", timeout) - ff.decrementMetrics(ft, protocol) - getReapedCounter(string(ft)).Inc() - } - } - return checked, evicted -} -// evictIdleLogsSubs mirrors evictIdleSubs for logs filters, which live behind the -// aggregator and share one remote-filter update per eviction cycle. -func (ff *Filters) evictIdleLogsSubs(timeout time.Duration) (checked, evicted int) { - var toEvict []LogsSubID - ff.logsSubs.logsFilters.Range(func(id LogsSubID, filter *LogsFilter) error { - checked++ - if filter.sender != nil && filter.sender.CloseIfIdle(timeout) { - toEvict = append(toEvict, id) - } - return nil - }) - for _, id := range toEvict { - filter, ok := ff.logsSubs.logsFilters.Get(id) - if !ok || filter.sender == nil { - continue + evicted := 0 + logsEvicted := false + for _, v := range victims { + var removed bool + switch v.ft { + case FilterTypeHeads: + removed = ff.unsubscribeHeadsInternal(HeadsSubID(v.id)) + case FilterTypePendingTxs: + removed = ff.unsubscribePendingTxsInternal(PendingTxsSubID(v.id)) + case FilterTypeLogs: + removed = ff.removeLogsSubscription(LogsSubID(v.id), false) + logsEvicted = logsEvicted || removed } - protocol := filter.sender.Protocol() - if !ff.logsSubs.removeLogsFilter(id) { + if !removed { continue } - ff.deleteLogStore(id) evicted++ - ff.logger.Info("[rpc] [filters] evicted idle filter", "type", FilterTypeLogs, "id", id, "protocol", protocol, "timeout", timeout) - ff.decrementMetrics(FilterTypeLogs, protocol) - getReapedCounter(string(FilterTypeLogs)).Inc() + ff.logger.Info("[rpc] [filters] evicted idle filter", "type", v.ft, "id", v.id, "protocol", v.protocol, "timeout", timeout) + ff.decrementMetrics(v.ft, v.protocol) + reapedSubscriptionsCounter.WithLabelValues(string(v.ft)).Inc() } - if evicted > 0 { + if logsEvicted { ff.updateRemoteLogsFilter() } - return checked, evicted -} -// trackedSub resolves the tracking surface of the subscription with the given ID and type. -func (ff *Filters) trackedSub(id SubscriptionID, ft FilterType) (SubTracker, bool) { - switch ft { - case FilterTypeHeads: - if sub, ok := ff.headsSubs.Get(HeadsSubID(id)); ok { - return sub, true - } - case FilterTypePendingTxs: - if sub, ok := ff.pendingTxsSubs.Get(PendingTxsSubID(id)); ok { - return sub, true - } - case FilterTypeLogs: - if filter, ok := ff.logsSubs.logsFilters.Get(LogsSubID(id)); ok && filter.sender != nil { - return filter.sender, true - } + if evicted > 0 { + ff.logger.Debug("[rpc] [filters] eviction cycle complete", "checked", checked, "evicted", evicted) + } else { + ff.logger.Trace("[rpc] [filters] eviction cycle complete, no stale filters", "checked", checked) } - return nil, false } -// TouchSubscription resets the eviction deadline for the given filter and reports -// whether the subscription exists. Called on every poll, so it must stay cheap — -// no logging on the hot path. -func (ff *Filters) TouchSubscription(id SubscriptionID, ft FilterType) bool { - sub, ok := ff.trackedSub(id, ft) +// TouchSubscription resets the eviction deadline of the subscription and reports its +// filter type, so a poll on a quiet chain keeps the filter alive even when nothing is +// buffered. Called on every poll: a single map lookup, no logging. +func (ff *Filters) TouchSubscription(id SubscriptionID) (FilterType, bool) { + sub, ok := ff.trackedSubs.Get(id) if !ok { - return false + return "", false } - sub.Touch() - return true + sub.tracker.Touch() + return sub.ft, true } -// TrackSubscription enables timeout tracking and metrics for a subscription. -// This should be called for HTTP polling filters. For WebSocket subscriptions, call -// SetSubscriptionProtocol instead as they are "evicted" once connections are closed. -func (ff *Filters) TrackSubscription(id SubscriptionID, ft FilterType, protocol string) { - ff.registerSubscription(id, ft, protocol, true) -} - -// SetSubscriptionProtocol sets the protocol for metrics tracking without enabling timeout. -func (ff *Filters) SetSubscriptionProtocol(id SubscriptionID, ft FilterType, protocol string) { - ff.registerSubscription(id, ft, protocol, false) -} - -func (ff *Filters) registerSubscription(id SubscriptionID, ft FilterType, protocol string, enableTimeout bool) { - sub, ok := ff.trackedSub(id, ft) - if !ok { - // Skip metrics for a missing subscription — nothing would ever decrement them. - return - } - sub.SetProtocol(protocol) - if enableTimeout { - sub.EnableTimeout() - } - ff.logger.Debug("[rpc] [filters] registered subscription", "type", ft, "id", id, "protocol", protocol, "evictable", enableTimeout) - ff.incrementMetrics(ft, protocol) +func (ff *Filters) registerSubscription(id SubscriptionID, ft FilterType, tracker SubTracker) { + ff.trackedSubs.Put(id, trackedSub{ft: ft, tracker: tracker}) + ff.logger.Debug("[rpc] [filters] registered subscription", "type", ft, "id", id, "protocol", tracker.Protocol()) + ff.incrementMetrics(ft, tracker.Protocol()) } // subscribeToPendingTransactions subscribes to pending transactions using the given transaction pool client. @@ -584,10 +527,11 @@ func (ff *Filters) HandlePendingLogs(reply *txpoolproto.OnPendingLogsReply) { // SubscribeNewHeads subscribes to new block headers and returns a channel to receive the headers // and a subscription ID to manage the subscription. -func (ff *Filters) SubscribeNewHeads(size int) (<-chan *types.Header, HeadsSubID) { +func (ff *Filters) SubscribeNewHeads(size int, protocol SubProtocol) (<-chan *types.Header, HeadsSubID) { id := HeadsSubID(generateSubscriptionID()) - sub := newChanSub[*types.Header](size) + sub := newChanSub[*types.Header](size, protocol) ff.headsSubs.Put(id, sub) + ff.registerSubscription(SubscriptionID(id), FilterTypeHeads, sub) return sub.ch, id } @@ -609,7 +553,11 @@ func (ff *Filters) UnsubscribeHeads(id HeadsSubID) bool { } func (ff *Filters) unsubscribeHeadsInternal(id HeadsSubID) bool { - return unsubscribeSubInternal(ff.headsSubs, ff.pendingHeadsStores, id) + if !unsubscribeSubInternal(ff.headsSubs, ff.pendingHeadsStores, id) { + return false + } + ff.trackedSubs.Delete(SubscriptionID(id)) + return true } // unsubscribeSubInternal tears down a subscription and its buffered store without @@ -632,7 +580,7 @@ func unsubscribeSubInternal[K comparable, T, S any](subs *concurrent.SyncMap[K, // and a subscription ID to manage the subscription. It uses the specified filter criteria. func (ff *Filters) SubscribePendingLogs(size int) (<-chan types.Logs, PendingLogsSubID) { id := PendingLogsSubID(generateSubscriptionID()) - sub := newChanSub[types.Logs](size) + sub := newChanSub[types.Logs](size, "") ff.pendingLogsSubs.Put(id, sub) return sub.ch, id } @@ -653,7 +601,7 @@ func (ff *Filters) UnsubscribePendingLogs(id PendingLogsSubID) bool { // and a subscription ID to manage the subscription. func (ff *Filters) SubscribePendingBlock(size int) (<-chan *types.Block, PendingBlockSubID) { id := PendingBlockSubID(generateSubscriptionID()) - sub := newChanSub[*types.Block](size) + sub := newChanSub[*types.Block](size, "") ff.pendingBlockSubs.Put(id, sub) return sub.ch, id } @@ -672,10 +620,11 @@ func (ff *Filters) UnsubscribePendingBlock(id PendingBlockSubID) bool { // SubscribePendingTxs subscribes to pending transactions and returns a channel to receive the transactions // and a subscription ID to manage the subscription. -func (ff *Filters) SubscribePendingTxs(size int) (<-chan []types.Transaction, PendingTxsSubID) { +func (ff *Filters) SubscribePendingTxs(size int, protocol SubProtocol) (<-chan []types.Transaction, PendingTxsSubID) { id := PendingTxsSubID(generateSubscriptionID()) - sub := newChanSub[[]types.Transaction](size) + sub := newChanSub[[]types.Transaction](size, protocol) ff.pendingTxsSubs.Put(id, sub) + ff.registerSubscription(SubscriptionID(id), FilterTypePendingTxs, sub) return sub.ch, id } @@ -697,13 +646,17 @@ func (ff *Filters) UnsubscribePendingTxs(id PendingTxsSubID) bool { } func (ff *Filters) unsubscribePendingTxsInternal(id PendingTxsSubID) bool { - return unsubscribeSubInternal(ff.pendingTxsSubs, ff.pendingTxsStores, id) + if !unsubscribeSubInternal(ff.pendingTxsSubs, ff.pendingTxsStores, id) { + return false + } + ff.trackedSubs.Delete(SubscriptionID(id)) + return true } // SubscribeReceipts subscribes to transaction receipts and returns a channel to receive the receipts // and a subscription ID to manage the subscription. func (ff *Filters) SubscribeReceipts(size int, criteria filters.ReceiptsFilterCriteria) (<-chan *remoteproto.SubscribeReceiptsReply, ReceiptsSubID) { - sub := newChanSub[*remoteproto.SubscribeReceiptsReply](size) + sub := newChanSub[*remoteproto.SubscribeReceiptsReply](size, "") id := ff.receiptsSubs.insertReceiptsFilter(sub, criteria.TransactionHashes, ff.config.RpcSubscriptionFiltersMaxLogs) if err := ff.sendReceiptsFilterUpdate(); err != nil { ff.logger.Warn("Could not update remote receipts filter", "err", err) @@ -745,8 +698,8 @@ func (ff *Filters) sendReceiptsFilterUpdate() error { // SubscribeLogs subscribes to logs using the specified filter criteria and returns a channel to receive the logs // and a subscription ID to manage the subscription. -func (ff *Filters) SubscribeLogs(size int, criteria filters.FilterCriteria) (<-chan *types.Log, LogsSubID) { - sub := newChanSub[*types.Log](size) +func (ff *Filters) SubscribeLogs(size int, criteria filters.FilterCriteria, protocol SubProtocol) (<-chan *types.Log, LogsSubID) { + sub := newChanSub[*types.Log](size, protocol) id, f := ff.logsSubs.insertLogsFilter(sub) // Initialize address and topic maps @@ -813,9 +766,11 @@ func (ff *Filters) SubscribeLogs(size int, criteria filters.FilterCriteria) (<-c if err := loaded.(func(*remoteproto.LogsFilterRequest) error)(lfr); err != nil { ff.logger.Warn("Could not update remote logs filter", "err", err) ff.logsSubs.removeLogsFilter(id) + return sub.ch, id } } + ff.registerSubscription(SubscriptionID(id), FilterTypeLogs, sub) return sub.ch, id } @@ -826,35 +781,16 @@ func (ff *Filters) loadLogsRequester() any { return ff.logsRequestor.Load() } -func (ff *Filters) HasSubscription(id LogsSubID) bool { - return ff.logsSubs.hasLogsFilter(id) -} - -// HasHeadsSubscription returns true if a heads (new block headers) subscription exists for the given ID. -func (ff *Filters) HasHeadsSubscription(id HeadsSubID) bool { - _, ok := ff.headsSubs.Get(id) - return ok -} - -// HasPendingTxsSubscription returns true if a pending transactions subscription exists for the given ID. -func (ff *Filters) HasPendingTxsSubscription(id PendingTxsSubID) bool { - _, ok := ff.pendingTxsSubs.Get(id) - return ok -} - // UnsubscribeLogs unsubscribes from logs using the given subscription ID. // It returns true if the unsubscription was successful, otherwise false. func (ff *Filters) UnsubscribeLogs(id LogsSubID) bool { - // Get protocol before unsubscribing - var protocol string filter, ok := ff.logsSubs.logsFilters.Get(id) - if ok && filter.sender != nil { - protocol = filter.sender.Protocol() - } if !ok { ff.logger.Debug("[rpc] [filters] unsubscribe logs filter not found", "id", id) + return false } - if !ff.unsubscribeLogsInternal(id) { + protocol := filter.sender.Protocol() + if !ff.removeLogsSubscription(id, true) { return false } ff.logger.Debug("[rpc] [filters] unsubscribed logs filter", "id", id, "protocol", protocol) @@ -862,18 +798,21 @@ func (ff *Filters) UnsubscribeLogs(id LogsSubID) bool { return true } -// unsubscribeLogsInternal is the logs counterpart of unsubscribeSubInternal (metrics -// stay with the caller); logs additionally push the shrunken aggregate to the remote. -func (ff *Filters) unsubscribeLogsInternal(id LogsSubID) bool { +// removeLogsSubscription is the logs counterpart of unsubscribeSubInternal (metrics +// stay with the caller); logs additionally push the shrunken aggregate to the remote, +// which eviction batches into one update per cycle via pushRemote=false. The store +// and tracking entry are released regardless of whether the remote update succeeds — +// a failed update only means the upstream keeps sending logs we'll now discard, it +// does not reanimate the local subscription. +func (ff *Filters) removeLogsSubscription(id LogsSubID, pushRemote bool) bool { if !ff.logsSubs.removeLogsFilter(id) { return false } - // The local filter has been removed; the log store and metric must be released - // regardless of whether the remote-filter update succeeds. A failed remote update - // only means the upstream will keep sending logs we'll now discard — it does not - // reanimate the local subscription. - ff.updateRemoteLogsFilter() + if pushRemote { + ff.updateRemoteLogsFilter() + } ff.deleteLogStore(id) + ff.trackedSubs.Delete(SubscriptionID(id)) return true } @@ -1175,15 +1114,18 @@ func (ff *Filters) WithTemporalOverlay(tx kv.TemporalTx) kv.TemporalTx { return tx } -func (ff *Filters) incrementMetrics(ft FilterType, protocol string) { - activeSubscriptionsGauge.WithLabelValues(string(ft), protocol).Inc() - getSubscriptionCounter("created", string(ft), protocol).Inc() +func (ff *Filters) incrementMetrics(ft FilterType, protocol SubProtocol) { + if protocol == "" { + return // internal subscription, not tracked + } + activeSubscriptionsGauge.WithLabelValues(string(ft), string(protocol)).Inc() + createdSubscriptionsCounter.WithLabelValues(string(ft), string(protocol)).Inc() } -func (ff *Filters) decrementMetrics(ft FilterType, protocol string) { +func (ff *Filters) decrementMetrics(ft FilterType, protocol SubProtocol) { if protocol == "" { - return // Not tracked + return // internal subscription, not tracked } - activeSubscriptionsGauge.WithLabelValues(string(ft), protocol).Dec() - getSubscriptionCounter("unsubscribed", string(ft), protocol).Inc() + activeSubscriptionsGauge.WithLabelValues(string(ft), string(protocol)).Dec() + unsubscribedSubscriptionsCounter.WithLabelValues(string(ft), string(protocol)).Inc() } diff --git a/rpc/rpchelper/filters_deadlock_test.go b/rpc/rpchelper/filters_deadlock_test.go index 637023b8b9d..46c167bc344 100644 --- a/rpc/rpchelper/filters_deadlock_test.go +++ b/rpc/rpchelper/filters_deadlock_test.go @@ -44,7 +44,7 @@ func TestFiltersDeadlock(t *testing.T) { ctx, cancel := context.WithCancel(context.TODO()) for i := 0; i < subCount; i++ { n := &sub{} - n.ch, n.id = f.SubscribeLogs(128, crit) + n.ch, n.id = f.SubscribeLogs(128, crit, "") // start a loop similar to an rpcdaemon subscription, that calls unsubscribe on return go func() { defer f.UnsubscribeLogs(n.id) diff --git a/rpc/rpchelper/filters_eviction_test.go b/rpc/rpchelper/filters_eviction_test.go index 26be5ffe24e..45ae53c189d 100644 --- a/rpc/rpchelper/filters_eviction_test.go +++ b/rpc/rpchelper/filters_eviction_test.go @@ -44,15 +44,17 @@ func backdateSub[T any](t *testing.T, sub Sub[T], age time.Duration) { cs.lock.Unlock() } +func (ff *Filters) hasTrackedSub(id SubscriptionID) bool { + _, ok := ff.trackedSubs.Get(id) + return ok +} + func TestEvictStaleSubscriptionsRemovesIdleFilters(t *testing.T) { f := newTestFilters(t) - headsCh, headsID := f.SubscribeNewHeads(8) - f.TrackSubscription(SubscriptionID(headsID), FilterTypeHeads, ProtocolHTTP) - txsCh, txsID := f.SubscribePendingTxs(8) - f.TrackSubscription(SubscriptionID(txsID), FilterTypePendingTxs, ProtocolHTTP) - logsCh, logsID := f.SubscribeLogs(8, filters.FilterCriteria{}) - f.TrackSubscription(SubscriptionID(logsID), FilterTypeLogs, ProtocolHTTP) + headsCh, headsID := f.SubscribeNewHeads(8, ProtocolHTTP) + txsCh, txsID := f.SubscribePendingTxs(8, ProtocolHTTP) + logsCh, logsID := f.SubscribeLogs(8, filters.FilterCriteria{}, ProtocolHTTP) headsSub, ok := f.headsSubs.Get(headsID) require.True(t, ok) @@ -66,9 +68,15 @@ func TestEvictStaleSubscriptionsRemovesIdleFilters(t *testing.T) { f.evictStaleSubscriptions(time.Hour) - require.False(t, f.HasHeadsSubscription(headsID)) - require.False(t, f.HasPendingTxsSubscription(txsID)) - require.False(t, f.HasSubscription(logsID)) + _, ok = f.headsSubs.Get(headsID) + require.False(t, ok) + _, ok = f.pendingTxsSubs.Get(txsID) + require.False(t, ok) + _, ok = f.logsSubs.logsFilters.Get(logsID) + require.False(t, ok) + require.False(t, f.hasTrackedSub(SubscriptionID(headsID))) + require.False(t, f.hasTrackedSub(SubscriptionID(txsID))) + require.False(t, f.hasTrackedSub(SubscriptionID(logsID))) _, open := <-headsCh require.False(t, open) @@ -81,10 +89,8 @@ func TestEvictStaleSubscriptionsRemovesIdleFilters(t *testing.T) { func TestTouchSubscriptionPreventsEviction(t *testing.T) { f := newTestFilters(t) - _, touchedID := f.SubscribePendingTxs(8) - f.TrackSubscription(SubscriptionID(touchedID), FilterTypePendingTxs, ProtocolHTTP) - _, idleID := f.SubscribePendingTxs(8) - f.TrackSubscription(SubscriptionID(idleID), FilterTypePendingTxs, ProtocolHTTP) + _, touchedID := f.SubscribePendingTxs(8, ProtocolHTTP) + _, idleID := f.SubscribePendingTxs(8, ProtocolHTTP) for _, id := range []PendingTxsSubID{touchedID, idleID} { sub, ok := f.pendingTxsSubs.Get(id) @@ -92,22 +98,26 @@ func TestTouchSubscriptionPreventsEviction(t *testing.T) { backdateSub(t, sub, 2*time.Hour) } - f.TouchSubscription(SubscriptionID(touchedID), FilterTypePendingTxs) + ft, ok := f.TouchSubscription(SubscriptionID(touchedID)) + require.True(t, ok) + require.Equal(t, FilterTypePendingTxs, ft) f.evictStaleSubscriptions(time.Hour) - require.True(t, f.HasPendingTxsSubscription(touchedID)) - require.False(t, f.HasPendingTxsSubscription(idleID)) + _, ok = f.pendingTxsSubs.Get(touchedID) + require.True(t, ok) + _, ok = f.pendingTxsSubs.Get(idleID) + require.False(t, ok) } func TestEvictStaleSubscriptionsSkipsWebSocketSubscriptions(t *testing.T) { f := newTestFilters(t) - _, id := f.SubscribeNewHeads(8) - f.SetSubscriptionProtocol(SubscriptionID(id), FilterTypeHeads, ProtocolWS) + _, id := f.SubscribeNewHeads(8, ProtocolWS) f.evictStaleSubscriptions(time.Nanosecond) - require.True(t, f.HasHeadsSubscription(id)) + _, ok := f.headsSubs.Get(id) + require.True(t, ok) } // A forwarding goroutine can drain channel-buffered items after its subscription is @@ -117,22 +127,25 @@ func TestAddAfterUnsubscribeDoesNotOrphanStore(t *testing.T) { f := newTestFilters(t) t.Run("heads", func(t *testing.T) { - _, id := f.SubscribeNewHeads(8) + _, id := f.SubscribeNewHeads(8, ProtocolHTTP) require.True(t, f.UnsubscribeHeads(id)) + require.False(t, f.hasTrackedSub(SubscriptionID(id))) f.AddPendingBlock(id, &types.Header{}) _, ok := f.pendingHeadsStores.Get(id) require.False(t, ok) }) t.Run("pendingTxs", func(t *testing.T) { - _, id := f.SubscribePendingTxs(8) + _, id := f.SubscribePendingTxs(8, ProtocolHTTP) require.True(t, f.UnsubscribePendingTxs(id)) + require.False(t, f.hasTrackedSub(SubscriptionID(id))) f.AddPendingTxs(id, []types.Transaction{}) _, ok := f.pendingTxsStores.Get(id) require.False(t, ok) }) t.Run("logs", func(t *testing.T) { - _, id := f.SubscribeLogs(8, filters.FilterCriteria{}) + _, id := f.SubscribeLogs(8, filters.FilterCriteria{}, ProtocolHTTP) require.True(t, f.UnsubscribeLogs(id)) + require.False(t, f.hasTrackedSub(SubscriptionID(id))) f.AddLogs(id, &types.Log{}) _, ok := f.logsStores.Get(id) require.False(t, ok) @@ -150,8 +163,7 @@ func TestLogsEvictionBatchesRemoteFilterUpdate(t *testing.T) { ids := make([]LogsSubID, 0, 3) for range 3 { - _, id := f.SubscribeLogs(8, filters.FilterCriteria{}) - f.TrackSubscription(SubscriptionID(id), FilterTypeLogs, ProtocolHTTP) + _, id := f.SubscribeLogs(8, filters.FilterCriteria{}, ProtocolHTTP) lf, ok := f.logsSubs.logsFilters.Get(id) require.True(t, ok) backdateSub(t, lf.sender, 2*time.Hour) @@ -162,7 +174,8 @@ func TestLogsEvictionBatchesRemoteFilterUpdate(t *testing.T) { f.evictStaleSubscriptions(time.Hour) for _, id := range ids { - require.False(t, f.HasSubscription(id)) + _, ok := f.logsSubs.logsFilters.Get(id) + require.False(t, ok) } require.EqualValues(t, 1, updates.Load()) } @@ -170,8 +183,7 @@ func TestLogsEvictionBatchesRemoteFilterUpdate(t *testing.T) { func TestConcurrentTouchAndEviction(t *testing.T) { f := newTestFilters(t) - _, id := f.SubscribePendingTxs(8) - f.TrackSubscription(SubscriptionID(id), FilterTypePendingTxs, ProtocolHTTP) + _, id := f.SubscribePendingTxs(8, ProtocolHTTP) stop := make(chan struct{}) var wg sync.WaitGroup @@ -183,7 +195,7 @@ func TestConcurrentTouchAndEviction(t *testing.T) { case <-stop: return default: - f.TouchSubscription(SubscriptionID(id), FilterTypePendingTxs) + f.TouchSubscription(SubscriptionID(id)) runtime.Gosched() } } @@ -197,5 +209,6 @@ func TestConcurrentTouchAndEviction(t *testing.T) { close(stop) wg.Wait() - require.True(t, f.HasPendingTxsSubscription(id)) + _, ok := f.pendingTxsSubs.Get(id) + require.True(t, ok) } diff --git a/rpc/rpchelper/filters_test.go b/rpc/rpchelper/filters_test.go index 08f0d77165e..fc35bfb73c5 100644 --- a/rpc/rpchelper/filters_test.go +++ b/rpc/rpchelper/filters_test.go @@ -84,7 +84,7 @@ func TestFilters_SingleSubscription_OnlyTopicsSubscribedAreBroadcast(t *testing. Topics: [][]common.Hash{{subbedTopic}}, } - outChan, _ := f.SubscribeLogs(10, criteria) + outChan, _ := f.SubscribeLogs(10, criteria, "") // now create a log for some other topic and distribute it log := createLog() @@ -118,7 +118,7 @@ func TestFilters_SingleSubscription_EmptyTopicsInCriteria_OnlyTopicsSubscribedAr Topics: [][]common.Hash{{nilTopic, subbedTopic, nilTopic}}, } - outChan, _ := f.SubscribeLogs(10, criteria) + outChan, _ := f.SubscribeLogs(10, criteria, "") // now create a log for some other topic and distribute it log := createLog() @@ -151,7 +151,7 @@ func TestFilters_SingleSubscription_TopicPositionWildcardIsPreserved(t *testing. Topics: [][]common.Hash{{topic0}, nil, {topic2}}, } - outChan, _ := f.SubscribeLogs(10, criteria) + outChan, _ := f.SubscribeLogs(10, criteria, "") matchingLog := createLog() matchingLog.Topics = []*typesproto.H256{ @@ -188,7 +188,7 @@ func TestFilters_SingleSubscription_WildcardOnlyTopicRowMatches(t *testing.T) { Topics: [][]common.Hash{nil}, } - outChan, _ := f.SubscribeLogs(10, criteria) + outChan, _ := f.SubscribeLogs(10, criteria, "") log1 := createLog() f.OnNewLogs(log1) @@ -218,8 +218,8 @@ func TestFilters_TwoSubscriptionsWithDifferentCriteria(t *testing.T) { Topics: [][]common.Hash{{topic1}}, } - chan1, _ := f.SubscribeLogs(256, criteria1) - chan2, _ := f.SubscribeLogs(256, criteria2) + chan1, _ := f.SubscribeLogs(256, criteria1, "") + chan2, _ := f.SubscribeLogs(256, criteria2, "") // now create a log for some other topic and distribute it log := createLog() @@ -263,9 +263,9 @@ func TestFilters_ThreeSubscriptionsWithDifferentCriteria(t *testing.T) { Topics: [][]common.Hash{}, } - chan1, _ := f.SubscribeLogs(256, criteria1) - chan2, _ := f.SubscribeLogs(256, criteria2) - chan3, _ := f.SubscribeLogs(256, criteria3) + chan1, _ := f.SubscribeLogs(256, criteria1, "") + chan2, _ := f.SubscribeLogs(256, criteria2, "") + chan3, _ := f.SubscribeLogs(256, criteria3, "") // now create a log for some other topic and distribute it log := createLog() @@ -332,7 +332,7 @@ func TestFilters_SubscribeLogsGeneratesCorrectLogFilterRequest(t *testing.T) { Addresses: []common.Address{}, Topics: [][]common.Hash{}, } - _, id1 := f.SubscribeLogs(1, criteria1) + _, id1 := f.SubscribeLogs(1, criteria1, "") // request should have all addresses and topics enabled if lastFilterRequest.AllAddresses == false { @@ -347,7 +347,7 @@ func TestFilters_SubscribeLogsGeneratesCorrectLogFilterRequest(t *testing.T) { Addresses: []common.Address{address1}, Topics: [][]common.Hash{}, } - _, id2 := f.SubscribeLogs(1, criteria2) + _, id2 := f.SubscribeLogs(1, criteria2, "") // request should have all addresses and all topics still and the new address if lastFilterRequest.AllAddresses == false { @@ -365,7 +365,7 @@ func TestFilters_SubscribeLogsGeneratesCorrectLogFilterRequest(t *testing.T) { Addresses: []common.Address{}, Topics: [][]common.Hash{{topic1}}, } - _, id3 := f.SubscribeLogs(1, criteria3) + _, id3 := f.SubscribeLogs(1, criteria3, "") // request should have all addresses and all topics as well as the previous address and new topic if lastFilterRequest.AllAddresses == false { @@ -450,7 +450,7 @@ func TestFilters_AddLogs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { config := FiltersConfig{RpcSubscriptionFiltersMaxLogs: tt.maxLogs} f := New(t.Context(), config, nil, nil, nil, func() {}, log.New(), nil) - _, logID := f.SubscribeLogs(8, filters.FilterCriteria{}) + _, logID := f.SubscribeLogs(8, filters.FilterCriteria{}, "") logEntry := &types.Log{Address: common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87")} for i := 0; i < tt.numToAdd; i++ { @@ -484,7 +484,7 @@ func TestFilters_AddPendingBlocks(t *testing.T) { t.Run(tt.name, func(t *testing.T) { config := FiltersConfig{RpcSubscriptionFiltersMaxHeaders: tt.maxHeaders} f := New(t.Context(), config, nil, nil, nil, func() {}, log.New(), nil) - _, blockID := f.SubscribeNewHeads(8) + _, blockID := f.SubscribeNewHeads(8, "") header := &types.Header{} for i := 0; i < tt.numToAdd; i++ { @@ -519,7 +519,7 @@ func TestFilters_AddPendingTxs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { config := FiltersConfig{RpcSubscriptionFiltersMaxTxs: tt.maxTxs} f := New(t.Context(), config, nil, nil, nil, func() {}, log.New(), nil) - _, txID := f.SubscribePendingTxs(8) + _, txID := f.SubscribePendingTxs(8, "") var txn types.Transaction = types.NewTransaction(0, common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87"), uint256.NewInt(10), 50000, uint256.NewInt(10), nil) txn, _ = txn.WithSignature(*types.LatestSignerForChainID(nil), common.Hex2Bytes("9bea4c4daac7c7c52e093e6a4c35dbbcf8856f1af7b059ba20253e70848d094f8a8fae537ce25ed8cb5af9adac3f141af69bd515bd2ba031522df09b97dd72b100")) diff --git a/rpc/rpchelper/logsfilter.go b/rpc/rpchelper/logsfilter.go index 32d097d6eaf..7b63b42ea79 100644 --- a/rpc/rpchelper/logsfilter.go +++ b/rpc/rpchelper/logsfilter.go @@ -102,8 +102,8 @@ func (a *LogsFilterAggregator) removeLogsFilter(filterId LogsSubID) bool { // hasLogsFilter checks if a log filter identified by filterId is present in the LogsFilterAggregator. func (a *LogsFilterAggregator) hasLogsFilter(filterId LogsSubID) bool { - a.logsFilterLock.Lock() - defer a.logsFilterLock.Unlock() + a.logsFilterLock.RLock() + defer a.logsFilterLock.RUnlock() _, ok := a.logsFilters.Get(filterId) return ok diff --git a/rpc/rpchelper/metrics.go b/rpc/rpchelper/metrics.go index d425e02aeb6..0afd12368a6 100644 --- a/rpc/rpchelper/metrics.go +++ b/rpc/rpchelper/metrics.go @@ -26,19 +26,11 @@ const ( protocolLabelName = "protocol" ) -// Protocol types for metrics labeling -const ( - ProtocolHTTP = "http" - ProtocolWS = "ws" -) - var ( - // activeSubscriptionsGauge tracks current active subscriptions by filter type and protocol - activeSubscriptionsGauge = metrics.GetOrCreateGaugeVec( - "subscriptions_active", - []string{filterLabelName, protocolLabelName}, - "Current number of active subscriptions", - ) + activeSubscriptionsGauge = metrics.GetOrCreateGaugeVec("subscriptions_active", []string{filterLabelName, protocolLabelName}, "Current number of active subscriptions") + createdSubscriptionsCounter = metrics.GetOrCreateCounterVec("subscriptions_created_total", []string{filterLabelName, protocolLabelName}, "Total number of subscriptions created") + unsubscribedSubscriptionsCounter = metrics.GetOrCreateCounterVec("subscriptions_unsubscribed_total", []string{filterLabelName, protocolLabelName}, "Total number of subscriptions removed by client unsubscribe or timeout eviction") + reapedSubscriptionsCounter = metrics.GetOrCreateCounterVec("subscriptions_reaped_total", []string{filterLabelName}, "Total number of idle subscriptions evicted by timeout") activeSubscriptionsLogsAllAddressesGauge = metrics.GetOrCreateGauge("subscriptions_logs_all_addresses") activeSubscriptionsLogsAllTopicsGauge = metrics.GetOrCreateGauge("subscriptions_logs_all_topics") @@ -46,18 +38,3 @@ var ( activeSubscriptionsLogsTopicsGauge = metrics.GetOrCreateGauge("subscriptions_logs_topics") activeSubscriptionsLogsClientGauge = metrics.GetOrCreateGaugeVec("subscriptions_logs_client", []string{clientLabelName}, "Current number of subscriptions by client") ) - -// getSubscriptionCounter returns a counter for subscription lifecycle events. -// pattern: subscriptions__total{filter="",protocol=""} -func getSubscriptionCounter(event, filterType, protocol string) metrics.Counter { - return metrics.GetOrCreateCounter( - `subscriptions_` + event + `_total{filter="` + filterType + `",protocol="` + protocol + `"}`, - ) -} - -// getReapedCounter returns a counter for reaped (timeout-evicted) subscriptions. -func getReapedCounter(filterType string) metrics.Counter { - return metrics.GetOrCreateCounter( - `subscriptions_reaped_total{filter="` + filterType + `"}`, - ) -} diff --git a/rpc/rpchelper/subscription.go b/rpc/rpchelper/subscription.go index 02863f91071..b9e33bc2e03 100644 --- a/rpc/rpchelper/subscription.go +++ b/rpc/rpchelper/subscription.go @@ -21,41 +21,55 @@ import ( "time" ) +// SubProtocol says how a subscription is consumed and thereby its lifecycle: HTTP +// polling filters are subject to timeout eviction, WS subscriptions end with their +// connection, and the empty value marks internal subscriptions that are neither +// evicted nor counted in metrics. +type SubProtocol string + +const ( + ProtocolHTTP SubProtocol = "http" + ProtocolWS SubProtocol = "ws" +) + // a simple interface for subscriptions for rpc helper type Sub[T any] interface { Send(T) Close() - CloseIfIdle(timeout time.Duration) bool // Close and report true when idle longer than timeout SubTracker } -// SubTracker is the element-type-independent tracking surface of a subscription, +// SubTracker is the element-type-independent surface of a subscription, // used for timeout-based eviction and metrics. type SubTracker interface { - Touch() // Reset last access time (for HTTP polling filters) - SetProtocol(protocol string) // Set protocol label for metrics - EnableTimeout() // Enable timeout tracking (HTTP only) - Protocol() string // Get protocol label for metrics + Touch() // reset the eviction deadline (HTTP polling filters) + CloseIfIdle(timeout time.Duration) bool // close and report true when idle longer than timeout + Protocol() SubProtocol } type chan_sub[T any] struct { - lock sync.Mutex // protects all fields of this struct + lock sync.Mutex // protects all mutable fields of this struct ch chan T closed bool - protocol string + protocol SubProtocol // immutable after construction // lastAccess is the last poll time for timeout eviction; zero means no // timeout tracking (push subscriptions die with their connection instead). lastAccess time.Time } // newChanSub - buffered channel -func newChanSub[T any](size int) *chan_sub[T] { +func newChanSub[T any](size int, protocol SubProtocol) *chan_sub[T] { if size < 8 { // set min size to 8 size = 8 } - return &chan_sub[T]{ - ch: make(chan T, size), + s := &chan_sub[T]{ + ch: make(chan T, size), + protocol: protocol, + } + if protocol == ProtocolHTTP { + s.lastAccess = time.Now() } + return s } func (s *chan_sub[T]) Send(x T) { s.lock.Lock() @@ -84,23 +98,8 @@ func (s *chan_sub[T]) closeLocked() { close(s.ch) } -// SetProtocol sets the protocol label for metrics tracking. -func (s *chan_sub[T]) SetProtocol(protocol string) { - s.lock.Lock() - defer s.lock.Unlock() - s.protocol = protocol -} - -// EnableTimeout enables timeout tracking for this subscription (HTTP polling filters). -// For WebSocket subscriptions, don't call this - they will never be evicted. -func (s *chan_sub[T]) EnableTimeout() { - s.lock.Lock() - defer s.lock.Unlock() - s.lastAccess = time.Now() -} - // Touch resets the last access time, preventing timeout eviction. -// Only effective if EnableTimeout was called. +// A no-op unless the subscription is tracked for eviction. func (s *chan_sub[T]) Touch() { s.lock.Lock() defer s.lock.Unlock() @@ -123,9 +122,6 @@ func (s *chan_sub[T]) CloseIfIdle(timeout time.Duration) bool { return true } -// Protocol returns the protocol label for metrics. Returns "" if SetProtocol was never called. -func (s *chan_sub[T]) Protocol() string { - s.lock.Lock() - defer s.lock.Unlock() +func (s *chan_sub[T]) Protocol() SubProtocol { return s.protocol } From e8041e5a793e51a490a8aa67250f412304e45fed Mon Sep 17 00:00:00 2001 From: yperbasis Date: Mon, 6 Jul 2026 16:41:20 +0200 Subject: [PATCH 11/11] rpc: fail loudly when a logs/receipts subscription can't update the remote filter Previously SubscribeLogs/SubscribeReceipts removed the just-created filter on a failed remote-filter update but still returned the (closed) channel and dead id, so eth_newFilter appeared to succeed while every subsequent poll returned 'filter not found', and WS/sync subscribers waited on a channel that would never fire. They now return an error and install nothing; eth_newFilter, eth_subscribe (logs, receipts), and eth_sendRawTransactionSync propagate it to the client. --- rpc/jsonrpc/eth_filters.go | 38 ++++++++++++------ rpc/jsonrpc/eth_filters_test.go | 2 +- rpc/jsonrpc/eth_subscribe_test.go | 2 +- rpc/jsonrpc/send_transaction.go | 5 ++- rpc/rpchelper/filters.go | 19 ++++----- rpc/rpchelper/filters_deadlock_test.go | 2 +- rpc/rpchelper/filters_eviction_test.go | 9 +++-- rpc/rpchelper/filters_subscribe_test.go | 53 +++++++++++++++++++++++++ rpc/rpchelper/filters_test.go | 46 ++++++++++----------- 9 files changed, 124 insertions(+), 52 deletions(-) create mode 100644 rpc/rpchelper/filters_subscribe_test.go diff --git a/rpc/jsonrpc/eth_filters.go b/rpc/jsonrpc/eth_filters.go index 03e19fa313c..2e6854bb69b 100644 --- a/rpc/jsonrpc/eth_filters.go +++ b/rpc/jsonrpc/eth_filters.go @@ -63,7 +63,10 @@ func (api *APIImpl) NewFilter(_ context.Context, crit filters.FilterCriteria) (s if api.filters == nil { return "", rpc.ErrNotificationsUnsupported } - logs, id := api.filters.SubscribeLogs(256, crit, rpchelper.ProtocolHTTP) + logs, id, err := api.filters.SubscribeLogs(256, crit, rpchelper.ProtocolHTTP) + if err != nil { + return "", err + } go func() { for lg := range logs { api.filters.AddLogs(id, lg) @@ -152,7 +155,7 @@ func (api *APIImpl) GetFilterLogs(_ context.Context, index string) ([]*types.Log // the channel closes or the client goes away. subscribe is called inside the goroutine // and must return the item channel plus an unsubscribe func. notify receives an emit // func that sends a payload to the client, logging on failure. -func subscribeRPC[T any](ctx context.Context, apiFilters *rpchelper.Filters, subscribe func() (<-chan T, func()), notify func(emit func(payload any), item T), closedWarn string) (*rpc.Subscription, error) { +func subscribeRPC[T any](ctx context.Context, apiFilters *rpchelper.Filters, subscribe func() (<-chan T, func(), error), notify func(emit func(payload any), item T), closedWarn string) (*rpc.Subscription, error) { if apiFilters == nil { return &rpc.Subscription{}, rpc.ErrNotificationsUnsupported } @@ -161,11 +164,14 @@ func subscribeRPC[T any](ctx context.Context, apiFilters *rpchelper.Filters, sub return &rpc.Subscription{}, rpc.ErrNotificationsUnsupported } + ch, unsubscribe, err := subscribe() + if err != nil { + return nil, err + } rpcSub := notifier.CreateSubscription() go func() { defer dbg.LogPanic() - ch, unsubscribe := subscribe() defer unsubscribe() emit := func(payload any) { @@ -193,9 +199,9 @@ func subscribeRPC[T any](ctx context.Context, apiFilters *rpchelper.Filters, sub // NewHeads send a notification each time a new (header) block is appended to the chain. func (api *APIImpl) NewHeads(ctx context.Context) (*rpc.Subscription, error) { return subscribeRPC(ctx, api.filters, - func() (<-chan *types.Header, func()) { + func() (<-chan *types.Header, func(), error) { headers, id := api.filters.SubscribeNewHeads(32, rpchelper.ProtocolWS) - return headers, func() { api.filters.UnsubscribeHeads(id) } + return headers, func() { api.filters.UnsubscribeHeads(id) }, nil }, func(emit func(payload any), h *types.Header) { if h != nil { @@ -217,9 +223,9 @@ func (api *APIImpl) NewPendingTransactionsWithBody(ctx context.Context) (*rpc.Su func (api *APIImpl) subscribePendingTransactions(ctx context.Context, chanSize int, fullTx bool) (*rpc.Subscription, error) { return subscribeRPC(ctx, api.filters, - func() (<-chan []types.Transaction, func()) { + func() (<-chan []types.Transaction, func(), error) { txsCh, id := api.filters.SubscribePendingTxs(chanSize, rpchelper.ProtocolWS) - return txsCh, func() { api.filters.UnsubscribePendingTxs(id) } + return txsCh, func() { api.filters.UnsubscribePendingTxs(id) }, nil }, func(emit func(payload any), txs []types.Transaction) { for _, t := range txs { @@ -238,9 +244,12 @@ func (api *APIImpl) subscribePendingTransactions(ctx context.Context, chanSize i // Logs send a notification each time a new log appears. func (api *APIImpl) Logs(ctx context.Context, crit filters.FilterCriteria) (*rpc.Subscription, error) { return subscribeRPC(ctx, api.filters, - func() (<-chan *types.Log, func()) { - logs, id := api.filters.SubscribeLogs(api.SubscribeLogsChannelSize, crit, rpchelper.ProtocolWS) - return logs, func() { api.filters.UnsubscribeLogs(id) } + func() (<-chan *types.Log, func(), error) { + logs, id, err := api.filters.SubscribeLogs(api.SubscribeLogsChannelSize, crit, rpchelper.ProtocolWS) + if err != nil { + return nil, nil, err + } + return logs, func() { api.filters.UnsubscribeLogs(id) }, nil }, func(emit func(payload any), h *types.Log) { if h != nil { @@ -253,9 +262,12 @@ func (api *APIImpl) Logs(ctx context.Context, crit filters.FilterCriteria) (*rpc // TransactionReceipts send a notification each time a new receipt appears. func (api *APIImpl) TransactionReceipts(ctx context.Context, crit filters.ReceiptsFilterCriteria) (*rpc.Subscription, error) { return subscribeRPC(ctx, api.filters, - func() (<-chan *remoteproto.SubscribeReceiptsReply, func()) { - receipts, id := api.filters.SubscribeReceipts(api.SubscribeLogsChannelSize, crit) - return receipts, func() { api.filters.UnsubscribeReceipts(id) } + func() (<-chan *remoteproto.SubscribeReceiptsReply, func(), error) { + receipts, id, err := api.filters.SubscribeReceipts(api.SubscribeLogsChannelSize, crit) + if err != nil { + return nil, nil, err + } + return receipts, func() { api.filters.UnsubscribeReceipts(id) }, nil }, func(emit func(payload any), protoReceipt *remoteproto.SubscribeReceiptsReply) { if protoReceipt != nil { diff --git a/rpc/jsonrpc/eth_filters_test.go b/rpc/jsonrpc/eth_filters_test.go index fdcf81c9bca..00457bf426a 100644 --- a/rpc/jsonrpc/eth_filters_test.go +++ b/rpc/jsonrpc/eth_filters_test.go @@ -144,7 +144,7 @@ func TestLogsSubscribeAndUnsubscribe_WithoutConcurrentMapIssue(t *testing.T) { for i := 0; i < 1000; i++ { wg.Add(1) go func(idx int) { - _, id := ff.SubscribeLogs(32, crit, "") + _, id, _ := ff.SubscribeLogs(32, crit, "") defer func() { time.Sleep(100 * time.Nanosecond) ff.UnsubscribeLogs(id) diff --git a/rpc/jsonrpc/eth_subscribe_test.go b/rpc/jsonrpc/eth_subscribe_test.go index efef8a8cfff..dd4c1943596 100644 --- a/rpc/jsonrpc/eth_subscribe_test.go +++ b/rpc/jsonrpc/eth_subscribe_test.go @@ -94,7 +94,7 @@ func TestEthSubscribeReceipts(t *testing.T) { } ff := rpchelper.New(ctx, rpchelper.DefaultFiltersConfig, backend, nil, nil, onNewSnapshot, m.Log, nil) subscriptionReadyWg.Wait() - newReceipts, id := ff.SubscribeReceipts(16, filters.ReceiptsFilterCriteria{ + newReceipts, id, _ := ff.SubscribeReceipts(16, filters.ReceiptsFilterCriteria{ TransactionHashes: []common.Hash{}, }) defer ff.UnsubscribeReceipts(id) diff --git a/rpc/jsonrpc/send_transaction.go b/rpc/jsonrpc/send_transaction.go index fa8c6b74257..3948a58b8fb 100644 --- a/rpc/jsonrpc/send_transaction.go +++ b/rpc/jsonrpc/send_transaction.go @@ -103,7 +103,10 @@ func (api *APIImpl) SendRawTransactionSync(ctx context.Context, encodedTx hexuti criteria := filters.ReceiptsFilterCriteria{ TransactionHashes: []common.Hash{hash}, } - receiptsCh, id := api.filters.SubscribeReceipts(128, criteria) + receiptsCh, id, err := api.filters.SubscribeReceipts(128, criteria) + if err != nil { + return nil, err + } defer api.filters.UnsubscribeReceipts(id) // Theoretically, we should subscribe *before* submitting the transaction, but then we couldn't filter by hash. diff --git a/rpc/rpchelper/filters.go b/rpc/rpchelper/filters.go index 6a2f94b8cdc..e2ed32fd5bb 100644 --- a/rpc/rpchelper/filters.go +++ b/rpc/rpchelper/filters.go @@ -654,15 +654,16 @@ func (ff *Filters) unsubscribePendingTxsInternal(id PendingTxsSubID) bool { } // SubscribeReceipts subscribes to transaction receipts and returns a channel to receive the receipts -// and a subscription ID to manage the subscription. -func (ff *Filters) SubscribeReceipts(size int, criteria filters.ReceiptsFilterCriteria) (<-chan *remoteproto.SubscribeReceiptsReply, ReceiptsSubID) { +// and a subscription ID to manage the subscription. When the remote filter update fails, no subscription +// is installed and the error is returned. +func (ff *Filters) SubscribeReceipts(size int, criteria filters.ReceiptsFilterCriteria) (<-chan *remoteproto.SubscribeReceiptsReply, ReceiptsSubID, error) { sub := newChanSub[*remoteproto.SubscribeReceiptsReply](size, "") id := ff.receiptsSubs.insertReceiptsFilter(sub, criteria.TransactionHashes, ff.config.RpcSubscriptionFiltersMaxLogs) if err := ff.sendReceiptsFilterUpdate(); err != nil { - ff.logger.Warn("Could not update remote receipts filter", "err", err) ff.receiptsSubs.removeReceiptsFilter(id) + return nil, "", fmt.Errorf("could not update remote receipts filter: %w", err) } - return sub.ch, id + return sub.ch, id, nil } // UnsubscribeReceipts unsubscribes from transaction receipts using the given subscription ID. @@ -697,8 +698,9 @@ func (ff *Filters) sendReceiptsFilterUpdate() error { } // SubscribeLogs subscribes to logs using the specified filter criteria and returns a channel to receive the logs -// and a subscription ID to manage the subscription. -func (ff *Filters) SubscribeLogs(size int, criteria filters.FilterCriteria, protocol SubProtocol) (<-chan *types.Log, LogsSubID) { +// and a subscription ID to manage the subscription. When the remote filter update fails, no subscription is +// installed and the error is returned. +func (ff *Filters) SubscribeLogs(size int, criteria filters.FilterCriteria, protocol SubProtocol) (<-chan *types.Log, LogsSubID, error) { sub := newChanSub[*types.Log](size, protocol) id, f := ff.logsSubs.insertLogsFilter(sub) @@ -764,14 +766,13 @@ func (ff *Filters) SubscribeLogs(size int, criteria filters.FilterCriteria, prot loaded := ff.loadLogsRequester() if loaded != nil { if err := loaded.(func(*remoteproto.LogsFilterRequest) error)(lfr); err != nil { - ff.logger.Warn("Could not update remote logs filter", "err", err) ff.logsSubs.removeLogsFilter(id) - return sub.ch, id + return nil, "", fmt.Errorf("could not update remote logs filter: %w", err) } } ff.registerSubscription(SubscriptionID(id), FilterTypeLogs, sub) - return sub.ch, id + return sub.ch, id, nil } // loadLogsRequester loads the current logs requester and returns it. diff --git a/rpc/rpchelper/filters_deadlock_test.go b/rpc/rpchelper/filters_deadlock_test.go index 46c167bc344..ff827e712f0 100644 --- a/rpc/rpchelper/filters_deadlock_test.go +++ b/rpc/rpchelper/filters_deadlock_test.go @@ -44,7 +44,7 @@ func TestFiltersDeadlock(t *testing.T) { ctx, cancel := context.WithCancel(context.TODO()) for i := 0; i < subCount; i++ { n := &sub{} - n.ch, n.id = f.SubscribeLogs(128, crit, "") + n.ch, n.id, _ = f.SubscribeLogs(128, crit, "") // start a loop similar to an rpcdaemon subscription, that calls unsubscribe on return go func() { defer f.UnsubscribeLogs(n.id) diff --git a/rpc/rpchelper/filters_eviction_test.go b/rpc/rpchelper/filters_eviction_test.go index 45ae53c189d..c608701a6a2 100644 --- a/rpc/rpchelper/filters_eviction_test.go +++ b/rpc/rpchelper/filters_eviction_test.go @@ -54,7 +54,8 @@ func TestEvictStaleSubscriptionsRemovesIdleFilters(t *testing.T) { headsCh, headsID := f.SubscribeNewHeads(8, ProtocolHTTP) txsCh, txsID := f.SubscribePendingTxs(8, ProtocolHTTP) - logsCh, logsID := f.SubscribeLogs(8, filters.FilterCriteria{}, ProtocolHTTP) + logsCh, logsID, err := f.SubscribeLogs(8, filters.FilterCriteria{}, ProtocolHTTP) + require.NoError(t, err) headsSub, ok := f.headsSubs.Get(headsID) require.True(t, ok) @@ -143,7 +144,8 @@ func TestAddAfterUnsubscribeDoesNotOrphanStore(t *testing.T) { require.False(t, ok) }) t.Run("logs", func(t *testing.T) { - _, id := f.SubscribeLogs(8, filters.FilterCriteria{}, ProtocolHTTP) + _, id, err := f.SubscribeLogs(8, filters.FilterCriteria{}, ProtocolHTTP) + require.NoError(t, err) require.True(t, f.UnsubscribeLogs(id)) require.False(t, f.hasTrackedSub(SubscriptionID(id))) f.AddLogs(id, &types.Log{}) @@ -163,7 +165,8 @@ func TestLogsEvictionBatchesRemoteFilterUpdate(t *testing.T) { ids := make([]LogsSubID, 0, 3) for range 3 { - _, id := f.SubscribeLogs(8, filters.FilterCriteria{}, ProtocolHTTP) + _, id, err := f.SubscribeLogs(8, filters.FilterCriteria{}, ProtocolHTTP) + require.NoError(t, err) lf, ok := f.logsSubs.logsFilters.Get(id) require.True(t, ok) backdateSub(t, lf.sender, 2*time.Hour) diff --git a/rpc/rpchelper/filters_subscribe_test.go b/rpc/rpchelper/filters_subscribe_test.go new file mode 100644 index 00000000000..de153d1c849 --- /dev/null +++ b/rpc/rpchelper/filters_subscribe_test.go @@ -0,0 +1,53 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package rpchelper + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/node/gointerfaces/remoteproto" + "github.com/erigontech/erigon/rpc/filters" +) + +// A filter id must never be handed out for a subscription that is not installed: +// when the remote filter update fails, Subscribe* reports the error instead. +func TestSubscribeLogsRemoteUpdateFailureReturnsError(t *testing.T) { + f := newTestFilters(t) + f.logsRequestor.Store(func(*remoteproto.LogsFilterRequest) error { + return errors.New("remote logs source unavailable") + }) + + _, id, err := f.SubscribeLogs(8, filters.FilterCriteria{}, ProtocolHTTP) + require.Error(t, err) + _, ok := f.logsSubs.logsFilters.Get(id) + require.False(t, ok) + require.False(t, f.hasTrackedSub(SubscriptionID(id))) +} + +func TestSubscribeReceiptsRemoteUpdateFailureReturnsError(t *testing.T) { + f := newTestFilters(t) + f.receiptsRequestor.Store(func(*remoteproto.ReceiptsFilterRequest) error { + return errors.New("remote receipts source unavailable") + }) + + _, id, err := f.SubscribeReceipts(8, filters.ReceiptsFilterCriteria{}) + require.Error(t, err) + require.False(t, f.receiptsSubs.removeReceiptsFilter(id)) +} diff --git a/rpc/rpchelper/filters_test.go b/rpc/rpchelper/filters_test.go index fc35bfb73c5..9dc3f89537e 100644 --- a/rpc/rpchelper/filters_test.go +++ b/rpc/rpchelper/filters_test.go @@ -84,7 +84,7 @@ func TestFilters_SingleSubscription_OnlyTopicsSubscribedAreBroadcast(t *testing. Topics: [][]common.Hash{{subbedTopic}}, } - outChan, _ := f.SubscribeLogs(10, criteria, "") + outChan, _, _ := f.SubscribeLogs(10, criteria, "") // now create a log for some other topic and distribute it log := createLog() @@ -118,7 +118,7 @@ func TestFilters_SingleSubscription_EmptyTopicsInCriteria_OnlyTopicsSubscribedAr Topics: [][]common.Hash{{nilTopic, subbedTopic, nilTopic}}, } - outChan, _ := f.SubscribeLogs(10, criteria, "") + outChan, _, _ := f.SubscribeLogs(10, criteria, "") // now create a log for some other topic and distribute it log := createLog() @@ -151,7 +151,7 @@ func TestFilters_SingleSubscription_TopicPositionWildcardIsPreserved(t *testing. Topics: [][]common.Hash{{topic0}, nil, {topic2}}, } - outChan, _ := f.SubscribeLogs(10, criteria, "") + outChan, _, _ := f.SubscribeLogs(10, criteria, "") matchingLog := createLog() matchingLog.Topics = []*typesproto.H256{ @@ -188,7 +188,7 @@ func TestFilters_SingleSubscription_WildcardOnlyTopicRowMatches(t *testing.T) { Topics: [][]common.Hash{nil}, } - outChan, _ := f.SubscribeLogs(10, criteria, "") + outChan, _, _ := f.SubscribeLogs(10, criteria, "") log1 := createLog() f.OnNewLogs(log1) @@ -218,8 +218,8 @@ func TestFilters_TwoSubscriptionsWithDifferentCriteria(t *testing.T) { Topics: [][]common.Hash{{topic1}}, } - chan1, _ := f.SubscribeLogs(256, criteria1, "") - chan2, _ := f.SubscribeLogs(256, criteria2, "") + chan1, _, _ := f.SubscribeLogs(256, criteria1, "") + chan2, _, _ := f.SubscribeLogs(256, criteria2, "") // now create a log for some other topic and distribute it log := createLog() @@ -263,9 +263,9 @@ func TestFilters_ThreeSubscriptionsWithDifferentCriteria(t *testing.T) { Topics: [][]common.Hash{}, } - chan1, _ := f.SubscribeLogs(256, criteria1, "") - chan2, _ := f.SubscribeLogs(256, criteria2, "") - chan3, _ := f.SubscribeLogs(256, criteria3, "") + chan1, _, _ := f.SubscribeLogs(256, criteria1, "") + chan2, _, _ := f.SubscribeLogs(256, criteria2, "") + chan3, _, _ := f.SubscribeLogs(256, criteria3, "") // now create a log for some other topic and distribute it log := createLog() @@ -332,7 +332,7 @@ func TestFilters_SubscribeLogsGeneratesCorrectLogFilterRequest(t *testing.T) { Addresses: []common.Address{}, Topics: [][]common.Hash{}, } - _, id1 := f.SubscribeLogs(1, criteria1, "") + _, id1, _ := f.SubscribeLogs(1, criteria1, "") // request should have all addresses and topics enabled if lastFilterRequest.AllAddresses == false { @@ -347,7 +347,7 @@ func TestFilters_SubscribeLogsGeneratesCorrectLogFilterRequest(t *testing.T) { Addresses: []common.Address{address1}, Topics: [][]common.Hash{}, } - _, id2 := f.SubscribeLogs(1, criteria2, "") + _, id2, _ := f.SubscribeLogs(1, criteria2, "") // request should have all addresses and all topics still and the new address if lastFilterRequest.AllAddresses == false { @@ -365,7 +365,7 @@ func TestFilters_SubscribeLogsGeneratesCorrectLogFilterRequest(t *testing.T) { Addresses: []common.Address{}, Topics: [][]common.Hash{{topic1}}, } - _, id3 := f.SubscribeLogs(1, criteria3, "") + _, id3, _ := f.SubscribeLogs(1, criteria3, "") // request should have all addresses and all topics as well as the previous address and new topic if lastFilterRequest.AllAddresses == false { @@ -450,7 +450,7 @@ func TestFilters_AddLogs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { config := FiltersConfig{RpcSubscriptionFiltersMaxLogs: tt.maxLogs} f := New(t.Context(), config, nil, nil, nil, func() {}, log.New(), nil) - _, logID := f.SubscribeLogs(8, filters.FilterCriteria{}, "") + _, logID, _ := f.SubscribeLogs(8, filters.FilterCriteria{}, "") logEntry := &types.Log{Address: common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87")} for i := 0; i < tt.numToAdd; i++ { @@ -602,7 +602,7 @@ func TestFilters_SingleReceiptsSubscription_OnlyTransactionHashesSubscribedAreBr TransactionHashes: []common.Hash{txHash1}, } - outChan, _ := f.SubscribeReceipts(10, criteria) + outChan, _, _ := f.SubscribeReceipts(10, criteria) // Create a receipt for a different transaction hash receipt := createReceipt(txHash2) @@ -633,7 +633,7 @@ func TestFilters_ReceiptsSubscription_EmptyFilterSubscribesToAll(t *testing.T) { TransactionHashes: []common.Hash{}, } - outChan, _ := f.SubscribeReceipts(10, criteria) + outChan, _, _ := f.SubscribeReceipts(10, criteria) // Any receipt should be received receipt1 := createReceipt(txHash1) @@ -665,8 +665,8 @@ func TestFilters_TwoReceiptsSubscriptionsWithDifferentCriteria(t *testing.T) { TransactionHashes: []common.Hash{txHash1}, } - chan1, _ := f.SubscribeReceipts(256, criteria1) - chan2, _ := f.SubscribeReceipts(256, criteria2) + chan1, _, _ := f.SubscribeReceipts(256, criteria1) + chan2, _, _ := f.SubscribeReceipts(256, criteria2) // Create a receipt for txHash2 receipt := createReceipt(txHash2) @@ -708,9 +708,9 @@ func TestFilters_ThreeReceiptsSubscriptionsWithDifferentCriteria(t *testing.T) { TransactionHashes: []common.Hash{txHash1, txHash2}, } - chan1, _ := f.SubscribeReceipts(256, criteria1) - chan2, _ := f.SubscribeReceipts(256, criteria2) - chan3, _ := f.SubscribeReceipts(256, criteria3) + chan1, _, _ := f.SubscribeReceipts(256, criteria1) + chan2, _, _ := f.SubscribeReceipts(256, criteria2) + chan3, _, _ := f.SubscribeReceipts(256, criteria3) // Receipt for txHash3 (not subscribed by chan2 or chan3) receipt := createReceipt(txHash3) @@ -774,7 +774,7 @@ func TestFilters_SubscribeReceiptsGeneratesCorrectReceiptsFilterRequest(t *testi criteria1 := filters.ReceiptsFilterCriteria{ TransactionHashes: []common.Hash{}, } - _, id1 := f.SubscribeReceipts(1, criteria1) + _, id1, _ := f.SubscribeReceipts(1, criteria1) // Request should have AllTransactions=true and empty TransactionHashes if !lastFilterRequest.AllTransactions { @@ -788,7 +788,7 @@ func TestFilters_SubscribeReceiptsGeneratesCorrectReceiptsFilterRequest(t *testi criteria2 := filters.ReceiptsFilterCriteria{ TransactionHashes: []common.Hash{txHash1}, } - _, id2 := f.SubscribeReceipts(1, criteria2) + _, id2, _ := f.SubscribeReceipts(1, criteria2) // Request should have AllTransactions=true and include txHash1 // Backend uses OR logic: send if (AllTransactions OR hash matches) @@ -817,7 +817,7 @@ func TestFilters_SubscribeReceiptsGeneratesCorrectReceiptsFilterRequest(t *testi criteria3 := filters.ReceiptsFilterCriteria{ TransactionHashes: []common.Hash{txHash2, txHash3}, } - _, id3 := f.SubscribeReceipts(1, criteria3) + _, id3, _ := f.SubscribeReceipts(1, criteria3) // Request should have all three transaction hashes if len(lastFilterRequest.TransactionHashes) != 3 {