From 071ae3f618a10bbe5c4204692cbd2b770acbf0d2 Mon Sep 17 00:00:00 2001
From: Calvin Lau <38898718+calvinaco@users.noreply.github.com>
Date: Fri, 16 Jul 2021 18:13:15 +0800
Subject: [PATCH] rpc: fix `eth_getBlockTransactionCountByNumber` crash on
 block not found (#254)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Fix GetBlockTransactionCountByNumber crashed on block not found

* Fix getTransactionByBlock*

* Update log based on review

* Apply suggestions from code review

* Update ethereum/rpc/namespaces/eth/api.go

* Apply suggestions from code review

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
---
 ethereum/rpc/namespaces/eth/api.go | 24 +++++++++-
 tests/rpc/rpc_test.go              | 75 ++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/ethereum/rpc/namespaces/eth/api.go b/ethereum/rpc/namespaces/eth/api.go
index 3b8d4334..91366615 100644
--- a/ethereum/rpc/namespaces/eth/api.go
+++ b/ethereum/rpc/namespaces/eth/api.go
@@ -273,6 +273,12 @@ func (e *PublicAPI) GetBlockTransactionCountByHash(hash common.Hash) *hexutil.Ui
 
 	resBlock, err := e.clientCtx.Client.BlockByHash(e.ctx, hash.Bytes())
 	if err != nil {
+		e.logger.Debug("block not found", "hash", hash.Hex(), "error", err.Error())
+		return nil
+	}
+
+	if resBlock.Block == nil {
+		e.logger.Debug("block not found", "hash", hash.Hex())
 		return nil
 	}
 
@@ -282,9 +288,15 @@ func (e *PublicAPI) GetBlockTransactionCountByHash(hash common.Hash) *hexutil.Ui
 
 // GetBlockTransactionCountByNumber returns the number of transactions in the block identified by number.
 func (e *PublicAPI) GetBlockTransactionCountByNumber(blockNum rpctypes.BlockNumber) *hexutil.Uint {
-	e.logger.Debug("eth_getBlockTransactionCountByNumber", "block number", blockNum)
+	e.logger.Debug("eth_getBlockTransactionCountByNumber", "height", blockNum.Int64())
 	resBlock, err := e.clientCtx.Client.Block(e.ctx, blockNum.TmHeight())
 	if err != nil {
+		e.logger.Debug("block not found", "height", blockNum.Int64(), "error", err.Error())
+		return nil
+	}
+
+	if resBlock.Block == nil {
+		e.logger.Debug("block not found", "height", blockNum.Int64())
 		return nil
 	}
 
@@ -668,6 +680,11 @@ func (e *PublicAPI) GetTransactionByBlockHashAndIndex(hash common.Hash, idx hexu
 		return nil, nil
 	}
 
+	if resBlock.Block == nil {
+		e.logger.Debug("block not found", "hash", hash.Hex())
+		return nil, nil
+	}
+
 	i := int(idx)
 	if i >= len(resBlock.Block.Txs) {
 		e.logger.Debug("block txs index out of bound", "index", i)
@@ -706,6 +723,11 @@ func (e *PublicAPI) GetTransactionByBlockNumberAndIndex(blockNum rpctypes.BlockN
 		return nil, nil
 	}
 
+	if resBlock.Block == nil {
+		e.logger.Debug("block not found", "height", blockNum.Int64())
+		return nil, nil
+	}
+
 	i := int(idx)
 	if i >= len(resBlock.Block.Txs) {
 		e.logger.Debug("block txs index out of bound", "index", i)
diff --git a/tests/rpc/rpc_test.go b/tests/rpc/rpc_test.go
index c4466792..5f6a4b83 100644
--- a/tests/rpc/rpc_test.go
+++ b/tests/rpc/rpc_test.go
@@ -214,6 +214,81 @@ func TestEth_GetTransactionCount(t *testing.T) {
 	require.Equal(t, prev, post-1)
 }
 
+func TestETH_GetBlockTransactionCountByHash(t *testing.T) {
+	txHash := sendTestTransaction(t)
+
+	time.Sleep(time.Second * 5)
+
+	param := []string{txHash.String()}
+	rpcRes := call(t, "eth_getTransactionReceipt", param)
+	require.Nil(t, rpcRes.Error)
+
+	receipt := make(map[string]interface{})
+	err := json.Unmarshal(rpcRes.Result, &receipt)
+	require.NoError(t, err)
+	require.NotEmpty(t, receipt)
+
+	blockHash := receipt["blockHash"].(string)
+
+	param = []string{blockHash}
+	rpcRes = call(t, "eth_getBlockTransactionCountByHash", param)
+
+	var res hexutil.Uint
+	err = res.UnmarshalJSON(rpcRes.Result)
+	require.NoError(t, err)
+	require.Equal(t, "0x1", res.String())
+}
+
+func TestETH_GetBlockTransactionCountByHash_BlockHashNotFound(t *testing.T) {
+	anyBlockHash := "0xb3b20624f8f0f86eb50dd04688409e5cea4bd02d700bf6e79e9384d47d6a5a35"
+	param := []string{anyBlockHash}
+	rpcRes := call(t, "eth_getBlockTransactionCountByHash", param)
+
+	var result interface{}
+	err := json.Unmarshal(rpcRes.Result, &result)
+	require.NoError(t, err)
+	require.Nil(t, result)
+}
+
+func TestETH_GetTransactionByBlockHashAndIndex(t *testing.T) {
+	txHash := sendTestTransaction(t)
+
+	time.Sleep(time.Second * 5)
+
+	param := []string{txHash.String()}
+	rpcRes := call(t, "eth_getTransactionReceipt", param)
+	require.Nil(t, rpcRes.Error)
+
+	receipt := make(map[string]interface{})
+	err := json.Unmarshal(rpcRes.Result, &receipt)
+	require.NoError(t, err)
+	require.NotEmpty(t, receipt)
+
+	blockHash := receipt["blockHash"].(string)
+
+	param = []string{blockHash, "0x0"}
+	rpcRes = call(t, "eth_getTransactionByBlockHashAndIndex", param)
+
+	tx := make(map[string]interface{})
+	err = json.Unmarshal(rpcRes.Result, &tx)
+	require.NoError(t, err)
+	require.NotNil(t, tx)
+	require.Equal(t, blockHash, tx["blockHash"].(string))
+	require.Equal(t, "0x0", tx["transactionIndex"].(string))
+}
+
+func TestETH_GetTransactionByBlockHashAndIndex_BlockHashNotFound(t *testing.T) {
+	anyBlockHash := "0xb3b20624f8f0f86eb50dd04688409e5cea4bd02d700bf6e79e9384d47d6a5a35"
+
+	param := []string{anyBlockHash, "0x0"}
+	rpcRes := call(t, "eth_getTransactionByBlockHashAndIndex", param)
+
+	var result interface{}
+	err := json.Unmarshal(rpcRes.Result, &result)
+	require.NoError(t, err)
+	require.Nil(t, result)
+}
+
 func TestEth_GetTransactionLogs(t *testing.T) {
 	// TODO: this test passes on when run on its own, but fails when run with the other tests
 	if testing.Short() {