evm: use block proposer address for COINBASE opcode (#291)

* evm: use block proposer address for COINBASE opcode

* test

* c++

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.
 

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

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())

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
+}

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)
+			}
+		})
+	}
+
+}