ethclient/simulated: Fix flaky rollback test

[kashitaka]

Fixes: #32252

This PR addresses a flakiness in the rollback test.

I found nonce collision caused transactions occasionally fail to send. I tried to change error message in the failed test like:

	if err = client.SendTransaction(ctx, signedTx); err != nil {
		t.Fatalf("failed to send transaction: %v, nonce: %d", err, signedTx.Nonce())
	}

and I occasionally got test failure with this message:

=== CONT  TestFlakyFunction/Run_#100
    rollback_test.go:44: failed to send transaction: already known, nonce: 0
--- FAIL: TestFlakyFunction/Run_#100 (0.07s)

Although nonces are obtained via PendingNonceAt, we observed that, in rare cases (approximately 1 in 1000), two transactions from the same sender end up with the same nonce. This likely happens because tx0 has not yet propagated to the transaction pool before tx1 requests its nonce. When the test succeeds, tx0 and tx1 have nonces 0 and 1, respectively. However, in rare failures, both transactions end up with nonce 0.

Fix

We modified the test to explicitly assign nonces to each transaction. By controlling the nonce values manually, we eliminated the race condition and ensured consistent behavior. After several thousand runs, the flakiness was no longer reproducible in my local environment.

Additional

Reduced internal polling interval in pendingStateHasTx() to speed up test execution without impacting stability. It reduces test time for TestTransactionRollbackBehavior from about 7 seconds to 2 seconds.

[rjl493456442]

Can you modify the testSendSignedTx, providing the nonce manually instead of fetching from the pendingNonce?

So that you can get rid of the time.Sleep between the tx submission.

[kashitaka]

@rjl493456442 Thank you for your feedback. I applied it in 74caa26

Can you modify the testSendSignedTx, providing the nonce manually instead of fetching from the pendingNonce?
So that you can get rid of the time.Sleep between the tx submission.

Agreed, that sounds like a more stable approach. I initially assumed the test specifically wanted to fetch the nonce via PendingNonceAt, so I didn’t modify that part. But if it’s okay to change, I agree it’s better this way.

Since we can’t modify the nonce on an already constructed tx, I didn’t change testSendSignedTx itself — instead, I updated newTx and newBlobTx to accept and use a manually provided nonce. Could you take a look and let me know if this approach looks legit?

[rjl493456442]

Any particular reason for changing the wait time duration?

[rjl493456442]

Our CI system is occasionally overloaded, have a loose duration is usually preferred

[kashitaka]

@rjl493456442 Thanks for the comment! As mentioned in the PR description, It reduces test time for TestTransactionRollbackBehavior from about 7 seconds to 2 seconds.

I shortened the wait time because I ran the test thousands times to reproduce the flakiness, and here was the bottle neck. The test remained stable with the shorter duration, so I kept the change for some improvement.

[rjl493456442]

I don't get it. This loop is for checking the availability of the receipt. Once it's found, the loop will be terminated.

If the CI can be finished within 2s, then setting it with 7s has no side effect.

[kashitaka]

I’m not sure if this behavior is intentional or unexpected. In this line:

 	if !pendingStateHasTx(client, btx2) || !pendingStateHasTx(client, tx2) || !pendingStateHasTx(client, tx3) {

the polling loop exits almost immediately, since the receipt is available and TransactionReceipt() succeeds — just as you explained. However, in this part:

	if pendingStateHasTx(client, btx0) || pendingStateHasTx(client, tx0) || pendingStateHasTx(client, tx1) {

each call to pendingStateHasTx takes the full 2 seconds and takes 6 seconds in total, because:

receipt, err = client.TransactionReceipt(ctx, tx.Hash())

returns an error like "transaction indexing is in progress" and doesn't drop into break in the 2 sec polling, which seems to happen when querying uncommitted transactions. And that’s why I shortened the polling timeout.

[kashitaka]

@rjl493456442 If you prefer a more relaxed polling interval, please let me know—I’m happy to revert that change.

[rjl493456442]

Thanks for your solid work!