Skip to content

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