Skip to content

Commit eb7aef4

Browse files
author
kashitaka
authored
ethclient/simulated: Fix flaky rollback test (ethereum#32280)
This PR addresses a flakiness in the rollback test discussed in ethereum#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.
1 parent b64a500 commit eb7aef4

File tree

2 files changed

+18
-25
lines changed

2 files changed

+18
-25
lines changed

ethclient/simulated/backend_test.go

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func simTestBackend(testAddr common.Address) *Backend {
5252
)
5353
}
5454

55-
func newBlobTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error) {
55+
func newBlobTx(sim *Backend, key *ecdsa.PrivateKey, nonce uint64) (*types.Transaction, error) {
5656
client := sim.Client()
5757

5858
testBlob := &kzg4844.Blob{0x00}
@@ -67,12 +67,8 @@ func newBlobTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error)
6767

6868
addr := crypto.PubkeyToAddress(key.PublicKey)
6969
chainid, _ := client.ChainID(context.Background())
70-
nonce, err := client.PendingNonceAt(context.Background(), addr)
71-
if err != nil {
72-
return nil, err
73-
}
74-
7570
chainidU256, _ := uint256.FromBig(chainid)
71+
7672
tx := types.NewTx(&types.BlobTx{
7773
ChainID: chainidU256,
7874
GasTipCap: gasTipCapU256,
@@ -88,18 +84,15 @@ func newBlobTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error)
8884
return types.SignTx(tx, types.LatestSignerForChainID(chainid), key)
8985
}
9086

91-
func newTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error) {
87+
func newTx(sim *Backend, key *ecdsa.PrivateKey, nonce uint64) (*types.Transaction, error) {
9288
client := sim.Client()
9389

9490
// create a signed transaction to send
9591
head, _ := client.HeaderByNumber(context.Background(), nil) // Should be child's, good enough
9692
gasPrice := new(big.Int).Add(head.BaseFee, big.NewInt(params.GWei))
9793
addr := crypto.PubkeyToAddress(key.PublicKey)
9894
chainid, _ := client.ChainID(context.Background())
99-
nonce, err := client.PendingNonceAt(context.Background(), addr)
100-
if err != nil {
101-
return nil, err
102-
}
95+
10396
tx := types.NewTx(&types.DynamicFeeTx{
10497
ChainID: chainid,
10598
Nonce: nonce,
@@ -161,7 +154,7 @@ func TestSendTransaction(t *testing.T) {
161154
client := sim.Client()
162155
ctx := context.Background()
163156

164-
signedTx, err := newTx(sim, testKey)
157+
signedTx, err := newTx(sim, testKey, 0)
165158
if err != nil {
166159
t.Errorf("could not create transaction: %v", err)
167160
}
@@ -252,7 +245,7 @@ func TestForkResendTx(t *testing.T) {
252245
parent, _ := client.HeaderByNumber(ctx, nil)
253246

254247
// 2.
255-
tx, err := newTx(sim, testKey)
248+
tx, err := newTx(sim, testKey, 0)
256249
if err != nil {
257250
t.Fatalf("could not create transaction: %v", err)
258251
}
@@ -297,7 +290,7 @@ func TestCommitReturnValue(t *testing.T) {
297290
}
298291

299292
// Create a block in the original chain (containing a transaction to force different block hashes)
300-
tx, _ := newTx(sim, testKey)
293+
tx, _ := newTx(sim, testKey, 0)
301294
if err := client.SendTransaction(ctx, tx); err != nil {
302295
t.Errorf("sending transaction: %v", err)
303296
}

ethclient/simulated/rollback_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,19 @@ func TestTransactionRollbackBehavior(t *testing.T) {
3838
defer sim.Close()
3939
client := sim.Client()
4040

41-
btx0 := testSendSignedTx(t, testKey, sim, true)
42-
tx0 := testSendSignedTx(t, testKey2, sim, false)
43-
tx1 := testSendSignedTx(t, testKey2, sim, false)
41+
btx0 := testSendSignedTx(t, testKey, sim, true, 0)
42+
tx0 := testSendSignedTx(t, testKey2, sim, false, 0)
43+
tx1 := testSendSignedTx(t, testKey2, sim, false, 1)
4444

4545
sim.Rollback()
4646

4747
if pendingStateHasTx(client, btx0) || pendingStateHasTx(client, tx0) || pendingStateHasTx(client, tx1) {
4848
t.Fatalf("all transactions were not rolled back")
4949
}
5050

51-
btx2 := testSendSignedTx(t, testKey, sim, true)
52-
tx2 := testSendSignedTx(t, testKey2, sim, false)
53-
tx3 := testSendSignedTx(t, testKey2, sim, false)
51+
btx2 := testSendSignedTx(t, testKey, sim, true, 0)
52+
tx2 := testSendSignedTx(t, testKey2, sim, false, 0)
53+
tx3 := testSendSignedTx(t, testKey2, sim, false, 1)
5454

5555
sim.Commit()
5656

@@ -61,7 +61,7 @@ func TestTransactionRollbackBehavior(t *testing.T) {
6161

6262
// testSendSignedTx sends a signed transaction to the simulated backend.
6363
// It does not commit the block.
64-
func testSendSignedTx(t *testing.T, key *ecdsa.PrivateKey, sim *Backend, isBlobTx bool) *types.Transaction {
64+
func testSendSignedTx(t *testing.T, key *ecdsa.PrivateKey, sim *Backend, isBlobTx bool, nonce uint64) *types.Transaction {
6565
t.Helper()
6666
client := sim.Client()
6767
ctx := context.Background()
@@ -71,9 +71,9 @@ func testSendSignedTx(t *testing.T, key *ecdsa.PrivateKey, sim *Backend, isBlobT
7171
signedTx *types.Transaction
7272
)
7373
if isBlobTx {
74-
signedTx, err = newBlobTx(sim, key)
74+
signedTx, err = newBlobTx(sim, key, nonce)
7575
} else {
76-
signedTx, err = newTx(sim, key)
76+
signedTx, err = newTx(sim, key, nonce)
7777
}
7878
if err != nil {
7979
t.Fatalf("failed to create transaction: %v", err)
@@ -96,13 +96,13 @@ func pendingStateHasTx(client Client, tx *types.Transaction) bool {
9696
)
9797

9898
// Poll for receipt with timeout
99-
deadline := time.Now().Add(2 * time.Second)
99+
deadline := time.Now().Add(200 * time.Millisecond)
100100
for time.Now().Before(deadline) {
101101
receipt, err = client.TransactionReceipt(ctx, tx.Hash())
102102
if err == nil && receipt != nil {
103103
break
104104
}
105-
time.Sleep(100 * time.Millisecond)
105+
time.Sleep(5 * time.Millisecond)
106106
}
107107

108108
if err != nil {

0 commit comments

Comments
 (0)