Skip to content

Commit a965fdd

Browse files
kashitakaallformless
authored andcommitted
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 dbdaca5 commit a965fdd

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,
@@ -92,18 +88,15 @@ func newBlobTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error)
9288
return types.SignTx(tx, types.LatestSignerForChainID(chainid), key)
9389
}
9490

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

9894
// create a signed transaction to send
9995
head, _ := client.HeaderByNumber(context.Background(), nil) // Should be child's, good enough
10096
gasPrice := new(big.Int).Add(head.BaseFee, big.NewInt(params.GWei))
10197
addr := crypto.PubkeyToAddress(key.PublicKey)
10298
chainid, _ := client.ChainID(context.Background())
103-
nonce, err := client.PendingNonceAt(context.Background(), addr)
104-
if err != nil {
105-
return nil, err
106-
}
99+
107100
tx := types.NewTx(&types.DynamicFeeTx{
108101
ChainID: chainid,
109102
Nonce: nonce,
@@ -165,7 +158,7 @@ func TestSendTransaction(t *testing.T) {
165158
client := sim.Client()
166159
ctx := context.Background()
167160

168-
signedTx, err := newTx(sim, testKey)
161+
signedTx, err := newTx(sim, testKey, 0)
169162
if err != nil {
170163
t.Errorf("could not create transaction: %v", err)
171164
}
@@ -256,7 +249,7 @@ func TestForkResendTx(t *testing.T) {
256249
parent, _ := client.HeaderByNumber(ctx, nil)
257250

258251
// 2.
259-
tx, err := newTx(sim, testKey)
252+
tx, err := newTx(sim, testKey, 0)
260253
if err != nil {
261254
t.Fatalf("could not create transaction: %v", err)
262255
}
@@ -301,7 +294,7 @@ func TestCommitReturnValue(t *testing.T) {
301294
}
302295

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

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)