Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sebastianst merged 7 commits into from
Dec 2, 2024
Merged

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

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
962d66c
add additional assertions to test to check for tx ordering
geoknee Nov 27, 2024
35f96c4
enhance test to handle tx confirmation ID ordering
geoknee Nov 27, 2024
f81b53d
increase size of 0th tx to expose race condition
geoknee Nov 27, 2024
6c9b358
change approach since the backend does not order txs by nonce
geoknee Nov 27, 2024
2444ee8
tidy
geoknee Nov 27, 2024
0e76129
clarify
geoknee Nov 27, 2024
0f3851f
Update op-service/txmgr/queue_test.go
geoknee Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 38 additions & 9 deletions op-service/txmgr/queue_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ func TestQueue_Send(t *testing.T) {
calls []queueCall // calls to the queue
txs []testTx // txs to generate from the factory (and potentially error in send)
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",
Expand All @@ -75,7 +79,8 @@ func TestQueue_Send(t *testing.T) {
{},
{},
},
nonces: []uint64{0, 1},
nonces: []uint64{0, 1},
confirmedIds: []uint{0, 1},
},
{
name: "no limit",
Expand All @@ -88,7 +93,8 @@ func TestQueue_Send(t *testing.T) {
{},
{},
},
nonces: []uint64{0, 1},
nonces: []uint64{0, 1},
confirmedIds: []uint{0, 1},
},
{
name: "single threaded",
Expand All @@ -101,7 +107,8 @@ func TestQueue_Send(t *testing.T) {
txs: []testTx{
{},
},
nonces: []uint64{0},
nonces: []uint64{0},
confirmedIds: []uint{0},
},
{
name: "single threaded blocking",
Expand All @@ -117,7 +124,8 @@ 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",
Expand All @@ -137,7 +145,8 @@ 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",
Expand All @@ -152,7 +161,8 @@ func TestQueue_Send(t *testing.T) {
{sendErr: true},
{},
},
nonces: []uint64{0, 1},
nonces: []uint64{0, 1},
confirmedIds: []uint{0},
},
}
for _, test := range testCases {
Expand All @@ -176,9 +186,11 @@ func TestQueue_Send(t *testing.T) {

// track the nonces, and return any expected errors from tx sending
var (
nonces []uint64
nonceMu sync.Mutex
nonces []uint64
nonceForTxId map[uint]uint64 // maps from txid to nonce
nonceMu sync.Mutex
)
nonceForTxId = make(map[uint]uint64)
sendTx := func(ctx context.Context, tx *types.Transaction) error {
index := int(tx.Data()[0])
nonceMu.Lock()
Expand All @@ -191,8 +203,12 @@ func TestQueue_Send(t *testing.T) {
if testTx != nil && testTx.sendErr {
return core.ErrNonceTooLow
}

txHash := tx.Hash()
nonceMu.Lock()
backend.mine(&txHash, tx.GasFeeCap(), nil)
nonceForTxId[uint(index)] = tx.Nonce()
nonceMu.Unlock()
return nil
}
backend.setTxSender(sendTx)
Expand All @@ -209,15 +225,28 @@ func TestQueue_Send(t *testing.T) {
TxData: []byte{byte(i)},
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)
queued := c.call(i, candidate, receiptChs[i], queue)
require.Equal(t, c.queued, queued, msg)
}
// wait for the queue to drain (all txs complete or failed)
_ = 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)
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
for i, c := range test.calls {
if !c.queued {
Expand Down