diff --git a/eth/fetcher/tx_fetcher.go b/eth/fetcher/tx_fetcher.go index 97d1e29862..1c192d4112 100644 --- a/eth/fetcher/tx_fetcher.go +++ b/eth/fetcher/tx_fetcher.go @@ -202,22 +202,23 @@ type TxFetcher struct { fetchTxs func(string, []common.Hash) error // Retrieves a set of txs from a remote peer dropPeer func(string) // Drops a peer in case of announcement violation - step chan struct{} // Notification channel when the fetcher loop iterates - clock mclock.Clock // Time wrapper to simulate in tests - rand *mrand.Rand // Randomizer to use in tests instead of map range loops (soft-random) + step chan struct{} // Notification channel when the fetcher loop iterates + clock mclock.Clock // Monotonic clock or simulated clock for tests + realTime func() time.Time // Real system time or simulated time for tests + rand *mrand.Rand // Randomizer to use in tests instead of map range loops (soft-random) } // NewTxFetcher creates a transaction fetcher to retrieve transaction // based on hash announcements. func NewTxFetcher(hasTx func(common.Hash) bool, addTxs func([]*types.Transaction) []error, fetchTxs func(string, []common.Hash) error, dropPeer func(string)) *TxFetcher { - return NewTxFetcherForTests(hasTx, addTxs, fetchTxs, dropPeer, mclock.System{}, nil) + return NewTxFetcherForTests(hasTx, addTxs, fetchTxs, dropPeer, mclock.System{}, time.Now, nil) } // NewTxFetcherForTests is a testing method to mock out the realtime clock with // a simulated version and the internal randomness with a deterministic one. func NewTxFetcherForTests( hasTx func(common.Hash) bool, addTxs func([]*types.Transaction) []error, fetchTxs func(string, []common.Hash) error, dropPeer func(string), - clock mclock.Clock, rand *mrand.Rand) *TxFetcher { + clock mclock.Clock, realTime func() time.Time, rand *mrand.Rand) *TxFetcher { return &TxFetcher{ notify: make(chan *txAnnounce), cleanup: make(chan *txDelivery), @@ -237,6 +238,7 @@ func NewTxFetcherForTests( fetchTxs: fetchTxs, dropPeer: dropPeer, clock: clock, + realTime: realTime, rand: rand, } } @@ -293,7 +295,7 @@ func (f *TxFetcher) Notify(peer string, types []byte, sizes []uint32, hashes []c // isKnownUnderpriced reports whether a transaction hash was recently found to be underpriced. func (f *TxFetcher) isKnownUnderpriced(hash common.Hash) bool { prevTime, ok := f.underpriced.Peek(hash) - if ok && prevTime.Before(time.Now().Add(-maxTxUnderpricedTimeout)) { + if ok && prevTime.Before(f.realTime().Add(-maxTxUnderpricedTimeout)) { f.underpriced.Remove(hash) return false } diff --git a/eth/fetcher/tx_fetcher_test.go b/eth/fetcher/tx_fetcher_test.go index 52b3591086..7f3080f5f6 100644 --- a/eth/fetcher/tx_fetcher_test.go +++ b/eth/fetcher/tx_fetcher_test.go @@ -2150,9 +2150,22 @@ func containsHashInAnnounces(slice []announce, hash common.Hash) bool { return false } -// Tests that a transaction is forgotten after the timeout. +// TestTransactionForgotten verifies that underpriced transactions are properly +// forgotten after the timeout period, testing both the exact timeout boundary +// and the cleanup of the underpriced cache. func TestTransactionForgotten(t *testing.T) { - fetcher := NewTxFetcher( + // Test ensures that underpriced transactions are properly forgotten after a timeout period, + // including checks for timeout boundary and cache cleanup. + t.Parallel() + + // Create a mock clock for deterministic time control + mockClock := new(mclock.Simulated) + mockTime := func() time.Time { + nanoTime := int64(mockClock.Now()) + return time.Unix(nanoTime/1000000000, nanoTime%1000000000) + } + + fetcher := NewTxFetcherForTests( func(common.Hash) bool { return false }, func(txs []*types.Transaction) []error { errs := make([]error, len(txs)) @@ -2163,24 +2176,83 @@ func TestTransactionForgotten(t *testing.T) { }, func(string, []common.Hash) error { return nil }, func(string) {}, + mockClock, + mockTime, + rand.New(rand.NewSource(0)), // Use fixed seed for deterministic behavior ) fetcher.Start() defer fetcher.Stop() - // Create one TX which is 5 minutes old, and one which is recent - tx1 := types.NewTx(&types.LegacyTx{Nonce: 0}) - tx1.SetTime(time.Now().Add(-maxTxUnderpricedTimeout - 1*time.Second)) - tx2 := types.NewTx(&types.LegacyTx{Nonce: 1}) - // Enqueue both in the fetcher. They will be immediately tagged as underpriced - if err := fetcher.Enqueue("asdf", []*types.Transaction{tx1, tx2}, false); err != nil { + // Create two test transactions with the same timestamp + tx1 := types.NewTransaction(0, common.Address{}, big.NewInt(100), 21000, big.NewInt(1), nil) + tx2 := types.NewTransaction(1, common.Address{}, big.NewInt(100), 21000, big.NewInt(1), nil) + + now := mockTime() + tx1.SetTime(now) + tx2.SetTime(now) + + // Initial state: both transactions should be marked as underpriced + if err := fetcher.Enqueue("peer", []*types.Transaction{tx1, tx2}, false); err != nil { t.Fatal(err) } - // isKnownUnderpriced should trigger removal of the first tx (no longer be known underpriced) - if fetcher.isKnownUnderpriced(tx1.Hash()) { - t.Fatal("transaction should be forgotten by now") + if !fetcher.isKnownUnderpriced(tx1.Hash()) { + t.Error("tx1 should be underpriced") } - // isKnownUnderpriced should not trigger removal of the second if !fetcher.isKnownUnderpriced(tx2.Hash()) { - t.Fatal("transaction should be known underpriced") + t.Error("tx2 should be underpriced") + } + + // Verify cache size + if size := fetcher.underpriced.Len(); size != 2 { + t.Errorf("wrong underpriced cache size: got %d, want %d", size, 2) + } + + // Just before timeout: transactions should still be underpriced + mockClock.Run(maxTxUnderpricedTimeout - time.Second) + if !fetcher.isKnownUnderpriced(tx1.Hash()) { + t.Error("tx1 should still be underpriced before timeout") + } + if !fetcher.isKnownUnderpriced(tx2.Hash()) { + t.Error("tx2 should still be underpriced before timeout") + } + + // Exactly at timeout boundary: transactions should still be present + mockClock.Run(time.Second) + if !fetcher.isKnownUnderpriced(tx1.Hash()) { + t.Error("tx1 should be present exactly at timeout") + } + if !fetcher.isKnownUnderpriced(tx2.Hash()) { + t.Error("tx2 should be present exactly at timeout") + } + + // After timeout: transactions should be forgotten + mockClock.Run(time.Second) + if fetcher.isKnownUnderpriced(tx1.Hash()) { + t.Error("tx1 should be forgotten after timeout") + } + if fetcher.isKnownUnderpriced(tx2.Hash()) { + t.Error("tx2 should be forgotten after timeout") + } + + // Verify cache is empty + if size := fetcher.underpriced.Len(); size != 0 { + t.Errorf("wrong underpriced cache size after timeout: got %d, want 0", size) + } + + // Re-enqueue tx1 with updated timestamp + tx1.SetTime(mockTime()) + if err := fetcher.Enqueue("peer", []*types.Transaction{tx1}, false); err != nil { + t.Fatal(err) + } + if !fetcher.isKnownUnderpriced(tx1.Hash()) { + t.Error("tx1 should be underpriced after re-enqueueing with new timestamp") + } + if fetcher.isKnownUnderpriced(tx2.Hash()) { + t.Error("tx2 should remain forgotten") + } + + // Verify final cache state + if size := fetcher.underpriced.Len(); size != 1 { + t.Errorf("wrong final underpriced cache size: got %d, want 1", size) } } diff --git a/tests/fuzzers/txfetcher/txfetcher_fuzzer.go b/tests/fuzzers/txfetcher/txfetcher_fuzzer.go index 51f2fc3b4d..c136253a62 100644 --- a/tests/fuzzers/txfetcher/txfetcher_fuzzer.go +++ b/tests/fuzzers/txfetcher/txfetcher_fuzzer.go @@ -84,7 +84,12 @@ func fuzz(input []byte) int { }, func(string, []common.Hash) error { return nil }, nil, - clock, rand, + clock, + func() time.Time { + nanoTime := int64(clock.Now()) + return time.Unix(nanoTime/1000000000, nanoTime%1000000000) + }, + rand, ) f.Start() defer f.Stop()