fix eth_estimateGas response for reverted txs

jsonrpc/endpoints_eth.go

@@ -15,6 +15,7 @@ import (
 	"github.com/0xPolygonHermez/zkevm-node/log"
 	"github.com/0xPolygonHermez/zkevm-node/pool"
 	"github.com/0xPolygonHermez/zkevm-node/state"
+	"github.com/0xPolygonHermez/zkevm-node/state/runtime"
 	"github.com/ethereum/go-ethereum/common"
 	ethTypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/gorilla/websocket"
@@ -149,7 +150,14 @@ func (e *EthEndpoints) EstimateGas(arg *types.TxArgs, blockArg *types.BlockNumbe
 			return rpcErrorResponse(types.DefaultErrorCode, "failed to convert arguments into an unsigned transaction", err)
 		}
 
-		gasEstimation, err := e.state.EstimateGas(tx, sender, blockToProcess, dbTx)
+		gasEstimation, returnValue, err := e.state.EstimateGas(tx, sender, blockToProcess, dbTx)
+		if errors.Is(err, runtime.ErrExecutionReverted) {
+			data := make([]byte, len(returnValue))
+			copy(data, returnValue)
+			return rpcErrorResponseWithData(types.RevertedErrorCode, err.Error(), &data, nil)
+		} else if err != nil {
+			return rpcErrorResponse(types.DefaultErrorCode, err.Error(), nil)
+		}
 		if err != nil {
 			return rpcErrorResponse(types.DefaultErrorCode, err.Error(), nil)
 		}

jsonrpc/types/interfaces.go

@@ -29,7 +29,7 @@ type StateInterface interface {
 	PrepareWebSocket()
 	BeginStateTransaction(ctx context.Context) (pgx.Tx, error)
 	DebugTransaction(ctx context.Context, transactionHash common.Hash, traceConfig state.TraceConfig, dbTx pgx.Tx) (*runtime.ExecutionResult, error)
-	EstimateGas(transaction *types.Transaction, senderAddress common.Address, l2BlockNumber *uint64, dbTx pgx.Tx) (uint64, error)
+	EstimateGas(transaction *types.Transaction, senderAddress common.Address, l2BlockNumber *uint64, dbTx pgx.Tx) (uint64, []byte, error)
 	GetBalance(ctx context.Context, address common.Address, root common.Hash) (*big.Int, error)
 	GetCode(ctx context.Context, address common.Address, root common.Hash) ([]byte, error)
 	GetL2BlockByHash(ctx context.Context, hash common.Hash, dbTx pgx.Tx) (*types.Block, error)

state/state.go

@@ -161,7 +161,7 @@ func (s *State) GetStorageAt(ctx context.Context, address common.Address, positi
 }
 
 // EstimateGas for a transaction
-func (s *State) EstimateGas(transaction *types.Transaction, senderAddress common.Address, l2BlockNumber *uint64, dbTx pgx.Tx) (uint64, error) {
+func (s *State) EstimateGas(transaction *types.Transaction, senderAddress common.Address, l2BlockNumber *uint64, dbTx pgx.Tx) (uint64, []byte, error) {
 	const ethTransferGas = 21000
 
 	var lowEnd uint64
@@ -171,7 +171,7 @@ func (s *State) EstimateGas(transaction *types.Transaction, senderAddress common
 
 	lastBatches, l2BlockStateRoot, err := s.PostgresStorage.GetLastNBatchesByL2BlockNumber(ctx, l2BlockNumber, two, dbTx)
 	if err != nil {
-		return 0, err
+		return 0, nil, err
 	}
 
 	// Get latest batch from the database to get globalExitRoot and Timestamp
@@ -185,15 +185,15 @@ func (s *State) EstimateGas(transaction *types.Transaction, senderAddress common
 
 	lowEnd, err = core.IntrinsicGas(transaction.Data(), transaction.AccessList(), s.isContractCreation(transaction), true, false, false)
 	if err != nil {
-		return 0, err
+		return 0, nil, err
 	}
 
 	if lowEnd == ethTransferGas && transaction.To() != nil {
 		code, err := s.tree.GetCode(ctx, *transaction.To(), l2BlockStateRoot.Bytes())
 		if err != nil {
 			log.Warnf("error while getting transaction.to() code %v", err)
 		} else if len(code) == 0 {
-			return lowEnd, nil
+			return lowEnd, nil, nil
 		}
 	}
 
@@ -211,15 +211,15 @@ func (s *State) EstimateGas(transaction *types.Transaction, senderAddress common
 			if errors.Is(err, ErrNotFound) {
 				senderBalance = big.NewInt(0)
 			} else {
-				return 0, err
+				return 0, nil, err
 			}
 		}
 
 		availableBalance = new(big.Int).Set(senderBalance)
 
 		if transaction.Value() != nil {
 			if transaction.Value().Cmp(availableBalance) > 0 {
-				return 0, ErrInsufficientFunds
+				return 0, nil, ErrInsufficientFunds
 			}
 
 			availableBalance.Sub(availableBalance, transaction.Value())
@@ -240,8 +240,7 @@ func (s *State) EstimateGas(transaction *types.Transaction, senderAddress common
 
 	// Run the transaction with the specified gas value.
 	// Returns a status indicating if the transaction failed, if it was reverted and the accompanying error
-	testTransaction := func(gas uint64, shouldOmitErr bool) (bool, bool, uint64, error) {
-		var gasUsed uint64
+	testTransaction := func(gas uint64, shouldOmitErr bool) (failed, reverted bool, gasUsed uint64, returnValue []byte, err error) {
 		tx := types.NewTx(&types.LegacyTx{
 			Nonce:    transaction.Nonce(),
 			To:       transaction.To(),
@@ -254,7 +253,7 @@ func (s *State) EstimateGas(transaction *types.Transaction, senderAddress common
 		batchL2Data, err := EncodeUnsignedTransaction(*tx, s.cfg.ChainID, nil)
 		if err != nil {
 			log.Errorf("error encoding unsigned transaction ", err)
-			return false, false, gasUsed, err
+			return false, false, gasUsed, nil, err
 		}
 
 		forkID := GetForkIDByBatchNumber(s.cfg.ForkIDIntervals, lastBatch.BatchNumber)
@@ -290,13 +289,13 @@ func (s *State) EstimateGas(transaction *types.Transaction, senderAddress common
 		log.Debugf("executor time: %vms", time.Since(txExecutionOnExecutorTime).Milliseconds())
 		if err != nil {
 			log.Errorf("error estimating gas: %v", err)
-			return false, false, gasUsed, err
+			return false, false, gasUsed, nil, err
 		}
 		gasUsed = processBatchResponse.Responses[0].GasUsed
 		if processBatchResponse.Error != executor.EXECUTOR_ERROR_NO_ERROR {
 			err = executor.ExecutorErr(processBatchResponse.Error)
 			s.eventLog.LogExecutorError(ctx, processBatchResponse.Error, processBatchRequest)
-			return false, false, gasUsed, err
+			return false, false, gasUsed, nil, err
 		}
 
 		// Check if an out of gas error happened during EVM execution
@@ -307,34 +306,35 @@ func (s *State) EstimateGas(transaction *types.Transaction, senderAddress common
 				// Specifying the transaction failed, but not providing an error
 				// is an indication that a valid error occurred due to low gas,
 				// which will increase the lower bound for the search
-				return true, false, gasUsed, nil
+				return true, false, gasUsed, nil, nil
 			}
 
 			if isEVMRevertError(err) {
 				// The EVM reverted during execution, attempt to extract the
 				// error message and return it
-				return true, true, gasUsed, constructErrorFromRevert(err, processBatchResponse.Responses[0].ReturnValue)
+				returnValue := processBatchResponse.Responses[0].ReturnValue
+				return true, true, gasUsed, returnValue, constructErrorFromRevert(err, returnValue)
 			}
 
-			return true, false, gasUsed, err
+			return true, false, gasUsed, nil, err
 		}
 
-		return false, false, gasUsed, nil
+		return false, false, gasUsed, nil, nil
 	}
 
 	txExecutions := []time.Duration{}
 	var totalExecutionTime time.Duration
 
 	// Check if the highEnd is a good value to make the transaction pass
-	failed, reverted, gasUsed, err := testTransaction(highEnd, false)
+	failed, reverted, gasUsed, returnValue, err := testTransaction(highEnd, false)
 	log.Debugf("Estimate gas. Trying to execute TX with %v gas", highEnd)
 	if failed {
 		if reverted {
-			return 0, err
+			return 0, returnValue, err
 		}
 
 		// The transaction shouldn't fail, for whatever reason, at highEnd
-		return 0, fmt.Errorf(
+		return 0, nil, fmt.Errorf(
 			"unable to apply transaction even for the highest gas limit %d: %w",
 			highEnd,
 			err,
@@ -352,14 +352,14 @@ func (s *State) EstimateGas(transaction *types.Transaction, senderAddress common
 
 		log.Debugf("Estimate gas. Trying to execute TX with %v gas", mid)
 
-		failed, reverted, _, testErr := testTransaction(mid, true)
+		failed, reverted, _, _, testErr := testTransaction(mid, true)
 		executionTime := time.Since(txExecutionStart)
 		totalExecutionTime += executionTime
 		txExecutions = append(txExecutions, executionTime)
 		if testErr != nil && !reverted {
 			// Reverts are ignored in the binary search, but are checked later on
 			// during the execution for the optimal gas limit found
-			return 0, testErr
+			return 0, nil, testErr
 		}
 
 		if failed {
@@ -379,7 +379,7 @@ func (s *State) EstimateGas(transaction *types.Transaction, senderAddress common
 	} else {
 		log.Error("Estimate gas. Tx not executed")
 	}
-	return highEnd, nil
+	return highEnd, nil, nil
 }
 
 // Checks if executor level valid gas errors occurred

state/state_test.go

@@ -2226,15 +2226,15 @@ func TestExecutorEstimateGas(t *testing.T) {
 	blockNumber, err := testState.GetLastL2BlockNumber(ctx, nil)
 	require.NoError(t, err)
 
-	estimatedGas, err := testState.EstimateGas(signedTx2, sequencerAddress, &blockNumber, nil)
+	estimatedGas, _, err := testState.EstimateGas(signedTx2, sequencerAddress, &blockNumber, nil)
 	require.NoError(t, err)
 	log.Debugf("Estimated gas = %v", estimatedGas)
 
 	nonce++
 	tx3 := types.NewTransaction(nonce, scAddress, new(big.Int), 40000, new(big.Int).SetUint64(1), common.Hex2Bytes("4abbb40a"))
 	signedTx3, err := auth.Signer(auth.From, tx3)
 	require.NoError(t, err)
-	_, err = testState.EstimateGas(signedTx3, sequencerAddress, &blockNumber, nil)
+	_, _, err = testState.EstimateGas(signedTx3, sequencerAddress, &blockNumber, nil)
 	require.Error(t, err)
 }
 
@@ -2367,7 +2367,7 @@ func TestExecutorGasRefund(t *testing.T) {
 	signedTx2, err := auth.Signer(auth.From, tx2)
 	require.NoError(t, err)
 
-	estimatedGas, err := testState.EstimateGas(signedTx2, sequencerAddress, nil, nil)
+	estimatedGas, _, err := testState.EstimateGas(signedTx2, sequencerAddress, nil, nil)
 	require.NoError(t, err)
 	log.Debugf("Estimated gas = %v", estimatedGas)
 
@@ -2589,7 +2589,7 @@ func TestExecutorGasEstimationMultisig(t *testing.T) {
 	blockNumber, err := testState.GetLastL2BlockNumber(ctx, nil)
 	require.NoError(t, err)
 
-	estimatedGas, err := testState.EstimateGas(signedTx6, sequencerAddress, &blockNumber, nil)
+	estimatedGas, _, err := testState.EstimateGas(signedTx6, sequencerAddress, &blockNumber, nil)
 	require.NoError(t, err)
 	log.Debugf("Estimated gas = %v", estimatedGas)
 

test/e2e/jsonrpc2_test.go

@@ -369,6 +369,67 @@ func Test_RevertOnSCCallTransaction(t *testing.T) {
 	}
 }
 
+func Test_RevertOnSCCallGasEstimation(t *testing.T) {
+	if testing.Short() {
+		t.Skip()
+	}
+	setup()
+	defer teardown()
+
+	ctx := context.Background()
+
+	for _, network := range networks {
+		log.Infof("Network %s", network.Name)
+
+		client := operations.MustGetClient(network.URL)
+		auth := operations.MustGetAuth(network.PrivateKey, network.ChainID)
+
+		auth.GasLimit = 1000000
+
+		_, scTx, sc, err := Revert2.DeployRevert2(auth, client)
+		require.NoError(t, err)
+
+		err = operations.WaitTxToBeMined(ctx, client, scTx, operations.DefaultTimeoutTxToBeMined)
+		require.NoError(t, err)
+
+		tx, err := sc.GenerateError(auth)
+		require.NoError(t, err)
+
+		err = operations.WaitTxToBeMined(ctx, client, tx, operations.DefaultTimeoutTxToBeMined)
+		errMsg := err.Error()
+		prefix := "transaction has failed, reason: execution reverted: Today is not juernes"
+		hasPrefix := strings.HasPrefix(errMsg, prefix)
+		require.True(t, hasPrefix)
+
+		receipt, err := client.TransactionReceipt(ctx, tx.Hash())
+		require.NoError(t, err)
+
+		assert.Equal(t, receipt.Status, ethTypes.ReceiptStatusFailed)
+
+		msg := ethereum.CallMsg{
+			From: auth.From,
+			To:   tx.To(),
+			Gas:  tx.Gas(),
+
+			Value: tx.Value(),
+			Data:  tx.Data(),
+		}
+		result, err := client.EstimateGas(ctx, msg)
+		require.NotNil(t, err)
+		require.Equal(t, uint64(0), result)
+		rpcErr := err.(rpc.Error)
+		assert.Equal(t, 3, rpcErr.ErrorCode())
+		assert.Equal(t, "execution reverted: Today is not juernes", rpcErr.Error())
+
+		dataErr := err.(rpc.DataError)
+		data := dataErr.ErrorData().(string)
+		decodedData := hex.DecodeBig(data)
+		unpackedData, err := abi.UnpackRevert(decodedData.Bytes())
+		require.NoError(t, err)
+		assert.Equal(t, "Today is not juernes", unpackedData)
+	}
+}
+
 func TestCallMissingParameters(t *testing.T) {
 	if testing.Short() {
 		t.Skip()