diff --git a/op-node/rollup/derive/batch_queue.go b/op-node/rollup/derive/batch_queue.go index 158dcee721eccf7ffec4a22a86df61d2e97818ba..789149443abb80569fd87fcc42dae045458802aa 100644 --- a/op-node/rollup/derive/batch_queue.go +++ b/op-node/rollup/derive/batch_queue.go @@ -220,6 +220,11 @@ batchLoop: // i.e. if the sequence window expired, we create empty batches for the current epoch expiryEpoch := epoch.Number + bq.config.SeqWindowSize forceEmptyBatches := (expiryEpoch == bq.origin.Number && outOfData) || expiryEpoch < bq.origin.Number + firstOfEpoch := epoch.Number == l2SafeHead.L1Origin.Number+1 + + bq.log.Trace("Potentially generating an empty batch", + "expiryEpoch", expiryEpoch, "forceEmptyBatches", forceEmptyBatches, "nextTimestamp", nextTimestamp, + "epoch_time", epoch.Time, "len_l1_blocks", len(bq.l1Blocks), "firstOfEpoch", firstOfEpoch) if !forceEmptyBatches { // sequence window did not expire yet, still room to receive batches for the current epoch, @@ -233,8 +238,10 @@ batchLoop: nextEpoch := bq.l1Blocks[1] // Fill with empty L2 blocks of the same epoch until we meet the time of the next L1 origin, - // to preserve that L2 time >= L1 time - if nextTimestamp < nextEpoch.Time { + // to preserve that L2 time >= L1 time. If this is the first block of the epoch, always generate a + // batch to ensure that we at least have one batch per epoch. + if nextTimestamp < nextEpoch.Time || firstOfEpoch { + bq.log.Trace("Generating next batch", "epoch", epoch, "timestamp", nextTimestamp) return &BatchData{ BatchV1{ ParentHash: l2SafeHead.Hash, @@ -248,6 +255,7 @@ batchLoop: // At this point we have auto generated every batch for the current epoch // that we can, so we can advance to the next epoch. + bq.log.Trace("Advancing internal L1 blocks", "next_timestamp", nextTimestamp, "next_epoch_time", nextEpoch.Time) bq.l1Blocks = bq.l1Blocks[1:] return nil, io.EOF } diff --git a/op-node/rollup/derive/batch_queue_test.go b/op-node/rollup/derive/batch_queue_test.go index 8f14beec46a593cd83063462560c9c4ba4b83a89..e967a3cfc45a4b89e7c7e0840c61d1e1b054cefa 100644 --- a/op-node/rollup/derive/batch_queue_test.go +++ b/op-node/rollup/derive/batch_queue_test.go @@ -183,6 +183,99 @@ func TestBatchQueueEager(t *testing.T) { } } +// TestBatchQueueInvalidInternalAdvance asserts that we do not miss an epoch when generating batches. +// This is a regression test for CLI-3378. +func TestBatchQueueInvalidInternalAdvance(t *testing.T) { + log := testlog.Logger(t, log.LvlTrace) + l1 := L1Chain([]uint64{10, 15, 20, 25, 30}) + safeHead := eth.L2BlockRef{ + Hash: mockHash(10, 2), + Number: 0, + ParentHash: common.Hash{}, + Time: 10, + L1Origin: l1[0].ID(), + SequenceNumber: 0, + } + cfg := &rollup.Config{ + Genesis: rollup.Genesis{ + L2Time: 10, + }, + BlockTime: 2, + MaxSequencerDrift: 600, + SeqWindowSize: 2, + } + + batches := []*BatchData{b(12, l1[0]), b(14, l1[0]), b(16, l1[0]), b(18, l1[0]), b(20, l1[0]), b(22, l1[0]), nil} + errors := []error{nil, nil, nil, nil, nil, nil, io.EOF} + + input := &fakeBatchQueueInput{ + batches: batches, + errors: errors, + origin: l1[0], + } + + bq := NewBatchQueue(log, cfg, input) + _ = bq.Reset(context.Background(), l1[0], eth.SystemConfig{}) + + // Load continuous batches for epoch 0 + for i := 0; i < len(batches); i++ { + b, e := bq.NextBatch(context.Background(), safeHead) + require.ErrorIs(t, e, errors[i]) + require.Equal(t, batches[i], b) + + if b != nil { + safeHead.Number += 1 + safeHead.Time += 2 + safeHead.Hash = mockHash(b.Timestamp, 2) + safeHead.L1Origin = b.Epoch() + } + } + + // Advance to origin 1. No forced batches yet. + input.origin = l1[1] + b, e := bq.NextBatch(context.Background(), safeHead) + require.ErrorIs(t, e, io.EOF) + require.Nil(t, b) + + // Advance to origin 2. No forced batches yet because we are still on epoch 0 + // & have batches for epoch 0. + input.origin = l1[2] + b, e = bq.NextBatch(context.Background(), safeHead) + require.ErrorIs(t, e, io.EOF) + require.Nil(t, b) + + // Advance to origin 3. Should generate one empty batch. + input.origin = l1[3] + b, e = bq.NextBatch(context.Background(), safeHead) + require.Nil(t, e) + require.NotNil(t, b) + require.Equal(t, safeHead.Time+2, b.Timestamp) + require.Equal(t, rollup.Epoch(1), b.EpochNum) + safeHead.Number += 1 + safeHead.Time += 2 + safeHead.Hash = mockHash(b.Timestamp, 2) + safeHead.L1Origin = b.Epoch() + b, e = bq.NextBatch(context.Background(), safeHead) + require.ErrorIs(t, e, io.EOF) + require.Nil(t, b) + + // Advance to origin 4. Should generate one empty batch. + input.origin = l1[4] + b, e = bq.NextBatch(context.Background(), safeHead) + require.Nil(t, e) + require.NotNil(t, b) + require.Equal(t, rollup.Epoch(2), b.EpochNum) + require.Equal(t, safeHead.Time+2, b.Timestamp) + safeHead.Number += 1 + safeHead.Time += 2 + safeHead.Hash = mockHash(b.Timestamp, 2) + safeHead.L1Origin = b.Epoch() + b, e = bq.NextBatch(context.Background(), safeHead) + require.ErrorIs(t, e, io.EOF) + require.Nil(t, b) + +} + func TestBatchQueueMissing(t *testing.T) { log := testlog.Logger(t, log.LvlCrit) l1 := L1Chain([]uint64{10, 15, 20, 25}) diff --git a/specs/derivation.md b/specs/derivation.md index 023f1786015cf3e840b32f7bd21b5e4a7159b17e..c8c6cab355953b8e798950be63097b3114485875 100644 --- a/specs/derivation.md +++ b/specs/derivation.md @@ -617,6 +617,10 @@ then an empty batch can be derived with the following properties: - If `next_timestamp < next_epoch.time`: the current L1 origin is repeated, to preserve the L2 time invariant. - `epoch_num = epoch.number` - `epoch_hash = epoch.hash` +- If the batch is the first batch of the epoch, that epoch is used instead of advancing the epoch to ensure that +there is at least one L2 block per epoch. + - `epoch_num = epoch.number` + - `epoch_hash = epoch.hash` - Otherwise, - `epoch_num = next_epoch.number` - `epoch_hash = next_epoch.hash`