From 724bc7690a133b75e80756653538d250b9438818 Mon Sep 17 00:00:00 2001 From: HuangYi Date: Thu, 11 Aug 2022 19:00:26 +0800 Subject: [PATCH] address review suggestions - test indexer in backend unit test - add comments --- default.nix | 2 +- docs/api/proto-docs.md | 2 +- server/indexer.go => indexer/kv_indexer.go | 71 +++++++++++----------- proto/ethermint/types/v1/indexer.proto | 3 +- rpc/backend/backend.go | 8 ++- rpc/backend/backend_suite_test.go | 7 ++- rpc/backend/blocks_info.go | 2 +- rpc/backend/evm_backend_test.go | 4 +- rpc/backend/tx_info.go | 5 +- rpc/backend/utils.go | 15 ----- rpc/types/utils.go | 16 +++++ server/indexer_cmd.go | 14 +++-- server/start.go | 8 +-- types/indexer.pb.go | 3 +- 14 files changed, 91 insertions(+), 69 deletions(-) rename server/indexer.go => indexer/kv_indexer.go (98%) diff --git a/default.nix b/default.nix index 4bc36173d6..c319aeaf69 100644 --- a/default.nix +++ b/default.nix @@ -17,7 +17,7 @@ in buildGoApplication rec { inherit pname version tags ldflags; src = lib.sourceByRegex ./. [ - "^(x|app|cmd|client|server|crypto|rpc|types|encoding|ethereum|testutil|version|go.mod|go.sum|gomod2nix.toml)($|/.*)" + "^(x|app|cmd|client|server|crypto|rpc|types|encoding|ethereum|indexer|testutil|version|go.mod|go.sum|gomod2nix.toml)($|/.*)" "^tests(/.*[.]go)?$" ]; modules = ./gomod2nix.toml; diff --git a/docs/api/proto-docs.md b/docs/api/proto-docs.md index 803e10cdcc..a9d9cbf3a3 100644 --- a/docs/api/proto-docs.md +++ b/docs/api/proto-docs.md @@ -1199,7 +1199,7 @@ TxResult is the value stored in eth tx indexer | `height` | [int64](#int64) | | the block height | | `tx_index` | [uint32](#uint32) | | cosmos tx index | | `msg_index` | [uint32](#uint32) | | the msg index in a batch tx | -| `eth_tx_index` | [int32](#int32) | | eth tx index | +| `eth_tx_index` | [int32](#int32) | | eth tx index, the index in the list of valid eth tx in the block, aka. the transaction list returned by eth_getBlock api. | | `failed` | [bool](#bool) | | if the eth tx is failed | | `gas_used` | [uint64](#uint64) | | gas used by tx, if exceeds block gas limit, it's set to gas limit which is what's actually deducted by ante handler. | | `cumulative_gas_used` | [uint64](#uint64) | | the cumulative gas used within current batch tx | diff --git a/server/indexer.go b/indexer/kv_indexer.go similarity index 98% rename from server/indexer.go rename to indexer/kv_indexer.go index 0cf9449c50..bc8afce4fa 100644 --- a/server/indexer.go +++ b/indexer/kv_indexer.go @@ -1,4 +1,4 @@ -package server +package indexer import ( "fmt" @@ -9,7 +9,6 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authante "github.com/cosmos/cosmos-sdk/x/auth/ante" "github.com/ethereum/go-ethereum/common" - "github.com/evmos/ethermint/rpc/backend" rpctypes "github.com/evmos/ethermint/rpc/types" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/log" @@ -57,7 +56,7 @@ func (kv *KVIndexer) IndexBlock(block *tmtypes.Block, txResults []*abci.Response var ethTxIndex int32 for txIndex, tx := range block.Txs { result := txResults[txIndex] - if !backend.TxSuccessOrExceedsBlockGasLimit(result) { + if !rpctypes.TxSuccessOrExceedsBlockGasLimit(result) { continue } @@ -131,31 +130,6 @@ func (kv *KVIndexer) FirstIndexedBlock() (int64, error) { return LoadFirstBlock(kv.db) } -// isEthTx check if the tx is an eth tx -func isEthTx(tx sdk.Tx) bool { - extTx, ok := tx.(authante.HasExtensionOptionsTx) - if !ok { - return false - } - opts := extTx.GetExtensionOptions() - if len(opts) != 1 || opts[0].GetTypeUrl() != "/ethermint.evm.v1.ExtensionOptionsEthereumTx" { - return false - } - return true -} - -// saveTxResult index the txResult into the kv db batch -func saveTxResult(codec codec.Codec, batch dbm.Batch, txHash common.Hash, txResult *ethermint.TxResult) error { - bz := codec.MustMarshal(txResult) - if err := batch.Set(TxHashKey(txHash), bz); err != nil { - return sdkerrors.Wrap(err, "set tx-hash key") - } - if err := batch.Set(TxIndexKey(txResult.Height, txResult.EthTxIndex), txHash.Bytes()); err != nil { - return sdkerrors.Wrap(err, "set tx-index key") - } - return nil -} - // GetByTxHash finds eth tx by eth tx hash func (kv *KVIndexer) GetByTxHash(hash common.Hash) (*ethermint.TxResult, error) { bz, err := kv.db.Get(TxHashKey(hash)) @@ -196,14 +170,6 @@ func TxIndexKey(blockNumber int64, txIndex int32) []byte { return append(append([]byte{KeyPrefixTxIndex}, bz1...), bz2...) } -func parseBlockNumberFromKey(key []byte) (int64, error) { - if len(key) != TxIndexKeyLength { - return 0, fmt.Errorf("wrong tx index key length, expect: %d, got: %d", TxIndexKeyLength, len(key)) - } - - return int64(sdk.BigEndianToUint64(key[1:9])), nil -} - // LoadLastBlock returns the latest indexed block number, returns -1 if db is empty func LoadLastBlock(db dbm.DB) (int64, error) { it, err := db.ReverseIterator([]byte{KeyPrefixTxIndex}, []byte{KeyPrefixTxIndex + 1}) @@ -229,3 +195,36 @@ func LoadFirstBlock(db dbm.DB) (int64, error) { } return parseBlockNumberFromKey(it.Key()) } + +// isEthTx check if the tx is an eth tx +func isEthTx(tx sdk.Tx) bool { + extTx, ok := tx.(authante.HasExtensionOptionsTx) + if !ok { + return false + } + opts := extTx.GetExtensionOptions() + if len(opts) != 1 || opts[0].GetTypeUrl() != "/ethermint.evm.v1.ExtensionOptionsEthereumTx" { + return false + } + return true +} + +// saveTxResult index the txResult into the kv db batch +func saveTxResult(codec codec.Codec, batch dbm.Batch, txHash common.Hash, txResult *ethermint.TxResult) error { + bz := codec.MustMarshal(txResult) + if err := batch.Set(TxHashKey(txHash), bz); err != nil { + return sdkerrors.Wrap(err, "set tx-hash key") + } + if err := batch.Set(TxIndexKey(txResult.Height, txResult.EthTxIndex), txHash.Bytes()); err != nil { + return sdkerrors.Wrap(err, "set tx-index key") + } + return nil +} + +func parseBlockNumberFromKey(key []byte) (int64, error) { + if len(key) != TxIndexKeyLength { + return 0, fmt.Errorf("wrong tx index key length, expect: %d, got: %d", TxIndexKeyLength, len(key)) + } + + return int64(sdk.BigEndianToUint64(key[1:9])), nil +} diff --git a/proto/ethermint/types/v1/indexer.proto b/proto/ethermint/types/v1/indexer.proto index c58799d3d1..e1d0be03c9 100644 --- a/proto/ethermint/types/v1/indexer.proto +++ b/proto/ethermint/types/v1/indexer.proto @@ -16,7 +16,8 @@ message TxResult { // the msg index in a batch tx uint32 msg_index = 3; - // eth tx index + // eth tx index, the index in the list of valid eth tx in the block, + // aka. the transaction list returned by eth_getBlock api. int32 eth_tx_index = 4; // if the eth tx is failed bool failed = 5; diff --git a/rpc/backend/backend.go b/rpc/backend/backend.go index 75594a130b..340524cd0f 100644 --- a/rpc/backend/backend.go +++ b/rpc/backend/backend.go @@ -144,7 +144,13 @@ type Backend struct { } // NewBackend creates a new Backend instance for cosmos and ethereum namespaces -func NewBackend(ctx *server.Context, logger log.Logger, clientCtx client.Context, allowUnprotectedTxs bool, indexer ethermint.EVMTxIndexer) *Backend { +func NewBackend( + ctx *server.Context, + logger log.Logger, + clientCtx client.Context, + allowUnprotectedTxs bool, + indexer ethermint.EVMTxIndexer, +) *Backend { chainID, err := ethermint.ParseChainID(clientCtx.ChainID) if err != nil { panic(err) diff --git a/rpc/backend/backend_suite_test.go b/rpc/backend/backend_suite_test.go index 2d1bd2ddbf..016f525ad5 100644 --- a/rpc/backend/backend_suite_test.go +++ b/rpc/backend/backend_suite_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "testing" + dbm "github.com/tendermint/tm-db" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/crypto/keyring" "github.com/cosmos/cosmos-sdk/server" @@ -20,6 +22,7 @@ import ( "github.com/evmos/ethermint/app" "github.com/evmos/ethermint/crypto/hd" "github.com/evmos/ethermint/encoding" + "github.com/evmos/ethermint/indexer" "github.com/evmos/ethermint/rpc/backend/mocks" rpctypes "github.com/evmos/ethermint/rpc/types" evmtypes "github.com/evmos/ethermint/x/evm/types" @@ -56,7 +59,9 @@ func (suite *BackendTestSuite) SetupTest() { allowUnprotectedTxs := false - suite.backend = NewBackend(ctx, ctx.Logger, clientCtx, allowUnprotectedTxs, nil) + idxer := indexer.NewKVIndexer(dbm.NewMemDB(), ctx.Logger, clientCtx) + + suite.backend = NewBackend(ctx, ctx.Logger, clientCtx, allowUnprotectedTxs, idxer) suite.backend.queryClient.QueryClient = mocks.NewQueryClient(suite.T()) suite.backend.clientCtx.Client = mocks.NewClient(suite.T()) suite.backend.ctx = rpctypes.ContextWithHeight(1) diff --git a/rpc/backend/blocks_info.go b/rpc/backend/blocks_info.go index ca97153b9b..653725a96d 100644 --- a/rpc/backend/blocks_info.go +++ b/rpc/backend/blocks_info.go @@ -297,7 +297,7 @@ func (b *Backend) GetEthereumMsgsFromTendermintBlock( // Check if tx exists on EVM by cross checking with blockResults: // - Include unsuccessful tx that exceeds block gas limit // - Exclude unsuccessful tx with any other error but ExceedBlockGasLimit - if !TxSuccessOrExceedsBlockGasLimit(txResults[i]) { + if !rpctypes.TxSuccessOrExceedsBlockGasLimit(txResults[i]) { b.logger.Debug("invalid tx result code", "cosmos-hash", hexutil.Encode(tx.Hash())) continue } diff --git a/rpc/backend/evm_backend_test.go b/rpc/backend/evm_backend_test.go index f17eed47e4..4e78917b09 100644 --- a/rpc/backend/evm_backend_test.go +++ b/rpc/backend/evm_backend_test.go @@ -949,7 +949,7 @@ func (suite *BackendTestSuite) TestGetEthereumMsgsFromTendermintBlock() { TxsResults: []*types.ResponseDeliverTx{ { Code: 1, - Log: ExceedBlockGasLimitError, + Log: ethrpc.ExceedBlockGasLimitError, }, }, }, @@ -964,7 +964,7 @@ func (suite *BackendTestSuite) TestGetEthereumMsgsFromTendermintBlock() { TxsResults: []*types.ResponseDeliverTx{ { Code: 0, - Log: ExceedBlockGasLimitError, + Log: ethrpc.ExceedBlockGasLimitError, }, }, }, diff --git a/rpc/backend/tx_info.go b/rpc/backend/tx_info.go index e84180e2fd..a75e6015c3 100644 --- a/rpc/backend/tx_info.go +++ b/rpc/backend/tx_info.go @@ -57,6 +57,7 @@ func (b *Backend) GetTransactionByHash(txHash common.Hash) (*rpctypes.RPCTransac } } } + // if we still unable to find the eth tx index, return error, shouldn't happen. if res.EthTxIndex == -1 { return nil, errors.New("can't find index of ethereum tx") } @@ -94,6 +95,7 @@ func (b *Backend) getTransactionByHashPending(txHash common.Hash) (*rpctypes.RPC } if msg.Hash == hexTx { + // use zero block values since it's not included in a block yet rpctx, err := rpctypes.NewTransactionFromMsg( msg, common.Hash{}, @@ -185,6 +187,7 @@ func (b *Backend) GetTransactionReceipt(hash common.Hash) (map[string]interface{ } } } + // return error if still unable to find the eth tx index if res.EthTxIndex == -1 { return nil, errors.New("can't find index of ethereum tx") } @@ -320,7 +323,7 @@ func (b *Backend) queryTendermintTxIndexer(query string, txGetter func(*rpctypes return nil, errors.New("ethereum tx not found") } txResult := resTxs.Txs[0] - if !TxSuccessOrExceedsBlockGasLimit(&txResult.TxResult) { + if !rpctypes.TxSuccessOrExceedsBlockGasLimit(&txResult.TxResult) { return nil, errors.New("invalid ethereum tx") } diff --git a/rpc/backend/utils.go b/rpc/backend/utils.go index 737df49afd..d95a6854f8 100644 --- a/rpc/backend/utils.go +++ b/rpc/backend/utils.go @@ -23,10 +23,6 @@ import ( evmtypes "github.com/evmos/ethermint/x/evm/types" ) -// ExceedBlockGasLimitError defines the error message when tx execution exceeds the block gas limit. -// The tx fee is deducted in ante handler, so it shouldn't be ignored in JSON-RPC API. -const ExceedBlockGasLimitError = "out of gas in location: block gas meter; gasWanted:" - type txGasAndReward struct { gasUsed uint64 reward *big.Int @@ -247,17 +243,6 @@ func ParseTxLogsFromEvent(event abci.Event) ([]*ethtypes.Log, error) { return evmtypes.LogsToEthereum(logs), nil } -// TxExceedBlockGasLimit returns true if the tx exceeds block gas limit. -func TxExceedBlockGasLimit(res *abci.ResponseDeliverTx) bool { - return strings.Contains(res.Log, ExceedBlockGasLimitError) -} - -// TxSuccessOrExceedsBlockGasLimit returnsrue if the transaction was successful -// or if it failed with an ExceedBlockGasLimit error -func TxSuccessOrExceedsBlockGasLimit(res *abci.ResponseDeliverTx) bool { - return res.Code == 0 || TxExceedBlockGasLimit(res) -} - // ShouldIgnoreGasUsed returns true if the gasUsed in result should be ignored // workaround for issue: https://github.com/cosmos/cosmos-sdk/issues/10832 func ShouldIgnoreGasUsed(res *abci.ResponseDeliverTx) bool { diff --git a/rpc/types/utils.go b/rpc/types/utils.go index 7e0aebd938..5843ed24ef 100644 --- a/rpc/types/utils.go +++ b/rpc/types/utils.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "math/big" + "strings" abci "github.com/tendermint/tendermint/abci/types" tmtypes "github.com/tendermint/tendermint/types" @@ -22,6 +23,10 @@ import ( "github.com/ethereum/go-ethereum/params" ) +// ExceedBlockGasLimitError defines the error message when tx execution exceeds the block gas limit. +// The tx fee is deducted in ante handler, so it shouldn't be ignored in JSON-RPC API. +const ExceedBlockGasLimitError = "out of gas in location: block gas meter; gasWanted:" + // RawTxToEthTx returns a evm MsgEthereum transaction from raw tx bytes. func RawTxToEthTx(clientCtx client.Context, txBz tmtypes.Tx) ([]*evmtypes.MsgEthereumTx, error) { tx, err := clientCtx.TxConfig.TxDecoder()(txBz) @@ -243,3 +248,14 @@ func CheckTxFee(gasPrice *big.Int, gas uint64, cap float64) error { } return nil } + +// TxExceedBlockGasLimit returns true if the tx exceeds block gas limit. +func TxExceedBlockGasLimit(res *abci.ResponseDeliverTx) bool { + return strings.Contains(res.Log, ExceedBlockGasLimitError) +} + +// TxSuccessOrExceedsBlockGasLimit returnsrue if the transaction was successful +// or if it failed with an ExceedBlockGasLimit error +func TxSuccessOrExceedsBlockGasLimit(res *abci.ResponseDeliverTx) bool { + return res.Code == 0 || TxExceedBlockGasLimit(res) +} diff --git a/server/indexer_cmd.go b/server/indexer_cmd.go index 8a90d8226d..8430fd7adc 100644 --- a/server/indexer_cmd.go +++ b/server/indexer_cmd.go @@ -7,6 +7,7 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/server" + "github.com/evmos/ethermint/indexer" tmnode "github.com/tendermint/tendermint/node" sm "github.com/tendermint/tendermint/state" tmstore "github.com/tendermint/tendermint/store" @@ -28,6 +29,11 @@ func NewIndexTxCmd() *cobra.Command { return err } + direction := args[0] + if direction != "backward" && direction != "forward" { + return fmt.Errorf("unknown index direction, expect: backward|forward, got: %s", direction) + } + cfg := serverCtx.Config home := cfg.RootDir logger := serverCtx.Logger @@ -36,7 +42,7 @@ func NewIndexTxCmd() *cobra.Command { logger.Error("failed to open evm indexer DB", "error", err.Error()) return err } - indexer := NewKVIndexer(idxDB, logger.With("module", "evmindex"), clientCtx) + idxer := indexer.NewKVIndexer(idxDB, logger.With("module", "evmindex"), clientCtx) // open local tendermint db, because the local rpc won't be available. tmdb, err := tmnode.DefaultDBProvider(&tmnode.DBContext{ID: "blockstore", Config: cfg}) @@ -60,7 +66,7 @@ func NewIndexTxCmd() *cobra.Command { if err != nil { return err } - if err := indexer.IndexBlock(blk, resBlk.DeliverTxs); err != nil { + if err := idxer.IndexBlock(blk, resBlk.DeliverTxs); err != nil { return err } fmt.Println(height) @@ -69,7 +75,7 @@ func NewIndexTxCmd() *cobra.Command { switch args[0] { case "backward": - first, err := indexer.FirstIndexedBlock() + first, err := idxer.FirstIndexedBlock() if err != nil { return err } @@ -82,7 +88,7 @@ func NewIndexTxCmd() *cobra.Command { } } case "forward": - latest, err := indexer.LastIndexedBlock() + latest, err := idxer.LastIndexedBlock() if err != nil { return err } diff --git a/server/start.go b/server/start.go index 5639c058a8..f84dbe6873 100644 --- a/server/start.go +++ b/server/start.go @@ -40,10 +40,10 @@ import ( servergrpc "github.com/cosmos/cosmos-sdk/server/grpc" "github.com/cosmos/cosmos-sdk/server/types" + "github.com/evmos/ethermint/indexer" ethdebug "github.com/evmos/ethermint/rpc/namespaces/ethereum/debug" "github.com/evmos/ethermint/server/config" srvflags "github.com/evmos/ethermint/server/flags" - ethermint "github.com/evmos/ethermint/types" ) // StartCmd runs the service passed in, either stand-alone or in-process with @@ -332,7 +332,6 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, appCreator ty app.RegisterTendermintService(clientCtx) } - var indexer ethermint.EVMTxIndexer if config.JSONRPC.EnableIndexer { idxDB, err := OpenIndexerDB(home, server.GetAppDBBackend(ctx.Viper)) if err != nil { @@ -340,8 +339,8 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, appCreator ty return err } idxLogger := ctx.Logger.With("module", "evmindex") - indexer = NewKVIndexer(idxDB, idxLogger, clientCtx) - indexerService := NewEVMIndexerService(indexer, clientCtx.Client) + idxer := indexer.NewKVIndexer(idxDB, idxLogger, clientCtx) + indexerService := NewEVMIndexerService(idxer, clientCtx.Client) indexerService.SetLogger(idxLogger) errCh := make(chan error) @@ -510,6 +509,7 @@ func openDB(rootDir string, backendType dbm.BackendType) (dbm.DB, error) { return dbm.NewDB("application", backendType, dataDir) } +// OpenIndexerDB opens the custom eth indexer db, using the same db backend as the main app func OpenIndexerDB(rootDir string, backendType dbm.BackendType) (dbm.DB, error) { dataDir := filepath.Join(rootDir, "data") return dbm.NewDB("evmindexer", backendType, dataDir) diff --git a/types/indexer.pb.go b/types/indexer.pb.go index aa61b1ee09..2e15c18545 100644 --- a/types/indexer.pb.go +++ b/types/indexer.pb.go @@ -31,7 +31,8 @@ type TxResult struct { TxIndex uint32 `protobuf:"varint,2,opt,name=tx_index,json=txIndex,proto3" json:"tx_index,omitempty"` // the msg index in a batch tx MsgIndex uint32 `protobuf:"varint,3,opt,name=msg_index,json=msgIndex,proto3" json:"msg_index,omitempty"` - // eth tx index + // eth tx index, the index in the list of valid eth tx in the block, + // aka. the transaction list returned by eth_getBlock api. EthTxIndex int32 `protobuf:"varint,4,opt,name=eth_tx_index,json=ethTxIndex,proto3" json:"eth_tx_index,omitempty"` // if the eth tx is failed Failed bool `protobuf:"varint,5,opt,name=failed,proto3" json:"failed,omitempty"`