From eb7aef45a73b3151b25a1174d6cddcd0cf2fee0c Mon Sep 17 00:00:00 2001 From: kashitaka Date: Mon, 28 Jul 2025 23:17:36 +0900 Subject: [PATCH] ethclient/simulated: Fix flaky rollback test (#32280) This PR addresses a flakiness in the rollback test discussed in https://github.com/ethereum/go-ethereum/issues/32252 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`. 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. 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. --- ethclient/simulated/backend_test.go | 21 +++++++-------------- ethclient/simulated/rollback_test.go | 22 +++++++++++----------- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go index 7a399d41f3..ee20cd171a 100644 --- a/ethclient/simulated/backend_test.go +++ b/ethclient/simulated/backend_test.go @@ -52,7 +52,7 @@ func simTestBackend(testAddr common.Address) *Backend { ) } -func newBlobTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error) { +func newBlobTx(sim *Backend, key *ecdsa.PrivateKey, nonce uint64) (*types.Transaction, error) { client := sim.Client() testBlob := &kzg4844.Blob{0x00} @@ -67,12 +67,8 @@ func newBlobTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error) addr := crypto.PubkeyToAddress(key.PublicKey) chainid, _ := client.ChainID(context.Background()) - nonce, err := client.PendingNonceAt(context.Background(), addr) - if err != nil { - return nil, err - } - chainidU256, _ := uint256.FromBig(chainid) + tx := types.NewTx(&types.BlobTx{ ChainID: chainidU256, GasTipCap: gasTipCapU256, @@ -88,7 +84,7 @@ func newBlobTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error) return types.SignTx(tx, types.LatestSignerForChainID(chainid), key) } -func newTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error) { +func newTx(sim *Backend, key *ecdsa.PrivateKey, nonce uint64) (*types.Transaction, error) { client := sim.Client() // create a signed transaction to send @@ -96,10 +92,7 @@ func newTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error) { gasPrice := new(big.Int).Add(head.BaseFee, big.NewInt(params.GWei)) addr := crypto.PubkeyToAddress(key.PublicKey) chainid, _ := client.ChainID(context.Background()) - nonce, err := client.PendingNonceAt(context.Background(), addr) - if err != nil { - return nil, err - } + tx := types.NewTx(&types.DynamicFeeTx{ ChainID: chainid, Nonce: nonce, @@ -161,7 +154,7 @@ func TestSendTransaction(t *testing.T) { client := sim.Client() ctx := context.Background() - signedTx, err := newTx(sim, testKey) + signedTx, err := newTx(sim, testKey, 0) if err != nil { t.Errorf("could not create transaction: %v", err) } @@ -252,7 +245,7 @@ func TestForkResendTx(t *testing.T) { parent, _ := client.HeaderByNumber(ctx, nil) // 2. - tx, err := newTx(sim, testKey) + tx, err := newTx(sim, testKey, 0) if err != nil { t.Fatalf("could not create transaction: %v", err) } @@ -297,7 +290,7 @@ func TestCommitReturnValue(t *testing.T) { } // Create a block in the original chain (containing a transaction to force different block hashes) - tx, _ := newTx(sim, testKey) + tx, _ := newTx(sim, testKey, 0) if err := client.SendTransaction(ctx, tx); err != nil { t.Errorf("sending transaction: %v", err) } diff --git a/ethclient/simulated/rollback_test.go b/ethclient/simulated/rollback_test.go index 57c59496d5..093467d291 100644 --- a/ethclient/simulated/rollback_test.go +++ b/ethclient/simulated/rollback_test.go @@ -38,9 +38,9 @@ func TestTransactionRollbackBehavior(t *testing.T) { defer sim.Close() client := sim.Client() - btx0 := testSendSignedTx(t, testKey, sim, true) - tx0 := testSendSignedTx(t, testKey2, sim, false) - tx1 := testSendSignedTx(t, testKey2, sim, false) + btx0 := testSendSignedTx(t, testKey, sim, true, 0) + tx0 := testSendSignedTx(t, testKey2, sim, false, 0) + tx1 := testSendSignedTx(t, testKey2, sim, false, 1) sim.Rollback() @@ -48,9 +48,9 @@ func TestTransactionRollbackBehavior(t *testing.T) { t.Fatalf("all transactions were not rolled back") } - btx2 := testSendSignedTx(t, testKey, sim, true) - tx2 := testSendSignedTx(t, testKey2, sim, false) - tx3 := testSendSignedTx(t, testKey2, sim, false) + btx2 := testSendSignedTx(t, testKey, sim, true, 0) + tx2 := testSendSignedTx(t, testKey2, sim, false, 0) + tx3 := testSendSignedTx(t, testKey2, sim, false, 1) sim.Commit() @@ -61,7 +61,7 @@ func TestTransactionRollbackBehavior(t *testing.T) { // testSendSignedTx sends a signed transaction to the simulated backend. // It does not commit the block. -func testSendSignedTx(t *testing.T, key *ecdsa.PrivateKey, sim *Backend, isBlobTx bool) *types.Transaction { +func testSendSignedTx(t *testing.T, key *ecdsa.PrivateKey, sim *Backend, isBlobTx bool, nonce uint64) *types.Transaction { t.Helper() client := sim.Client() ctx := context.Background() @@ -71,9 +71,9 @@ func testSendSignedTx(t *testing.T, key *ecdsa.PrivateKey, sim *Backend, isBlobT signedTx *types.Transaction ) if isBlobTx { - signedTx, err = newBlobTx(sim, key) + signedTx, err = newBlobTx(sim, key, nonce) } else { - signedTx, err = newTx(sim, key) + signedTx, err = newTx(sim, key, nonce) } if err != nil { t.Fatalf("failed to create transaction: %v", err) @@ -96,13 +96,13 @@ func pendingStateHasTx(client Client, tx *types.Transaction) bool { ) // Poll for receipt with timeout - deadline := time.Now().Add(2 * time.Second) + deadline := time.Now().Add(200 * time.Millisecond) for time.Now().Before(deadline) { receipt, err = client.TransactionReceipt(ctx, tx.Hash()) if err == nil && receipt != nil { break } - time.Sleep(100 * time.Millisecond) + time.Sleep(5 * time.Millisecond) } if err != nil {