Skip to content

Conversation

@ceyonur
Copy link
Contributor

@ceyonur ceyonur commented May 22, 2025

This introduces an error when the filter has both blockHash and fromBlock/toBlock, since these are mutually exclusive. Seems the tests were actually returning not found error, which went undetected since there was no check on the actual returned error in the test.

@ceyonur ceyonur requested a review from rjl493456442 as a code owner May 22, 2025 14:30
@fjl fjl changed the title filters: fix TestInvalidGetLogsRequest eth/filters: fix TestInvalidGetLogsRequest May 23, 2025
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these tests were just meant to check that FromBlock/ToBlock is mutually exclusive with BlockHash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but it does not actually return any error for these cases in the GetLogs (there is an if/else check)

@fjl fjl self-assigned this May 27, 2025
@fjl fjl force-pushed the fix-TestInvalidGetLogsRequest branch from 049d47b to 57d7022 Compare August 1, 2025 13:42
@fjl fjl changed the title eth/filters: fix TestInvalidGetLogsRequest eth/filters: fix error when blockHash is used with fromBlock/toBlock Aug 1, 2025
@fjl fjl added this to the 1.16.2 milestone Aug 1, 2025
@fjl fjl merged commit 038ff76 into ethereum:master Aug 1, 2025
3 of 5 checks passed
howjmay pushed a commit to iotaledger/go-ethereum that referenced this pull request Aug 27, 2025
…thereum#31877)

This introduces an error when the filter has both `blockHash` and
`fromBlock`/`toBlock`, since these are mutually exclusive. Seems the
tests were actually returning `not found` error, which went undetected
since there was no check on the actual returned error in the test.
maoueh pushed a commit to streamingfast/go-ethereum that referenced this pull request Sep 4, 2025
…thereum#31877)

This introduces an error when the filter has both `blockHash` and
`fromBlock`/`toBlock`, since these are mutually exclusive. Seems the
tests were actually returning `not found` error, which went undetected
since there was no check on the actual returned error in the test.
gballet pushed a commit to gballet/go-ethereum that referenced this pull request Sep 11, 2025
…thereum#31877)

This introduces an error when the filter has both `blockHash` and
`fromBlock`/`toBlock`, since these are mutually exclusive. Seems the
tests were actually returning `not found` error, which went undetected
since there was no check on the actual returned error in the test.
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.

2 participants