From 2bd4083b61337456cc44284f2fedef9758e0f396 Mon Sep 17 00:00:00 2001 From: Janet Liang <56005637+janet-harmony@users.noreply.github.com> Date: Fri, 2 Oct 2020 12:51:17 -0700 Subject: [PATCH] [core] Return EVM errors from ApplyMessage for external use (#3375) * [rpc] Return EVM errors as RPC errors for smart contract Calls * [core] Return ExecutionResult from TransitionDb & ApplyMessage * [core] Update ApplyTransation to use ExecutionResult * [rpc] Update doEstimateGas to use ExecutionResult * [rpc] Remove redundant VMError error message wrap * [rpc] Remove debug prints * [rpc] Revert hmy_call RPC behavior to keep backwards compatibility * [core] Return ExecutionResult struct instead of pointer to struct --- core/state_processor.go | 13 +++++----- core/state_transition.go | 55 +++++++++++++++++++++++----------------- hmy/hmy.go | 5 ++-- rpc/contract.go | 33 +++++++++++++----------- rpc/transaction.go | 4 +-- 5 files changed, 61 insertions(+), 49 deletions(-) diff --git a/core/state_processor.go b/core/state_processor.go index 6664f5056..d27603e3c 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -195,7 +195,7 @@ func ApplyTransaction(config *params.ChainConfig, bc ChainContext, author *commo // about the transaction and calling mechanisms. vmenv := vm.NewEVM(context, statedb, config, cfg) // Apply the transaction to the current state (included in the env) - _, gas, failed, err := ApplyMessage(vmenv, msg, gp) + result, err := ApplyMessage(vmenv, msg, gp) if err != nil { return nil, nil, 0, err } @@ -206,13 +206,14 @@ func ApplyTransaction(config *params.ChainConfig, bc ChainContext, author *commo } else { root = statedb.IntermediateRoot(config.IsS3(header.Epoch())).Bytes() } - *usedGas += gas + *usedGas += result.UsedGas + failedExe := result.VMErr != nil // Create a new receipt for the transaction, storing the intermediate root and gas used by the tx // based on the eip phase, we're passing whether the root touch-delete accounts. - receipt := types.NewReceipt(root, failed, *usedGas) + receipt := types.NewReceipt(root, failedExe, *usedGas) receipt.TxHash = tx.Hash() - receipt.GasUsed = gas + receipt.GasUsed = result.UsedGas // if the transaction created a contract, store the creation address in the receipt. if msg.To() == nil { receipt.ContractAddress = crypto.CreateAddress(vmenv.Context.Origin, tx.Nonce()) @@ -226,13 +227,13 @@ func ApplyTransaction(config *params.ChainConfig, bc ChainContext, author *commo var cxReceipt *types.CXReceipt // Do not create cxReceipt if EVM call failed - if txType == types.SubtractionOnly && !failed { + if txType == types.SubtractionOnly && !failedExe { cxReceipt = &types.CXReceipt{tx.Hash(), msg.From(), msg.To(), tx.ShardID(), tx.ToShardID(), msg.Value()} } else { cxReceipt = nil } - return receipt, cxReceipt, gas, err + return receipt, cxReceipt, result.UsedGas, err } // ApplyStakingTransaction attempts to apply a staking transaction to the given state database diff --git a/core/state_transition.go b/core/state_transition.go index 8f99a5b37..c05b09dd1 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -22,15 +22,14 @@ import ( "math/big" "sort" - staking2 "github.com/harmony-one/harmony/staking" - "github.com/harmony-one/harmony/staking/network" - "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/rlp" "github.com/harmony-one/harmony/core/types" "github.com/harmony-one/harmony/core/vm" "github.com/harmony-one/harmony/internal/params" "github.com/harmony-one/harmony/internal/utils" + staking2 "github.com/harmony-one/harmony/staking" + "github.com/harmony-one/harmony/staking/network" staking "github.com/harmony-one/harmony/staking/types" "github.com/pkg/errors" ) @@ -97,6 +96,13 @@ type Message interface { BlockNum() *big.Int } +// ExecutionResult is the return value from a transaction committed to the DB +type ExecutionResult struct { + ReturnData []byte + UsedGas uint64 + VMErr error +} + // IntrinsicGas computes the 'intrinsic gas' for a message with the given data. func IntrinsicGas(data []byte, contractCreation, homestead, istanbul, isValidatorCreation bool) (uint64, error) { // Set the starting gas for the raw transaction @@ -157,7 +163,7 @@ func NewStateTransition(evm *vm.EVM, msg Message, gp *GasPool, bc ChainContext) // the gas used (which includes gas refunds) and an error if it failed. An error always // indicates a core error meaning that the message would always fail for that particular // state and would never be accepted within a block. -func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool) ([]byte, uint64, bool, error) { +func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool) (ExecutionResult, error) { return NewStateTransition(evm, msg, gp, nil).TransitionDb() } @@ -218,9 +224,9 @@ func (st *StateTransition) preCheck() error { // TransitionDb will transition the state by applying the current message and // returning the result including the used gas. It returns an error if failed. // An error indicates a consensus issue. -func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, failed bool, err error) { - if err = st.preCheck(); err != nil { - return +func (st *StateTransition) TransitionDb() (ExecutionResult, error) { + if err := st.preCheck(); err != nil { + return ExecutionResult{}, err } msg := st.msg sender := vm.AccountRef(msg.From()) @@ -231,34 +237,33 @@ func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, failed bo // Pay intrinsic gas gas, err := IntrinsicGas(st.data, contractCreation, homestead, istanbul, false) if err != nil { - return nil, 0, false, err + return ExecutionResult{}, err } if err = st.useGas(gas); err != nil { - return nil, 0, false, err + return ExecutionResult{}, err } - var ( - evm = st.evm - // vm errors do not effect consensus and are therefor - // not assigned to err, except for insufficient balance - // error. - vmerr error - ) + evm := st.evm + + var ret []byte + // All VM errors are valid except for insufficient balance, therefore returned separately + var vmErr error + if contractCreation { - ret, _, st.gas, vmerr = evm.Create(sender, st.data, st.gas, st.value) + ret, _, st.gas, vmErr = evm.Create(sender, st.data, st.gas, st.value) } else { // Increment the nonce for the next transaction st.state.SetNonce(msg.From(), st.state.GetNonce(sender.Address())+1) - ret, st.gas, vmerr = evm.Call(sender, st.to(), st.data, st.gas, st.value) + ret, st.gas, vmErr = evm.Call(sender, st.to(), st.data, st.gas, st.value) } - if vmerr != nil { - utils.Logger().Info().Err(vmerr).Msg("VM returned with error") + if vmErr != nil { + utils.Logger().Info().Err(vmErr).Msg("VM returned with error") // The only possible consensus-error would be if there wasn't // sufficient balance to make the transfer happen. The first // balance transfer may never fail. - if vmerr == vm.ErrInsufficientBalance { - return nil, 0, false, vmerr + if vmErr == vm.ErrInsufficientBalance { + return ExecutionResult{}, vmErr } } st.refundGas() @@ -269,7 +274,11 @@ func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, failed bo st.state.AddBalance(st.evm.Coinbase, txFee) } - return ret, st.gasUsed(), vmerr != nil, err + return ExecutionResult{ + ReturnData: ret, + UsedGas: st.gasUsed(), + VMErr: vmErr, + }, err } func (st *StateTransition) refundGas() { diff --git a/hmy/hmy.go b/hmy/hmy.go index e760f590f..0fd0e07c3 100644 --- a/hmy/hmy.go +++ b/hmy/hmy.go @@ -200,11 +200,10 @@ func (hmy *Harmony) GetNodeMetadata() commonRPC.NodeMetadata { } // GetEVM returns a new EVM entity -func (hmy *Harmony) GetEVM(ctx context.Context, msg core.Message, state *state.DB, header *block.Header) (*vm.EVM, func() error, error) { +func (hmy *Harmony) GetEVM(ctx context.Context, msg core.Message, state *state.DB, header *block.Header) (*vm.EVM, error) { state.SetBalance(msg.From(), math.MaxBig256) - vmError := func() error { return nil } vmCtx := core.NewEVMContext(msg, header, hmy.BlockChain, nil) - return vm.NewEVM(vmCtx, state, hmy.BlockChain.Config(), *hmy.BlockChain.GetVMConfig()), vmError, nil + return vm.NewEVM(vmCtx, state, hmy.BlockChain.Config(), *hmy.BlockChain.GetVMConfig()), nil } // ChainDb .. diff --git a/rpc/contract.go b/rpc/contract.go index 1fea9a121..4cc3df90b 100644 --- a/rpc/contract.go +++ b/rpc/contract.go @@ -15,7 +15,7 @@ import ( "github.com/harmony-one/harmony/core/types" "github.com/harmony-one/harmony/core/vm" "github.com/harmony-one/harmony/hmy" - internal_common "github.com/harmony-one/harmony/internal/common" + hmyCommon "github.com/harmony-one/harmony/internal/common" "github.com/harmony-one/harmony/internal/utils" ) @@ -50,10 +50,13 @@ func (s *PublicContractService) Call( blockNum := blockNumber.EthBlockNumber() // Execute call - result, _, _, err := doCall(ctx, s.hmy, args, blockNum, vm.Config{}, CallTimeout, s.hmy.RPCGasCap) + result, err := doCall(ctx, s.hmy, args, blockNum, vm.Config{}, CallTimeout, s.hmy.RPCGasCap) + if err != nil { + return nil, err + } - // Response output is the same for all versions - return result, err + // If VM returns error, still return the ReturnData, which is the contract error message + return result.ReturnData, nil } // GetCode returns the code stored at the given address in the state for the given block number. @@ -64,7 +67,7 @@ func (s *PublicContractService) GetCode( blockNum := blockNumber.EthBlockNumber() // Fetch state - address := internal_common.ParseAddr(addr) + address := hmyCommon.ParseAddr(addr) state, _, err := s.hmy.StateAndHeaderByNumber(ctx, blockNum) if state == nil || err != nil { return nil, err @@ -89,7 +92,7 @@ func (s *PublicContractService) GetStorageAt( if state == nil || err != nil { return nil, err } - address := internal_common.ParseAddr(addr) + address := hmyCommon.ParseAddr(addr) res := state.GetState(address, common.HexToHash(key)) // Response output is the same for all versions @@ -100,7 +103,7 @@ func (s *PublicContractService) GetStorageAt( func doCall( ctx context.Context, hmy *hmy.Harmony, args CallArgs, blockNum rpc.BlockNumber, vmCfg vm.Config, timeout time.Duration, globalGasCap *big.Int, -) ([]byte, uint64, bool, error) { +) (core.ExecutionResult, error) { defer func(start time.Time) { utils.Logger().Debug(). Dur("runtime", time.Since(start)). @@ -110,7 +113,7 @@ func doCall( // Fetch state state, header, err := hmy.StateAndHeaderByNumber(ctx, blockNum) if state == nil || err != nil { - return nil, 0, false, err + return core.ExecutionResult{}, err } // Set sender address or use a default if none specified @@ -166,9 +169,9 @@ func doCall( defer cancel() // Get a new instance of the EVM. - evm, vmError, err := hmy.GetEVM(ctx, msg, state, header) + evm, err := hmy.GetEVM(ctx, msg, state, header) if err != nil { - return nil, 0, false, err + return core.ExecutionResult{}, err } // Wait for the context to be done and cancel the evm. Even if the @@ -181,16 +184,16 @@ func doCall( // Setup the gas pool (also for unmetered requests) // and apply the message. gp := new(core.GasPool).AddGas(math.MaxUint64) - res, gas, failed, err := core.ApplyMessage(evm, msg, gp) - if err := vmError(); err != nil { - return nil, 0, false, err + result, err := core.ApplyMessage(evm, msg, gp) + if err != nil { + return core.ExecutionResult{}, err } // If the timer caused an abort, return an appropriate error message if evm.Cancelled() { - return nil, 0, false, fmt.Errorf("execution aborted (timeout = %v)", timeout) + return core.ExecutionResult{}, fmt.Errorf("execution aborted (timeout = %v)", timeout) } // Response output is the same for all versions - return res, gas, failed, err + return result, nil } diff --git a/rpc/transaction.go b/rpc/transaction.go index 80cbcb273..4062020e4 100644 --- a/rpc/transaction.go +++ b/rpc/transaction.go @@ -724,8 +724,8 @@ func doEstimateGas( executable := func(gas uint64) bool { args.Gas = (*hexutil.Uint64)(&gas) - _, _, failed, err := doCall(ctx, hmy, args, blockNum, vm.Config{}, 0, big.NewInt(int64(max))) - if err != nil || failed { + result, err := doCall(ctx, hmy, args, blockNum, vm.Config{}, 0, big.NewInt(int64(max))) + if err != nil || result.VMErr == nil { return false } return true