From 9d1ce30ecdfaa78e5aa5d784b434af3ce13a1ab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Wed, 14 Jul 2021 16:44:51 -0400 Subject: [PATCH] evm: use block proposer address for `COINBASE` opcode (#291) * evm: use block proposer address for COINBASE opcode * test * c++ --- CHANGELOG.md | 1 + app/ante/eth.go | 6 ++- x/evm/keeper/grpc_query.go | 7 ++- x/evm/keeper/state_transition.go | 32 ++++++++++-- x/evm/keeper/state_transition_test.go | 73 +++++++++++++++++++++++++++ 5 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 x/evm/keeper/state_transition_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 107dfa9a..63396a5e 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. diff --git a/app/ante/eth.go b/app/ante/eth.go index 62c9069c..06dc292b 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 17e5c01a..63d01f1e 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 a90ce4c6..332744a4 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 block proposer address", + ) + } + + coinbase := common.BytesToAddress(validator.GetOperator()) + return coinbase, nil +} diff --git a/x/evm/keeper/state_transition_test.go b/x/evm/keeper/state_transition_test.go new file mode 100644 index 00000000..2bd2c8e9 --- /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) + } + }) + } + +}