EVM-801 Drop tx bug for updateAccountSkipsCounts (#1865)

txpool/account.go

@@ -357,12 +357,12 @@ func (a *account) promote() (promoted []*types.Transaction, pruned []*types.Tran
 
 // resetSkips sets 0 to skips
 func (a *account) resetSkips() {
-	a.skips = 0
+	atomic.StoreUint64(&a.skips, 0)
 }
 
 // incrementSkips increments skips
-func (a *account) incrementSkips() {
-	a.skips++
+func (a *account) incrementSkips() uint64 {
+	return atomic.AddUint64(&a.skips, 1)
 }
 
 // getLowestTx returns the transaction with lowest nonce, which might be popped next

txpool/mock_test.go

@@ -18,6 +18,7 @@ type defaultMockStore struct {
 
 	getBlockByHashFn   func(types.Hash, bool) (*types.Block, bool)
 	calculateBaseFeeFn func(*types.Header) uint64
+	nonce              uint64
 }
 
 func NewDefaultMockStore(header *types.Header) defaultMockStore {
@@ -34,7 +35,7 @@ func (m defaultMockStore) Header() *types.Header {
 }
 
 func (m defaultMockStore) GetNonce(types.Hash, types.Address) uint64 {
-	return 0
+	return m.nonce
 }
 
 func (m defaultMockStore) GetBlockByHash(hash types.Hash, full bool) (*types.Block, bool) {

txpool/txpool.go

@@ -393,9 +393,14 @@ func (p *TxPool) Pop(tx *types.Transaction) {
 // Drop clears the entire account associated with the given transaction
 // and reverts its next (expected) nonce.
 func (p *TxPool) Drop(tx *types.Transaction) {
-	// fetch associated account
 	account := p.accounts.get(tx.From)
+	p.dropAccount(account, tx.Nonce, tx)
+}
 
+// dropAccount clears all promoted and enqueued tx from the account
+// signals EventType_DROPPED for provided hash, clears all the slots and metrics
+// and sets nonce to provided nonce
+func (p *TxPool) dropAccount(account *account, nextNonce uint64, tx *types.Transaction) {
 	account.promoted.lock(true)
 	account.enqueued.lock(true)
 	account.nonceToTx.lock()
@@ -419,7 +424,6 @@ func (p *TxPool) Drop(tx *types.Transaction) {
 	}
 
 	// rollback nonce
-	nextNonce := tx.Nonce
 	account.setNonce(nextNonce)
 
 	// reset accounts nonce map
@@ -1001,6 +1005,7 @@ func (p *TxPool) resetAccounts(stateNonces map[types.Address]uint64) {
 // updateAccountSkipsCounts update the accounts' skips,
 // the number of the consecutive blocks that doesn't have the account's transactions
 func (p *TxPool) updateAccountSkipsCounts(latestActiveAccounts map[types.Address]uint64) {
+	stateRoot := p.store.Header().StateRoot
 	p.accounts.Range(
 		func(key, value interface{}) bool {
 			address, _ := key.(types.Address)
@@ -1019,14 +1024,13 @@ func (p *TxPool) updateAccountSkipsCounts(latestActiveAccounts map[types.Address
 				return true
 			}
 
-			account.incrementSkips()
-
-			if account.skips < maxAccountSkips {
+			if account.incrementSkips() < maxAccountSkips {
 				return true
 			}
 
 			// account has been skipped too many times
-			p.Drop(firstTx)
+			nextNonce := p.store.GetNonce(stateRoot, firstTx.From)
+			p.dropAccount(account, nextNonce, firstTx)
 
 			account.resetSkips()
 

txpool/txpool_test.go

@@ -2055,6 +2055,51 @@ func Test_updateAccountSkipsCounts(t *testing.T) {
 		assert.Equal(t, slotsRequired(tx), pool.gauge.read())
 		checkTxExistence(t, pool, tx.Hash, true)
 	})
+
+	t.Run("drop should set nonce to current account nonce from store", func(t *testing.T) {
+		t.Parallel()
+
+		const storeNonce = uint64(10)
+
+		// create pool
+		store := defaultMockStore{
+			DefaultHeader: mockHeader,
+			nonce:         storeNonce,
+		}
+
+		pool, err := newTestPool(store)
+		assert.NoError(t, err)
+
+		pool.SetSigner(&mockSigner{})
+
+		accountMap := pool.accounts.initOnce(addr1, storeNonce)
+		accountMap.enqueued.push(&types.Transaction{
+			Nonce: storeNonce + 2,
+			Hash:  types.StringToHash("0xffa"),
+		})
+		accountMap.enqueued.push(&types.Transaction{
+			Nonce: storeNonce + 4,
+			Hash:  types.StringToHash("0xff1"),
+		})
+		accountMap.promoted.push(&types.Transaction{
+			Nonce: storeNonce,
+			Hash:  types.StringToHash("0xff2"),
+		})
+		accountMap.promoted.push(&types.Transaction{
+			Nonce: storeNonce + 1,
+			Hash:  types.StringToHash("0xff3"),
+		})
+		accountMap.setNonce(storeNonce + 3)
+		accountMap.skips = maxAccountSkips - 1
+
+		pool.updateAccountSkipsCounts(map[types.Address]uint64{})
+
+		// make sure the account queue is empty and skips is reset
+		assert.Zero(t, accountMap.enqueued.length())
+		assert.Equal(t, uint64(0), accountMap.promoted.length())
+		assert.Equal(t, uint64(0), accountMap.skips)
+		assert.Equal(t, storeNonce, accountMap.getNonce())
+	})
 }
 
 func Test_TxPool_validateTx(t *testing.T) {