Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EstimateGas endpoint - nonce should be automatically set the current nonce for the account #1819

Merged
merged 2 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jsonrpc/debug_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,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
}
Expand Down
15 changes: 15 additions & 0 deletions jsonrpc/debug_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions jsonrpc/eth_blockchain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
19 changes: 8 additions & 11 deletions jsonrpc/eth_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions jsonrpc/eth_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions jsonrpc/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion jsonrpc/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading