Commit 6e799a6c authored by George Knee's avatar George Knee Committed by GitHub

txmgr/Queue: add additional assertions to test to check for tx ordering (#13124)

* add additional assertions to test to check for tx ordering

* enhance test to handle tx confirmation ID ordering

* increase size of 0th tx to expose race condition

* change approach since the backend does not order txs by nonce

* tidy

* clarify

* Update op-service/txmgr/queue_test.go
Co-authored-by: default avatarSebastian Stammler <seb@oplabs.co>

---------
Co-authored-by: default avatarSebastian Stammler <seb@oplabs.co>
parent cdae7f2b
...@@ -63,6 +63,10 @@ func TestQueue_Send(t *testing.T) { ...@@ -63,6 +63,10 @@ func TestQueue_Send(t *testing.T) {
calls []queueCall // calls to the queue calls []queueCall // calls to the queue
txs []testTx // txs to generate from the factory (and potentially error in send) txs []testTx // txs to generate from the factory (and potentially error in send)
nonces []uint64 // expected sent tx nonces after all calls are made nonces []uint64 // expected sent tx nonces after all calls are made
// With Holocene, it is important that transactions are included on chain in the same order as they are sent.
// The txmgr.Queue.Send() method should ensure nonces are determined _synchronously_ even if transactions
// are otherwise launched asynchronously.
confirmedIds []uint // expected tx Ids after all calls are made
}{ }{
{ {
name: "success", name: "success",
...@@ -76,6 +80,7 @@ func TestQueue_Send(t *testing.T) { ...@@ -76,6 +80,7 @@ func TestQueue_Send(t *testing.T) {
{}, {},
}, },
nonces: []uint64{0, 1}, nonces: []uint64{0, 1},
confirmedIds: []uint{0, 1},
}, },
{ {
name: "no limit", name: "no limit",
...@@ -89,6 +94,7 @@ func TestQueue_Send(t *testing.T) { ...@@ -89,6 +94,7 @@ func TestQueue_Send(t *testing.T) {
{}, {},
}, },
nonces: []uint64{0, 1}, nonces: []uint64{0, 1},
confirmedIds: []uint{0, 1},
}, },
{ {
name: "single threaded", name: "single threaded",
...@@ -102,6 +108,7 @@ func TestQueue_Send(t *testing.T) { ...@@ -102,6 +108,7 @@ func TestQueue_Send(t *testing.T) {
{}, {},
}, },
nonces: []uint64{0}, nonces: []uint64{0},
confirmedIds: []uint{0},
}, },
{ {
name: "single threaded blocking", name: "single threaded blocking",
...@@ -118,6 +125,7 @@ func TestQueue_Send(t *testing.T) { ...@@ -118,6 +125,7 @@ func TestQueue_Send(t *testing.T) {
{}, {},
}, },
nonces: []uint64{0, 1, 2}, nonces: []uint64{0, 1, 2},
confirmedIds: []uint{0, 2, 3},
}, },
{ {
name: "dual threaded blocking", name: "dual threaded blocking",
...@@ -138,6 +146,7 @@ func TestQueue_Send(t *testing.T) { ...@@ -138,6 +146,7 @@ func TestQueue_Send(t *testing.T) {
{}, {},
}, },
nonces: []uint64{0, 1, 2, 3, 4}, nonces: []uint64{0, 1, 2, 3, 4},
confirmedIds: []uint{0, 1, 3, 4, 5},
}, },
{ {
name: "subsequent txs fail after tx failure", name: "subsequent txs fail after tx failure",
...@@ -153,6 +162,7 @@ func TestQueue_Send(t *testing.T) { ...@@ -153,6 +162,7 @@ func TestQueue_Send(t *testing.T) {
{}, {},
}, },
nonces: []uint64{0, 1}, nonces: []uint64{0, 1},
confirmedIds: []uint{0},
}, },
} }
for _, test := range testCases { for _, test := range testCases {
...@@ -177,8 +187,10 @@ func TestQueue_Send(t *testing.T) { ...@@ -177,8 +187,10 @@ func TestQueue_Send(t *testing.T) {
// track the nonces, and return any expected errors from tx sending // track the nonces, and return any expected errors from tx sending
var ( var (
nonces []uint64 nonces []uint64
nonceForTxId map[uint]uint64 // maps from txid to nonce
nonceMu sync.Mutex nonceMu sync.Mutex
) )
nonceForTxId = make(map[uint]uint64)
sendTx := func(ctx context.Context, tx *types.Transaction) error { sendTx := func(ctx context.Context, tx *types.Transaction) error {
index := int(tx.Data()[0]) index := int(tx.Data()[0])
nonceMu.Lock() nonceMu.Lock()
...@@ -191,8 +203,12 @@ func TestQueue_Send(t *testing.T) { ...@@ -191,8 +203,12 @@ func TestQueue_Send(t *testing.T) {
if testTx != nil && testTx.sendErr { if testTx != nil && testTx.sendErr {
return core.ErrNonceTooLow return core.ErrNonceTooLow
} }
txHash := tx.Hash() txHash := tx.Hash()
nonceMu.Lock()
backend.mine(&txHash, tx.GasFeeCap(), nil) backend.mine(&txHash, tx.GasFeeCap(), nil)
nonceForTxId[uint(index)] = tx.Nonce()
nonceMu.Unlock()
return nil return nil
} }
backend.setTxSender(sendTx) backend.setTxSender(sendTx)
...@@ -209,15 +225,28 @@ func TestQueue_Send(t *testing.T) { ...@@ -209,15 +225,28 @@ func TestQueue_Send(t *testing.T) {
TxData: []byte{byte(i)}, TxData: []byte{byte(i)},
To: &common.Address{}, To: &common.Address{},
} }
if i == 0 {
// Make the first tx much larger to expose
// any race conditions in the queue
candidate.TxData = make([]byte, 100_000)
}
receiptChs[i] = make(chan TxReceipt[int], 1) receiptChs[i] = make(chan TxReceipt[int], 1)
queued := c.call(i, candidate, receiptChs[i], queue) queued := c.call(i, candidate, receiptChs[i], queue)
require.Equal(t, c.queued, queued, msg) require.Equal(t, c.queued, queued, msg)
} }
// wait for the queue to drain (all txs complete or failed) // wait for the queue to drain (all txs complete or failed)
_ = queue.Wait() _ = queue.Wait()
// check that the nonces match
// NOTE the backend in this test does not order transactions based on the nonce
// So what we want to check is that the txs match expectations when they are ordered
// in the same way as the nonces.
slices.Sort(nonces) slices.Sort(nonces)
require.Equal(t, test.nonces, nonces, "expected nonces do not match") require.Equal(t, test.nonces, nonces, "expected nonces do not match")
for i, id := range test.confirmedIds {
require.Equal(t, nonces[i], nonceForTxId[id],
"nonce for tx id %d was %d instead of %d", id, nonceForTxId[id], nonces[i])
}
// check receipts // check receipts
for i, c := range test.calls { for i, c := range test.calls {
if !c.queued { if !c.queued {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment