feat(node): implement engine P2P sync mode #204
Conversation
seolaoh
force-pushed
the
feat/enable-engine-p2p-sync
branch
from
cd3a02d to
f941fcd
Compare
| @@ -165,6 +172,11 @@ func (eq *EngineQueue) SetUnsafeHead(head eth.L2BlockRef) { | |||
| eq.metrics.RecordL2Ref("l2_unsafe", head) | |||
| } | |||
|
|
|||
| func (eq *EngineQueue) SetEngineSyncTarget(head eth.L2BlockRef) { | |||
There was a problem hiding this comment.
@seolaoh Is there a known use case for this function?
There was a problem hiding this comment.
It is not used, but I think it is added just like SetUnsafeHead.
| L2EngineSyncEnabled = &cli.BoolFlag{ | ||
| Name: "l2.engine-sync", | ||
| Usage: "Enables or disables execution engine P2P sync", | ||
| EnvVars: prefixEnvVar("L2_ENGINE_SYNC_ENABLED"), | ||
| Required: false, | ||
| } | ||
| SkipSyncStartCheck = &cli.BoolFlag{ | ||
| Name: "l2.skip-sync-start-check", | ||
| Usage: "Skip 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", | ||
| EnvVars: prefixEnvVar("L2_SKIP_SYNC_START_CHECK"), | ||
| Required: false, | ||
| } |
There was a problem hiding this comment.
When seeing this, the default value has been set to false. But no default value is set in our PR. Do you have any specific reason?
There was a problem hiding this comment.
Because when do not set Value of cli.BoolFlag, the default value is false.
| fc := eth.ForkchoiceState{ | ||
| HeadBlockHash: eq.unsafeHead.Hash, | ||
| HeadBlockHash: eq.engineSyncTarget.Hash, |
There was a problem hiding this comment.
This tryUpdateEngine is called when fork choice is needed.
But can we sure that engineSyncTarget is always same with the unsafeHead when sync is done?
There was a problem hiding this comment.
During engine is syncing, when a node receives unsafe payload, node continuously requests geth to update unsafe block using tryUpdateEngine. In this case, eq.unsafeHead is not changed but eq.engineSyncTarget is updated with the newly received unsafe payload. So here we use eq.engineSyncTarget. And after engine has finished syncing, a node requests geth to update safe and finalized block using tryUpdateEngine. When eq.engineSyncTarget and eq.unsafeHead is different, it will do not tryUpdateEngine thanks to the line 257-260.
seolaoh
force-pushed
the
feat/enable-engine-p2p-sync
branch
2 times, most recently
from
81ba988 to
c0cc65e
Compare
- Add new flag `engine-sync` and `sync.Config` struct for engine p2p sync - Modify `EngineQueue` to support engine p2p sync - Add e2e test cases - Modify related components to pass sync config - Modify execution engine specs - Fix reorg depth check, Add `skip-sync-start-check` flag See Optimism PR#6243.
seolaoh
force-pushed
the
feat/enable-engine-p2p-sync
branch
from
c0cc65e to
8535324
Compare
|
This PR should be merged after #212 is merged! |
Description
Enable to trigger the Execution Engine’s P2P sync for a faster sync and Happy-path sync with newly introduced flag
L2_ENGINE_SYNC_ENABLED.Also there is another related branch in kroma-geth repository to support snap sync.
See Optimism PR#6243.