Skip to content

Commit

Permalink
EVM-800 nonce too low from eth_getTransactionCount (#1853)
Browse files Browse the repository at this point in the history
  • Loading branch information
igorcrevar authored Aug 31, 2023
1 parent 1095172 commit 9b5bbe4
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 38 deletions.
19 changes: 18 additions & 1 deletion jsonrpc/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,31 @@ func (bnh *BlockNumberOrHash) UnmarshalJSON(data []byte) error {
return nil
}

// stringToBlockNumberSafe parses string and returns Block Number
// works similar to stringToBlockNumber
// but treats empty string or pending block number as a latest
func stringToBlockNumberSafe(str string) (BlockNumber, error) {
switch str {
case "", pending:
return LatestBlockNumber, nil
default:
return stringToBlockNumber(str)
}
}

// stringToBlockNumber parses string and returns Block Number
// empty string is treated as an error
// pending blocks are allowed
func stringToBlockNumber(str string) (BlockNumber, error) {
if str == "" {
return 0, fmt.Errorf("value is empty")
}

str = strings.Trim(str, "\"")
switch str {
case pending, latest:
case pending:
return PendingBlockNumber, nil
case latest:
return LatestBlockNumber, nil
case earliest:
return EarliestBlockNumber, nil
Expand Down
9 changes: 9 additions & 0 deletions jsonrpc/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ func TestBlockNumberOrHash_UnmarshalJSON(t *testing.T) {

blockNumberZero := BlockNumber(0x0)
blockNumberLatest := LatestBlockNumber
blockNumberPending := PendingBlockNumber

tests := []struct {
name string
Expand Down Expand Up @@ -58,6 +59,14 @@ func TestBlockNumberOrHash_UnmarshalJSON(t *testing.T) {
BlockNumber: &blockNumberLatest,
},
},
{
"should unmarshal pending block number properly",
`"pending"`,
false,
BlockNumberOrHash{
BlockNumber: &blockNumberPending,
},
},
{
"should unmarshal block number 0 properly #1",
`{"blockNumber": "0x0"}`,
Expand Down
2 changes: 1 addition & 1 deletion jsonrpc/dispatcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ func TestDispatcherFuncDecode(t *testing.T) {
{
"filter",
`[{"fromBlock": "pending", "toBlock": "earliest"}]`,
LogQuery{fromBlock: LatestBlockNumber, toBlock: EarliestBlockNumber}, // pending = latest
LogQuery{fromBlock: LatestBlockNumber, toBlock: EarliestBlockNumber}, // pending == latest
},
}

Expand Down
4 changes: 2 additions & 2 deletions jsonrpc/eth_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error
return nil, err
}

forksInTime := e.store.GetForksInTime(uint64(number))
forksInTime := e.store.GetForksInTime(header.Number)

var standardGas uint64
if transaction.IsContractCreation() && forksInTime.Homestead {
Expand Down Expand Up @@ -717,7 +717,7 @@ func (e *Eth) GetTransactionCount(address types.Address, filter BlockNumberOrHas

// The filter is empty, use the latest block by default
if filter.BlockNumber == nil && filter.BlockHash == nil {
filter.BlockNumber, _ = createBlockNumberPointer("latest")
filter.BlockNumber, _ = createBlockNumberPointer(latest)
}

if filter.BlockNumber == nil {
Expand Down
5 changes: 1 addition & 4 deletions jsonrpc/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,9 @@ type nonceGetter interface {
func GetNextNonce(address types.Address, number BlockNumber, store nonceGetter) (uint64, error) {
if number == PendingBlockNumber {
// Grab the latest pending nonce from the TxPool
//
// If the account is not initialized in the local TxPool,
// return the latest nonce from the world state
res := store.GetNonce(address)

return res, nil
return store.GetNonce(address), nil
}

header, err := GetBlockHeader(number, store)
Expand Down
17 changes: 5 additions & 12 deletions jsonrpc/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,13 @@ func (q *LogQuery) UnmarshalJSON(data []byte) error {

q.BlockHash = obj.BlockHash

if obj.FromBlock == "" {
q.fromBlock = LatestBlockNumber
} else {
if q.fromBlock, err = stringToBlockNumber(obj.FromBlock); err != nil {
return err
}
// pending from/to blocks or "" is treated as a latest block
if q.fromBlock, err = stringToBlockNumberSafe(obj.FromBlock); err != nil {
return err
}

if obj.ToBlock == "" {
q.toBlock = LatestBlockNumber
} else {
if q.toBlock, err = stringToBlockNumber(obj.ToBlock); err != nil {
return err
}
if q.toBlock, err = stringToBlockNumberSafe(obj.ToBlock); err != nil {
return err
}

if obj.Address != nil {
Expand Down
38 changes: 20 additions & 18 deletions jsonrpc/query_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package jsonrpc

import (
"reflect"
"testing"

"github.com/0xPolygon/polygon-edge/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

var (
Expand Down Expand Up @@ -99,13 +100,23 @@ func TestFilterDecode(t *testing.T) {
},
},
},
{
`{
"fromBlock": "earliest",
"toBlock": ""
}`,
&LogQuery{
fromBlock: EarliestBlockNumber,
toBlock: LatestBlockNumber, // empty is converted to the latest
},
},
{
`{
"fromBlock": "pending",
"toBlock": "earliest"
}`,
&LogQuery{
fromBlock: LatestBlockNumber, // pending = latest
fromBlock: LatestBlockNumber, // pending is converted to the latest
toBlock: EarliestBlockNumber,
},
},
Expand All @@ -121,22 +132,15 @@ func TestFilterDecode(t *testing.T) {
},
}

for indx, c := range cases {
for _, c := range cases {
res := &LogQuery{}
err := res.UnmarshalJSON([]byte(c.str))

if err != nil && c.res != nil {
t.Fatal(err)
}

if err == nil && c.res == nil {
t.Fatal("it should fail")
}

if c.res != nil {
if !reflect.DeepEqual(res, c.res) {
t.Fatalf("bad %d", indx)
}
require.NoError(t, err)
require.Equal(t, c.res, res)
} else {
require.Error(t, err)
}
}
}
Expand Down Expand Up @@ -223,9 +227,7 @@ func TestFilterMatch(t *testing.T) {
},
}

for indx, c := range cases {
if c.filter.Match(c.log) != c.match {
t.Fatalf("bad %d", indx)
}
for _, c := range cases {
assert.Equal(t, c.match, c.filter.Match(c.log))
}
}
4 changes: 4 additions & 0 deletions txpool/txpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,8 @@ func (p *TxPool) addTx(origin txOrigin, tx *types.Transaction) error {
} else if oldTxWithSameNonce.GetGasPrice(p.baseFee).Cmp(
tx.GetGasPrice(p.baseFee)) >= 0 {
// if tx with same nonce does exist and has same or better gas price -> return error
metrics.IncrCounter([]string{txPoolMetrics, "underpriced_tx"}, 1)

return ErrUnderpriced
}

Expand All @@ -822,6 +824,8 @@ func (p *TxPool) addTx(origin txOrigin, tx *types.Transaction) error {

// reject low nonce tx
if tx.Nonce < accountNonce {
metrics.IncrCounter([]string{txPoolMetrics, "nonce_too_low_tx"}, 1)

return ErrNonceTooLow
}
}
Expand Down

0 comments on commit 9b5bbe4

Please sign in to comment.