From b2769e655678559fda856c869a202be913e39458 Mon Sep 17 00:00:00 2001 From: Paul Lange Date: Thu, 1 Aug 2024 13:22:57 +0200 Subject: [PATCH 1/4] op-node: Add option to only use finalized blocks as l1origin --- op-node/rollup/driver/config.go | 4 ++ op-node/rollup/driver/driver.go | 9 +++- op-node/rollup/driver/finalized.go | 33 +++++++++++++++ op-node/rollup/driver/finalized_test.go | 56 +++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 op-node/rollup/driver/finalized.go create mode 100644 op-node/rollup/driver/finalized_test.go diff --git a/op-node/rollup/driver/config.go b/op-node/rollup/driver/config.go index f4013b95e1de..e3bc7e063c8b 100644 --- a/op-node/rollup/driver/config.go +++ b/op-node/rollup/driver/config.go @@ -20,4 +20,8 @@ type Config struct { // SequencerMaxSafeLag is the maximum number of L2 blocks for restricting the distance between L2 safe and unsafe. // Disabled if 0. SequencerMaxSafeLag uint64 `json:"sequencer_max_safe_lag"` + + // SequencerUseFinalized is true when the sequencer should use only finalized L1 blocks as origin. + // If this is set to true, the value of `SequencerConfDepth` is ignored. + SequencerUseFinalized bool `json:"sequencer_use_finalized"` } diff --git a/op-node/rollup/driver/driver.go b/op-node/rollup/driver/driver.go index d65f996c4e5b..5f90c201c0fe 100644 --- a/op-node/rollup/driver/driver.go +++ b/op-node/rollup/driver/driver.go @@ -176,8 +176,13 @@ func NewDriver( l1 = NewMeteredL1Fetcher(l1, metrics) l1State := NewL1State(log, metrics) - sequencerConfDepth := NewConfDepth(driverCfg.SequencerConfDepth, l1State.L1Head, l1) - findL1Origin := NewL1OriginSelector(log, cfg, sequencerConfDepth) + var l1Fetcher L1Blocks + if driverCfg.SequencerUseFinalized { + l1Fetcher = NewFinalized(l1State.L1Finalized, l1) + } else { + l1Fetcher = NewConfDepth(driverCfg.SequencerConfDepth, l1State.L1Head, l1) + } + findL1Origin := NewL1OriginSelector(log, cfg, l1Fetcher) verifConfDepth := NewConfDepth(driverCfg.VerifierConfDepth, l1State.L1Head, l1) ec := engine.NewEngineController(l2, log, metrics, cfg, syncCfg.SyncMode, synchronousEvents) engineResetDeriver := engine.NewEngineResetDeriver(driverCtx, log, cfg, l1, l2, syncCfg, synchronousEvents) diff --git a/op-node/rollup/driver/finalized.go b/op-node/rollup/driver/finalized.go new file mode 100644 index 000000000000..94dc519c8594 --- /dev/null +++ b/op-node/rollup/driver/finalized.go @@ -0,0 +1,33 @@ +package driver + +import ( + "context" + + "github.com/ethereum/go-ethereum" + + "github.com/ethereum-optimism/optimism/op-node/rollup/derive" + "github.com/ethereum-optimism/optimism/op-service/eth" +) + +type finalized struct { + derive.L1Fetcher + l1Finalized func() eth.L1BlockRef +} + +func NewFinalized(l1Finalized func() eth.L1BlockRef, fetcher derive.L1Fetcher) *finalized { + return &finalized{L1Fetcher: fetcher, l1Finalized: l1Finalized} +} + +func (f *finalized) L1BlockRefByNumber(ctx context.Context, num uint64) (eth.L1BlockRef, error) { + // Don't apply the finalized wrapper if l1Finalized is empty (as it is during the startup case before the l1State is initialized). + l1Finalized := f.l1Finalized() + if l1Finalized == (eth.L1BlockRef{}) { + return f.L1Fetcher.L1BlockRefByNumber(ctx, num) + } + if num == 0 || num <= l1Finalized.Number { + return f.L1Fetcher.L1BlockRefByNumber(ctx, num) + } + return eth.L1BlockRef{}, ethereum.NotFound +} + +var _ derive.L1Fetcher = (*finalized)(nil) diff --git a/op-node/rollup/driver/finalized_test.go b/op-node/rollup/driver/finalized_test.go new file mode 100644 index 000000000000..77b8983326d7 --- /dev/null +++ b/op-node/rollup/driver/finalized_test.go @@ -0,0 +1,56 @@ +package driver + +import ( + "context" + "testing" + + "github.com/ethereum/go-ethereum" + "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/require" + + "github.com/ethereum-optimism/optimism/op-service/eth" + "github.com/ethereum-optimism/optimism/op-service/testutils" +) + +type finalizedTest struct { + name string + final uint64 + hash common.Hash // hash of finalized block + req uint64 + pass bool +} + +func (ft *finalizedTest) Run(t *testing.T) { + l1Fetcher := &testutils.MockL1Source{} + l1Finalized := eth.L1BlockRef{Number: ft.final, Hash: ft.hash} + l1FinalizedGetter := func() eth.L1BlockRef { return l1Finalized } + + f := NewFinalized(l1FinalizedGetter, l1Fetcher) + + if ft.pass { + // no calls to the l1Fetcher are made if the block number is not finalized yet + l1Fetcher.ExpectL1BlockRefByNumber(ft.req, eth.L1BlockRef{Number: ft.req}, nil) + } + + out, err := f.L1BlockRefByNumber(context.Background(), ft.req) + l1Fetcher.AssertExpectations(t) + + if ft.pass { + require.NoError(t, err) + require.Equal(t, out, eth.L1BlockRef{Number: ft.req}) + } else { + require.Equal(t, ethereum.NotFound, err) + } +} + +func TestFinalized(t *testing.T) { + testCases := []finalizedTest{ + {name: "finalized", final: 10, hash: exHash, req: 10, pass: true}, + {name: "finalized past", final: 10, hash: exHash, req: 8, pass: true}, + {name: "not finalized", final: 10, hash: exHash, req: 11, pass: false}, + {name: "no L1 state", req: 10, pass: true}, + } + for _, tc := range testCases { + t.Run(tc.name, tc.Run) + } +} From 51e5443a208f4836aa1c0eca029515dedcbb4588 Mon Sep 17 00:00:00 2001 From: Paul Lange Date: Fri, 9 Aug 2024 12:31:54 +0200 Subject: [PATCH 2/4] driver: use finalized block for derivation as well --- op-node/rollup/driver/driver.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/op-node/rollup/driver/driver.go b/op-node/rollup/driver/driver.go index 5f90c201c0fe..c6dd2f70a602 100644 --- a/op-node/rollup/driver/driver.go +++ b/op-node/rollup/driver/driver.go @@ -176,14 +176,16 @@ func NewDriver( l1 = NewMeteredL1Fetcher(l1, metrics) l1State := NewL1State(log, metrics) - var l1Fetcher L1Blocks + var l1Blocks L1Blocks + var l1Fetcher derive.L1Fetcher if driverCfg.SequencerUseFinalized { + l1Blocks = NewFinalized(l1State.L1Finalized, l1) l1Fetcher = NewFinalized(l1State.L1Finalized, l1) } else { - l1Fetcher = NewConfDepth(driverCfg.SequencerConfDepth, l1State.L1Head, l1) + l1Blocks = NewConfDepth(driverCfg.SequencerConfDepth, l1State.L1Head, l1) + l1Fetcher = NewConfDepth(driverCfg.VerifierConfDepth, l1State.L1Head, l1) } - findL1Origin := NewL1OriginSelector(log, cfg, l1Fetcher) - verifConfDepth := NewConfDepth(driverCfg.VerifierConfDepth, l1State.L1Head, l1) + findL1Origin := NewL1OriginSelector(log, cfg, l1Blocks) ec := engine.NewEngineController(l2, log, metrics, cfg, syncCfg.SyncMode, synchronousEvents) engineResetDeriver := engine.NewEngineResetDeriver(driverCtx, log, cfg, l1, l2, syncCfg, synchronousEvents) clSync := clsync.NewCLSync(log, cfg, metrics, synchronousEvents) @@ -196,7 +198,7 @@ func NewDriver( } attributesHandler := attributes.NewAttributesHandler(log, cfg, driverCtx, l2, synchronousEvents) - derivationPipeline := derive.NewDerivationPipeline(log, cfg, verifConfDepth, l1Blobs, plasma, l2, metrics) + derivationPipeline := derive.NewDerivationPipeline(log, cfg, l1Fetcher, l1Blobs, plasma, l2, metrics) pipelineDeriver := derive.NewPipelineDeriver(driverCtx, derivationPipeline, synchronousEvents) attrBuilder := derive.NewFetchingAttributesBuilder(cfg, l1, l2) meteredEngine := NewMeteredEngine(cfg, ec, metrics, log) // Only use the metered engine in the sequencer b/c it records sequencing metrics. From 4a915e4224a10e53df95cb475ff45b4866678fa4 Mon Sep 17 00:00:00 2001 From: Paul Lange Date: Tue, 20 Aug 2024 13:42:30 +0200 Subject: [PATCH 3/4] node: use finalized block when loading runtime config --- op-node/node/node.go | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/op-node/node/node.go b/op-node/node/node.go index fb5f981d8f14..c57e5d48888f 100644 --- a/op-node/node/node.go +++ b/op-node/node/node.go @@ -221,21 +221,32 @@ func (n *OpNode) initRuntimeConfig(ctx context.Context, cfg *Config) error { return eth.L1BlockRef{}, err } - // Apply confirmation-distance - blNum := l1Head.Number - if blNum >= confDepth { - blNum -= confDepth - } - fetchCtx, fetchCancel = context.WithTimeout(ctx, time.Second*10) - confirmed, err := n.l1Source.L1BlockRefByNumber(fetchCtx, blNum) - fetchCancel() - if err != nil { - n.log.Error("failed to fetch confirmed L1 block for runtime config loading", "err", err, "number", blNum) - return eth.L1BlockRef{}, err + var safeBlock eth.L1BlockRef + if cfg.Driver.SequencerUseFinalized { + fetchCtx, fetchCancel = context.WithTimeout(ctx, time.Second*10) + safeBlock, err = n.l1Source.L1BlockRefByLabel(fetchCtx, eth.Finalized) + fetchCancel() + if err != nil { + n.log.Error("failed to fetch finalized L1 block for runtime config loading", "err", err) + return eth.L1BlockRef{}, err + } + } else { + // Apply confirmation-distance + blNum := l1Head.Number + if blNum >= confDepth { + blNum -= confDepth + } + fetchCtx, fetchCancel = context.WithTimeout(ctx, time.Second*10) + safeBlock, err = n.l1Source.L1BlockRefByNumber(fetchCtx, blNum) + fetchCancel() + if err != nil { + n.log.Error("failed to fetch confirmed L1 block for runtime config loading", "err", err, "number", blNum) + return eth.L1BlockRef{}, err + } } fetchCtx, fetchCancel = context.WithTimeout(ctx, time.Second*10) - err = n.runCfg.Load(fetchCtx, confirmed) + err = n.runCfg.Load(fetchCtx, safeBlock) fetchCancel() if err != nil { n.log.Error("failed to fetch runtime config data", "err", err) From d54a0e8dbc4845d82fef46a74417d471d8c2c8b4 Mon Sep 17 00:00:00 2001 From: Paul Lange Date: Tue, 20 Aug 2024 13:38:35 +0200 Subject: [PATCH 4/4] sequencer: stall if finality lags to far behind --- op-node/rollup/driver/config.go | 2 ++ op-node/rollup/driver/state.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/op-node/rollup/driver/config.go b/op-node/rollup/driver/config.go index e3bc7e063c8b..724491636595 100644 --- a/op-node/rollup/driver/config.go +++ b/op-node/rollup/driver/config.go @@ -24,4 +24,6 @@ type Config struct { // SequencerUseFinalized is true when the sequencer should use only finalized L1 blocks as origin. // If this is set to true, the value of `SequencerConfDepth` is ignored. SequencerUseFinalized bool `json:"sequencer_use_finalized"` + + SequencerMaxFinalizedLag uint64 `json:"sequencer_max_finalized_lag"` } diff --git a/op-node/rollup/driver/state.go b/op-node/rollup/driver/state.go index 73ab3a0923bd..4d4defcfdafb 100644 --- a/op-node/rollup/driver/state.go +++ b/op-node/rollup/driver/state.go @@ -229,6 +229,36 @@ func (s *Driver) eventLoop() { s.log.Error("unexpected error from event-draining", "err", err) } + if s.driverConfig.SequencerEnabled && !s.driverConfig.SequencerStopped && + s.l1State.L1Head() != (eth.L1BlockRef{}) && s.l1State.L1Finalized() != (eth.L1BlockRef{}) && + s.Derivation.DerivationReady() { + + // If the L1 does fall behind the limit with finalizing blocks, delay creating new blocks + // until the finalized lag is below SequencerMaxFinalizedLag. + numBlocksSinceFinalization := s.l1State.L1Head().Number - s.l1State.L1Finalized().Number + if numBlocksSinceFinalization > s.driverConfig.SequencerMaxFinalizedLag { + if sequencerCh != nil { + s.log.Warn( + "Delay creating new block since finalized lag exceeds limit", + "finalized_l1", s.l1State.L1Finalized(), + "l1_head", s.l1State.L1Head(), + "unsafe_l2", s.Engine.UnsafeL2Head(), + "safe_l2", s.Engine.SafeL2Head(), + ) + sequencerCh = nil + } + // respCh := make(chan hashAndError) + // s.stopSequencer <- respCh + // resp := <-respCh + // if resp.err != nil { + // log.Error("Failed to stop sequencer", "err", resp.err) + // } else { + // log.Info("Sequencer has been stopped", "hash", resp.hash) + // } + // } else { + // s.startSequencer + } + } // If we are sequencing, and the L1 state is ready, update the trigger for the next sequencer action. // This may adjust at any time based on fork-choice changes or previous errors. // And avoid sequencing if the derivation pipeline indicates the engine is not ready.