diff --git a/evmrpc/block.go b/evmrpc/block.go index 32c3a446e3..8dd90c2de4 100644 --- a/evmrpc/block.go +++ b/evmrpc/block.go @@ -4,7 +4,6 @@ import ( "context" "crypto/sha256" "encoding/hex" - "errors" "fmt" "math/big" "strings" @@ -172,10 +171,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 +190,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 +219,15 @@ 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) - if errors.Is(err, ErrBlockNotFoundByHash) { - return nil, nil - } + // Ethereum JSON-RPC: non-existent block hash (unknown OR above safe latest) + // => null, not an error. The helper handles both cases. + block, err := blockByHashOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, blockHash[:], 1) if err != nil { return nil, err } + if block == nil { + return nil, nil + } // Validate EVM block height for pacific-1 chain sdkCtx := a.ctxProvider(LatestCtxHeight) @@ -261,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) } @@ -289,18 +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 - heightPtr, err := GetBlockNumberByNrOrHash(ctx, a.tmClient, a.watermarks, blockNrOrHash) - if errors.Is(err, ErrBlockNotFoundByHash) { - return nil, nil + // Ethereum JSON-RPC: non-existent / above-watermark block => null, not an error. + // 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 + err error + ) + if blockNrOrHash.BlockHash != nil { + block, err = blockByHashOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, blockNrOrHash.BlockHash[:], 1) + } else { + var numberPtr *int64 + if numberPtr, err = getBlockNumber(ctx, a.tmClient, *blockNrOrHash.BlockNumber); err == nil { + block, err = blockByNumberOrNullForJSONRPC(ctx, a.tmClient, a.watermarks, numberPtr, 1) + } } if err != nil { return nil, err } - - block, err := blockByNumberRespectingWatermarks(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 diff --git a/evmrpc/height_availability_test.go b/evmrpc/height_availability_test.go index 9cbeee1e94..ff7acff152 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 @@ -178,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/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.go b/evmrpc/tx.go index aec8230980..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 @@ -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..423995b211 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) { @@ -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)) diff --git a/evmrpc/watermark_manager.go b/evmrpc/watermark_manager.go index 03d0cf89d3..04d4aa206e 100644 --- a/evmrpc/watermark_manager.go +++ b/evmrpc/watermark_manager.go @@ -270,6 +270,50 @@ 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. 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, + wm *WatermarkManager, + hash []byte, + maxRetries int, +) (*coretypes.ResultBlock, error) { + block, err := blockByHashRespectingWatermarks(ctx, c, wm, hash, maxRetries) + if errors.Is(err, ErrBlockHeightNotYetAvailable) || errors.Is(err, ErrBlockNotFoundByHash) { + return nil, nil + } + return block, err +} + func (m *WatermarkManager) fetchTendermintWatermarks(ctx context.Context) (int64, int64, error) { if m.tmClient == nil { return 0, 0, errNoHeightSource 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) + }) +}