Skip to content

Commit

Permalink
op-node: Add sycmode flag and remove old snap sync flag
Browse files Browse the repository at this point in the history
  • Loading branch information
trianglesphere committed Nov 30, 2023
1 parent 99cc9a7 commit e52b320
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 20 deletions.
2 changes: 1 addition & 1 deletion op-e2e/actions/l2_verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func (s *L2Verifier) ActL2PipelineStep(t Testing) {

s.l2PipelineIdle = false
err := s.derivation.Step(t.Ctx())
if err == io.EOF || (err != nil && errors.Is(err, derive.EngineP2PSyncing)) {
if err == io.EOF || (err != nil && errors.Is(err, derive.EngineELSyncing)) {
s.l2PipelineIdle = true
return
} else if err != nil && errors.Is(err, derive.NotEnoughData) {
Expand Down
2 changes: 1 addition & 1 deletion op-e2e/actions/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func TestEngineP2PSync(gt *testing.T) {

miner, seqEng, sequencer := setupSequencerTest(t, sd, log)
// Enable engine P2P sync
_, verifier := setupVerifier(t, sd, log, miner.L1Client(t, sd.RollupCfg), &sync.Config{EngineSync: true})
_, verifier := setupVerifier(t, sd, log, miner.L1Client(t, sd.RollupCfg), &sync.Config{SyncMode: sync.ELSync})

seqEngCl, err := sources.NewEngineClient(seqEng.RPCClient(), log, nil, sources.EngineClientDefaultConfig(sd.RollupCfg))
require.NoError(t, err)
Expand Down
16 changes: 15 additions & 1 deletion op-node/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"time"

"github.com/ethereum-optimism/optimism/op-node/chaincfg"
"github.com/ethereum-optimism/optimism/op-node/rollup/sync"
openum "github.com/ethereum-optimism/optimism/op-service/enum"
oplog "github.com/ethereum-optimism/optimism/op-service/log"
"github.com/ethereum-optimism/optimism/op-service/sources"
Expand Down Expand Up @@ -45,6 +46,16 @@ var (
EnvVars: prefixEnvVars("NETWORK"),
}
/* Optional Flags */
SyncModeFlag = &cli.GenericFlag{
Name: "syncmode",
Usage: "IN DEVELOPMENT: Options are l1-only, consensus-layer, execution-layer.",
EnvVars: prefixEnvVars("SYNCMODE"),
Value: func() *sync.Mode {
out := sync.CLSync
return &out
}(),
Hidden: true,
}
RPCListenAddr = &cli.StringFlag{
Name: "rpc.addr",
Usage: "RPC listening address",
Expand Down Expand Up @@ -228,12 +239,14 @@ var (
EnvVars: prefixEnvVars("L2_BACKUP_UNSAFE_SYNC_RPC_TRUST_RPC"),
Required: false,
}
// Delete this flag at a later date.
L2EngineSyncEnabled = &cli.BoolFlag{
Name: "l2.engine-sync",
Usage: "Enables or disables execution engine P2P sync",
Usage: "WARNING: Deprecated. Use --syncmode=snap instead",
EnvVars: prefixEnvVars("L2_ENGINE_SYNC_ENABLED"),
Required: false,
Value: false,
Hidden: true,
}
SkipSyncStartCheck = &cli.BoolFlag{
Name: "l2.skip-sync-start-check",
Expand Down Expand Up @@ -273,6 +286,7 @@ var requiredFlags = []cli.Flag{
}

var optionalFlags = []cli.Flag{
SyncModeFlag,
RPCListenAddr,
RPCListenPort,
RollupConfig,
Expand Down
10 changes: 5 additions & 5 deletions op-node/rollup/derive/engine_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func (eq *EngineQueue) Step(ctx context.Context) error {
}
if eq.isEngineSyncing() {
// Make pipeline first focus to sync unsafe blocks to engineSyncTarget
return EngineP2PSyncing
return EngineELSyncing
}
if eq.safeAttributes != nil {
return eq.tryNextSafeAttributes(ctx)
Expand Down Expand Up @@ -458,8 +458,8 @@ func (eq *EngineQueue) tryUpdateEngine(ctx context.Context) error {
// checkNewPayloadStatus checks returned status of engine_newPayloadV1 request for next unsafe payload.
// It returns true if the status is acceptable.
func (eq *EngineQueue) checkNewPayloadStatus(status eth.ExecutePayloadStatus) bool {
if eq.syncCfg.EngineSync {
// Allow SYNCING and ACCEPTED if engine P2P sync is enabled
if eq.syncCfg.SyncMode == sync.ELSync {
// Allow SYNCING and ACCEPTED if engine EL sync is enabled
return status == eth.ExecutionValid || status == eth.ExecutionSyncing || status == eth.ExecutionAccepted
}
return status == eth.ExecutionValid
Expand All @@ -468,7 +468,7 @@ func (eq *EngineQueue) checkNewPayloadStatus(status eth.ExecutePayloadStatus) bo
// checkForkchoiceUpdatedStatus checks returned status of engine_forkchoiceUpdatedV1 request for next unsafe payload.
// It returns true if the status is acceptable.
func (eq *EngineQueue) checkForkchoiceUpdatedStatus(status eth.ExecutePayloadStatus) bool {
if eq.syncCfg.EngineSync {
if eq.syncCfg.SyncMode == sync.ELSync {
// Allow SYNCING if engine P2P sync is enabled
return status == eth.ExecutionValid || status == eth.ExecutionSyncing
}
Expand All @@ -490,7 +490,7 @@ func (eq *EngineQueue) tryNextUnsafePayload(ctx context.Context) error {
}

// Ensure that the unsafe payload builds upon the current unsafe head
if !eq.syncCfg.EngineSync && first.ParentHash != eq.unsafeHead.Hash {
if eq.syncCfg.SyncMode != sync.ELSync && first.ParentHash != eq.unsafeHead.Hash {
if uint64(first.BlockNumber) == eq.unsafeHead.Number+1 {
eq.log.Info("skipping unsafe payload, since it does not build onto the existing unsafe chain", "safe", eq.safeHead.ID(), "unsafe", first.ID(), "payload", first.ID())
eq.unsafePayloads.Pop()
Expand Down
4 changes: 2 additions & 2 deletions op-node/rollup/derive/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,5 @@ var ErrCritical = NewCriticalError(nil)
// but if it is retried enough times, it will eventually return a real value or io.EOF
var NotEnoughData = errors.New("not enough data")

// EngineP2PSyncing implies that the execution engine is currently in progress of P2P sync.
var EngineP2PSyncing = errors.New("engine is P2P syncing")
// EngineELSyncing implies that the execution engine is currently in progress of syncing.
var EngineELSyncing = errors.New("engine is performing EL sync")
4 changes: 2 additions & 2 deletions op-node/rollup/derive/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,12 @@ func (dp *DerivationPipeline) Step(ctx context.Context) error {
return nil
}
}

// Now step the engine queue. It will pull earlier data as needed.
if err := dp.eng.Step(ctx); err == io.EOF {
// If every stage has returned io.EOF, try to advance the L1 Origin
return dp.traversal.AdvanceL1Block(ctx)
} else if errors.Is(err, EngineP2PSyncing) {
} else if errors.Is(err, EngineELSyncing) {
// Log that we are still waiting for EL to sync?
return err
} else if err != nil {
return fmt.Errorf("engine stage failed: %w", err)
Expand Down
2 changes: 1 addition & 1 deletion op-node/rollup/driver/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ func (s *Driver) eventLoop() {
stepAttempts = 0
s.metrics.SetDerivationIdle(true)
continue
} else if err != nil && errors.Is(err, derive.EngineP2PSyncing) {
} else if err != nil && errors.Is(err, derive.EngineELSyncing) {
s.log.Debug("Derivation process went idle because the engine is syncing", "progress", s.derivation.Origin(), "sync_target", s.derivation.EngineSyncTarget(), "err", err)
stepAttempts = 0
s.metrics.SetDerivationIdle(true)
Expand Down
52 changes: 49 additions & 3 deletions op-node/rollup/sync/config.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,54 @@
package sync

import (
"fmt"
"slices"
)

type Mode string

// There are three kinds of sync mode that the op-node does:
// 1. In l1-only (L1) sync mode, the op-node only derives data from L1 & does not import unsafe blocks.
// 2. In consensus-layer (CL) sync, the op-node fully drives the execution client and imports unsafe blocks &
// fetches unsafe blocks that it has missed.
// 3. In execution-layer (EL) sync, the op-node tells the execution client to sync towards the tip of the chain.
// It will consolidate the chain as usual. This allows execution clients to snap sync if they are capable of it.
const (
// L1Sync Mode = "l1-only" // L1 Sync is ignored for now. Will need more modifications to imlement.
CLSync Mode = "consensus-layer"
ELSync Mode = "execution-layer"
)

var Modes = []Mode{CLSync, ELSync}

func (m Mode) String() string {
return string(m)
}

func (m *Mode) Set(value string) error {
if !ValidMode(Mode(value)) {
return fmt.Errorf("unknown sync mode: %q", value)
}
*m = Mode(value)
return nil
}

func (m *Mode) Clone() any {
cpy := *m
return &cpy
}

func ValidMode(value Mode) bool {
return slices.Contains(Modes, value)
}

type Config struct {
// EngineSync is true when the EngineQueue can trigger execution engine P2P sync.
EngineSync bool `json:"engine_sync"`
// SkipSyncStartCheck skip the sanity check of consistency of L1 origins of the unsafe L2 blocks when determining the sync-starting point. This defers the L1-origin verification, and is recommended to use in when utilizing l2.engine-sync
// SyncMode is defined above.
SyncMode Mode `json:"syncmode"`
// SkipSyncStartCheck skip the sanity check of consistency of L1 origins of the unsafe L2 blocks when determining the sync-starting point.
// This defers the L1-origin verification, and is recommended to use in when utilizing --syncmode=EL on op-node and --syncmode=snap on op-geth
// Warning: This will be removed when we implement proper checkpoints.
// Note: We probably need to detect the condition that snap sync has not complete when we do a restart prior to running sync-start if we are doing
// snap sync with a genesis finalization data.
SkipSyncStartCheck bool `json:"skip_sync_start_check"`
}
22 changes: 18 additions & 4 deletions op-node/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package opnode
import (
"crypto/rand"
"encoding/json"
"errors"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -64,7 +65,10 @@ func NewConfig(ctx *cli.Context, log log.Logger) (*node.Config, error) {

l2SyncEndpoint := NewL2SyncEndpointConfig(ctx)

syncConfig := NewSyncConfig(ctx)
syncConfig, err := NewSyncConfig(ctx, log)
if err != nil {
return nil, fmt.Errorf("failed to create the sync config: %w", err)
}

haltOption := ctx.String(flags.RollupHalt.Name)
if haltOption == "none" {
Expand Down Expand Up @@ -243,9 +247,19 @@ func NewSnapshotLogger(ctx *cli.Context) (log.Logger, error) {
return logger, nil
}

func NewSyncConfig(ctx *cli.Context) *sync.Config {
return &sync.Config{
EngineSync: ctx.Bool(flags.L2EngineSyncEnabled.Name),
func NewSyncConfig(ctx *cli.Context, log log.Logger) (*sync.Config, error) {
if ctx.IsSet(flags.L2EngineSyncEnabled.Name) && ctx.IsSet(flags.SyncModeFlag.Name) {
return nil, errors.New("cannot set both --l2.engine-sync and --syncmode at the same time.")
} else if ctx.IsSet(flags.L2EngineSyncEnabled.Name) {
log.Error("l2.engine-sync is deprecated and will be removed in a future release. Use --syncmode=execution-layer instead.")
}
cfg := &sync.Config{
SyncMode: sync.Mode(strings.ToLower(ctx.String(flags.SyncModeFlag.Name))),
SkipSyncStartCheck: ctx.Bool(flags.SkipSyncStartCheck.Name),
}
if ctx.Bool(flags.L2EngineSyncEnabled.Name) {
cfg.SyncMode = sync.ELSync
}

return cfg, nil
}

0 comments on commit e52b320

Please sign in to comment.