From f738ef1a9b4a323dc9dd91df236dee724407d84f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Faruk=20Irmak?= <omerfirmak@gmail.com>
Date: Mon, 11 Dec 2023 12:02:45 +0300
Subject: [PATCH] Fix EstimateFee returning TxnExecutionErr on legacy endpoint
 (#1539)

---
 rpc/handlers.go      |  4 +--
 rpc/handlers_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/rpc/handlers.go b/rpc/handlers.go
index e66ba8e092..dbd55acc96 100644
--- a/rpc/handlers.go
+++ b/rpc/handlers.go
@@ -1298,8 +1298,8 @@ func (h *Handler) EstimateFee(broadcastedTxns []BroadcastedTransaction,
 
 func (h *Handler) LegacyEstimateFee(broadcastedTxns []BroadcastedTransaction, id BlockID) ([]FeeEstimate, *jsonrpc.Error) {
 	result, err := h.simulateTransactions(id, broadcastedTxns, []SimulationFlag{SkipFeeChargeFlag}, true, true)
-	if err != nil {
-		return nil, err
+	if err != nil && err.Code == ErrTransactionExecutionError.Code {
+		return nil, makeContractError(errors.New(err.Data.(TransactionExecutionErrorData).ExecutionError))
 	}
 
 	return utils.Map(result, func(tx SimulatedTransaction) FeeEstimate {
diff --git a/rpc/handlers_test.go b/rpc/handlers_test.go
index 593b1c4a79..2d0f0e526b 100644
--- a/rpc/handlers_test.go
+++ b/rpc/handlers_test.go
@@ -3690,3 +3690,61 @@ func TestSpecVersion(t *testing.T) {
 	require.Nil(t, rpcErr)
 	require.Equal(t, "0.5.1", legacyVersion)
 }
+
+func TestEstimateFee(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	const network = utils.Mainnet
+
+	mockReader := mocks.NewMockReader(mockCtrl)
+	mockVM := mocks.NewMockVM(mockCtrl)
+	log := utils.NewNopZapLogger()
+	handler := rpc.New(mockReader, nil, network, nil, nil, mockVM, "", log)
+
+	mockState := mocks.NewMockStateHistoryReader(mockCtrl)
+	mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil).AnyTimes()
+	mockReader.EXPECT().HeadsHeader().Return(&core.Header{}, nil).AnyTimes()
+	sequencerAddress := core.NetworkBlockHashMetaInfo(network).FallBackSequencerAddress
+
+	t.Run("ok with zero values", func(t *testing.T) {
+		mockVM.EXPECT().Execute(nil, nil, uint64(0), uint64(0), sequencerAddress, mockState, network, []*felt.Felt{}, true, false, true, nil, nil, false).
+			Return([]*felt.Felt{}, []json.RawMessage{}, nil)
+
+		_, err := handler.EstimateFee([]rpc.BroadcastedTransaction{}, []rpc.SimulationFlag{}, rpc.BlockID{Latest: true})
+		require.Nil(t, err)
+	})
+
+	t.Run("ok with zero values, skip validate", func(t *testing.T) {
+		mockVM.EXPECT().Execute(nil, nil, uint64(0), uint64(0), sequencerAddress, mockState, network, []*felt.Felt{}, true, true, true, nil, nil, false).
+			Return([]*felt.Felt{}, []json.RawMessage{}, nil)
+
+		_, err := handler.EstimateFee([]rpc.BroadcastedTransaction{}, []rpc.SimulationFlag{rpc.SkipValidateFlag}, rpc.BlockID{Latest: true})
+		require.Nil(t, err)
+	})
+
+	t.Run("transaction execution error", func(t *testing.T) {
+		mockVM.EXPECT().Execute(nil, nil, uint64(0), uint64(0), sequencerAddress, mockState, network, []*felt.Felt{}, true, true, true, nil, nil, false).
+			Return(nil, nil, vm.TransactionExecutionError{
+				Index: 44,
+				Cause: errors.New("oops"),
+			})
+
+		_, err := handler.EstimateFee([]rpc.BroadcastedTransaction{}, []rpc.SimulationFlag{rpc.SkipValidateFlag}, rpc.BlockID{Latest: true})
+		require.Equal(t, rpc.ErrTransactionExecutionError.CloneWithData(rpc.TransactionExecutionErrorData{
+			TransactionIndex: 44,
+			ExecutionError:   "oops",
+		}), err)
+
+		mockVM.EXPECT().Execute(nil, nil, uint64(0), uint64(0), sequencerAddress, mockState, network, []*felt.Felt{}, true, false, true, nil, nil, true).
+			Return(nil, nil, vm.TransactionExecutionError{
+				Index: 44,
+				Cause: errors.New("oops"),
+			})
+
+		_, err = handler.LegacyEstimateFee([]rpc.BroadcastedTransaction{}, rpc.BlockID{Latest: true})
+		require.Equal(t, rpc.ErrContractError.CloneWithData(rpc.ContractErrorData{
+			RevertError: "oops",
+		}), err)
+	})
+}