feat(x/evmengine): support exec engine snap sync #506
Conversation
0xHansLee
requested review from
LeoHChen,
edisonz0718,
gezerui0124,
jdubpark,
limengformal and
ramtinms
as code owners
0xHansLee
force-pushed
the
hans/support-state-sync
branch
from
eab2045 to
bb9b865
Compare
0xHansLee
force-pushed
the
hans/support-state-sync
branch
from
bb9b865 to
0f74e9e
Compare
| UpdatedAt time.Time | ||
| } | ||
|
|
||
| isExecEngSyncing bool |
There was a problem hiding this comment.
Just for learning the code base, this keeper is always on a single go routine ?
0xHansLee
force-pushed
the
hans/support-state-sync
branch
from
0f74e9e to
a090b44
Compare
Binary uploaded successfully 🎉📦 Version Name: 1.1.2-unstable-b29fa5f |
| } else if isSyncing(fcr.PayloadStatus) { | ||
| log.Warn(ctx, "Processing finalized payload halted while evm syncing (will retry)", nil, "payload_height", payload.Number) | ||
|
|
||
| return false, nil // Retry |
There was a problem hiding this comment.
Does snap sync require return false, nil // Retry line to be removed here?
Asking since this changes the expected behavior. Originally, we expected isSyncing == true to retry the fetch logic. But removing return false, nil means retryForever will stop retrying when the node is syncing (or accepted) and continue to the next logic of delivering EL events to CL handlers.
I believe this will lead to non-determinism for a node if its execution client keeps returning SYNCING or ACCEPTED and eventually exceed the expected block time and progresses to the next block with non-updated execution client (which shouldn't happen in the original behavior, where the node should hang as it awaits syncing).
There was a problem hiding this comment.
Yes it is required for snap sync by requesting forkChoiceUpdated with the latest block.
The forkChoiceUpdated here is just to set safe and finalized block of EL chains. No fetched data is not required and used for finalized CL block. Only the data from finalized msg is used to process FinalizeBlock. That means, in CL perspective, it is fine to process the block with the finalized msg (just EL is syncing), not leading to non-determinism.
As I mentioned in the trust issue, the non-updated payload is not imported immediately. It is stashed for the update later by forkChoiceUpdated.
Previously, when the execution engine was syncing, it would keep retrying until evm syncing is done. However, evm syncing is stuck while downloading chain data because the requested head falls behind by more than 128 blocks, making it impossible to retrieve the missing data from other peers. To support snap sync, `newPayloadV3` and `forkChoiceUpdatedV3` requests are now made with the latest head while the execution engine is syncing. During syncing, `prepareProposal`, `processProposal`, and `postFinalize` are skipped. issue: piplabs#505
Previously, when the execution engine was syncing, it would keep retrying until evm syncing is done. However, evm syncing is stuck while downloading chain data because the requested head falls behind by more than 128 blocks, making it impossible to retrieve the missing data from other peers.
To support snap sync,
newPayloadV3andforkChoiceUpdatedV3requests are now made with the latest head while the execution engine is syncing. During syncing,prepareProposal,processProposal, andpostFinalizeare skipped.issue: #505