diff --git a/accounts/abi/abi.go b/accounts/abi/abi.go index 254b1f7fb..d94e2ed1a 100644 --- a/accounts/abi/abi.go +++ b/accounts/abi/abi.go @@ -19,8 +19,11 @@ package abi import ( "bytes" "encoding/json" + "errors" "fmt" "io" + + "github.com/XinFinOrg/XDC-Subnet/crypto" ) // The ABI holds information about a contract's context and available @@ -144,3 +147,26 @@ func (abi *ABI) MethodById(sigdata []byte) (*Method, error) { } return nil, fmt.Errorf("no method with id: %#x", sigdata[:4]) } + + +// revertSelector is a special function selector for revert reason unpacking. +var revertSelector = crypto.Keccak256([]byte("Error(string)"))[:4] + +// UnpackRevert resolves the abi-encoded revert reason. According to the solidity +// spec https://solidity.readthedocs.io/en/latest/control-structures.html#revert, +// the provided revert reason is abi-encoded as if it were a call to a function +// `Error(string)`. So it's a special tool for it. +func UnpackRevert(data []byte) (string, error) { + if len(data) < 4 { + return "", errors.New("invalid data for unpacking") + } + if !bytes.Equal(data[:4], revertSelector) { + return "", errors.New("invalid data for unpacking") + } + typ, _ := NewType("string") + unpacked, err := (Arguments{{Type: typ}}).Unpack2(data[4:]) + if err != nil { + return "", err + } + return unpacked[0].(string), nil +} diff --git a/accounts/abi/argument.go b/accounts/abi/argument.go index 512d8fdfa..a45885c94 100644 --- a/accounts/abi/argument.go +++ b/accounts/abi/argument.go @@ -18,6 +18,7 @@ package abi import ( "encoding/json" + "errors" "fmt" "reflect" "strings" @@ -100,6 +101,17 @@ func (arguments Arguments) Unpack(v interface{}, data []byte) error { return arguments.unpackAtomic(v, marshalledValues) } +// Unpack2 performs the operation hexdata -> Go format. +func (arguments Arguments) Unpack2(data []byte) ([]interface{}, error) { + if len(data) == 0 { + if len(arguments.NonIndexed()) != 0 { + return nil, errors.New("abi: attempting to unmarshall an empty string while arguments are expected") + } + return make([]interface{}, 0), nil + } + return arguments.UnpackValues(data) +} + func (arguments Arguments) unpackTuple(v interface{}, marshalledValues []interface{}) error { var ( diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 27978dd48..3790956a4 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -363,7 +363,7 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call XDPoSChain.Call // callContract implements common code between normal and pending contract calls. // state is modified during execution, make sure to copy it if necessary. -func (b *SimulatedBackend) callContract(ctx context.Context, call XDPoSChain.CallMsg, block *types.Block, statedb *state.StateDB) ([]byte, uint64, bool, error) { +func (b *SimulatedBackend) callContract(ctx context.Context, call XDPoSChain.CallMsg, block *types.Block, statedb *state.StateDB) (ret []byte, usedGas uint64, failed bool, err error) { // Ensure message is initialized properly. if call.GasPrice == nil { call.GasPrice = big.NewInt(1) @@ -391,7 +391,8 @@ func (b *SimulatedBackend) callContract(ctx context.Context, call XDPoSChain.Cal vmenv := vm.NewEVM(evmContext, statedb, nil, b.config, vm.Config{}) gaspool := new(core.GasPool).AddGas(math.MaxUint64) owner := common.Address{} - return core.NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner) + ret, usedGas, failed, err, _ = core.NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner) + return } // SendTransaction updates the pending block to include the given transaction. diff --git a/core/state_processor.go b/core/state_processor.go index 07550658b..28248ba06 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -409,7 +409,7 @@ func ApplyTransaction(config *params.ChainConfig, tokensFee map[common.Address]* // End Bypass blacklist address // Apply the transaction to the current state (included in the env) - _, gas, failed, err := ApplyMessage(vmenv, msg, gp, coinbaseOwner) + _, gas, failed, err, _ := ApplyMessage(vmenv, msg, gp, coinbaseOwner) if err != nil { return nil, 0, err, false diff --git a/core/state_transition.go b/core/state_transition.go index 8a1387c3a..2d3d09780 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -131,7 +131,7 @@ func NewStateTransition(evm *vm.EVM, msg Message, gp *GasPool) *StateTransition // 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, owner common.Address) ([]byte, uint64, bool, error) { +func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool, owner common.Address) ([]byte, uint64, bool, error, error) { return NewStateTransition(evm, msg, gp).TransitionDb(owner) } @@ -217,7 +217,7 @@ func (st *StateTransition) preCheck() error { // TransitionDb will transition the state by applying the current message and // returning the result including the the used gas. It returns an error if it // failed. An error indicates a consensus issue. -func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedGas uint64, failed bool, err error) { +func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedGas uint64, failed bool, err error, vmErr error) { if err = st.preCheck(); err != nil { return } @@ -230,10 +230,10 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG // Pay intrinsic gas gas, err := IntrinsicGas(st.data, contractCreation, homestead) if err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } if err = st.useGas(gas); err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } var ( @@ -263,7 +263,7 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG // sufficient balance to make the transfer happen. The first // balance transfer may never fail. if vmerr == vm.ErrInsufficientBalance { - return nil, 0, false, vmerr + return nil, 0, false, vmerr, nil } } st.refundGas() @@ -276,7 +276,7 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG st.state.AddBalance(st.evm.Coinbase, new(big.Int).Mul(new(big.Int).SetUint64(st.gasUsed()), st.gasPrice)) } - return ret, st.gasUsed(), vmerr != nil, err + return ret, st.gasUsed(), vmerr != nil, err, vmerr } func (st *StateTransition) refundGas() { diff --git a/core/token_validator.go b/core/token_validator.go index 7a5ac2b81..a1efae02f 100644 --- a/core/token_validator.go +++ b/core/token_validator.go @@ -112,7 +112,7 @@ func CallContractWithState(call ethereum.CallMsg, chain consensus.ChainContext, vmenv := vm.NewEVM(evmContext, statedb, nil, chain.Config(), vm.Config{}) gaspool := new(GasPool).AddGas(1000000) owner := common.Address{} - rval, _, _, err := NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner) + rval, _, _, err, _ := NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner) if err != nil { return nil, err } diff --git a/eth/api_tracer.go b/eth/api_tracer.go index 87c9308fd..2cacf45b8 100644 --- a/eth/api_tracer.go +++ b/eth/api_tracer.go @@ -480,7 +480,7 @@ func (api *PrivateDebugAPI) traceBlock(ctx context.Context, block *types.Block, vmenv := vm.NewEVM(vmctx, statedb, XDCxState, api.config, vm.Config{}) owner := common.Address{} - if _, _, _, err := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(msg.Gas()), owner); err != nil { + if _, _, _, err, _ := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(msg.Gas()), owner); err != nil { failed = err break } @@ -636,7 +636,7 @@ func (api *PrivateDebugAPI) traceTx(ctx context.Context, message core.Message, v vmenv := vm.NewEVM(vmctx, statedb, nil, api.config, vm.Config{Debug: true, Tracer: tracer}) owner := common.Address{} - ret, gas, failed, err := core.ApplyMessage(vmenv, message, new(core.GasPool).AddGas(message.Gas()), owner) + ret, gas, failed, err, _ := core.ApplyMessage(vmenv, message, new(core.GasPool).AddGas(message.Gas()), owner) if err != nil { return nil, fmt.Errorf("tracing failed: %v", err) } diff --git a/eth/tracers/tracers_test.go b/eth/tracers/tracers_test.go index e106d5a23..0be41b8cb 100644 --- a/eth/tracers/tracers_test.go +++ b/eth/tracers/tracers_test.go @@ -183,7 +183,7 @@ func TestPrestateTracerCreate2(t *testing.T) { t.Fatalf("failed to prepare transaction for tracing: %v", err) } st := core.NewStateTransition(evm, msg, new(core.GasPool).AddGas(tx.Gas())) - if _, _, _, err = st.TransitionDb(common.Address{}); err != nil { + if _, _, _, err, _ = st.TransitionDb(common.Address{}); err != nil { t.Fatalf("failed to execute transaction: %v", err) } // Retrieve the trace result and compare against the etalon @@ -258,7 +258,7 @@ func TestCallTracer(t *testing.T) { t.Fatalf("failed to prepare transaction for tracing: %v", err) } st := core.NewStateTransition(evm, msg, new(core.GasPool).AddGas(tx.Gas())) - if _, _, _, err = st.TransitionDb(common.Address{}); err != nil { + if _, _, _, err, _ = st.TransitionDb(common.Address{}); err != nil { t.Fatalf("failed to execute transaction: %v", err) } // Retrieve the trace result and compare against the etalon diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 9daf5875f..abceb9eee 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -32,6 +32,7 @@ import ( "github.com/XinFinOrg/XDC-Subnet/accounts" "github.com/XinFinOrg/XDC-Subnet/accounts/abi/bind" + "github.com/XinFinOrg/XDC-Subnet/accounts/abi" "github.com/XinFinOrg/XDC-Subnet/accounts/keystore" "github.com/XinFinOrg/XDC-Subnet/common" "github.com/XinFinOrg/XDC-Subnet/common/hexutil" @@ -1045,12 +1046,12 @@ type CallArgs struct { Data hexutil.Bytes `json:"data"` } -func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr rpc.BlockNumber, vmCfg vm.Config, timeout time.Duration) ([]byte, uint64, bool, error) { +func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr rpc.BlockNumber, vmCfg vm.Config, timeout time.Duration) ([]byte, uint64, bool, error, error) { defer func(start time.Time) { log.Debug("Executing EVM call finished", "runtime", time.Since(start)) }(time.Now()) statedb, header, err := s.b.StateAndHeaderByNumber(ctx, blockNr) if statedb == nil || err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } // Set sender address or use a default if none specified addr := args.From @@ -1088,20 +1089,20 @@ func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr block, err := s.b.BlockByNumber(ctx, blockNr) if err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } author, err := s.b.GetEngine().Author(block.Header()) if err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } XDCxState, err := s.b.XDCxService().GetTradingState(block, author) if err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } // Get a new instance of the EVM. evm, vmError, err := s.b.GetEVM(ctx, msg, statedb, XDCxState, header, vmCfg) if err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } // Wait for the context to be done and cancel the evm. Even if the // EVM has finished, cancelling may be done (repeatedly) @@ -1114,18 +1115,64 @@ func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr // and apply the message. gp := new(core.GasPool).AddGas(math.MaxUint64) owner := common.Address{} - res, gas, failed, err := core.ApplyMessage(evm, msg, gp, owner) + res, gas, failed, err, vmErr := core.ApplyMessage(evm, msg, gp, owner) if err := vmError(); err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } - return res, gas, failed, 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), nil + } + if err != nil { + return res, 0, false, fmt.Errorf("err: %w (supplied gas %d)", err, msg.Gas()), nil + } + return res, gas, failed, err, vmErr +} + +func newRevertError(res []byte) *revertError { + reason, errUnpack := abi.UnpackRevert(res) + err := errors.New("execution reverted") + if errUnpack == nil { + err = fmt.Errorf("execution reverted: %v", reason) + } + return &revertError{ + error: err, + reason: hexutil.Encode(res), + } +} + +// revertError is an API error that encompasses an EVM revertal with JSON error +// code and a binary data blob. +type revertError struct { + error + reason string // revert reason hex encoded +} + +// ErrorCode returns the JSON error code for a revertal. +// See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal +func (e *revertError) ErrorCode() int { + return 3 +} + +// ErrorData returns the hex encoded revert reason. +func (e *revertError) ErrorData() interface{} { + return e.reason } // Call executes the given transaction on the state for the given block number. // It doesn't make and changes in the state/blockchain and is useful to execute and retrieve values. func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNr rpc.BlockNumber) (hexutil.Bytes, error) { - result, _, _, err := s.doCall(ctx, args, blockNr, vm.Config{}, 5*time.Second) - return (hexutil.Bytes)(result), err + result, _, failed, err, vmErr := s.doCall(ctx, args, blockNr, vm.Config{}, 5*time.Second) + if err != nil { + return nil, err + } + // If the result contains a revert reason, try to unpack and return it. + if failed && len(result) > 0 { + return nil, newRevertError(result) + } + + return (hexutil.Bytes)(result), vmErr } // EstimateGas returns an estimate of the amount of gas needed to execute the @@ -1150,29 +1197,58 @@ func (s *PublicBlockChainAPI) EstimateGas(ctx context.Context, args CallArgs) (h cap = hi // Create a helper to check if a gas allowance results in an executable transaction - executable := func(gas uint64) bool { + executable := func(gas uint64) (bool, []byte, error, error) { args.Gas = hexutil.Uint64(gas) - _, _, failed, err := s.doCall(ctx, args, rpc.LatestBlockNumber, vm.Config{}, 0) - if err != nil || failed { - log.Warn("[EstimateGas] api", "err", err) - return false + res, _, failed, err, vmErr := s.doCall(ctx, args, rpc.LatestBlockNumber, vm.Config{}, 0) + if err != nil { + if errors.Is(err, core.ErrIntrinsicGas) { + return false, nil, nil, nil // Special case, raise gas limit + } + return false, nil, err, nil // Bail out } - return true + if failed { + return false, res, nil, vmErr + } + + return true, nil, nil, nil } // Execute the binary search and hone in on an executable gas limit for lo+1 < hi { mid := (hi + lo) / 2 - if !executable(mid) { + ok, _, err, _ := executable(mid) + + // If the error is not nil(consensus error), it means the provided message + // call or transaction will never be accepted no matter how much gas it is + // assigned. Return the error directly, don't struggle any more. + if err != nil { + return 0, err + } + + if !ok { lo = mid } else { hi = mid } } + // Reject the transaction as invalid if it still fails at the highest allowance if hi == cap { - if !executable(hi) { - return 0, fmt.Errorf("gas required exceeds allowance or always failing transaction") + ok, res, err, vmErr := executable(hi) + if err != nil { + return 0, err + } + + if !ok { + if vmErr != vm.ErrOutOfGas { + if len(res) > 0 { + return 0, newRevertError(res) + } + return 0, vmErr + } + + // Otherwise, the specified gas cap is too low + return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap) } } return hexutil.Uint64(hi), nil diff --git a/les/odr_test.go b/les/odr_test.go index 4161bcce0..0dd5b4a33 100644 --- a/les/odr_test.go +++ b/les/odr_test.go @@ -141,7 +141,7 @@ func odrContractCall(ctx context.Context, db ethdb.Database, config *params.Chai //vmenv := core.NewEnv(statedb, config, bc, msg, header, vm.Config{}) gp := new(core.GasPool).AddGas(math.MaxUint64) owner := common.Address{} - ret, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) + ret, _, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) res = append(res, ret...) } } else { @@ -158,7 +158,7 @@ func odrContractCall(ctx context.Context, db ethdb.Database, config *params.Chai vmenv := vm.NewEVM(context, statedb, nil, config, vm.Config{}) gp := new(core.GasPool).AddGas(math.MaxUint64) owner := common.Address{} - ret, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) + ret, _, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) if statedb.Error() == nil { res = append(res, ret...) } diff --git a/light/odr_test.go b/light/odr_test.go index 2ef2ea176..057e7cb49 100644 --- a/light/odr_test.go +++ b/light/odr_test.go @@ -188,7 +188,7 @@ func odrContractCall(ctx context.Context, db ethdb.Database, bc *core.BlockChain vmenv := vm.NewEVM(context, st, nil, config, vm.Config{}) gp := new(core.GasPool).AddGas(math.MaxUint64) owner := common.Address{} - ret, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) + ret, _, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) res = append(res, ret...) if st.Error() != nil { return res, st.Error() diff --git a/tests/state_test_util.go b/tests/state_test_util.go index 02c87a03f..c1dfe6fa7 100644 --- a/tests/state_test_util.go +++ b/tests/state_test_util.go @@ -144,7 +144,7 @@ func (t *StateTest) Run(subtest StateSubtest, vmconfig vm.Config) (*state.StateD snapshot := statedb.Snapshot() coinbase := &t.json.Env.Coinbase - if _, _, _, err := core.ApplyMessage(evm, msg, gaspool, *coinbase); err != nil { + if _, _, _, err, _ := core.ApplyMessage(evm, msg, gaspool, *coinbase); err != nil { statedb.RevertToSnapshot(snapshot) } if logs := rlpHash(statedb.Logs()); logs != common.Hash(post.Logs) {