op-node: refactor sequencer to use EngineControl, test sequencing

op-e2e/actions/l2_sequencer.go

@@ -1,37 +1,52 @@
 package actions
 
 import (
+	"context"
+
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/stretchr/testify/require"
 
 	"github.com/ethereum-optimism/optimism/op-node/eth"
-	"github.com/ethereum-optimism/optimism/op-node/metrics"
 	"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/driver"
 )
 
+// MockL1OriginSelector is a shim to override the origin as sequencer, so we can force it to stay on an older origin.
+type MockL1OriginSelector struct {
+	actual         *driver.L1OriginSelector
+	originOverride eth.L1BlockRef // override which origin gets picked
+}
+
+func (m *MockL1OriginSelector) FindL1Origin(ctx context.Context, l1Head eth.L1BlockRef, l2Head eth.L2BlockRef) (eth.L1BlockRef, error) {
+	if m.originOverride != (eth.L1BlockRef{}) {
+		return m.originOverride, nil
+	}
+	return m.actual.FindL1Origin(ctx, l1Head, l2Head)
+}
+
 // L2Sequencer is an actor that functions like a rollup node,
 // without the full P2P/API/Node stack, but just the derivation state, and simplified driver with sequencing ability.
 type L2Sequencer struct {
 	L2Verifier
 
-	sequencer        *driver.Sequencer
-	l1OriginSelector *driver.L1OriginSelector
-
-	seqOldOrigin bool // stay on current L1 origin when sequencing a block, unless forced to adopt the next origin
+	sequencer *driver.Sequencer
 
 	failL2GossipUnsafeBlock error // mock error
+
+	mockL1OriginSelector *MockL1OriginSelector
 }
 
 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)
+	l1OriginSelector := &MockL1OriginSelector{
+		actual: driver.NewL1OriginSelector(log, cfg, l1, seqConfDepth),
+	}
 	return &L2Sequencer{
 		L2Verifier:              *ver,
-		sequencer:               driver.NewSequencer(log, cfg, eng, ver.derivation, attrBuilder, metrics.NoopMetrics),
-		l1OriginSelector:        driver.NewL1OriginSelector(log, cfg, l1, seqConfDepth),
-		seqOldOrigin:            false,
+		sequencer:               driver.NewSequencer(log, cfg, ver.derivation, attrBuilder, l1OriginSelector),
+		mockL1OriginSelector:    l1OriginSelector,
 		failL2GossipUnsafeBlock: nil,
 	}
 }
@@ -47,22 +62,7 @@ func (s *L2Sequencer) ActL2StartBlock(t Testing) {
 		return
 	}
 
-	parent := s.derivation.UnsafeL2Head()
-	var origin eth.L1BlockRef
-	if s.seqOldOrigin {
-		// force old origin, for testing purposes
-		oldOrigin, err := s.l1.L1BlockRefByHash(t.Ctx(), parent.L1Origin.Hash)
-		require.NoError(t, err, "failed to get current origin: %s", parent.L1Origin)
-		origin = oldOrigin
-		s.seqOldOrigin = false // don't repeat this
-	} else {
-		// select origin the real way
-		l1Origin, err := s.l1OriginSelector.FindL1Origin(t.Ctx(), s.l1State.L1Head(), parent)
-		require.NoError(t, err)
-		origin = l1Origin
-	}
-
-	err := s.sequencer.StartBuildingBlock(t.Ctx(), origin)
+	err := s.sequencer.StartBuildingBlock(t.Ctx(), s.l1State.L1Head())
 	require.NoError(t, err, "failed to start block building")
 
 	s.l2Building = true
@@ -76,24 +76,21 @@ func (s *L2Sequencer) ActL2EndBlock(t Testing) {
 	}
 	s.l2Building = false
 
-	payload, err := s.sequencer.CompleteBuildingBlock(t.Ctx())
+	_, err := s.sequencer.CompleteBuildingBlock(t.Ctx())
 	// TODO: there may be legitimate temporary errors here, if we mock engine API RPC-failure.
 	// For advanced tests we can catch those and print a warning instead.
 	require.NoError(t, err)
 
-	ref, err := derive.PayloadToBlockRef(payload, &s.rollupCfg.Genesis)
-	require.NoError(t, err, "payload must convert to block ref")
-	s.derivation.SetUnsafeHead(ref)
 	// TODO: action-test publishing of payload on p2p
 }
 
 // ActL2KeepL1Origin makes the sequencer use the current L1 origin, even if the next origin is available.
 func (s *L2Sequencer) ActL2KeepL1Origin(t Testing) {
-	if s.seqOldOrigin { // don't do this twice
-		t.InvalidAction("already decided to keep old L1 origin")
-		return
-	}
-	s.seqOldOrigin = true
+	parent := s.derivation.UnsafeL2Head()
+	// force old origin, for testing purposes
+	oldOrigin, err := s.l1.L1BlockRefByHash(t.Ctx(), parent.L1Origin.Hash)
+	require.NoError(t, err, "failed to get current origin: %s", parent.L1Origin)
+	s.mockL1OriginSelector.originOverride = oldOrigin
 }
 
 // ActBuildToL1Head builds empty blocks until (incl.) the L1 head becomes the L2 origin
