EstimateGas endpoint - nonce should be automatically set the current …

…nonce for the account (#1819)

jsonrpc/debug_endpoint.go

@@ -183,7 +183,7 @@ func (d *Debug) TraceCall(
 		return nil, ErrHeaderNotFound
 	}
 
-	tx, err := DecodeTxn(arg, header.Number, d.store)
+	tx, err := DecodeTxn(arg, header.Number, d.store, true)
 	if err != nil {
 		return nil, err
 	}

jsonrpc/debug_endpoint_test.go

@@ -630,6 +630,12 @@ func TestTraceCall(t *testing.T) {
 
 					return testTraceResult, nil
 				},
+				headerFn: func() *types.Header {
+					return testLatestHeader
+				},
+				getAccountFn: func(h types.Hash, a types.Address) (*Account, error) {
+					return &Account{Nonce: 1}, nil
+				},
 			},
 			result: testTraceResult,
 			err:    false,
@@ -648,6 +654,12 @@ func TestTraceCall(t *testing.T) {
 
 					return nil, false
 				},
+				headerFn: func() *types.Header {
+					return testLatestHeader
+				},
+				getAccountFn: func(h types.Hash, a types.Address) (*Account, error) {
+					return &Account{Nonce: 1}, nil
+				},
 			},
 			result: nil,
 			err:    true,
@@ -667,6 +679,9 @@ func TestTraceCall(t *testing.T) {
 				headerFn: func() *types.Header {
 					return testLatestHeader
 				},
+				getAccountFn: func(h types.Hash, a types.Address) (*Account, error) {
+					return &Account{Nonce: 1}, nil
+				},
 			},
 			result: nil,
 			err:    true,

jsonrpc/eth_blockchain_test.go

@@ -594,6 +594,10 @@ func (m *mockBlockStore) TxPoolSubscribe(request *proto.SubscribeRequest) (<-cha
 	return nil, nil, nil
 }
 
+func (m *mockBlockStore) GetAccount(root types.Hash, addr types.Address) (*Account, error) {
+	return &Account{Nonce: 0}, nil
+}
+
 func newTestBlock(number uint64, hash types.Hash) *types.Block {
 	return &types.Block{
 		Header: &types.Header{

jsonrpc/eth_endpoint.go

@@ -444,7 +444,7 @@ func (e *Eth) Call(arg *txnArgs, filter BlockNumberOrHash, apiOverride *stateOve
 		return nil, err
 	}
 
-	transaction, err := DecodeTxn(arg, header.Number, e.store)
+	transaction, err := DecodeTxn(arg, header.Number, e.store, true)
 	if err != nil {
 		return nil, err
 	}
@@ -492,7 +492,8 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error
 		return nil, err
 	}
 
-	transaction, err := DecodeTxn(arg, header.Number, e.store)
+	// testTransaction should execute tx with nonce always set to the current expected nonce for the account
+	transaction, err := DecodeTxn(arg, header.Number, e.store, true)
 	if err != nil {
 		return nil, err
 	}
@@ -589,8 +590,7 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error
 
 	// Checks if EVM level valid gas errors occurred
 	isGasEVMError := func(err error) bool {
-		return errors.Is(err, runtime.ErrOutOfGas) ||
-			errors.Is(err, runtime.ErrCodeStoreOutOfGas)
+		return errors.Is(err, runtime.ErrOutOfGas) || errors.Is(err, runtime.ErrCodeStoreOutOfGas)
 	}
 
 	// Checks if the EVM reverted during execution
@@ -601,11 +601,9 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error
 	// Run the transaction with the specified gas value.
 	// Returns a status indicating if the transaction failed and the accompanying error
 	testTransaction := func(gas uint64, shouldOmitErr bool) (bool, error) {
-		// Create a dummy transaction with the new gas
-		txn := transaction.Copy()
-		txn.Gas = gas
+		transaction.Gas = gas
 
-		result, applyErr := e.store.ApplyTxn(header, txn, nil)
+		result, applyErr := e.store.ApplyTxn(header, transaction, nil)
 
 		if applyErr != nil {
 			// Check the application error.
@@ -643,11 +641,10 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error
 
 	// Start the binary search for the lowest possible gas price
 	for lowEnd < highEnd {
-		mid := (lowEnd + highEnd) / 2
+		mid := lowEnd + ((highEnd - lowEnd) >> 1) // (lowEnd + highEnd) / 2 can overflow
 
 		failed, testErr := testTransaction(mid, true)
-		if testErr != nil &&
-			!isEVMRevertError(testErr) {
+		if testErr != nil && !isEVMRevertError(testErr) {
 			// Reverts are ignored in the binary search, but are checked later on
 			// during the execution for the optimal gas limit found
 			return 0, testErr

jsonrpc/eth_endpoint_test.go

@@ -163,12 +163,13 @@ func TestEth_DecodeTxn(t *testing.T) {
 			if tt.res != nil {
 				tt.res.ComputeHash(1)
 			}
+
 			store := newMockStore()
 			for addr, acc := range tt.accounts {
 				store.SetAccount(addr, acc)
 			}
 
-			res, err := DecodeTxn(tt.arg, 1, store)
+			res, err := DecodeTxn(tt.arg, 1, store, false)
 			assert.Equal(t, tt.res, res)
 			assert.Equal(t, tt.err, err)
 		})
@@ -288,7 +289,7 @@ func TestEth_TxnType(t *testing.T) {
 		Nonce:     0,
 		Type:      types.DynamicFeeTx,
 	}
-	res, err := DecodeTxn(args, 1, store)
+	res, err := DecodeTxn(args, 1, store, false)
 
 	expectedRes.ComputeHash(1)
 	assert.NoError(t, err)

jsonrpc/helper.go

@@ -165,15 +165,15 @@ func GetNextNonce(address types.Address, number BlockNumber, store nonceGetter)
 	return acc.Nonce, nil
 }
 
-func DecodeTxn(arg *txnArgs, blockNumber uint64, store nonceGetter) (*types.Transaction, error) {
+func DecodeTxn(arg *txnArgs, blockNumber uint64, store nonceGetter, forceSetNonce bool) (*types.Transaction, error) {
 	if arg == nil {
 		return nil, errors.New("missing value for required argument 0")
 	}
 	// set default values
 	if arg.From == nil {
 		arg.From = &types.ZeroAddress
 		arg.Nonce = argUintPtr(0)
-	} else if arg.Nonce == nil {
+	} else if arg.Nonce == nil || forceSetNonce {
 		// get nonce from the pool
 		nonce, err := GetNextNonce(*arg.From, LatestBlockNumber, store)
 		if err != nil {

jsonrpc/helper_test.go

@@ -825,7 +825,7 @@ func TestDecodeTxn(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			t.Parallel()
 
-			tx, err := DecodeTxn(test.arg, 1, test.store)
+			tx, err := DecodeTxn(test.arg, 1, test.store, false)
 
 			// DecodeTxn computes hash of tx
 			if !test.err {