From 098da6d0cc0e0c4cefbddf632df1057383973e4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Thu, 5 Aug 2021 12:24:06 -0400 Subject: [PATCH] evm: code comments and cleanup (#404) * evm: code comments and cleanup * context comment * abci, tx type event and metrics * statedb comments --- x/evm/keeper/abci.go | 8 +++--- x/evm/keeper/keeper.go | 27 +++++++++++++------- x/evm/keeper/msg_server.go | 8 +++++- x/evm/keeper/state_transition.go | 44 ++++++++++++++++++++------------ x/evm/keeper/statedb.go | 39 ++++++++++++++++++---------- x/evm/types/events.go | 1 + x/evm/types/key.go | 17 ------------ 7 files changed, 83 insertions(+), 61 deletions(-) diff --git a/x/evm/keeper/abci.go b/x/evm/keeper/abci.go index c422ce12..ca7bcfda 100644 --- a/x/evm/keeper/abci.go +++ b/x/evm/keeper/abci.go @@ -12,17 +12,15 @@ import ( "github.com/tharsis/ethermint/x/evm/types" ) -// BeginBlock sets the block hash -> block height map for the previous block height -// and resets the Bloom filter and the transaction count to 0. +// BeginBlock sets the sdk Context and EIP155 chain id to the Keeper. func (k *Keeper) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) k.WithContext(ctx) k.WithChainID(ctx) } -// EndBlock updates the accounts and commits state objects to the KV Store, while -// deleting the empty ones. It also sets the bloom filers for the request block to -// the store. The EVM end block logic doesn't update the validator set, thus it returns +// EndBlock also retrieves the bloom filter value from the transient store and commits it to the +// KVStore. The EVM end block logic doesn't update the validator set, thus it returns // an empty slice. func (k *Keeper) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) []abci.ValidatorUpdate { defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker) diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go index 913d7031..42df738b 100644 --- a/x/evm/keeper/keeper.go +++ b/x/evm/keeper/keeper.go @@ -17,33 +17,42 @@ import ( "github.com/tharsis/ethermint/x/evm/types" ) -// Keeper wraps the CommitStateDB, allowing us to pass in SDK context while adhering -// to the StateDB interface. +// Keeper grants access to the EVM module state and implements the go-ethereum StateDB interface. type Keeper struct { // Protobuf codec cdc codec.BinaryCodec // Store key required for the EVM Prefix KVStore. It is required by: - // - storing Account's Storage State - // - storing Account's Code + // - storing account's Storage State + // - storing account's Code // - storing transaction Logs - // - storing block height -> bloom filter map. Needed for the Web3 API. + // - storing Bloom filters by block height. Needed for the Web3 API. storeKey sdk.StoreKey // key to access the transient store, which is reset on every block during Commit transientKey sdk.StoreKey - paramSpace paramtypes.Subspace + // module specific parameter space that can be configured through governance + paramSpace paramtypes.Subspace + // access to account state accountKeeper types.AccountKeeper - bankKeeper types.BankKeeper + // update balance and accounting operations with coins + bankKeeper types.BankKeeper + // access historical headers for EVM state transition execution stakingKeeper types.StakingKeeper + // Context for accessing the store, emit events and log info. + // It is kept as a field to make is accessible by the StateDb + // functions. Resets on every transaction/block. ctx sdk.Context - // set in `BeginCachedContext`, used by `GetCommittedState` api. + // Context of the committed state (before transaction execution). + // Required for StateDB.CommitedState. Set in `BeginCachedContext`. committedCtx sdk.Context // chain ID number obtained from the context's chain id eip155ChainID *big.Int - debug bool + // trace EVM state transition execution. This value is obtained from the `--trace` flag. + // For more info check https://geth.ethereum.org/docs/dapp/tracing + debug bool } // NewKeeper generates new evm module keeper diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 35a8466a..795636b9 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" "time" "github.com/armon/go-metrics" @@ -18,6 +19,10 @@ import ( var _ types.MsgServer = &Keeper{} +// EthereumTx implements the gRPC MsgServer interface. It receives a transaction which is then +// executed (i.e applied) against the go-ethereum EVM. The provided SDK Context is set to the Keeper +// so that it can implements and call the StateDB methods without receiving it as a function +// parameter. func (k *Keeper) EthereumTx(goCtx context.Context, msg *types.MsgEthereumTx) (*types.MsgEthereumTxResponse, error) { defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), types.TypeMsgEthereumTx) @@ -27,7 +32,7 @@ func (k *Keeper) EthereumTx(goCtx context.Context, msg *types.MsgEthereumTx) (*t sender := msg.From tx := msg.AsTransaction() - var labels []metrics.Label + labels := []metrics.Label{telemetry.NewLabel("tx_type", fmt.Sprintf("%d", tx.Type()))} if tx.To() == nil { labels = []metrics.Label{ telemetry.NewLabel("execution", "create"), @@ -89,6 +94,7 @@ func (k *Keeper) EthereumTx(goCtx context.Context, msg *types.MsgEthereumTx) (*t sdk.EventTypeMessage, sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory), sdk.NewAttribute(sdk.AttributeKeySender, sender), + sdk.NewAttribute(types.AttributeKeyTxType, fmt.Sprintf("%d", tx.Type())), ), }) diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index c03d10d2..bcadb43e 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -24,9 +24,10 @@ import ( "github.com/ethereum/go-ethereum/params" ) -// 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). +// NewEVM generates a go-ethereum VM from the provided Message fields and the chain parameters +// (ChainConfig and module Params). It additionally sets the validator operator address as the +// coinbase address to make it available for the COINBASE opcode, even though there is no +// beneficiary of the coinbase transaction (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, @@ -45,8 +46,8 @@ func (k *Keeper) NewEVM(msg core.Message, config *params.ChainConfig, params typ return vm.NewEVM(blockCtx, txCtx, k, config, vmConfig) } -// VMConfig creates an EVM configuration from the module parameters and the debug setting. -// The config generated uses the default JumpTable from the EVM. +// VMConfig creates an EVM configuration from the debug setting and the extra EIPs enabled on the +// module parameters. The config generated uses the default JumpTable from the EVM. func (k Keeper) VMConfig(params types.Params) vm.Config { return vm.Config{ Debug: k.debug, @@ -73,13 +74,14 @@ func (k Keeper) GetHashFn() vm.GetHashFunc { return common.BytesToHash(headerHash) } - // only recompute the hash if not set + // only recompute the hash if not set (eg: checkTxState) contextBlockHeader := k.ctx.BlockHeader() header, err := tmtypes.HeaderFromProto(&contextBlockHeader) if err != nil { k.Logger(k.ctx).Error("failed to cast tendermint header from proto", "error", err) return common.Hash{} } + headerHash = header.Hash() return common.BytesToHash(headerHash) @@ -107,7 +109,7 @@ func (k Keeper) GetHashFn() vm.GetHashFunc { } // ApplyTransaction runs and attempts to perform a state transition with the given transaction (i.e Message), that will -// only be persisted to the underlying KVStore if the transaction does not error. +// only be persisted (committed) to the underlying KVStore if the transaction does not fail. // // Gas tracking // @@ -149,15 +151,19 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT // create an ethereum EVM instance and run the message evm := k.NewEVM(msg, ethCfg, params, coinbase) - k.SetTxHashTransient(tx.Hash()) + txHash := tx.Hash() + + // set the transaction hash and index to the impermanent (transient) block state so that it's also + // available on the StateDB functions (eg: AddLog) + k.SetTxHashTransient(txHash) k.IncreaseTxIndexTransient() + // pass false to execute in real mode, which do actual gas refunding res, err := k.ApplyMessage(evm, msg, ethCfg, false) if err != nil { return nil, stacktrace.Propagate(err, "failed to apply ethereum core message") } - txHash := tx.Hash() res.Hash = txHash.Hex() logs := k.GetTxLogs(txHash) @@ -226,7 +232,7 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT // // Query mode // -// The grpc query endpoint EthCall calls this in query mode, and since the query handler don't call AnteHandler, +// The gRPC query endpoint from 'eth_call' calls this method in query mode, and since the query handler don't call AnteHandler, // so we don't do real gas refund in that case. func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainConfig, query bool) (*types.MsgEthereumTxResponse, error) { var ( @@ -256,17 +262,19 @@ func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainCo } if query { - // query handlers don't call ante handler to deduct gas fee, so don't do actual refund here, because the - // module account balance might not be enough + // gRPC query handlers don't go through the AnteHandler to deduct the gas fee from the sender or have access historical state. + // We don't refund gas to the sender. + // For more info, see: https://github.com/tharsis/ethermint/issues/229 and https://github.com/cosmos/cosmos-sdk/issues/9636 leftoverGas += k.GasToRefund(msg.Gas() - leftoverGas) } else { - // refund gas prior to handling the vm error in order to set the updated gas meter + // refund gas prior to handling the vm error in order to match the Ethereum gas consumption instead of the default SDK one. leftoverGas, err = k.RefundGas(msg, leftoverGas) if err != nil { return nil, stacktrace.Propagate(err, "failed to refund gas leftover gas to sender %s", msg.From()) } } + // EVM execution error needs to be available for the JSON-RPC client var vmError string if vmErr != nil { vmError = vmErr.Error() @@ -280,7 +288,7 @@ func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainCo }, nil } -// GetEthIntrinsicGas get the transaction intrinsic gas cost +// GetEthIntrinsicGas returns the intrinsic gas cost for the transaction func (k *Keeper) GetEthIntrinsicGas(msg core.Message, cfg *params.ChainConfig, isContractCreation bool) (uint64, error) { height := big.NewInt(k.ctx.BlockHeight()) homestead := cfg.IsHomestead(height) @@ -289,7 +297,8 @@ func (k *Keeper) GetEthIntrinsicGas(msg core.Message, cfg *params.ChainConfig, i return core.IntrinsicGas(msg.Data(), msg.AccessList(), isContractCreation, homestead, istanbul) } -// GasToRefund calculate the amount of gas should refund to sender +// GasToRefund calculates the amount of gas the state machine should refund to the sender. It is +// capped by half of the gas consumed. func (k *Keeper) GasToRefund(gasConsumed uint64) uint64 { // Apply refund counter, capped to half of the used gas. refund := gasConsumed / 2 @@ -305,6 +314,7 @@ func (k *Keeper) GasToRefund(gasConsumed uint64) uint64 { // returned by the EVM execution, thus ignoring the previous intrinsic gas consumed during in the // AnteHandler. func (k *Keeper) RefundGas(msg core.Message, leftoverGas uint64) (uint64, error) { + // safety check: leftover gas after execution should never exceed the gas limit defined on the message if leftoverGas > msg.Gas() { return leftoverGas, stacktrace.Propagate( sdkerrors.Wrapf(types.ErrInconsistentGas, "leftover gas cannot be greater than gas limit (%d > %d)", leftoverGas, msg.Gas()), @@ -313,10 +323,12 @@ func (k *Keeper) RefundGas(msg core.Message, leftoverGas uint64) (uint64, error) } gasConsumed := msg.Gas() - leftoverGas - refund := k.GasToRefund(gasConsumed) + // calculate available gas to refund and add it to the leftover gas amount + refund := k.GasToRefund(gasConsumed) leftoverGas += refund + // safety check: leftover gas after refund should never exceed the gas limit defined on the message if leftoverGas > msg.Gas() { return leftoverGas, stacktrace.Propagate( sdkerrors.Wrapf(types.ErrInconsistentGas, "leftover gas cannot be greater than gas limit (%d > %d)", leftoverGas, msg.Gas()), diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index 02a312ee..62a62e0b 100644 --- a/x/evm/keeper/statedb.go +++ b/x/evm/keeper/statedb.go @@ -24,7 +24,9 @@ var _ vm.StateDB = &Keeper{} // ---------------------------------------------------------------------------- // CreateAccount creates a new EthAccount instance from the provided address and -// sets the value to store. +// sets the value to store. If an account with the given address already exists, +// this function also resets any preexisting code and storage associated with that +// address. func (k *Keeper) CreateAccount(addr common.Address) { cosmosAddr := sdk.AccAddress(addr.Bytes()) @@ -51,7 +53,9 @@ func (k *Keeper) CreateAccount(addr common.Address) { // Balance // ---------------------------------------------------------------------------- -// AddBalance calls CommitStateDB.AddBalance using the passed in context +// AddBalance adds the given amount to the address balance coin by minting new +// coins and transferring them to the address. The coin denomination is obtained +// from the module parameters. func (k *Keeper) AddBalance(addr common.Address, amount *big.Int) { if amount.Sign() != 1 { k.Logger(k.ctx).Debug( @@ -94,7 +98,10 @@ func (k *Keeper) AddBalance(addr common.Address, amount *big.Int) { ) } -// SubBalance calls CommitStateDB.SubBalance using the passed in context +// SubBalance subtracts the given amount from the address balance by transferring the +// coins to an escrow account and then burning them. The coin denomination is obtained +// from the module parameters. This function performs a no-op if the amount is negative +// or the user doesn't have enough funds for the transfer. func (k *Keeper) SubBalance(addr common.Address, amount *big.Int) { if amount.Sign() != 1 { k.Logger(k.ctx).Debug( @@ -111,7 +118,7 @@ func (k *Keeper) SubBalance(addr common.Address, amount *big.Int) { coins := sdk.Coins{sdk.NewCoin(params.EvmDenom, sdk.NewIntFromBigInt(amount))} if err := k.bankKeeper.SendCoinsFromAccountToModule(k.ctx, cosmosAddr, types.ModuleName, coins); err != nil { - k.Logger(k.ctx).Error( + k.Logger(k.ctx).Debug( "failed to send from account to module when subtracting balance", "ethereum-address", addr.Hex(), "cosmos-address", cosmosAddr.String(), @@ -138,7 +145,8 @@ func (k *Keeper) SubBalance(addr common.Address, amount *big.Int) { ) } -// GetBalance calls CommitStateDB.GetBalance using the passed in context +// GetBalance returns the EVM denomination balance of the provided address. The +// denomination is obtained from the module parameters. func (k *Keeper) GetBalance(addr common.Address) *big.Int { cosmosAddr := sdk.AccAddress(addr.Bytes()) params := k.GetParams(k.ctx) @@ -151,7 +159,8 @@ func (k *Keeper) GetBalance(addr common.Address) *big.Int { // Nonce // ---------------------------------------------------------------------------- -// GetNonce calls CommitStateDB.GetNonce using the passed in context +// GetNonce retrieves the account with the given address and returns the tx +// sequence (i.e nonce). The function performs a no-op if the account is not found. func (k *Keeper) GetNonce(addr common.Address) uint64 { cosmosAddr := sdk.AccAddress(addr.Bytes()) nonce, err := k.accountKeeper.GetSequence(k.ctx, cosmosAddr) @@ -209,7 +218,8 @@ func (k *Keeper) SetNonce(addr common.Address, nonce uint64) { // Code // ---------------------------------------------------------------------------- -// GetCodeHash calls CommitStateDB.GetCodeHash using the passed in context +// GetCodeHash fetches the account from the store and returns its code hash. If the account doesn't +// exist or is not an EthAccount type, GetCodeHash returns the empty code hash value. func (k *Keeper) GetCodeHash(addr common.Address) common.Hash { cosmosAddr := sdk.AccAddress(addr.Bytes()) account := k.accountKeeper.GetAccount(k.ctx, cosmosAddr) @@ -225,7 +235,8 @@ func (k *Keeper) GetCodeHash(addr common.Address) common.Hash { return common.HexToHash(ethAccount.CodeHash) } -// GetCode calls CommitStateDB.GetCode using the passed in context +// GetCode returns the code byte array associated with the given address. +// If the code hash from the account is empty, this function returns nil. func (k *Keeper) GetCode(addr common.Address) []byte { hash := k.GetCodeHash(addr) @@ -247,7 +258,8 @@ func (k *Keeper) GetCode(addr common.Address) []byte { return code } -// SetCode calls CommitStateDB.SetCode using the passed in context +// SetCode stores the code byte array to the application KVStore and sets the +// code hash to the given account. The code is deleted from the store if it is empty. func (k *Keeper) SetCode(addr common.Address, code []byte) { if bytes.Equal(code, types.EmptyCodeHash) { k.Logger(k.ctx).Debug("passed in EmptyCodeHash, but expected empty code") @@ -309,7 +321,7 @@ func (k *Keeper) GetCodeSize(addr common.Address) int { // execution has finalized. The refund value is cleared on every transaction and // at the end of every block. -// AddRefund adds the given amount of gas to the refund cached value. +// AddRefund adds the given amount of gas to the refund transient value. func (k *Keeper) AddRefund(gas uint64) { refund := k.GetRefund() @@ -319,7 +331,7 @@ func (k *Keeper) AddRefund(gas uint64) { store.Set(types.KeyPrefixTransientRefund, sdk.Uint64ToBigEndian(refund)) } -// SubRefund subtracts the given amount of gas from the refund value. This function +// SubRefund subtracts the given amount of gas from the transient refund value. This function // will panic if gas amount is greater than the stored refund. func (k *Keeper) SubRefund(gas uint64) { refund := k.GetRefund() @@ -336,7 +348,7 @@ func (k *Keeper) SubRefund(gas uint64) { } // GetRefund returns the amount of gas available for return after the tx execution -// finalises. This value is reset to 0 on every transaction. +// finalizes. This value is reset to 0 on every transaction. func (k *Keeper) GetRefund() uint64 { store := k.ctx.TransientStore(k.transientKey) @@ -622,7 +634,8 @@ func (k *Keeper) AddPreimage(_ common.Hash, _ []byte) {} // Iterator // ---------------------------------------------------------------------------- -// ForEachStorage calls CommitStateDB.ForEachStorage using passed in context +// ForEachStorage uses the store iterator to iterate over all the state keys and perform a callback +// function on each of them. func (k *Keeper) ForEachStorage(addr common.Address, cb func(key, value common.Hash) bool) error { store := k.ctx.KVStore(k.storeKey) prefix := types.AddressStoragePrefix(addr) diff --git a/x/evm/types/events.go b/x/evm/types/events.go index a4bc4c7f..79c576b1 100644 --- a/x/evm/types/events.go +++ b/x/evm/types/events.go @@ -8,6 +8,7 @@ const ( AttributeKeyRecipient = "recipient" AttributeKeyTxHash = "txHash" AttributeKeyEthereumTxHash = "ethereumTxHash" + AttributeKeyTxType = "txType" // tx failed in eth vm execution AttributeKeyEthereumTxFailed = "ethereumTxFailed" AttributeValueCategory = ModuleName diff --git a/x/evm/types/key.go b/x/evm/types/key.go index b42e6fef..45d04047 100644 --- a/x/evm/types/key.go +++ b/x/evm/types/key.go @@ -30,9 +30,6 @@ const ( prefixLogs prefixCode prefixStorage - prefixChainConfig - prefixHashTxReceipt - prefixBlockHeightTxs ) const ( @@ -53,9 +50,6 @@ var ( KeyPrefixLogs = []byte{prefixLogs} KeyPrefixCode = []byte{prefixCode} KeyPrefixStorage = []byte{prefixStorage} - KeyPrefixChainConfig = []byte{prefixChainConfig} - KeyPrefixHashTxReceipt = []byte{prefixHashTxReceipt} - KeyPrefixBlockHeightTxs = []byte{prefixBlockHeightTxs} ) var ( @@ -85,17 +79,6 @@ func StateKey(address ethcmn.Address, key []byte) []byte { return append(AddressStoragePrefix(address), key...) } -// KeyHashTxReceipt returns a key for accessing tx receipt data by hash. -func KeyHashTxReceipt(hash ethcmn.Hash) []byte { - return append(KeyPrefixHashTxReceipt, hash.Bytes()...) -} - -// KeyBlockHeightTxs returns a key for accessing tx hash list by block height. -func KeyBlockHeightTxs(height uint64) []byte { - heightBytes := sdk.Uint64ToBigEndian(height) - return append(KeyPrefixBlockHeightTxs, heightBytes...) -} - // KeyAddressStorage returns the key hash to access a given account state. The composite key // (address + hash) is hashed using Keccak256. func KeyAddressStorage(address ethcmn.Address, hash ethcmn.Hash) ethcmn.Hash {