Skip to content

Commit

Permalink
op-node: move engine controller into its own package (#10782)
Browse files Browse the repository at this point in the history
  • Loading branch information
protolambda authored Jun 11, 2024
1 parent 52d6e4b commit c9fd3b4
Show file tree
Hide file tree
Showing 20 changed files with 220 additions and 209 deletions.
15 changes: 8 additions & 7 deletions op-e2e/actions/l2_verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/ethereum-optimism/optimism/op-node/rollup/clsync"
"github.com/ethereum-optimism/optimism/op-node/rollup/derive"
"github.com/ethereum-optimism/optimism/op-node/rollup/driver"
"github.com/ethereum-optimism/optimism/op-node/rollup/engine"
"github.com/ethereum-optimism/optimism/op-node/rollup/finality"
"github.com/ethereum-optimism/optimism/op-node/rollup/sync"
"github.com/ethereum-optimism/optimism/op-service/client"
Expand All @@ -31,19 +32,19 @@ type L2Verifier struct {
log log.Logger

eng interface {
derive.Engine
engine.Engine
L2BlockRefByNumber(ctx context.Context, num uint64) (eth.L2BlockRef, error)
}

syncDeriver *driver.SyncDeriver

// L2 rollup
engine *derive.EngineController
engine *engine.EngineController
derivation *derive.DerivationPipeline
clSync *clsync.CLSync

attributesHandler driver.AttributesHandler
safeHeadListener derive.SafeHeadListener
safeHeadListener rollup.SafeHeadListener
finalizer driver.Finalizer
syncCfg *sync.Config

Expand All @@ -61,7 +62,7 @@ type L2Verifier struct {
}

type L2API interface {
derive.Engine
engine.Engine
L2BlockRefByNumber(ctx context.Context, num uint64) (eth.L2BlockRef, error)
InfoByHash(ctx context.Context, hash common.Hash) (eth.BlockInfo, error)
// GetProof returns a proof of the account, it may return a nil result without error if the address was not found.
Expand All @@ -70,13 +71,13 @@ type L2API interface {
}

type safeDB interface {
derive.SafeHeadListener
rollup.SafeHeadListener
node.SafeDBReader
}

func NewL2Verifier(t Testing, log log.Logger, l1 derive.L1Fetcher, blobsSrc derive.L1BlobsFetcher, plasmaSrc driver.PlasmaIface, eng L2API, cfg *rollup.Config, syncCfg *sync.Config, safeHeadListener safeDB) *L2Verifier {
metrics := &testutils.TestDerivationMetrics{}
engine := derive.NewEngineController(eng, log, metrics, cfg, syncCfg.SyncMode)
engine := engine.NewEngineController(eng, log, metrics, cfg, syncCfg.SyncMode)

clSync := clsync.NewCLSync(log, cfg, metrics, engine)

Expand Down Expand Up @@ -273,7 +274,7 @@ func (s *L2Verifier) ActL2PipelineStep(t Testing) {
} else if err != nil && errors.Is(err, derive.ErrReset) {
s.log.Warn("Derivation pipeline is reset", "err", err)
s.derivation.Reset()
if err := derive.ResetEngine(t.Ctx(), s.log, s.rollupCfg, s.engine, s.l1, s.eng, s.syncCfg, s.safeHeadListener); err != nil {
if err := engine.ResetEngine(t.Ctx(), s.log, s.rollupCfg, s.engine, s.l1, s.eng, s.syncCfg, s.safeHeadListener); err != nil {
s.log.Error("Derivation pipeline not ready, failed to reset engine", "err", err)
// Derivation-pipeline will return a new ResetError until we confirm the engine has been successfully reset.
return
Expand Down
11 changes: 5 additions & 6 deletions op-node/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ import (
"sync/atomic"
"time"

"github.com/ethereum-optimism/optimism/op-node/node/safedb"
"github.com/ethereum-optimism/optimism/op-node/rollup/derive"
plasma "github.com/ethereum-optimism/optimism/op-plasma"
"github.com/ethereum-optimism/optimism/op-service/httputil"

"github.com/hashicorp/go-multierror"
"github.com/libp2p/go-libp2p/core/peer"

Expand All @@ -22,13 +17,17 @@ import (

"github.com/ethereum-optimism/optimism/op-node/heartbeat"
"github.com/ethereum-optimism/optimism/op-node/metrics"
"github.com/ethereum-optimism/optimism/op-node/node/safedb"
"github.com/ethereum-optimism/optimism/op-node/p2p"
"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-node/rollup/conductor"
"github.com/ethereum-optimism/optimism/op-node/rollup/driver"
"github.com/ethereum-optimism/optimism/op-node/rollup/sync"
"github.com/ethereum-optimism/optimism/op-node/version"
plasma "github.com/ethereum-optimism/optimism/op-plasma"
"github.com/ethereum-optimism/optimism/op-service/client"
"github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum-optimism/optimism/op-service/httputil"
"github.com/ethereum-optimism/optimism/op-service/oppprof"
"github.com/ethereum-optimism/optimism/op-service/retry"
"github.com/ethereum-optimism/optimism/op-service/sources"
Expand All @@ -37,7 +36,7 @@ import (
var ErrAlreadyClosed = errors.New("node is already closed")

type closableSafeDB interface {
derive.SafeHeadListener
rollup.SafeHeadListener
SafeDBReader
io.Closer
}
Expand Down
9 changes: 5 additions & 4 deletions op-node/rollup/attributes/attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ import (
"github.com/ethereum-optimism/optimism/op-node/rollup/async"
"github.com/ethereum-optimism/optimism/op-node/rollup/conductor"
"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"
)

type Engine interface {
derive.EngineControl
engine.EngineControl

SetUnsafeHead(eth.L2BlockRef)
SetSafeHead(eth.L2BlockRef)
Expand Down Expand Up @@ -146,13 +147,13 @@ func (eq *AttributesHandler) forceNextSafeAttributes(ctx context.Context, attrib
}
if err != nil {
switch errType {
case derive.BlockInsertTemporaryErr:
case engine.BlockInsertTemporaryErr:
// RPC errors are recoverable, we can retry the buffered payload attributes later.
return derive.NewTemporaryError(fmt.Errorf("temporarily cannot insert new safe block: %w", err))
case derive.BlockInsertPrestateErr:
case engine.BlockInsertPrestateErr:
_ = eq.ec.CancelPayload(ctx, true)
return derive.NewResetError(fmt.Errorf("need reset to resolve pre-state problem: %w", err))
case derive.BlockInsertPayloadErr:
case engine.BlockInsertPayloadErr:
_ = eq.ec.CancelPayload(ctx, true)
eq.log.Warn("could not process payload derived from L1 data, dropping batch", "err", err)
// Count the number of deposits to see if the tx list is deposit only.
Expand Down
15 changes: 8 additions & 7 deletions op-node/rollup/attributes/attributes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"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/engine"
"github.com/ethereum-optimism/optimism/op-node/rollup/sync"
"github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum-optimism/optimism/op-service/testlog"
Expand Down Expand Up @@ -181,7 +182,7 @@ func TestAttributesHandler(t *testing.T) {
t.Run("drop stale attributes", func(t *testing.T) {
logger := testlog.Logger(t, log.LevelInfo)
eng := &testutils.MockEngine{}
ec := derive.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ec := engine.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ah := NewAttributesHandler(logger, cfg, ec, eng)
defer eng.AssertExpectations(t)

Expand All @@ -195,7 +196,7 @@ func TestAttributesHandler(t *testing.T) {
t.Run("pending gets reorged", func(t *testing.T) {
logger := testlog.Logger(t, log.LevelInfo)
eng := &testutils.MockEngine{}
ec := derive.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ec := engine.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ah := NewAttributesHandler(logger, cfg, ec, eng)
defer eng.AssertExpectations(t)

Expand All @@ -210,7 +211,7 @@ func TestAttributesHandler(t *testing.T) {
t.Run("consolidation fails", func(t *testing.T) {
logger := testlog.Logger(t, log.LevelInfo)
eng := &testutils.MockEngine{}
ec := derive.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ec := engine.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ah := NewAttributesHandler(logger, cfg, ec, eng)

ec.SetUnsafeHead(refA1)
Expand Down Expand Up @@ -264,7 +265,7 @@ func TestAttributesHandler(t *testing.T) {
fn := func(t *testing.T, lastInSpan bool) {
logger := testlog.Logger(t, log.LevelInfo)
eng := &testutils.MockEngine{}
ec := derive.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ec := engine.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ah := NewAttributesHandler(logger, cfg, ec, eng)

ec.SetUnsafeHead(refA1)
Expand Down Expand Up @@ -323,7 +324,7 @@ func TestAttributesHandler(t *testing.T) {

logger := testlog.Logger(t, log.LevelInfo)
eng := &testutils.MockEngine{}
ec := derive.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ec := engine.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ah := NewAttributesHandler(logger, cfg, ec, eng)

ec.SetUnsafeHead(refA0)
Expand Down Expand Up @@ -374,7 +375,7 @@ func TestAttributesHandler(t *testing.T) {

logger := testlog.Logger(t, log.LevelInfo)
eng := &testutils.MockEngine{}
ec := derive.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ec := engine.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ah := NewAttributesHandler(logger, cfg, ec, eng)

ec.SetUnsafeHead(refA0)
Expand All @@ -398,7 +399,7 @@ func TestAttributesHandler(t *testing.T) {
t.Run("no attributes", func(t *testing.T) {
logger := testlog.Logger(t, log.LevelInfo)
eng := &testutils.MockEngine{}
ec := derive.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ec := engine.NewEngineController(eng, logger, metrics.NoopMetrics, cfg, sync.CLSync)
ah := NewAttributesHandler(logger, cfg, ec, eng)
defer eng.AssertExpectations(t)

Expand Down
3 changes: 2 additions & 1 deletion op-node/rollup/clsync/clsync.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ 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-service/eth"
)

Expand All @@ -20,7 +21,7 @@ type Metrics interface {
}

type Engine interface {
derive.EngineState
engine.EngineState
InsertUnsafePayload(ctx context.Context, payload *eth.ExecutionPayloadEnvelope, ref eth.L2BlockRef) error
}

Expand Down
114 changes: 0 additions & 114 deletions op-node/rollup/derive/iface.go

This file was deleted.

10 changes: 10 additions & 0 deletions op-node/rollup/derive/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/log"

"github.com/ethereum-optimism/optimism/op-node/rollup"
Expand Down Expand Up @@ -36,6 +37,15 @@ type ResettableStage interface {
Reset(ctx context.Context, base eth.L1BlockRef, baseCfg eth.SystemConfig) error
}

type L2Source interface {
PayloadByHash(context.Context, common.Hash) (*eth.ExecutionPayloadEnvelope, error)
PayloadByNumber(context.Context, uint64) (*eth.ExecutionPayloadEnvelope, error)
L2BlockRefByLabel(ctx context.Context, label eth.BlockLabel) (eth.L2BlockRef, error)
L2BlockRefByHash(ctx context.Context, l2Hash common.Hash) (eth.L2BlockRef, error)
L2BlockRefByNumber(ctx context.Context, num uint64) (eth.L2BlockRef, error)
SystemConfigL2Fetcher
}

// DerivationPipeline is updated with new L1 data, and the Step() function can be iterated on to generate attributes
type DerivationPipeline struct {
log log.Logger
Expand Down
2 changes: 0 additions & 2 deletions op-node/rollup/derive/pipeline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package derive

import "github.com/ethereum-optimism/optimism/op-service/testutils"

var _ Engine = (*testutils.MockEngine)(nil)

var _ L1Fetcher = (*testutils.MockL1Source)(nil)

var _ Metrics = (*testutils.TestDerivationMetrics)(nil)
Loading

0 comments on commit c9fd3b4

Please sign in to comment.