ethclient/simulated: Fix flaky rollback test (#32280)
Some checks are pending
/ Linux Build (push) Waiting to run
/ Linux Build (arm) (push) Waiting to run
/ Windows Build (push) Waiting to run
/ Docker Image (push) Waiting to run

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.
This commit is contained in:
kashitaka 2025-07-28 23:17:36 +09:00 committed by GitHub
parent b64a500163
commit eb7aef45a7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 18 additions and 25 deletions

View file

@ -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)
}

View file

@ -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 {