From 66d1d5d1f8c8c6ba76e80e6964fac86c0233561c Mon Sep 17 00:00:00 2001 From: Alex Sharov Date: Thu, 1 Apr 2021 12:15:22 +0700 Subject: [PATCH] corner_case_return_null_when_block_not (#1646) --- .../commands/corner_cases_support_test.go | 59 +++++++++++++++++++ cmd/rpcdaemon/commands/debug_api_test.go | 2 + cmd/rpcdaemon/commands/eth_api_test.go | 1 + cmd/rpcdaemon/commands/eth_block.go | 12 ++-- cmd/rpcdaemon/commands/eth_call_test.go | 1 + cmd/rpcdaemon/commands/eth_receipts.go | 2 +- cmd/rpcdaemon/commands/eth_txs.go | 6 +- cmd/rpcdaemon/commands/eth_uncles.go | 15 ++--- cmd/rpcdaemon/commands/trace_adhoc_test.go | 2 + 9 files changed, 83 insertions(+), 17 deletions(-) create mode 100644 cmd/rpcdaemon/commands/corner_cases_support_test.go diff --git a/cmd/rpcdaemon/commands/corner_cases_support_test.go b/cmd/rpcdaemon/commands/corner_cases_support_test.go new file mode 100644 index 00000000000..98ec9003ac1 --- /dev/null +++ b/cmd/rpcdaemon/commands/corner_cases_support_test.go @@ -0,0 +1,59 @@ +package commands + +import ( + "context" + "testing" + + "github.com/ledgerwatch/turbo-geth/common" + "github.com/ledgerwatch/turbo-geth/rpc" + "github.com/stretchr/testify/require" +) + +// TestNotFoundMustReturnNil - next methods - when record not found in db - must return nil instead of error +// see https://github.com/ledgerwatch/turbo-geth/issues/1645 +func TestNotFoundMustReturnNil(t *testing.T) { + require := require.New(t) + db, err := createTestKV() + if err != nil { + t.Fatalf("create test db: %v", err) + } + defer db.Close() + api := NewEthAPI(db, nil, 5000000, nil, nil) + ctx := context.Background() + + a, err := api.GetTransactionByBlockNumberAndIndex(ctx, 10_000, 1) + require.Nil(a) + require.Nil(err) + + b, err := api.GetTransactionByBlockHashAndIndex(ctx, common.Hash{}, 1) + require.Nil(b) + require.Nil(err) + + c, err := api.GetTransactionByBlockNumberAndIndex(ctx, 10_000, 1) + require.Nil(c) + require.Nil(err) + + d, err := api.GetTransactionReceipt(ctx, common.Hash{}) + require.Nil(d) + require.Nil(err) + + e, err := api.GetBlockByHash(ctx, rpc.BlockNumberOrHashWithHash(common.Hash{}, true), false) + require.Nil(e) + require.Nil(err) + + f, err := api.GetBlockByNumber(ctx, 10_000, false) + require.Nil(f) + require.Nil(err) + + g, err := api.GetUncleByBlockHashAndIndex(ctx, common.Hash{}, 1) + require.Nil(g) + require.Nil(err) + + h, err := api.GetUncleByBlockNumberAndIndex(ctx, 10_000, 1) + require.Nil(h) + require.Nil(err) + + j, err := api.GetBlockTransactionCountByNumber(ctx, 10_000) + require.Nil(j) + require.Nil(err) +} diff --git a/cmd/rpcdaemon/commands/debug_api_test.go b/cmd/rpcdaemon/commands/debug_api_test.go index 8785b70472c..3abec7c2c35 100644 --- a/cmd/rpcdaemon/commands/debug_api_test.go +++ b/cmd/rpcdaemon/commands/debug_api_test.go @@ -36,6 +36,7 @@ func TestTraceTransaction(t *testing.T) { if err != nil { t.Fatalf("create test db: %v", err) } + defer db.Close() api := NewPrivateDebugAPI(db, 0, nil) for _, tt := range debugTraceTransactionTests { result, err1 := api.TraceTransaction(context.Background(), common.HexToHash(tt.txHash), &tracers.TraceConfig{}) @@ -60,6 +61,7 @@ func TestTraceTransactionNoRefund(t *testing.T) { if err != nil { t.Fatalf("create test db: %v", err) } + defer db.Close() api := NewPrivateDebugAPI(db, 0, nil) for _, tt := range debugTraceTransactionNoRefundTests { var norefunds bool = true diff --git a/cmd/rpcdaemon/commands/eth_api_test.go b/cmd/rpcdaemon/commands/eth_api_test.go index aa8bbb93a30..61dc132b8a9 100644 --- a/cmd/rpcdaemon/commands/eth_api_test.go +++ b/cmd/rpcdaemon/commands/eth_api_test.go @@ -12,6 +12,7 @@ func TestGetTransactionReceipt(t *testing.T) { if err != nil { t.Fatalf("create test db: %v", err) } + defer db.Close() api := NewEthAPI(db, nil, 5000000, nil, nil) // Call GetTransactionReceipt for transaction which is not in the database if _, err := api.GetTransactionReceipt(context.Background(), common.Hash{}); err != nil { diff --git a/cmd/rpcdaemon/commands/eth_block.go b/cmd/rpcdaemon/commands/eth_block.go index 31d2ca92571..41c50282166 100644 --- a/cmd/rpcdaemon/commands/eth_block.go +++ b/cmd/rpcdaemon/commands/eth_block.go @@ -31,7 +31,7 @@ func (api *APIImpl) GetBlockByNumber(ctx context.Context, number rpc.BlockNumber return nil, err } if block == nil { - return nil, fmt.Errorf("block not found: %d", blockNum) + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } td, err := rawdb.ReadTd(ethdb.NewRoTxDb(tx), block.Hash(), blockNum) @@ -56,10 +56,10 @@ func (api *APIImpl) GetBlockByHash(ctx context.Context, numberOrHash rpc.BlockNu // some web3.js based apps (like ethstats client) for some reason call // eth_getBlockByHash with a block number as a parameter // so no matter how weird that is, we would love to support that. - if numberOrHash.BlockNumber != nil { - return api.GetBlockByNumber(ctx, *numberOrHash.BlockNumber, fullTx) + if numberOrHash.BlockNumber == nil { + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } - return nil, fmt.Errorf("block not found") + return api.GetBlockByNumber(ctx, *numberOrHash.BlockNumber, fullTx) } hash := *numberOrHash.BlockHash @@ -76,7 +76,7 @@ func (api *APIImpl) GetBlockByHash(ctx context.Context, numberOrHash rpc.BlockNu return nil, err } if block == nil { - return nil, fmt.Errorf("block not found: %x", hash) + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } number := block.NumberU64() @@ -114,7 +114,7 @@ func (api *APIImpl) GetBlockTransactionCountByNumber(ctx context.Context, blockN return nil, err } if block == nil { - return nil, fmt.Errorf("block not found: %d", blockNum) + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } n := hexutil.Uint(len(block.Transactions())) return &n, nil diff --git a/cmd/rpcdaemon/commands/eth_call_test.go b/cmd/rpcdaemon/commands/eth_call_test.go index 289aefd00e7..42591036305 100644 --- a/cmd/rpcdaemon/commands/eth_call_test.go +++ b/cmd/rpcdaemon/commands/eth_call_test.go @@ -13,6 +13,7 @@ func TestEstimateGas(t *testing.T) { if err != nil { t.Fatalf("create test db: %v", err) } + defer db.Close() api := NewEthAPI(db, nil, 5000000, nil, nil) var from = common.HexToAddress("0x71562b71999873db5b286df957af199ec94617f7") var to = common.HexToAddress("0x0d3ab14bbad3d99f4203bd7a11acb94882050e7e") diff --git a/cmd/rpcdaemon/commands/eth_receipts.go b/cmd/rpcdaemon/commands/eth_receipts.go index e146f9f752d..9ae40b95e02 100644 --- a/cmd/rpcdaemon/commands/eth_receipts.go +++ b/cmd/rpcdaemon/commands/eth_receipts.go @@ -203,7 +203,7 @@ func (api *APIImpl) GetTransactionReceipt(ctx context.Context, hash common.Hash) // Retrieve the transaction and assemble its EVM context txn, blockHash, blockNumber, txIndex := rawdb.ReadTransaction(ethdb.NewRoTxDb(tx), hash) if txn == nil { - return nil, nil + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } cc, err := api.chainConfig(tx) diff --git a/cmd/rpcdaemon/commands/eth_txs.go b/cmd/rpcdaemon/commands/eth_txs.go index 0a3fbabbd0a..a807482c2d8 100644 --- a/cmd/rpcdaemon/commands/eth_txs.go +++ b/cmd/rpcdaemon/commands/eth_txs.go @@ -22,7 +22,7 @@ func (api *APIImpl) GetTransactionByHash(ctx context.Context, hash common.Hash) // https://infura.io/docs/ethereum/json-rpc/eth-getTransactionByHash txn, blockHash, blockNumber, txIndex := rawdb.ReadTransaction(ethdb.NewRoTxDb(tx), hash) if txn == nil { - return nil, fmt.Errorf("transaction %#x not found", hash) + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } return newRPCTransaction(txn, blockHash, blockNumber, txIndex), nil } @@ -41,7 +41,7 @@ func (api *APIImpl) GetTransactionByBlockHashAndIndex(ctx context.Context, block return nil, err } if block == nil { - return nil, fmt.Errorf("block %#x not found", blockHash) + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } txs := block.Transactions() @@ -71,7 +71,7 @@ func (api *APIImpl) GetTransactionByBlockNumberAndIndex(ctx context.Context, blo return nil, err } if block == nil { - return nil, fmt.Errorf("block %d not found", blockNum) + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } txs := block.Transactions() diff --git a/cmd/rpcdaemon/commands/eth_uncles.go b/cmd/rpcdaemon/commands/eth_uncles.go index 5bcfca90c42..28a6cc8c951 100644 --- a/cmd/rpcdaemon/commands/eth_uncles.go +++ b/cmd/rpcdaemon/commands/eth_uncles.go @@ -2,7 +2,6 @@ package commands import ( "context" - "fmt" "github.com/ledgerwatch/turbo-geth/common" "github.com/ledgerwatch/turbo-geth/common/hexutil" @@ -31,7 +30,7 @@ func (api *APIImpl) GetUncleByBlockNumberAndIndex(ctx context.Context, number rp return nil, err } if block == nil { - return nil, fmt.Errorf("block not found: %d", blockNum) + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } hash := block.Hash() additionalFields := make(map[string]interface{}) @@ -63,7 +62,7 @@ func (api *APIImpl) GetUncleByBlockHashAndIndex(ctx context.Context, hash common return nil, err } if block == nil { - return nil, fmt.Errorf("block not found: %x", hash) + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } number := block.NumberU64() additionalFields := make(map[string]interface{}) @@ -102,9 +101,10 @@ func (api *APIImpl) GetUncleCountByBlockNumber(ctx context.Context, number rpc.B if err != nil { return nil, err } - if block != nil { - n = hexutil.Uint(len(block.Uncles())) + if block == nil { + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } + n = hexutil.Uint(len(block.Uncles())) return &n, nil } @@ -121,8 +121,9 @@ func (api *APIImpl) GetUncleCountByBlockHash(ctx context.Context, hash common.Ha if err != nil { return &n, err } - if block != nil { - n = hexutil.Uint(len(block.Uncles())) + if block == nil { + return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645 } + n = hexutil.Uint(len(block.Uncles())) return &n, nil } diff --git a/cmd/rpcdaemon/commands/trace_adhoc_test.go b/cmd/rpcdaemon/commands/trace_adhoc_test.go index 112e8a24141..dd2eda0af33 100644 --- a/cmd/rpcdaemon/commands/trace_adhoc_test.go +++ b/cmd/rpcdaemon/commands/trace_adhoc_test.go @@ -15,6 +15,7 @@ func TestEmptyQuery(t *testing.T) { if err != nil { t.Fatalf("create test db: %v", err) } + defer db.Close() api := NewTraceAPI(db, nil, &cli.Flags{}) // Call GetTransactionReceipt for transaction which is not in the database var latest rpc.BlockNumber = rpc.LatestBlockNumber @@ -34,6 +35,7 @@ func TestCoinbaseBalance(t *testing.T) { if err != nil { t.Fatalf("create test db: %v", err) } + defer db.Close() api := NewTraceAPI(db, nil, &cli.Flags{}) // Call GetTransactionReceipt for transaction which is not in the database var latest rpc.BlockNumber = rpc.LatestBlockNumber