evmrpc: use BlockID.Hash in GetTransaction (CON-257)#3350
Merged
Conversation
GetTransaction returned the eth blockHash by recomputing block.Block.Header.Hash() from the tendermint Block returned by /block. That works under CometBFT because Header is fully populated and Header.Hash() == BlockID.Hash, but breaks under Autobahn where the Block returned by GigaRouter.BlockByNumber has only ChainID / Height / Time set on the Header — Header.Hash() then computes a Merkle root that doesn't match anything stored anywhere. The downstream effect: go-ethereum's tracers.API.TraceTransaction calls backend.GetTransaction, then blockByNumberAndHash(num, hash). That helper falls back to BlockByHash when the freshly-fetched block's hash doesn't equal `hash` — and BlockByHash returns ErrBlockNotFoundByHash because the recomputed sparse-header hash isn't indexed. debug_traceTransaction surfaces "block not found by hash" instead of a real trace. Fix: read blockHash from block.BlockID.Hash, which is the actual block hash the EVM receipt store recorded during FinalizeBlock — identical to Header.Hash() under CometBFT, correct under Autobahn. Test: TestGetTransactionUsesBlockIDHash with two subtests, each asserting both the fixture invariant (Header.Hash() == BlockID.Hash under CometBFT, != under Autobahn) and the contract (GetTransaction returns BlockID.Hash). The Autobahn subtest fails on the original code; the CometBFT subtest pins the fixture so a future Autobahn shape that accidentally matches Header.Hash() can't pass without being noticed. Verified live: debug_traceTransaction now returns proper traces with callTracer / prestateTracer / prestateTracer-diffMode against a 4-node Autobahn cluster (and continues to work under CometBFT — evmrpc tests all green). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wen-coding
changed the title
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3350 +/- ##
=======================================
Coverage 59.17% 59.17%
=======================================
Files 2097 2097
Lines 172509 172509
=======================================
Hits 102079 102079
Misses 61570 61570
Partials 8860 8860
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
bdchatham
reviewed
Apr 30, 2026
| txIndex := hexutil.Uint(receipt.TransactionIndex) | ||
| tmTx := block.Block.Txs[txIndex] | ||
| tx = getEthTxForTxBz(tmTx, b.txConfigProvider(block.Block.Height).TxDecoder()) | ||
| blockHash = common.BytesToHash(block.Block.Header.Hash().Bytes()) |
Contributor
There was a problem hiding this comment.
Is this translation only required in this code path or potentially other places too?
Contributor
Author
There was a problem hiding this comment.
I think this is the only place that matters for now.
We also have:
evmrpc/subscribe.gousingheader.Header.Hash().String()onEventDataNewBlockHeaderevmrpc/filter.gousingblock.Block.Hash()onEventDataNewBlock
These two are both triggered on events, and Autobahn currently don't invoke events. I don't know whether Autobahn will ever need these events. If we do then we can fix them there.
pompon0
reviewed
May 1, 2026
| // BlockID.Hash carries the actual block hash that the EVM receipt | ||
| // store recorded during FinalizeBlock: same on both engines, | ||
| // correct under both. | ||
| blockHash = common.BytesToHash(block.BlockID.Hash) |
Contributor
There was a problem hiding this comment.
are the block metadata guaranteed to be correct here? We find holes in Tendermint verification every now and then.
Contributor
Author
There was a problem hiding this comment.
Are you asking about the timing issue? It's correct on our end:
GetTransaction(txHash) does Receipt(txHash) first, so it will only return the hash here if a receipt exists, so the block must have finished execution and the hash should be good here.
pompon0
approved these changes
May 1, 2026
jewei1997
approved these changes
May 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backend.GetTransactionreturned the ethblockHashby recomputingblock.Block.Header.Hash(). That's value-equivalent toBlockID.Hashunder CometBFT (full Header populated by the BlockStore), but wrong under Autobahn —GigaRouter.translateGlobalBlockreturns a ResultBlock with onlyChainID/Height/Timeon the Header, soHeader.Hash()recomputes a Merkle root that doesn't match any stored value.Downstream symptom
go-ethereum's
tracers.API.TraceTransactioncallsbackend.GetTransaction(hash), then runsblockByNumberAndHash(num, blockHash). When that helper seesblock.Hash() != blockHash, it falls back toBlockByHash(blockHash)— which under Autobahn returnsErrBlockNotFoundByHashbecause the recomputed sparse-header hash isn't indexed anywhere.debug_traceTransactionsurfaces"block not found by hash"instead of a real trace.Fix
Read
blockHashfromblock.BlockID.Hash. That's the actual block hash the EVM receipt store recorded duringFinalizeBlock:Header.Hash()— value-equivalent substitutionVerification
go test ./evmrpc/...— 4 packages green (incl. new test, fails without fix)debug_traceTransactionon a 4-node Autobahn cluster now returns proper traces withcallTracer/prestateTracer/prestateTracer-diffMode(previously:block not found by hash)🤖 Generated with Claude Code