eth/filters, cmd: add config of eth_getLogs address limit #33320 #32327 by gzliudan · Pull Request #1961 · XinFinOrg/XDPoSChain

gzliudan · 2026-01-19T06:17:09Z

Proposed changes

Ref: ethereum#32327

Types of changes

What types of changes does your code introduce to XDC network?
Put an ✅ in the boxes that apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation Update (if none of the other choices apply)
Regular KTLO or any of the maintaince work. e.g code style
CICD Improvement

Impacted Components

Which part of the codebase this PR will touch base on,

Put an ✅ in the boxes that apply

Checklist

Put an ✅ in the boxes once you have confirmed below actions (or provide reasons on not doing so) that

This PR has sufficient test coverage (unit/integration test) OR I have provided reason in the PR description for not having test coverage
Provide an end-to-end test plan in the PR description on how to manually test it on the devnet/testnet.
Tested the backwards compatibility.
Tested with XDC nodes running this version co-exist with those running the previous version.
Relevant documentation has been updated as part of this PR
N/A

coderabbitai · 2026-01-19T06:17:18Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

🔍 Trigger a full review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

…33320

Copilot

Pull request overview

Adds a configurable cap for eth_getLogs (and log subscriptions) to limit the number of addresses/topic-alternatives accepted in filter criteria, exposed via node config and CLI.

Changes:

Introduce LogQueryLimit in the filters system and ETH config, with TOML + CLI wiring.
Enforce the limit in eth_getLogs and log subscriptions, updating error handling and tests accordingly.
Update tests to validate the new limit behavior and remove legacy hard-coded max-address unmarshalling coverage.

Reviewed changes

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

Show a summary per file

File	Description
eth/filters/filter_system.go	Adds `LogQueryLimit` to filter system config and applies it in `SubscribeLogs`.
eth/filters/api.go	Adds API-level enforcement for `LogQueryLimit` in `GetLogs` and updates error types.
eth/filters/filter_system_test.go	Updates filter system tests to use the configured limit instead of a fixed constant.
eth/filters/api_test.go	Removes legacy test that depended on the old hard-coded max-address limit during JSON unmarshal.
eth/ethconfig/config.go	Adds `LogQueryLimit` to ETH config defaults and struct documentation.
eth/ethconfig/gen_config.go	Extends TOML marshal/unmarshal support for the new config field.
cmd/utils/flags.go	Adds CLI flag + config plumbing for `LogQueryLimit` and passes it into filter system config (in this helper).
cmd/XDC/main.go	Registers the new CLI flag in the XDC command.

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

Copilot · 2026-01-27T03:59:20Z

