Skip to content

eth, eth/filters: implement API error code for pruned blocks #31361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Apr 1, 2025

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Mar 12, 2025

Implements #31275

I find it a bit weird to define the error type in eth/ but I feel like that is the place to return it.

@fjl
Copy link
Contributor

fjl commented Mar 13, 2025

I have folded #31366 into this PR. Doesn't make sense to have it standalone, since changes are required in the API backend to support earliest.

@@ -428,3 +455,8 @@ func (b *EthAPIBackend) StateAtBlock(ctx context.Context, block *types.Block, re
func (b *EthAPIBackend) StateAtTransaction(ctx context.Context, block *types.Block, txIndex int, reexec uint64) (*types.Transaction, vm.BlockContext, *state.StateDB, tracers.StateReleaseFunc, error) {
return b.eth.stateAtTransaction(ctx, block, txIndex, reexec)
}

type prunedHistoryError struct{}
Copy link
Member

Choose a reason for hiding this comment

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

It's indeed weird to have duplicated error definitions in different package. Nitpick :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep agree I now made the definition in api_backend public instead.

@rjl493456442
Copy link
Member

Please rebase. Otherwise sgtm.

Note: There may be other areas that also require cutoff handling. It's fine to merge these first

@s1na
Copy link
Contributor Author

s1na commented Mar 25, 2025

There may be other areas that also require cutoff handling

I thought of all state-related APIs (eth_getBalance, eth_call) but I doubt archive nodes will want to prune their node. Makes me think we should probably check in the prune history command and explicitly reject it for archive nodes.

fjl
fjl previously approved these changes Apr 1, 2025
Copy link
Contributor

@fjl fjl left a comment

Choose a reason for hiding this comment

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

Rebased. It passes workload tests on Sepolia, so I will merge.

@fjl fjl changed the title eth: API error code for pruned blocks eth, eth/filters: implement API error code for pruned blocks Apr 1, 2025
@fjl fjl added this to the 1.15.8 milestone Apr 1, 2025
@fjl fjl merged commit bc36f2d into ethereum:master Apr 1, 2025
3 of 4 checks passed
thinkAfCod pushed a commit to thinkAfCod/go-ethereum that referenced this pull request Apr 2, 2025
…m#31361)

Implements ethereum#31275

---------

Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
fjl added a commit that referenced this pull request Apr 3, 2025
These were caused by crossed merges of recent PRs #31414 and #31361
sivaratrisrinivas pushed a commit to sivaratrisrinivas/go-ethereum that referenced this pull request Apr 21, 2025
…m#31361)

Implements ethereum#31275

---------

Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
sivaratrisrinivas pushed a commit to sivaratrisrinivas/go-ethereum that referenced this pull request Apr 21, 2025
Rampex1 pushed a commit to streamingfast/go-ethereum that referenced this pull request May 15, 2025
…m#31361)

Implements ethereum#31275

---------

Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
Rampex1 pushed a commit to streamingfast/go-ethereum that referenced this pull request May 15, 2025
fjl added a commit that referenced this pull request Jun 2, 2025
This changes the API backend to return null for not-found blocks. This behavior
is required by the RPC When `BlockByNumberOrHash` always returned an error
for this case ever since being added in #19491.
The backend method has a couple of call sites, and all of them handle a `nil`
block result because `BlockByNumber` returns `nil` for not-found.

The only case where this makes a real difference is for `eth_getBlockReceipts`,
which was changed in #31361 to actually forward the error from `BlockByNumberOrHash`
to the caller.
jsvisa pushed a commit to jsvisa/go-ethereum that referenced this pull request Jun 5, 2025
This changes the API backend to return null for not-found blocks. This behavior
is required by the RPC When `BlockByNumberOrHash` always returned an error
for this case ever since being added in ethereum#19491.
The backend method has a couple of call sites, and all of them handle a `nil`
block result because `BlockByNumber` returns `nil` for not-found.

The only case where this makes a real difference is for `eth_getBlockReceipts`,
which was changed in ethereum#31361 to actually forward the error from `BlockByNumberOrHash`
to the caller.
Signed-off-by: jsvisa <delweng@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants