imp: clean up the block fetch logic in json-rpc (#1081)

* clean up the block fetch logic in json-rpc

deduplicate some codes

fix EthBlockFromTm

fix latest block height

* add bug fix changelog

CHANGELOG.md

@@ -38,6 +38,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## Unreleased
 
+### API Breaking
+
+- (rpc) [tharsis#1081](https://github.com/tharsis/ethermint/pull/1081) Deduplicate some json-rpc logic codes, cleanup several dead functions.
+
 ### Improvements
 
 * (cli) [tharsis#1086](https://github.com/tharsis/ethermint/pull/1086) Add rollback command.
@@ -46,6 +50,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * (rpc) [tharsis#1082](https://github.com/tharsis/ethermint/pull/1082) fix gas price returned in getTransaction api.
 * (evm) [tharsis#1088](https://github.com/tharsis/ethermint/pull/1088) Fix ability to append log in tx post processing.
+* (rpc) [tharsis#1081](https://github.com/tharsis/ethermint/pull/1081) fix `debug_getBlockRlp`/`debug_printBlock` don't filter failed transactions.
 
 ## [v0.15.0] - 2022-05-09
 

rpc/backend/evm_backend.go

@@ -20,7 +20,6 @@ import (
 	"github.com/ethereum/go-ethereum/rpc"
 
 	tmrpctypes "github.com/tendermint/tendermint/rpc/core/types"
-	tmtypes "github.com/tendermint/tendermint/types"
 
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -71,7 +70,7 @@ func (b *Backend) GetBlockByNumber(blockNum types.BlockNumber, fullTx bool) (map
 		return nil, nil
 	}
 
-	res, err := b.EthBlockFromTendermint(resBlock.Block, fullTx)
+	res, err := b.EthBlockFromTendermint(resBlock, fullTx)
 	if err != nil {
 		b.logger.Debug("EthBlockFromTendermint failed", "height", blockNum, "error", err.Error())
 		return nil, err
@@ -82,61 +81,36 @@ func (b *Backend) GetBlockByNumber(blockNum types.BlockNumber, fullTx bool) (map
 
 // GetBlockByHash returns the block identified by hash.
 func (b *Backend) GetBlockByHash(hash common.Hash, fullTx bool) (map[string]interface{}, error) {
-	resBlock, err := b.clientCtx.Client.BlockByHash(b.ctx, hash.Bytes())
+	resBlock, err := b.GetTendermintBlockByHash(hash)
 	if err != nil {
-		b.logger.Debug("BlockByHash block not found", "hash", hash.Hex(), "error", err.Error())
 		return nil, err
 	}
-
-	if resBlock == nil || resBlock.Block == nil {
-		b.logger.Debug("BlockByHash block not found", "hash", hash.Hex())
+	if resBlock == nil {
+		// block not found
 		return nil, nil
 	}
 
-	return b.EthBlockFromTendermint(resBlock.Block, fullTx)
+	return b.EthBlockFromTendermint(resBlock, fullTx)
 }
 
 // BlockByNumber returns the block identified by number.
 func (b *Backend) BlockByNumber(blockNum types.BlockNumber) (*ethtypes.Block, error) {
-	height := blockNum.Int64()
-
-	switch blockNum {
-	case types.EthLatestBlockNumber:
-		currentBlockNumber, _ := b.BlockNumber()
-		if currentBlockNumber > 0 {
-			height = int64(currentBlockNumber)
-		}
-	case types.EthPendingBlockNumber:
-		currentBlockNumber, _ := b.BlockNumber()
-		if currentBlockNumber > 0 {
-			height = int64(currentBlockNumber)
-		}
-	case types.EthEarliestBlockNumber:
-		height = 1
-	default:
-		if blockNum < 0 {
-			return nil, errors.Errorf("incorrect block height: %d", height)
-		}
-	}
-
-	resBlock, err := b.clientCtx.Client.Block(b.ctx, &height)
+	resBlock, err := b.GetTendermintBlockByNumber(blockNum)
 	if err != nil {
-		b.logger.Debug("HeaderByNumber failed", "height", height)
 		return nil, err
 	}
-
-	if resBlock == nil || resBlock.Block == nil {
-		return nil, errors.Errorf("block not found for height %d", height)
+	if resBlock == nil {
+		// block not found
+		return nil, errors.Errorf("block not found for height %d", blockNum)
 	}
 
-	return b.EthBlockFromTm(resBlock.Block)
+	return b.EthBlockFromTm(resBlock)
 }
 
 // BlockByHash returns the block identified by hash.
 func (b *Backend) BlockByHash(hash common.Hash) (*ethtypes.Block, error) {
-	resBlock, err := b.clientCtx.Client.BlockByHash(b.ctx, hash.Bytes())
+	resBlock, err := b.GetTendermintBlockByHash(hash)
 	if err != nil {
-		b.logger.Debug("HeaderByHash failed", "hash", hash.Hex())
 		return nil, err
 	}
 
@@ -144,10 +118,11 @@ func (b *Backend) BlockByHash(hash common.Hash) (*ethtypes.Block, error) {
 		return nil, errors.Errorf("block not found for hash %s", hash)
 	}
 
-	return b.EthBlockFromTm(resBlock.Block)
+	return b.EthBlockFromTm(resBlock)
 }
 
-func (b *Backend) EthBlockFromTm(block *tmtypes.Block) (*ethtypes.Block, error) {
+func (b *Backend) EthBlockFromTm(resBlock *tmrpctypes.ResultBlock) (*ethtypes.Block, error) {
+	block := resBlock.Block
 	height := block.Height
 	bloom, err := b.BlockBloom(&height)
 	if err != nil {
@@ -162,23 +137,16 @@ func (b *Backend) EthBlockFromTm(block *tmtypes.Block) (*ethtypes.Block, error)
 
 	ethHeader := types.EthHeaderFromTendermint(block.Header, bloom, baseFee)
 
-	var txs []*ethtypes.Transaction
-	for _, txBz := range block.Txs {
-		tx, err := b.clientCtx.TxConfig.TxDecoder()(txBz)
-		if err != nil {
-			b.logger.Debug("failed to decode transaction in block", "height", height, "error", err.Error())
-			continue
-		}
+	resBlockResult, err := b.clientCtx.Client.BlockResults(b.ctx, &block.Height)
+	if err != nil {
+		return nil, err
+	}
 
-		for _, msg := range tx.GetMsgs() {
-			ethMsg, ok := msg.(*evmtypes.MsgEthereumTx)
-			if !ok {
-				continue
-			}
+	msgs := b.GetEthereumMsgsFromTendermintBlock(resBlock, resBlockResult)
 
-			tx := ethMsg.AsTransaction()
-			txs = append(txs, tx)
-		}
+	txs := make([]*ethtypes.Transaction, len(msgs))
+	for i, ethMsg := range msgs {
+		txs[i] = ethMsg.AsTransaction()
 	}
 
 	// TODO: add tx receipts
@@ -189,38 +157,22 @@ func (b *Backend) EthBlockFromTm(block *tmtypes.Block) (*ethtypes.Block, error)
 // GetTendermintBlockByNumber returns a Tendermint format block by block number
 func (b *Backend) GetTendermintBlockByNumber(blockNum types.BlockNumber) (*tmrpctypes.ResultBlock, error) {
 	height := blockNum.Int64()
-	currentBlockNumber, _ := b.BlockNumber()
-
-	switch blockNum {
-	case types.EthLatestBlockNumber:
-		if currentBlockNumber > 0 {
-			height = int64(currentBlockNumber)
-		}
-	case types.EthPendingBlockNumber:
-		if currentBlockNumber > 0 {
-			height = int64(currentBlockNumber)
-		}
-	case types.EthEarliestBlockNumber:
-		height = 1
-	default:
-		if blockNum < 0 {
-			return nil, errors.Errorf("cannot fetch a negative block height: %d", height)
-		}
-		if height > int64(currentBlockNumber) {
-			return nil, nil
+	if height <= 0 {
+		// fetch the latest block number from the app state, more accurate than the tendermint block store state.
+		n, err := b.BlockNumber()
+		if err != nil {
+			return nil, err
 		}
+		height = int64(n)
 	}
-
 	resBlock, err := b.clientCtx.Client.Block(b.ctx, &height)
 	if err != nil {
-		if resBlock, err = b.clientCtx.Client.Block(b.ctx, nil); err != nil {
-			b.logger.Debug("tendermint client failed to get latest block", "height", height, "error", err.Error())
-			return nil, nil
-		}
+		b.logger.Debug("tendermint client failed to get block", "height", height, "error", err.Error())
+		return nil, err
 	}
 
 	if resBlock.Block == nil {
-		b.logger.Debug("GetBlockByNumber block not found", "height", height)
+		b.logger.Debug("GetTendermintBlockByNumber block not found", "height", height)
 		return nil, nil
 	}
 
@@ -232,10 +184,11 @@ func (b *Backend) GetTendermintBlockByHash(blockHash common.Hash) (*tmrpctypes.R
 	resBlock, err := b.clientCtx.Client.BlockByHash(b.ctx, blockHash.Bytes())
 	if err != nil {
 		b.logger.Debug("tendermint client failed to get block", "blockHash", blockHash.Hex(), "error", err.Error())
+		return nil, err
 	}
 
 	if resBlock == nil || resBlock.Block == nil {
-		b.logger.Debug("GetBlockByNumber block not found", "blockHash", blockHash.Hex())
+		b.logger.Debug("GetTendermintBlockByHash block not found", "blockHash", blockHash.Hex())
 		return nil, nil
 	}
 
@@ -264,67 +217,43 @@ func (b *Backend) BlockBloom(height *int64) (ethtypes.Bloom, error) {
 
 // EthBlockFromTendermint returns a JSON-RPC compatible Ethereum block from a given Tendermint block and its block result.
 func (b *Backend) EthBlockFromTendermint(
-	block *tmtypes.Block,
+	resBlock *tmrpctypes.ResultBlock,
 	fullTx bool,
 ) (map[string]interface{}, error) {
 	ethRPCTxs := []interface{}{}
-
-	ctx := types.ContextWithHeight(block.Height)
+	block := resBlock.Block
 
 	baseFee, err := b.BaseFee(block.Height)
 	if err != nil {
 		return nil, err
 	}
 
-	resBlockResult, err := b.clientCtx.Client.BlockResults(ctx, &block.Height)
+	resBlockResult, err := b.clientCtx.Client.BlockResults(b.ctx, &block.Height)
 	if err != nil {
 		return nil, err
 	}
 
-	txResults := resBlockResult.TxsResults
-	txIndex := uint64(0)
-
-	for i, txBz := range block.Txs {
-		tx, err := b.clientCtx.TxConfig.TxDecoder()(txBz)
-		if err != nil {
-			b.logger.Debug("failed to decode transaction in block", "height", block.Height, "error", err.Error())
+	msgs := b.GetEthereumMsgsFromTendermintBlock(resBlock, resBlockResult)
+	for txIndex, ethMsg := range msgs {
+		if !fullTx {
+			hash := common.HexToHash(ethMsg.Hash)
+			ethRPCTxs = append(ethRPCTxs, hash)
 			continue
 		}
 
-		for _, msg := range tx.GetMsgs() {
-			ethMsg, ok := msg.(*evmtypes.MsgEthereumTx)
-			if !ok {
-				continue
-			}
-
-			tx := ethMsg.AsTransaction()
-
-			// check tx exists on EVM by cross checking with blockResults
-			if txResults[i].Code != 0 {
-				b.logger.Debug("invalid tx result code", "hash", tx.Hash().Hex())
-				continue
-			}
-
-			if !fullTx {
-				hash := tx.Hash()
-				ethRPCTxs = append(ethRPCTxs, hash)
-				continue
-			}
-
-			rpcTx, err := types.NewRPCTransaction(
-				tx,
-				common.BytesToHash(block.Hash()),
-				uint64(block.Height),
-				txIndex,
-				baseFee,
-			)
-			if err != nil {
-				b.logger.Debug("NewTransactionFromData for receipt failed", "hash", tx.Hash().Hex(), "error", err.Error())
-				continue
-			}
-			ethRPCTxs = append(ethRPCTxs, rpcTx)
-			txIndex++
+		tx := ethMsg.AsTransaction()
+		rpcTx, err := types.NewRPCTransaction(
+			tx,
+			common.BytesToHash(block.Hash()),
+			uint64(block.Height),
+			uint64(txIndex),
+			baseFee,
+		)
+		if err != nil {
+			b.logger.Debug("NewTransactionFromData for receipt failed", "hash", tx.Hash().Hex(), "error", err.Error())
+			continue
 		}
+		ethRPCTxs = append(ethRPCTxs, rpcTx)
 	}
 
 	bloom, err := b.BlockBloom(&block.Height)
@@ -336,6 +265,7 @@ func (b *Backend) EthBlockFromTendermint(
 		ConsAddress: sdk.ConsAddress(block.Header.ProposerAddress).String(),
 	}
 
+	ctx := types.ContextWithHeight(block.Height)
 	res, err := b.queryClient.ValidatorAccount(ctx, req)
 	if err != nil {
 		b.logger.Debug(
@@ -361,10 +291,10 @@ func (b *Backend) EthBlockFromTendermint(
 
 	gasUsed := uint64(0)
 
-	for _, txsResult := range txResults {
+	for _, txsResult := range resBlockResult.TxsResults {
 		// workaround for cosmos-sdk bug. https://github.com/cosmos/cosmos-sdk/issues/10832
 		if txsResult.GetCode() == 11 && txsResult.GetLog() == "no block gas left to run tx: out of gas" {
-			// block gas limit has exceeded, other txs must have failed with same reason.
+			// block gas limit has exceeded, other txs must have failed for the same reason.
 			break
 		}
 		gasUsed += uint64(txsResult.GetGasUsed())
@@ -386,31 +316,13 @@ func (b *Backend) CurrentHeader() *ethtypes.Header {
 
 // HeaderByNumber returns the block header identified by height.
 func (b *Backend) HeaderByNumber(blockNum types.BlockNumber) (*ethtypes.Header, error) {
-	height := blockNum.Int64()
-
-	switch blockNum {
-	case types.EthLatestBlockNumber:
-		currentBlockNumber, _ := b.BlockNumber()
-		if currentBlockNumber > 0 {
-			height = int64(currentBlockNumber)
-		}
-	case types.EthPendingBlockNumber:
-		currentBlockNumber, _ := b.BlockNumber()
-		if currentBlockNumber > 0 {
-			height = int64(currentBlockNumber)
-		}
-	case types.EthEarliestBlockNumber:
-		height = 1
-	default:
-		if blockNum < 0 {
-			return nil, errors.Errorf("incorrect block height: %d", height)
-		}
+	resBlock, err := b.GetTendermintBlockByNumber(blockNum)
+	if err != nil {
+		return nil, err
 	}
 
-	resBlock, err := b.clientCtx.Client.Block(b.ctx, &height)
-	if err != nil {
-		b.logger.Debug("HeaderByNumber failed")
-		return nil, err
+	if resBlock == nil {
+		return nil, errors.Errorf("block not found for height %d", blockNum)
 	}
 
 	bloom, err := b.BlockBloom(&resBlock.Block.Height)
@@ -430,13 +342,11 @@ func (b *Backend) HeaderByNumber(blockNum types.BlockNumber) (*ethtypes.Header,
 
 // HeaderByHash returns the block header identified by hash.
 func (b *Backend) HeaderByHash(blockHash common.Hash) (*ethtypes.Header, error) {
-	resBlock, err := b.clientCtx.Client.BlockByHash(b.ctx, blockHash.Bytes())
+	resBlock, err := b.GetTendermintBlockByHash(blockHash)
 	if err != nil {
-		b.logger.Debug("HeaderByHash failed", "hash", blockHash.Hex())
 		return nil, err
 	}
-
-	if resBlock == nil || resBlock.Block == nil {
+	if resBlock == nil {
 		return nil, errors.Errorf("block not found for hash %s", blockHash.Hex())
 	}
 
@@ -498,36 +408,15 @@ func (b *Backend) GetLogsByHeight(height *int64) ([][]*ethtypes.Log, error) {
 
 // GetLogs returns all the logs from all the ethereum transactions in a block.
 func (b *Backend) GetLogs(hash common.Hash) ([][]*ethtypes.Log, error) {
-	block, err := b.clientCtx.Client.BlockByHash(b.ctx, hash.Bytes())
+	resBlock, err := b.GetTendermintBlockByHash(hash)
 	if err != nil {
 		return nil, err
 	}
-	return b.GetLogsByHeight(&block.Block.Header.Height)
-}
-
-func (b *Backend) GetLogsByNumber(blockNum types.BlockNumber) ([][]*ethtypes.Log, error) {
-	height := blockNum.Int64()
-
-	switch blockNum {
-	case types.EthLatestBlockNumber:
-		currentBlockNumber, _ := b.BlockNumber()
-		if currentBlockNumber > 0 {
-			height = int64(currentBlockNumber)
-		}
-	case types.EthPendingBlockNumber:
-		currentBlockNumber, _ := b.BlockNumber()
-		if currentBlockNumber > 0 {
-			height = int64(currentBlockNumber)
-		}
-	case types.EthEarliestBlockNumber:
-		height = 1
-	default:
-		if blockNum < 0 {
-			return nil, errors.Errorf("incorrect block height: %d", height)
-		}
+	if resBlock == nil {
+		return nil, errors.Errorf("block not found for hash %s", hash)
 	}
 
-	return b.GetLogsByHeight(&height)
+	return b.GetLogsByHeight(&resBlock.Block.Header.Height)
 }
 
 // BloomStatus returns the BloomBitsBlocks and the number of processed sections maintained

rpc/namespaces/ethereum/eth/api.go

@@ -342,7 +342,7 @@ 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", "height", blockNum.Int64())
-	block, err := e.clientCtx.Client.Block(e.ctx, blockNum.TmHeight())
+	block, err := e.backend.GetTendermintBlockByNumber(blockNum)
 	if err != nil {
 		e.logger.Debug("block not found", "height", blockNum.Int64(), "error", err.Error())
 		return nil
@@ -800,7 +800,7 @@ func (e *PublicAPI) GetTransactionByBlockHashAndIndex(hash common.Hash, idx hexu
 func (e *PublicAPI) GetTransactionByBlockNumberAndIndex(blockNum rpctypes.BlockNumber, idx hexutil.Uint) (*rpctypes.RPCTransaction, error) {
 	e.logger.Debug("eth_getTransactionByBlockNumberAndIndex", "number", blockNum, "index", idx)
 
-	block, err := e.clientCtx.Client.Block(e.ctx, blockNum.TmHeight())
+	block, err := e.backend.GetTendermintBlockByNumber(blockNum)
 	if err != nil {
 		e.logger.Debug("block not found", "height", blockNum.Int64(), "error", err.Error())
 		return nil, nil

rpc/namespaces/ethereum/eth/filters/api.go

@@ -29,7 +29,7 @@ type Backend interface {
 	HeaderByNumber(blockNum types.BlockNumber) (*ethtypes.Header, error)
 	HeaderByHash(blockHash common.Hash) (*ethtypes.Header, error)
 	GetLogs(blockHash common.Hash) ([][]*ethtypes.Log, error)
-	GetLogsByNumber(blockNum types.BlockNumber) ([][]*ethtypes.Log, error)
+	GetLogsByHeight(*int64) ([][]*ethtypes.Log, error)
 	BlockBloom(height *int64) (ethtypes.Bloom, error)
 
 	BloomStatus() (uint64, uint64)

rpc/namespaces/ethereum/eth/filters/filters.go

@@ -172,7 +172,7 @@ func (f *Filter) blockLogs(height int64, bloom ethtypes.Bloom) ([]*ethtypes.Log,
 
 	// DANGER: do not call GetLogs(header.Hash())
 	// eth header's hash doesn't match tm block hash
-	logsList, err := f.backend.GetLogsByNumber(types.BlockNumber(height))
+	logsList, err := f.backend.GetLogsByHeight(&height)
 	if err != nil {
 		return []*ethtypes.Log{}, errors.Wrapf(err, "failed to fetch logs block number %d", height)
 	}

rpc/types/utils.go

@@ -3,7 +3,6 @@ package types
 import (
 	"bytes"
 	"context"
-	"encoding/hex"
 	"fmt"
 	"math/big"
 	"strconv"
@@ -132,37 +131,6 @@ func FormatBlock(
 	return result
 }
 
-type DataError interface {
-	Error() string          // returns the message
-	ErrorData() interface{} // returns the error data
-}
-
-type dataError struct {
-	msg  string
-	data string
-}
-
-func (d *dataError) Error() string {
-	return d.msg
-}
-
-func (d *dataError) ErrorData() interface{} {
-	return d.data
-}
-
-type SDKTxLogs struct {
-	Log string `json:"log"`
-}
-
-const LogRevertedFlag = "transaction reverted"
-
-func ErrRevertedWith(data []byte) DataError {
-	return &dataError{
-		msg:  "VM execution error.",
-		data: fmt.Sprintf("0x%s", hex.EncodeToString(data)),
-	}
-}
-
 // NewTransactionFromMsg returns a transaction that will serialize to the RPC
 // representation, with the given location metadata set (if available).
 func NewTransactionFromMsg(