From b2d89204e4d51467744da313d980f7e1aa2e6402 Mon Sep 17 00:00:00 2001 From: Tudor Malene Date: Tue, 17 Dec 2024 13:06:13 +0200 Subject: [PATCH] fix errors and gas estimate --- go/enclave/evm/evm_facade.go | 46 ++----------------- go/enclave/l2chain/l2_chain.go | 14 +----- go/enclave/rpc/EstimateGas.go | 80 +++++++++------------------------- go/enclave/rpc/TenEthCall.go | 16 +++---- go/enclave/rpc/rpc_utils.go | 37 ++++++++++++++++ go/responses/responses.go | 6 ++- go/responses/types.go | 4 +- 7 files changed, 75 insertions(+), 128 deletions(-) diff --git a/go/enclave/evm/evm_facade.go b/go/enclave/evm/evm_facade.go index 89e3397b1a..ef180697c2 100644 --- a/go/enclave/evm/evm_facade.go +++ b/go/enclave/evm/evm_facade.go @@ -15,14 +15,11 @@ import ( "github.com/holiman/uint256" enclaveconfig "github.com/ten-protocol/go-ten/go/enclave/config" - "github.com/ethereum/go-ethereum/accounts/abi" - "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/params" "github.com/ten-protocol/go-ten/go/common" - "github.com/ten-protocol/go-ten/go/common/errutil" "github.com/ten-protocol/go-ten/go/common/gethencoding" "github.com/ten-protocol/go-ten/go/common/log" "github.com/ten-protocol/go-ten/go/common/measure" @@ -32,7 +29,6 @@ import ( gethcommon "github.com/ethereum/go-ethereum/common" gethcore "github.com/ethereum/go-ethereum/core" gethlog "github.com/ethereum/go-ethereum/log" - gethrpc "github.com/ten-protocol/go-ten/lib/gethfork/rpc" ) var ErrGasNotEnoughForL1 = errors.New("gas limit too low to pay for execution and l1 fees") @@ -313,19 +309,14 @@ func ExecuteCall( // 3 - error check the ApplyMessage // Read the error stored in the database. - if dbErr := cleanState.Error(); dbErr != nil { - return nil, newErrorWithReasonAndCode(dbErr) - } - - // If the result contains a revert reason, try to unpack and return it. - if result != nil && len(result.Revert()) > 0 { - return nil, newRevertError(result) + if vmerr := cleanState.Error(); vmerr != nil { + return nil, vmerr } if err != nil { // also return the result as the result can be evaluated on some errors like ErrIntrinsicGas logger.Debug(fmt.Sprintf("Error applying msg %v:", msg), log.CtrErrKey, err) - return result, err + return result, fmt.Errorf("err: %w (supplied gas %d)", err, msg.GasLimit) } return result, nil @@ -344,37 +335,6 @@ func initParams(storage storage.Storage, gethEncodingService gethencoding.Encodi return NewTenChainContext(storage, gethEncodingService, config, l), vmCfg } -func newErrorWithReasonAndCode(err error) error { - result := &errutil.DataError{ - Err: err.Error(), - } - - var e gethrpc.Error - ok := errors.As(err, &e) - if ok { - result.Code = e.ErrorCode() - } - var de gethrpc.DataError - ok = errors.As(err, &de) - if ok { - result.Reason = de.ErrorData() - } - return result -} - -func newRevertError(result *gethcore.ExecutionResult) error { - reason, errUnpack := abi.UnpackRevert(result.Revert()) - err := errors.New("execution reverted") - if errUnpack == nil { - err = fmt.Errorf("execution reverted: %v", reason) - } - return &errutil.DataError{ - Err: err.Error(), - Reason: hexutil.Encode(result.Revert()), - Code: 3, // todo - magic number, really needs thought around the value and made a constant - } -} - // used as a wrapper around the vm.EVM to allow for easier calling of smart contract view functions type localContractCaller struct { evm *vm.EVM diff --git a/go/enclave/l2chain/l2_chain.go b/go/enclave/l2chain/l2_chain.go index 352d4c4d1a..3a47dcd85b 100644 --- a/go/enclave/l2chain/l2_chain.go +++ b/go/enclave/l2chain/l2_chain.go @@ -79,12 +79,6 @@ func (oc *tenChain) Call(ctx context.Context, apiArgs *gethapi.TransactionArgs, return nil, err } - // the execution might have succeeded (err == nil) but the evm contract logic might have failed (result.Failed() == true) - if result.Failed() { - oc.logger.Debug(fmt.Sprintf("Obs_Call: Failed to execute contract %s.", apiArgs.To), log.CtrErrKey, result.Err) - return nil, result.Err - } - if oc.logger.Enabled(context.Background(), gethlog.LevelTrace) { oc.logger.Trace("Obs_Call successful", "result", hexutils.BytesToHex(result.ReturnData)) } @@ -117,13 +111,7 @@ func (oc *tenChain) ObsCallAtBlock(ctx context.Context, apiArgs *gethapi.Transac batch.Header.Root.Hex())) } - result, err := evm.ExecuteCall(ctx, callMsg, blockState, batch.Header, oc.storage, oc.gethEncodingService, oc.chainConfig, oc.gasEstimationCap, oc.config, oc.logger) - if err != nil { - // also return the result as the result can be evaluated on some errors like ErrIntrinsicGas - return result, err - } - - return result, nil + return evm.ExecuteCall(ctx, callMsg, blockState, batch.Header, oc.storage, oc.gethEncodingService, oc.chainConfig, oc.gasEstimationCap, oc.config, oc.logger) } // GetChainStateAtTransaction Returns the state of the chain at certain block height after executing transactions up to the selected transaction diff --git a/go/enclave/rpc/EstimateGas.go b/go/enclave/rpc/EstimateGas.go index 1af819a45b..3ab73d2e36 100644 --- a/go/enclave/rpc/EstimateGas.go +++ b/go/enclave/rpc/EstimateGas.go @@ -6,9 +6,9 @@ import ( "fmt" "math/big" - "github.com/ethereum/go-ethereum/accounts/abi" + "github.com/ethereum/go-ethereum/params" + "github.com/ethereum/go-ethereum/core/vm" - "github.com/ten-protocol/go-ten/go/common" "github.com/ten-protocol/go-ten/go/common/measure" "github.com/ten-protocol/go-ten/go/enclave/core" @@ -17,7 +17,6 @@ import ( gethcore "github.com/ethereum/go-ethereum/core" "github.com/ten-protocol/go-ten/go/common/gethapi" "github.com/ten-protocol/go-ten/go/common/gethencoding" - "github.com/ten-protocol/go-ten/go/common/syserr" gethrpc "github.com/ten-protocol/go-ten/lib/gethfork/rpc" ) @@ -99,12 +98,11 @@ func EstimateGasExecute(builder *CallBuilder[CallParamsWithBlock, hexutil.Uint64 // Notice that unfortunately, some slots might ve considered warm, which skews the estimation. // The single pass will run once at the highest gas cap and return gas used. Not completely reliable, // but is quick. - executionGasEstimate, gasPrice, err := rpc.estimateGasSinglePass(builder.ctx, txArgs, blockNumber, rpc.config.GasLocalExecutionCapFlag) + executionGasEstimate, revert, gasPrice, err := estimateGasSinglePass(builder.ctx, rpc, txArgs, blockNumber, rpc.config.GasLocalExecutionCapFlag) if err != nil { - err = fmt.Errorf("unable to estimate transaction - %w", err) - - if errors.Is(err, syserr.InternalError{}) { - return err + if len(revert) > 0 { + builder.Err = newRevertError(revert) + return nil } // return EVM error @@ -127,7 +125,7 @@ func EstimateGasExecute(builder *CallBuilder[CallParamsWithBlock, hexutil.Uint64 return nil } -func (rpc *EncryptionManager) calculateMaxGasCap(ctx context.Context, gasCap uint64, argsGas *hexutil.Uint64) uint64 { +func calculateMaxGasCap(ctx context.Context, rpc *EncryptionManager, gasCap uint64, argsGas *hexutil.Uint64) uint64 { // Fetch the current batch header to get the batch gas limit batchHeader, err := rpc.storage.FetchHeadBatchHeader(ctx) if err != nil { @@ -144,7 +142,7 @@ func (rpc *EncryptionManager) calculateMaxGasCap(ctx context.Context, gasCap uin // If args.Gas is specified, take the minimum of gasCap and args.Gas if argsGas != nil { argsGasUint64 := uint64(*argsGas) - if argsGasUint64 < gasCap { + if argsGasUint64 < gasCap && argsGasUint64 >= params.TxGas { rpc.logger.Debug("Gas cap adjusted based on args.Gas", "argsGas", argsGasUint64, "previousGasCap", gasCap, @@ -189,39 +187,33 @@ func calculateProxyOverhead(txArgs *gethapi.TransactionArgs) uint64 { // The modifications are an overhead buffer and a 20% increase to account for warm storage slots. This is because the stateDB // for the head batch might not be fully clean in terms of the running call. Cold storage slots cost far more than warm ones to // read and write. -func (rpc *EncryptionManager) estimateGasSinglePass(ctx context.Context, args *gethapi.TransactionArgs, blkNumber *gethrpc.BlockNumber, gasCap uint64) (hexutil.Uint64, *big.Int, common.SystemError) { - maxGasCap := rpc.calculateMaxGasCap(ctx, gasCap, args.Gas) +func estimateGasSinglePass(ctx context.Context, rpc *EncryptionManager, args *gethapi.TransactionArgs, blkNumber *gethrpc.BlockNumber, globalGasCap uint64) (hexutil.Uint64, []byte, *big.Int, error) { + maxGasCap := calculateMaxGasCap(ctx, rpc, globalGasCap, args.Gas) // allowance will either be the maxGasCap or the balance allowance. // If the users funds are floaty, this might cause issues combined with the l1 pricing. - allowance, feeCap, err := rpc.normalizeFeeCapAndAdjustGasLimit(ctx, args, blkNumber, maxGasCap) + allowance, feeCap, err := normalizeFeeCapAndAdjustGasLimit(ctx, rpc, args, blkNumber, maxGasCap) if err != nil { - return 0, nil, err + return 0, nil, nil, err } - // Set the gas limit to the provided gasCap - args.Gas = (*hexutil.Uint64)(&allowance) - // Perform a single gas estimation pass using isGasEnough - failed, result, err := rpc.isGasEnough(ctx, args, allowance, blkNumber) + failed, result, err := isGasEnough(ctx, rpc, args, allowance, blkNumber) if err != nil { // Return zero values and the encountered error if estimation fails - return 0, nil, err + return 0, nil, nil, err } if failed { - if result != nil && result.Err != vm.ErrOutOfGas { //nolint: errorlint - if len(result.Revert()) > 0 { - return 0, gethcommon.Big0, newRevertError(result) - } - return 0, gethcommon.Big0, result.Err + if result != nil && !errors.Is(result.Err, vm.ErrOutOfGas) { + return 0, result.Revert(), nil, result.Err } // If the gas cap is insufficient, return an appropriate error - return 0, nil, fmt.Errorf("gas required exceeds the provided gas cap (%d)", gasCap) + return 0, nil, nil, fmt.Errorf("gas required exceeds allowance (%d)", globalGasCap) } if result == nil { // If there's no result, something went wrong - return 0, nil, fmt.Errorf("no execution result returned") + return 0, nil, nil, fmt.Errorf("no execution result returned") } // Extract the gas used from the execution result. @@ -235,10 +227,10 @@ func (rpc *EncryptionManager) estimateGasSinglePass(ctx context.Context, args *g gasUsedBig.Div(gasUsedBig, big.NewInt(100)) gasUsed := hexutil.Uint64(gasUsedBig.Uint64()) - return gasUsed, feeCap, nil + return gasUsed, nil, feeCap, nil } -func (rpc *EncryptionManager) normalizeFeeCapAndAdjustGasLimit(ctx context.Context, args *gethapi.TransactionArgs, blkNumber *gethrpc.BlockNumber, hi uint64) (uint64, *big.Int, error) { +func normalizeFeeCapAndAdjustGasLimit(ctx context.Context, rpc *EncryptionManager, args *gethapi.TransactionArgs, blkNumber *gethrpc.BlockNumber, hi uint64) (uint64, *big.Int, error) { // Normalize the max fee per gas the call is willing to spend. var feeCap *big.Int if args.GasPrice != nil && (args.MaxFeePerGas != nil || args.MaxPriorityFeePerGas != nil) { @@ -289,7 +281,7 @@ func (rpc *EncryptionManager) normalizeFeeCapAndAdjustGasLimit(ctx context.Conte // Create a helper to check if a gas allowance results in an executable transaction // isGasEnough returns whether the gaslimit should be raised, lowered, or if it was impossible to execute the message -func (rpc *EncryptionManager) isGasEnough(ctx context.Context, args *gethapi.TransactionArgs, gas uint64, blkNumber *gethrpc.BlockNumber) (bool, *gethcore.ExecutionResult, error) { +func isGasEnough(ctx context.Context, rpc *EncryptionManager, args *gethapi.TransactionArgs, gas uint64, blkNumber *gethrpc.BlockNumber) (bool, *gethcore.ExecutionResult, error) { defer core.LogMethodDuration(rpc.logger, measure.NewStopwatch(), "enclave.go:IsGasEnough") args.Gas = (*hexutil.Uint64)(&gas) result, err := rpc.chain.ObsCallAtBlock(ctx, args, blkNumber) @@ -301,33 +293,3 @@ func (rpc *EncryptionManager) isGasEnough(ctx context.Context, args *gethapi.Tra } return result.Failed(), result, nil } - -func newRevertError(result *gethcore.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 -} diff --git a/go/enclave/rpc/TenEthCall.go b/go/enclave/rpc/TenEthCall.go index f5a22f00fb..f9dcdf9c7d 100644 --- a/go/enclave/rpc/TenEthCall.go +++ b/go/enclave/rpc/TenEthCall.go @@ -1,13 +1,11 @@ package rpc import ( - "errors" "fmt" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ten-protocol/go-ten/go/common/gethencoding" "github.com/ten-protocol/go-ten/go/common/log" - "github.com/ten-protocol/go-ten/go/common/syserr" ) func TenCallValidate(reqParams []any, builder *CallBuilder[CallParamsWithBlock, string], _ *EncryptionManager) error { @@ -52,16 +50,16 @@ func TenCallExecute(builder *CallBuilder[CallParamsWithBlock, string], rpc *Encr execResult, err := rpc.chain.Call(builder.ctx, apiArgs, blkNumber) if err != nil { rpc.logger.Debug("Failed eth_call.", log.ErrKey, err) - - // return system errors to the host - if errors.Is(err, syserr.InternalError{}) { - return err - } - - builder.Err = err + return err + } + // If the result contains a revert reason, try to unpack and return it. + if len(execResult.Revert()) > 0 { + builder.Err = newRevertError(execResult.Revert()) return nil } + builder.Err = execResult.Err + var encodedResult string if len(execResult.ReturnData) != 0 { encodedResult = hexutil.Encode(execResult.ReturnData) diff --git a/go/enclave/rpc/rpc_utils.go b/go/enclave/rpc/rpc_utils.go index 8752889d1b..bfe26da0a1 100644 --- a/go/enclave/rpc/rpc_utils.go +++ b/go/enclave/rpc/rpc_utils.go @@ -3,6 +3,10 @@ package rpc import ( "fmt" + "github.com/ethereum/go-ethereum/accounts/abi" + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/core/vm" + "github.com/ten-protocol/go-ten/go/common/gethapi" gethrpc "github.com/ten-protocol/go-ten/lib/gethfork/rpc" @@ -36,3 +40,36 @@ func storeTxEnabled[P any, R any](rpc *EncryptionManager, builder *CallBuilder[P } return true } + +// lifted from go-ethereum +// revertError is an API error that encompasses an EVM revert 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 revert. +// 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 +} + +// newRevertError creates a revertError instance with the provided revert data. +func newRevertError(revert []byte) *revertError { + err := vm.ErrExecutionReverted + + reason, errUnpack := abi.UnpackRevert(revert) + if errUnpack == nil { + err = fmt.Errorf("%w: %v", vm.ErrExecutionReverted, reason) + } + return &revertError{ + error: err, + reason: hexutil.Encode(revert), + } +} diff --git a/go/responses/responses.go b/go/responses/responses.go index f231083bd9..27dc9cf197 100644 --- a/go/responses/responses.go +++ b/go/responses/responses.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" + "github.com/ten-protocol/go-ten/lib/gethfork/rpc" + "github.com/ten-protocol/go-ten/go/common/errutil" "github.com/ten-protocol/go-ten/go/common/syserr" @@ -170,9 +172,9 @@ func DecodeResponse[T any](encoded []byte) (*T, error) { return resp.Result, nil } -func convertError(err error) *errutil.DataError { +func convertError(err error) rpc.DataError { // check if it's a serialized error and handle any error wrapping that might have occurred - var e *errutil.DataError + var e rpc.DataError if ok := errors.As(err, &e); ok { return e } diff --git a/go/responses/types.go b/go/responses/types.go index ae7cf1c90b..4d978117ae 100644 --- a/go/responses/types.go +++ b/go/responses/types.go @@ -4,7 +4,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/core/types" - "github.com/ten-protocol/go-ten/go/common/errutil" + "github.com/ten-protocol/go-ten/lib/gethfork/rpc" ) type ViewingKeyEncryptor func([]byte) ([]byte, error) @@ -13,7 +13,7 @@ type ViewingKeyEncryptor func([]byte) ([]byte, error) // which will be decoded only on the client side. type UserResponse[T any] struct { Result *T - Err *errutil.DataError + Err rpc.DataError } // Error - converts the encoded string in the response into a normal error and returns it.