Skip to content

Commit 89dd964

Browse files
Address review comments on batch resizing test
The test for ErrMessageTooLarge batch resizing had several issues identified in code review. This commit addresses them to make the test more robust and better verify the intended behavior. Key changes: - Enable fallback (DisableDapFallbackStoreDataOnChain = false) instead of disabling it, so the test proves the batch poster *chooses* not to fall back when receiving ErrMessageTooLarge, rather than being unable to - Replace manual L1 block advancement loops with AdvanceL1 helper - Use WaitForTx to properly verify follower sync instead of just checking BlockNumber (which only confirms the node is responding) - Add follower sync verification to Phase 2 (was missing) - Fail immediately on any non-CustomDA batch type instead of only tracking brotli batches - Allow one oversized batch in Phase 2 to handle the race condition between changing the max size limit and batches already in flight
1 parent 6ef966a commit 89dd964

File tree

1 file changed

+43
-26
lines changed

1 file changed

+43
-26
lines changed

system_tests/multi_writer_fallback_test.go

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,8 +1070,9 @@ func TestBatchResizingWithoutFallback_MessageTooLarge(t *testing.T) {
10701070
// Disable AnyTrust
10711071
builder.nodeConfig.DA.AnyTrust.Enable = false
10721072

1073-
// Disable fallback to on-chain to ensure we're testing resize, not fallback
1074-
builder.nodeConfig.BatchPoster.DisableDapFallbackStoreDataOnChain = true
1073+
// Enable fallback to on-chain - this proves the batch poster *chooses* not to fall back
1074+
// when receiving ErrMessageTooLarge (it resizes instead), rather than being unable to fall back
1075+
builder.nodeConfig.BatchPoster.DisableDapFallbackStoreDataOnChain = false
10751076

10761077
// Configure batch posting to use size-based triggers
10771078
builder.nodeConfig.BatchPoster.MaxDelay = 60 * time.Second
@@ -1107,27 +1108,28 @@ func TestBatchResizingWithoutFallback_MessageTooLarge(t *testing.T) {
11071108
Require(t, err)
11081109

11091110
// Generate enough transactions to create a ~10KB batch
1111+
var phase1LastTxHash common.Hash
11101112
for i := 0; i < 250; i++ {
11111113
tx := builder.L2Info.PrepareTx("Owner", "User2",
11121114
builder.L2Info.TransferGas, big.NewInt(1e12), nil)
11131115
err := builder.L2.Client.SendTransaction(ctx, tx)
11141116
Require(t, err)
1117+
phase1LastTxHash = tx.Hash()
11151118
}
11161119

11171120
t.Log("Phase 1: Generated 250 transactions")
11181121

11191122
// Create L1 blocks to trigger batch posting
1120-
for i := 0; i < 30; i++ {
1121-
SendWaitTestTransactions(t, ctx, builder.L1.Client, []*types.Transaction{
1122-
builder.L1Info.PrepareTx("Faucet", "User", 30000, big.NewInt(1e12), nil),
1123-
})
1124-
}
1123+
AdvanceL1(t, ctx, builder.L1.Client, builder.L1Info, 30)
11251124

1126-
// Wait for batch to post
1127-
time.Sleep(time.Second * 2)
1125+
// Give batch poster time to post the batch
1126+
time.Sleep(time.Second * 3)
11281127

1129-
// Verify follower synced
1130-
_, err = l2B.Client.BlockNumber(ctx)
1128+
// Create more L1 blocks for follower to read and process
1129+
AdvanceL1(t, ctx, builder.L1.Client, builder.L1Info, 10)
1130+
1131+
// Verify follower synced by waiting for the last transaction
1132+
_, err = WaitForTx(ctx, l2B.Client, phase1LastTxHash, time.Second*60)
11311133
Require(t, err)
11321134

11331135
l1BlockAfterPhase1, err := builder.L1.Client.BlockNumber(ctx)
@@ -1167,6 +1169,8 @@ func TestBatchResizingWithoutFallback_MessageTooLarge(t *testing.T) {
11671169
if payloadSize < 8_000 || payloadSize > 12_000 {
11681170
t.Errorf("Phase 1: CustomDA payload size %d outside expected range 8KB-12KB", payloadSize)
11691171
}
1172+
} else {
1173+
t.Fatalf("Phase 1: Expected CustomDA batch but found unexpected batch type (header=0x%02x)", headerByte)
11701174
}
11711175
}
11721176

@@ -1187,25 +1191,31 @@ func TestBatchResizingWithoutFallback_MessageTooLarge(t *testing.T) {
11871191
Require(t, err)
11881192

11891193
// Generate more transactions to create another large batch
1194+
var phase2LastTxHash common.Hash
11901195
for i := 0; i < 250; i++ {
11911196
tx := builder.L2Info.PrepareTx("Owner", "User2",
11921197
builder.L2Info.TransferGas, big.NewInt(1e12), nil)
11931198
err := builder.L2.Client.SendTransaction(ctx, tx)
11941199
Require(t, err)
1200+
phase2LastTxHash = tx.Hash()
11951201
}
11961202

11971203
t.Log("Phase 2: Generated 250 transactions")
11981204

11991205
// Create L1 blocks to trigger batch posting attempts
12001206
// The batch poster will initially try to build a ~10KB batch (based on previous max size),
12011207
// hit ErrMessageTooLarge, then query GetMaxMessageSize again (getting 5KB), and rebuild smaller batches
1202-
for i := 0; i < 40; i++ {
1203-
SendWaitTestTransactions(t, ctx, builder.L1.Client, []*types.Transaction{
1204-
builder.L1Info.PrepareTx("Faucet", "User", 30000, big.NewInt(1e12), nil),
1205-
})
1206-
}
1208+
AdvanceL1(t, ctx, builder.L1.Client, builder.L1Info, 40)
12071209

1208-
time.Sleep(time.Second * 3)
1210+
// Give batch poster time to post the batches (may need multiple retries due to resize)
1211+
time.Sleep(time.Second * 5)
1212+
1213+
// Create more L1 blocks for follower to read and process
1214+
AdvanceL1(t, ctx, builder.L1.Client, builder.L1Info, 10)
1215+
1216+
// Verify follower synced by waiting for the last transaction
1217+
_, err = WaitForTx(ctx, l2B.Client, phase2LastTxHash, time.Second*60)
1218+
Require(t, err)
12091219

12101220
l1BlockAfterPhase2, err := builder.L1.Client.BlockNumber(ctx)
12111221
Require(t, err)
@@ -1216,7 +1226,7 @@ func TestBatchResizingWithoutFallback_MessageTooLarge(t *testing.T) {
12161226
Require(t, err)
12171227

12181228
phase2CustomDABatches := 0
1219-
phase2CalldataBatches := 0
1229+
phase2OversizedBatches := 0
12201230
var phase2MaxPayloadSize int
12211231
for _, batch := range phase2Batches {
12221232
serializedBatch, err := batch.Serialize(ctx, builder.L1.Client)
@@ -1238,19 +1248,26 @@ func TestBatchResizingWithoutFallback_MessageTooLarge(t *testing.T) {
12381248
phase2MaxPayloadSize = payloadSize
12391249
}
12401250

1241-
// Verify batch is approximately 5KB (3KB-6KB range)
1251+
// Track batches exceeding the new limit
1252+
// The first batch may exceed the limit due to race condition (built before limit change)
12421253
if payloadSize > 6_000 {
1243-
t.Errorf("Phase 2: CustomDA payload size %d exceeds expected max ~5KB", payloadSize)
1254+
phase2OversizedBatches++
1255+
t.Logf("Phase 2: Batch %d exceeds 6KB limit (expected for first batch due to race)", phase2CustomDABatches)
12441256
}
1245-
} else if daprovider.IsBrotliMessageHeaderByte(headerByte) {
1246-
phase2CalldataBatches++
1247-
t.Logf("Phase 2: Found Calldata batch (header=0x%02x) - unexpected!", headerByte)
1257+
} else {
1258+
t.Fatalf("Phase 2: Expected CustomDA batch but found unexpected batch type (header=0x%02x)", headerByte)
12481259
}
12491260
}
12501261

1251-
// Verify no fallback to calldata occurred
1252-
if phase2CalldataBatches > 0 {
1253-
t.Fatalf("Phase 2: Expected no fallback to calldata, but found %d calldata batches", phase2CalldataBatches)
1262+
// Allow at most 1 oversized batch due to a race condition between changing the max size
1263+
// and the batch poster's existing work. The batch poster doesn't constantly re-query
1264+
// GetMaxMessageSize - it queries once, builds batches up to that limit, and only
1265+
// re-queries after receiving ErrMessageTooLarge. So when we change the limit from 10KB
1266+
// to 5KB, there may be a batch already built with the old limit. That batch gets posted,
1267+
// triggers ErrMessageTooLarge, and then the batch poster queries the new limit and
1268+
// rebuilds subsequent batches to fit within 5KB.
1269+
if phase2OversizedBatches > 1 {
1270+
t.Errorf("Phase 2: Expected at most 1 oversized batch (race condition), but found %d", phase2OversizedBatches)
12541271
}
12551272

12561273
if phase2CustomDABatches == 0 {

0 commit comments

Comments
 (0)