From 543df5b5ba783977e6704a8e86f9e009909c7531 Mon Sep 17 00:00:00 2001 From: geoknee Date: Mon, 8 Dec 2025 16:40:46 +0000 Subject: [PATCH 01/38] WIP --- op-node/rollup/sequencing/origin_selector.go | 1 + 1 file changed, 1 insertion(+) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index c5f23407d7652..32919fba020bd 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -87,6 +87,7 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc // of slack. For simplicity, we implement our Sequencer to always start building on the latest // L1 block when we can. if nextOrigin != (eth.L1BlockRef{}) && l2Head.Time+los.cfg.BlockTime >= nextOrigin.Time { + los.log.Info("Starting to build on top of the next origin", "next_origin", nextOrigin) return nextOrigin, nil } From ea66bee54c66a35072f71741ad4ce5f211fcf607 Mon Sep 17 00:00:00 2001 From: geoknee Date: Mon, 8 Dec 2025 22:24:56 +0000 Subject: [PATCH 02/38] wip --- justfile | 4 + op-e2e/actions/helpers/l2_batcher.go | 2 +- op-e2e/actions/helpers/l2_sequencer.go | 9 +- op-e2e/actions/helpers/utils.go | 2 +- op-e2e/actions/proofs/helpers/env.go | 4 +- .../proofs/sequence_window_expiry_test.go | 108 +++++++++++++----- op-node/rollup/sequencing/origin_selector.go | 5 + 7 files changed, 97 insertions(+), 37 deletions(-) diff --git a/justfile b/justfile index 14665b5f726a7..68cd88c3d276b 100644 --- a/justfile +++ b/justfile @@ -1,3 +1,4 @@ +@include # Checks that TODO comments have corresponding issues. todo-checker: ./ops/scripts/todo-checker.sh @@ -40,3 +41,6 @@ update-op-geth ref: go mod tidy; \ echo "Updated op-geth to $ver" + +build-contracts: + JUSTFILE=$(pwd)/packages/contracts-bedrock/justfile just build-dev diff --git a/op-e2e/actions/helpers/l2_batcher.go b/op-e2e/actions/helpers/l2_batcher.go index e0d8eac5a284e..2e1a941721d39 100644 --- a/op-e2e/actions/helpers/l2_batcher.go +++ b/op-e2e/actions/helpers/l2_batcher.go @@ -270,7 +270,7 @@ func (s *L2Batcher) Buffer(t Testing, bufferOpts ...BufferOption) error { } } - s.ActCreateChannel(t, s.rollupCfg.IsDelta(block.Time()), options.channelModifiers...) + s.ActCreateChannel(t, s.rollupCfg.IsDelta(block.Time()) && !s.l2BatcherCfg.ForceSubmitSingularBatch, options.channelModifiers...) if _, err := s.L2ChannelOut.AddBlock(s.rollupCfg, block); err != nil { return err diff --git a/op-e2e/actions/helpers/l2_sequencer.go b/op-e2e/actions/helpers/l2_sequencer.go index 97d8100b1a5d6..f0e9772eedc6a 100644 --- a/op-e2e/actions/helpers/l2_sequencer.go +++ b/op-e2e/actions/helpers/l2_sequencer.go @@ -40,8 +40,8 @@ func (m *MockL1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bl return m.actual.FindL1Origin(ctx, l2Head) } -func (m *MockL1OriginSelector) SetRecoverMode(bool) { - // noop +func (m *MockL1OriginSelector) SetRecoverMode(b bool) { + m.actual.SetRecoverMode(b) } // L2Sequencer is an actor that functions like a rollup node, @@ -87,6 +87,7 @@ func NewL2Sequencer(t Testing, log log.Logger, l1 derive.L1Fetcher, blobSrc deri ver.eventSys.Register("sequencer", seq, opts) ver.eventSys.Register("origin-selector", originSelector, opts) require.NoError(t, seq.Init(t.Ctx(), true)) + return &L2Sequencer{ L2Verifier: ver, sequencer: seq, @@ -272,3 +273,7 @@ func (s *L2Sequencer) ActBuildL2ToInterop(t Testing) { s.ActL2EmptyBlock(t) } } + +func (s *L2Sequencer) ActSetRecoverMode(t Testing, b bool) { + s.sequencer.SetRecoverMode(b) +} diff --git a/op-e2e/actions/helpers/utils.go b/op-e2e/actions/helpers/utils.go index 9fa5d0438741c..eee471ff156d1 100644 --- a/op-e2e/actions/helpers/utils.go +++ b/op-e2e/actions/helpers/utils.go @@ -13,7 +13,7 @@ import ( func DefaultRollupTestParams() *e2eutils.TestParams { return &e2eutils.TestParams{ MaxSequencerDrift: 40, - SequencerWindowSize: 120, + SequencerWindowSize: 10, ChannelTimeout: 120, L1BlockTime: 15, AllocType: config.DefaultAllocType, diff --git a/op-e2e/actions/proofs/helpers/env.go b/op-e2e/actions/proofs/helpers/env.go index 6872c1d0e1efa..92038affcc8ee 100644 --- a/op-e2e/actions/proofs/helpers/env.go +++ b/op-e2e/actions/proofs/helpers/env.go @@ -243,7 +243,7 @@ func (env *L2FaultProofEnv) BatchAndMine(t helpers.Testing) { // Returns the L2 Safe Block Reference func (env *L2FaultProofEnv) BatchMineAndSync(t helpers.Testing) eth.L2BlockRef { t.Helper() - id := env.Miner.UnsafeID() + // id := env.Miner.UnsafeID() env.BatchAndMine(t) env.Sequencer.ActL1HeadSignal(t) env.Sequencer.ActL2PipelineFull(t) @@ -251,7 +251,7 @@ func (env *L2FaultProofEnv) BatchMineAndSync(t helpers.Testing) eth.L2BlockRef { // Assertions syncStatus := env.Sequencer.SyncStatus() - require.Equal(t, syncStatus.UnsafeL2.L1Origin, id, "UnsafeL2.L1Origin should equal L1 Unsafe ID before batch submitted") + // require.Equal(t, syncStatus.UnsafeL2.L1Origin, id, "UnsafeL2.L1Origin should equal L1 Unsafe ID before batch submitted") require.Equal(t, syncStatus.UnsafeL2, syncStatus.SafeL2, "UnsafeL2 should equal SafeL2") return syncStatus.SafeL2 diff --git a/op-e2e/actions/proofs/sequence_window_expiry_test.go b/op-e2e/actions/proofs/sequence_window_expiry_test.go index cb702fe8eb6a4..f959ed73fcf39 100644 --- a/op-e2e/actions/proofs/sequence_window_expiry_test.go +++ b/op-e2e/actions/proofs/sequence_window_expiry_test.go @@ -5,8 +5,6 @@ import ( actionsHelpers "github.com/ethereum-optimism/optimism/op-e2e/actions/helpers" "github.com/ethereum-optimism/optimism/op-e2e/actions/proofs/helpers" - "github.com/ethereum-optimism/optimism/op-program/client/claim" - "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" ) @@ -14,17 +12,29 @@ import ( func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { t := actionsHelpers.NewDefaultTesting(gt) tp := helpers.NewTestParams() - env := helpers.NewL2FaultProofEnv(t, testCfg, tp, helpers.NewBatcherCfg()) - - // Mine an empty block for gas estimation purposes. + bc := helpers.NewBatcherCfg() + + // It seems more difficult to recover with span batches, since the singular batches within are invalidated atomically. + // That is to say, if the oldest batch in the span batch fails the sequencing window check (l1 origin + seq window < l1 inclusion) + // All following batches are invalidated / dropped as well. + // Although, if the same blocks were batched with singular batches, wouldn't the older blocks being rejected invalidate that later batches anyway? + // Perhaps not in the case of recover mode since the noTxPool mode means autoderviation actually fills the gap with identical blocks anyway. + // It seems like this is actually just a problem with derivation, it would be possible to consider modifying the rules to allow for recovery from span batches too. + bc.ForceSubmitSingularBatch = true + env := helpers.NewL2FaultProofEnv(t, testCfg, tp, bc) + + // Mine an empty L1 block for gas estimation purposes. env.Miner.ActEmptyBlock(t) // Expire the sequence window by building `SequenceWindow + 1` empty blocks on L1. + // note that tp.SequencerWindowSize is 10. + // If we were sequencing properly, we would expect the unsafe head to be up to 15*10 = 150 + // because l2 block time is 1s, l1 block time is 15s, and sequence window is 10 blocks. for i := 0; i < int(tp.SequencerWindowSize)+1; i++ { env.Alice.L1.ActResetTxOpts(t) env.Alice.ActDeposit(t) - env.Miner.ActL1StartBlock(12)(t) + env.Miner.ActL1StartBlock(15)(t) env.Miner.ActL1IncludeTx(env.Alice.Address())(t) env.Miner.ActL1EndBlock(t) @@ -45,7 +55,43 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { require.Greater(t, l2SafeHead.Number.Uint64(), uint64(0)) // Run the FPP on one of the auto-derived blocks. - env.RunFaultProofProgram(t, l2SafeHead.Number.Uint64()/2, testCfg.CheckResult, testCfg.InputParams...) + // env.RunFaultProofProgram(t, l2SafeHead.Number.Uint64()/2, testCfg.CheckResult, testCfg.InputParams...) + + // Set recover mode on the sequencer: + // I am not actually convinced we need this... + env.Sequencer.ActSetRecoverMode(t, true) + + // Allow the l1 origin to catch up to the l1 head + lag := tp.SequencerWindowSize + for i := 0; i < int(tp.SequencerWindowSize)*100; i++ { + env.Sequencer.ActL2StartBlock(t) + env.Sequencer.ActL2EndBlock(t) + if i%30 == 0 { + env.BatchMineAndSync(t) + } else if i%15 == 0 { + env.Miner.ActEmptyBlock(t) + } + l1Head := env.Miner.UnsafeNum() + ss := env.Sequencer.SyncStatus() + t.Log( + "unsafeL2.Number", ss.UnsafeL2.Number, + "safeL2.Number", ss.SafeL2.Number, + "currentL1", ss.CurrentL1.Number, + "l1_origin_unsafe", ss.UnsafeL2.L1Origin.Number, + "l1_origin_safe", ss.SafeL2.L1Origin.Number, + "l1_head", l1Head) + lag = ss.CurrentL1.Number - ss.SafeL2.L1Origin.Number + t.Log("lag", lag) // TODO this lag starts out equal to the sequencing window, and eventually decreases to a lower value + if lag < 5 { // What is a reasonable exit condition here? Let's say half the sequencing window + break + } + } + + // env.Sequencer.ActWaitForUserTransactions(t) + + // Run the test until the l1 origin is close to tip, to cover any residual issues with recover mode + // + // Asser that the unsafe chain keeps progressing (and we don't e.g. violate sequencer drift.) } // Runs a that proves a block in a chain where the batcher opens a channel, the sequence window expires, and then the @@ -132,7 +178,7 @@ func Test_ProgramAction_SequenceWindowExpired(gt *testing.T) { matrix := helpers.NewMatrix[any]() defer matrix.Run(gt) - forks := helpers.ForkMatrix{helpers.Granite, helpers.LatestFork} + forks := helpers.ForkMatrix{helpers.LatestFork} matrix.AddTestCase( "HonestClaim", nil, @@ -140,27 +186,27 @@ func Test_ProgramAction_SequenceWindowExpired(gt *testing.T) { runSequenceWindowExpireTest, helpers.ExpectNoError(), ) - matrix.AddTestCase( - "JunkClaim", - nil, - forks, - runSequenceWindowExpireTest, - helpers.ExpectError(claim.ErrClaimNotValid), - helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), - ) - matrix.AddTestCase( - "ChannelCloseAfterWindowExpiry-HonestClaim", - nil, - forks, - runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, - helpers.ExpectNoError(), - ) - matrix.AddTestCase( - "ChannelCloseAfterWindowExpiry-JunkClaim", - nil, - forks, - runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, - helpers.ExpectError(claim.ErrClaimNotValid), - helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), - ) + // matrix.AddTestCase( + // "JunkClaim", + // nil, + // forks, + // runSequenceWindowExpireTest, + // helpers.ExpectError(claim.ErrClaimNotValid), + // helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), + // ) + // matrix.AddTestCase( + // "ChannelCloseAfterWindowExpiry-HonestClaim", + // nil, + // forks, + // runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, + // helpers.ExpectNoError(), + // ) + // matrix.AddTestCase( + // "ChannelCloseAfterWindowExpiry-JunkClaim", + // nil, + // forks, + // runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, + // helpers.ExpectError(claim.ErrClaimNotValid), + // helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), + // ) } diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index 32919fba020bd..3f16288c00718 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -91,6 +91,8 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc return nextOrigin, nil } + los.log.Info("next origin is foo and l2head", "next_origin", nextOrigin, "l2_head_time", l2Head.Time, "next_origin_time", nextOrigin.Time) + msd := los.spec.MaxSequencerDrift(currentOrigin.Time) log := los.log.New("current", currentOrigin, "current_time", currentOrigin.Time, "l2_head", l2Head, "l2_head_time", l2Head.Time, "max_seq_drift", msd) @@ -99,6 +101,7 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc // If we are not past the max sequencer drift, we can just return the current origin. if !pastSeqDrift { + log.Info("not pass seq drift") return currentOrigin, nil } @@ -131,12 +134,14 @@ func (los *L1OriginSelector) CurrentAndNextOrigin(ctx context.Context, l2Head et if los.recoverMode.Load() { currentOrigin, err := los.l1.L1BlockRefByHash(ctx, l2Head.L1Origin.Hash) if err != nil { + los.log.Error("failed to fetch current L1 origin", "error", err) return eth.L1BlockRef{}, eth.L1BlockRef{}, derive.NewTemporaryError(fmt.Errorf("failed to fetch current L1 origin: %w", err)) } los.currentOrigin = currentOrigin nextOrigin, err := los.l1.L1BlockRefByNumber(ctx, currentOrigin.Number+1) if err != nil { + los.log.Error("failed to fetch next L1 origin", "error", err) return eth.L1BlockRef{}, eth.L1BlockRef{}, derive.NewTemporaryError(fmt.Errorf("failed to fetch next L1 origin: %w", err)) } From dbaa3abcc99797a74eab70abfef3edc11a698ca1 Mon Sep 17 00:00:00 2001 From: geoknee Date: Tue, 9 Dec 2025 12:37:18 +0000 Subject: [PATCH 03/38] WIP --- .../proofs/sequence_window_expiry_test.go | 97 ++++++++++++------- op-node/rollup/sequencing/sequencer.go | 2 + 2 files changed, 66 insertions(+), 33 deletions(-) diff --git a/op-e2e/actions/proofs/sequence_window_expiry_test.go b/op-e2e/actions/proofs/sequence_window_expiry_test.go index f959ed73fcf39..2fa405a26d4e6 100644 --- a/op-e2e/actions/proofs/sequence_window_expiry_test.go +++ b/op-e2e/actions/proofs/sequence_window_expiry_test.go @@ -1,6 +1,7 @@ package proofs import ( + "strconv" "testing" actionsHelpers "github.com/ethereum-optimism/optimism/op-e2e/actions/helpers" @@ -8,19 +9,29 @@ import ( "github.com/stretchr/testify/require" ) +type testCase struct { + recoverMode bool + spanBatches bool +} + // Run a test that proves a deposit-only block generated due to sequence window expiry. -func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { +func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[testCase]) { t := actionsHelpers.NewDefaultTesting(gt) tp := helpers.NewTestParams() bc := helpers.NewBatcherCfg() - // It seems more difficult to recover with span batches, since the singular batches within are invalidated atomically. - // That is to say, if the oldest batch in the span batch fails the sequencing window check (l1 origin + seq window < l1 inclusion) - // All following batches are invalidated / dropped as well. - // Although, if the same blocks were batched with singular batches, wouldn't the older blocks being rejected invalidate that later batches anyway? - // Perhaps not in the case of recover mode since the noTxPool mode means autoderviation actually fills the gap with identical blocks anyway. - // It seems like this is actually just a problem with derivation, it would be possible to consider modifying the rules to allow for recovery from span batches too. - bc.ForceSubmitSingularBatch = true + if testCfg.Custom.spanBatches { + // It seems more difficult to recover with span batches, since the singular batches within are invalidated atomically. + // That is to say, if the oldest batch in the span batch fails the sequencing window check (l1 origin + seq window < l1 inclusion) + // All following batches are invalidated / dropped as well. + // Although, if the same blocks were batched with singular batches, wouldn't the older blocks being rejected invalidate that later batches anyway? + // Perhaps not in the case of recover mode since the noTxPool mode means autoderviation actually fills the gap with identical blocks anyway. + // It seems like this is actually just a problem with derivation, it would be possible to consider modifying the rules to allow for recovery from span batches too. + bc.ForceSubmitSpanBatch = true + } else { + bc.ForceSubmitSingularBatch = true + } + env := helpers.NewL2FaultProofEnv(t, testCfg, tp, bc) // Mine an empty L1 block for gas estimation purposes. @@ -55,24 +66,36 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { require.Greater(t, l2SafeHead.Number.Uint64(), uint64(0)) // Run the FPP on one of the auto-derived blocks. - // env.RunFaultProofProgram(t, l2SafeHead.Number.Uint64()/2, testCfg.CheckResult, testCfg.InputParams...) + env.RunFaultProofProgram(t, l2SafeHead.Number.Uint64()/2, testCfg.CheckResult, testCfg.InputParams...) // Set recover mode on the sequencer: // I am not actually convinced we need this... - env.Sequencer.ActSetRecoverMode(t, true) - - // Allow the l1 origin to catch up to the l1 head - lag := tp.SequencerWindowSize - for i := 0; i < int(tp.SequencerWindowSize)*100; i++ { - env.Sequencer.ActL2StartBlock(t) - env.Sequencer.ActL2EndBlock(t) - if i%30 == 0 { + // and I think it does more harm than good when + // the lag approaches zero + env.Sequencer.ActSetRecoverMode(t, testCfg.Custom.recoverMode) + + // Build both chains and assert the l1 origin catches back up with the tip of the L1 chain. + numL1Blocks := 0 + timeout := tp.SequencerWindowSize * 5 + ss := env.Sequencer.SyncStatus() + lag := ss.CurrentL1.Number - ss.SafeL2.L1Origin.Number + require.GreaterOrEqual(t, lag, tp.SequencerWindowSize, "Lag is less than sequencing window size") + for numL1Blocks < int(timeout) { + for range tp.L1BlockTime / env.Sd.RollupCfg.BlockTime { + env.Sequencer.ActL2StartBlock(t) + env.Alice.L2.ActMakeTx(t) + env.Engine.ActL2IncludeTx(env.Alice.Address()) + // RecoverMode will ensure no user transactions, so don't mess with that here. + env.Sequencer.ActL2EndBlock(t) + } + if numL1Blocks%2 == 0 { env.BatchMineAndSync(t) - } else if i%15 == 0 { + } else { env.Miner.ActEmptyBlock(t) } + numL1Blocks++ l1Head := env.Miner.UnsafeNum() - ss := env.Sequencer.SyncStatus() + ss = env.Sequencer.SyncStatus() t.Log( "unsafeL2.Number", ss.UnsafeL2.Number, "safeL2.Number", ss.SafeL2.Number, @@ -81,17 +104,18 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { "l1_origin_safe", ss.SafeL2.L1Origin.Number, "l1_head", l1Head) lag = ss.CurrentL1.Number - ss.SafeL2.L1Origin.Number - t.Log("lag", lag) // TODO this lag starts out equal to the sequencing window, and eventually decreases to a lower value - if lag < 5 { // What is a reasonable exit condition here? Let's say half the sequencing window + t.Log("lag", lag) // This lag starts out equal to the sequencing window, and eventually decreases to a lower value + if lag <= 1 { // We can't expect to get a lag of 0, so a lag of 1 is the best we can hope for. break } } - // env.Sequencer.ActWaitForUserTransactions(t) + if uint64(numL1Blocks) > timeout { + t.Fatal("L1 Origin did not catch up to tip within % L1 blocks", numL1Blocks) + } else { + t.Logf("L1 Origin caught up to within %d blocks of the tip within %d L1 blocks (sequencing window size %d)", lag, numL1Blocks, tp.SequencerWindowSize) + } - // Run the test until the l1 origin is close to tip, to cover any residual issues with recover mode - // - // Asser that the unsafe chain keeps progressing (and we don't e.g. violate sequencer drift.) } // Runs a that proves a block in a chain where the batcher opens a channel, the sequence window expires, and then the @@ -175,17 +199,24 @@ func runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test(gt *testing.T, t } func Test_ProgramAction_SequenceWindowExpired(gt *testing.T) { - matrix := helpers.NewMatrix[any]() + matrix := helpers.NewMatrix[testCase]() defer matrix.Run(gt) forks := helpers.ForkMatrix{helpers.LatestFork} - matrix.AddTestCase( - "HonestClaim", - nil, - forks, - runSequenceWindowExpireTest, - helpers.ExpectNoError(), - ) + // TODO add recoverMode=true test case + for _, testCase := range []testCase{ + {recoverMode: false, spanBatches: false}, + {recoverMode: false, spanBatches: true}, + } { + matrix.AddTestCase( + "HonestClaim-recoverMode-"+strconv.FormatBool(testCase.recoverMode)+"-spanBatches-"+strconv.FormatBool(testCase.spanBatches), + testCase, + forks, + runSequenceWindowExpireTest, + helpers.ExpectNoError(), + ) + } + // matrix.AddTestCase( // "JunkClaim", // nil, diff --git a/op-node/rollup/sequencing/sequencer.go b/op-node/rollup/sequencing/sequencer.go index 9e2b348c93b84..1bb259826eacf 100644 --- a/op-node/rollup/sequencing/sequencer.go +++ b/op-node/rollup/sequencing/sequencer.go @@ -377,6 +377,7 @@ func (d *Sequencer) onSequencerAction(ev SequencerActionEvent) { d.latest.Ref = ref } else { if d.latest.Info != (eth.PayloadInfo{}) { + d.log.Debug("Payload is known, we must have resumed sequencer-actions after a temporary error, meaning that we have seen BuildSealedEvent already.") // We should not repeat the seal request. d.nextActionOK = false // No known payload for block building job, @@ -388,6 +389,7 @@ func (d *Sequencer) onSequencerAction(ev SequencerActionEvent) { DerivedFrom: eth.L1BlockRef{}, }) } else if d.latest == (BuildingState{}) { + d.log.Debug("foobar.") // If we have not started building anything, start building. d.startBuildingBlock() } From 73339162d78a1ebf2daadab01736382eed6f4527 Mon Sep 17 00:00:00 2001 From: geoknee Date: Tue, 9 Dec 2025 14:01:57 +0000 Subject: [PATCH 04/38] Treat NotFound next L1 origin as chain end --- op-node/rollup/sequencing/origin_selector.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index 3f16288c00718..ae3c6311ab4de 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -140,7 +140,12 @@ func (los *L1OriginSelector) CurrentAndNextOrigin(ctx context.Context, l2Head et } los.currentOrigin = currentOrigin nextOrigin, err := los.l1.L1BlockRefByNumber(ctx, currentOrigin.Number+1) - if err != nil { + if errors.Is(err, ethereum.NotFound) { + // If the next origin is not found, it means we are at the end of the chain. + // In this case, we set the next origin to an empty block reference. + los.nextOrigin = eth.L1BlockRef{} + return los.currentOrigin, los.nextOrigin, nil + } else if err != nil { los.log.Error("failed to fetch next L1 origin", "error", err) return eth.L1BlockRef{}, eth.L1BlockRef{}, derive.NewTemporaryError(fmt.Errorf("failed to fetch next L1 origin: %w", err)) From acc6c33359bcad37f11c066c997795669d9105c2 Mon Sep 17 00:00:00 2001 From: geoknee Date: Tue, 9 Dec 2025 14:36:59 +0000 Subject: [PATCH 05/38] Use recover mode in sequence window expiry test --- .../proofs/sequence_window_expiry_test.go | 179 ++++++++++-------- 1 file changed, 96 insertions(+), 83 deletions(-) diff --git a/op-e2e/actions/proofs/sequence_window_expiry_test.go b/op-e2e/actions/proofs/sequence_window_expiry_test.go index 2fa405a26d4e6..ad0f142399727 100644 --- a/op-e2e/actions/proofs/sequence_window_expiry_test.go +++ b/op-e2e/actions/proofs/sequence_window_expiry_test.go @@ -1,36 +1,38 @@ package proofs import ( - "strconv" "testing" actionsHelpers "github.com/ethereum-optimism/optimism/op-e2e/actions/helpers" "github.com/ethereum-optimism/optimism/op-e2e/actions/proofs/helpers" + "github.com/ethereum-optimism/optimism/op-e2e/e2eutils" + "github.com/ethereum-optimism/optimism/op-program/client/claim" + "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" ) -type testCase struct { - recoverMode bool - spanBatches bool -} - -// Run a test that proves a deposit-only block generated due to sequence window expiry. -func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[testCase]) { +// Run a test that proves a deposit-only block generated due to sequence window expiry, +// and then recovers the chain using sequencer recover mode. +func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { t := actionsHelpers.NewDefaultTesting(gt) - tp := helpers.NewTestParams() + const SEQUENCER_WINDOW_SIZE = 10 // (short, to keep test fast) + tp := helpers.NewTestParams(func(p *e2eutils.TestParams) { + p.SequencerWindowSize = SEQUENCER_WINDOW_SIZE + }) + + // It seems more difficult (almost impossible)to recover from sequencing window expiry with span batches, + // since the singular batches within are invalidated _atomically_. + // That is to say, if the oldest batch in the span batch fails the sequencing window chec + // (l1 origin + seq window < l1 inclusion) + // All following batches are invalidated / dropped as well. + // https://github.com/ethereum-optimism/optimism/blob/73339162d78a1ebf2daadab01736382eed6f4527/op-node/rollup/derive/batches.go#L96-L100 + // + // If the same blocks were batched with singular batches, the validation rules are different + // https://github.com/ethereum-optimism/optimism/blob/73339162d78a1ebf2daadab01736382eed6f4527/op-node/rollup/derive/batches.go#L83-L86 + // In the case of recover mode, the noTxPool=true condition means autoderviation actually fills + // the gap with identical blocks anyway, meaning the following batches are actually still valid. bc := helpers.NewBatcherCfg() - - if testCfg.Custom.spanBatches { - // It seems more difficult to recover with span batches, since the singular batches within are invalidated atomically. - // That is to say, if the oldest batch in the span batch fails the sequencing window check (l1 origin + seq window < l1 inclusion) - // All following batches are invalidated / dropped as well. - // Although, if the same blocks were batched with singular batches, wouldn't the older blocks being rejected invalidate that later batches anyway? - // Perhaps not in the case of recover mode since the noTxPool mode means autoderviation actually fills the gap with identical blocks anyway. - // It seems like this is actually just a problem with derivation, it would be possible to consider modifying the rules to allow for recovery from span batches too. - bc.ForceSubmitSpanBatch = true - } else { - bc.ForceSubmitSingularBatch = true - } + bc.ForceSubmitSingularBatch = true env := helpers.NewL2FaultProofEnv(t, testCfg, tp, bc) @@ -39,8 +41,6 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[testCas // Expire the sequence window by building `SequenceWindow + 1` empty blocks on L1. // note that tp.SequencerWindowSize is 10. - // If we were sequencing properly, we would expect the unsafe head to be up to 15*10 = 150 - // because l2 block time is 1s, l1 block time is 15s, and sequence window is 10 blocks. for i := 0; i < int(tp.SequencerWindowSize)+1; i++ { env.Alice.L1.ActResetTxOpts(t) env.Alice.ActDeposit(t) @@ -65,57 +65,75 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[testCas l2SafeHead = env.Engine.L2Chain().CurrentSafeBlock() require.Greater(t, l2SafeHead.Number.Uint64(), uint64(0)) - // Run the FPP on one of the auto-derived blocks. - env.RunFaultProofProgram(t, l2SafeHead.Number.Uint64()/2, testCfg.CheckResult, testCfg.InputParams...) - // Set recover mode on the sequencer: - // I am not actually convinced we need this... - // and I think it does more harm than good when - // the lag approaches zero - env.Sequencer.ActSetRecoverMode(t, testCfg.Custom.recoverMode) + env.Sequencer.ActSetRecoverMode(t, true) + // Since recover mode only affects the L2 CL (op-node), + // it won't stop the test environment injecting transactions + // directly into the engine. So we will force the engine + // to ignore such injections if recover mode is enabled. + env.Engine.EngineApi.SetForceEmpty(true) + + // Define "lag" as the difference between the current L1 block number and the safe L2 block's L1 origin number. + computeLag := func() int { + ss := env.Sequencer.SyncStatus() + return int(ss.CurrentL1.Number - ss.SafeL2.L1Origin.Number) + } - // Build both chains and assert the l1 origin catches back up with the tip of the L1 chain. + // Build both chains and assert the L1 origin catches back up with the tip of the L1 chain. + lag := computeLag() + require.GreaterOrEqual(t, uint64(lag), tp.SequencerWindowSize, "Lag is less than sequencing window size") numL1Blocks := 0 timeout := tp.SequencerWindowSize * 5 - ss := env.Sequencer.SyncStatus() - lag := ss.CurrentL1.Number - ss.SafeL2.L1Origin.Number - require.GreaterOrEqual(t, lag, tp.SequencerWindowSize, "Lag is less than sequencing window size") for numL1Blocks < int(timeout) { for range tp.L1BlockTime / env.Sd.RollupCfg.BlockTime { env.Sequencer.ActL2StartBlock(t) - env.Alice.L2.ActMakeTx(t) - env.Engine.ActL2IncludeTx(env.Alice.Address()) - // RecoverMode will ensure no user transactions, so don't mess with that here. + env.Bob.L2.ActResetTxOpts(t) + env.Bob.L2.ActMakeTx(t) + env.Engine.ActL2IncludeTx(env.Bob.Address())(t) + // RecoverMode (above, if enabled) should prevent this + // transaction from being included in the block, which + // is critical for recover mode to work. env.Sequencer.ActL2EndBlock(t) } if numL1Blocks%2 == 0 { + // Simulate batching every 2 L1 blocks env.BatchMineAndSync(t) } else { + // Keep the L1 chain moving forward env.Miner.ActEmptyBlock(t) } numL1Blocks++ - l1Head := env.Miner.UnsafeNum() - ss = env.Sequencer.SyncStatus() - t.Log( - "unsafeL2.Number", ss.UnsafeL2.Number, - "safeL2.Number", ss.SafeL2.Number, - "currentL1", ss.CurrentL1.Number, - "l1_origin_unsafe", ss.UnsafeL2.L1Origin.Number, - "l1_origin_safe", ss.SafeL2.L1Origin.Number, - "l1_head", l1Head) - lag = ss.CurrentL1.Number - ss.SafeL2.L1Origin.Number - t.Log("lag", lag) // This lag starts out equal to the sequencing window, and eventually decreases to a lower value - if lag <= 1 { // We can't expect to get a lag of 0, so a lag of 1 is the best we can hope for. + lag = computeLag() + t.Log("lag", lag) // This lag starts out equal to the sequencing window, and eventually decreases to 1. + if lag <= 1 { // A lag of 1 is the minimum possible. break } } - if uint64(numL1Blocks) > timeout { - t.Fatal("L1 Origin did not catch up to tip within % L1 blocks", numL1Blocks) + if uint64(numL1Blocks) >= timeout { + t.Fatal("L1 Origin did not catch up to tip within %d L1 blocks (lag is %d)", numL1Blocks, lag) } else { t.Logf("L1 Origin caught up to within %d blocks of the tip within %d L1 blocks (sequencing window size %d)", lag, numL1Blocks, tp.SequencerWindowSize) } + // Disable recover mode so we can get some user transactions in again. + env.Sequencer.ActSetRecoverMode(t, false) + env.Engine.EngineApi.SetForceEmpty(false) + l2SafeBefore := env.Sequencer.L2Safe() + env.Sequencer.ActL2StartBlock(t) + env.Bob.L2.ActResetTxOpts(t) + env.Bob.L2.ActMakeTx(t) + env.Engine.ActL2IncludeTx(env.Bob.Address())(t) + env.Sequencer.ActL2EndBlock(t) + env.BatchMineAndSync(t) + l2Safe := env.Sequencer.L2Safe() + require.Equal(t, l2Safe.Number, l2SafeBefore.Number+1, "safe chain did not progress with user transactions") + l2SafeBlock, err := env.Engine.EthClient().BlockByHash(t.Ctx(), l2Safe.Hash) + require.NoError(t, err) + // Assert safe block has at least two transactions + require.GreaterOrEqual(t, len(l2SafeBlock.Transactions()), 2, "safe block did not have at least two transactions") + + // env.RunFaultProofProgramFromGenesis(t, l2Safe.Number, testCfg.CheckResult, testCfg.InputParams...) } // Runs a that proves a block in a chain where the batcher opens a channel, the sequence window expires, and then the @@ -199,45 +217,40 @@ func runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test(gt *testing.T, t } func Test_ProgramAction_SequenceWindowExpired(gt *testing.T) { - matrix := helpers.NewMatrix[testCase]() + matrix := helpers.NewMatrix[any]() defer matrix.Run(gt) forks := helpers.ForkMatrix{helpers.LatestFork} - // TODO add recoverMode=true test case - for _, testCase := range []testCase{ - {recoverMode: false, spanBatches: false}, - {recoverMode: false, spanBatches: true}, - } { + { matrix.AddTestCase( - "HonestClaim-recoverMode-"+strconv.FormatBool(testCase.recoverMode)+"-spanBatches-"+strconv.FormatBool(testCase.spanBatches), - testCase, + "HonestClaim", + nil, forks, runSequenceWindowExpireTest, helpers.ExpectNoError(), ) } - - // matrix.AddTestCase( - // "JunkClaim", - // nil, - // forks, - // runSequenceWindowExpireTest, - // helpers.ExpectError(claim.ErrClaimNotValid), - // helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), - // ) - // matrix.AddTestCase( - // "ChannelCloseAfterWindowExpiry-HonestClaim", - // nil, - // forks, - // runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, - // helpers.ExpectNoError(), - // ) - // matrix.AddTestCase( - // "ChannelCloseAfterWindowExpiry-JunkClaim", - // nil, - // forks, - // runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, - // helpers.ExpectError(claim.ErrClaimNotValid), - // helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), - // ) + matrix.AddTestCase( + "JunkClaim", + nil, + forks, + runSequenceWindowExpireTest, + helpers.ExpectError(claim.ErrClaimNotValid), + helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), + ) + matrix.AddTestCase( + "ChannelCloseAfterWindowExpiry-HonestClaim", + nil, + forks, + runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, + helpers.ExpectNoError(), + ) + matrix.AddTestCase( + "ChannelCloseAfterWindowExpiry-JunkClaim", + nil, + forks, + runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, + helpers.ExpectError(claim.ErrClaimNotValid), + helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), + ) } From e1bfc22d75f88bbc7b7c244f92ccc94d8ae95cf8 Mon Sep 17 00:00:00 2001 From: geoknee Date: Tue, 9 Dec 2025 14:41:57 +0000 Subject: [PATCH 06/38] Invoke fault proof earlier and fix typos Run env.RunFaultProofProgram after computing l2SafeHead (l2SafeHead.Number.Uint64()/2) and replace the FromGenesis call with RunFaultProofProgram. Fix minor comment typos and wrap a long log line. --- .../actions/proofs/sequence_window_expiry_test.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/op-e2e/actions/proofs/sequence_window_expiry_test.go b/op-e2e/actions/proofs/sequence_window_expiry_test.go index ad0f142399727..9ba682528e2ff 100644 --- a/op-e2e/actions/proofs/sequence_window_expiry_test.go +++ b/op-e2e/actions/proofs/sequence_window_expiry_test.go @@ -20,9 +20,9 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { p.SequencerWindowSize = SEQUENCER_WINDOW_SIZE }) - // It seems more difficult (almost impossible)to recover from sequencing window expiry with span batches, + // It seems more difficult (almost impossible) to recover from sequencing window expiry with span batches, // since the singular batches within are invalidated _atomically_. - // That is to say, if the oldest batch in the span batch fails the sequencing window chec + // That is to say, if the oldest batch in the span batch fails the sequencing window check // (l1 origin + seq window < l1 inclusion) // All following batches are invalidated / dropped as well. // https://github.com/ethereum-optimism/optimism/blob/73339162d78a1ebf2daadab01736382eed6f4527/op-node/rollup/derive/batches.go#L96-L100 @@ -65,6 +65,8 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { l2SafeHead = env.Engine.L2Chain().CurrentSafeBlock() require.Greater(t, l2SafeHead.Number.Uint64(), uint64(0)) + env.RunFaultProofProgram(t, l2SafeHead.Number.Uint64()/2, testCfg.CheckResult, testCfg.InputParams...) + // Set recover mode on the sequencer: env.Sequencer.ActSetRecoverMode(t, true) // Since recover mode only affects the L2 CL (op-node), @@ -90,7 +92,7 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { env.Bob.L2.ActResetTxOpts(t) env.Bob.L2.ActMakeTx(t) env.Engine.ActL2IncludeTx(env.Bob.Address())(t) - // RecoverMode (above, if enabled) should prevent this + // RecoverMode (enabled above) should prevent this // transaction from being included in the block, which // is critical for recover mode to work. env.Sequencer.ActL2EndBlock(t) @@ -113,7 +115,8 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { if uint64(numL1Blocks) >= timeout { t.Fatal("L1 Origin did not catch up to tip within %d L1 blocks (lag is %d)", numL1Blocks, lag) } else { - t.Logf("L1 Origin caught up to within %d blocks of the tip within %d L1 blocks (sequencing window size %d)", lag, numL1Blocks, tp.SequencerWindowSize) + t.Logf("L1 Origin caught up to within %d blocks of the tip within %d L1 blocks (sequencing window size %d)", + lag, numL1Blocks, tp.SequencerWindowSize) } // Disable recover mode so we can get some user transactions in again. @@ -133,7 +136,7 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { // Assert safe block has at least two transactions require.GreaterOrEqual(t, len(l2SafeBlock.Transactions()), 2, "safe block did not have at least two transactions") - // env.RunFaultProofProgramFromGenesis(t, l2Safe.Number, testCfg.CheckResult, testCfg.InputParams...) + env.RunFaultProofProgram(t, l2Safe.Number, testCfg.CheckResult, testCfg.InputParams...) } // Runs a that proves a block in a chain where the batcher opens a channel, the sequence window expires, and then the From 0aa6adcca2043de5de0340efd159e6b1ee18210f Mon Sep 17 00:00:00 2001 From: geoknee Date: Tue, 9 Dec 2025 14:56:36 +0000 Subject: [PATCH 07/38] reduce diff --- justfile | 5 ---- op-e2e/actions/helpers/l2_sequencer.go | 1 - op-e2e/actions/helpers/utils.go | 2 +- op-e2e/actions/proofs/helpers/env.go | 3 --- .../proofs/sequence_window_expiry_test.go | 23 ++++++++----------- op-node/rollup/sequencing/origin_selector.go | 7 +----- op-node/rollup/sequencing/sequencer.go | 2 -- 7 files changed, 12 insertions(+), 31 deletions(-) diff --git a/justfile b/justfile index 68cd88c3d276b..00822da84c73d 100644 --- a/justfile +++ b/justfile @@ -1,4 +1,3 @@ -@include # Checks that TODO comments have corresponding issues. todo-checker: ./ops/scripts/todo-checker.sh @@ -40,7 +39,3 @@ update-op-geth ref: go mod edit -replace=github.com/ethereum/go-ethereum=github.com/ethereum-optimism/op-geth@"$ver"; \ go mod tidy; \ echo "Updated op-geth to $ver" - - -build-contracts: - JUSTFILE=$(pwd)/packages/contracts-bedrock/justfile just build-dev diff --git a/op-e2e/actions/helpers/l2_sequencer.go b/op-e2e/actions/helpers/l2_sequencer.go index f0e9772eedc6a..e509e57186602 100644 --- a/op-e2e/actions/helpers/l2_sequencer.go +++ b/op-e2e/actions/helpers/l2_sequencer.go @@ -87,7 +87,6 @@ func NewL2Sequencer(t Testing, log log.Logger, l1 derive.L1Fetcher, blobSrc deri ver.eventSys.Register("sequencer", seq, opts) ver.eventSys.Register("origin-selector", originSelector, opts) require.NoError(t, seq.Init(t.Ctx(), true)) - return &L2Sequencer{ L2Verifier: ver, sequencer: seq, diff --git a/op-e2e/actions/helpers/utils.go b/op-e2e/actions/helpers/utils.go index eee471ff156d1..9fa5d0438741c 100644 --- a/op-e2e/actions/helpers/utils.go +++ b/op-e2e/actions/helpers/utils.go @@ -13,7 +13,7 @@ import ( func DefaultRollupTestParams() *e2eutils.TestParams { return &e2eutils.TestParams{ MaxSequencerDrift: 40, - SequencerWindowSize: 10, + SequencerWindowSize: 120, ChannelTimeout: 120, L1BlockTime: 15, AllocType: config.DefaultAllocType, diff --git a/op-e2e/actions/proofs/helpers/env.go b/op-e2e/actions/proofs/helpers/env.go index 92038affcc8ee..d700763d7f79f 100644 --- a/op-e2e/actions/proofs/helpers/env.go +++ b/op-e2e/actions/proofs/helpers/env.go @@ -243,15 +243,12 @@ func (env *L2FaultProofEnv) BatchAndMine(t helpers.Testing) { // Returns the L2 Safe Block Reference func (env *L2FaultProofEnv) BatchMineAndSync(t helpers.Testing) eth.L2BlockRef { t.Helper() - // id := env.Miner.UnsafeID() env.BatchAndMine(t) env.Sequencer.ActL1HeadSignal(t) env.Sequencer.ActL2PipelineFull(t) // Assertions - syncStatus := env.Sequencer.SyncStatus() - // require.Equal(t, syncStatus.UnsafeL2.L1Origin, id, "UnsafeL2.L1Origin should equal L1 Unsafe ID before batch submitted") require.Equal(t, syncStatus.UnsafeL2, syncStatus.SafeL2, "UnsafeL2 should equal SafeL2") return syncStatus.SafeL2 diff --git a/op-e2e/actions/proofs/sequence_window_expiry_test.go b/op-e2e/actions/proofs/sequence_window_expiry_test.go index 9ba682528e2ff..6a764413bfa6d 100644 --- a/op-e2e/actions/proofs/sequence_window_expiry_test.go +++ b/op-e2e/actions/proofs/sequence_window_expiry_test.go @@ -40,12 +40,11 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { env.Miner.ActEmptyBlock(t) // Expire the sequence window by building `SequenceWindow + 1` empty blocks on L1. - // note that tp.SequencerWindowSize is 10. for i := 0; i < int(tp.SequencerWindowSize)+1; i++ { env.Alice.L1.ActResetTxOpts(t) env.Alice.ActDeposit(t) - env.Miner.ActL1StartBlock(15)(t) + env.Miner.ActL1StartBlock(tp.L1BlockTime)(t) env.Miner.ActL1IncludeTx(env.Alice.Address())(t) env.Miner.ActL1EndBlock(t) @@ -107,7 +106,7 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { numL1Blocks++ lag = computeLag() t.Log("lag", lag) // This lag starts out equal to the sequencing window, and eventually decreases to 1. - if lag <= 1 { // A lag of 1 is the minimum possible. + if lag == 1 { // A lag of 1 is the minimum possible. break } } @@ -223,16 +222,14 @@ func Test_ProgramAction_SequenceWindowExpired(gt *testing.T) { matrix := helpers.NewMatrix[any]() defer matrix.Run(gt) - forks := helpers.ForkMatrix{helpers.LatestFork} - { - matrix.AddTestCase( - "HonestClaim", - nil, - forks, - runSequenceWindowExpireTest, - helpers.ExpectNoError(), - ) - } + forks := helpers.ForkMatrix{helpers.Granite, helpers.LatestFork} + matrix.AddTestCase( + "HonestClaim", + nil, + forks, + runSequenceWindowExpireTest, + helpers.ExpectNoError(), + ) matrix.AddTestCase( "JunkClaim", nil, diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index ae3c6311ab4de..6c91351b5bc4a 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -87,12 +87,9 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc // of slack. For simplicity, we implement our Sequencer to always start building on the latest // L1 block when we can. if nextOrigin != (eth.L1BlockRef{}) && l2Head.Time+los.cfg.BlockTime >= nextOrigin.Time { - los.log.Info("Starting to build on top of the next origin", "next_origin", nextOrigin) return nextOrigin, nil } - los.log.Info("next origin is foo and l2head", "next_origin", nextOrigin, "l2_head_time", l2Head.Time, "next_origin_time", nextOrigin.Time) - msd := los.spec.MaxSequencerDrift(currentOrigin.Time) log := los.log.New("current", currentOrigin, "current_time", currentOrigin.Time, "l2_head", l2Head, "l2_head_time", l2Head.Time, "max_seq_drift", msd) @@ -101,7 +98,6 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc // If we are not past the max sequencer drift, we can just return the current origin. if !pastSeqDrift { - log.Info("not pass seq drift") return currentOrigin, nil } @@ -134,7 +130,6 @@ func (los *L1OriginSelector) CurrentAndNextOrigin(ctx context.Context, l2Head et if los.recoverMode.Load() { currentOrigin, err := los.l1.L1BlockRefByHash(ctx, l2Head.L1Origin.Hash) if err != nil { - los.log.Error("failed to fetch current L1 origin", "error", err) return eth.L1BlockRef{}, eth.L1BlockRef{}, derive.NewTemporaryError(fmt.Errorf("failed to fetch current L1 origin: %w", err)) } @@ -143,10 +138,10 @@ func (los *L1OriginSelector) CurrentAndNextOrigin(ctx context.Context, l2Head et if errors.Is(err, ethereum.NotFound) { // If the next origin is not found, it means we are at the end of the chain. // In this case, we set the next origin to an empty block reference. + los.log.Error("next L1 origin not found, recover mode likely brought L1 origin up to the tip of the chain", "error", err) los.nextOrigin = eth.L1BlockRef{} return los.currentOrigin, los.nextOrigin, nil } else if err != nil { - los.log.Error("failed to fetch next L1 origin", "error", err) return eth.L1BlockRef{}, eth.L1BlockRef{}, derive.NewTemporaryError(fmt.Errorf("failed to fetch next L1 origin: %w", err)) } diff --git a/op-node/rollup/sequencing/sequencer.go b/op-node/rollup/sequencing/sequencer.go index 1bb259826eacf..9e2b348c93b84 100644 --- a/op-node/rollup/sequencing/sequencer.go +++ b/op-node/rollup/sequencing/sequencer.go @@ -377,7 +377,6 @@ func (d *Sequencer) onSequencerAction(ev SequencerActionEvent) { d.latest.Ref = ref } else { if d.latest.Info != (eth.PayloadInfo{}) { - d.log.Debug("Payload is known, we must have resumed sequencer-actions after a temporary error, meaning that we have seen BuildSealedEvent already.") // We should not repeat the seal request. d.nextActionOK = false // No known payload for block building job, @@ -389,7 +388,6 @@ func (d *Sequencer) onSequencerAction(ev SequencerActionEvent) { DerivedFrom: eth.L1BlockRef{}, }) } else if d.latest == (BuildingState{}) { - d.log.Debug("foobar.") // If we have not started building anything, start building. d.startBuildingBlock() } From 9e95a844e53c9ee8a6d8322b5eaf1b5344755dfc Mon Sep 17 00:00:00 2001 From: geoknee Date: Tue, 9 Dec 2025 15:24:53 +0000 Subject: [PATCH 08/38] Use requireL1OriginAt helper in test Replace manual error assertions around FindL1Origin with requireL1OriginAt and remove the now-unused derive import. --- op-node/rollup/sequencing/origin_selector_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector_test.go b/op-node/rollup/sequencing/origin_selector_test.go index be8a0dc76255a..2f2e12f147f01 100644 --- a/op-node/rollup/sequencing/origin_selector_test.go +++ b/op-node/rollup/sequencing/origin_selector_test.go @@ -7,7 +7,6 @@ import ( "github.com/ethereum-optimism/optimism/op-node/rollup" "github.com/ethereum-optimism/optimism/op-node/rollup/confdepth" - "github.com/ethereum-optimism/optimism/op-node/rollup/derive" "github.com/ethereum-optimism/optimism/op-node/rollup/engine" "github.com/ethereum-optimism/optimism/op-service/eth" "github.com/ethereum-optimism/optimism/op-service/testlog" @@ -213,9 +212,7 @@ func TestOriginSelectorAdvances(t *testing.T) { // is not ready yet by simulating a NotFound error. l1.ExpectL1BlockRefByHash(c.Hash, c, nil) l1.ExpectL1BlockRefByNumber(d.Number, eth.BlockRef{}, ethereum.NotFound) - _, err := s.FindL1Origin(ctx, l2Head) - require.ErrorIs(t, err, derive.ErrTemporary) - require.ErrorIs(t, err, ethereum.NotFound) + requireL1OriginAt(l2Head, c) // Now, simulate the block being ready, and ensure // that the origin advances to the next block. From 90abe3d14df327def7a0d0a1a31b1befffc23747 Mon Sep 17 00:00:00 2001 From: geoknee Date: Wed, 10 Dec 2025 09:21:12 +0000 Subject: [PATCH 09/38] Introduce L2Sequencer.ActMaybeL2StartBlock --- op-e2e/actions/helpers/l2_sequencer.go | 18 +++-- .../proofs/sequence_window_expiry_test.go | 79 +++++++++++-------- 2 files changed, 61 insertions(+), 36 deletions(-) diff --git a/op-e2e/actions/helpers/l2_sequencer.go b/op-e2e/actions/helpers/l2_sequencer.go index e509e57186602..ef3d4e29dfbfc 100644 --- a/op-e2e/actions/helpers/l2_sequencer.go +++ b/op-e2e/actions/helpers/l2_sequencer.go @@ -98,20 +98,28 @@ func NewL2Sequencer(t Testing, log log.Logger, l1 derive.L1Fetcher, blobSrc deri // ActL2StartBlock starts building of a new L2 block on top of the head func (s *L2Sequencer) ActL2StartBlock(t Testing) { + err := s.ActMaybeL2StartBlock(t) + require.NoError(t, err, "failed to start block building") +} + +// ActMaybeL2StartBlock tries to start building a new L2 block on top of the head +func (s *L2Sequencer) ActMaybeL2StartBlock(t Testing) error { require.NoError(t, s.drainer.Drain()) // can't build when other work is still blocking if !s.L2PipelineIdle { t.InvalidAction("cannot start L2 build when derivation is not idle") - return + return nil } if s.l2Building { t.InvalidAction("already started building L2 block") - return + return nil } s.synchronousEvents.Emit(t.Ctx(), sequencing.SequencerActionEvent{}) - require.NoError(t, s.drainer.DrainUntil(event.Is[engine.BuildStartedEvent], false), - "failed to start block building") - + err := s.drainer.DrainUntil(event.Is[engine.BuildStartedEvent], false) + if err != nil { + return err + } s.l2Building = true + return nil } // ActL2EndBlock completes a new L2 block and applies it to the L2 chain as new canonical unsafe head diff --git a/op-e2e/actions/proofs/sequence_window_expiry_test.go b/op-e2e/actions/proofs/sequence_window_expiry_test.go index 6a764413bfa6d..b698afb0c1ce5 100644 --- a/op-e2e/actions/proofs/sequence_window_expiry_test.go +++ b/op-e2e/actions/proofs/sequence_window_expiry_test.go @@ -6,8 +6,6 @@ import ( actionsHelpers "github.com/ethereum-optimism/optimism/op-e2e/actions/helpers" "github.com/ethereum-optimism/optimism/op-e2e/actions/proofs/helpers" "github.com/ethereum-optimism/optimism/op-e2e/e2eutils" - "github.com/ethereum-optimism/optimism/op-program/client/claim" - "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" ) @@ -80,14 +78,30 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { return int(ss.CurrentL1.Number - ss.SafeL2.L1Origin.Number) } + computeDrift := func() int { + ss := env.Sequencer.SyncStatus() + l2header, err := env.Engine.EthClient().HeaderByHash(t.Ctx(), ss.SafeL2.Hash) + require.NoError(t, err) + l1header, err := env.Miner.EthClient().HeaderByHash(t.Ctx(), ss.SafeL2.L1Origin.Hash) + require.NoError(t, err) + t.Log("l2header.Time", l2header.Time) + t.Log("l1header.Time", l1header.Time) + return int(l2header.Time) - int(l1header.Time) + } + // Build both chains and assert the L1 origin catches back up with the tip of the L1 chain. lag := computeLag() + t.Log("lag", lag) require.GreaterOrEqual(t, uint64(lag), tp.SequencerWindowSize, "Lag is less than sequencing window size") numL1Blocks := 0 - timeout := tp.SequencerWindowSize * 5 + timeout := tp.SequencerWindowSize * 50 + for numL1Blocks < int(timeout) { - for range tp.L1BlockTime / env.Sd.RollupCfg.BlockTime { - env.Sequencer.ActL2StartBlock(t) + for range 100 * tp.L1BlockTime / env.Sd.RollupCfg.BlockTime { // go at 100x real time + err := env.Sequencer.ActMaybeL2StartBlock(t) + if err != nil { + break + } env.Bob.L2.ActResetTxOpts(t) env.Bob.L2.ActMakeTx(t) env.Engine.ActL2IncludeTx(env.Bob.Address())(t) @@ -105,8 +119,10 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { } numL1Blocks++ lag = computeLag() - t.Log("lag", lag) // This lag starts out equal to the sequencing window, and eventually decreases to 1. - if lag == 1 { // A lag of 1 is the minimum possible. + t.Log("lag", lag) + drift := computeDrift() + t.Log("drift", drift) + if lag == 1 && numL1Blocks > 10 { // A lag of 1 is the minimum possible. break } } @@ -222,7 +238,8 @@ func Test_ProgramAction_SequenceWindowExpired(gt *testing.T) { matrix := helpers.NewMatrix[any]() defer matrix.Run(gt) - forks := helpers.ForkMatrix{helpers.Granite, helpers.LatestFork} + // forks := helpers.ForkMatrix{helpers.Granite, helpers.LatestFork} + forks := helpers.ForkMatrix{helpers.LatestFork} matrix.AddTestCase( "HonestClaim", nil, @@ -230,27 +247,27 @@ func Test_ProgramAction_SequenceWindowExpired(gt *testing.T) { runSequenceWindowExpireTest, helpers.ExpectNoError(), ) - matrix.AddTestCase( - "JunkClaim", - nil, - forks, - runSequenceWindowExpireTest, - helpers.ExpectError(claim.ErrClaimNotValid), - helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), - ) - matrix.AddTestCase( - "ChannelCloseAfterWindowExpiry-HonestClaim", - nil, - forks, - runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, - helpers.ExpectNoError(), - ) - matrix.AddTestCase( - "ChannelCloseAfterWindowExpiry-JunkClaim", - nil, - forks, - runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, - helpers.ExpectError(claim.ErrClaimNotValid), - helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), - ) + // matrix.AddTestCase( + // "JunkClaim", + // nil, + // forks, + // runSequenceWindowExpireTest, + // helpers.ExpectError(claim.ErrClaimNotValid), + // helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), + // ) + // matrix.AddTestCase( + // "ChannelCloseAfterWindowExpiry-HonestClaim", + // nil, + // forks, + // runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, + // helpers.ExpectNoError(), + // ) + // matrix.AddTestCase( + // "ChannelCloseAfterWindowExpiry-JunkClaim", + // nil, + // forks, + // runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, + // helpers.ExpectError(claim.ErrClaimNotValid), + // helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), + // ) } From 16f51981a83ba7fb54cf487de1c5d401b6a220d2 Mon Sep 17 00:00:00 2001 From: geoknee Date: Wed, 10 Dec 2025 10:36:32 +0000 Subject: [PATCH 10/38] add TestRecoverModeWhenChainHealthy acceptance test sysgo only --- .../tests/sequencer/init_test.go | 15 +++++++ .../tests/sequencer/recover_mode_test.go | 44 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 op-acceptance-tests/tests/sequencer/init_test.go create mode 100644 op-acceptance-tests/tests/sequencer/recover_mode_test.go diff --git a/op-acceptance-tests/tests/sequencer/init_test.go b/op-acceptance-tests/tests/sequencer/init_test.go new file mode 100644 index 0000000000000..1873716f0279a --- /dev/null +++ b/op-acceptance-tests/tests/sequencer/init_test.go @@ -0,0 +1,15 @@ +package sequencer + +import ( + "testing" + + "github.com/ethereum-optimism/optimism/op-devstack/compat" + "github.com/ethereum-optimism/optimism/op-devstack/presets" +) + +// TestMain creates the test-setups against the shared backend +func TestMain(m *testing.M) { + presets.DoMain(m, presets.WithMinimal(), + presets.WithCompatibleTypes(compat.SysGo), + ) +} diff --git a/op-acceptance-tests/tests/sequencer/recover_mode_test.go b/op-acceptance-tests/tests/sequencer/recover_mode_test.go new file mode 100644 index 0000000000000..548218716ab74 --- /dev/null +++ b/op-acceptance-tests/tests/sequencer/recover_mode_test.go @@ -0,0 +1,44 @@ +package sequencer + +import ( + "testing" + "time" + + "github.com/ethereum-optimism/optimism/op-devstack/devtest" + "github.com/ethereum-optimism/optimism/op-devstack/presets" + "github.com/stretchr/testify/require" +) + +// TestRecoverModeWhenChainHealthy checks that the chain +// can progress as normal when recover mode is activated. +// Recover mode is designed to recover from a sequencing +// window expiry when there are ample L1 blocks to eagerly +// progress the l1 origin to. But when the l1 origin is +// close to the tip of the l1 chain, the eagerness would cause +// a delay in unsafe block production while the sequencer waits +// for the next l1 origin to become available. Recover mode +// has since been patched, and the sequencer will not demand the +// next l1 origin until it is actually available. This tests +// protects against a regeression in that behavior. +func TestRecoverModeWhenChainHealthy(gt *testing.T) { + t := devtest.ParallelT(gt) + sys := presets.NewMinimal(t) + tracer := t.Tracer() + ctx := t.Ctx() + + sys.L2CL.SetSequencerRecoverMode(true) + blockTime := sys.L2Chain.Escape().RollupConfig().BlockTime + numL2Blocks := uint64(20) + waitTime := time.Duration(blockTime*numL2Blocks+5) * time.Second + + num := sys.L2CL.SyncStatus().UnsafeL2.Number + new_num := num + require.Eventually(t, func() bool { + ctx, span := tracer.Start(ctx, "check head") + defer span.End() + + new_num, num = sys.L2CL.SyncStatus().UnsafeL2.Number, new_num + t.Logger().InfoContext(ctx, "unsafe head", "number", new_num, "safe head", sys.L2CL.SyncStatus().SafeL2.Number) + return new_num >= numL2Blocks + }, waitTime, time.Duration(blockTime)*time.Second) +} From dd9492eccb8ec81e2ad040fb7e384a506adf5f3f Mon Sep 17 00:00:00 2001 From: geoknee Date: Wed, 10 Dec 2025 11:15:13 +0000 Subject: [PATCH 11/38] Add SetSequencerRecoverMode and enable debug logs --- op-acceptance-tests/tests/sequencer/init_test.go | 2 ++ op-devstack/dsl/l2_cl.go | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/op-acceptance-tests/tests/sequencer/init_test.go b/op-acceptance-tests/tests/sequencer/init_test.go index 1873716f0279a..ed9fb2d955542 100644 --- a/op-acceptance-tests/tests/sequencer/init_test.go +++ b/op-acceptance-tests/tests/sequencer/init_test.go @@ -1,6 +1,7 @@ package sequencer import ( + "log/slog" "testing" "github.com/ethereum-optimism/optimism/op-devstack/compat" @@ -11,5 +12,6 @@ import ( func TestMain(m *testing.M) { presets.DoMain(m, presets.WithMinimal(), presets.WithCompatibleTypes(compat.SysGo), + presets.WithLogLevel(slog.LevelDebug), ) } diff --git a/op-devstack/dsl/l2_cl.go b/op-devstack/dsl/l2_cl.go index 862394fc522c4..2d9874d3987c2 100644 --- a/op-devstack/dsl/l2_cl.go +++ b/op-devstack/dsl/l2_cl.go @@ -91,6 +91,10 @@ func (cl *L2CLNode) StopSequencer() common.Hash { return unsafeHead } +func (cl *L2CLNode) SetSequencerRecoverMode(b bool) error { + return cl.inner.RollupAPI().SetRecoverMode(cl.ctx, b) +} + func (cl *L2CLNode) SyncStatus() *eth.SyncStatus { ctx, cancel := context.WithTimeout(cl.ctx, DefaultTimeout) defer cancel() From 418a9856475cc1bc5deabe08a5f4cdc1309eafdd Mon Sep 17 00:00:00 2001 From: geoknee Date: Wed, 10 Dec 2025 11:48:59 +0000 Subject: [PATCH 12/38] Adjust L1 block time and sequence window test Set default L1 block time to 12s to match action helper assumptions. Increase sequencer window size in the test to 50. Compute drift from UnsafeL2 headers (use UnsafeL2.L1Origin). Simplify L1 mining to always BatchMineAndSync and remove the extra numL1Blocks > 10 lag guard. --- op-e2e/actions/helpers/utils.go | 2 +- .../proofs/sequence_window_expiry_test.go | 20 +++++++------------ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/op-e2e/actions/helpers/utils.go b/op-e2e/actions/helpers/utils.go index 9fa5d0438741c..e8dc404ea164a 100644 --- a/op-e2e/actions/helpers/utils.go +++ b/op-e2e/actions/helpers/utils.go @@ -15,7 +15,7 @@ func DefaultRollupTestParams() *e2eutils.TestParams { MaxSequencerDrift: 40, SequencerWindowSize: 120, ChannelTimeout: 120, - L1BlockTime: 15, + L1BlockTime: 12, // Many of the action helpers assume a 12s L1 block time AllocType: config.DefaultAllocType, } } diff --git a/op-e2e/actions/proofs/sequence_window_expiry_test.go b/op-e2e/actions/proofs/sequence_window_expiry_test.go index b698afb0c1ce5..f400647bfcf9a 100644 --- a/op-e2e/actions/proofs/sequence_window_expiry_test.go +++ b/op-e2e/actions/proofs/sequence_window_expiry_test.go @@ -13,7 +13,7 @@ import ( // and then recovers the chain using sequencer recover mode. func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { t := actionsHelpers.NewDefaultTesting(gt) - const SEQUENCER_WINDOW_SIZE = 10 // (short, to keep test fast) + const SEQUENCER_WINDOW_SIZE = 50 // (short, to keep test fast) tp := helpers.NewTestParams(func(p *e2eutils.TestParams) { p.SequencerWindowSize = SEQUENCER_WINDOW_SIZE }) @@ -78,11 +78,12 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { return int(ss.CurrentL1.Number - ss.SafeL2.L1Origin.Number) } + // Define "drift" as the difference between the current L2 block's timestamp and the unsafe L2 block's L1 origin's timestamp. computeDrift := func() int { ss := env.Sequencer.SyncStatus() - l2header, err := env.Engine.EthClient().HeaderByHash(t.Ctx(), ss.SafeL2.Hash) + l2header, err := env.Engine.EthClient().HeaderByHash(t.Ctx(), ss.UnsafeL2.Hash) require.NoError(t, err) - l1header, err := env.Miner.EthClient().HeaderByHash(t.Ctx(), ss.SafeL2.L1Origin.Hash) + l1header, err := env.Miner.EthClient().HeaderByHash(t.Ctx(), ss.UnsafeL2.L1Origin.Hash) require.NoError(t, err) t.Log("l2header.Time", l2header.Time) t.Log("l1header.Time", l1header.Time) @@ -110,19 +111,12 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { // is critical for recover mode to work. env.Sequencer.ActL2EndBlock(t) } - if numL1Blocks%2 == 0 { - // Simulate batching every 2 L1 blocks - env.BatchMineAndSync(t) - } else { - // Keep the L1 chain moving forward - env.Miner.ActEmptyBlock(t) - } + env.BatchMineAndSync(t) // Mines 1 block on L1 numL1Blocks++ lag = computeLag() t.Log("lag", lag) - drift := computeDrift() - t.Log("drift", drift) - if lag == 1 && numL1Blocks > 10 { // A lag of 1 is the minimum possible. + t.Log("drift", computeDrift()) + if lag == 1 { // A lag of 1 is the minimum possible. break } } From 3b22b347f73774c0bf639aade750c10c9dc703d5 Mon Sep 17 00:00:00 2001 From: geoknee Date: Wed, 10 Dec 2025 12:03:18 +0000 Subject: [PATCH 13/38] restore stub --- .../proofs/sequence_window_expiry_test.go | 67 ++++++++++++------- 1 file changed, 41 insertions(+), 26 deletions(-) diff --git a/op-e2e/actions/proofs/sequence_window_expiry_test.go b/op-e2e/actions/proofs/sequence_window_expiry_test.go index f400647bfcf9a..6e85163d9b074 100644 --- a/op-e2e/actions/proofs/sequence_window_expiry_test.go +++ b/op-e2e/actions/proofs/sequence_window_expiry_test.go @@ -6,6 +6,8 @@ import ( actionsHelpers "github.com/ethereum-optimism/optimism/op-e2e/actions/helpers" "github.com/ethereum-optimism/optimism/op-e2e/actions/proofs/helpers" "github.com/ethereum-optimism/optimism/op-e2e/e2eutils" + "github.com/ethereum-optimism/optimism/op-program/client/claim" + "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" ) @@ -93,6 +95,8 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { // Build both chains and assert the L1 origin catches back up with the tip of the L1 chain. lag := computeLag() t.Log("lag", lag) + drift := computeDrift() + t.Log("drift", drift) require.GreaterOrEqual(t, uint64(lag), tp.SequencerWindowSize, "Lag is less than sequencing window size") numL1Blocks := 0 timeout := tp.SequencerWindowSize * 50 @@ -110,12 +114,15 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { // transaction from being included in the block, which // is critical for recover mode to work. env.Sequencer.ActL2EndBlock(t) + drift = computeDrift() + t.Log("drift", drift) } env.BatchMineAndSync(t) // Mines 1 block on L1 numL1Blocks++ lag = computeLag() t.Log("lag", lag) - t.Log("drift", computeDrift()) + drift = computeDrift() + t.Log("drift", drift) if lag == 1 { // A lag of 1 is the minimum possible. break } @@ -128,6 +135,15 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { lag, numL1Blocks, tp.SequencerWindowSize) } + switch { + case drift == 0: + t.Fatal("drift is zero, this implies the unsafe l2 head is pinned to the l1 head") + case drift > int(tp.MaxSequencerDrift): + t.Fatal("drift is too high") + default: + t.Log("drift", drift) + } + // Disable recover mode so we can get some user transactions in again. env.Sequencer.ActSetRecoverMode(t, false) env.Engine.EngineApi.SetForceEmpty(false) @@ -232,8 +248,7 @@ func Test_ProgramAction_SequenceWindowExpired(gt *testing.T) { matrix := helpers.NewMatrix[any]() defer matrix.Run(gt) - // forks := helpers.ForkMatrix{helpers.Granite, helpers.LatestFork} - forks := helpers.ForkMatrix{helpers.LatestFork} + forks := helpers.ForkMatrix{helpers.Granite, helpers.LatestFork} matrix.AddTestCase( "HonestClaim", nil, @@ -241,27 +256,27 @@ func Test_ProgramAction_SequenceWindowExpired(gt *testing.T) { runSequenceWindowExpireTest, helpers.ExpectNoError(), ) - // matrix.AddTestCase( - // "JunkClaim", - // nil, - // forks, - // runSequenceWindowExpireTest, - // helpers.ExpectError(claim.ErrClaimNotValid), - // helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), - // ) - // matrix.AddTestCase( - // "ChannelCloseAfterWindowExpiry-HonestClaim", - // nil, - // forks, - // runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, - // helpers.ExpectNoError(), - // ) - // matrix.AddTestCase( - // "ChannelCloseAfterWindowExpiry-JunkClaim", - // nil, - // forks, - // runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, - // helpers.ExpectError(claim.ErrClaimNotValid), - // helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), - // ) + matrix.AddTestCase( + "JunkClaim", + nil, + forks, + runSequenceWindowExpireTest, + helpers.ExpectError(claim.ErrClaimNotValid), + helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), + ) + matrix.AddTestCase( + "ChannelCloseAfterWindowExpiry-HonestClaim", + nil, + forks, + runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, + helpers.ExpectNoError(), + ) + matrix.AddTestCase( + "ChannelCloseAfterWindowExpiry-JunkClaim", + nil, + forks, + runSequenceWindowExpire_ChannelCloseAfterWindowExpiry_Test, + helpers.ExpectError(claim.ErrClaimNotValid), + helpers.WithL2Claim(common.HexToHash("0xdeadbeef")), + ) } From 8b0f3d186d8af494b07738eb2897e4771bd99f87 Mon Sep 17 00:00:00 2001 From: geoknee Date: Wed, 10 Dec 2025 21:16:18 +0000 Subject: [PATCH 14/38] WIP --- .../rollup/sequencing/origin_selector_2.go | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 op-node/rollup/sequencing/origin_selector_2.go diff --git a/op-node/rollup/sequencing/origin_selector_2.go b/op-node/rollup/sequencing/origin_selector_2.go new file mode 100644 index 0000000000000..d61e11320de1f --- /dev/null +++ b/op-node/rollup/sequencing/origin_selector_2.go @@ -0,0 +1,82 @@ +package sequencing + +import ( + "fmt" + + "github.com/ethereum-optimism/optimism/op-service/eth" +) + +// FindL1OriginOfNextL2Block finds the L1 origin of the next L2 block. +// It returns an error if there is no way to build a block satisfying +// derivation constraints with the supplied l1OriginChild. +// You can pass a nil pointer for l1OriginChild if it is temporarily +// unavailable. +// TODO: is there a way we can insist on having the l1OriginChild? +// TODO: does the PR introducing async l1 origin fetching provide a hint we should not do that? +func FindL1OriginOfNextL2Block( + l2Head *eth.L2BlockRef, + l1Origin *eth.L1BlockRef, l1OriginChild *eth.L1BlockRef, + matchAutoDerivation bool) (*eth.L1BlockRef, error) { + + if l1Origin == nil { + panic("supplied l1 origin is nil") + } + + if l2Head.L1Origin.Hash != l1Origin.Hash { + // TODO maybe return sentinel error for reorg detected? + panic("supplied l1 origin is not the origin of the supplied l2 head") + } + + if l1OriginChild != nil && l1OriginChild.ParentHash != l1Origin.Hash { + panic("supplied l1 origin child is not a child of the supplied l1 origin") + } + + l2BlockTime := uint64(2) // TODO generalize this + maxDrift := uint64(1800) // TODO generalize this + nextL2BlockTime := l2Head.Time + l2BlockTime + + driftStick := nextL2BlockTime - l1Origin.Time + driftTwist := nextL2BlockTime - l1OriginChild.Time + + if driftTwist > maxDrift { + return nil, fmt.Errorf("drift of next L2 block exceeds maximum %d even when progressing to l1OriginChild", maxDrift) + } + + if matchAutoDerivation { + // THIS SECTION SHOULD MATCH PROTOCOL RULES EXACTLY + // e.g. https://github.com/ethereum-optimism/optimism/blob/3b22b347f73774c0bf639aade750c10c9dc703d5/op-node/rollup/derive/base_batch_stage.go#L162-L206 + if l1OriginChild == nil { + return nil, fmt.Errorf("l1OriginChild is nil but required to match auto derivation, try again later") + } + // Progress to l1OriginChild if doing so would respect the requirement + // that L2 blocks cannot point to a future L1 block. + if nextL2BlockTime >= l1OriginChild.Time { + return l1OriginChild, nil + } + // If we cannot adopt the l1OriginChild, use the current l1 origin. + if nextL2BlockTime < l1OriginChild.Time { + return l1Origin, nil + } + } else { + // THIS SECTION IS partly POLICY, and can be modified/optimized + // by exploiting the freedom allowed by the nonzero sequencer drift. + // Progress to l1OriginChild if it exists + // and if doing so would respect the requirement + // that L2 blocks cannot point to a future L1 block. + if l1OriginChild != nil && nextL2BlockTime >= l1OriginChild.Time { + return l1OriginChild, nil + } + if l1OriginChild != nil && nextL2BlockTime < l1OriginChild.Time { + // This is the only time we are allowed to violate the max drift condition + // in order to preserve the l2time >= l1time invariant + return l1Origin, nil + } + // Otherwise, stick with the current L1 origin unless doing so would exceed the maximum drift. + if driftStick > maxDrift { + // We are allowed to violate this condition ONLY when + return nil, fmt.Errorf("cannot progress to l1OriginChild and drift of next L2 block would exceed maximum %d even when progressing to l1Origin", maxDrift) + } + return l1Origin, nil + } + +} From d7d1ffa03a10abdf57f0b94028056dd519e19c3f Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 10:23:04 +0000 Subject: [PATCH 15/38] name errs --- .../rollup/sequencing/origin_selector_2.go | 89 ++++++++++--------- 1 file changed, 48 insertions(+), 41 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector_2.go b/op-node/rollup/sequencing/origin_selector_2.go index d61e11320de1f..e447997d5ca18 100644 --- a/op-node/rollup/sequencing/origin_selector_2.go +++ b/op-node/rollup/sequencing/origin_selector_2.go @@ -3,59 +3,63 @@ package sequencing import ( "fmt" + "github.com/ethereum-optimism/optimism/op-node/rollup" "github.com/ethereum-optimism/optimism/op-service/eth" ) +var ( + ErrInvalidL1Origin = fmt.Errorf("origin-selector: currentL1Origin.Hash != l2Head.L1Origin.Hash") + ErrInvalidL1OriginChild = fmt.Errorf("origin-selector: nextL1Origin.ParentHash != currentL1Origin.Hash") + ErrCannotSatisfyMaxSequencerDrift = fmt.Errorf("origin-selector: cannot satisfy max sequencer drift") + ErrNextL1OriginRequired = fmt.Errorf("origin-selector: nextL1Origin not supplied but required to satisfy constraints") +) + // FindL1OriginOfNextL2Block finds the L1 origin of the next L2 block. // It returns an error if there is no way to build a block satisfying // derivation constraints with the supplied l1OriginChild. -// You can pass a nil pointer for l1OriginChild if it is temporarily -// unavailable. -// TODO: is there a way we can insist on having the l1OriginChild? -// TODO: does the PR introducing async l1 origin fetching provide a hint we should not do that? +// You can pass a nil pointer for l1OriginChild if it is not yet available, func FindL1OriginOfNextL2Block( + cfg *rollup.Config, l2Head *eth.L2BlockRef, - l1Origin *eth.L1BlockRef, l1OriginChild *eth.L1BlockRef, + currentL1Origin *eth.L1BlockRef, nextL1Origin *eth.L1BlockRef, matchAutoDerivation bool) (*eth.L1BlockRef, error) { - if l1Origin == nil { - panic("supplied l1 origin is nil") + if currentL1Origin == nil { + panic("origin-selector: currentl1Origin is nil") } - - if l2Head.L1Origin.Hash != l1Origin.Hash { - // TODO maybe return sentinel error for reorg detected? - panic("supplied l1 origin is not the origin of the supplied l2 head") + if l2Head.L1Origin.Hash != currentL1Origin.Hash { + return nil, ErrInvalidL1Origin } - - if l1OriginChild != nil && l1OriginChild.ParentHash != l1Origin.Hash { - panic("supplied l1 origin child is not a child of the supplied l1 origin") + if nextL1Origin != nil && nextL1Origin.ParentHash != currentL1Origin.Hash { + return nil, ErrInvalidL1OriginChild } - l2BlockTime := uint64(2) // TODO generalize this - maxDrift := uint64(1800) // TODO generalize this + l2BlockTime := cfg.BlockTime + maxDrift := rollup.NewChainSpec(cfg).MaxSequencerDrift(currentL1Origin.Time) nextL2BlockTime := l2Head.Time + l2BlockTime + driftCurrent := nextL2BlockTime - currentL1Origin.Time + driftNext := nextL2BlockTime - nextL1Origin.Time - driftStick := nextL2BlockTime - l1Origin.Time - driftTwist := nextL2BlockTime - l1OriginChild.Time - - if driftTwist > maxDrift { - return nil, fmt.Errorf("drift of next L2 block exceeds maximum %d even when progressing to l1OriginChild", maxDrift) + if driftNext > maxDrift { + return nil, fmt.Errorf("%w: driftNext %d > maxDrift %d", ErrCannotSatisfyMaxSequencerDrift, driftNext, maxDrift) } if matchAutoDerivation { // THIS SECTION SHOULD MATCH PROTOCOL RULES EXACTLY // e.g. https://github.com/ethereum-optimism/optimism/blob/3b22b347f73774c0bf639aade750c10c9dc703d5/op-node/rollup/derive/base_batch_stage.go#L162-L206 - if l1OriginChild == nil { - return nil, fmt.Errorf("l1OriginChild is nil but required to match auto derivation, try again later") + // If we don't have the nextL1Origin, return an error so we can try again when we do have it. + if nextL1Origin == nil { + // This can cause unsafe block production to slow to the rate of L1 block production, if the L1 origin is up to the L1 l2Head. + // Code higher up the call stack should ensure that matchAutoDerivation is false under such conditions. + return nil, ErrNextL1OriginRequired } // Progress to l1OriginChild if doing so would respect the requirement // that L2 blocks cannot point to a future L1 block. - if nextL2BlockTime >= l1OriginChild.Time { - return l1OriginChild, nil - } - // If we cannot adopt the l1OriginChild, use the current l1 origin. - if nextL2BlockTime < l1OriginChild.Time { - return l1Origin, nil + if nextL2BlockTime >= nextL1Origin.Time { + return nextL1Origin, nil + } else { + // If we cannot adopt the l1OriginChild, use the current l1 origin. + return currentL1Origin, nil } } else { // THIS SECTION IS partly POLICY, and can be modified/optimized @@ -63,20 +67,23 @@ func FindL1OriginOfNextL2Block( // Progress to l1OriginChild if it exists // and if doing so would respect the requirement // that L2 blocks cannot point to a future L1 block. - if l1OriginChild != nil && nextL2BlockTime >= l1OriginChild.Time { - return l1OriginChild, nil + if nextL1Origin == nil { + // If we don't yet have the nextL1Origin, stick with the current L1 origin unless doing so would exceed the maximum drift. + if driftCurrent > maxDrift { + // Return an error so the caller knows it needs to fetch the next l1 origin now. + return nil, fmt.Errorf("%w: drift of next L2 block would exceed maximum %d unless nextl1Origin is adopted", ErrNextL1OriginRequired, maxDrift) + } + return currentL1Origin, nil } - if l1OriginChild != nil && nextL2BlockTime < l1OriginChild.Time { - // This is the only time we are allowed to violate the max drift condition - // in order to preserve the l2time >= l1time invariant - return l1Origin, nil + // Progress to l1OriginChild if doing so would respect the requirement + // that L2 blocks cannot point to a future L1 block. + if nextL2BlockTime >= nextL1Origin.Time { + // Adopt it if + return nextL1Origin, nil } - // Otherwise, stick with the current L1 origin unless doing so would exceed the maximum drift. - if driftStick > maxDrift { - // We are allowed to violate this condition ONLY when - return nil, fmt.Errorf("cannot progress to l1OriginChild and drift of next L2 block would exceed maximum %d even when progressing to l1Origin", maxDrift) + if nextL2BlockTime < nextL1Origin.Time { + // If we cannot adopt the l1OriginChild, use the current l1 origin. + return currentL1Origin, nil } - return l1Origin, nil } - } From 1c08546e1433f82e79dc298da9e5c12612ed0f6e Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 11:03:15 +0000 Subject: [PATCH 16/38] refactor --- .../rollup/sequencing/origin_selector_2.go | 42 ++++++------------- 1 file changed, 12 insertions(+), 30 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector_2.go b/op-node/rollup/sequencing/origin_selector_2.go index e447997d5ca18..af15f14bf5106 100644 --- a/op-node/rollup/sequencing/origin_selector_2.go +++ b/op-node/rollup/sequencing/origin_selector_2.go @@ -44,30 +44,12 @@ func FindL1OriginOfNextL2Block( return nil, fmt.Errorf("%w: driftNext %d > maxDrift %d", ErrCannotSatisfyMaxSequencerDrift, driftNext, maxDrift) } - if matchAutoDerivation { - // THIS SECTION SHOULD MATCH PROTOCOL RULES EXACTLY - // e.g. https://github.com/ethereum-optimism/optimism/blob/3b22b347f73774c0bf639aade750c10c9dc703d5/op-node/rollup/derive/base_batch_stage.go#L162-L206 - // If we don't have the nextL1Origin, return an error so we can try again when we do have it. - if nextL1Origin == nil { + if nextL1Origin == nil { + if matchAutoDerivation { // This can cause unsafe block production to slow to the rate of L1 block production, if the L1 origin is up to the L1 l2Head. // Code higher up the call stack should ensure that matchAutoDerivation is false under such conditions. return nil, ErrNextL1OriginRequired - } - // Progress to l1OriginChild if doing so would respect the requirement - // that L2 blocks cannot point to a future L1 block. - if nextL2BlockTime >= nextL1Origin.Time { - return nextL1Origin, nil } else { - // If we cannot adopt the l1OriginChild, use the current l1 origin. - return currentL1Origin, nil - } - } else { - // THIS SECTION IS partly POLICY, and can be modified/optimized - // by exploiting the freedom allowed by the nonzero sequencer drift. - // Progress to l1OriginChild if it exists - // and if doing so would respect the requirement - // that L2 blocks cannot point to a future L1 block. - if nextL1Origin == nil { // If we don't yet have the nextL1Origin, stick with the current L1 origin unless doing so would exceed the maximum drift. if driftCurrent > maxDrift { // Return an error so the caller knows it needs to fetch the next l1 origin now. @@ -75,15 +57,15 @@ func FindL1OriginOfNextL2Block( } return currentL1Origin, nil } - // Progress to l1OriginChild if doing so would respect the requirement - // that L2 blocks cannot point to a future L1 block. - if nextL2BlockTime >= nextL1Origin.Time { - // Adopt it if - return nextL1Origin, nil - } - if nextL2BlockTime < nextL1Origin.Time { - // If we cannot adopt the l1OriginChild, use the current l1 origin. - return currentL1Origin, nil - } + } + + // Progress to l1OriginChild if doing so would respect the requirement + // that L2 blocks cannot point to a future L1 block. + if nextL2BlockTime >= nextL1Origin.Time { + // Adopt it if + return nextL1Origin, nil + } else { + // If we cannot adopt the l1OriginChild, use the current l1 origin. + return currentL1Origin, nil } } From 7187af4da4b06ade4c9aba1141c58962fb169013 Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 11:23:22 +0000 Subject: [PATCH 17/38] fix --- op-node/rollup/sequencing/origin_selector_2.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector_2.go b/op-node/rollup/sequencing/origin_selector_2.go index af15f14bf5106..d3c7c5eddf814 100644 --- a/op-node/rollup/sequencing/origin_selector_2.go +++ b/op-node/rollup/sequencing/origin_selector_2.go @@ -8,10 +8,9 @@ import ( ) var ( - ErrInvalidL1Origin = fmt.Errorf("origin-selector: currentL1Origin.Hash != l2Head.L1Origin.Hash") - ErrInvalidL1OriginChild = fmt.Errorf("origin-selector: nextL1Origin.ParentHash != currentL1Origin.Hash") - ErrCannotSatisfyMaxSequencerDrift = fmt.Errorf("origin-selector: cannot satisfy max sequencer drift") - ErrNextL1OriginRequired = fmt.Errorf("origin-selector: nextL1Origin not supplied but required to satisfy constraints") + ErrInvalidL1Origin = fmt.Errorf("origin-selector: currentL1Origin.Hash != l2Head.L1Origin.Hash") + ErrInvalidL1OriginChild = fmt.Errorf("origin-selector: nextL1Origin.ParentHash != currentL1Origin.Hash") + ErrNextL1OriginRequired = fmt.Errorf("origin-selector: nextL1Origin not supplied but required to satisfy constraints") ) // FindL1OriginOfNextL2Block finds the L1 origin of the next L2 block. @@ -38,11 +37,6 @@ func FindL1OriginOfNextL2Block( maxDrift := rollup.NewChainSpec(cfg).MaxSequencerDrift(currentL1Origin.Time) nextL2BlockTime := l2Head.Time + l2BlockTime driftCurrent := nextL2BlockTime - currentL1Origin.Time - driftNext := nextL2BlockTime - nextL1Origin.Time - - if driftNext > maxDrift { - return nil, fmt.Errorf("%w: driftNext %d > maxDrift %d", ErrCannotSatisfyMaxSequencerDrift, driftNext, maxDrift) - } if nextL1Origin == nil { if matchAutoDerivation { From b33328a80686133901c1f6ac6e78a15255f6e61a Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 11:24:54 +0000 Subject: [PATCH 18/38] add test --- .../sequencing/origin_selector2_test.go | 109 ++++++++++++++++++ .../rollup/sequencing/origin_selector_2.go | 2 +- 2 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 op-node/rollup/sequencing/origin_selector2_test.go diff --git a/op-node/rollup/sequencing/origin_selector2_test.go b/op-node/rollup/sequencing/origin_selector2_test.go new file mode 100644 index 0000000000000..1a28c74f5a91b --- /dev/null +++ b/op-node/rollup/sequencing/origin_selector2_test.go @@ -0,0 +1,109 @@ +package sequencing + +import ( + "testing" + + "github.com/ethereum-optimism/optimism/op-node/rollup" + "github.com/ethereum-optimism/optimism/op-service/eth" + "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/require" +) + +func TestFindL1OriginOfNextL2Block(t *testing.T) { + cfg := &rollup.Config{ + MaxSequencerDrift: 500, + BlockTime: 2, + } + + require.Panics(t, func() { + FindL1OriginOfNextL2Block( + cfg, + nil, + nil, + nil, + false) + }) + + type testCase struct { + name string + l2Head *eth.L2BlockRef + currentL1Origin *eth.L1BlockRef + nextL1Origin *eth.L1BlockRef + matchAutoderivation bool + expectedResult *eth.L1BlockRef + expectedError error + } + + // L1 chain: a100(1200) <-a101(1212) + // /\ + // L2 chain \_ b1000 (1220) + a100 := ð.L1BlockRef{ + Number: 100, + Hash: common.Hash{'a', '1', '0', '0'}, + Time: 1200, + } + a101 := ð.L1BlockRef{ + Number: 101, + ParentHash: a100.Hash, + Hash: common.Hash{'a', '0', '0'}, + Time: 1212, + } + b1000 := ð.L2BlockRef{ + Number: 1000, + Hash: common.Hash{'b', '1', '0', '0', '0'}, + L1Origin: a100.ID(), + Time: 1220, + } + + tcs := []testCase{ + { + name: "normal operation, progress because we can", + l2Head: b1000, + currentL1Origin: a100, + nextL1Origin: a101, + expectedResult: a101, + }, + { + name: "recover mode, progress because we can", + l2Head: b1000, + currentL1Origin: a100, + nextL1Origin: a101, + expectedResult: a101, + matchAutoderivation: true, + }, + { + name: "normal operation, don't need to progress", + l2Head: b1000, + currentL1Origin: a100, + nextL1Origin: nil, + expectedResult: a100, + }, + { + name: "recover mode, need to progress but can't", + l2Head: b1000, + currentL1Origin: a100, + nextL1Origin: nil, + matchAutoderivation: true, + expectedError: ErrNextL1OriginRequired, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + result, err := FindL1OriginOfNextL2Block( + cfg, + tc.l2Head, + tc.currentL1Origin, + tc.nextL1Origin, + tc.matchAutoderivation) + if tc.expectedError != nil { + require.ErrorIs(t, err, tc.expectedError) + } else { + require.NoError(t, err) + } + if result != tc.expectedResult { + t.Errorf("expected result %v, got %v", tc.expectedResult, result) + } + }) + } +} diff --git a/op-node/rollup/sequencing/origin_selector_2.go b/op-node/rollup/sequencing/origin_selector_2.go index d3c7c5eddf814..1bd177f1f44e5 100644 --- a/op-node/rollup/sequencing/origin_selector_2.go +++ b/op-node/rollup/sequencing/origin_selector_2.go @@ -40,7 +40,7 @@ func FindL1OriginOfNextL2Block( if nextL1Origin == nil { if matchAutoDerivation { - // This can cause unsafe block production to slow to the rate of L1 block production, if the L1 origin is up to the L1 l2Head. + // This can cause unsafe block production to slow to the rate of L1 block production, if the L1 origin is caught up to the L1 Head. // Code higher up the call stack should ensure that matchAutoDerivation is false under such conditions. return nil, ErrNextL1OriginRequired } else { From d164cfc9285b677aa505b6fdf6abcb6c2fc32b09 Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 11:39:25 +0000 Subject: [PATCH 19/38] Rename error constant and add L1 origin tests Rename ErrInvalidL1OriginChild to ErrNextL1OriginOrphaned and adjust the error text. Add test cases covering L1 reorg and invalid L1 origin, and refactor the test case construction in the unit test. --- .../sequencing/origin_selector2_test.go | 58 ++++++++++++++++--- .../rollup/sequencing/origin_selector_2.go | 4 +- 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector2_test.go b/op-node/rollup/sequencing/origin_selector2_test.go index 1a28c74f5a91b..7d2e9ee7cf7f2 100644 --- a/op-node/rollup/sequencing/origin_selector2_test.go +++ b/op-node/rollup/sequencing/origin_selector2_test.go @@ -34,9 +34,11 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { expectedError error } - // L1 chain: a100(1200) <-a101(1212) + tcs := []testCase{} + + // L1 chain: a100(1200) <- [ a101(1212) ] // /\ - // L2 chain \_ b1000 (1220) + // L2 chain \_ b1000(1220) a100 := ð.L1BlockRef{ Number: 100, Hash: common.Hash{'a', '1', '0', '0'}, @@ -55,15 +57,15 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { Time: 1220, } - tcs := []testCase{ - { + tcs = append(tcs, + testCase{ name: "normal operation, progress because we can", l2Head: b1000, currentL1Origin: a100, nextL1Origin: a101, expectedResult: a101, }, - { + testCase{ name: "recover mode, progress because we can", l2Head: b1000, currentL1Origin: a100, @@ -71,14 +73,14 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { expectedResult: a101, matchAutoderivation: true, }, - { + testCase{ name: "normal operation, don't need to progress", l2Head: b1000, currentL1Origin: a100, nextL1Origin: nil, expectedResult: a100, }, - { + testCase{ name: "recover mode, need to progress but can't", l2Head: b1000, currentL1Origin: a100, @@ -86,7 +88,49 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { matchAutoderivation: true, expectedError: ErrNextL1OriginRequired, }, + ) + + // L1 chain: c100(1200) <-[x]- c101(1212) + // /\ + // L2 chain \_[x]- d/e1000(1220) + c100 := ð.L1BlockRef{ + Number: 100, + Hash: common.Hash{'a', '1', '0', '0'}, + Time: 1200, + } + c101 := ð.L1BlockRef{ + Number: 101, + ParentHash: common.Hash{}, // does not point to c100 + Hash: common.Hash{'a', '0', '0'}, + Time: 1212, } + d1000 := ð.L2BlockRef{ + Number: 1000, + L1Origin: c100.ID(), + Hash: common.Hash{'d', '1', '0', '0', '0'}, + Time: 1220, + } + e1000 := ð.L2BlockRef{ + Number: 1000, + L1Origin: eth.BlockID{}, // does not point to c100 + Hash: common.Hash{'d', '1', '0', '0', '0'}, + Time: 1220, + } + tcs = append(tcs, + testCase{ + name: "L1 reorg", + currentL1Origin: c100, + nextL1Origin: c101, + l2Head: d1000, + expectedError: ErrNextL1OriginOrphaned, + }, + testCase{ + name: "Invalid l1 origin", + currentL1Origin: c100, + nextL1Origin: c101, + l2Head: e1000, + expectedError: ErrInvalidL1Origin, + }) for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { diff --git a/op-node/rollup/sequencing/origin_selector_2.go b/op-node/rollup/sequencing/origin_selector_2.go index 1bd177f1f44e5..944b6a43f86f7 100644 --- a/op-node/rollup/sequencing/origin_selector_2.go +++ b/op-node/rollup/sequencing/origin_selector_2.go @@ -9,7 +9,7 @@ import ( var ( ErrInvalidL1Origin = fmt.Errorf("origin-selector: currentL1Origin.Hash != l2Head.L1Origin.Hash") - ErrInvalidL1OriginChild = fmt.Errorf("origin-selector: nextL1Origin.ParentHash != currentL1Origin.Hash") + ErrNextL1OriginOrphaned = fmt.Errorf("origin-selector: nextL1Origin.ParentHash != currentL1Origin.Hash") ErrNextL1OriginRequired = fmt.Errorf("origin-selector: nextL1Origin not supplied but required to satisfy constraints") ) @@ -30,7 +30,7 @@ func FindL1OriginOfNextL2Block( return nil, ErrInvalidL1Origin } if nextL1Origin != nil && nextL1Origin.ParentHash != currentL1Origin.Hash { - return nil, ErrInvalidL1OriginChild + return nil, ErrNextL1OriginOrphaned } l2BlockTime := cfg.BlockTime From 9fb356056d9b6e29b3669cf70529de812166a632 Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 12:42:48 +0000 Subject: [PATCH 20/38] Use drift check for next L1 origin and update tests Compute driftNext and use it to decide adoption of the next L1 origin instead of comparing absolute timestamps. Bump MaxSequencerDrift to 1800 and add tests covering max-drift and negative-drift scenarios. --- .../sequencing/origin_selector2_test.go | 52 ++++++++++++++++++- .../rollup/sequencing/origin_selector_2.go | 6 ++- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector2_test.go b/op-node/rollup/sequencing/origin_selector2_test.go index 7d2e9ee7cf7f2..603f91ebde5e4 100644 --- a/op-node/rollup/sequencing/origin_selector2_test.go +++ b/op-node/rollup/sequencing/origin_selector2_test.go @@ -11,7 +11,7 @@ import ( func TestFindL1OriginOfNextL2Block(t *testing.T) { cfg := &rollup.Config{ - MaxSequencerDrift: 500, + MaxSequencerDrift: 1800, // Use Fjord constant value BlockTime: 2, } @@ -36,6 +36,9 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { tcs := []testCase{} + // Scenarios with valid data, no drift concerns + // but availability of next l1 origin is modulated. + // // L1 chain: a100(1200) <- [ a101(1212) ] // /\ // L2 chain \_ b1000(1220) @@ -90,6 +93,7 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { }, ) + // Bad input data / reorg scenarios // L1 chain: c100(1200) <-[x]- c101(1212) // /\ // L2 chain \_[x]- d/e1000(1220) @@ -132,6 +136,52 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { expectedError: ErrInvalidL1Origin, }) + // Drift at maximum, + // L1 chain: a100(1200) <- [ a101(1212) ] + // /\ + // L2 chain \_ f1000(3000) + f1000 := ð.L2BlockRef{ + Number: 1000, + L1Origin: a100.ID(), + Hash: common.Hash{'f', '1', '0', '0', '0'}, + Time: 3000, + } + tcs = append(tcs, + testCase{ + name: "Drift at maximum, nextL1Origin available", + currentL1Origin: a100, + nextL1Origin: a101, + l2Head: f1000, + expectedResult: a101, + }, + testCase{ + name: "Drift at maximum, nextL1Origin unavailable", + currentL1Origin: a100, + l2Head: f1000, + expectedError: ErrNextL1OriginRequired, + }) + + // Negative drift, + // L1 chain: a100(1200) <- a101(1212) + // /\ + // L2 chain \_ g1000(1200) + // Current drift is 0 + // adopting the nextLOrigin would make it negative (add 2 subtract 12) + g1000 := ð.L2BlockRef{ + Number: 1000, + L1Origin: a100.ID(), + Hash: common.Hash{'g', '1', '0', '0', '0'}, + Time: 1200, + } + tcs = append(tcs, + testCase{ + name: "Negative drift", + currentL1Origin: a100, + nextL1Origin: a101, + l2Head: g1000, + expectedResult: a100, + }) + for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { result, err := FindL1OriginOfNextL2Block( diff --git a/op-node/rollup/sequencing/origin_selector_2.go b/op-node/rollup/sequencing/origin_selector_2.go index 944b6a43f86f7..406a639d3043f 100644 --- a/op-node/rollup/sequencing/origin_selector_2.go +++ b/op-node/rollup/sequencing/origin_selector_2.go @@ -53,9 +53,11 @@ func FindL1OriginOfNextL2Block( } } + driftNext := int64(nextL2BlockTime) - int64(nextL1Origin.Time) + // Progress to l1OriginChild if doing so would respect the requirement - // that L2 blocks cannot point to a future L1 block. - if nextL2BlockTime >= nextL1Origin.Time { + // that L2 blocks cannot point to a future L1 block (negative drift). + if driftNext >= 0 { // Adopt it if return nextL1Origin, nil } else { From 84e6db962bc007e521cb50af62a9f799e97b991c Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 13:27:17 +0000 Subject: [PATCH 21/38] Refactor L1 origin selection and error handling Delegate origin decision logic to FindL1OriginOfNextL2Block and handle synchronous fetch when in recover mode. Remove recover-mode fetching from CurrentAndNextOrigin and return cached values instead. Update sequencer error handling to distinguish invalid/orphaned origin (which triggers a reset) from temporary lookup errors. --- op-node/rollup/sequencing/origin_selector.go | 89 ++++++------------- .../rollup/sequencing/origin_selector_2.go | 5 +- op-node/rollup/sequencing/sequencer.go | 19 ++-- 3 files changed, 38 insertions(+), 75 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index 6c91351b5bc4a..bd178c54da47f 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -3,7 +3,6 @@ package sequencing import ( "context" "errors" - "fmt" "sync" "sync/atomic" "time" @@ -70,86 +69,48 @@ func (los *L1OriginSelector) OnEvent(ctx context.Context, ev event.Event) bool { return true } -// FindL1Origin determines what the next L1 Origin should be. -// The L1 Origin is either the L2 Head's Origin, or the following L1 block -// if the next L2 block's time is greater than or equal to the L2 Head's Origin. -// The origin selection relies purely on block numbers and it is the caller's -// responsibility to detect and handle L1 reorgs. +// FindL1Origin determines what the L1 Origin for the next L2 Block should be. func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) { currentOrigin, nextOrigin, err := los.CurrentAndNextOrigin(ctx, l2Head) if err != nil { return eth.L1BlockRef{}, err } - // If the next L2 block time is greater than the next origin block's time, we can choose to - // start building on top of the next origin. Sequencer implementation has some leeway here and - // could decide to continue to build on top of the previous origin until the Sequencer runs out - // of slack. For simplicity, we implement our Sequencer to always start building on the latest - // L1 block when we can. - if nextOrigin != (eth.L1BlockRef{}) && l2Head.Time+los.cfg.BlockTime >= nextOrigin.Time { - return nextOrigin, nil - } - - msd := los.spec.MaxSequencerDrift(currentOrigin.Time) - log := los.log.New("current", currentOrigin, "current_time", currentOrigin.Time, - "l2_head", l2Head, "l2_head_time", l2Head.Time, "max_seq_drift", msd) - - pastSeqDrift := l2Head.Time+los.cfg.BlockTime-currentOrigin.Time > msd - - // If we are not past the max sequencer drift, we can just return the current origin. - if !pastSeqDrift { - return currentOrigin, nil - } - - // Otherwise, we need to find the next L1 origin block in order to continue producing blocks. - log.Warn("Next L2 block time is past the sequencer drift + current origin time") - - if nextOrigin == (eth.L1BlockRef{}) { - fetchCtx, cancel := context.WithTimeout(ctx, 10*time.Second) - defer cancel() - - // If the next origin is not set, we need to fetch it now. - nextOrigin, err = los.fetch(fetchCtx, currentOrigin.Number+1) - if err != nil { - return eth.L1BlockRef{}, fmt.Errorf("cannot build next L2 block past current L1 origin %s by more than sequencer time drift, and failed to find next L1 origin: %w", currentOrigin, err) + // If in recover mode, get next origin synchronously + matchAutoDerivation := los.recoverMode.Load() + if matchAutoDerivation && nextOrigin == eth.L1BlockRef{} { + nextOrigin, err = los.fetch(ctx) + if errors.Is(err, ethereum.NotFound) { + // We caught up to tip and no longer want to match auto derivation + matchAutoDerivation = false + } else if err != nil { + return eth.L1BlockRef{}, err } } - // If the next origin is ahead of the L2 head, we must return the current origin. - if l2Head.Time+los.cfg.BlockTime < nextOrigin.Time { - return currentOrigin, nil + // TODO harmonize how we represent "no data for next origin" + var nextOriginPtr *eth.L1BlockRef + if (nextOrigin != eth.L1BlockRef{}) { + nextOriginPtr = &nextOrigin } - return nextOrigin, nil + // defer to pure function + o, err := FindL1OriginOfNextL2Block(los.cfg, + &l2Head, + ¤tOrigin, + nextOriginPtr, + matchAutoDerivation) + + return *o, err } +// CurrentAndNextOrigin returns the current cached values for the current L1 origin for the supplied l2Head, and it's successor. +// It only performs a fetch to L1 if the cache is invalid. +// The cache can be update asynchronously by other methods on L1OriginSelector. func (los *L1OriginSelector) CurrentAndNextOrigin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, eth.L1BlockRef, error) { los.mu.Lock() defer los.mu.Unlock() - if los.recoverMode.Load() { - currentOrigin, err := los.l1.L1BlockRefByHash(ctx, l2Head.L1Origin.Hash) - if err != nil { - return eth.L1BlockRef{}, eth.L1BlockRef{}, - derive.NewTemporaryError(fmt.Errorf("failed to fetch current L1 origin: %w", err)) - } - los.currentOrigin = currentOrigin - nextOrigin, err := los.l1.L1BlockRefByNumber(ctx, currentOrigin.Number+1) - if errors.Is(err, ethereum.NotFound) { - // If the next origin is not found, it means we are at the end of the chain. - // In this case, we set the next origin to an empty block reference. - los.log.Error("next L1 origin not found, recover mode likely brought L1 origin up to the tip of the chain", "error", err) - los.nextOrigin = eth.L1BlockRef{} - return los.currentOrigin, los.nextOrigin, nil - } else if err != nil { - return eth.L1BlockRef{}, eth.L1BlockRef{}, - derive.NewTemporaryError(fmt.Errorf("failed to fetch next L1 origin: %w", err)) - } - los.nextOrigin = nextOrigin - los.log.Info("origin selector in recover mode", "current_origin", los.currentOrigin, "next_origin", los.nextOrigin, "l2_head", l2Head) - return los.currentOrigin, los.nextOrigin, nil - } - if l2Head.L1Origin == los.currentOrigin.ID() { // Most likely outcome: the L2 head is still on the current origin. } else if l2Head.L1Origin == los.nextOrigin.ID() { diff --git a/op-node/rollup/sequencing/origin_selector_2.go b/op-node/rollup/sequencing/origin_selector_2.go index 406a639d3043f..135e3a055241f 100644 --- a/op-node/rollup/sequencing/origin_selector_2.go +++ b/op-node/rollup/sequencing/origin_selector_2.go @@ -15,8 +15,9 @@ var ( // FindL1OriginOfNextL2Block finds the L1 origin of the next L2 block. // It returns an error if there is no way to build a block satisfying -// derivation constraints with the supplied l1OriginChild. -// You can pass a nil pointer for l1OriginChild if it is not yet available, +// derivation constraints with the supplied data. +// You can pass a nil pointer for l1OriginChild if it is not yet available +// removing the need for block building to wait on the result of network calls func FindL1OriginOfNextL2Block( cfg *rollup.Config, l2Head *eth.L2BlockRef, diff --git a/op-node/rollup/sequencing/sequencer.go b/op-node/rollup/sequencing/sequencer.go index 9e2b348c93b84..ddb12df36590a 100644 --- a/op-node/rollup/sequencing/sequencer.go +++ b/op-node/rollup/sequencing/sequencer.go @@ -503,21 +503,22 @@ func (d *Sequencer) startBuildingBlock() { // Figure out which L1 origin block we're going to be building on top of. l1Origin, err := d.l1OriginSelector.FindL1Origin(ctx, l2Head) - if err != nil { - d.nextAction = d.timeNow().Add(time.Second) - d.nextActionOK = d.active.Load() - d.log.Error("Error finding next L1 Origin", "err", err) - d.emitter.Emit(d.ctx, rollup.L1TemporaryErrorEvent{Err: err}) - return - } - - if !(l2Head.L1Origin.Hash == l1Origin.ParentHash || l2Head.L1Origin.Hash == l1Origin.Hash) { + switch err { + case ErrInvalidL1Origin, ErrNextL1OriginOrphaned: d.metrics.RecordSequencerInconsistentL1Origin(l2Head.L1Origin, l1Origin.ID()) d.emitter.Emit(d.ctx, rollup.ResetEvent{ Err: fmt.Errorf("cannot build new L2 block with L1 origin %s (parent L1 %s) on current L2 head %s with L1 origin %s", l1Origin, l1Origin.ParentHash, l2Head, l2Head.L1Origin), }) return + case ErrNextL1OriginRequired: + fallthrough + default: + d.nextAction = d.timeNow().Add(time.Second) + d.nextActionOK = d.active.Load() + d.log.Error("Error finding next L1 Origin", "err", err) + d.emitter.Emit(d.ctx, rollup.L1TemporaryErrorEvent{Err: err}) + return } d.log.Info("Started sequencing new block", "parent", l2Head, "l1Origin", l1Origin) From 3ca92430ba4619246b253cc5603b1cf3bdd7e5ea Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 13:50:40 +0000 Subject: [PATCH 22/38] fixes --- op-node/rollup/sequencing/origin_selector.go | 6 ++++-- op-node/rollup/sequencing/sequencer.go | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index bd178c54da47f..fb1ebb52891c4 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -71,6 +71,8 @@ func (los *L1OriginSelector) OnEvent(ctx context.Context, ev event.Event) bool { // FindL1Origin determines what the L1 Origin for the next L2 Block should be. func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) { + + // Get cached values for currentOrigin and nextOrigin currentOrigin, nextOrigin, err := los.CurrentAndNextOrigin(ctx, l2Head) if err != nil { return eth.L1BlockRef{}, err @@ -78,8 +80,8 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc // If in recover mode, get next origin synchronously matchAutoDerivation := los.recoverMode.Load() - if matchAutoDerivation && nextOrigin == eth.L1BlockRef{} { - nextOrigin, err = los.fetch(ctx) + if (matchAutoDerivation && nextOrigin == eth.L1BlockRef{}) { + nextOrigin, err = los.fetch(ctx, currentOrigin.Number+1) if errors.Is(err, ethereum.NotFound) { // We caught up to tip and no longer want to match auto derivation matchAutoDerivation = false diff --git a/op-node/rollup/sequencing/sequencer.go b/op-node/rollup/sequencing/sequencer.go index ddb12df36590a..d655a2ef2625f 100644 --- a/op-node/rollup/sequencing/sequencer.go +++ b/op-node/rollup/sequencing/sequencer.go @@ -504,6 +504,7 @@ func (d *Sequencer) startBuildingBlock() { // Figure out which L1 origin block we're going to be building on top of. l1Origin, err := d.l1OriginSelector.FindL1Origin(ctx, l2Head) switch err { + case nil: case ErrInvalidL1Origin, ErrNextL1OriginOrphaned: d.metrics.RecordSequencerInconsistentL1Origin(l2Head.L1Origin, l1Origin.ID()) d.emitter.Emit(d.ctx, rollup.ResetEvent{ From 581028e8775c3deb1b0053a87c92f9c8f06b23a2 Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 14:53:52 +0000 Subject: [PATCH 23/38] fixes --- op-node/rollup/sequencing/origin_selector.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index fb1ebb52891c4..a706f6c9b7b4c 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -103,7 +103,10 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc nextOriginPtr, matchAutoDerivation) - return *o, err + if err != nil { + return *o, err + } + return eth.BlockRef{}, err } // CurrentAndNextOrigin returns the current cached values for the current L1 origin for the supplied l2Head, and it's successor. From f90208e8f616ec3e866294f8350eb07d91c66c3e Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 14:55:28 +0000 Subject: [PATCH 24/38] lint --- op-acceptance-tests/tests/sequencer/recover_mode_test.go | 3 ++- op-node/rollup/sequencing/origin_selector2_test.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/op-acceptance-tests/tests/sequencer/recover_mode_test.go b/op-acceptance-tests/tests/sequencer/recover_mode_test.go index 548218716ab74..a0f3382e181c9 100644 --- a/op-acceptance-tests/tests/sequencer/recover_mode_test.go +++ b/op-acceptance-tests/tests/sequencer/recover_mode_test.go @@ -26,7 +26,8 @@ func TestRecoverModeWhenChainHealthy(gt *testing.T) { tracer := t.Tracer() ctx := t.Ctx() - sys.L2CL.SetSequencerRecoverMode(true) + err := sys.L2CL.SetSequencerRecoverMode(true) + require.NoError(t, err) blockTime := sys.L2Chain.Escape().RollupConfig().BlockTime numL2Blocks := uint64(20) waitTime := time.Duration(blockTime*numL2Blocks+5) * time.Second diff --git a/op-node/rollup/sequencing/origin_selector2_test.go b/op-node/rollup/sequencing/origin_selector2_test.go index 603f91ebde5e4..f63595682481f 100644 --- a/op-node/rollup/sequencing/origin_selector2_test.go +++ b/op-node/rollup/sequencing/origin_selector2_test.go @@ -16,7 +16,7 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { } require.Panics(t, func() { - FindL1OriginOfNextL2Block( + _, _ = FindL1OriginOfNextL2Block( cfg, nil, nil, From c361b5c9191a5730ff78210f77302cbf6d525606 Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 15:41:34 +0000 Subject: [PATCH 25/38] don't use pointers saves some translation code --- op-node/rollup/sequencing/origin_selector.go | 19 +++-------- .../sequencing/origin_selector2_test.go | 34 +++++++++---------- .../rollup/sequencing/origin_selector_2.go | 21 ++++++------ 3 files changed, 30 insertions(+), 44 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index a706f6c9b7b4c..c2af1b45939a1 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -90,23 +90,12 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc } } - // TODO harmonize how we represent "no data for next origin" - var nextOriginPtr *eth.L1BlockRef - if (nextOrigin != eth.L1BlockRef{}) { - nextOriginPtr = &nextOrigin - } - // defer to pure function - o, err := FindL1OriginOfNextL2Block(los.cfg, - &l2Head, - ¤tOrigin, - nextOriginPtr, + return FindL1OriginOfNextL2Block(los.cfg, + l2Head, + currentOrigin, + nextOrigin, matchAutoDerivation) - - if err != nil { - return *o, err - } - return eth.BlockRef{}, err } // CurrentAndNextOrigin returns the current cached values for the current L1 origin for the supplied l2Head, and it's successor. diff --git a/op-node/rollup/sequencing/origin_selector2_test.go b/op-node/rollup/sequencing/origin_selector2_test.go index f63595682481f..6b09b88d06f64 100644 --- a/op-node/rollup/sequencing/origin_selector2_test.go +++ b/op-node/rollup/sequencing/origin_selector2_test.go @@ -18,19 +18,19 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { require.Panics(t, func() { _, _ = FindL1OriginOfNextL2Block( cfg, - nil, - nil, - nil, + eth.L2BlockRef{}, + eth.L1BlockRef{}, + eth.L1BlockRef{}, false) }) type testCase struct { name string - l2Head *eth.L2BlockRef - currentL1Origin *eth.L1BlockRef - nextL1Origin *eth.L1BlockRef + l2Head eth.L2BlockRef + currentL1Origin eth.L1BlockRef + nextL1Origin eth.L1BlockRef matchAutoderivation bool - expectedResult *eth.L1BlockRef + expectedResult eth.L1BlockRef expectedError error } @@ -42,18 +42,18 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { // L1 chain: a100(1200) <- [ a101(1212) ] // /\ // L2 chain \_ b1000(1220) - a100 := ð.L1BlockRef{ + a100 := eth.L1BlockRef{ Number: 100, Hash: common.Hash{'a', '1', '0', '0'}, Time: 1200, } - a101 := ð.L1BlockRef{ + a101 := eth.L1BlockRef{ Number: 101, ParentHash: a100.Hash, Hash: common.Hash{'a', '0', '0'}, Time: 1212, } - b1000 := ð.L2BlockRef{ + b1000 := eth.L2BlockRef{ Number: 1000, Hash: common.Hash{'b', '1', '0', '0', '0'}, L1Origin: a100.ID(), @@ -80,14 +80,12 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { name: "normal operation, don't need to progress", l2Head: b1000, currentL1Origin: a100, - nextL1Origin: nil, expectedResult: a100, }, testCase{ name: "recover mode, need to progress but can't", l2Head: b1000, currentL1Origin: a100, - nextL1Origin: nil, matchAutoderivation: true, expectedError: ErrNextL1OriginRequired, }, @@ -97,24 +95,24 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { // L1 chain: c100(1200) <-[x]- c101(1212) // /\ // L2 chain \_[x]- d/e1000(1220) - c100 := ð.L1BlockRef{ + c100 := eth.L1BlockRef{ Number: 100, Hash: common.Hash{'a', '1', '0', '0'}, Time: 1200, } - c101 := ð.L1BlockRef{ + c101 := eth.L1BlockRef{ Number: 101, ParentHash: common.Hash{}, // does not point to c100 Hash: common.Hash{'a', '0', '0'}, Time: 1212, } - d1000 := ð.L2BlockRef{ + d1000 := eth.L2BlockRef{ Number: 1000, L1Origin: c100.ID(), Hash: common.Hash{'d', '1', '0', '0', '0'}, Time: 1220, } - e1000 := ð.L2BlockRef{ + e1000 := eth.L2BlockRef{ Number: 1000, L1Origin: eth.BlockID{}, // does not point to c100 Hash: common.Hash{'d', '1', '0', '0', '0'}, @@ -140,7 +138,7 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { // L1 chain: a100(1200) <- [ a101(1212) ] // /\ // L2 chain \_ f1000(3000) - f1000 := ð.L2BlockRef{ + f1000 := eth.L2BlockRef{ Number: 1000, L1Origin: a100.ID(), Hash: common.Hash{'f', '1', '0', '0', '0'}, @@ -167,7 +165,7 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { // L2 chain \_ g1000(1200) // Current drift is 0 // adopting the nextLOrigin would make it negative (add 2 subtract 12) - g1000 := ð.L2BlockRef{ + g1000 := eth.L2BlockRef{ Number: 1000, L1Origin: a100.ID(), Hash: common.Hash{'g', '1', '0', '0', '0'}, diff --git a/op-node/rollup/sequencing/origin_selector_2.go b/op-node/rollup/sequencing/origin_selector_2.go index 135e3a055241f..de424bcec18aa 100644 --- a/op-node/rollup/sequencing/origin_selector_2.go +++ b/op-node/rollup/sequencing/origin_selector_2.go @@ -20,18 +20,18 @@ var ( // removing the need for block building to wait on the result of network calls func FindL1OriginOfNextL2Block( cfg *rollup.Config, - l2Head *eth.L2BlockRef, - currentL1Origin *eth.L1BlockRef, nextL1Origin *eth.L1BlockRef, - matchAutoDerivation bool) (*eth.L1BlockRef, error) { + l2Head eth.L2BlockRef, + currentL1Origin eth.L1BlockRef, nextL1Origin eth.L1BlockRef, + matchAutoDerivation bool) (eth.L1BlockRef, error) { - if currentL1Origin == nil { + if (currentL1Origin == eth.L1BlockRef{}) { panic("origin-selector: currentl1Origin is nil") } if l2Head.L1Origin.Hash != currentL1Origin.Hash { - return nil, ErrInvalidL1Origin + return eth.L1BlockRef{}, ErrInvalidL1Origin } - if nextL1Origin != nil && nextL1Origin.ParentHash != currentL1Origin.Hash { - return nil, ErrNextL1OriginOrphaned + if (nextL1Origin != eth.L1BlockRef{} && nextL1Origin.ParentHash != currentL1Origin.Hash) { + return eth.L1BlockRef{}, ErrNextL1OriginOrphaned } l2BlockTime := cfg.BlockTime @@ -39,16 +39,16 @@ func FindL1OriginOfNextL2Block( nextL2BlockTime := l2Head.Time + l2BlockTime driftCurrent := nextL2BlockTime - currentL1Origin.Time - if nextL1Origin == nil { + if (nextL1Origin == eth.L1BlockRef{}) { if matchAutoDerivation { // This can cause unsafe block production to slow to the rate of L1 block production, if the L1 origin is caught up to the L1 Head. // Code higher up the call stack should ensure that matchAutoDerivation is false under such conditions. - return nil, ErrNextL1OriginRequired + return eth.L1BlockRef{}, ErrNextL1OriginRequired } else { // If we don't yet have the nextL1Origin, stick with the current L1 origin unless doing so would exceed the maximum drift. if driftCurrent > maxDrift { // Return an error so the caller knows it needs to fetch the next l1 origin now. - return nil, fmt.Errorf("%w: drift of next L2 block would exceed maximum %d unless nextl1Origin is adopted", ErrNextL1OriginRequired, maxDrift) + return eth.L1BlockRef{}, fmt.Errorf("%w: drift of next L2 block would exceed maximum %d unless nextl1Origin is adopted", ErrNextL1OriginRequired, maxDrift) } return currentL1Origin, nil } @@ -59,7 +59,6 @@ func FindL1OriginOfNextL2Block( // Progress to l1OriginChild if doing so would respect the requirement // that L2 blocks cannot point to a future L1 block (negative drift). if driftNext >= 0 { - // Adopt it if return nextL1Origin, nil } else { // If we cannot adopt the l1OriginChild, use the current l1 origin. From 55d709bee82022c8963fb249d8ec422696770743 Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 11 Dec 2025 15:53:52 +0000 Subject: [PATCH 26/38] handle retries without a "temporary error" fixes action tests --- op-node/rollup/sequencing/origin_selector.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index c2af1b45939a1..36f93c5f22c24 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -70,6 +70,7 @@ func (los *L1OriginSelector) OnEvent(ctx context.Context, ev event.Event) bool { } // FindL1Origin determines what the L1 Origin for the next L2 Block should be. +// It wraps the FindL1OriginOfNextL2Block function and handles caching and network requests. func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) { // Get cached values for currentOrigin and nextOrigin @@ -91,14 +92,27 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc } // defer to pure function - return FindL1OriginOfNextL2Block(los.cfg, + o, err := FindL1OriginOfNextL2Block(los.cfg, l2Head, currentOrigin, nextOrigin, matchAutoDerivation) + + // If the cache doesn't have the next origin, but we now + // know we definitely need it, fetch it and try again. + if errors.Is(err, ErrNextL1OriginRequired) { + nextOrigin, _ = los.fetch(ctx, currentOrigin.Number+1) + return FindL1OriginOfNextL2Block(los.cfg, + l2Head, + currentOrigin, + nextOrigin, + matchAutoDerivation) + } else { + return o, err + } } -// CurrentAndNextOrigin returns the current cached values for the current L1 origin for the supplied l2Head, and it's successor. +// CurrentAndNextOrigin returns the current cached values for the current L1 origin for the supplied l2Head, and its successor. // It only performs a fetch to L1 if the cache is invalid. // The cache can be update asynchronously by other methods on L1OriginSelector. func (los *L1OriginSelector) CurrentAndNextOrigin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, eth.L1BlockRef, error) { From 69bf84d88c3554d440cc972f091c450506761b91 Mon Sep 17 00:00:00 2001 From: geoknee Date: Fri, 12 Dec 2025 10:08:03 +0000 Subject: [PATCH 27/38] use Fjord drift constant --- op-e2e/actions/proofs/sequence_window_expiry_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/op-e2e/actions/proofs/sequence_window_expiry_test.go b/op-e2e/actions/proofs/sequence_window_expiry_test.go index 6e85163d9b074..b7e26125d103c 100644 --- a/op-e2e/actions/proofs/sequence_window_expiry_test.go +++ b/op-e2e/actions/proofs/sequence_window_expiry_test.go @@ -18,6 +18,7 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { const SEQUENCER_WINDOW_SIZE = 50 // (short, to keep test fast) tp := helpers.NewTestParams(func(p *e2eutils.TestParams) { p.SequencerWindowSize = SEQUENCER_WINDOW_SIZE + p.MaxSequencerDrift = 1800 // use 1800 seconds (30 minutes), which is the protocol constant since Fjord }) // It seems more difficult (almost impossible) to recover from sequencing window expiry with span batches, From 5e7e859ef34c9096b3801f5af014045a2dbf83c5 Mon Sep 17 00:00:00 2001 From: geoknee Date: Fri, 12 Dec 2025 10:26:06 +0000 Subject: [PATCH 28/38] fix origin_selector_test mainly just asserting on sentinel errors, and taking account of small optimization (fewer network calls) --- .../rollup/sequencing/origin_selector_test.go | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector_test.go b/op-node/rollup/sequencing/origin_selector_test.go index 2f2e12f147f01..cb3a55c487b65 100644 --- a/op-node/rollup/sequencing/origin_selector_test.go +++ b/op-node/rollup/sequencing/origin_selector_test.go @@ -207,16 +207,14 @@ func TestOriginSelectorAdvances(t *testing.T) { } if recoverMode { - // In recovery mode (only) we make two RPC calls. + // In recovery mode (only) we make an RPC call to find the next origin. // First, cover the case where the nextOrigin // is not ready yet by simulating a NotFound error. - l1.ExpectL1BlockRefByHash(c.Hash, c, nil) l1.ExpectL1BlockRefByNumber(d.Number, eth.BlockRef{}, ethereum.NotFound) requireL1OriginAt(l2Head, c) // Now, simulate the block being ready, and ensure // that the origin advances to the next block. - l1.ExpectL1BlockRefByHash(c.Hash, c, nil) l1.ExpectL1BlockRefByNumber(d.Number, d, nil) requireL1OriginAt(l2Head, d) } else { @@ -341,7 +339,7 @@ func TestOriginSelectorFetchesNextOrigin(t *testing.T) { // // There are 3 blocks [a, b, c]. After advancing to b, a reorg is simulated // where b is reorged and replaced by providing a `c` next that has a different parent hash. -// The origin should still provide c as the next origin so upstream services can detect the reorg. +// A sentinel error should be returned. func TestOriginSelectorHandlesReorg(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -381,6 +379,11 @@ func TestOriginSelectorHandlesReorg(t *testing.T) { require.Equal(t, l1ref, next) } + requireFindL1OriginError := func(e error) { + _, err := s.FindL1Origin(ctx, l2Head) + require.ErrorIs(t, err, e) + } + requireFindl1OriginEqual(a) // Selection is stable until the next origin is fetched @@ -410,9 +413,8 @@ func TestOriginSelectorHandlesReorg(t *testing.T) { handled = s.OnEvent(context.Background(), engine.ForkchoiceUpdateEvent{UnsafeL2Head: l2Head}) require.True(t, handled) - // The next origin should be `c` now, otherwise an upstream service cannot detect the reorg - // and the origin will be stuck at `b` - requireFindl1OriginEqual(c) + // We shuold get a sentinel error + requireFindL1OriginError(ErrNextL1OriginOrphaned) } // TestOriginSelectorRespectsOriginTiming ensures that the origin selector @@ -590,7 +592,7 @@ func TestOriginSelectorStrictConfDepth(t *testing.T) { s := NewL1OriginSelector(ctx, log, cfg, confDepthL1) _, err := s.FindL1Origin(ctx, l2Head) - require.ErrorContains(t, err, "sequencer time drift") + require.ErrorIs(t, err, ErrNextL1OriginRequired) } func u64ptr(n uint64) *uint64 { @@ -776,11 +778,11 @@ func TestOriginSelectorHandlesLateL1Blocks(t *testing.T) { s := NewL1OriginSelector(ctx, log, cfg, confDepthL1) _, err := s.FindL1Origin(ctx, l2Head) - require.ErrorContains(t, err, "sequencer time drift") + require.ErrorIs(t, err, ErrNextL1OriginRequired) l1Head = c _, err = s.FindL1Origin(ctx, l2Head) - require.ErrorContains(t, err, "sequencer time drift") + require.ErrorIs(t, err, ErrNextL1OriginRequired) l1Head = d next, err := s.FindL1Origin(ctx, l2Head) From b04f62b04f2be590379aa948f2d4b9b1be8fced7 Mon Sep 17 00:00:00 2001 From: geoknee Date: Fri, 12 Dec 2025 15:14:42 +0000 Subject: [PATCH 29/38] Simplify FindL1Origin --- op-node/rollup/sequencing/origin_selector.go | 42 ++++++++------------ 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index 36f93c5f22c24..d5d0a164d60d5 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -72,49 +72,41 @@ func (los *L1OriginSelector) OnEvent(ctx context.Context, ev event.Event) bool { // FindL1Origin determines what the L1 Origin for the next L2 Block should be. // It wraps the FindL1OriginOfNextL2Block function and handles caching and network requests. func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) { - // Get cached values for currentOrigin and nextOrigin currentOrigin, nextOrigin, err := los.CurrentAndNextOrigin(ctx, l2Head) if err != nil { return eth.L1BlockRef{}, err } - - // If in recover mode, get next origin synchronously - matchAutoDerivation := los.recoverMode.Load() - if (matchAutoDerivation && nextOrigin == eth.L1BlockRef{}) { - nextOrigin, err = los.fetch(ctx, currentOrigin.Number+1) - if errors.Is(err, ethereum.NotFound) { - // We caught up to tip and no longer want to match auto derivation - matchAutoDerivation = false - } else if err != nil { - return eth.L1BlockRef{}, err - } - } - - // defer to pure function + // Try to find the L1 origin given the current data in cache o, err := FindL1OriginOfNextL2Block(los.cfg, l2Head, currentOrigin, nextOrigin, - matchAutoDerivation) + los.recoverMode.Load()) // If the cache doesn't have the next origin, but we now // know we definitely need it, fetch it and try again. if errors.Is(err, ErrNextL1OriginRequired) { - nextOrigin, _ = los.fetch(ctx, currentOrigin.Number+1) - return FindL1OriginOfNextL2Block(los.cfg, - l2Head, - currentOrigin, - nextOrigin, - matchAutoDerivation) - } else { - return o, err + nextOrigin, err = los.fetch(ctx, currentOrigin.Number+1) + if err == nil || (los.recoverMode.Load() && errors.Is(err, ethereum.NotFound)) { + // If we got the origin, or we are in recover mode and the origin is not found + // (because we recovered the l1 origin up to the l1 tip) + // try again with matchAutoDerivation = false. + return FindL1OriginOfNextL2Block(los.cfg, + l2Head, + currentOrigin, + nextOrigin, + false) + } else { + return eth.L1BlockRef{}, ErrNextL1OriginRequired + } } + return o, err } // CurrentAndNextOrigin returns the current cached values for the current L1 origin for the supplied l2Head, and its successor. // It only performs a fetch to L1 if the cache is invalid. -// The cache can be update asynchronously by other methods on L1OriginSelector. +// The cache can be updated asynchronously by other methods on L1OriginSelector. func (los *L1OriginSelector) CurrentAndNextOrigin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, eth.L1BlockRef, error) { los.mu.Lock() defer los.mu.Unlock() From 08dd4d3332e9d549c505e12a6b465e400565b5ef Mon Sep 17 00:00:00 2001 From: geoknee Date: Fri, 12 Dec 2025 15:39:47 +0000 Subject: [PATCH 30/38] move new pure function into a method on los --- op-node/rollup/sequencing/origin_selector.go | 65 +++++- .../sequencing/origin_selector2_test.go | 201 ------------------ .../rollup/sequencing/origin_selector_2.go | 67 ------ .../rollup/sequencing/origin_selector_test.go | 191 +++++++++++++++++ 4 files changed, 254 insertions(+), 270 deletions(-) delete mode 100644 op-node/rollup/sequencing/origin_selector2_test.go delete mode 100644 op-node/rollup/sequencing/origin_selector_2.go diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index d5d0a164d60d5..b7963bdc3c394 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -3,6 +3,7 @@ package sequencing import ( "context" "errors" + "fmt" "sync" "sync/atomic" "time" @@ -78,7 +79,7 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc return eth.L1BlockRef{}, err } // Try to find the L1 origin given the current data in cache - o, err := FindL1OriginOfNextL2Block(los.cfg, + o, err := los.findL1OriginOfNextL2Block( l2Head, currentOrigin, nextOrigin, @@ -92,7 +93,7 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc // If we got the origin, or we are in recover mode and the origin is not found // (because we recovered the l1 origin up to the l1 tip) // try again with matchAutoDerivation = false. - return FindL1OriginOfNextL2Block(los.cfg, + return los.findL1OriginOfNextL2Block( l2Head, currentOrigin, nextOrigin, @@ -205,3 +206,63 @@ func (los *L1OriginSelector) reset() { los.currentOrigin = eth.L1BlockRef{} los.nextOrigin = eth.L1BlockRef{} } + +var ( + ErrInvalidL1Origin = fmt.Errorf("origin-selector: currentL1Origin.Hash != l2Head.L1Origin.Hash") + ErrNextL1OriginOrphaned = fmt.Errorf("origin-selector: nextL1Origin.ParentHash != currentL1Origin.Hash") + ErrNextL1OriginRequired = fmt.Errorf("origin-selector: nextL1Origin not supplied but required to satisfy constraints") +) + +// FindL1OriginOfNextL2Block finds the L1 origin of the next L2 block. +// It returns an error if there is no way to build a block satisfying +// derivation constraints with the supplied data. +// You can pass a nil pointer for l1OriginChild if it is not yet available +// removing the need for block building to wait on the result of network calls +// This method is designed to be pure (it only reads the cfg property of the receiver) +// and should not have any side effects. +func (los *L1OriginSelector) findL1OriginOfNextL2Block( + l2Head eth.L2BlockRef, + currentL1Origin eth.L1BlockRef, nextL1Origin eth.L1BlockRef, + matchAutoDerivation bool) (eth.L1BlockRef, error) { + + if (currentL1Origin == eth.L1BlockRef{}) { + panic("origin-selector: currentl1Origin is nil") + } + if l2Head.L1Origin.Hash != currentL1Origin.Hash { + return eth.L1BlockRef{}, ErrInvalidL1Origin + } + if (nextL1Origin != eth.L1BlockRef{} && nextL1Origin.ParentHash != currentL1Origin.Hash) { + return eth.L1BlockRef{}, ErrNextL1OriginOrphaned + } + + l2BlockTime := los.cfg.BlockTime + maxDrift := rollup.NewChainSpec(los.cfg).MaxSequencerDrift(currentL1Origin.Time) + nextL2BlockTime := l2Head.Time + l2BlockTime + driftCurrent := nextL2BlockTime - currentL1Origin.Time + + if (nextL1Origin == eth.L1BlockRef{}) { + if matchAutoDerivation { + // This can cause unsafe block production to slow to the rate of L1 block production, if the L1 origin is caught up to the L1 Head. + // Code higher up the call stack should ensure that matchAutoDerivation is false under such conditions. + return eth.L1BlockRef{}, ErrNextL1OriginRequired + } else { + // If we don't yet have the nextL1Origin, stick with the current L1 origin unless doing so would exceed the maximum drift. + if driftCurrent > maxDrift { + // Return an error so the caller knows it needs to fetch the next l1 origin now. + return eth.L1BlockRef{}, fmt.Errorf("%w: drift of next L2 block would exceed maximum %d unless nextl1Origin is adopted", ErrNextL1OriginRequired, maxDrift) + } + return currentL1Origin, nil + } + } + + driftNext := int64(nextL2BlockTime) - int64(nextL1Origin.Time) + + // Progress to l1OriginChild if doing so would respect the requirement + // that L2 blocks cannot point to a future L1 block (negative drift). + if driftNext >= 0 { + return nextL1Origin, nil + } else { + // If we cannot adopt the l1OriginChild, use the current l1 origin. + return currentL1Origin, nil + } +} diff --git a/op-node/rollup/sequencing/origin_selector2_test.go b/op-node/rollup/sequencing/origin_selector2_test.go deleted file mode 100644 index 6b09b88d06f64..0000000000000 --- a/op-node/rollup/sequencing/origin_selector2_test.go +++ /dev/null @@ -1,201 +0,0 @@ -package sequencing - -import ( - "testing" - - "github.com/ethereum-optimism/optimism/op-node/rollup" - "github.com/ethereum-optimism/optimism/op-service/eth" - "github.com/ethereum/go-ethereum/common" - "github.com/stretchr/testify/require" -) - -func TestFindL1OriginOfNextL2Block(t *testing.T) { - cfg := &rollup.Config{ - MaxSequencerDrift: 1800, // Use Fjord constant value - BlockTime: 2, - } - - require.Panics(t, func() { - _, _ = FindL1OriginOfNextL2Block( - cfg, - eth.L2BlockRef{}, - eth.L1BlockRef{}, - eth.L1BlockRef{}, - false) - }) - - type testCase struct { - name string - l2Head eth.L2BlockRef - currentL1Origin eth.L1BlockRef - nextL1Origin eth.L1BlockRef - matchAutoderivation bool - expectedResult eth.L1BlockRef - expectedError error - } - - tcs := []testCase{} - - // Scenarios with valid data, no drift concerns - // but availability of next l1 origin is modulated. - // - // L1 chain: a100(1200) <- [ a101(1212) ] - // /\ - // L2 chain \_ b1000(1220) - a100 := eth.L1BlockRef{ - Number: 100, - Hash: common.Hash{'a', '1', '0', '0'}, - Time: 1200, - } - a101 := eth.L1BlockRef{ - Number: 101, - ParentHash: a100.Hash, - Hash: common.Hash{'a', '0', '0'}, - Time: 1212, - } - b1000 := eth.L2BlockRef{ - Number: 1000, - Hash: common.Hash{'b', '1', '0', '0', '0'}, - L1Origin: a100.ID(), - Time: 1220, - } - - tcs = append(tcs, - testCase{ - name: "normal operation, progress because we can", - l2Head: b1000, - currentL1Origin: a100, - nextL1Origin: a101, - expectedResult: a101, - }, - testCase{ - name: "recover mode, progress because we can", - l2Head: b1000, - currentL1Origin: a100, - nextL1Origin: a101, - expectedResult: a101, - matchAutoderivation: true, - }, - testCase{ - name: "normal operation, don't need to progress", - l2Head: b1000, - currentL1Origin: a100, - expectedResult: a100, - }, - testCase{ - name: "recover mode, need to progress but can't", - l2Head: b1000, - currentL1Origin: a100, - matchAutoderivation: true, - expectedError: ErrNextL1OriginRequired, - }, - ) - - // Bad input data / reorg scenarios - // L1 chain: c100(1200) <-[x]- c101(1212) - // /\ - // L2 chain \_[x]- d/e1000(1220) - c100 := eth.L1BlockRef{ - Number: 100, - Hash: common.Hash{'a', '1', '0', '0'}, - Time: 1200, - } - c101 := eth.L1BlockRef{ - Number: 101, - ParentHash: common.Hash{}, // does not point to c100 - Hash: common.Hash{'a', '0', '0'}, - Time: 1212, - } - d1000 := eth.L2BlockRef{ - Number: 1000, - L1Origin: c100.ID(), - Hash: common.Hash{'d', '1', '0', '0', '0'}, - Time: 1220, - } - e1000 := eth.L2BlockRef{ - Number: 1000, - L1Origin: eth.BlockID{}, // does not point to c100 - Hash: common.Hash{'d', '1', '0', '0', '0'}, - Time: 1220, - } - tcs = append(tcs, - testCase{ - name: "L1 reorg", - currentL1Origin: c100, - nextL1Origin: c101, - l2Head: d1000, - expectedError: ErrNextL1OriginOrphaned, - }, - testCase{ - name: "Invalid l1 origin", - currentL1Origin: c100, - nextL1Origin: c101, - l2Head: e1000, - expectedError: ErrInvalidL1Origin, - }) - - // Drift at maximum, - // L1 chain: a100(1200) <- [ a101(1212) ] - // /\ - // L2 chain \_ f1000(3000) - f1000 := eth.L2BlockRef{ - Number: 1000, - L1Origin: a100.ID(), - Hash: common.Hash{'f', '1', '0', '0', '0'}, - Time: 3000, - } - tcs = append(tcs, - testCase{ - name: "Drift at maximum, nextL1Origin available", - currentL1Origin: a100, - nextL1Origin: a101, - l2Head: f1000, - expectedResult: a101, - }, - testCase{ - name: "Drift at maximum, nextL1Origin unavailable", - currentL1Origin: a100, - l2Head: f1000, - expectedError: ErrNextL1OriginRequired, - }) - - // Negative drift, - // L1 chain: a100(1200) <- a101(1212) - // /\ - // L2 chain \_ g1000(1200) - // Current drift is 0 - // adopting the nextLOrigin would make it negative (add 2 subtract 12) - g1000 := eth.L2BlockRef{ - Number: 1000, - L1Origin: a100.ID(), - Hash: common.Hash{'g', '1', '0', '0', '0'}, - Time: 1200, - } - tcs = append(tcs, - testCase{ - name: "Negative drift", - currentL1Origin: a100, - nextL1Origin: a101, - l2Head: g1000, - expectedResult: a100, - }) - - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - result, err := FindL1OriginOfNextL2Block( - cfg, - tc.l2Head, - tc.currentL1Origin, - tc.nextL1Origin, - tc.matchAutoderivation) - if tc.expectedError != nil { - require.ErrorIs(t, err, tc.expectedError) - } else { - require.NoError(t, err) - } - if result != tc.expectedResult { - t.Errorf("expected result %v, got %v", tc.expectedResult, result) - } - }) - } -} diff --git a/op-node/rollup/sequencing/origin_selector_2.go b/op-node/rollup/sequencing/origin_selector_2.go deleted file mode 100644 index de424bcec18aa..0000000000000 --- a/op-node/rollup/sequencing/origin_selector_2.go +++ /dev/null @@ -1,67 +0,0 @@ -package sequencing - -import ( - "fmt" - - "github.com/ethereum-optimism/optimism/op-node/rollup" - "github.com/ethereum-optimism/optimism/op-service/eth" -) - -var ( - ErrInvalidL1Origin = fmt.Errorf("origin-selector: currentL1Origin.Hash != l2Head.L1Origin.Hash") - ErrNextL1OriginOrphaned = fmt.Errorf("origin-selector: nextL1Origin.ParentHash != currentL1Origin.Hash") - ErrNextL1OriginRequired = fmt.Errorf("origin-selector: nextL1Origin not supplied but required to satisfy constraints") -) - -// FindL1OriginOfNextL2Block finds the L1 origin of the next L2 block. -// It returns an error if there is no way to build a block satisfying -// derivation constraints with the supplied data. -// You can pass a nil pointer for l1OriginChild if it is not yet available -// removing the need for block building to wait on the result of network calls -func FindL1OriginOfNextL2Block( - cfg *rollup.Config, - l2Head eth.L2BlockRef, - currentL1Origin eth.L1BlockRef, nextL1Origin eth.L1BlockRef, - matchAutoDerivation bool) (eth.L1BlockRef, error) { - - if (currentL1Origin == eth.L1BlockRef{}) { - panic("origin-selector: currentl1Origin is nil") - } - if l2Head.L1Origin.Hash != currentL1Origin.Hash { - return eth.L1BlockRef{}, ErrInvalidL1Origin - } - if (nextL1Origin != eth.L1BlockRef{} && nextL1Origin.ParentHash != currentL1Origin.Hash) { - return eth.L1BlockRef{}, ErrNextL1OriginOrphaned - } - - l2BlockTime := cfg.BlockTime - maxDrift := rollup.NewChainSpec(cfg).MaxSequencerDrift(currentL1Origin.Time) - nextL2BlockTime := l2Head.Time + l2BlockTime - driftCurrent := nextL2BlockTime - currentL1Origin.Time - - if (nextL1Origin == eth.L1BlockRef{}) { - if matchAutoDerivation { - // This can cause unsafe block production to slow to the rate of L1 block production, if the L1 origin is caught up to the L1 Head. - // Code higher up the call stack should ensure that matchAutoDerivation is false under such conditions. - return eth.L1BlockRef{}, ErrNextL1OriginRequired - } else { - // If we don't yet have the nextL1Origin, stick with the current L1 origin unless doing so would exceed the maximum drift. - if driftCurrent > maxDrift { - // Return an error so the caller knows it needs to fetch the next l1 origin now. - return eth.L1BlockRef{}, fmt.Errorf("%w: drift of next L2 block would exceed maximum %d unless nextl1Origin is adopted", ErrNextL1OriginRequired, maxDrift) - } - return currentL1Origin, nil - } - } - - driftNext := int64(nextL2BlockTime) - int64(nextL1Origin.Time) - - // Progress to l1OriginChild if doing so would respect the requirement - // that L2 blocks cannot point to a future L1 block (negative drift). - if driftNext >= 0 { - return nextL1Origin, nil - } else { - // If we cannot adopt the l1OriginChild, use the current l1 origin. - return currentL1Origin, nil - } -} diff --git a/op-node/rollup/sequencing/origin_selector_test.go b/op-node/rollup/sequencing/origin_selector_test.go index cb3a55c487b65..325682678fba2 100644 --- a/op-node/rollup/sequencing/origin_selector_test.go +++ b/op-node/rollup/sequencing/origin_selector_test.go @@ -810,3 +810,194 @@ func TestOriginSelectorMiscEvent(t *testing.T) { handled := s.OnEvent(context.Background(), rollup.L1TemporaryErrorEvent{}) require.False(t, handled) } + +func TestFindL1OriginOfNextL2Block(t *testing.T) { + cfg := &rollup.Config{ + MaxSequencerDrift: 1800, // Use Fjord constant value + BlockTime: 2, + } + + los := NewL1OriginSelector(context.Background(), testlog.Logger(t, log.LevelDebug), cfg, &testutils.MockL1Source{}) + + require.Panics(t, func() { + _, _ = los.findL1OriginOfNextL2Block( + eth.L2BlockRef{}, + eth.L1BlockRef{}, + eth.L1BlockRef{}, + false) + }) + + type testCase struct { + name string + l2Head eth.L2BlockRef + currentL1Origin eth.L1BlockRef + nextL1Origin eth.L1BlockRef + matchAutoderivation bool + expectedResult eth.L1BlockRef + expectedError error + } + + tcs := []testCase{} + + // Scenarios with valid data, no drift concerns + // but availability of next l1 origin is modulated. + // + // L1 chain: a100(1200) <- [ a101(1212) ] + // /\ + // L2 chain \_ b1000(1220) + a100 := eth.L1BlockRef{ + Number: 100, + Hash: common.Hash{'a', '1', '0', '0'}, + Time: 1200, + } + a101 := eth.L1BlockRef{ + Number: 101, + ParentHash: a100.Hash, + Hash: common.Hash{'a', '0', '0'}, + Time: 1212, + } + b1000 := eth.L2BlockRef{ + Number: 1000, + Hash: common.Hash{'b', '1', '0', '0', '0'}, + L1Origin: a100.ID(), + Time: 1220, + } + + tcs = append(tcs, + testCase{ + name: "normal operation, progress because we can", + l2Head: b1000, + currentL1Origin: a100, + nextL1Origin: a101, + expectedResult: a101, + }, + testCase{ + name: "recover mode, progress because we can", + l2Head: b1000, + currentL1Origin: a100, + nextL1Origin: a101, + expectedResult: a101, + matchAutoderivation: true, + }, + testCase{ + name: "normal operation, don't need to progress", + l2Head: b1000, + currentL1Origin: a100, + expectedResult: a100, + }, + testCase{ + name: "recover mode, need to progress but can't", + l2Head: b1000, + currentL1Origin: a100, + matchAutoderivation: true, + expectedError: ErrNextL1OriginRequired, + }, + ) + + // Bad input data / reorg scenarios + // L1 chain: c100(1200) <-[x]- c101(1212) + // /\ + // L2 chain \_[x]- d/e1000(1220) + c100 := eth.L1BlockRef{ + Number: 100, + Hash: common.Hash{'a', '1', '0', '0'}, + Time: 1200, + } + c101 := eth.L1BlockRef{ + Number: 101, + ParentHash: common.Hash{}, // does not point to c100 + Hash: common.Hash{'a', '0', '0'}, + Time: 1212, + } + d1000 := eth.L2BlockRef{ + Number: 1000, + L1Origin: c100.ID(), + Hash: common.Hash{'d', '1', '0', '0', '0'}, + Time: 1220, + } + e1000 := eth.L2BlockRef{ + Number: 1000, + L1Origin: eth.BlockID{}, // does not point to c100 + Hash: common.Hash{'d', '1', '0', '0', '0'}, + Time: 1220, + } + tcs = append(tcs, + testCase{ + name: "L1 reorg", + currentL1Origin: c100, + nextL1Origin: c101, + l2Head: d1000, + expectedError: ErrNextL1OriginOrphaned, + }, + testCase{ + name: "Invalid l1 origin", + currentL1Origin: c100, + nextL1Origin: c101, + l2Head: e1000, + expectedError: ErrInvalidL1Origin, + }) + + // Drift at maximum, + // L1 chain: a100(1200) <- [ a101(1212) ] + // /\ + // L2 chain \_ f1000(3000) + f1000 := eth.L2BlockRef{ + Number: 1000, + L1Origin: a100.ID(), + Hash: common.Hash{'f', '1', '0', '0', '0'}, + Time: 3000, + } + tcs = append(tcs, + testCase{ + name: "Drift at maximum, nextL1Origin available", + currentL1Origin: a100, + nextL1Origin: a101, + l2Head: f1000, + expectedResult: a101, + }, + testCase{ + name: "Drift at maximum, nextL1Origin unavailable", + currentL1Origin: a100, + l2Head: f1000, + expectedError: ErrNextL1OriginRequired, + }) + + // Negative drift, + // L1 chain: a100(1200) <- a101(1212) + // /\ + // L2 chain \_ g1000(1200) + // Current drift is 0 + // adopting the nextLOrigin would make it negative (add 2 subtract 12) + g1000 := eth.L2BlockRef{ + Number: 1000, + L1Origin: a100.ID(), + Hash: common.Hash{'g', '1', '0', '0', '0'}, + Time: 1200, + } + tcs = append(tcs, + testCase{ + name: "Negative drift", + currentL1Origin: a100, + nextL1Origin: a101, + l2Head: g1000, + expectedResult: a100, + }) + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + result, err := los.findL1OriginOfNextL2Block( + tc.l2Head, + tc.currentL1Origin, + tc.nextL1Origin, + tc.matchAutoderivation) + if tc.expectedError != nil { + require.ErrorIs(t, err, tc.expectedError) + } else { + require.NoError(t, err) + } + if result != tc.expectedResult { + t.Errorf("expected result %v, got %v", tc.expectedResult, result) + } + }) + } +} From 204f4a15d003fc2d3f20b72a9e987d020ced4ba2 Mon Sep 17 00:00:00 2001 From: geoknee Date: Mon, 15 Dec 2025 09:19:01 +0000 Subject: [PATCH 31/38] Update comment to refer to empty nextL1Origin --- op-node/rollup/sequencing/origin_selector.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index b7963bdc3c394..92d059bfea645 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -216,7 +216,7 @@ var ( // FindL1OriginOfNextL2Block finds the L1 origin of the next L2 block. // It returns an error if there is no way to build a block satisfying // derivation constraints with the supplied data. -// You can pass a nil pointer for l1OriginChild if it is not yet available +// You can pass an empty nextL1Origin if it is not yet available // removing the need for block building to wait on the result of network calls // This method is designed to be pure (it only reads the cfg property of the receiver) // and should not have any side effects. @@ -226,7 +226,7 @@ func (los *L1OriginSelector) findL1OriginOfNextL2Block( matchAutoDerivation bool) (eth.L1BlockRef, error) { if (currentL1Origin == eth.L1BlockRef{}) { - panic("origin-selector: currentl1Origin is nil") + panic("origin-selector: currentL1Origin is empty") } if l2Head.L1Origin.Hash != currentL1Origin.Hash { return eth.L1BlockRef{}, ErrInvalidL1Origin From 057e394bf970b46e9668ab5103099879b3d0b36e Mon Sep 17 00:00:00 2001 From: geoknee Date: Mon, 15 Dec 2025 09:22:07 +0000 Subject: [PATCH 32/38] Use errors.Is for L1 origin error checks --- op-node/rollup/sequencing/sequencer.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/op-node/rollup/sequencing/sequencer.go b/op-node/rollup/sequencing/sequencer.go index d655a2ef2625f..76e743af1dd46 100644 --- a/op-node/rollup/sequencing/sequencer.go +++ b/op-node/rollup/sequencing/sequencer.go @@ -503,16 +503,16 @@ func (d *Sequencer) startBuildingBlock() { // Figure out which L1 origin block we're going to be building on top of. l1Origin, err := d.l1OriginSelector.FindL1Origin(ctx, l2Head) - switch err { - case nil: - case ErrInvalidL1Origin, ErrNextL1OriginOrphaned: + switch { + case err == nil: + case errors.Is(err, ErrInvalidL1Origin), errors.Is(err, ErrNextL1OriginOrphaned): d.metrics.RecordSequencerInconsistentL1Origin(l2Head.L1Origin, l1Origin.ID()) d.emitter.Emit(d.ctx, rollup.ResetEvent{ Err: fmt.Errorf("cannot build new L2 block with L1 origin %s (parent L1 %s) on current L2 head %s with L1 origin %s", l1Origin, l1Origin.ParentHash, l2Head, l2Head.L1Origin), }) return - case ErrNextL1OriginRequired: + case errors.Is(err, ErrNextL1OriginRequired): fallthrough default: d.nextAction = d.timeNow().Add(time.Second) From 099e1b33e19bc7a1aefdfae569c2f964564ddc3f Mon Sep 17 00:00:00 2001 From: geoknee Date: Mon, 15 Dec 2025 09:41:15 +0000 Subject: [PATCH 33/38] Return L1 origin on validation errors --- op-node/rollup/sequencing/origin_selector.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index 92d059bfea645..fa32e62e38820 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -229,10 +229,10 @@ func (los *L1OriginSelector) findL1OriginOfNextL2Block( panic("origin-selector: currentL1Origin is empty") } if l2Head.L1Origin.Hash != currentL1Origin.Hash { - return eth.L1BlockRef{}, ErrInvalidL1Origin + return currentL1Origin, ErrInvalidL1Origin } if (nextL1Origin != eth.L1BlockRef{} && nextL1Origin.ParentHash != currentL1Origin.Hash) { - return eth.L1BlockRef{}, ErrNextL1OriginOrphaned + return nextL1Origin, ErrNextL1OriginOrphaned } l2BlockTime := los.cfg.BlockTime From ce9fa62d0c0325304fc37d91d87aa2e16a7f8356 Mon Sep 17 00:00:00 2001 From: geoknee Date: Mon, 15 Dec 2025 09:54:47 +0000 Subject: [PATCH 34/38] Add expectedResult to origin selector tests Set expectedResult for the test cases with l2Head d1000 and e1000 to assert the expected L1 origins (c101 and c100 respectively) --- op-node/rollup/sequencing/origin_selector_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/op-node/rollup/sequencing/origin_selector_test.go b/op-node/rollup/sequencing/origin_selector_test.go index 325682678fba2..5ab947df69fed 100644 --- a/op-node/rollup/sequencing/origin_selector_test.go +++ b/op-node/rollup/sequencing/origin_selector_test.go @@ -927,6 +927,7 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { currentL1Origin: c100, nextL1Origin: c101, l2Head: d1000, + expectedResult: c101, expectedError: ErrNextL1OriginOrphaned, }, testCase{ @@ -934,6 +935,7 @@ func TestFindL1OriginOfNextL2Block(t *testing.T) { currentL1Origin: c100, nextL1Origin: c101, l2Head: e1000, + expectedResult: c100, expectedError: ErrInvalidL1Origin, }) From 389f2b6bc403ba58288f48ae14e37a8bec55846e Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 18 Dec 2025 10:54:07 +0000 Subject: [PATCH 35/38] Add assertion message and clarify origin comments Provide a helpful failure message in sequence_window_expiry_test when the safe head doesn't advance after the sequencing window expires. Document that the current L1 origin should always be non-empty and add a panic guard. Clarify the rationale for requiring the next L1 origin, include a source link, and note the effect on unsafe block production. --- op-e2e/actions/proofs/sequence_window_expiry_test.go | 3 ++- op-node/rollup/sequencing/origin_selector.go | 10 +++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/op-e2e/actions/proofs/sequence_window_expiry_test.go b/op-e2e/actions/proofs/sequence_window_expiry_test.go index b7e26125d103c..cd614afb97abc 100644 --- a/op-e2e/actions/proofs/sequence_window_expiry_test.go +++ b/op-e2e/actions/proofs/sequence_window_expiry_test.go @@ -63,7 +63,8 @@ func runSequenceWindowExpireTest(gt *testing.T, testCfg *helpers.TestCfg[any]) { // Ensure the safe head advanced forcefully. l2SafeHead = env.Engine.L2Chain().CurrentSafeBlock() - require.Greater(t, l2SafeHead.Number.Uint64(), uint64(0)) + require.Greater(t, l2SafeHead.Number.Uint64(), uint64(0), + "The safe head failed to progress after the sequencing window expired (expected deposit-only blocks to be derived).") env.RunFaultProofProgram(t, l2SafeHead.Number.Uint64()/2, testCfg.CheckResult, testCfg.InputParams...) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index fa32e62e38820..d8681709eb84e 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -108,6 +108,8 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc // CurrentAndNextOrigin returns the current cached values for the current L1 origin for the supplied l2Head, and its successor. // It only performs a fetch to L1 if the cache is invalid. // The cache can be updated asynchronously by other methods on L1OriginSelector. +// The returned currentOrigin should _always_ be non-empty, because it is populated from l2Head whose +// l1Origin is first specified in the rollup.Config.Genesis.L1 and progressed to non-empty values thereafter. func (los *L1OriginSelector) CurrentAndNextOrigin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, eth.L1BlockRef, error) { los.mu.Lock() defer los.mu.Unlock() @@ -226,6 +228,10 @@ func (los *L1OriginSelector) findL1OriginOfNextL2Block( matchAutoDerivation bool) (eth.L1BlockRef, error) { if (currentL1Origin == eth.L1BlockRef{}) { + // This would indicate a programming error, since the currentL1Origin + // should _always_ be available. + // The first value (for block 1) is specified in rollup.Config.Genesis.L1 + // and it is then only updated to non-empty values. panic("origin-selector: currentL1Origin is empty") } if l2Head.L1Origin.Hash != currentL1Origin.Hash { @@ -242,7 +248,9 @@ func (los *L1OriginSelector) findL1OriginOfNextL2Block( if (nextL1Origin == eth.L1BlockRef{}) { if matchAutoDerivation { - // This can cause unsafe block production to slow to the rate of L1 block production, if the L1 origin is caught up to the L1 Head. + // See https://github.com/ethereum-optimism/optimism/blob/ce9fa62d0c0325304fc37d91d87aa2e16a7f8356/op-node/rollup/derive/base_batch_stage.go#L186-L205 + // We need the next L1 origin to decide whether we can eagerly adopt it. + // NOTE: This can cause unsafe block production to slow to the rate of L1 block production, if the L1 origin is caught up to the L1 Head. // Code higher up the call stack should ensure that matchAutoDerivation is false under such conditions. return eth.L1BlockRef{}, ErrNextL1OriginRequired } else { From 4e76749c5f5c79135095b162772c35e1ded4b3f3 Mon Sep 17 00:00:00 2001 From: geoknee Date: Thu, 18 Dec 2025 11:16:16 +0000 Subject: [PATCH 36/38] Store recoverMode and add comment period --- op-node/rollup/sequencing/origin_selector.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index d8681709eb84e..eb06c8302e586 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -73,6 +73,7 @@ func (los *L1OriginSelector) OnEvent(ctx context.Context, ev event.Event) bool { // FindL1Origin determines what the L1 Origin for the next L2 Block should be. // It wraps the FindL1OriginOfNextL2Block function and handles caching and network requests. func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) { + recoverMode := los.recoverMode.Load() // Get cached values for currentOrigin and nextOrigin currentOrigin, nextOrigin, err := los.CurrentAndNextOrigin(ctx, l2Head) if err != nil { @@ -83,13 +84,13 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2Bloc l2Head, currentOrigin, nextOrigin, - los.recoverMode.Load()) + recoverMode) // If the cache doesn't have the next origin, but we now // know we definitely need it, fetch it and try again. if errors.Is(err, ErrNextL1OriginRequired) { nextOrigin, err = los.fetch(ctx, currentOrigin.Number+1) - if err == nil || (los.recoverMode.Load() && errors.Is(err, ethereum.NotFound)) { + if err == nil || (recoverMode && errors.Is(err, ethereum.NotFound)) { // If we got the origin, or we are in recover mode and the origin is not found // (because we recovered the l1 origin up to the l1 tip) // try again with matchAutoDerivation = false. @@ -219,7 +220,7 @@ var ( // It returns an error if there is no way to build a block satisfying // derivation constraints with the supplied data. // You can pass an empty nextL1Origin if it is not yet available -// removing the need for block building to wait on the result of network calls +// removing the need for block building to wait on the result of network calls. // This method is designed to be pure (it only reads the cfg property of the receiver) // and should not have any side effects. func (los *L1OriginSelector) findL1OriginOfNextL2Block( From fd8d7fad2218577963437550db6206ecbf349dc7 Mon Sep 17 00:00:00 2001 From: George Knee Date: Thu, 18 Dec 2025 14:34:24 +0000 Subject: [PATCH 37/38] Update op-node/rollup/sequencing/origin_selector.go Co-authored-by: almanax-ai[bot] <174396398+almanax-ai[bot]@users.noreply.github.com> --- op-node/rollup/sequencing/origin_selector.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index eb06c8302e586..cbb5426031677 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -245,7 +245,7 @@ func (los *L1OriginSelector) findL1OriginOfNextL2Block( l2BlockTime := los.cfg.BlockTime maxDrift := rollup.NewChainSpec(los.cfg).MaxSequencerDrift(currentL1Origin.Time) nextL2BlockTime := l2Head.Time + l2BlockTime - driftCurrent := nextL2BlockTime - currentL1Origin.Time + driftCurrent := int64(nextL2BlockTime) - int64(currentL1Origin.Time) if (nextL1Origin == eth.L1BlockRef{}) { if matchAutoDerivation { From 420c249fd79d6f763291abf80723fe8484c6363d Mon Sep 17 00:00:00 2001 From: George Knee Date: Thu, 18 Dec 2025 14:56:01 +0000 Subject: [PATCH 38/38] Update op-node/rollup/sequencing/origin_selector.go Co-authored-by: almanax-ai[bot] <174396398+almanax-ai[bot]@users.noreply.github.com> --- op-node/rollup/sequencing/origin_selector.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index cbb5426031677..368226db34e26 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -256,7 +256,7 @@ func (los *L1OriginSelector) findL1OriginOfNextL2Block( return eth.L1BlockRef{}, ErrNextL1OriginRequired } else { // If we don't yet have the nextL1Origin, stick with the current L1 origin unless doing so would exceed the maximum drift. - if driftCurrent > maxDrift { + if driftCurrent > int64(maxDrift) { // Return an error so the caller knows it needs to fetch the next l1 origin now. return eth.L1BlockRef{}, fmt.Errorf("%w: drift of next L2 block would exceed maximum %d unless nextl1Origin is adopted", ErrNextL1OriginRequired, maxDrift) }