feat(evmonly): add eth_getBlockByNumber and eth_getBlockByHash to the EVM-only executor - #4208
Conversation
Add the two block-lookup RPCs to the Autobahn EVM-only executor's JSON-RPC server, matching go-ethereum's response shape while sourcing data from the already-existing Backend.Block/BlockByHash and receipt store. - New giga/evmonly/rpc/block.go: blockAPI.GetBlockByNumber/GetBlockByHash. latest/safe/finalized/pending resolve to the current committed block; any other explicit height (including a past one) is served directly, since block/receipt data is retained indefinitely unlike this application's current-only state view; a height outside the committed range, or "earliest" (this go-ethereum fork's literal height 0, which this executor never commits), resolves to null. - gasUsed is read from the last transaction's receipt CumulativeGasUsed rather than summed per transaction, avoiding an all-receipts fetch. Hash-only transaction lists are the Ethereum keccak256 hash of each decoded transaction, not Block.GetTxHashes()'s Tendermint-internal hash. - parentHash/stateRoot/transactionsRoot/receiptsRoot/miner/logsBloom stay zero-valued: Autobahn's translation from its internal block into this RPC's coretypes.ResultBlock shape never populates the header fields they would read from (confirmed by a scratch test against the real translation, not assumed). - Extend Backend with BlockByHash and EvmGasLimit; thread EvmGasLimit through proxy.Proxy (optional-interface pattern, matching EvmChainConfig) and evmOnlyApplication, which never changes the gas limit after InitChain, so the current value is also correct for any past block. - Update integration_test/autobahn/README.md's RPC surface list and add a cast block usage example.
…rebase giga-1's eth_getTransactionByHash gained a real EvmBaseFee() accessor during review (replacing a hardcoded zero) after this branch's eth_call/tx work was written. Align eth_getBlockByNumber/eth_getBlockByHash's full-tx encoding and baseFeePerGas field the same way, and fix the test fixture the rebase otherwise leaves with a nil EvmBaseFee panic.
PR SummaryMedium Risk Overview Block lookups use the existing height-based Reviewed by Cursor Bugbot for commit ee8de7b. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## giga-1 #4208 +/- ##
=======================================
Coverage 65.55% 65.56%
=======================================
Files 2081 2083 +2
Lines 157460 157387 -73
=======================================
- Hits 103222 103187 -35
+ Misses 54097 54059 -38
Partials 141 141
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0fc7e53. Configure here.
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds eth_getBlockByNumber/eth_getBlockByHash to the EVM-only executor with well-scoped plumbing (Backend.BlockByHash, EvmGasLimit through Proxy/Environment/app) and good test coverage, including a cross-check that block tx hashes match eth_getTransactionByHash. No blockers; three non-blocking issues: an unhandled pruned-height error path that contradicts the "retained indefinitely" claim, a mixHash that disagrees with the executor's actual PREVRANDAO, and an O(N) receipt read in the fullTx path.
Findings: 0 blocking | 3 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
- 3 suggestion(s)/nit(s) flagged inline on specific lines.
| height = &h | ||
| } | ||
| block, err := api.backend.Block(ctx, &coretypes.RequestBlockInfo{Height: height}) | ||
| if errors.Is(err, coretypes.ErrHeightExceedsChainHead) || errors.Is(err, coretypes.ErrZeroOrNegativeHeight) { |
There was a problem hiding this comment.
[suggestion] ErrHeightNotAvailable is not handled here, so a height below the retention watermark surfaces as a JSON-RPC error rather than null. Under Autobahn env.Block routes to gigaRouterCommon.BlockByNumber, which maps atypes.ErrPruned to coretypes.WrapErrHeightNotAvailable(...) (sei-tendermint/internal/p2p/giga_router_common.go:146); env.getHeight can also emit it via the height < base branch. This also makes the surrounding claims inaccurate: the godoc above ("block data is retained indefinitely, so every other explicit height ... is looked up directly") and the README ("block and receipt data is retained indefinitely, so any past height works the same as latest", integration_test/autobahn/README.md:503) both promise something the block store does not guarantee once pruning kicks in. Suggest adding coretypes.ErrHeightNotAvailable to the null-mapping set (matching the spirit of evmrpc's blockByNumberOrNullForJSONRPC) and softening the two doc claims to "retained for the node's retention window".
There was a problem hiding this comment.
fixed, this aligns with EIP-4444 behaviro where non-retained blocks return null instead of an error
| "hash": blockHash, | ||
| "parentHash": common.BytesToHash(block.Block.LastBlockID.Hash), | ||
| "nonce": ethtypes.BlockNonce{}, // inapplicable to Sei | ||
| "mixHash": common.Hash{}, // inapplicable to Sei |
There was a problem hiding this comment.
[suggestion] mixHash is hardcoded to zero and labelled "inapplicable to Sei", but on this execution path it is applicable: evmOnlyApplication executes every block with PrevRandao: evmOnlyPrevRandao(timestamp) (sei-tendermint/internal/evmonlyapp/app.go:436), which buildBlockContext feeds to vm.BlockContext.Random (giga/evmonly/executor.go:368), so PREVRANDAO inside a contract returns keccak256(be64(timestamp)). A forked backend (forge script --broadcast, the use case this PR targets) reads mixHash from the header and will simulate PREVRANDAO with a different value than the node executes. Since the PR deliberately enumerates and pins the real header gaps, this one is worth returning correctly: derive it from the block timestamp the same way, and pin it in TestEncodeBlockDocumentedHeaderGaps (or a sibling test) against the executor's derivation rather than duplicating the constant. If it is intentionally left zero, the // inapplicable to Sei comment should say why instead.
There was a problem hiding this comment.
The current code aligns with v2 performance. No changes will be made in this PR.
| if err != nil { | ||
| return nil, err | ||
| } | ||
| stored, err := api.receiptFor(ctx, ethtx.Hash()) |
There was a problem hiding this comment.
[suggestion] The fullTx branch performs one receipt-store read per transaction, but the only consumers are replaceFrom (a documented edge case for legacy shapes where signature recovery yields the zero address) and lastReceipt. For a load-test-sized Autobahn block this turns eth_getBlockByNumber(latest, true) into thousands of sequential store lookups. It also contradicts the rationale stated at line 156 and in the encodeBlock godoc, which justifies omitting logsBloom and summed gasUsed precisely by "a receipt per transaction" being too expensive — a cost this branch already pays. Consider reading the receipt lazily, only when result.From == (common.Address{}), plus the one read for the last transaction, so both branches cost a single lookup in the common case.
There was a problem hiding this comment.
Punting, we don't have an efective bulk receipt query method. Will pursue later,
Godocs and test comments here read as design narration (which convention this matches, why an internal function was picked, what the underlying Autobahn translation does and doesn't populate). Cut to what each function returns and what a reader needs to know; the git history and this PR's description carry the rest.
wen-coding
left a comment
wen-coding
left a comment
There was a problem hiding this comment.
LGTM, seidroid inline comments might be worth addressing
- resolveBlockByNumber now maps ErrHeightNotAvailable to null, matching the null treatment already given to future/zero heights - fix stale encodeBlock/README claims that block data is retained indefinitely and that parentHash/stateRoot/etc are always zero - note gasUsed/cumulativeGasUsed is scoped to one Autobahn lane, not every lane executing concurrently; revisit with superblocks - defer the per-tx receipt read in the fullTx path pending a bulk receipt-load API Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 participants
Describe your changes and provide context
Adds
eth_getBlockByNumberandeth_getBlockByHashto the Autobahn EVM-only executor's RPC server, on top ofeth_getTransactionByHash(#4205). This unblocksforge script --broadcast, which unconditionally callseth_getBlockByNumber("latest", false)to instantiate its forked backend — even with--skip-simulationand explicit gas flags, it currently hard-fails withthe method eth_getBlockByNumber does not exist.blockAPI(giga/evmonly/rpc/block.go) withGetBlockByNumber/GetBlockByHash, reusing the existingBackend.Block(by height) and a newly addedBackend.BlockByHash— the latter delegates straight toEnvironment.BlockByHash, which was already Autobahn-aware and already returnsnullfor an unknown hash rather than an errorGetBalance/GetTransactionCount/Call), this does not reuserequireCurrentState's "current only" restriction: block and receipt data is retained per-height indefinitely (proven byGetTransactionReceiptalready serving arbitrary past heights), unlike account state, which has no historical view yet.latest/safe/finalized/pendingresolve to the current committed block; any other explicit height is served directly; a future height or (in this fork)earliest(rpc.EarliestBlockNumber == 0, an unused height) both fall through tonullGetTransactionByHashdoes — deliberately does not useBlock.GetTxHashes(), which returns a Tendermint-internal hash rather than the Ethereum keccak256 hash every other endpoint here keys by; a dedicated test cross-checks the two paths produce identical hashesgasUsedfor a block comes from the last transaction's receiptCumulativeGasUsed, not a per-tx sumEvmGasLimitplumbed throughProxy/Environment/evmOnlyApplication, mirroring the existingEvmCall/EvmChainConfigoptional-interface pattern; verified the app's gas limit never changes afterInitChain, so using the "current" value is exact for any historical block, not an approximationparentHash/stateRoot/transactionsRoot/receiptsRoot/miner/logsBloomare documented, confirmed gaps (verified against the realgigaRouterCommonblock-translation path, not assumed) rather than silently wrong valuesgiga-1's current tip after feat(evmonly): add eth_getTransactionByHash RPC to the EVM-only executor #4205 merged; also alignedblock.go's base fee handling with the realEvmBaseFee()accessor feat(evmonly): add eth_getTransactionByHash RPC to the EVM-only executor #4205 gained during its own review (was a hardcoded zero when this branch was written against the pre-review version)cast blockusage example in the Autobahn READMETesting performed to validate your change
go build ./...go test -count=1 ./giga/evmonly/... ./sei-tendermint/internal/proxy/... ./sei-tendermint/internal/evmonlyapp/... ./sei-tendermint/internal/rpc/core/... ./sei-tendermint/node/...TestEncodeBlockHashOnlyListMatchesGetTransactionByHashcross-checks the hash-only list againstGetTransactionByHashfor the same transactionsTestEncodeBlockDocumentedHeaderGapspins which header fields are real vs. always-zero on this execution pathgolangci-lint runv2.13.2 scoped to all touched packages: 0 issuesgolangci-lint fmt --diff: clean🤖 Generated with Claude Code