+	if api.logQueryLimit != 0 {
+		if len(crit.Addresses) > api.logQueryLimit {
+			return nil, errExceedLogQueryLimit
+		}
+		for _, topics := range crit.Topics {


Same as in SubscribeLogs: using api.logQueryLimit != 0 means negative values would reject even empty addresses/topics. Treat only positive values as an active cap (e.g., > 0) or validate the config before constructing the API.

Copilot · 2026-01-27T03:59:21Z

+	errExceedMaxTopics     = errors.New("exceed max topics")
+	errExceedLogQueryLimit = errors.New("exceed max addresses or topics per search position")


errInvalidTopic/errInvalidBlockRange/errBlockHashWithRange now return JSON-RPC “invalid params” (-32602) via invalidParamsError, but errExceedMaxTopics / errExceedLogQueryLimit are still plain errors.New and will be reported with the default RPC error code. These are also invalid-params cases; consider defining them via invalidParamsErr(...) for consistent JSON-RPC error codes.

Suggested change

errExceedMaxTopics = errors.New("exceed max topics")

errExceedLogQueryLimit = errors.New("exceed max addresses or topics per search position")

errExceedMaxTopics = invalidParamsErr("exceed max topics")

errExceedLogQueryLimit = invalidParamsErr("exceed max addresses or topics per search position")

Copilot · 2026-01-27T03:59:21Z

+	RPCGlobalLogQueryLimit = &cli.IntFlag{
+		Name:     "rpc.logquerylimit",
+		Usage:    "Maximum number of alternative addresses or topics allowed per search position in eth_getLogs filter criteria (0 = no cap)",
+		Value:    ethconfig.Defaults.LogQueryLimit,
+		Category: flags.APICategory,
+	}


Flag naming is inconsistent with the established pattern in this file where the primary name uses dashes and the dot-form is provided as an alias (e.g. cache-blocklogs/cache.blocklogs, rpc-txfeecap/rpc.txfeecap). Consider renaming the primary flag to rpc-logquerylimit and adding rpc.logquerylimit as an alias to match existing conventions.

Copilot · 2026-01-27T03:59:21Z

 	filterSystem := filters.NewFilterSystem(backend, filters.Config{
-		LogCacheSize: ethcfg.FilterLogCacheSize,
+		LogCacheSize:  ethcfg.FilterLogCacheSize,
+		LogQueryLimit: ethcfg.LogQueryLimit,
 	})


This wires ethcfg.LogQueryLimit into the filter system created here, but there are other filters.NewFilterSystem call sites that still don’t pass the new field (e.g. eth/backend.go:429). If those code paths are used to expose the eth filter APIs, the new rpc.logquerylimit/TOML setting won’t have any effect. Consider updating all production call sites to propagate LogQueryLimit (or centralizing FilterSystem creation).

Copilot · 2026-01-27T03:59:21Z

-	Timeout      time.Duration // how long filters stay active (default: 5min)
+	LogCacheSize  int           // maximum number of cached blocks (default: 32)
+	Timeout       time.Duration // how long filters stay active (default: 5min)
+	LogQueryLimit int           // maximum number of addresses allowed in filter criteria (default: 1000)


LogQueryLimit’s comment is inaccurate: the limit is enforced for both the number of addresses and the number of topic alternatives per topic-position, and 0 is treated as “no cap” by the callers. Please update the field comment to match the actual semantics (and avoid claiming a default of 1000 here unless withDefaults enforces it).

Suggested change

LogQueryLimit int // maximum number of addresses allowed in filter criteria (default: 1000)

LogQueryLimit int // maximum number of addresses and topic alternatives per position allowed in log filter criteria; 0 means no limit

Copilot · 2026-01-27T03:59:22Z

+	if es.sys.cfg.LogQueryLimit != 0 {
+		if len(crit.Addresses) > es.sys.cfg.LogQueryLimit {
+			return nil, errExceedLogQueryLimit
+		}
+		for _, topics := range crit.Topics {


The limit guard uses LogQueryLimit != 0. If a user misconfigures this to a negative number (possible via TOML/CLI), len(...) > LogQueryLimit will be true even for empty slices, effectively breaking log subscriptions. Consider treating only positive values as an active cap (e.g., > 0) and/or validating the config on startup.

gzliudan requested review from AnilChinchawale, anunay-xin, benjamin202410, liam-lai and wanwiset25 January 19, 2026 06:18

gzliudan force-pushed the getLogs-address-limit branch from 62d7470 to 89a2f90 Compare January 19, 2026 06:56

gzliudan changed the title ~~eth/filters, cmd: add config of eth_getLogs address limit #32327~~ eth/filters, cmd: add config of eth_getLogs address limit #33320 #32327 Jan 19, 2026

gzliudan force-pushed the getLogs-address-limit branch from 89a2f90 to 138bfeb Compare January 19, 2026 07:10

anunay-xin approved these changes Jan 19, 2026

View reviewed changes

gzliudan force-pushed the getLogs-address-limit branch from 138bfeb to bf63562 Compare January 19, 2026 07:40

wanwiset25 approved these changes Jan 26, 2026

View reviewed changes

gzliudan added 2 commits January 27, 2026 11:47

eth/filters: change error code for invalid parameter errors ethereum#…

2e85502

…33320

eth/filters, cmd: add config of eth_getLogs address limit ethereum#32327

9439af8

Copilot AI review requested due to automatic review settings January 27, 2026 03:51

gzliudan force-pushed the getLogs-address-limit branch from bf63562 to 9439af8 Compare January 27, 2026 03:51

Copilot started reviewing on behalf of gzliudan January 27, 2026 03:51 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

AnilChinchawale approved these changes Jan 29, 2026

View reviewed changes

AnilChinchawale merged commit 4768d00 into XinFinOrg:dev-upgrade Jan 29, 2026
19 checks passed

gzliudan deleted the getLogs-address-limit branch January 29, 2026 06:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

eth/filters, cmd: add config of eth_getLogs address limit #33320 #32327#1961

eth/filters, cmd: add config of eth_getLogs address limit #33320 #32327#1961
AnilChinchawale merged 2 commits intoXinFinOrg:dev-upgradefrom
gzliudan:getLogs-address-limit

gzliudan commented Jan 19, 2026

Uh oh!

coderabbitai Bot commented Jan 19, 2026 •

edited

Loading

Review skipped

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		errExceedMaxTopics = errors.New("exceed max topics")
		errExceedLogQueryLimit = errors.New("exceed max addresses or topics per search position")

	LogQueryLimit int // maximum number of addresses allowed in filter criteria (default: 1000)
	LogQueryLimit int // maximum number of addresses and topic alternatives per position allowed in log filter criteria; 0 means no limit

Conversation

gzliudan commented Jan 19, 2026

Proposed changes

Types of changes

Impacted Components

Checklist

Uh oh!

coderabbitai Bot commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

coderabbitai Bot commented Jan 19, 2026 •

edited

Loading