Commit 1a71e894 authored by Joshua Gutow's avatar Joshua Gutow

op-node: Fix invalid advance

parent 525e1721
...@@ -220,10 +220,11 @@ batchLoop: ...@@ -220,10 +220,11 @@ batchLoop:
// i.e. if the sequence window expired, we create empty batches for the current epoch // i.e. if the sequence window expired, we create empty batches for the current epoch
expiryEpoch := epoch.Number + bq.config.SeqWindowSize expiryEpoch := epoch.Number + bq.config.SeqWindowSize
forceEmptyBatches := (expiryEpoch == bq.origin.Number && outOfData) || expiryEpoch < bq.origin.Number 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", bq.log.Trace("Potentially generating an empty batch",
"expiryEpoch", expiryEpoch, "forceEmptyBatches", forceEmptyBatches, "nextTimestamp", nextTimestamp, "expiryEpoch", expiryEpoch, "forceEmptyBatches", forceEmptyBatches, "nextTimestamp", nextTimestamp,
"epoch_time", epoch.Time, "len_l1_blocks", len(bq.l1Blocks)) "epoch_time", epoch.Time, "len_l1_blocks", len(bq.l1Blocks), "firstOfEpoch", firstOfEpoch)
if !forceEmptyBatches { if !forceEmptyBatches {
// sequence window did not expire yet, still room to receive batches for the current epoch, // sequence window did not expire yet, still room to receive batches for the current epoch,
...@@ -237,8 +238,9 @@ batchLoop: ...@@ -237,8 +238,9 @@ batchLoop:
nextEpoch := bq.l1Blocks[1] nextEpoch := bq.l1Blocks[1]
// Fill with empty L2 blocks of the same epoch until we meet the time of the next L1 origin, // 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 // to preserve that L2 time >= L1 time. If this is the first block of the epoch, always generate a
if nextTimestamp < nextEpoch.Time { // 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) bq.log.Trace("Generating next batch", "epoch", epoch, "timestamp", nextTimestamp)
return &BatchData{ return &BatchData{
BatchV1{ BatchV1{
......
...@@ -183,7 +183,8 @@ func TestBatchQueueEager(t *testing.T) { ...@@ -183,7 +183,8 @@ func TestBatchQueueEager(t *testing.T) {
} }
} }
// Tests CLI-3378 // 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) { func TestBatchQueueInvalidInternalAdvance(t *testing.T) {
log := testlog.Logger(t, log.LvlTrace) log := testlog.Logger(t, log.LvlTrace)
l1 := L1Chain([]uint64{10, 15, 20, 25, 30}) l1 := L1Chain([]uint64{10, 15, 20, 25, 30})
...@@ -245,28 +246,30 @@ func TestBatchQueueInvalidInternalAdvance(t *testing.T) { ...@@ -245,28 +246,30 @@ func TestBatchQueueInvalidInternalAdvance(t *testing.T) {
// Advance to origin 3. Should generate one empty batch. // Advance to origin 3. Should generate one empty batch.
input.origin = l1[3] input.origin = l1[3]
// b, e = bq.NextBatch(context.Background(), safeHead) b, e = bq.NextBatch(context.Background(), safeHead)
// require.Nil(t, e) require.Nil(t, e)
// require.NotNil(t, b) require.NotNil(t, b)
// require.Equal(t, safeHead.Time+2, b.Timestamp) require.Equal(t, safeHead.Time+2, b.Timestamp)
// safeHead.Number += 1 require.Equal(t, rollup.Epoch(1), b.EpochNum)
// safeHead.Time += 2 safeHead.Number += 1
// safeHead.Hash = mockHash(b.Timestamp, 2) safeHead.Time += 2
// safeHead.L1Origin = b.Epoch() safeHead.Hash = mockHash(b.Timestamp, 2)
safeHead.L1Origin = b.Epoch()
b, e = bq.NextBatch(context.Background(), safeHead) b, e = bq.NextBatch(context.Background(), safeHead)
require.ErrorIs(t, e, io.EOF) require.ErrorIs(t, e, io.EOF)
require.Nil(t, b) require.Nil(t, b)
// Advance to origin 4. Should generate one empty batch. // Advance to origin 4. Should generate one empty batch.
input.origin = l1[4] input.origin = l1[4]
// b, e = bq.NextBatch(context.Background(), safeHead) b, e = bq.NextBatch(context.Background(), safeHead)
// require.Nil(t, e) require.Nil(t, e)
// require.NotNil(t, b) require.NotNil(t, b)
// require.Equal(t, safeHead.Time+2, b.Timestamp) require.Equal(t, rollup.Epoch(2), b.EpochNum)
// safeHead.Number += 1 require.Equal(t, safeHead.Time+2, b.Timestamp)
// safeHead.Time += 2 safeHead.Number += 1
// safeHead.Hash = mockHash(b.Timestamp, 2) safeHead.Time += 2
// safeHead.L1Origin = b.Epoch() safeHead.Hash = mockHash(b.Timestamp, 2)
safeHead.L1Origin = b.Epoch()
b, e = bq.NextBatch(context.Background(), safeHead) b, e = bq.NextBatch(context.Background(), safeHead)
require.ErrorIs(t, e, io.EOF) require.ErrorIs(t, e, io.EOF)
require.Nil(t, b) require.Nil(t, b)
......
...@@ -617,6 +617,10 @@ then an empty batch can be derived with the following properties: ...@@ -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. - If `next_timestamp < next_epoch.time`: the current L1 origin is repeated, to preserve the L2 time invariant.
- `epoch_num = epoch.number` - `epoch_num = epoch.number`
- `epoch_hash = epoch.hash` - `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, - Otherwise,
- `epoch_num = next_epoch.number` - `epoch_num = next_epoch.number`
- `epoch_hash = next_epoch.hash` - `epoch_hash = next_epoch.hash`
......
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