From 7c7068ae2b11d8b568f3ba3dfaa265c26b41fa60 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 14 Jul 2021 20:07:57 +0200 Subject: [PATCH 1/3] evm: use block proposer address for COINBASE opcode --- app/ante/eth.go | 6 ++++-- x/evm/keeper/grpc_query.go | 7 ++++++- x/evm/keeper/state_transition.go | 32 ++++++++++++++++++++++++++++---- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/app/ante/eth.go b/app/ante/eth.go index 62c9069c58..06dc292b14 100644 --- a/app/ante/eth.go +++ b/app/ante/eth.go @@ -26,7 +26,8 @@ type EVMKeeper interface { GetParams(ctx sdk.Context) evmtypes.Params WithContext(ctx sdk.Context) ResetRefundTransient(ctx sdk.Context) - NewEVM(msg core.Message, config *params.ChainConfig, params evmtypes.Params) *vm.EVM + GetCoinbaseAddress() (common.Address, error) + NewEVM(msg core.Message, config *params.ChainConfig, params evmtypes.Params, coinbase common.Address) *vm.EVM GetCodeHash(addr common.Address) common.Hash } @@ -388,7 +389,8 @@ func (ctd CanTransferDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate ) } - evm := ctd.evmKeeper.NewEVM(coreMsg, ethCfg, params) + // NOTE: pass in an empty coinbase address as we don't need it for the check below + evm := ctd.evmKeeper.NewEVM(coreMsg, ethCfg, params, common.Address{}) // check that caller has enough balance to cover asset transfer for **topmost** call // NOTE: here the gas consumed is from the context with the infinite gas meter diff --git a/x/evm/keeper/grpc_query.go b/x/evm/keeper/grpc_query.go index 17e5c01ada..63d01f1e47 100644 --- a/x/evm/keeper/grpc_query.go +++ b/x/evm/keeper/grpc_query.go @@ -379,7 +379,12 @@ func (k Keeper) EthCall(c context.Context, req *types.EthCallRequest) (*types.Ms params := k.GetParams(ctx) ethCfg := params.ChainConfig.EthereumConfig(k.eip155ChainID) - evm := k.NewEVM(msg, ethCfg, params) + coinbase, err := k.GetCoinbaseAddress() + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + + evm := k.NewEVM(msg, ethCfg, params, coinbase) res, err := k.ApplyMessage(evm, msg, ethCfg) if err != nil { return nil, status.Error(codes.Internal, err.Error()) diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index a90ce4c67e..4e3438b27c 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -13,6 +13,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ethermint "github.com/tharsis/ethermint/types" "github.com/tharsis/ethermint/x/evm/types" @@ -24,13 +25,15 @@ import ( "github.com/ethereum/go-ethereum/params" ) -// NewEVM generates an ethereum VM from the provided Message fields and the ChainConfig. -func (k *Keeper) NewEVM(msg core.Message, config *params.ChainConfig, params types.Params) *vm.EVM { +// NewEVM generates an ethereum VM from the provided Message fields and the chain parameters +// (config). It sets the validator operator address as the coinbase address to make it available for +// the COINBASE opcode, even though there is no beneficiary (since we're not mining). +func (k *Keeper) NewEVM(msg core.Message, config *params.ChainConfig, params types.Params, coinbase common.Address) *vm.EVM { blockCtx := vm.BlockContext{ CanTransfer: core.CanTransfer, Transfer: core.Transfer, GetHash: k.GetHashFn(), - Coinbase: common.Address{}, // there's no beneficiary since we're not mining + Coinbase: coinbase, GasLimit: ethermint.BlockGasLimit(k.ctx), BlockNumber: big.NewInt(k.ctx.BlockHeight()), Time: big.NewInt(k.ctx.BlockHeader().Time.Unix()), @@ -127,7 +130,13 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT cacheCtx, commit := k.ctx.CacheContext() k.ctx = cacheCtx - evm := k.NewEVM(msg, ethCfg, params) + // get the coinbase address from the block proposer + coinbase, err := k.GetCoinbaseAddress() + if err != nil { + return nil, stacktrace.Propagate(err, "failed to obtain coinbase address") + } + + evm := k.NewEVM(msg, ethCfg, params, coinbase) k.SetTxHashTransient(tx.Hash()) k.IncreaseTxIndexTransient() @@ -325,3 +334,18 @@ func (k *Keeper) resetGasMeterAndConsumeGas(gasUsed uint64) { k.ctx.GasMeter().RefundGas(k.ctx.GasMeter().GasConsumed(), "reset the gas count") k.ctx.GasMeter().ConsumeGas(gasUsed, "apply evm transaction") } + +// GetCoinbaseAddress returns the block proposer's validator operator address. +func (k Keeper) GetCoinbaseAddress() (common.Address, error) { + consAddr := sdk.ConsAddress(k.ctx.BlockHeader().ProposerAddress) + validator, found := k.stakingKeeper.GetValidatorByConsAddr(k.ctx, consAddr) + if !found { + return common.Address{}, stacktrace.Propagate( + sdkerrors.Wrap(stakingtypes.ErrNoValidatorFound, consAddr.String()), + "failed to retrieve validator from proposer address", + ) + } + + coinbase := common.BytesToAddress(validator.GetOperator()) + return coinbase, nil +} From e27e1aaa2ab9da161f58b07ee8856044b7e423e7 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 14 Jul 2021 22:34:05 +0200 Subject: [PATCH 2/3] test --- x/evm/keeper/state_transition.go | 2 +- x/evm/keeper/state_transition_test.go | 73 +++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 x/evm/keeper/state_transition_test.go diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index 4e3438b27c..332744a4ba 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -342,7 +342,7 @@ func (k Keeper) GetCoinbaseAddress() (common.Address, error) { if !found { return common.Address{}, stacktrace.Propagate( sdkerrors.Wrap(stakingtypes.ErrNoValidatorFound, consAddr.String()), - "failed to retrieve validator from proposer address", + "failed to retrieve validator from block proposer address", ) } diff --git a/x/evm/keeper/state_transition_test.go b/x/evm/keeper/state_transition_test.go new file mode 100644 index 0000000000..2bd2c8e94f --- /dev/null +++ b/x/evm/keeper/state_transition_test.go @@ -0,0 +1,73 @@ +package keeper_test + +import ( + "fmt" + + "github.com/tharsis/ethermint/tests" + + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" +) + +func (suite *KeeperTestSuite) TestGetCoinbaseAddress() { + valOpAddr := tests.GenerateAddress() + + testCases := []struct { + msg string + malleate func() + expPass bool + }{ + { + "validator not found", + func() {}, + false, + }, + { + "success", + func() { + valConsAddr, privkey := tests.NewAddrKey() + + pkAny, err := codectypes.NewAnyWithValue(privkey.PubKey()) + suite.Require().NoError(err) + + validator := stakingtypes.Validator{ + OperatorAddress: sdk.ValAddress(valOpAddr.Bytes()).String(), + ConsensusPubkey: pkAny, + } + + suite.app.StakingKeeper.SetValidator(suite.ctx, validator) + err = suite.app.StakingKeeper.SetValidatorByConsAddr(suite.ctx, validator) + suite.Require().NoError(err) + + header := suite.ctx.BlockHeader() + header.ProposerAddress = valConsAddr.Bytes() + suite.ctx = suite.ctx.WithBlockHeader(header) + + validator, found := suite.app.StakingKeeper.GetValidatorByConsAddr(suite.ctx, valConsAddr.Bytes()) + suite.Require().True(found) + + suite.app.EvmKeeper.WithContext(suite.ctx) + suite.Require().NotEmpty(suite.ctx.BlockHeader().ProposerAddress) + }, + true, + }, + } + + for _, tc := range testCases { + suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { + suite.SetupTest() // reset + + tc.malleate() + + coinbase, err := suite.app.EvmKeeper.GetCoinbaseAddress() + if tc.expPass { + suite.Require().NoError(err) + suite.Require().Equal(valOpAddr, coinbase) + } else { + suite.Require().Error(err) + } + }) + } + +} From 702cd4a144caee0b45bd6d20a309461ba54bee16 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 14 Jul 2021 22:35:39 +0200 Subject: [PATCH 3/3] c++ --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 107dfa9a84..63396a5ebd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (evm) [tharsis#291](https://github.com/tharsis/ethermint/pull/291) Use block proposer address (validator operator) for `COINBASE` opcode. * (rpc) [tharsis#81](https://github.com/tharsis/ethermint/pull/81) Fix transaction hashing and decoding on `eth_sendTransaction`. * (rpc) [tharsis#45](https://github.com/tharsis/ethermint/pull/45) Use `EmptyUncleHash` and `EmptyRootHash` for empty ethereum `Header` fields.