fix errors and gas estimate

go/common/errutil/evm_serialisable.go

@@ -1,5 +1,7 @@
 package errutil
 
+import "fmt"
+
 // DataError is an API error that encompasses an EVM error with a code and a reason
 type DataError struct {
 	Code   int         `json:"code"`
@@ -18,3 +20,7 @@ func (e DataError) ErrorCode() int {
 func (e DataError) ErrorData() interface{} {
 	return e.Reason
 }
+
+func (e DataError) String() string {
+	return fmt.Sprintf("Data Error. Message: %s, Data: %v", e.Err, e.Reason)
+}

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

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

go/enclave/rpc/EstimateGas.go

@@ -6,9 +6,10 @@ import (
 	"fmt"
 	"math/big"
 
-	"github.com/ethereum/go-ethereum/accounts/abi"
 	"github.com/ethereum/go-ethereum/core/vm"
-	"github.com/ten-protocol/go-ten/go/common"
+
+	"github.com/ethereum/go-ethereum/params"
+
 	"github.com/ten-protocol/go-ten/go/common/measure"
 	"github.com/ten-protocol/go-ten/go/enclave/core"
 
@@ -17,7 +18,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 +99,12 @@ 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)
+			rpc.logger.Debug("revert error", "err", builder.Err)
+			return nil
 		}
 
 		// return EVM error
@@ -127,7 +127,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 +144,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 +189,34 @@ 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) {
+			rpc.logger.Debug("Failed gas estimation", "error", result.Err)
+			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 +230,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,45 +284,13 @@ 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)
 	if err != nil {
-		if errors.Is(err, gethcore.ErrIntrinsicGas) {
-			return true, nil, nil // Special case, raise gas limit
-		}
+		// since we estimate gas in a single pass, any error is just returned
 		return true, nil, err // Bail out
 	}
 	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
-}

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)

go/enclave/rpc/rpc_utils.go

@@ -3,6 +3,12 @@ package rpc
 import (
 	"fmt"
 
+	"github.com/ten-protocol/go-ten/go/common/errutil"
+
+	"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 +42,18 @@ func storeTxEnabled[P any, R any](rpc *EncryptionManager, builder *CallBuilder[P
 	}
 	return true
 }
+
+// newRevertError creates a revertError instance with the provided revert data.
+func newRevertError(revert []byte) *errutil.DataError {
+	err := vm.ErrExecutionReverted
+
+	reason, errUnpack := abi.UnpackRevert(revert)
+	if errUnpack == nil {
+		err = fmt.Errorf("%w: %v", vm.ErrExecutionReverted, reason)
+	}
+	return &errutil.DataError{
+		Err:    err.Error(),
+		Code:   3, // See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal
+		Reason: hexutil.Encode(revert),
+	}
+}