Repository: ethereum/go-ethereum (language: go)

Issue:
Flaky test
Looks like we have a flaky test:

```
--- FAIL: TestSimulatedBeaconSendWithdrawals (12.08s)
    simulated_beacon_test.go:139: timed out without including all withdrawals/txs
FAIL
FAIL	github.com/ethereum/go-ethereum/eth/catalyst	14.812s
ok  	github.com/ethereum/go-ethereum/eth/downloader	41.536s
```

https://ci.appveyor.com/project/ethereum/go-ethereum/builds/51503642/job/4ff083bblgggkpm8


Discussion / hints:
To fix this we can change the simulated beacon period (from 12 to maybe 20 seconds) as I think the blocks are not produced/processed in set time. Locally it worked fine (I can make this change if this seems reasonable).
@MariusVanDerWijden 
@MariusVanDerWijden 
To fix this, I made two small changes:
Faster block generation: I reduced the block creation time from 1 second to 0.5 seconds, so blocks are produced more quickly.
Longer timeout: I increased the test timeout from 12 seconds to 30 seconds, giving it more time to finish.
With these changes, the test now completes reliably within the expected time. I’ve tested it locally several times, and it works consistently.
Thanks for taking the time to review this! If there’s anything else I should adjust, just let me know. 😊
May I work on this if its available?

Fix patch:
diff --git a/eth/fetcher/tx_fetcher.go b/eth/fetcher/tx_fetcher.go
index 97d1e2986212..1c192d411221 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
 	}


Test patch:
diff --git a/eth/fetcher/tx_fetcher_test.go b/eth/fetcher/tx_fetcher_test.go
index 52b35910862d..7f3080f5f6f0 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")
+	}
+	if !fetcher.isKnownUnderpriced(tx2.Hash()) {
+		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")
 	}
-	// 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 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 51f2fc3b4d97..c136253a62be 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()