Skip to content

N
nebula
Commit 679522bd authored by mergify[bot]'s avatar mergify[bot] Committed by GitHub
Browse files
Options

Merge pull request #4808 from ethereum-optimism/fix-sequencer-origin-selection 

op-node: ensure sequencer selects L1 origin within sequencer time drift and use conf depth util
parents a49e1040 72f0f9e6
Hide whitespace changes
Inline Side-by-side
Showing with 136 additions and 81 deletions
op-e2e/actions/l2_sequencer.go
View file @ 679522bd
......@@ -18,11 +18,11 @@ type MockL1OriginSelector struct {
originOverride eth.L1BlockRef // override which origin gets picked
}
func (m *MockL1OriginSelector) FindL1Origin(ctx context.Context, l1Head eth.L1BlockRef, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) {
func (m *MockL1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) {
if m.originOverride != (eth.L1BlockRef{}) {
return m.originOverride, nil
}
return m.actual.FindL1Origin(ctx, l1Head, l2Head)
return m.actual.FindL1Origin(ctx, l2Head)
}
// L2Sequencer is an actor that functions like a rollup node,
......@@ -40,8 +40,9 @@ type L2Sequencer struct {
func NewL2Sequencer(t Testing, log log.Logger, l1 derive.L1Fetcher, eng L2API, cfg *rollup.Config, seqConfDepth uint64) *L2Sequencer {
ver := NewL2Verifier(t, log, l1, eng, cfg)
attrBuilder := derive.NewFetchingAttributesBuilder(cfg, l1, eng)
seqConfDepthL1 := driver.NewConfDepth(seqConfDepth, ver.l1State.L1Head, l1)
l1OriginSelector := &MockL1OriginSelector{
actual: driver.NewL1OriginSelector(log, cfg, l1, seqConfDepth),
actual: driver.NewL1OriginSelector(log, cfg, seqConfDepthL1),
}
return &L2Sequencer{
L2Verifier: *ver,
......@@ -62,7 +63,7 @@ func (s *L2Sequencer) ActL2StartBlock(t Testing) {
return
}
err := s.sequencer.StartBuildingBlock(t.Ctx(), s.l1State.L1Head())
err := s.sequencer.StartBuildingBlock(t.Ctx())
require.NoError(t, err, "failed to start block building")
s.l2Building = true
......@@ -106,7 +107,7 @@ func (s *L2Sequencer) ActBuildToL1Head(t Testing) {
func (s *L2Sequencer) ActBuildToL1HeadExcl(t Testing) {
for {
s.ActL2PipelineFull(t)
nextOrigin, err := s.mockL1OriginSelector.FindL1Origin(t.Ctx(), s.l1State.L1Head(), s.derivation.UnsafeL2Head())
nextOrigin, err := s.mockL1OriginSelector.FindL1Origin(t.Ctx(), s.derivation.UnsafeL2Head())
require.NoError(t, err)
if nextOrigin.Number >= s.l1State.L1Head().Number {
break
......
op-node/rollup/driver/driver.go
View file @ 679522bd
......@@ -66,10 +66,10 @@ type L1StateIface interface {
}
type SequencerIface interface {
StartBuildingBlock(ctx context.Context, l1Head eth.L1BlockRef) error
StartBuildingBlock(ctx context.Context) error
CompleteBuildingBlock(ctx context.Context) (*eth.ExecutionPayload, error)
PlanNextSequencerAction() time.Duration
RunNextSequencerAction(ctx context.Context, l1Head eth.L1BlockRef) *eth.ExecutionPayload
RunNextSequencerAction(ctx context.Context) *eth.ExecutionPayload
BuildingOnto() eth.L2BlockRef
}
......@@ -81,7 +81,8 @@ type Network interface {
// NewDriver composes an events handler that tracks L1 state, triggers L2 derivation, and optionally sequences new L2 blocks.
func NewDriver(driverCfg *Config, cfg *rollup.Config, l2 L2Chain, l1 L1Chain, network Network, log log.Logger, snapshotLog log.Logger, metrics Metrics) *Driver {
l1State := NewL1State(log, metrics)
findL1Origin := NewL1OriginSelector(log, cfg, l1, driverCfg.SequencerConfDepth)
sequencerConfDepth := NewConfDepth(driverCfg.SequencerConfDepth, l1State.L1Head, l1)
findL1Origin := NewL1OriginSelector(log, cfg, sequencerConfDepth)
verifConfDepth := NewConfDepth(driverCfg.VerifierConfDepth, l1State.L1Head, l1)
derivationPipeline := derive.NewDerivationPipeline(log, cfg, verifConfDepth, l2, metrics)
attrBuilder := derive.NewFetchingAttributesBuilder(cfg, l1, l2)
......
op-node/rollup/driver/origin_selector.go
View file @ 679522bd
......@@ -2,7 +2,10 @@ package driver
import (
"context"
"errors"
"fmt"
"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum-optimism/optimism/op-node/eth"
......@@ -19,63 +22,49 @@ type L1OriginSelector struct {
log log.Logger
cfg *rollup.Config
l1 L1Blocks
sequencingConfDepth uint64
l1 L1Blocks
}
func NewL1OriginSelector(log log.Logger, cfg *rollup.Config, l1 L1Blocks, sequencingConfDepth uint64) *L1OriginSelector {
func NewL1OriginSelector(log log.Logger, cfg *rollup.Config, l1 L1Blocks) *L1OriginSelector {
return &L1OriginSelector{
log: log,
cfg: cfg,
l1: l1,
sequencingConfDepth: sequencingConfDepth,
log: log,
cfg: cfg,
l1: l1,
}
}
// 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.
func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l1Head eth.L1BlockRef, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) {
// If we are at the head block, don't do a lookup.
if l2Head.L1Origin.Hash == l1Head.Hash {
return l1Head, nil
}
// Grab a reference to the current L1 origin block.
func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) {
// Grab a reference to the current L1 origin block. This call is by hash and thus easily cached.
currentOrigin, err := los.l1.L1BlockRefByHash(ctx, l2Head.L1Origin.Hash)
if err != nil {
return eth.L1BlockRef{}, err
}
log := los.log.New("current", currentOrigin, "current_time", currentOrigin.Time,
"l2_head", l2Head, "l2_head_time", l2Head.Time)
// If we are past the sequencer depth, we may want to advance the origin, but need to still
// check the time of the next origin.
pastSeqDrift := l2Head.Time+los.cfg.BlockTime > currentOrigin.Time+los.cfg.MaxSequencerDrift
if pastSeqDrift {
log.Info("Next L2 block time is past the sequencer drift + current origin time",
"current", currentOrigin, "current_time", currentOrigin.Time,
"l1_head", l1Head, "l1_head_time", l1Head.Time,
"l2_head", l2Head, "l2_head_time", l2Head.Time,
"depth", los.sequencingConfDepth)
}
if !pastSeqDrift && currentOrigin.Number+1+los.sequencingConfDepth > l1Head.Number {
// TODO: we can decide to ignore confirmation depth if we would be forced
// to make an empty block (only deposits) by staying on the current origin.
log.Info("sequencing with old origin to preserve conf depth",
"current", currentOrigin, "current_time", currentOrigin.Time,
"l1_head", l1Head, "l1_head_time", l1Head.Time,
"l2_head", l2Head, "l2_head_time", l2Head.Time,
"depth", los.sequencingConfDepth)
return currentOrigin, nil
log.Warn("Next L2 block time is past the sequencer drift + current origin time")
}
// Attempt to find the next L1 origin block, where the next origin is the immediate child of
// the current origin block.
// The L1 source can be shimmed to hide new L1 blocks and enforce a sequencer confirmation distance.
nextOrigin, err := los.l1.L1BlockRefByNumber(ctx, currentOrigin.Number+1)
if err != nil {
// TODO: this could result in a bad origin being selected if we are past the seq
// drift & should instead advance to the next origin.
log.Error("Failed to get next origin. Falling back to current origin", "err", err)
if pastSeqDrift {
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 errors.Is(err, ethereum.NotFound) {
log.Debug("No next L1 block found, repeating current origin")
} else {
log.Error("Failed to get next origin. Falling back to current origin", "err", err)
}
return currentOrigin, nil
}
......
op-node/rollup/driver/origin_selector_test.go
View file @ 679522bd
......@@ -27,6 +27,7 @@ func TestOriginSelectorAdvances(t *testing.T) {
BlockTime: 2,
}
l1 := &testutils.MockL1Source{}
defer l1.AssertExpectations(t)
a := eth.L1BlockRef{
Hash: common.Hash{'a'},
Number: 10,
......@@ -46,9 +47,8 @@ func TestOriginSelectorAdvances(t *testing.T) {
l1.ExpectL1BlockRefByHash(a.Hash, a, nil)
l1.ExpectL1BlockRefByNumber(b.Number, b, nil)
s := NewL1OriginSelector(log, cfg, l1, 0)
next, err := s.FindL1Origin(context.Background(), b, l2Head)
s := NewL1OriginSelector(log, cfg, l1)
next, err := s.FindL1Origin(context.Background(), l2Head)
require.Nil(t, err)
require.Equal(t, b, next)
}
......@@ -68,6 +68,7 @@ func TestOriginSelectorRespectsOriginTiming(t *testing.T) {
BlockTime: 2,
}
l1 := &testutils.MockL1Source{}
defer l1.AssertExpectations(t)
a := eth.L1BlockRef{
Hash: common.Hash{'a'},
Number: 10,
......@@ -87,15 +88,14 @@ func TestOriginSelectorRespectsOriginTiming(t *testing.T) {
l1.ExpectL1BlockRefByHash(a.Hash, a, nil)
l1.ExpectL1BlockRefByNumber(b.Number, b, nil)
s := NewL1OriginSelector(log, cfg, l1, 0)
next, err := s.FindL1Origin(context.Background(), b, l2Head)
s := NewL1OriginSelector(log, cfg, l1)
next, err := s.FindL1Origin(context.Background(), l2Head)
require.Nil(t, err)
require.Equal(t, a, next)
}
// TestOriginSelectorRespectsConfDepth ensures that the origin selector
// will respects the confirmation depth requirement
// will respect the confirmation depth requirement
//
// There are 2 L1 blocks at time 20 & 25. The L2 Head is at time 27.
// The next L2 time is 29 which enough to normally select block `b`
......@@ -108,6 +108,7 @@ func TestOriginSelectorRespectsConfDepth(t *testing.T) {
BlockTime: 2,
}
l1 := &testutils.MockL1Source{}
defer l1.AssertExpectations(t)
a := eth.L1BlockRef{
Hash: common.Hash{'a'},
Number: 10,
......@@ -125,33 +126,32 @@ func TestOriginSelectorRespectsConfDepth(t *testing.T) {
}
l1.ExpectL1BlockRefByHash(a.Hash, a, nil)
l1.ExpectL1BlockRefByNumber(b.Number, b, nil)
s := NewL1OriginSelector(log, cfg, l1, 10)
confDepthL1 := NewConfDepth(10, func() eth.L1BlockRef { return b }, l1)
s := NewL1OriginSelector(log, cfg, confDepthL1)
next, err := s.FindL1Origin(context.Background(), b, l2Head)
next, err := s.FindL1Origin(context.Background(), l2Head)
require.Nil(t, err)
require.Equal(t, a, next)
}
// TestOriginSelectorRespectsMaxSeqDrift ensures that the origin selector
// will advance if the time delta between the current L1 origin and the next
// L2 block is greater than the sequencer drift. This needs to occur even
// if conf depth needs to be ignored
// TestOriginSelectorStrictConfDepth ensures that the origin selector will maintain the sequencer conf depth,
// even while the time delta between the current L1 origin and the next
// L2 block is greater than the sequencer drift.
// It's more important to maintain safety with an empty block than to maintain liveness with poor conf depth.
//
// There are 2 L1 blocks at time 20 & 25. The L2 Head is at time 27.
// The next L2 time is 29. The sequencer drift is 8 so the L2 head is
// valid with origin `a`, but the next L2 block is not valid with origin `b.`
// This is because 29 (next L2 time) > 20 (origin) + 8 (seq drift) => invalid block.
// Even though the LOS would normally refuse to advance because block `b` does not
// have enough confirmations, it should in this instance.
func TestOriginSelectorRespectsMaxSeqDrift(t *testing.T) {
// We maintain confirmation distance, even though we would shift to the next origin if we could.
func TestOriginSelectorStrictConfDepth(t *testing.T) {
log := testlog.Logger(t, log.LvlCrit)
cfg := &rollup.Config{
MaxSequencerDrift: 8,
BlockTime: 2,
}
l1 := &testutils.MockL1Source{}
defer l1.AssertExpectations(t)
a := eth.L1BlockRef{
Hash: common.Hash{'a'},
Number: 10,
......@@ -169,13 +169,11 @@ func TestOriginSelectorRespectsMaxSeqDrift(t *testing.T) {
}
l1.ExpectL1BlockRefByHash(a.Hash, a, nil)
l1.ExpectL1BlockRefByNumber(b.Number, b, nil)
s := NewL1OriginSelector(log, cfg, l1, 10)
confDepthL1 := NewConfDepth(10, func() eth.L1BlockRef { return b }, l1)
s := NewL1OriginSelector(log, cfg, confDepthL1)
next, err := s.FindL1Origin(context.Background(), b, l2Head)
require.Nil(t, err)
require.Equal(t, b, next)
_, err := s.FindL1Origin(context.Background(), l2Head)
require.ErrorContains(t, err, "sequencer time drift")
}
// TestOriginSelectorSeqDriftRespectsNextOriginTime
......@@ -191,6 +189,7 @@ func TestOriginSelectorSeqDriftRespectsNextOriginTime(t *testing.T) {
BlockTime: 2,
}
l1 := &testutils.MockL1Source{}
defer l1.AssertExpectations(t)
a := eth.L1BlockRef{
Hash: common.Hash{'a'},
Number: 10,
......@@ -210,9 +209,76 @@ func TestOriginSelectorSeqDriftRespectsNextOriginTime(t *testing.T) {
l1.ExpectL1BlockRefByHash(a.Hash, a, nil)
l1.ExpectL1BlockRefByNumber(b.Number, b, nil)
s := NewL1OriginSelector(log, cfg, l1, 10)
next, err := s.FindL1Origin(context.Background(), b, l2Head)
s := NewL1OriginSelector(log, cfg, l1)
next, err := s.FindL1Origin(context.Background(), l2Head)
require.Nil(t, err)
require.Equal(t, a, next)
}
// TestOriginSelectorHandlesLateL1Blocks tests the forced repeat of the previous origin,
// but with a conf depth that first prevents it from learning about the need to repeat.
//
// There are 2 L1 blocks at time 20 & 100. The L2 Head is at time 27.
// The next L2 time is 29. Even though the next L2 time is past the seq
// drift, the origin should remain on block `a` because the next origin's
// time is greater than the next L2 time.
// Due to a conf depth of 2, block `b` is not immediately visible,
// and the origin selection should fail until it is visible, by waiting for block `c`.
func TestOriginSelectorHandlesLateL1Blocks(t *testing.T) {
log := testlog.Logger(t, log.LvlCrit)
cfg := &rollup.Config{
MaxSequencerDrift: 8,
BlockTime: 2,
}
l1 := &testutils.MockL1Source{}
defer l1.AssertExpectations(t)
a := eth.L1BlockRef{
Hash: common.Hash{'a'},
Number: 10,
Time: 20,
}
b := eth.L1BlockRef{
Hash: common.Hash{'b'},
Number: 11,
Time: 100,
ParentHash: a.Hash,
}
c := eth.L1BlockRef{
Hash: common.Hash{'c'},
Number: 12,
Time: 150,
ParentHash: b.Hash,
}
d := eth.L1BlockRef{
Hash: common.Hash{'d'},
Number: 13,
Time: 200,
ParentHash: c.Hash,
}
l2Head := eth.L2BlockRef{
L1Origin: a.ID(),
Time: 27,
}
// l2 head does not change, so we start at the same origin again and again until we meet the conf depth
l1.ExpectL1BlockRefByHash(a.Hash, a, nil)
l1.ExpectL1BlockRefByHash(a.Hash, a, nil)
l1.ExpectL1BlockRefByHash(a.Hash, a, nil)
l1.ExpectL1BlockRefByNumber(b.Number, b, nil)
l1Head := b
confDepthL1 := NewConfDepth(2, func() eth.L1BlockRef { return l1Head }, l1)
s := NewL1OriginSelector(log, cfg, confDepthL1)
_, err := s.FindL1Origin(context.Background(), l2Head)
require.ErrorContains(t, err, "sequencer time drift")
l1Head = c
_, err = s.FindL1Origin(context.Background(), l2Head)
require.ErrorContains(t, err, "sequencer time drift")
l1Head = d
next, err := s.FindL1Origin(context.Background(), l2Head)
require.Nil(t, err)
require.Equal(t, a, next, "must stay on a because the L1 time may not be higher than the L2 time")
}
op-node/rollup/driver/sequencer.go
View file @ 679522bd
......@@ -20,7 +20,7 @@ type Downloader interface {
}
type L1OriginSelectorIface interface {
FindL1Origin(ctx context.Context, l1Head eth.L1BlockRef, l2Head eth.L2BlockRef) (eth.L1BlockRef, error)
FindL1Origin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, error)
}
// Sequencer implements the sequencing interface of the driver: it starts and completes block building jobs.
......@@ -51,11 +51,11 @@ func NewSequencer(log log.Logger, cfg *rollup.Config, engine derive.EngineContro
}
// StartBuildingBlock initiates a block building job on top of the given L2 head, safe and finalized blocks, and using the provided l1Origin.
func (d *Sequencer) StartBuildingBlock(ctx context.Context, l1Head eth.L1BlockRef) error {
func (d *Sequencer) StartBuildingBlock(ctx context.Context) error {
l2Head := d.engine.UnsafeL2Head()
// Figure out which L1 origin block we're going to be building on top of.
l1Origin, err := d.l1OriginSelector.FindL1Origin(ctx, l1Head, l2Head)
l1Origin, err := d.l1OriginSelector.FindL1Origin(ctx, l2Head)
if err != nil {
d.log.Error("Error finding next L1 Origin", "err", err)
return err
......@@ -159,7 +159,7 @@ func (d *Sequencer) BuildingOnto() eth.L2BlockRef {
// RunNextSequencerAction starts new block building work, or seals existing work,
// and is best timed by first awaiting the delay returned by PlanNextSequencerAction.
// If a new block is successfully sealed, it will be returned for publishing, nil otherwise.
func (d *Sequencer) RunNextSequencerAction(ctx context.Context, l1Head eth.L1BlockRef) *eth.ExecutionPayload {
func (d *Sequencer) RunNextSequencerAction(ctx context.Context) *eth.ExecutionPayload {
if _, buildingID, _ := d.engine.BuildingPayload(); buildingID != (eth.PayloadID{}) {
payload, err := d.CompleteBuildingBlock(ctx)
if err != nil {
......@@ -174,7 +174,7 @@ func (d *Sequencer) RunNextSequencerAction(ctx context.Context, l1Head eth.L1Blo
return payload
}
} else {
err := d.StartBuildingBlock(ctx, l1Head)
err := d.StartBuildingBlock(ctx)
if err != nil {
d.log.Error("sequencer failed to start building new block", "err", err)
d.nextAction = d.timeNow().Add(time.Second)
......
op-node/rollup/driver/sequencer_test.go
View file @ 679522bd
......@@ -132,10 +132,10 @@ func (fn testAttrBuilderFn) PreparePayloadAttributes(ctx context.Context, l2Pare
var _ derive.AttributesBuilder = (testAttrBuilderFn)(nil)
type testOriginSelectorFn func(ctx context.Context, l1Head eth.L1BlockRef, l2Head eth.L2BlockRef) (eth.L1BlockRef, error)
type testOriginSelectorFn func(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, error)
func (fn testOriginSelectorFn) FindL1Origin(ctx context.Context, l1Head eth.L1BlockRef, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) {
return fn(ctx, l1Head, l2Head)
func (fn testOriginSelectorFn) FindL1Origin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) {
return fn(ctx, l2Head)
}
var _ L1OriginSelectorIface = (testOriginSelectorFn)(nil)
......@@ -262,7 +262,7 @@ func TestSequencerChaosMonkey(t *testing.T) {
maxL1BlockTimeGap := uint64(100)
// The origin selector just generates random L1 blocks based on RNG
var originErr error
originSelector := testOriginSelectorFn(func(ctx context.Context, l1Head eth.L1BlockRef, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) {
originSelector := testOriginSelectorFn(func(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) {
if originErr != nil {
return eth.L1BlockRef{}, originErr
}
......@@ -298,8 +298,6 @@ func TestSequencerChaosMonkey(t *testing.T) {
seq := NewSequencer(log, cfg, engControl, attrBuilder, originSelector)
seq.timeNow = clockFn
l1Head := eth.L1BlockRef{} // TODO this is getting removed
// try to build 1000 blocks, with 5x as many planning attempts, to handle errors and clock problems
desiredBlocks := 1000
for i := 0; i < 5*desiredBlocks && engControl.totalBuiltBlocks < desiredBlocks; i++ {
......@@ -339,7 +337,7 @@ func TestSequencerChaosMonkey(t *testing.T) {
default:
// no error
}
payload := seq.RunNextSequencerAction(context.Background(), l1Head)
payload := seq.RunNextSequencerAction(context.Background())
if payload != nil {
require.Equal(t, engControl.UnsafeL2Head().ID(), payload.ID(), "head must stay in sync with emitted payloads")
var tx types.Transaction
......
op-node/rollup/driver/state.go
View file @ 679522bd
......@@ -212,7 +212,7 @@ func (s *Driver) eventLoop() {
select {
case <-sequencerCh:
payload := s.sequencer.RunNextSequencerAction(ctx, s.l1State.L1Head())
payload := s.sequencer.RunNextSequencerAction(ctx)
if s.network != nil && payload != nil {
// Publishing of unsafe data via p2p is optional.
// Errors are not severe enough to change/halt sequencing but should be logged and metered.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment