Skip to content

eth/filters: rangeLogs should error on invalid block range#33763

Merged
s1na merged 2 commits intoethereum:masterfrom
vickkkkkyy:rangeLogs
Mar 18, 2026
Merged

eth/filters: rangeLogs should error on invalid block range#33763
s1na merged 2 commits intoethereum:masterfrom
vickkkkkyy:rangeLogs

Conversation

@vickkkkkyy
Copy link
Copy Markdown
Contributor

Ran into this while testing a monitoring script - passed fromBlock=100, toBlock=50 by mistake and got back an empty array. Took me way too long to figure out it was just my params being backwards lol.

Turns out updateChainView in the same file already returns errInvalidBlockRange for this case, so just making rangeLogs do the same thing.

rjl493456442
rjl493456442 previously approved these changes Feb 6, 2026
@vickkkkkyy
Copy link
Copy Markdown
Contributor Author

Greeting! is this good to process? cc @rjl493456442 and @s1na

@s1na s1na added this to the 1.17.2 milestone Mar 18, 2026
@s1na
Copy link
Copy Markdown
Contributor

s1na commented Mar 18, 2026

@vickkkkkyy tests are failing, please fix

--- FAIL: TestFiltersIndexed (3.62s)
    filter_test.go:391: test 12, expected error "", got "invalid block range params"
--- FAIL: TestFiltersHalfIndexed (3.93s)
    filter_test.go:391: test 12, expected error "", got "invalid block range params"
--- FAIL: TestFiltersUnindexed (1.00s)
    filter_test.go:391: test 12, expected error "", got "invalid block range params"
FAIL

@vickkkkkyy
Copy link
Copy Markdown
Contributor Author

@vickkkkkyy tests are failing, please fix

--- FAIL: TestFiltersIndexed (3.62s)
    filter_test.go:391: test 12, expected error "", got "invalid block range params"
--- FAIL: TestFiltersHalfIndexed (3.93s)
    filter_test.go:391: test 12, expected error "", got "invalid block range params"
--- FAIL: TestFiltersUnindexed (1.00s)
    filter_test.go:391: test 12, expected error "", got "invalid block range params"
FAIL

fixed

Copy link
Copy Markdown
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@s1na s1na merged commit 3341d8a into ethereum:master Mar 18, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants