diff --git a/op-e2e/actions/helpers/l2_sequencer.go b/op-e2e/actions/helpers/l2_sequencer.go index 98becdcc87a4a..424e12b23fda0 100644 --- a/op-e2e/actions/helpers/l2_sequencer.go +++ b/op-e2e/actions/helpers/l2_sequencer.go @@ -56,8 +56,9 @@ func NewL2Sequencer(t Testing, log log.Logger, l1 derive.L1Fetcher, blobSrc deri ver := NewL2Verifier(t, log, l1, blobSrc, altDASrc, eng, cfg, &sync.Config{}, safedb.Disabled, interopBackend) attrBuilder := derive.NewFetchingAttributesBuilder(cfg, l1, eng) seqConfDepthL1 := confdepth.NewConfDepth(seqConfDepth, ver.syncStatus.L1Head, l1) + originSelector := sequencing.NewL1OriginSelector(t.Ctx(), log, cfg, seqConfDepthL1) l1OriginSelector := &MockL1OriginSelector{ - actual: sequencing.NewL1OriginSelector(log, cfg, seqConfDepthL1), + actual: originSelector, } metr := metrics.NoopMetrics seqStateListener := node.DisabledConfigPersistence{} @@ -78,6 +79,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, diff --git a/op-node/rollup/driver/driver.go b/op-node/rollup/driver/driver.go index 81607e612d5a2..1fd751846cf3e 100644 --- a/op-node/rollup/driver/driver.go +++ b/op-node/rollup/driver/driver.go @@ -245,7 +245,8 @@ func NewDriver( asyncGossiper := async.NewAsyncGossiper(driverCtx, network, log, metrics) attrBuilder := derive.NewFetchingAttributesBuilder(cfg, l1, l2) sequencerConfDepth := confdepth.NewConfDepth(driverCfg.SequencerConfDepth, statusTracker.L1Head, l1) - findL1Origin := sequencing.NewL1OriginSelector(log, cfg, sequencerConfDepth) + findL1Origin := sequencing.NewL1OriginSelector(driverCtx, log, cfg, sequencerConfDepth) + sys.Register("origin-selector", findL1Origin, opts) sequencer = sequencing.NewSequencer(driverCtx, log, cfg, attrBuilder, findL1Origin, sequencerStateListener, sequencerConductor, asyncGossiper, metrics) sys.Register("sequencer", sequencer, opts) diff --git a/op-node/rollup/sequencing/origin_selector.go b/op-node/rollup/sequencing/origin_selector.go index 41bd645054157..67a489884fea9 100644 --- a/op-node/rollup/sequencing/origin_selector.go +++ b/op-node/rollup/sequencing/origin_selector.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "sync" "time" "github.com/ethereum/go-ethereum" @@ -11,6 +12,8 @@ import ( "github.com/ethereum-optimism/optimism/op-node/rollup" "github.com/ethereum-optimism/optimism/op-node/rollup/derive" + "github.com/ethereum-optimism/optimism/op-node/rollup/engine" + "github.com/ethereum-optimism/optimism/op-node/rollup/event" "github.com/ethereum-optimism/optimism/op-service/eth" ) @@ -20,15 +23,23 @@ type L1Blocks interface { } type L1OriginSelector struct { + ctx context.Context log log.Logger cfg *rollup.Config spec *rollup.ChainSpec l1 L1Blocks + + // Internal cache of L1 origins for faster access. + currentOrigin eth.L1BlockRef + nextOrigin eth.L1BlockRef + + mu sync.Mutex } -func NewL1OriginSelector(log log.Logger, cfg *rollup.Config, l1 L1Blocks) *L1OriginSelector { +func NewL1OriginSelector(ctx context.Context, log log.Logger, cfg *rollup.Config, l1 L1Blocks) *L1OriginSelector { return &L1OriginSelector{ + ctx: ctx, log: log, cfg: cfg, spec: rollup.NewChainSpec(cfg), @@ -36,62 +47,172 @@ func NewL1OriginSelector(log log.Logger, cfg *rollup.Config, l1 L1Blocks) *L1Ori } } +func (los *L1OriginSelector) OnEvent(ev event.Event) bool { + switch x := ev.(type) { + case engine.ForkchoiceUpdateEvent: + los.onForkchoiceUpdate(x.UnsafeL2Head) + case rollup.ResetEvent: + los.reset() + default: + return false + } + 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. 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) + c := make(chan eth.L1BlockRef, 1) + return los.findL1Origin(ctx, l2Head, c) +} + +// findL1Origin determines what the next L1 Origin should be. +// This private method receives a channel to send the next L1 origin block to, +// and may be used in tests to provide deterministic concurrency behavior. +func (los *L1OriginSelector) findL1Origin(ctx context.Context, l2Head eth.L2BlockRef, c chan eth.L1BlockRef) (eth.L1BlockRef, error) { + currentOrigin, nextOrigin, err := los.CurrentAndNextOrigin(ctx, l2Head) if err != nil { + close(c) return eth.L1BlockRef{}, err } + los.tryFetchNextOrigin(currentOrigin, nextOrigin, c) + + // 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) - seqDrift := l2Head.Time + los.cfg.BlockTime - currentOrigin.Time + 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. + // Alternatively, if the next origin is ahead of the L2 head, we must return the current origin. + if !pastSeqDrift || (nextOrigin != (eth.L1BlockRef{}) && l2Head.Time+los.cfg.BlockTime < nextOrigin.Time) { + 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, attempting to wait for fetch of next L1 origin") + + nextOrigin, ok := <-c + if !ok { + 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", currentOrigin) + } + + // Once again check if the next origin is ahead of the L2 head, and return the current origin if it is. + if l2Head.Time+los.cfg.BlockTime < nextOrigin.Time { + return currentOrigin, nil + } + + return nextOrigin, nil +} + +func (los *L1OriginSelector) CurrentAndNextOrigin(ctx context.Context, l2Head eth.L2BlockRef) (eth.L1BlockRef, eth.L1BlockRef, error) { + los.mu.Lock() + defer los.mu.Unlock() + + 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() { + // If the L2 head has progressed to the next origin, update the current and next origins. + los.currentOrigin = los.nextOrigin + los.nextOrigin = eth.L1BlockRef{} + } else { + // If for some reason the L2 head is not on the current or next origin, we need to find the + // current origin block and reset the next origin. + // This is most likely to occur on the first block after a restart. + + // 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{}, eth.L1BlockRef{}, err + } - // 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 := seqDrift > msd - if pastSeqDrift { - log.Warn("Next L2 block time is past the sequencer drift + current origin time") - seqDrift = msd + los.currentOrigin = currentOrigin + los.nextOrigin = eth.L1BlockRef{} } - // Calculate the maximum time we can spend attempting to fetch the next L1 origin block. - // Time spent fetching this information is time not spent building the next L2 block, so - // we generally prioritize keeping this value small, allowing for a nonzero failure rate. - // As the next L2 block time approaches the max sequencer drift, increase our tolerance for - // slower L1 fetches in order to avoid falling too far behind. - fetchTimeout := time.Second + (9*time.Second*time.Duration(seqDrift))/time.Duration(msd) - fetchCtx, cancel := context.WithTimeout(ctx, fetchTimeout) + return los.currentOrigin, los.nextOrigin, nil +} + +func (los *L1OriginSelector) maybeSetNextOrigin(nextOrigin eth.L1BlockRef) { + los.mu.Lock() + defer los.mu.Unlock() + + // Set the next origin if it is the immediate child of the current origin. + if nextOrigin.ParentHash == los.currentOrigin.Hash { + los.nextOrigin = nextOrigin + } +} + +func (los *L1OriginSelector) onForkchoiceUpdate(unsafeL2Head eth.L2BlockRef) { + currentOrigin, nextOrigin, err := los.CurrentAndNextOrigin(los.ctx, unsafeL2Head) + if err != nil { + log.Error("Failed to get current and next L1 origin on forkchoice update", "err", err) + return + } + + c := make(chan eth.L1BlockRef, 1) + los.tryFetchNextOrigin(currentOrigin, nextOrigin, c) + <-c +} + +// tryFetchNextOrigin schedules a fetch for the next L1 origin block if it is not already set. +// This method always closes the channel, even if the next origin is already set. +func (los *L1OriginSelector) tryFetchNextOrigin(currentOrigin, nextOrigin eth.L1BlockRef, c chan<- eth.L1BlockRef) { + // If the next origin is already set, we don't need to do anything. + if nextOrigin != (eth.L1BlockRef{}) { + close(c) + return + } + + // If the current origin is not set, we can't schedule the next origin check. + if currentOrigin == (eth.L1BlockRef{}) { + close(c) + return + } + + go func() { + los.fetch(currentOrigin.Number+1, c) + }() +} + +func (los *L1OriginSelector) fetch(number uint64, c chan<- eth.L1BlockRef) { + defer close(c) + + fetchCtx, cancel := context.WithTimeout(los.ctx, 10*time.Second) defer cancel() // 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(fetchCtx, currentOrigin.Number+1) + nextOrigin, err := los.l1.L1BlockRefByNumber(fetchCtx, number) if err != nil { - 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") + log.Debug("No next potential L1 origin found") } else { - log.Error("Failed to get next origin. Falling back to current origin", "err", err) + log.Error("Failed to get next L1 origin", "err", err) } - return currentOrigin, nil + return } - // 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 l2Head.Time+los.cfg.BlockTime >= nextOrigin.Time { - return nextOrigin, nil - } + los.maybeSetNextOrigin(nextOrigin) + + c <- nextOrigin +} + +func (los *L1OriginSelector) reset() { + los.mu.Lock() + defer los.mu.Unlock() - return currentOrigin, nil + los.currentOrigin = eth.L1BlockRef{} + los.nextOrigin = eth.L1BlockRef{} } diff --git a/op-node/rollup/sequencing/origin_selector_test.go b/op-node/rollup/sequencing/origin_selector_test.go index 44461eac3077c..e008a5f6a17c3 100644 --- a/op-node/rollup/sequencing/origin_selector_test.go +++ b/op-node/rollup/sequencing/origin_selector_test.go @@ -2,6 +2,7 @@ package sequencing import ( "context" + "errors" "testing" "github.com/ethereum-optimism/optimism/op-node/rollup" @@ -9,12 +10,58 @@ import ( "github.com/ethereum-optimism/optimism/op-service/eth" "github.com/ethereum-optimism/optimism/op-service/testlog" "github.com/ethereum-optimism/optimism/op-service/testutils" - "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/log" "github.com/stretchr/testify/require" ) +// TestOriginSelectorFetchCurrentError ensures that the origin selector +// returns an error when it cannot fetch the current origin and has no +// internal cached state. +func TestOriginSelectorFetchCurrentError(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + log := testlog.Logger(t, log.LevelCrit) + cfg := &rollup.Config{ + MaxSequencerDrift: 500, + 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: 25, + ParentHash: a.Hash, + } + l2Head := eth.L2BlockRef{ + L1Origin: a.ID(), + Time: 24, + } + + l1.ExpectL1BlockRefByHash(a.Hash, eth.L1BlockRef{}, errors.New("test error")) + + s := NewL1OriginSelector(ctx, log, cfg, l1) + + _, err := s.FindL1Origin(ctx, l2Head) + require.ErrorContains(t, err, "test error") + + // The same outcome occurs when the cached origin is different from that of the L2 head. + l1.ExpectL1BlockRefByHash(a.Hash, eth.L1BlockRef{}, errors.New("test error")) + + s = NewL1OriginSelector(ctx, log, cfg, l1) + s.currentOrigin = b + + _, err = s.FindL1Origin(ctx, l2Head) + require.ErrorContains(t, err, "test error") +} + // TestOriginSelectorAdvances ensures that the origin selector // advances the origin // @@ -23,6 +70,9 @@ import ( // is no conf depth to stop the origin selection so block `b` should // be the next L1 origin func TestOriginSelectorAdvances(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + log := testlog.Logger(t, log.LevelCrit) cfg := &rollup.Config{ MaxSequencerDrift: 500, @@ -46,13 +96,141 @@ func TestOriginSelectorAdvances(t *testing.T) { Time: 24, } - l1.ExpectL1BlockRefByHash(a.Hash, a, nil) + s := NewL1OriginSelector(ctx, log, cfg, l1) + s.currentOrigin = a + s.nextOrigin = b + + c := make(chan eth.L1BlockRef, 1) + next, err := s.findL1Origin(ctx, l2Head, c) + require.Nil(t, err) + require.Equal(t, b, next) + + // Wait for the origin selector's background fetch to finish. + // This fetch should not be triggered because the next origin is already known. + select { + case _, ok := <-c: + require.False(t, ok) + default: + t.Fatal("expected the background fetch to have not run") + } +} + +// TestOriginSelectorNextOrigin ensures that the origin selector +// handles the case where the L2 Head is based on the internal next origin. +// +// There are 2 L1 blocks at time 20 & 25. The L2 Head is at time 24. +// The next L2 time is 26 which is after the next L1 block time. There +// is no conf depth to stop the origin selection so block `b` should +// be the next L1 origin +func TestOriginSelectorAdvancesFromCache(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + log := testlog.Logger(t, log.LevelCrit) + cfg := &rollup.Config{ + MaxSequencerDrift: 500, + 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: 25, + ParentHash: a.Hash, + } + l2Head := eth.L2BlockRef{ + L1Origin: a.ID(), + Time: 24, + } + + // This is called as part of the background prefetch job l1.ExpectL1BlockRefByNumber(b.Number, b, nil) - s := NewL1OriginSelector(log, cfg, l1) - next, err := s.FindL1Origin(context.Background(), l2Head) + s := NewL1OriginSelector(ctx, log, cfg, l1) + s.nextOrigin = a + + c := make(chan eth.L1BlockRef, 1) + next, err := s.findL1Origin(ctx, l2Head, c) + require.Nil(t, err) + require.Equal(t, a, next) + + // Wait for the origin selector's background fetch to finish. + // This fetch should be triggered because the next origin is not already known. + next, ok := <-c + require.True(t, ok) + require.Equal(t, b, next) +} + +// TestOriginSelectorPrefetchesNextOrigin ensures that the origin selector +// prefetches the next origin when it can. +// +// The next L2 time is 26 which is after the next L1 block time. There +// is no conf depth to stop the origin selection so block `b` will +// be the next L1 origin as soon as it is fetched. +func TestOriginSelectorPrefetchesNextOrigin(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + log := testlog.Logger(t, log.LevelCrit) + cfg := &rollup.Config{ + MaxSequencerDrift: 500, + 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: 25, + ParentHash: a.Hash, + } + l2Head := eth.L2BlockRef{ + L1Origin: a.ID(), + Time: 24, + } + + // This is called as part of the background prefetch job + l1.ExpectL1BlockRefByNumber(b.Number, b, nil) + + s := NewL1OriginSelector(ctx, log, cfg, l1) + s.currentOrigin = a + + c := make(chan eth.L1BlockRef, 1) + next, err := s.findL1Origin(ctx, l2Head, c) + require.Nil(t, err) + require.Equal(t, a, next) + + // Wait for the origin selector's background fetch to finish. + // This fetch should be triggered because the next origin is not already known. + next, ok := <-c + require.True(t, ok) + require.Equal(t, b, next) + + // The next origin should be `b` now. + c = make(chan eth.L1BlockRef, 1) + next, err = s.findL1Origin(ctx, l2Head, c) require.Nil(t, err) require.Equal(t, b, next) + + // Wait for the origin selector's background fetch to finish. + // This fetch should not be triggered because the next origin is already known. + select { + case _, ok := <-c: + require.False(t, ok) + default: + t.Fatal("expected the background fetch to have not run") + } } // TestOriginSelectorRespectsOriginTiming ensures that the origin selector @@ -64,6 +242,9 @@ func TestOriginSelectorAdvances(t *testing.T) { // but it should select block `a` because the L2 block time must be ahead // of the the timestamp of it's L1 origin. func TestOriginSelectorRespectsOriginTiming(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + log := testlog.Logger(t, log.LevelCrit) cfg := &rollup.Config{ MaxSequencerDrift: 500, @@ -87,13 +268,79 @@ func TestOriginSelectorRespectsOriginTiming(t *testing.T) { Time: 22, } - l1.ExpectL1BlockRefByHash(a.Hash, a, nil) - l1.ExpectL1BlockRefByNumber(b.Number, b, nil) + s := NewL1OriginSelector(ctx, log, cfg, l1) + s.currentOrigin = a + s.nextOrigin = b - s := NewL1OriginSelector(log, cfg, l1) - next, err := s.FindL1Origin(context.Background(), l2Head) + c := make(chan eth.L1BlockRef, 1) + next, err := s.findL1Origin(ctx, l2Head, c) require.Nil(t, err) require.Equal(t, a, next) + + // Wait for the origin selector's background fetch to finish. + // This fetch should not be triggered because the next origin is already known. + select { + case _, ok := <-c: + require.False(t, ok) + default: + t.Fatal("expected the background fetch to have not run") + } +} + +// TestOriginSelectorRespectsSeqDrift +// +// 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. +// The origin selector does not yet know about block `b` so it should wait for the +// background fetch to complete synchronously. +func TestOriginSelectorRespectsSeqDrift(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + log := testlog.Logger(t, log.LevelCrit) + 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: 25, + ParentHash: a.Hash, + } + l2Head := eth.L2BlockRef{ + L1Origin: a.ID(), + Time: 27, + } + + l1.ExpectL1BlockRefByHash(a.Hash, a, nil) + + l1.ExpectL1BlockRefByNumber(b.Number, b, nil) + + s := NewL1OriginSelector(ctx, log, cfg, l1) + + c := make(chan eth.L1BlockRef, 1) + next, err := s.findL1Origin(ctx, l2Head, c) + require.NoError(t, err) + require.Equal(t, b, next) + + // Wait for the origin selector's background fetch to finish. + // This fetch should already be completed because findL1Origin would have waited for it. + select { + case _, ok := <-c: + require.False(t, ok) + default: + t.Fatal("expected the background fetch to have already completed") + } } // TestOriginSelectorRespectsConfDepth ensures that the origin selector @@ -104,6 +351,9 @@ func TestOriginSelectorRespectsOriginTiming(t *testing.T) { // as the origin, however block `b` is the L1 Head & the sequencer // needs to wait until that block is confirmed enough before advancing. func TestOriginSelectorRespectsConfDepth(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + log := testlog.Logger(t, log.LevelCrit) cfg := &rollup.Config{ MaxSequencerDrift: 500, @@ -127,13 +377,20 @@ func TestOriginSelectorRespectsConfDepth(t *testing.T) { Time: 27, } - l1.ExpectL1BlockRefByHash(a.Hash, a, nil) + // l1.ExpectL1BlockRefByHash(a.Hash, a, nil) confDepthL1 := confdepth.NewConfDepth(10, func() eth.L1BlockRef { return b }, l1) - s := NewL1OriginSelector(log, cfg, confDepthL1) + s := NewL1OriginSelector(ctx, log, cfg, confDepthL1) + s.currentOrigin = a - next, err := s.FindL1Origin(context.Background(), l2Head) + c := make(chan eth.L1BlockRef, 1) + next, err := s.findL1Origin(ctx, l2Head, c) require.Nil(t, err) require.Equal(t, a, next) + + // Wait for the origin selector's background fetch to finish. + // This fetch should not return a new origin because the conf depth has not been met. + _, ok := <-c + require.False(t, ok) } // TestOriginSelectorStrictConfDepth ensures that the origin selector will maintain the sequencer conf depth, @@ -147,6 +404,9 @@ func TestOriginSelectorRespectsConfDepth(t *testing.T) { // This is because 29 (next L2 time) > 20 (origin) + 8 (seq drift) => invalid block. // We maintain confirmation distance, even though we would shift to the next origin if we could. func TestOriginSelectorStrictConfDepth(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + log := testlog.Logger(t, log.LevelCrit) cfg := &rollup.Config{ MaxSequencerDrift: 8, @@ -172,10 +432,20 @@ func TestOriginSelectorStrictConfDepth(t *testing.T) { l1.ExpectL1BlockRefByHash(a.Hash, a, nil) confDepthL1 := confdepth.NewConfDepth(10, func() eth.L1BlockRef { return b }, l1) - s := NewL1OriginSelector(log, cfg, confDepthL1) + s := NewL1OriginSelector(ctx, log, cfg, confDepthL1) - _, err := s.FindL1Origin(context.Background(), l2Head) + c := make(chan eth.L1BlockRef, 1) + _, err := s.findL1Origin(ctx, l2Head, c) require.ErrorContains(t, err, "sequencer time drift") + + // Wait for the origin selector's background fetch to finish. + // This fetch should already be completed because findL1Origin would have waited for it. + select { + case _, ok := <-c: + require.False(t, ok) + default: + t.Fatal("expected the background fetch to have already completed") + } } func u64ptr(n uint64) *uint64 { @@ -187,6 +457,9 @@ func u64ptr(n uint64) *uint64 { // This time the same L1 origin is returned if no new L1 head is seen, instead of an error, // because the Fjord max sequencer drift is higher. func TestOriginSelector_FjordSeqDrift(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + log := testlog.Logger(t, log.LevelCrit) cfg := &rollup.Config{ MaxSequencerDrift: 8, @@ -200,18 +473,32 @@ func TestOriginSelector_FjordSeqDrift(t *testing.T) { Number: 10, Time: 20, } + b := eth.L1BlockRef{ + Hash: common.Hash{'b'}, + Number: 11, + Time: 22, + } l2Head := eth.L2BlockRef{ L1Origin: a.ID(), Time: 27, // next L2 block time would be past pre-Fjord seq drift } - l1.ExpectL1BlockRefByHash(a.Hash, a, nil) - l1.ExpectL1BlockRefByNumber(a.Number+1, eth.L1BlockRef{}, ethereum.NotFound) - s := NewL1OriginSelector(log, cfg, l1) + // This is called as part of the background prefetch job + l1.ExpectL1BlockRefByNumber(a.Number+1, b, nil) + + s := NewL1OriginSelector(ctx, log, cfg, l1) + s.currentOrigin = a - l1O, err := s.FindL1Origin(context.Background(), l2Head) + c := make(chan eth.L1BlockRef, 1) + l1O, err := s.findL1Origin(ctx, l2Head, c) require.NoError(t, err, "with Fjord activated, have increased max seq drift") require.Equal(t, a, l1O) + + // Wait for the origin selector's background fetch to finish. + // This fetch should be triggered because the next origin is not already known. + next, ok := <-c + require.True(t, ok) + require.Equal(t, b, next) } // TestOriginSelectorSeqDriftRespectsNextOriginTime @@ -221,6 +508,63 @@ func TestOriginSelector_FjordSeqDrift(t *testing.T) { // drift, the origin should remain on block `a` because the next origin's // time is greater than the next L2 time. func TestOriginSelectorSeqDriftRespectsNextOriginTime(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + log := testlog.Logger(t, log.LevelCrit) + 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, + } + l2Head := eth.L2BlockRef{ + L1Origin: a.ID(), + Time: 27, + } + + s := NewL1OriginSelector(ctx, log, cfg, l1) + s.currentOrigin = a + s.nextOrigin = b + + c := make(chan eth.L1BlockRef, 1) + next, err := s.findL1Origin(ctx, l2Head, c) + require.Nil(t, err) + require.Equal(t, a, next) + + // Wait for the origin selector's background fetch to finish. + // This fetch should not be triggered because the next origin is already known. + select { + case _, ok := <-c: + require.False(t, ok) + default: + t.Fatal("expected the background fetch to have not run") + } +} + +// TestOriginSelectorSeqDriftRespectsNextOriginTimeNoCache +// +// 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. +// The L1OriginSelector does not have the next origin cached, and must fetch it +// because the max sequencer drift has been exceeded. +func TestOriginSelectorSeqDriftRespectsNextOriginTimeNoCache(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + log := testlog.Logger(t, log.LevelCrit) cfg := &rollup.Config{ MaxSequencerDrift: 8, @@ -244,13 +588,24 @@ func TestOriginSelectorSeqDriftRespectsNextOriginTime(t *testing.T) { Time: 27, } - l1.ExpectL1BlockRefByHash(a.Hash, a, nil) l1.ExpectL1BlockRefByNumber(b.Number, b, nil) - s := NewL1OriginSelector(log, cfg, l1) - next, err := s.FindL1Origin(context.Background(), l2Head) + s := NewL1OriginSelector(ctx, log, cfg, l1) + s.currentOrigin = a + + c := make(chan eth.L1BlockRef, 1) + next, err := s.findL1Origin(ctx, l2Head, c) require.Nil(t, err) require.Equal(t, a, next) + + // Wait for the origin selector's background fetch to finish. + // This fetch should already be completed because findL1Origin would have waited for it. + select { + case _, ok := <-c: + require.False(t, ok) + default: + t.Fatal("expected the background fetch to have already completed") + } } // TestOriginSelectorHandlesLateL1Blocks tests the forced repeat of the previous origin, @@ -263,6 +618,9 @@ func TestOriginSelectorSeqDriftRespectsNextOriginTime(t *testing.T) { // 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) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + log := testlog.Logger(t, log.LevelCrit) cfg := &rollup.Config{ MaxSequencerDrift: 8, @@ -300,23 +658,52 @@ func TestOriginSelectorHandlesLateL1Blocks(t *testing.T) { // 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 := confdepth.NewConfDepth(2, func() eth.L1BlockRef { return l1Head }, l1) - s := NewL1OriginSelector(log, cfg, confDepthL1) + s := NewL1OriginSelector(ctx, log, cfg, confDepthL1) - _, err := s.FindL1Origin(context.Background(), l2Head) + ch := make(chan eth.L1BlockRef, 1) + _, err := s.findL1Origin(ctx, l2Head, ch) require.ErrorContains(t, err, "sequencer time drift") + // Wait for the origin selector's background fetch to finish. + // This fetch should already be completed because findL1Origin would have waited for it. + select { + case _, ok := <-ch: + require.False(t, ok) + default: + t.Fatal("expected the background fetch to have already completed") + } + l1Head = c - _, err = s.FindL1Origin(context.Background(), l2Head) + ch = make(chan eth.L1BlockRef, 1) + _, err = s.findL1Origin(ctx, l2Head, ch) require.ErrorContains(t, err, "sequencer time drift") + // Wait for the origin selector's background fetch to finish. + // This fetch should already be completed because findL1Origin would have waited for it. + select { + case _, ok := <-ch: + require.False(t, ok) + default: + t.Fatal("expected the background fetch to have already completed") + } + l1Head = d - next, err := s.FindL1Origin(context.Background(), l2Head) + ch = make(chan eth.L1BlockRef, 1) + next, err := s.findL1Origin(ctx, l2Head, ch) 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") + + // Wait for the origin selector's background fetch to finish. + // This fetch should already be completed because findL1Origin would have waited for it. + select { + case _, ok := <-ch: + require.False(t, ok) + default: + t.Fatal("expected the background fetch to have already completed") + } }