From d5273c9a7faea08b5bf3743903404c830d47d5c4 Mon Sep 17 00:00:00 2001 From: LeoZhang Date: Tue, 31 Oct 2023 17:41:47 +0800 Subject: [PATCH 1/2] [R4R]-[l2geth]feat: add data in eth_calls response when revert --- .../accounts/abi/bind/backends/simulated.go | 8 +- l2geth/core/state_processor.go | 6 +- l2geth/core/state_transition.go | 52 ++++++-- l2geth/core/vm/evm.go | 10 +- l2geth/core/vm/instructions.go | 14 +-- l2geth/core/vm/interpreter.go | 4 +- l2geth/eth/api_tracer.go | 6 +- l2geth/graphql/graphql.go | 16 +-- l2geth/internal/ethapi/api.go | 114 ++++++++++++------ l2geth/rpc/client.go | 8 +- l2geth/rpc/handler.go | 21 ++-- l2geth/rpc/json.go | 8 ++ l2geth/rpc/types.go | 6 + 13 files changed, 197 insertions(+), 76 deletions(-) diff --git a/l2geth/accounts/abi/bind/backends/simulated.go b/l2geth/accounts/abi/bind/backends/simulated.go index 7b204cfe27..dd3c989e9e 100644 --- a/l2geth/accounts/abi/bind/backends/simulated.go +++ b/l2geth/accounts/abi/bind/backends/simulated.go @@ -442,7 +442,13 @@ func (b *SimulatedBackend) callContract(ctx context.Context, call ethereum.CallM log.Error("new state transition fail", "err", err) return nil, 0, false, err } - return stateTransition.TransitionDb() + ret, usedGas, vmerr, err := stateTransition.TransitionDb() + failed := false + if vmerr != nil { + failed = true + } + + return ret, usedGas, failed, err } // SendTransaction updates the pending block to include the given transaction. diff --git a/l2geth/core/state_processor.go b/l2geth/core/state_processor.go index 22cc6b3d19..8b90a76363 100644 --- a/l2geth/core/state_processor.go +++ b/l2geth/core/state_processor.go @@ -111,10 +111,14 @@ func ApplyTransaction(config *params.ChainConfig, bc ChainContext, author *commo } // Apply the transaction to the current state (included in the env) - _, gas, failed, err := ApplyMessage(vmenv, msg, gp) + _, gas, vmerr, err := ApplyMessage(vmenv, msg, gp) if err != nil { return nil, err } + failed := false + if vmerr != nil { + failed = true + } // Update the state with pending changes var root []byte diff --git a/l2geth/core/state_transition.go b/l2geth/core/state_transition.go index a4074755ea..26ae69b9bd 100644 --- a/l2geth/core/state_transition.go +++ b/l2geth/core/state_transition.go @@ -89,6 +89,42 @@ type Message interface { QueueOrigin() types.QueueOrigin } +// ExecutionResult includes all output after executing given evm +// message no matter the execution itself is successful or not. +type ExecutionResult struct { + UsedGas uint64 // Total used gas but include the refunded gas + Err error // Any error encountered during the execution(listed in core/vm/errors.go) + ReturnData []byte // Returned data from evm(function result or data supplied with revert opcode) +} + +// Unwrap returns the internal evm error which allows us for further +// analysis outside. +func (result *ExecutionResult) Unwrap() error { + return result.Err +} + +// Failed returns the indicator whether the execution is successful or not +func (result *ExecutionResult) Failed() bool { return result.Err != nil } + +// Return is a helper function to help caller distinguish between revert reason +// and function return. Return returns the data after execution if no error occurs. +func (result *ExecutionResult) Return() []byte { + if result.Err != nil { + return nil + } + return common.CopyBytes(result.ReturnData) +} + +// Revert returns the concrete revert reason if the execution is aborted by `REVERT` +// opcode. Note the reason can be nil if no data supplied with revert opcode. +func (result *ExecutionResult) Revert() []byte { + log.Info("ExecutionResult", "result.Err", result.Err) + if result.Err != vm.ErrExecutionReverted { + return nil + } + return common.CopyBytes(result.ReturnData) +} + // IntrinsicGas computes the 'intrinsic gas' for a message with the given data. func IntrinsicGas(data []byte, contractCreation, isHomestead bool, isEIP2028 bool) (uint64, error) { // Set the starting gas for the raw transaction @@ -176,11 +212,11 @@ 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) ([]byte, uint64, bool, error) { +func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool) ([]byte, uint64, error, error) { stateTransition, err := NewStateTransition(evm, msg, gp) if err != nil { log.Error("apply message fall", "err", err) - return nil, 0, false, err + return nil, 0, err, err } return stateTransition.TransitionDb() } @@ -249,7 +285,7 @@ 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) { +func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, vmerr error, err error) { if err = st.preCheck(); err != nil { return } @@ -262,10 +298,10 @@ func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, failed bo // Pay intrinsic gas gas, err := IntrinsicGas(st.data, contractCreation, homestead, istanbul) if err != nil { - return nil, 0, false, err + return nil, 0, err, err } if err = st.useGas(gas); err != nil { - return nil, 0, false, err + return nil, 0, err, err } var ( @@ -273,7 +309,7 @@ func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, failed bo // vm errors do not effect consensus and are therefore // not assigned to err, except for insufficient balance // error. - vmerr error + // vmerr error ) // The access list gets created here @@ -295,7 +331,7 @@ func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, failed bo // 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, vmerr, vmerr } } st.refundGas() @@ -311,7 +347,7 @@ func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, failed bo st.state.AddBalance(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, err } func (st *StateTransition) refundGas() { diff --git a/l2geth/core/vm/evm.go b/l2geth/core/vm/evm.go index 6aef0f8ff4..a85e8820b7 100644 --- a/l2geth/core/vm/evm.go +++ b/l2geth/core/vm/evm.go @@ -299,7 +299,7 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas // when we're in homestead this also counts for code storage gas errors. if err != nil { evm.StateDB.RevertToSnapshot(snapshot) - if err != errExecutionReverted { + if err != ErrExecutionReverted { contract.UseGas(contract.Gas) } } @@ -339,7 +339,7 @@ func (evm *EVM) CallCode(caller ContractRef, addr common.Address, input []byte, ret, err = run(evm, contract, input, false) if err != nil { evm.StateDB.RevertToSnapshot(snapshot) - if err != errExecutionReverted { + if err != ErrExecutionReverted { contract.UseGas(contract.Gas) } } @@ -372,7 +372,7 @@ func (evm *EVM) DelegateCall(caller ContractRef, addr common.Address, input []by ret, err = run(evm, contract, input, false) if err != nil { evm.StateDB.RevertToSnapshot(snapshot) - if err != errExecutionReverted { + if err != ErrExecutionReverted { contract.UseGas(contract.Gas) } } @@ -413,7 +413,7 @@ func (evm *EVM) StaticCall(caller ContractRef, addr common.Address, input []byte ret, err = run(evm, contract, input, true) if err != nil { evm.StateDB.RevertToSnapshot(snapshot) - if err != errExecutionReverted { + if err != ErrExecutionReverted { contract.UseGas(contract.Gas) } } @@ -501,7 +501,7 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64, // when we're in homestead this also counts for code storage gas errors. if maxCodeSizeExceeded || (err != nil && (evm.chainRules.IsHomestead || err != ErrCodeStoreOutOfGas)) { evm.StateDB.RevertToSnapshot(snapshot) - if err != errExecutionReverted { + if err != ErrExecutionReverted { contract.UseGas(contract.Gas) } } diff --git a/l2geth/core/vm/instructions.go b/l2geth/core/vm/instructions.go index 1e68f58c00..923aa8f2fb 100644 --- a/l2geth/core/vm/instructions.go +++ b/l2geth/core/vm/instructions.go @@ -34,7 +34,7 @@ var ( ErrWriteProtection = errors.New("evm: write protection") ErrNonceUintOverflow = errors.New("nonce uint64 overflow") errReturnDataOutOfBounds = errors.New("evm: return data out of bounds") - errExecutionReverted = errors.New("evm: execution reverted") + ErrExecutionReverted = errors.New("evm: execution reverted") errMaxCodeSizeExceeded = errors.New("evm: max code size exceeded") errInvalidJump = errors.New("evm: invalid jump destination") ) @@ -721,7 +721,7 @@ func opCreate(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memor contract.Gas += returnGas interpreter.intPool.put(value, offset, size) - if suberr == errExecutionReverted { + if suberr == ErrExecutionReverted { return res, nil } return nil, nil @@ -749,7 +749,7 @@ func opCreate2(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memo contract.Gas += returnGas interpreter.intPool.put(endowment, offset, size, salt) - if suberr == errExecutionReverted { + if suberr == ErrExecutionReverted { return res, nil } return nil, nil @@ -775,7 +775,7 @@ func opCall(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory } else { stack.push(interpreter.intPool.get().SetUint64(1)) } - if err == nil || err == errExecutionReverted { + if err == nil || err == ErrExecutionReverted { ret = common.CopyBytes(ret) memory.Set(retOffset.Uint64(), retSize.Uint64(), ret) } @@ -805,7 +805,7 @@ func opCallCode(pc *uint64, interpreter *EVMInterpreter, contract *Contract, mem } else { stack.push(interpreter.intPool.get().SetUint64(1)) } - if err == nil || err == errExecutionReverted { + if err == nil || err == ErrExecutionReverted { ret = common.CopyBytes(ret) memory.Set(retOffset.Uint64(), retSize.Uint64(), ret) } @@ -831,7 +831,7 @@ func opDelegateCall(pc *uint64, interpreter *EVMInterpreter, contract *Contract, } else { stack.push(interpreter.intPool.get().SetUint64(1)) } - if err == nil || err == errExecutionReverted { + if err == nil || err == ErrExecutionReverted { ret = common.CopyBytes(ret) memory.Set(retOffset.Uint64(), retSize.Uint64(), ret) } @@ -857,7 +857,7 @@ func opStaticCall(pc *uint64, interpreter *EVMInterpreter, contract *Contract, m } else { stack.push(interpreter.intPool.get().SetUint64(1)) } - if err == nil || err == errExecutionReverted { + if err == nil || err == ErrExecutionReverted { ret = common.CopyBytes(ret) memory.Set(retOffset.Uint64(), retSize.Uint64(), ret) } diff --git a/l2geth/core/vm/interpreter.go b/l2geth/core/vm/interpreter.go index c5f913d9d9..0767076011 100644 --- a/l2geth/core/vm/interpreter.go +++ b/l2geth/core/vm/interpreter.go @@ -135,7 +135,7 @@ func NewEVMInterpreter(evm *EVM, cfg Config) *EVMInterpreter { // // It's important to note that any errors returned by the interpreter should be // considered a revert-and-consume-all-gas operation except for -// errExecutionReverted which means revert-and-keep-gas-left. +// ErrExecutionReverted which means revert-and-keep-gas-left. func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) (ret []byte, err error) { if in.intPool == nil { in.intPool = poolOfIntPools.get() @@ -289,7 +289,7 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) ( case err != nil: return nil, err case operation.reverts: - return res, errExecutionReverted + return res, ErrExecutionReverted case operation.halts: return res, nil case !operation.jumps: diff --git a/l2geth/eth/api_tracer.go b/l2geth/eth/api_tracer.go index 80c0f6619e..d6c3fb5f56 100644 --- a/l2geth/eth/api_tracer.go +++ b/l2geth/eth/api_tracer.go @@ -931,10 +931,14 @@ func (api *PrivateDebugAPI) traceTx(ctx context.Context, message core.Message, v // Run the transaction with tracing enabled. vmenv := vm.NewEVM(vmctx, statedb, chainConfig, vm.Config{Debug: true, Tracer: tracer}) - ret, gas, failed, err := core.ApplyMessage(vmenv, message, new(core.GasPool).AddGas(message.Gas())) + ret, gas, vmerr, err := core.ApplyMessage(vmenv, message, new(core.GasPool).AddGas(message.Gas())) if err != nil { return nil, fmt.Errorf("tracing failed: %v", err) } + failed := false + if vmerr != nil { + failed = true + } // Depending on the tracer type, format and return the output switch tracer := tracer.(type) { case *vm.StructLogger: diff --git a/l2geth/graphql/graphql.go b/l2geth/graphql/graphql.go index f9fc10d32a..c72ef2fa5f 100644 --- a/l2geth/graphql/graphql.go +++ b/l2geth/graphql/graphql.go @@ -776,14 +776,14 @@ func (b *Block) Call(ctx context.Context, args struct { return nil, err } } - result, gas, failed, err := ethapi.DoCall(ctx, b.backend, args.Data, *b.numberOrHash, nil, &vm.Config{}, 5*time.Second, b.backend.RPCGasCap()) + result, err := ethapi.DoCall(ctx, b.backend, args.Data, *b.numberOrHash, nil, &vm.Config{}, 5*time.Second, b.backend.RPCGasCap()) status := hexutil.Uint64(1) - if failed { + if result.Failed() { status = 0 } return &CallResult{ - data: hexutil.Bytes(result), - gasUsed: hexutil.Uint64(gas), + data: result.ReturnData, + gasUsed: hexutil.Uint64(result.UsedGas), status: status, }, err } @@ -842,14 +842,14 @@ func (p *Pending) Call(ctx context.Context, args struct { Data ethapi.CallArgs }) (*CallResult, error) { pendingBlockNr := rpc.BlockNumberOrHashWithNumber(rpc.PendingBlockNumber) - result, gas, failed, err := ethapi.DoCall(ctx, p.backend, args.Data, pendingBlockNr, nil, &vm.Config{}, 5*time.Second, p.backend.RPCGasCap()) + result, err := ethapi.DoCall(ctx, p.backend, args.Data, pendingBlockNr, nil, &vm.Config{}, 5*time.Second, p.backend.RPCGasCap()) status := hexutil.Uint64(1) - if failed { + if result.Failed() { status = 0 } return &CallResult{ - data: hexutil.Bytes(result), - gasUsed: hexutil.Uint64(gas), + data: result.ReturnData, + gasUsed: hexutil.Uint64(result.UsedGas), status: status, }, err } diff --git a/l2geth/internal/ethapi/api.go b/l2geth/internal/ethapi/api.go index 0995c2c0e8..485aac5949 100644 --- a/l2geth/internal/ethapi/api.go +++ b/l2geth/internal/ethapi/api.go @@ -830,12 +830,12 @@ type Account struct { StateDiff *map[common.Hash]common.Hash `json:"stateDiff"` } -func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides map[common.Address]Account, vmCfg *vm.Config, timeout time.Duration, globalGasCap *big.Int) ([]byte, uint64, bool, error) { +func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides map[common.Address]Account, vmCfg *vm.Config, timeout time.Duration, globalGasCap *big.Int) (*core.ExecutionResult, error) { defer func(start time.Time) { log.Debug("Executing EVM call finished", "runtime", time.Since(start)) }(time.Now()) state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { - return nil, 0, false, err + return nil, err } // Set sender address or use a default if none specified var addr common.Address @@ -865,7 +865,7 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo state.SetBalance(addr, (*big.Int)(*account.Balance)) } if account.State != nil && account.StateDiff != nil { - return nil, 0, false, fmt.Errorf("account %s has both 'state' and 'stateDiff'", addr.Hex()) + return nil, fmt.Errorf("account %s has both 'state' and 'stateDiff'", addr.Hex()) } // Replace entire state if caller requires. if account.State != nil { @@ -910,13 +910,13 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo if rcfg.UsingBVM { block, err := b.BlockByNumber(ctx, rpc.BlockNumber(header.Number.Uint64())) if err != nil { - return nil, 0, false, err + return nil, err } if block != nil { txs := block.Transactions() if header.Number.Uint64() != 0 { if len(txs) != 1 { - return nil, 0, false, fmt.Errorf("block %d has more than 1 transaction", header.Number.Uint64()) + return nil, fmt.Errorf("block %d has more than 1 transaction", header.Number.Uint64()) } tx := txs[0] blockNumber = tx.L1BlockNumber() @@ -943,7 +943,7 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo // Get a new instance of the EVM. evm, vmError, err := b.GetEVM(ctx, msg, state, header, vmCfg) if err != nil { - return nil, 0, false, err + return nil, err } // Wait for the context to be done and cancel the evm. Even if the // EVM has finished, cancelling may be done (repeatedly) @@ -955,15 +955,54 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo // 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) + res, gas, vmerr, err := core.ApplyMessage(evm, msg, gp) + result := &core.ExecutionResult{ + UsedGas: gas, + Err: vmerr, + ReturnData: res, + } if err := vmError(); err != nil { - return nil, 0, false, err + return nil, 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 nil, fmt.Errorf("execution aborted (timeout = %v)", timeout) + } + if err != nil { + return result, fmt.Errorf("err: %w (supplied gas %d)", err, msg.Gas()) } - return res, gas, failed, err + + return result, err +} + +func newRevertError(result *core.ExecutionResult) *revertError { + reason, errUnpack := abi.UnpackRevert(result.Revert()) + err := errors.New("execution reverted") + if errUnpack == nil { + err = fmt.Errorf("execution reverted: %v", reason) + } + return &revertError{ + error: err, + reason: hexutil.Encode(result.Revert()), + } +} + +// 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. @@ -977,19 +1016,15 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr if overrides != nil { accounts = *overrides } - result, _, failed, err := DoCall(ctx, s.b, args, blockNrOrHash, accounts, &vm.Config{}, 5*time.Second, s.b.RPCGasCap()) + result, err := DoCall(ctx, s.b, args, blockNrOrHash, accounts, &vm.Config{}, 5*time.Second, s.b.RPCGasCap()) if err != nil { return nil, err } - if failed { - reason, errUnpack := abi.UnpackRevert(result) - err := errors.New("execution reverted") - if errUnpack == nil { - err = fmt.Errorf("execution reverted: %v", reason) - } - return (hexutil.Bytes)(result), err + // If the result contains a revert reason, try to unpack and return it. + if len(result.Revert()) > 0 { + return nil, newRevertError(result) } - return (hexutil.Bytes)(result), err + return result.Return(), result.Err } // Mantle note: The gasPrice in Mantle is modified to always return 1 gwei. We @@ -1035,21 +1070,30 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash args.From = &common.Address{} } // Create a helper to check if a gas allowance results in an executable transaction - executable := func(gas uint64) (bool, []byte) { + executable := func(gas uint64) (bool, *core.ExecutionResult, error) { args.Gas = (*hexutil.Uint64)(&gas) - res, _, failed, err := DoCall(ctx, b, args, blockNrOrHash, nil, &vm.Config{}, 0, gasCap) - if err != nil || failed { - return false, res + result, err := DoCall(ctx, b, args, blockNrOrHash, nil, &vm.Config{}, 0, gasCap) + if err != nil { + if errors.Is(err, core.ErrIntrinsicGas) { + return true, nil, nil // Special case, raise gas limit + } + return true, nil, err } - return true, res + return result.Failed(), result, nil } // Execute the binary search and hone in on an executable gas limit for lo+1 < hi { mid := (hi + lo) / 2 - ok, _ := executable(mid) + failed, _, err := executable(mid) - if !ok { + // 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 failed { lo = mid } else { hi = mid @@ -1057,16 +1101,18 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash } // Reject the transaction as invalid if it still fails at the highest allowance if hi == cap { - ok, res := executable(hi) - if !ok { - if len(res) >= 4 && bytes.Equal(res[:4], abi.RevertSelector) { - reason, errUnpack := abi.UnpackRevert(res) - err := errors.New("execution reverted") - if errUnpack == nil { - err = fmt.Errorf("execution reverted: %v", reason) + failed, result, err := executable(hi) + if err != nil { + return 0, err + } + if failed { + if result != nil && result.Err != vm.ErrOutOfGas { + if len(result.Revert()) > 0 { + return 0, newRevertError(result) } - return 0, err + return 0, result.Err } + // Otherwise, the specified gas cap is too low return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap) } } diff --git a/l2geth/rpc/client.go b/l2geth/rpc/client.go index 15e413e616..3411731d61 100644 --- a/l2geth/rpc/client.go +++ b/l2geth/rpc/client.go @@ -276,6 +276,9 @@ func (c *Client) Call(result interface{}, method string, args ...interface{}) er // The result must be a pointer so that package json can unmarshal into it. You // can also pass nil, in which case the result is ignored. func (c *Client) CallContext(ctx context.Context, result interface{}, method string, args ...interface{}) error { + if result != nil && reflect.TypeOf(result).Kind() != reflect.Ptr { + return fmt.Errorf("call result parameter must be pointer or nil interface: %v", result) + } msg, err := c.newMessage(method, args...) if err != nil { return err @@ -300,7 +303,10 @@ func (c *Client) CallContext(ctx context.Context, result interface{}, method str case len(resp.Result) == 0: return ErrNoResult default: - return json.Unmarshal(resp.Result, &result) + if result == nil { + return nil + } + return json.Unmarshal(resp.Result, result) } } diff --git a/l2geth/rpc/handler.go b/l2geth/rpc/handler.go index 4b87577d44..fae95c29e5 100644 --- a/l2geth/rpc/handler.go +++ b/l2geth/rpc/handler.go @@ -34,21 +34,20 @@ import ( // // The entry points for incoming messages are: // -// h.handleMsg(message) -// h.handleBatch(message) +// h.handleMsg(message) +// h.handleBatch(message) // // Outgoing calls use the requestOp struct. Register the request before sending it // on the connection: // -// op := &requestOp{ids: ...} -// h.addRequestOp(op) +// op := &requestOp{ids: ...} +// h.addRequestOp(op) // // Now send the request, then wait for the reply to be delivered through handleMsg: // -// if err := op.wait(...); err != nil { -// h.removeRequestOp(op) // timeout, etc. -// } -// +// if err := op.wait(...); err != nil { +// h.removeRequestOp(op) // timeout, etc. +// } type handler struct { reg *serviceRegistry unsubscribeCb *callback @@ -296,7 +295,13 @@ func (h *handler) handleCallMsg(ctx *callProc, msg *jsonrpcMessage) *jsonrpcMess return nil case msg.isCall(): resp := h.handleCall(ctx, msg) + var ctx []interface{} + ctx = append(ctx, "reqid", idForLog{msg.ID}, "duration", time.Since(start)) if resp.Error != nil { + ctx = append(ctx, "err", resp.Error.Message) + if resp.Error.Data != nil { + ctx = append(ctx, "errdata", resp.Error.Data) + } h.log.Warn("Served "+msg.Method, "reqid", idForLog{msg.ID}, "t", time.Since(start), "err", resp.Error.Message) } else { h.log.Debug("Served "+msg.Method, "reqid", idForLog{msg.ID}, "t", time.Since(start)) diff --git a/l2geth/rpc/json.go b/l2geth/rpc/json.go index 61631a3d76..3be5d55f48 100644 --- a/l2geth/rpc/json.go +++ b/l2geth/rpc/json.go @@ -115,6 +115,10 @@ func errorMessage(err error) *jsonrpcMessage { if ok { msg.Error.Code = ec.ErrorCode() } + de, ok := err.(DataError) + if ok { + msg.Error.Data = de.ErrorData() + } return msg } @@ -135,6 +139,10 @@ func (err *jsonError) ErrorCode() int { return err.Code } +func (err *jsonError) ErrorData() interface{} { + return err.Data +} + // Conn is a subset of the methods of net.Conn which are sufficient for ServerCodec. type Conn interface { io.ReadWriteCloser diff --git a/l2geth/rpc/types.go b/l2geth/rpc/types.go index b19f96e0b6..d3f37c3c48 100644 --- a/l2geth/rpc/types.go +++ b/l2geth/rpc/types.go @@ -41,6 +41,12 @@ type Error interface { ErrorCode() int // returns the code } +// A DataError contains some data in addition to the error message. +type DataError interface { + Error() string // returns the message + ErrorData() interface{} // returns the error data +} + // ServerCodec implements reading, parsing and writing RPC messages for the server side of // a RPC session. Implementations must be go-routine safe since the codec can be called in // multiple go-routines concurrently. From 6da920ad460fda3136945fe6c7354aee1d58ce4f Mon Sep 17 00:00:00 2001 From: LeoZhang Date: Mon, 6 Nov 2023 15:02:08 +0800 Subject: [PATCH 2/2] [R4R]-[l2geth]feat: add data in eth_calls response when revert --- l2geth/core/state_transition.go | 1 - 1 file changed, 1 deletion(-) diff --git a/l2geth/core/state_transition.go b/l2geth/core/state_transition.go index 26ae69b9bd..b0d51d764b 100644 --- a/l2geth/core/state_transition.go +++ b/l2geth/core/state_transition.go @@ -118,7 +118,6 @@ func (result *ExecutionResult) Return() []byte { // Revert returns the concrete revert reason if the execution is aborted by `REVERT` // opcode. Note the reason can be nil if no data supplied with revert opcode. func (result *ExecutionResult) Revert() []byte { - log.Info("ExecutionResult", "result.Err", result.Err) if result.Err != vm.ErrExecutionReverted { return nil }