Fix handling of lower nonces (act only when nonce is known). Refactor…

…, rename, add tests.

txcache/eviction.go

@@ -160,7 +160,7 @@ func (cache *TxCache) evictLeastLikelyToSelectTransactions() *evictionJournal {
 
 		// Remove those transactions from "txListBySender".
 		for sender, nonce := range lowestToEvictBySender {
-			cache.txListBySender.evictTransactionsWithHigherOrEqualNonces([]byte(sender), nonce)
+			cache.txListBySender.removeTransactionsWithHigherOrEqualNonce([]byte(sender), nonce)
 		}
 
 		// Remove those transactions from "txByHash".

txcache/txCache.go

@@ -131,18 +131,18 @@ func (cache *TxCache) getSenders() []*txListForSender {
 	return cache.txListBySender.getSenders()
 }
 
-// RemoveTxByHash removes tx by hash
+// RemoveTxByHash removes transactions with nonces lower or equal to the given transaction's nonce
 func (cache *TxCache) RemoveTxByHash(txHash []byte) bool {
 	cache.mutTxOperation.Lock()
 	defer cache.mutTxOperation.Unlock()
 
 	tx, foundInByHash := cache.txByHash.removeTx(string(txHash))
 	if !foundInByHash {
-		// Transaction might have been removed in the meantime (e.g. due to NotifyAccountNonce).
+		// Transaction might have been removed in the meantime.
 		return false
 	}
 
-	evicted := cache.txListBySender.removeTxReturnEvicted(tx)
+	evicted := cache.txListBySender.removeTransactionsWithLowerOrEqualNonceReturnHashes(tx)
 	if len(evicted) > 0 {
 		cache.txByHash.RemoveTxsBulk(evicted)
 	}
@@ -277,6 +277,8 @@ func (cache *TxCache) UnRegisterHandler(string) {
 // NotifyAccountNonce should be called by external components (such as interceptors and transactions processor)
 // in order to inform the cache about initial nonce gap phenomena
 func (cache *TxCache) NotifyAccountNonce(accountKey []byte, nonce uint64) {
+	log.Trace("TxCache.NotifyAccountNonce", "account", accountKey, "nonce", nonce)
+
 	cache.txListBySender.notifyAccountNonce(accountKey, nonce)
 }
 

txcache/txCache_test.go

@@ -152,7 +152,12 @@ func Test_RemoveByTxHash(t *testing.T) {
 
 	removed := cache.RemoveTxByHash([]byte("hash-1"))
 	require.True(t, removed)
-	cache.Remove([]byte("hash-2"))
+
+	removed = cache.RemoveTxByHash([]byte("hash-2"))
+	require.True(t, removed)
+
+	removed = cache.RemoveTxByHash([]byte("hash-3"))
+	require.False(t, removed)
 
 	foundTx, ok := cache.GetByTxHash([]byte("hash-1"))
 	require.False(t, ok)
@@ -161,6 +166,8 @@ func Test_RemoveByTxHash(t *testing.T) {
 	foundTx, ok = cache.GetByTxHash([]byte("hash-2"))
 	require.False(t, ok)
 	require.Nil(t, foundTx)
+
+	require.Equal(t, uint64(0), cache.CountTx())
 }
 
 func Test_CountTx_And_Len(t *testing.T) {
@@ -216,7 +223,7 @@ func Test_RemoveByTxHash_RemovesFromByHash_WhenMapsInconsistency(t *testing.T) {
 	cache.AddTx(tx)
 
 	// Cause an inconsistency between the two internal maps (theoretically possible in case of misbehaving eviction)
-	_ = cache.txListBySender.removeTxReturnEvicted(tx)
+	_ = cache.txListBySender.removeTransactionsWithLowerOrEqualNonceReturnHashes(tx)
 
 	_ = cache.RemoveTxByHash(txHash)
 	require.Equal(t, 0, cache.txByHash.backingMap.Count())
@@ -281,7 +288,7 @@ func Test_GetTransactionsPoolForSender(t *testing.T) {
 	txs = cache.GetTransactionsPoolForSender(txSender2)
 	require.Equal(t, wrappedTxs2, txs)
 
-	cache.RemoveTxByHash(txHashes2[0])
+	_ = cache.RemoveTxByHash(txHashes2[0])
 	expectedTxs := wrappedTxs2[1:]
 	txs = cache.GetTransactionsPoolForSender(txSender2)
 	require.Equal(t, expectedTxs, txs)

txcache/txListBySenderMap.go

@@ -75,8 +75,8 @@ func (txMap *txListBySenderMap) addSender(sender string) *txListForSender {
 	return listForSender
 }
 
-// removeTxReturnEvicted removes a transaction from the map
-func (txMap *txListBySenderMap) removeTxReturnEvicted(tx *WrappedTransaction) [][]byte {
+// removeTransactionsWithLowerOrEqualNonceReturnHashes removes transactions with nonces lower or equal to the given transaction's nonce.
+func (txMap *txListBySenderMap) removeTransactionsWithLowerOrEqualNonceReturnHashes(tx *WrappedTransaction) [][]byte {
 	sender := string(tx.Tx.GetSndAddr())
 
 	listForSender, ok := txMap.getListForSender(sender)
@@ -87,7 +87,7 @@ func (txMap *txListBySenderMap) removeTxReturnEvicted(tx *WrappedTransaction) []
 		return nil
 	}
 
-	evicted := listForSender.evictTransactionsWithLowerOrEqualNonces(tx.Tx.GetNonce())
+	evicted := listForSender.removeTransactionsWithLowerOrEqualNonceReturnHashes(tx.Tx.GetNonce())
 	txMap.removeSenderIfEmpty(listForSender)
 	return evicted
 }
@@ -133,16 +133,16 @@ func (txMap *txListBySenderMap) notifyAccountNonce(accountKey []byte, nonce uint
 	listForSender.notifyAccountNonce(nonce)
 }
 
-// evictTransactionsWithHigherOrEqualNonces removes transactions with nonces higher or equal to the given nonce.
+// removeTransactionsWithHigherOrEqualNonce removes transactions with nonces higher or equal to the given nonce.
 // Useful for the eviction flow.
-func (txMap *txListBySenderMap) evictTransactionsWithHigherOrEqualNonces(accountKey []byte, nonce uint64) {
+func (txMap *txListBySenderMap) removeTransactionsWithHigherOrEqualNonce(accountKey []byte, nonce uint64) {
 	sender := string(accountKey)
 	listForSender, ok := txMap.getListForSender(sender)
 	if !ok {
 		return
 	}
 
-	listForSender.evictTransactionsWithHigherOrEqualNonces(nonce)
+	listForSender.removeTransactionsWithHigherOrEqualNonce(nonce)
 	txMap.removeSenderIfEmpty(listForSender)
 }
 

txcache/txListBySenderMap_test.go

@@ -19,7 +19,7 @@ func TestSendersMap_AddTx_IncrementsCounter(t *testing.T) {
 	require.Equal(t, int64(2), myMap.counter.Get())
 }
 
-func TestSendersMap_RemoveTx_AlsoRemovesSenderWhenNoTransactionLeft(t *testing.T) {
+func TestSendersMap_removeTransactionsWithLowerOrEqualNonceReturnHashes_alsoRemovesSenderWhenNoTransactionLeft(t *testing.T) {
 	myMap := newSendersMapToTest()
 
 	txAlice1 := createTx([]byte("a1"), "alice", 1)
@@ -33,16 +33,16 @@ func TestSendersMap_RemoveTx_AlsoRemovesSenderWhenNoTransactionLeft(t *testing.T
 	require.Equal(t, uint64(2), myMap.testGetListForSender("alice").countTx())
 	require.Equal(t, uint64(1), myMap.testGetListForSender("bob").countTx())
 
-	_ = myMap.removeTxReturnEvicted(txAlice1)
+	_ = myMap.removeTransactionsWithLowerOrEqualNonceReturnHashes(txAlice1)
 	require.Equal(t, int64(2), myMap.counter.Get())
 	require.Equal(t, uint64(1), myMap.testGetListForSender("alice").countTx())
 	require.Equal(t, uint64(1), myMap.testGetListForSender("bob").countTx())
 
-	_ = myMap.removeTxReturnEvicted(txAlice2)
+	_ = myMap.removeTransactionsWithLowerOrEqualNonceReturnHashes(txAlice2)
 	// All alice's transactions have been removed now
 	require.Equal(t, int64(1), myMap.counter.Get())
 
-	_ = myMap.removeTxReturnEvicted(txBob)
+	_ = myMap.removeTransactionsWithLowerOrEqualNonceReturnHashes(txBob)
 	// Also Bob has no more transactions
 	require.Equal(t, int64(0), myMap.counter.Get())
 }

txcache/txListForSender.go

@@ -205,7 +205,7 @@ func (listForSender *txListForSender) getSequentialTxs() []*WrappedTransaction {
 
 		if isFirstTx {
 			// Handle lower nonces.
-			if accountNonce > nonce {
+			if accountNonceKnown && accountNonce > nonce {
 				log.Trace("txListForSender.getSequentialTxs, lower nonce", "sender", listForSender.sender, "nonce", nonce, "accountNonce", accountNonce)
 				continue
 			}
@@ -253,8 +253,14 @@ func (listForSender *txListForSender) notifyAccountNonce(nonce uint64) {
 	_ = listForSender.accountNonceKnown.SetReturningPrevious()
 }
 
-// evictTransactionsWithLowerOrEqualNonces removes transactions with nonces lower or equal to the given nonce
-func (listForSender *txListForSender) evictTransactionsWithLowerOrEqualNonces(targetNonce uint64) [][]byte {
+// forgetAccountNonce resets the known account nonce
+func (listForSender *txListForSender) forgetAccountNonce() {
+	listForSender.accountNonce.Set(0)
+	listForSender.accountNonceKnown.Reset()
+}
+
+// removeTransactionsWithLowerOrEqualNonceReturnHashes removes transactions with nonces lower or equal to the given nonce
+func (listForSender *txListForSender) removeTransactionsWithLowerOrEqualNonceReturnHashes(targetNonce uint64) [][]byte {
 	evictedTxHashes := make([][]byte, 0)
 
 	// We don't allow concurrent goroutines to mutate a given sender's list
@@ -281,7 +287,7 @@ func (listForSender *txListForSender) evictTransactionsWithLowerOrEqualNonces(ta
 	return evictedTxHashes
 }
 
-func (listForSender *txListForSender) evictTransactionsWithHigherOrEqualNonces(givenNonce uint64) {
+func (listForSender *txListForSender) removeTransactionsWithHigherOrEqualNonce(givenNonce uint64) {
 	listForSender.mutex.Lock()
 	defer listForSender.mutex.Unlock()
 

txcache/txListForSender_test.go

@@ -119,7 +119,7 @@ func TestListForSender_NotifyAccountNonce(t *testing.T) {
 	require.True(t, list.accountNonceKnown.IsSet())
 }
 
-func TestListForSender_evictTransactionsWithLowerOrEqualNonces(t *testing.T) {
+func TestListForSender_removeTransactionsWithLowerOrEqualNonceReturnHashes(t *testing.T) {
 	list := newUnconstrainedListToTest()
 
 	list.AddTx(createTx([]byte("tx-42"), ".", 42))
@@ -129,70 +129,96 @@ func TestListForSender_evictTransactionsWithLowerOrEqualNonces(t *testing.T) {
 
 	require.Equal(t, 4, list.items.Len())
 
-	_ = list.evictTransactionsWithLowerOrEqualNonces(43)
+	_ = list.removeTransactionsWithLowerOrEqualNonceReturnHashes(43)
 	require.Equal(t, 2, list.items.Len())
 
-	_ = list.evictTransactionsWithLowerOrEqualNonces(44)
+	_ = list.removeTransactionsWithLowerOrEqualNonceReturnHashes(44)
 	require.Equal(t, 1, list.items.Len())
 
-	_ = list.evictTransactionsWithLowerOrEqualNonces(99)
+	_ = list.removeTransactionsWithLowerOrEqualNonceReturnHashes(99)
 	require.Equal(t, 0, list.items.Len())
 }
 
 func TestListForSender_getTxs(t *testing.T) {
-	list := newUnconstrainedListToTest()
-	list.notifyAccountNonce(42)
+	t.Run("no transactions", func(t *testing.T) {
+		list := newUnconstrainedListToTest()
+		list.notifyAccountNonce(42)
 
-	// No transaction, no gap
-	require.Len(t, list.getTxs(), 0)
-	require.Len(t, list.getTxsReversed(), 0)
-	require.Len(t, list.getSequentialTxs(), 0)
+		require.Len(t, list.getTxs(), 0)
+		require.Len(t, list.getTxsReversed(), 0)
+		require.Len(t, list.getSequentialTxs(), 0)
+	})
 
-	// One gap
-	list.AddTx(createTx([]byte("tx-43"), ".", 43))
-	require.Len(t, list.getTxs(), 1)
-	require.Len(t, list.getTxsReversed(), 1)
-	require.Len(t, list.getSequentialTxs(), 0)
+	t.Run("one transaction, one gap", func(t *testing.T) {
+		list := newUnconstrainedListToTest()
+		list.notifyAccountNonce(42)
 
-	// Resolve gap
-	list.AddTx(createTx([]byte("tx-42"), ".", 42))
-	require.Len(t, list.getTxs(), 2)
-	require.Len(t, list.getTxsReversed(), 2)
-	require.Len(t, list.getSequentialTxs(), 2)
-
-	require.Equal(t, []byte("tx-42"), list.getTxs()[0].TxHash)
-	require.Equal(t, []byte("tx-43"), list.getTxs()[1].TxHash)
-	require.Equal(t, list.getTxs(), list.getSequentialTxs())
-
-	require.Equal(t, []byte("tx-43"), list.getTxsReversed()[0].TxHash)
-	require.Equal(t, []byte("tx-42"), list.getTxsReversed()[1].TxHash)
-
-	// With nonce duplicates
-	list.AddTx(createTx([]byte("tx-42++"), ".", 42).withGasPrice(1.1 * oneBillion))
-	list.AddTx(createTx([]byte("tx-43++"), ".", 43).withGasPrice(1.1 * oneBillion))
-	require.Len(t, list.getTxs(), 4)
-	require.Len(t, list.getTxsReversed(), 4)
-	require.Len(t, list.getSequentialTxs(), 2)
-
-	require.Equal(t, []byte("tx-42++"), list.getSequentialTxs()[0].TxHash)
-	require.Equal(t, []byte("tx-43++"), list.getSequentialTxs()[1].TxHash)
-
-	require.Equal(t, []byte("tx-42++"), list.getTxs()[0].TxHash)
-	require.Equal(t, []byte("tx-42"), list.getTxs()[1].TxHash)
-	require.Equal(t, []byte("tx-43++"), list.getTxs()[2].TxHash)
-	require.Equal(t, []byte("tx-43"), list.getTxs()[3].TxHash)
-
-	require.Equal(t, []byte("tx-43"), list.getTxsReversed()[0].TxHash)
-	require.Equal(t, []byte("tx-43++"), list.getTxsReversed()[1].TxHash)
-	require.Equal(t, []byte("tx-42"), list.getTxsReversed()[2].TxHash)
-	require.Equal(t, []byte("tx-42++"), list.getTxsReversed()[3].TxHash)
-
-	// With lower nonces
-	list.notifyAccountNonce(43)
-	require.Len(t, list.getTxs(), 4)
-	require.Len(t, list.getTxsReversed(), 4)
-	require.Len(t, list.getSequentialTxs(), 1)
-	require.Equal(t, []byte("tx-43++"), list.getSequentialTxs()[0].TxHash)
+		// Gap
+		list.AddTx(createTx([]byte("tx-43"), ".", 43))
+		require.Len(t, list.getTxs(), 1)
+		require.Len(t, list.getTxsReversed(), 1)
+		require.Len(t, list.getSequentialTxs(), 0)
+
+		// Resolve gap
+		list.AddTx(createTx([]byte("tx-42"), ".", 42))
+		require.Len(t, list.getTxs(), 2)
+		require.Len(t, list.getTxsReversed(), 2)
+		require.Len(t, list.getSequentialTxs(), 2)
+
+		require.Equal(t, []byte("tx-42"), list.getTxs()[0].TxHash)
+		require.Equal(t, []byte("tx-43"), list.getTxs()[1].TxHash)
+		require.Equal(t, list.getTxs(), list.getSequentialTxs())
+
+		require.Equal(t, []byte("tx-43"), list.getTxsReversed()[0].TxHash)
+		require.Equal(t, []byte("tx-42"), list.getTxsReversed()[1].TxHash)
+	})
+
+	t.Run("with nonce duplicates", func(t *testing.T) {
+		list := newUnconstrainedListToTest()
+		list.notifyAccountNonce(42)
+
+		list.AddTx(createTx([]byte("tx-42"), ".", 42))
+		list.AddTx(createTx([]byte("tx-43"), ".", 43))
+
+		list.AddTx(createTx([]byte("tx-42++"), ".", 42).withGasPrice(1.1 * oneBillion))
+		list.AddTx(createTx([]byte("tx-43++"), ".", 43).withGasPrice(1.1 * oneBillion))
+
+		require.Len(t, list.getTxs(), 4)
+		require.Len(t, list.getTxsReversed(), 4)
+		require.Len(t, list.getSequentialTxs(), 2)
+
+		require.Equal(t, []byte("tx-42++"), list.getSequentialTxs()[0].TxHash)
+		require.Equal(t, []byte("tx-43++"), list.getSequentialTxs()[1].TxHash)
+
+		require.Equal(t, []byte("tx-42++"), list.getTxs()[0].TxHash)
+		require.Equal(t, []byte("tx-42"), list.getTxs()[1].TxHash)
+		require.Equal(t, []byte("tx-43++"), list.getTxs()[2].TxHash)
+		require.Equal(t, []byte("tx-43"), list.getTxs()[3].TxHash)
+
+		require.Equal(t, []byte("tx-43"), list.getTxsReversed()[0].TxHash)
+		require.Equal(t, []byte("tx-43++"), list.getTxsReversed()[1].TxHash)
+		require.Equal(t, []byte("tx-42"), list.getTxsReversed()[2].TxHash)
+		require.Equal(t, []byte("tx-42++"), list.getTxsReversed()[3].TxHash)
+	})
+
+	t.Run("with lower nonces", func(t *testing.T) {
+		list := newUnconstrainedListToTest()
+		list.notifyAccountNonce(43)
+
+		list.AddTx(createTx([]byte("tx-42"), ".", 42))
+		list.AddTx(createTx([]byte("tx-43"), ".", 43))
+
+		require.Len(t, list.getTxs(), 2)
+		require.Len(t, list.getTxsReversed(), 2)
+		require.Len(t, list.getSequentialTxs(), 1)
+		require.Equal(t, []byte("tx-43"), list.getSequentialTxs()[0].TxHash)
+
+		list.forgetAccountNonce()
+
+		require.Len(t, list.getTxs(), 2)
+		require.Len(t, list.getTxsReversed(), 2)
+		require.Len(t, list.getSequentialTxs(), 2)
+	})
 }
 
 func TestListForSender_DetectRaceConditions(t *testing.T) {