Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch ethermint gas tracking #27

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion indexer/kv_indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (kv *KVIndexer) IndexBlock(block *tmtypes.Block, txResults []*abci.Response
var ethTxIndex int32
for txIndex, tx := range block.Txs {
result := txResults[txIndex]
if !rpctypes.TxSuccessOrExceedsBlockGasLimit(result) {
if !rpctypes.TxSucessOrExpectedFailure(result) {
continue
}

Expand Down
4 changes: 4 additions & 0 deletions proto/ethermint/evm/v1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,8 @@ message QueryTraceTxRequest {
bytes proposer_address = 8 [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.ConsAddress"];
// chain_id is the the eip155 chain id parsed from the requested block header
int64 chain_id = 9;
// block_max_gas of the block of the requested transaction
int64 block_max_gas = 10;
}

// QueryTraceTxResponse defines TraceTx response
Expand All @@ -279,6 +281,8 @@ message QueryTraceBlockRequest {
bytes proposer_address = 8 [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.ConsAddress"];
// chain_id is the eip155 chain id parsed from the requested block header
int64 chain_id = 9;
// block_max_gas of the traced block
int64 block_max_gas = 10;
}

// QueryTraceBlockResponse defines TraceBlock response
Expand Down
3 changes: 2 additions & 1 deletion rpc/backend/blocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,9 @@ func (b *Backend) EthMsgsFromTendermintBlock(
for i, tx := range block.Txs {
// Check if tx exists on EVM by cross checking with blockResults:
// - Include unsuccessful tx that exceeds block gas limit
// - Include unsuccessful tx that failed when committing changes to stateDB
// - Exclude unsuccessful tx with any other error but ExceedBlockGasLimit
if !rpctypes.TxSuccessOrExceedsBlockGasLimit(txResults[i]) {
if !rpctypes.TxSucessOrExpectedFailure(txResults[i]) {
b.logger.Debug("invalid tx result code", "cosmos-hash", hexutil.Encode(tx.Hash()))
continue
}
Expand Down
6 changes: 3 additions & 3 deletions rpc/backend/evm_query_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,18 @@ var _ evmtypes.QueryClient = &mocks.EVMQueryClient{}
func RegisterTraceTransactionWithPredecessors(queryClient *mocks.EVMQueryClient, msgEthTx *evmtypes.MsgEthereumTx, predecessors []*evmtypes.MsgEthereumTx) {
data := []byte{0x7b, 0x22, 0x74, 0x65, 0x73, 0x74, 0x22, 0x3a, 0x20, 0x22, 0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x22, 0x7d}
queryClient.On("TraceTx", rpc.ContextWithHeight(1),
&evmtypes.QueryTraceTxRequest{Msg: msgEthTx, BlockNumber: 1, Predecessors: predecessors, ChainId: 9000}).
&evmtypes.QueryTraceTxRequest{Msg: msgEthTx, BlockNumber: 1, Predecessors: predecessors, ChainId: 9000, BlockMaxGas: -1}).
Return(&evmtypes.QueryTraceTxResponse{Data: data}, nil)
}

func RegisterTraceTransaction(queryClient *mocks.EVMQueryClient, msgEthTx *evmtypes.MsgEthereumTx) {
data := []byte{0x7b, 0x22, 0x74, 0x65, 0x73, 0x74, 0x22, 0x3a, 0x20, 0x22, 0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x22, 0x7d}
queryClient.On("TraceTx", rpc.ContextWithHeight(1), &evmtypes.QueryTraceTxRequest{Msg: msgEthTx, BlockNumber: 1, ChainId: 9000}).
queryClient.On("TraceTx", rpc.ContextWithHeight(1), &evmtypes.QueryTraceTxRequest{Msg: msgEthTx, BlockNumber: 1, ChainId: 9000, BlockMaxGas: -1}).
Return(&evmtypes.QueryTraceTxResponse{Data: data}, nil)
}

func RegisterTraceTransactionError(queryClient *mocks.EVMQueryClient, msgEthTx *evmtypes.MsgEthereumTx) {
queryClient.On("TraceTx", rpc.ContextWithHeight(1), &evmtypes.QueryTraceTxRequest{Msg: msgEthTx, BlockNumber: 1, ChainId: 9000}).
queryClient.On("TraceTx", rpc.ContextWithHeight(1), &evmtypes.QueryTraceTxRequest{Msg: msgEthTx, BlockNumber: 1, ChainId: 9000, BlockMaxGas: -1}).
Return(nil, errortypes.ErrInvalidRequest)
}

Expand Down
14 changes: 14 additions & 0 deletions rpc/backend/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ func (b *Backend) TraceTransaction(hash common.Hash, config *evmtypes.TraceConfi
return nil, fmt.Errorf("invalid transaction type %T", tx)
}

// fetch consensus params for determining block max gas value
cp, err := b.clientCtx.Client.ConsensusParams(b.ctx, &blk.Block.Height)
if err != nil {
return nil, err
}

traceTxRequest := evmtypes.QueryTraceTxRequest{
Msg: ethMessage,
Predecessors: predecessors,
Expand All @@ -100,6 +106,7 @@ func (b *Backend) TraceTransaction(hash common.Hash, config *evmtypes.TraceConfi
BlockHash: common.Bytes2Hex(blk.BlockID.Hash),
ProposerAddress: sdk.ConsAddress(blk.Block.ProposerAddress),
ChainId: b.chainID.Int64(),
BlockMaxGas: cp.ConsensusParams.Block.MaxGas,
}

if config != nil {
Expand Down Expand Up @@ -171,6 +178,12 @@ func (b *Backend) TraceBlock(height rpctypes.BlockNumber,
}
ctxWithHeight := rpctypes.ContextWithHeight(int64(contextHeight))

// fetch consensus params for determining block max gas value
cp, err := b.clientCtx.Client.ConsensusParams(b.ctx, &block.Block.Height)
if err != nil {
return nil, err
}

traceBlockRequest := &evmtypes.QueryTraceBlockRequest{
Txs: txsMessages,
TraceConfig: config,
Expand All @@ -179,6 +192,7 @@ func (b *Backend) TraceBlock(height rpctypes.BlockNumber,
BlockHash: common.Bytes2Hex(block.BlockID.Hash),
ProposerAddress: sdk.ConsAddress(block.Block.ProposerAddress),
ChainId: b.chainID.Int64(),
BlockMaxGas: cp.ConsensusParams.Block.MaxGas,
}

res, err := b.queryClient.TraceBlock(ctxWithHeight, traceBlockRequest)
Expand Down
24 changes: 18 additions & 6 deletions rpc/backend/tracing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,15 @@ func (suite *BackendTestSuite) TestTraceTransaction() {
{
"pass - transaction found in a block with multiple transactions",
func() {
queryClient := suite.backend.queryClient.QueryClient.(*mocks.EVMQueryClient)
client := suite.backend.clientCtx.Client.(*mocks.Client)
RegisterBlockMultipleTxs(client, 1, []types.Tx{txBz, txBz2})
var (
queryClient = suite.backend.queryClient.QueryClient.(*mocks.EVMQueryClient)
client = suite.backend.clientCtx.Client.(*mocks.Client)
height int64 = 1
)
_, err := RegisterBlockMultipleTxs(client, height, []types.Tx{txBz, txBz2})
suite.Require().NoError(err)
RegisterTraceTransactionWithPredecessors(queryClient, msgEthereumTx, []*evmtypes.MsgEthereumTx{msgEthereumTx})
RegisterConsensusParams(client, height)
},
&types.Block{Header: types.Header{Height: 1, ChainID: ChainID}, Data: types.Data{Txs: []types.Tx{txBz, txBz2}}},
[]*abci.ResponseDeliverTx{
Expand Down Expand Up @@ -146,10 +151,15 @@ func (suite *BackendTestSuite) TestTraceTransaction() {
{
"pass - transaction found",
func() {
queryClient := suite.backend.queryClient.QueryClient.(*mocks.EVMQueryClient)
client := suite.backend.clientCtx.Client.(*mocks.Client)
RegisterBlock(client, 1, txBz)
var (
queryClient = suite.backend.queryClient.QueryClient.(*mocks.EVMQueryClient)
client = suite.backend.clientCtx.Client.(*mocks.Client)
height int64 = 1
)
_, err := RegisterBlock(client, height, txBz)
suite.Require().NoError(err)
RegisterTraceTransaction(queryClient, msgEthereumTx)
RegisterConsensusParams(client, height)
},
&types.Block{Header: types.Header{Height: 1}, Data: types.Data{Txs: []types.Tx{txBz}}},
[]*abci.ResponseDeliverTx{
Expand Down Expand Up @@ -223,7 +233,9 @@ func (suite *BackendTestSuite) TestTraceBlock() {
"fail - cannot unmarshal data",
func() {
queryClient := suite.backend.queryClient.QueryClient.(*mocks.EVMQueryClient)
client := suite.backend.clientCtx.Client.(*mocks.Client)
RegisterTraceBlock(queryClient, []*evmtypes.MsgEthereumTx{msgEthTx})
RegisterConsensusParams(client, 1)
},
[]*evmtypes.TxTraceResult{},
&resBlockFilled,
Expand Down
2 changes: 1 addition & 1 deletion rpc/backend/tx_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ func (b *Backend) queryTendermintTxIndexer(query string, txGetter func(*rpctypes
return nil, errors.New("ethereum tx not found")
}
txResult := resTxs.Txs[0]
if !rpctypes.TxSuccessOrExceedsBlockGasLimit(&txResult.TxResult) {
if !rpctypes.TxSucessOrExpectedFailure(&txResult.TxResult) {
return nil, errors.New("invalid ethereum tx")
}

Expand Down
18 changes: 14 additions & 4 deletions rpc/types/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ import (
// 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:"

// StateDBCommitError defines the error message when commit after executing EVM transaction, for example
// transfer native token to a distribution module account 0x93354845030274cD4bf1686Abd60AB28EC52e1a7 using an evm type transaction
// note: the transfer amount cannot be set to 0, otherwise this problem will not be triggered
const StateDBCommitError = "failed to commit stateDB"

// 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)
Expand Down Expand Up @@ -273,8 +278,13 @@ 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)
// TxStateDBCommitError returns true if the evm tx commit error.
func TxStateDBCommitError(res *abci.ResponseDeliverTx) bool {
return strings.Contains(res.Log, StateDBCommitError)
}

// TxSucessOrExpectedFailure returns true if the transaction was successful
// or if it failed with an ExceedBlockGasLimit error or TxStateDBCommitError error
func TxSucessOrExpectedFailure(res *abci.ResponseDeliverTx) bool {
return res.Code == 0 || TxExceedBlockGasLimit(res) || TxStateDBCommitError(res)
}
46 changes: 41 additions & 5 deletions x/evm/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/core/vm"
ethparams "github.com/ethereum/go-ethereum/params"
abci "github.com/tendermint/tendermint/abci/types"

ethermint "github.com/evmos/ethermint/types"
"github.com/evmos/ethermint/x/evm/statedb"
Expand Down Expand Up @@ -289,7 +290,7 @@
}

// Binary search the gas requirement, as it may be higher than the amount used
var (

Check warning on line 293 in x/evm/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / Run golangci-lint

redefines-builtin-id: redefinition of the built-in function cap (revive)
lo = ethparams.TxGas - 1
hi uint64
cap uint64
Expand All @@ -314,7 +315,7 @@
if req.GasCap != 0 && hi > req.GasCap {
hi = req.GasCap
}
cap = hi

Check warning on line 318 in x/evm/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / Run golangci-lint

redefines-builtin-id: redefinition of the built-in function cap (revive)
cfg, err := k.EVMConfig(ctx, GetProposerAddress(ctx, req.ProposerAddress), chainID)
if err != nil {
return nil, status.Error(codes.Internal, "failed to load evm config")
Expand Down Expand Up @@ -402,8 +403,8 @@
return nil, status.Errorf(codes.InvalidArgument, "output limit cannot be negative, got %d", req.TraceConfig.Limit)
}

// minus one to get the context of block beginning
contextHeight := req.BlockNumber - 1
// get the context of block beginning
contextHeight := req.BlockNumber
if contextHeight < 1 {
// 0 is a special value in `ContextWithHeight`
contextHeight = 1
Expand All @@ -413,6 +414,12 @@
ctx = ctx.WithBlockHeight(contextHeight)
ctx = ctx.WithBlockTime(req.BlockTime)
ctx = ctx.WithHeaderHash(common.Hex2Bytes(req.BlockHash))

// to get the base fee we only need the block max gas in the consensus params
ctx = ctx.WithConsensusParams(&abci.ConsensusParams{
Block: &abci.BlockParams{MaxGas: req.BlockMaxGas},
})

chainID, err := getChainID(ctx, req.ChainId)
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
Expand All @@ -421,9 +428,21 @@
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to load evm config: %s", err.Error())
}

// compute and use base fee of the height that is being traced
baseFee := k.feeMarketKeeper.CalculateBaseFee(ctx)
if baseFee != nil {
cfg.BaseFee = baseFee
}

signer := ethtypes.MakeSigner(cfg.ChainConfig, big.NewInt(ctx.BlockHeight()))

txConfig := statedb.NewEmptyTxConfig(common.BytesToHash(ctx.HeaderHash().Bytes()))

// gas used at this point corresponds to GetProposerAddress & CalculateBaseFee
// need to reset gas meter per transaction to be consistent with tx execution
// and avoid stacking the gas used of every predecessor in the same gas meter

for i, tx := range req.Predecessors {
ethTx := tx.AsTransaction()
msg, err := ethTx.AsMessage(signer, cfg.BaseFee)
Expand All @@ -432,6 +451,8 @@
}
txConfig.TxHash = ethTx.Hash()
txConfig.TxIndex = uint(i)
// reset gas meter for each transaction
ctx = ctx.WithGasMeter(ethermint.NewInfiniteGasMeterWithLimit(uint64(0)))
Copy link

@DracoLi DracoLi Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this for debugging? Seem like this should be reset to msg.Gas() for the fix

rsp, err := k.ApplyMessageWithConfig(ctx, msg, types.NewNoOpTracer(), true, cfg, txConfig)
if err != nil {
continue
Expand Down Expand Up @@ -479,8 +500,8 @@
return nil, status.Errorf(codes.InvalidArgument, "output limit cannot be negative, got %d", req.TraceConfig.Limit)
}

// minus one to get the context of block beginning
contextHeight := req.BlockNumber - 1
// get the context of block beginning
contextHeight := req.BlockNumber
if contextHeight < 1 {
// 0 is a special value in `ContextWithHeight`
contextHeight = 1
Expand All @@ -490,6 +511,12 @@
ctx = ctx.WithBlockHeight(contextHeight)
ctx = ctx.WithBlockTime(req.BlockTime)
ctx = ctx.WithHeaderHash(common.Hex2Bytes(req.BlockHash))

// to get the base fee we only need the block max gas in the consensus params
ctx = ctx.WithConsensusParams(&abci.ConsensusParams{
Block: &abci.BlockParams{MaxGas: req.BlockMaxGas},
})

chainID, err := getChainID(ctx, req.ChainId)
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
Expand All @@ -499,6 +526,13 @@
if err != nil {
return nil, status.Error(codes.Internal, "failed to load evm config")
}

// compute and use base fee of height that is being traced
baseFee := k.feeMarketKeeper.CalculateBaseFee(ctx)
if baseFee != nil {
cfg.BaseFee = baseFee
}

signer := ethtypes.MakeSigner(cfg.ChainConfig, big.NewInt(ctx.BlockHeight()))
txsLength := len(req.Txs)
results := make([]*types.TxTraceResult, 0, txsLength)
Expand Down Expand Up @@ -601,7 +635,9 @@
tracer.Stop(errors.New("execution timeout"))
}
}()

// reset gas meter for tx
// to be consistent with tx execution gas meter
ctx = ctx.WithGasMeter(ethermint.NewInfiniteGasMeterWithLimit(msg.Gas()))
res, err := k.ApplyMessageWithConfig(ctx, msg, tracer, commitMessage, cfg, txConfig)
if err != nil {
return nil, 0, status.Error(codes.Internal, err.Error())
Expand Down
7 changes: 1 addition & 6 deletions x/evm/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,12 +358,7 @@ func (k Keeper) getBaseFee(ctx sdk.Context, london bool) *big.Int {

// GetMinGasMultiplier returns the MinGasMultiplier param from the fee market module
func (k Keeper) GetMinGasMultiplier(ctx sdk.Context) sdk.Dec {
fmkParmas := k.feeMarketKeeper.GetParams(ctx)
if fmkParmas.MinGasMultiplier.IsNil() {
// in case we are executing eth_call on a legacy block, returns a zero value.
return sdk.ZeroDec()
}
return fmkParmas.MinGasMultiplier
return k.feeMarketKeeper.GetParams(ctx).MinGasMultiplier
}

// ResetTransientGasUsed reset gas used to prepare for execution of current cosmos tx, called in ante handler.
Expand Down
1 change: 1 addition & 0 deletions x/evm/types/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ type FeeMarketKeeper interface {
GetBaseFee(ctx sdk.Context) *big.Int
GetParams(ctx sdk.Context) feemarkettypes.Params
AddTransientGasWanted(ctx sdk.Context, gasWanted uint64) (uint64, error)
CalculateBaseFee(ctx sdk.Context) *big.Int
}

// Event Hooks
Expand Down
Loading
Loading