@@ -109,7 +106,7 @@ func (s *L2Sequencer) ActBuildToL1Head(t Testing) {
 func (s *L2Sequencer) ActBuildToL1HeadExcl(t Testing) {
 	for {
 		s.ActL2PipelineFull(t)
-		nextOrigin, err := s.l1OriginSelector.FindL1Origin(t.Ctx(), s.l1State.L1Head(), s.derivation.UnsafeL2Head())
+		nextOrigin, err := s.mockL1OriginSelector.FindL1Origin(t.Ctx(), s.l1State.L1Head(), s.derivation.UnsafeL2Head())
 		require.NoError(t, err)
 		if nextOrigin.Number >= s.l1State.L1Head().Number {
 			break

op-node/rollup/derive/pipeline.go

@@ -148,13 +148,6 @@ func (dp *DerivationPipeline) BuildingPayload() (onto eth.L2BlockRef, id eth.Pay
 	return dp.eng.BuildingPayload()
 }
 
-// SetUnsafeHead changes the forkchoice state unsafe head, without changing the engine.
-//
-// deprecated: use the EngineControl interface instead.
-func (dp *DerivationPipeline) SetUnsafeHead(head eth.L2BlockRef) {
-	dp.eng.SetUnsafeHead(head)
-}
-
 // AddUnsafePayload schedules an execution payload to be processed, ahead of deriving it from L1
 func (dp *DerivationPipeline) AddUnsafePayload(payload *eth.ExecutionPayload) {
 	dp.eng.AddUnsafePayload(payload)

op-node/rollup/driver/driver.go

@@ -14,7 +14,6 @@ import (
 
 type Metrics interface {
 	RecordPipelineReset()
-	RecordSequencingError()
 	RecordPublishingError()
 	RecordDerivationError()
 
@@ -28,9 +27,8 @@ type Metrics interface {
 	SetDerivationIdle(idle bool)
 
 	RecordL1ReorgDepth(d uint64)
-	CountSequencedTxs(count int)
 
-	SequencerMetrics
+	EngineMetrics
 }
 
 type L1Chain interface {
@@ -48,7 +46,6 @@ type L2Chain interface {
 type DerivationPipeline interface {
 	Reset()
 	Step(ctx context.Context) error
-	SetUnsafeHead(head eth.L2BlockRef)
 	AddUnsafePayload(payload *eth.ExecutionPayload)
 	Finalize(ref eth.L1BlockRef)
 	FinalizedL1() eth.L1BlockRef
@@ -68,14 +65,12 @@ type L1StateIface interface {
 	L1Finalized() eth.L1BlockRef
 }
 
-type L1OriginSelectorIface interface {
-	FindL1Origin(ctx context.Context, l1Head eth.L1BlockRef, l2Head eth.L2BlockRef) (eth.L1BlockRef, error)
-}
-
 type SequencerIface interface {
-	StartBuildingBlock(ctx context.Context, l1Origin eth.L1BlockRef) error
+	StartBuildingBlock(ctx context.Context, l1Head eth.L1BlockRef) error
 	CompleteBuildingBlock(ctx context.Context) (*eth.ExecutionPayload, error)
-	PlanNextSequencerAction(sequenceErr error) (delay time.Duration, seal bool, onto eth.BlockID)
+	PlanNextSequencerAction() time.Duration
+	RunNextSequencerAction(ctx context.Context, l1Head eth.L1BlockRef) *eth.ExecutionPayload
+	BuildingOnto() eth.L2BlockRef
 }
 
 type Network interface {
@@ -90,7 +85,10 @@ func NewDriver(driverCfg *Config, cfg *rollup.Config, l2 L2Chain, l1 L1Chain, ne
 	verifConfDepth := NewConfDepth(driverCfg.VerifierConfDepth, l1State.L1Head, l1)
 	derivationPipeline := derive.NewDerivationPipeline(log, cfg, verifConfDepth, l2, metrics)
 	attrBuilder := derive.NewFetchingAttributesBuilder(cfg, l1, l2)
-	sequencer := NewSequencer(log, cfg, l2, derivationPipeline, attrBuilder, metrics)
+	engine := derivationPipeline
+	meteredEngine := NewMeteredEngine(cfg, engine, metrics, log)
+	sequencer := NewSequencer(log, cfg, meteredEngine, attrBuilder, findL1Origin)
+
 	return &Driver{
 		l1State:          l1State,
 		derivation:       derivationPipeline,
@@ -106,7 +104,6 @@ func NewDriver(driverCfg *Config, cfg *rollup.Config, l2 L2Chain, l1 L1Chain, ne
 		snapshotLog:      snapshotLog,
 		l1:               l1,
 		l2:               l2,
-		l1OriginSelector: findL1Origin,
 		sequencer:        sequencer,
 		network:          network,
 		metrics:          metrics,