Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ethclient/simulated: add timeout to fix flaky test #28821

Closed
wants to merge 1 commit into from

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Jan 16, 2024

We have a flaky test:

--- FAIL: TestForkResendTx (0.02s)
    backend_test.go:234: sending transaction: nonce too low: next nonce 1, tx nonce 0

The reason the test fails is that after rolling back a block (reorging out a transaction), the head-event which should trigger the txpool to reset the internal state is not handled fast enough, and when we try to re-submit the tx, the pool uses the old data, saying the nonce should be 1.

This is a crappy fix, but I don't see how to properly make sure the head-event was processed by subpools, when all we have is a backend and an ethclient.

@lightclient
Copy link
Member

Could you look at the pending nonce? Something like this

diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go
index 0237c1a0d..c27fbf39e 100644
--- a/ethclient/simulated/backend_test.go
+++ b/ethclient/simulated/backend_test.go
@@ -230,9 +230,23 @@ func TestForkResendTx(t *testing.T) {
 
 	// 5.
 	sim.Commit()
+
 	// The new-head-event needs to trickle out to the tx pool(s), so they reset
 	// their internal state.
-	time.Sleep(1 * time.Second)
+	timeout := time.Now().Add(time.Second)
+	for {
+		if time.Now().After(timeout) {
+			t.Fatalf("timeout while waiting for subpools to reorg")
+		}
+		nonce, err := client.PendingNonceAt(ctx, testAddr)
+		if err != nil {
+			t.Fatalf("failed to check nonce: %v", err)
+		}
+		if nonce == tx.Nonce() {
+			break
+		}
+	}
+
 	if err := client.SendTransaction(ctx, tx); err != nil {
 		t.Fatalf("sending transaction: %v", err)
 	}

@karalabe
Copy link
Member

Lemme try to fix this in a different way without the sleep

@holiman
Copy link
Contributor Author

holiman commented Jan 29, 2024

Lemme try to fix this in a different way without the sleep

This does seem fixed now, with #28837. Closing

@holiman holiman closed this Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants