From 93026b930ec198422a7bc4884c84901805266c61 Mon Sep 17 00:00:00 2001 From: Wen Date: Mon, 1 Jun 2026 18:48:11 -0400 Subject: [PATCH 1/7] evmrpc: return null on above-watermark for spec-compliant endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Endpoints that take a block identifier and return JSON null for missing blocks per Ethereum JSON-RPC spec currently surface the watermark race ("requested height N is not yet available; safe latest is N-1") as an error to clients. Wallets, indexers, and similar tools see a transient failure where they expected null, exactly when a tx has just been mined and the safe-latest watermark hasn't caught up yet. #3119 fixed eth_getBlockByNumber and #3501 fixed eth_getTransactionReceipt the same way. This generalizes the pattern via two helpers (blockByNumberOrNullForJSONRPC, blockByHashOrNullForJSONRPC) and applies the spec-aligned conversion to the remaining user-facing endpoints: eth_getBlockReceipts eth_getBlockTransactionCountByNumber eth_getBlockTransactionCountByHash eth_getBlockByHash (and sei_getBlockByHashExcludeTraceFail) eth_getTransactionByBlockNumberAndIndex eth_getTransactionByBlockHashAndIndex eth_getTransactionByHash State queries (GetBalance/Code/StorageAt) and simulation/filter/internal paths keep using blockByNumberRespectingWatermarks directly — they have different semantics (reject invalid heights, internal validation). Co-Authored-By: Claude Opus 4.7 (1M context) --- evmrpc/block.go | 25 +++++++++++++++--- evmrpc/height_availability_test.go | 12 ++++++--- evmrpc/tx.go | 21 ++++++++++++--- evmrpc/tx_test.go | 6 ++--- evmrpc/watermark_manager.go | 41 ++++++++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 14 deletions(-) diff --git a/evmrpc/block.go b/evmrpc/block.go index 32c3a446e3..f5f5d776b9 100644 --- a/evmrpc/block.go +++ b/evmrpc/block.go @@ -172,10 +172,14 @@ func (a *BlockAPI) GetBlockTransactionCountByNumber(ctx context.Context, number if err != nil { return nil, err } - block, err := blockByNumberRespectingWatermarks(ctx, a.tmClient, a.watermarks, numberPtr, 1) + // Ethereum JSON-RPC: non-existent / future numeric block => null, not an error. + block, err := blockByNumberOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, numberPtr, 1) if err != nil { return nil, err } + if block == nil { + return nil, nil + } return a.getEvmTxCount(block), nil } @@ -187,10 +191,14 @@ func (a *BlockAPI) GetBlockTransactionCountByHash(ctx context.Context, blockHash if blockHash == genesisBlockHash { return genesisBlockTxCount, nil } - block, err := blockByHashRespectingWatermarks(ctx, a.tmClient, a.watermarks, blockHash[:], 1) + // Ethereum JSON-RPC: non-existent block hash => null, not an error. + block, err := blockByHashOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, blockHash[:], 1) if err != nil { return nil, err } + if block == nil { + return nil, nil + } return a.getEvmTxCount(block), nil } @@ -212,13 +220,18 @@ func (a *BlockAPI) getBlockByHash(ctx context.Context, blockHash common.Hash, fu if blockHash == genesisBlockHash { return encodeGenesisBlock(), nil } - block, err := blockByHashRespectingWatermarks(ctx, a.tmClient, a.watermarks, blockHash[:], 1) + // Ethereum JSON-RPC: non-existent block hash (unknown OR above safe latest) + // => null, not an error. + block, err := blockByHashOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, blockHash[:], 1) if errors.Is(err, ErrBlockNotFoundByHash) { return nil, nil } if err != nil { return nil, err } + if block == nil { + return nil, nil + } // Validate EVM block height for pacific-1 chain sdkCtx := a.ctxProvider(LatestCtxHeight) @@ -298,10 +311,14 @@ func (a *BlockAPI) GetBlockReceipts(ctx context.Context, blockNrOrHash rpc.Block return nil, err } - block, err := blockByNumberRespectingWatermarks(ctx, a.tmClient, a.watermarks, heightPtr, 1) + // Ethereum JSON-RPC: non-existent / above-watermark block => null, not an error. + block, err := blockByNumberOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, heightPtr, 1) if err != nil { return nil, err } + if block == nil { + return nil, nil + } // Get all tx hashes for the block height := block.Block.Height diff --git a/evmrpc/height_availability_test.go b/evmrpc/height_availability_test.go index 9cbeee1e94..df0df5c8f7 100644 --- a/evmrpc/height_availability_test.go +++ b/evmrpc/height_availability_test.go @@ -107,7 +107,11 @@ func testTxConfigProvider(int64) client.TxConfig { return nil } func testCtxProvider(int64) sdk.Context { return sdk.Context{} } -func TestBlockAPIEnsureHeightUnavailable(t *testing.T) { +// GetBlockByHash for a block whose height sits above safe latest must return +// JSON null per the Ethereum JSON-RPC spec (the block doesn't exist from the +// caller's perspective), matching get-block-by-empty-hash.iox / get-block-by- +// notfound-hash.iox semantics. +func TestBlockAPIAboveWatermarkReturnsNull(t *testing.T) { t.Parallel() earliest := int64(1) @@ -117,9 +121,9 @@ func TestBlockAPIEnsureHeightUnavailable(t *testing.T) { watermarks := NewWatermarkManager(client, testCtxProvider, nil, nil) api := NewBlockAPI(client, nil, testCtxProvider, testTxConfigProvider, ConnectionTypeHTTP, watermarks, nil, nil) - _, err := api.GetBlockByHash(context.Background(), common.HexToHash(highBlockHashHex), false) - require.Error(t, err) - require.Contains(t, err.Error(), "requested height") + result, err := api.GetBlockByHash(context.Background(), common.HexToHash(highBlockHashHex), false) + require.NoError(t, err) + require.Nil(t, result) } // TestGetBlockByHashNotFoundReturnsNull verifies Ethereum-compatible behavior: empty or non-existent block hash diff --git a/evmrpc/tx.go b/evmrpc/tx.go index aec8230980..47ca99b346 100644 --- a/evmrpc/tx.go +++ b/evmrpc/tx.go @@ -213,10 +213,14 @@ func (t *TransactionAPI) getTransactionByBlockNumberAndIndex(ctx context.Context if err != nil { return nil, err } - block, err := blockByNumberRespectingWatermarks(ctx, t.tmClient, t.watermarks, blockNumber, 1) + // Ethereum JSON-RPC: non-existent block => null, not an error. + block, err := blockByNumberOrNullForJSONRPC(ctx, t.tmClient, t.watermarks, blockNumber, 1) if err != nil { return nil, err } + if block == nil { + return nil, nil + } return t.getTransactionWithBlock(block, txIndex, t.includeSynthetic) } @@ -229,10 +233,14 @@ func (t *TransactionAPI) GetTransactionByBlockHashAndIndex(ctx context.Context, _err = nil //not returning error for invalid tx index for complying with Ethereum JSON-RPC spec } }() - block, err := blockByHashRespectingWatermarks(ctx, t.tmClient, t.watermarks, blockHash[:], 1) + // Ethereum JSON-RPC: non-existent / above-watermark block => null, not an error. + block, err := blockByHashOrNullForJSONRPC(ctx, t.tmClient, t.watermarks, blockHash[:], 1) if err != nil { return nil, err } + if block == nil { + return nil, nil + } var idx uint32 idx, err = txIndexToUint32(txIndex) if err != nil { @@ -290,10 +298,17 @@ func (t *TransactionAPI) GetTransactionByHash(ctx context.Context, hash common.H return nil, err } blockNumber := int64(receipt.BlockNumber) //nolint:gosec - block, err := blockByNumberRespectingWatermarks(ctx, t.tmClient, t.watermarks, &blockNumber, 1) + // Ethereum JSON-RPC: tx whose block isn't safe-latest yet => null (not yet + // mined from the caller's perspective). The watermark race here mirrors + // the one fixed in getTransactionReceipt; ethers' tx.wait() and similar + // flows can call getTransactionByHash and propagate the error otherwise. + block, err := blockByNumberOrNullForJSONRPC(ctx, t.tmClient, t.watermarks, &blockNumber, 1) if err != nil { return nil, err } + if block == nil { + return nil, nil + } filteredMsgs := t.getFilteredMsgs(block) txIndex, found, ethtx, _ := GetEvmTxIndex(t.ctxProvider(LatestCtxHeight), block, filteredMsgs, receipt.TransactionIndex, t.keeper, t.cacheCreationMutex, t.globalBlockCache) if !found { diff --git a/evmrpc/tx_test.go b/evmrpc/tx_test.go index 2faf8a5ce8..a43551b0b3 100644 --- a/evmrpc/tx_test.go +++ b/evmrpc/tx_test.go @@ -453,9 +453,9 @@ func TestGetTransactionByBlockNumberAndIndexErrors(t *testing.T) { resObj = map[string]interface{}{} require.Nil(t, json.Unmarshal(resBody, &resObj)) - // Should get an error for non-existent block - errMap := resObj["error"].(map[string]interface{}) - require.NotNil(t, errMap["message"]) + // Non-existent block returns null result (Ethereum JSON-RPC spec). + require.Nil(t, resObj["error"]) + require.Nil(t, resObj["result"]) } func TestGetTransactionByBlockHashAndIndexErrors(t *testing.T) { diff --git a/evmrpc/watermark_manager.go b/evmrpc/watermark_manager.go index 03d0cf89d3..960a76b117 100644 --- a/evmrpc/watermark_manager.go +++ b/evmrpc/watermark_manager.go @@ -270,6 +270,47 @@ func blockByHashRespectingWatermarks( return block, nil } +// blockByNumberOrNullForJSONRPC wraps blockByNumberRespectingWatermarks for +// Ethereum JSON-RPC endpoints that must return null (not an error) when the +// requested block sits above the safe-latest watermark — i.e. the block does +// not yet exist from the caller's perspective. This is the spec contract for +// endpoints that take a block identifier and return null for non-existent +// blocks (eth_getBlockByNumber, eth_getBlockByHash, eth_getBlockReceipts, +// eth_getTransactionByHash, eth_getTransactionByBlock*AndIndex, etc.). +// +// Internal call sites that genuinely need the error (state queries that must +// reject invalid heights, simulation paths bound to a specific block) keep +// using blockByNumberRespectingWatermarks directly. +func blockByNumberOrNullForJSONRPC( + ctx context.Context, + c client.LocalClient, + wm *WatermarkManager, + heightPtr *int64, + maxRetries int, +) (*coretypes.ResultBlock, error) { + block, err := blockByNumberRespectingWatermarks(ctx, c, wm, heightPtr, maxRetries) + if errors.Is(err, ErrBlockHeightNotYetAvailable) { + return nil, nil + } + return block, err +} + +// blockByHashOrNullForJSONRPC is the by-hash counterpart of +// blockByNumberOrNullForJSONRPC. See that function for spec rationale. +func blockByHashOrNullForJSONRPC( + ctx context.Context, + c client.LocalClient, + wm *WatermarkManager, + hash []byte, + maxRetries int, +) (*coretypes.ResultBlock, error) { + block, err := blockByHashRespectingWatermarks(ctx, c, wm, hash, maxRetries) + if errors.Is(err, ErrBlockHeightNotYetAvailable) { + return nil, nil + } + return block, err +} + func (m *WatermarkManager) fetchTendermintWatermarks(ctx context.Context) (int64, int64, error) { if m.tmClient == nil { return 0, 0, errNoHeightSource From 3ec69fb1f641ab2074c4bf068dae21437399eaf4 Mon Sep 17 00:00:00 2001 From: Wen Date: Mon, 1 Jun 2026 18:59:29 -0400 Subject: [PATCH 2/7] evmrpc: convert above-watermark to null on GetBlockReceipts hash path Per cursor bugbot: GetBlockReceipts(blockHash) resolves the hash via GetBlockNumberByNrOrHash, which internally calls the original blockByHashRespectingWatermarks (not the new null-converting helper). An above-watermark hash returns ErrBlockHeightNotYetAvailable from that lookup, which propagates as an RPC error before the second blockByNumberOrNullForJSONRPC call is reached. Treat ErrBlockHeightNotYetAvailable from GetBlockNumberByNrOrHash the same as ErrBlockNotFoundByHash (return null per Ethereum JSON-RPC spec). Co-Authored-By: Claude Opus 4.7 (1M context) --- evmrpc/block.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/evmrpc/block.go b/evmrpc/block.go index f5f5d776b9..34549e1454 100644 --- a/evmrpc/block.go +++ b/evmrpc/block.go @@ -302,9 +302,12 @@ func (a *BlockAPI) GetBlockReceipts(ctx context.Context, blockNrOrHash rpc.Block if blockNrOrHash.BlockNumber != nil && *blockNrOrHash.BlockNumber == 0 { return []map[string]any{}, nil } - // Get height from params + // Get height from params. GetBlockNumberByNrOrHash resolves a hash through + // blockByHashRespectingWatermarks internally, so an above-watermark height + // surfaces here as ErrBlockHeightNotYetAvailable — convert to null per + // spec, alongside the not-found-by-hash case. heightPtr, err := GetBlockNumberByNrOrHash(ctx, a.tmClient, a.watermarks, blockNrOrHash) - if errors.Is(err, ErrBlockNotFoundByHash) { + if errors.Is(err, ErrBlockNotFoundByHash) || errors.Is(err, ErrBlockHeightNotYetAvailable) { return nil, nil } if err != nil { From b148365f78f9ad8e1d456538472ab194a943f4eb Mon Sep 17 00:00:00 2001 From: Wen Date: Mon, 1 Jun 2026 19:11:07 -0400 Subject: [PATCH 3/7] evmrpc: include ErrBlockNotFoundByHash in blockByHashOrNullForJSONRPC Per cursor bugbot: the by-hash helper only converted ErrBlockHeightNotYetAvailable; ErrBlockNotFoundByHash (genuinely unknown hash) still propagated as an RPC error from GetBlockTransactionCountByHash and GetTransactionByBlockHashAndIndex. Both forms are "block doesn't exist from the caller's perspective" and the Ethereum JSON-RPC spec maps both to null. Extend the helper to convert both, then drop the now-redundant explicit ErrBlockNotFoundByHash check from getBlockByHash so all by-hash endpoints share uniform spec-aligned semantics through the helper. Co-Authored-By: Claude Opus 4.7 (1M context) --- evmrpc/block.go | 5 +---- evmrpc/watermark_manager.go | 7 +++++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/evmrpc/block.go b/evmrpc/block.go index 34549e1454..4c65c7fca4 100644 --- a/evmrpc/block.go +++ b/evmrpc/block.go @@ -221,11 +221,8 @@ func (a *BlockAPI) getBlockByHash(ctx context.Context, blockHash common.Hash, fu return encodeGenesisBlock(), nil } // Ethereum JSON-RPC: non-existent block hash (unknown OR above safe latest) - // => null, not an error. + // => null, not an error. The helper handles both cases. block, err := blockByHashOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, blockHash[:], 1) - if errors.Is(err, ErrBlockNotFoundByHash) { - return nil, nil - } if err != nil { return nil, err } diff --git a/evmrpc/watermark_manager.go b/evmrpc/watermark_manager.go index 960a76b117..04d4aa206e 100644 --- a/evmrpc/watermark_manager.go +++ b/evmrpc/watermark_manager.go @@ -296,7 +296,10 @@ func blockByNumberOrNullForJSONRPC( } // blockByHashOrNullForJSONRPC is the by-hash counterpart of -// blockByNumberOrNullForJSONRPC. See that function for spec rationale. +// blockByNumberOrNullForJSONRPC. In addition to the above-watermark case it +// also converts ErrBlockNotFoundByHash to (nil, nil) — both are forms of +// "block doesn't exist from the caller's perspective" and the Ethereum +// JSON-RPC spec maps both to null. func blockByHashOrNullForJSONRPC( ctx context.Context, c client.LocalClient, @@ -305,7 +308,7 @@ func blockByHashOrNullForJSONRPC( maxRetries int, ) (*coretypes.ResultBlock, error) { block, err := blockByHashRespectingWatermarks(ctx, c, wm, hash, maxRetries) - if errors.Is(err, ErrBlockHeightNotYetAvailable) { + if errors.Is(err, ErrBlockHeightNotYetAvailable) || errors.Is(err, ErrBlockNotFoundByHash) { return nil, nil } return block, err From 06389ea8de387d5284058d49923cfff5db5dbb73 Mon Sep 17 00:00:00 2001 From: Wen Date: Mon, 1 Jun 2026 22:44:20 -0400 Subject: [PATCH 4/7] evmrpc: route GetBlockNumberByNrOrHash through the null-aligned helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: GetBlockReceipts had duplicated watermark handling — once via GetBlockNumberByNrOrHash and once via blockByNumberOrNullForJSONRPC. Push the "block doesn't exist" mapping (both unknown-hash and above- watermark) into GetBlockNumberByNrOrHash so callers serving Ethereum JSON-RPC endpoints can detect "null result" via a single heightPtr==nil check instead of two errors.Is branches. Single source of truth for the conversion; behavior is equivalent across all scenarios (verified by the existing height-availability + receipts tests). Co-Authored-By: Claude Opus 4.7 (1M context) --- evmrpc/block.go | 14 +++++++------- evmrpc/utils.go | 11 +++++++++-- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/evmrpc/block.go b/evmrpc/block.go index 4c65c7fca4..015e835352 100644 --- a/evmrpc/block.go +++ b/evmrpc/block.go @@ -299,17 +299,17 @@ func (a *BlockAPI) GetBlockReceipts(ctx context.Context, blockNrOrHash rpc.Block if blockNrOrHash.BlockNumber != nil && *blockNrOrHash.BlockNumber == 0 { return []map[string]any{}, nil } - // Get height from params. GetBlockNumberByNrOrHash resolves a hash through - // blockByHashRespectingWatermarks internally, so an above-watermark height - // surfaces here as ErrBlockHeightNotYetAvailable — convert to null per - // spec, alongside the not-found-by-hash case. + // Get height from params. GetBlockNumberByNrOrHash already maps both + // unknown-hash and above-watermark to (nil, nil) for the by-hash path, + // matching Ethereum JSON-RPC null semantics; by-number propagates other + // errors normally. heightPtr, err := GetBlockNumberByNrOrHash(ctx, a.tmClient, a.watermarks, blockNrOrHash) - if errors.Is(err, ErrBlockNotFoundByHash) || errors.Is(err, ErrBlockHeightNotYetAvailable) { - return nil, nil - } if err != nil { return nil, err } + if heightPtr == nil { + return nil, nil + } // Ethereum JSON-RPC: non-existent / above-watermark block => null, not an error. block, err := blockByNumberOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, heightPtr, 1) diff --git a/evmrpc/utils.go b/evmrpc/utils.go index 791d7e9599..017ae12f71 100644 --- a/evmrpc/utils.go +++ b/evmrpc/utils.go @@ -45,7 +45,11 @@ const Pacific1EVMLaunchHeight int64 = 79123881 // Ethereum-compatible RPCs should return result: null for this case instead of an error. var ErrBlockNotFoundByHash = errors.New("block not found by hash") -// GetBlockNumberByNrOrHash returns the height of the block with the given number or hash. +// GetBlockNumberByNrOrHash returns the height of the block with the given +// number or hash. For the hash path it uses the JSON-RPC-aligned helper so +// that "block doesn't exist" (unknown hash) and "block not yet safe-latest" +// (above watermark) both surface as (nil, nil) rather than distinct errors — +// callers serving Ethereum JSON-RPC endpoints can map either to a null result. func GetBlockNumberByNrOrHash(ctx context.Context, tmClient client.LocalClient, wm *WatermarkManager, blockNrOrHash rpc.BlockNumberOrHash) (*int64, error) { if blockNrOrHash.BlockHash != nil { // Synthetic genesis from eth_getBlockByNumber("0x0") is not stored under this hash in Tendermint. @@ -53,10 +57,13 @@ func GetBlockNumberByNrOrHash(ctx context.Context, tmClient client.LocalClient, z := int64(0) return &z, nil } - block, err := blockByHashRespectingWatermarks(ctx, tmClient, wm, blockNrOrHash.BlockHash[:], 1) + block, err := blockByHashOrNullForJSONRPC(ctx, tmClient, wm, blockNrOrHash.BlockHash[:], 1) if err != nil { return nil, err } + if block == nil { + return nil, nil + } height := block.Block.Height return &height, nil } From fa17babee528c5df8d693115c73868e4a3e6b4a0 Mon Sep 17 00:00:00 2001 From: Wen Date: Mon, 1 Jun 2026 22:51:32 -0400 Subject: [PATCH 5/7] evmrpc: consolidate inline null-conversion sites and add helper unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: - getBlockByNumber (was #3119) and getTransactionReceipt (was #3501) had inline `if errors.Is(err, ErrBlockHeightNotYetAvailable) { return nil, nil }` predating this PR's helpers. Route both through blockByNumberOrNullForJSONRPC so all spec-compliant conversion lives in one place. Behavior verified equivalent by the existing endpoint tests (TestGetBlockByNumber*, TestGetTransactionReceiptReturnsNullAboveWatermark). - Add direct unit tests covering each helper's branches (above-watermark, in-range, sentinel-not-found-by-hash, non-watermark error propagation). The Block:nil → ErrBlockNotFoundByHash path was previously not exercised by any test (MockClient returns a plain string error instead of the sentinel), so this closes that gap. `errors` import removed from block.go (no remaining callers after this consolidation; getBlockByHash still uses errors elsewhere... actually no it doesn't either after the prior commit, hence the removal). Co-Authored-By: Claude Opus 4.7 (1M context) --- evmrpc/block.go | 9 ++-- evmrpc/tx.go | 8 ++-- evmrpc/watermark_manager_test.go | 81 ++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 9 deletions(-) diff --git a/evmrpc/block.go b/evmrpc/block.go index 015e835352..a787f4a3a1 100644 --- a/evmrpc/block.go +++ b/evmrpc/block.go @@ -4,7 +4,6 @@ import ( "context" "crypto/sha256" "encoding/hex" - "errors" "fmt" "math/big" "strings" @@ -271,14 +270,14 @@ func (a *BlockAPI) getBlockByNumber( } } - block, err := blockByNumberRespectingWatermarks(ctx, a.tmClient, a.watermarks, numberPtr, 1) // Ethereum JSON-RPC: non-existent / future numeric block => null, not an error. - if errors.Is(err, ErrBlockHeightNotYetAvailable) { - return nil, nil - } + block, err := blockByNumberOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, numberPtr, 1) if err != nil { return nil, err } + if block == nil { + return nil, nil + } return EncodeTmBlock(a.ctxProvider, a.txConfigProvider, block, a.keeper, fullTx, a.includeBankTransfers, includeSyntheticTxs, excludeUntraceable, a.globalBlockCache, a.cacheCreationMutex) } diff --git a/evmrpc/tx.go b/evmrpc/tx.go index 47ca99b346..9ed91154ad 100644 --- a/evmrpc/tx.go +++ b/evmrpc/tx.go @@ -136,14 +136,14 @@ func getTransactionReceipt( } // Fetch block once — used both for ante-failure receipt population and encoding. height := int64(receipt.BlockNumber) //nolint:gosec - block, err := blockByNumberRespectingWatermarks(ctx, t.tmClient, t.watermarks, &height, 1) // Ethereum JSON-RPC: receipt for a block above safe latest => null, not an error. - if errors.Is(err, ErrBlockHeightNotYetAvailable) { - return nil, nil - } + block, err := blockByNumberOrNullForJSONRPC(ctx, t.tmClient, t.watermarks, &height, 1) if err != nil { return nil, err } + if block == nil { + return nil, nil + } // Fill in the receipt if the transaction has failed and used 0 gas // This case is for when a tx fails before it makes it to the VM diff --git a/evmrpc/watermark_manager_test.go b/evmrpc/watermark_manager_test.go index 95e4d868da..009cf1ad0b 100644 --- a/evmrpc/watermark_manager_test.go +++ b/evmrpc/watermark_manager_test.go @@ -317,3 +317,84 @@ func makeBlockResult(height int64) *coretypes.ResultBlock { }, } } + +// blockByNumberOrNullForJSONRPC: above-watermark height returns (nil, nil); +// in-range height returns the block; non-watermark errors propagate. +func TestBlockByNumberOrNullForJSONRPC(t *testing.T) { + t.Parallel() + + stat := &coretypes.ResultStatus{SyncInfo: coretypes.SyncInfo{LatestBlockHeight: 100, EarliestBlockHeight: 1}} + + t.Run("above watermark returns (nil, nil)", func(t *testing.T) { + c := &fakeTMClient{status: stat, blocksByHeight: map[int64]*coretypes.ResultBlock{150: makeBlockResult(150)}} + wm := NewWatermarkManager(c, nil, nil, nil) + h := int64(150) // above latest=100 + block, err := blockByNumberOrNullForJSONRPC(context.Background(), c, wm, &h, 0) + require.NoError(t, err) + require.Nil(t, block) + }) + + t.Run("in-range height returns block", func(t *testing.T) { + c := &fakeTMClient{status: stat, blocksByHeight: map[int64]*coretypes.ResultBlock{50: makeBlockResult(50)}} + wm := NewWatermarkManager(c, nil, nil, nil) + h := int64(50) + block, err := blockByNumberOrNullForJSONRPC(context.Background(), c, wm, &h, 0) + require.NoError(t, err) + require.NotNil(t, block) + require.Equal(t, int64(50), block.Block.Height) + }) + + t.Run("non-watermark error propagates", func(t *testing.T) { + // Watermark itself fails (no sources) — error is errNoHeightSource, + // which must NOT be silently converted to null. + wm := NewWatermarkManager(nil, nil, nil, nil) + h := int64(50) + _, err := blockByNumberOrNullForJSONRPC(context.Background(), nil, wm, &h, 0) + require.Error(t, err) + require.False(t, errors.Is(err, ErrBlockHeightNotYetAvailable)) + }) +} + +// blockByHashOrNullForJSONRPC: above-watermark AND unknown-hash both return +// (nil, nil); in-range hash returns the block; other errors propagate. +func TestBlockByHashOrNullForJSONRPC(t *testing.T) { + t.Parallel() + + stat := &coretypes.ResultStatus{SyncInfo: coretypes.SyncInfo{LatestBlockHeight: 100, EarliestBlockHeight: 1}} + + t.Run("above watermark returns (nil, nil)", func(t *testing.T) { + c := &fakeTMClient{status: stat, blockByHash: makeBlockResult(150)} // height above latest=100 + wm := NewWatermarkManager(c, nil, nil, nil) + block, err := blockByHashOrNullForJSONRPC(context.Background(), c, wm, []byte{0xaa}, 0) + require.NoError(t, err) + require.Nil(t, block) + }) + + t.Run("unknown hash (Block: nil) returns (nil, nil)", func(t *testing.T) { + // blockByHashWithRetry wraps Block:nil as ErrBlockNotFoundByHash; + // the helper must catch that sentinel too. + c := &fakeTMClient{status: stat, blockByHash: &coretypes.ResultBlock{Block: nil}} + wm := NewWatermarkManager(c, nil, nil, nil) + block, err := blockByHashOrNullForJSONRPC(context.Background(), c, wm, []byte{0xbb}, 0) + require.NoError(t, err) + require.Nil(t, block) + }) + + t.Run("in-range hash returns block", func(t *testing.T) { + c := &fakeTMClient{status: stat, blockByHash: makeBlockResult(50)} + wm := NewWatermarkManager(c, nil, nil, nil) + block, err := blockByHashOrNullForJSONRPC(context.Background(), c, wm, []byte{0xcc}, 0) + require.NoError(t, err) + require.NotNil(t, block) + require.Equal(t, int64(50), block.Block.Height) + }) + + t.Run("transport error propagates", func(t *testing.T) { + // A non-sentinel error from the TM client (e.g. RPC transport + // failure) must NOT be silently swallowed into null. + c := &fakeTMClient{status: stat, blockByHashErr: io.ErrUnexpectedEOF} + wm := NewWatermarkManager(c, nil, nil, nil) + _, err := blockByHashOrNullForJSONRPC(context.Background(), c, wm, []byte{0xdd}, 0) + require.ErrorIs(t, err, io.ErrUnexpectedEOF) + }) +} From 7d322d8abd9fd9bdb994363a99e69637f0fd5bc7 Mon Sep 17 00:00:00 2001 From: Wen Date: Mon, 1 Jun 2026 23:11:03 -0400 Subject: [PATCH 6/7] evmrpc: GetBlockReceipts must resolve "latest" tag, not return null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The centralization in 06389ea8 made GetBlockNumberByNrOrHash return (nil, nil) for unknown hashes, but the function already returned (nil, nil) for the "latest"/"safe"/"finalized"/"pending" named tags (via getBlockNumber) — meaning "caller should resolve to latest height". Overloading nil broke eth_getBlockReceipts("latest"): the new "if heightPtr == nil { return nil, nil }" guard intercepted the tag-means-latest case and returned JSON null instead of fetching the latest block's receipts. Disentangle: - Revert GetBlockNumberByNrOrHash to its pre-PR form (errors for unknown-hash / above-watermark). - GetBlockReceipts inlines hash-vs-number dispatch via the helpers directly; numberPtr=nil flows into blockByNumberOrNullForJSONRPC which routes it through wm.LatestHeight as intended. Regression test TestBlockAPILatestTagResolves exercises all four named tags against GetBlockReceipts and GetBlockTransactionCountByNumber. Other tag-accepting endpoints (getBlockByNumber, getTransactionByBlockNumberAndIndex) use the identical (getBlockNumber → helper → if block == nil) pattern and are safe by inspection — they don't short-circuit on numberPtr==nil between the two calls. Co-Authored-By: Claude Opus 4.7 (1M context) --- evmrpc/block.go | 36 +++++++++++++++++------------- evmrpc/height_availability_test.go | 36 ++++++++++++++++++++++++++++++ evmrpc/utils.go | 11 ++------- 3 files changed, 59 insertions(+), 24 deletions(-) diff --git a/evmrpc/block.go b/evmrpc/block.go index a787f4a3a1..1e23c66c1b 100644 --- a/evmrpc/block.go +++ b/evmrpc/block.go @@ -298,22 +298,28 @@ func (a *BlockAPI) GetBlockReceipts(ctx context.Context, blockNrOrHash rpc.Block if blockNrOrHash.BlockNumber != nil && *blockNrOrHash.BlockNumber == 0 { return []map[string]any{}, nil } - // Get height from params. GetBlockNumberByNrOrHash already maps both - // unknown-hash and above-watermark to (nil, nil) for the by-hash path, - // matching Ethereum JSON-RPC null semantics; by-number propagates other - // errors normally. - heightPtr, err := GetBlockNumberByNrOrHash(ctx, a.tmClient, a.watermarks, blockNrOrHash) - if err != nil { - return nil, err - } - if heightPtr == nil { - return nil, nil - } - // Ethereum JSON-RPC: non-existent / above-watermark block => null, not an error. - block, err := blockByNumberOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, heightPtr, 1) - if err != nil { - return nil, err + // Dispatch on hash vs number directly so a nil heightPtr from getBlockNumber + // (the "latest"/"safe"/"finalized"/"pending" tags) resolves to the safe-latest + // height via blockByNumberOrNullForJSONRPC rather than being misread as + // "block doesn't exist". + var block *coretypes.ResultBlock + if blockNrOrHash.BlockHash != nil { + b, err := blockByHashOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, blockNrOrHash.BlockHash[:], 1) + if err != nil { + return nil, err + } + block = b + } else { + numberPtr, err := getBlockNumber(ctx, a.tmClient, *blockNrOrHash.BlockNumber) + if err != nil { + return nil, err + } + b, err := blockByNumberOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, numberPtr, 1) + if err != nil { + return nil, err + } + block = b } if block == nil { return nil, nil diff --git a/evmrpc/height_availability_test.go b/evmrpc/height_availability_test.go index df0df5c8f7..ff7acff152 100644 --- a/evmrpc/height_availability_test.go +++ b/evmrpc/height_availability_test.go @@ -182,6 +182,42 @@ func TestGetBlockReceiptsNotFoundReturnsNull(t *testing.T) { require.Nil(t, receipts) } +// TestBlockAPILatestTagResolves verifies that block endpoints accepting +// "latest"/"safe"/"finalized"/"pending" tags resolve to the safe-latest height +// rather than returning JSON null. The tag arrives as numberPtr=nil from +// getBlockNumber; the by-number helper must route it through wm.LatestHeight. +// +// GetBlockByNumber and getTransactionByBlockNumberAndIndex use the identical +// (getBlockNumber → blockByNumberOrNullForJSONRPC → if block == nil) pattern +// but require a real keeper for downstream encoding so they're not exercised +// directly here. +func TestBlockAPILatestTagResolves(t *testing.T) { + t.Parallel() + + earliest := int64(1) + latest := int64(100) + client := newHeightTestClient(latest+5, earliest, latest) + watermarks := NewWatermarkManager(client, testCtxProvider, nil, nil) + api := NewBlockAPI(client, nil, testCtxProvider, testTxConfigProvider, ConnectionTypeHTTP, watermarks, nil, nil) + ctx := context.Background() + + tags := []rpc.BlockNumber{ + rpc.LatestBlockNumber, + rpc.SafeBlockNumber, + rpc.FinalizedBlockNumber, + rpc.PendingBlockNumber, + } + for _, tag := range tags { + receipts, err := api.GetBlockReceipts(ctx, rpc.BlockNumberOrHashWithNumber(tag)) + require.NoError(t, err) + require.NotNil(t, receipts, "GetBlockReceipts tag %v must resolve, not null", tag) + + count, err := api.GetBlockTransactionCountByNumber(ctx, tag) + require.NoError(t, err) + require.NotNil(t, count, "GetBlockTransactionCountByNumber tag %v must resolve, not null", tag) + } +} + // TestGetBlockTransactionCountByHashGenesis verifies that the genesis block hash returned by // eth_getBlockByNumber("0x0") is accepted by eth_getBlockTransactionCountByHash (consistency). func TestGetBlockTransactionCountByHashGenesis(t *testing.T) { diff --git a/evmrpc/utils.go b/evmrpc/utils.go index 017ae12f71..791d7e9599 100644 --- a/evmrpc/utils.go +++ b/evmrpc/utils.go @@ -45,11 +45,7 @@ const Pacific1EVMLaunchHeight int64 = 79123881 // Ethereum-compatible RPCs should return result: null for this case instead of an error. var ErrBlockNotFoundByHash = errors.New("block not found by hash") -// GetBlockNumberByNrOrHash returns the height of the block with the given -// number or hash. For the hash path it uses the JSON-RPC-aligned helper so -// that "block doesn't exist" (unknown hash) and "block not yet safe-latest" -// (above watermark) both surface as (nil, nil) rather than distinct errors — -// callers serving Ethereum JSON-RPC endpoints can map either to a null result. +// GetBlockNumberByNrOrHash returns the height of the block with the given number or hash. func GetBlockNumberByNrOrHash(ctx context.Context, tmClient client.LocalClient, wm *WatermarkManager, blockNrOrHash rpc.BlockNumberOrHash) (*int64, error) { if blockNrOrHash.BlockHash != nil { // Synthetic genesis from eth_getBlockByNumber("0x0") is not stored under this hash in Tendermint. @@ -57,13 +53,10 @@ func GetBlockNumberByNrOrHash(ctx context.Context, tmClient client.LocalClient, z := int64(0) return &z, nil } - block, err := blockByHashOrNullForJSONRPC(ctx, tmClient, wm, blockNrOrHash.BlockHash[:], 1) + block, err := blockByHashRespectingWatermarks(ctx, tmClient, wm, blockNrOrHash.BlockHash[:], 1) if err != nil { return nil, err } - if block == nil { - return nil, nil - } height := block.Block.Height return &height, nil } From 5d0bccbb822d11e39ce5246025a59568dc50b5a8 Mon Sep 17 00:00:00 2001 From: Wen Date: Mon, 1 Jun 2026 23:26:47 -0400 Subject: [PATCH 7/7] evmrpc: align MockClient.BlockByHash with production unknown-hash path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review-cleanup items in this commit: 1. MockClient.BlockByHash returned (nil, errors.New("not found")) for the test sentinel hash 0xbbbb..., whereas real Tendermint returns (ResultBlock{Block: nil}, nil) which blockByHashWithRetry wraps as ErrBlockNotFoundByHash. Tests against this mock exercised a different code path from production. Mock now matches production, and TestGetTransactionByBlockHashAndIndexErrors flips the unknown-hash assertion from "expect error" to "expect null result" — same Ethereum JSON-RPC spec contract as the surrounding tests. 2. GetBlockReceipts dispatch tightened: single err declaration, no shadow-and-assign through a temporary b variable. Co-Authored-By: Claude Opus 4.7 (1M context) --- evmrpc/block.go | 25 +++++++++++-------------- evmrpc/setup_test.go | 5 ++++- evmrpc/tx_test.go | 6 +++--- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/evmrpc/block.go b/evmrpc/block.go index 1e23c66c1b..8dd90c2de4 100644 --- a/evmrpc/block.go +++ b/evmrpc/block.go @@ -303,23 +303,20 @@ func (a *BlockAPI) GetBlockReceipts(ctx context.Context, blockNrOrHash rpc.Block // (the "latest"/"safe"/"finalized"/"pending" tags) resolves to the safe-latest // height via blockByNumberOrNullForJSONRPC rather than being misread as // "block doesn't exist". - var block *coretypes.ResultBlock + var ( + block *coretypes.ResultBlock + err error + ) if blockNrOrHash.BlockHash != nil { - b, err := blockByHashOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, blockNrOrHash.BlockHash[:], 1) - if err != nil { - return nil, err - } - block = b + block, err = blockByHashOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, blockNrOrHash.BlockHash[:], 1) } else { - numberPtr, err := getBlockNumber(ctx, a.tmClient, *blockNrOrHash.BlockNumber) - if err != nil { - return nil, err + var numberPtr *int64 + if numberPtr, err = getBlockNumber(ctx, a.tmClient, *blockNrOrHash.BlockNumber); err == nil { + block, err = blockByNumberOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, numberPtr, 1) } - b, err := blockByNumberOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, numberPtr, 1) - if err != nil { - return nil, err - } - block = b + } + if err != nil { + return nil, err } if block == nil { return nil, nil diff --git a/evmrpc/setup_test.go b/evmrpc/setup_test.go index b88fc4272b..51c6806708 100644 --- a/evmrpc/setup_test.go +++ b/evmrpc/setup_test.go @@ -371,7 +371,10 @@ func (c *MockClient) BlockByHash(_ context.Context, hash bytes.HexBytes) (*coret return c.mockBlock(MockHeight2), nil } if strings.ToLower(hash.String()) == "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" { - return nil, errors.New("not found") + // Match real Tendermint behavior for unknown hashes: ResultBlock with + // Block: nil + no error. blockByHashWithRetry wraps this as + // ErrBlockNotFoundByHash, which JSON-RPC endpoints convert to null. + return &coretypes.ResultBlock{Block: nil}, nil } return c.mockBlock(MockHeight8), nil } diff --git a/evmrpc/tx_test.go b/evmrpc/tx_test.go index a43551b0b3..423995b211 100644 --- a/evmrpc/tx_test.go +++ b/evmrpc/tx_test.go @@ -471,9 +471,9 @@ func TestGetTransactionByBlockHashAndIndexErrors(t *testing.T) { resObj := map[string]interface{}{} require.Nil(t, json.Unmarshal(resBody, &resObj)) - // Should get an error for non-existent block hash - errMap := resObj["error"].(map[string]interface{}) - require.NotNil(t, errMap["message"]) + // Non-existent block hash returns null result (Ethereum JSON-RPC spec). + require.Nil(t, resObj["error"]) + require.Nil(t, resObj["result"]) body = fmt.Sprintf(`{"jsonrpc": "2.0","method": "eth_getTransactionByBlockHashAndIndex","params":["%s","0xFFFFFFFFFF"],"id":"test"}`, TestBlockHash) req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("http://%s:%d", TestAddr, TestPort), strings.NewReader(body))