Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimistic sync p1 - attest only validated head #3434

Merged
merged 24 commits into from
Dec 15, 2021

Conversation

g11tech
Copy link
Contributor

@g11tech g11tech commented Nov 16, 2021

Motivation
There is a high possibility that EL is syncing while CL asks for execution payloads validation (or preparation in case of publish block).

Optimistic sync is for CL to accept the blocks even when the status is not known (tracked via payloadStatusUnknown).
However when the calls to EL for executing payload stops responding as syncing, the payloads status/forkchoice needs to be updated

Checklist:

  • Track valid/invalid/syncing and flow the information to the forkChoice/protoarray
  • DONOT participate in the if the head's payload status unkown or EL failure
    • block production - if merge not complete still propose a pre-merge block and keep chain going otherwise fail hard
    • attestation,
    • sync committee (sort of done w.r.t what other implementations have been doing i.e. publishing signature but failing on contribution)
      • publish contribution

Considered and Rejected
~~-[ ] Fail hard on invalid executePayload call after EL itself prepared payload via getPayload ~~ publishBlock will fail for now, could be escalated later
To be moved to other Task/PR:

Description
UPDATE: working smoothly (producing blocks etc) on a devnet3 node

Closes #3505

@codeclimate
Copy link

codeclimate bot commented Nov 16, 2021

Code Climate has analyzed commit dd66931 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@codecov
Copy link

codecov bot commented Nov 16, 2021

Codecov Report

Merging #3434 (dd66931) into master (5751ee5) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3434      +/-   ##
==========================================
- Coverage   37.90%   37.86%   -0.04%     
==========================================
  Files         308      308              
  Lines        8129     8144      +15     
  Branches     1248     1252       +4     
==========================================
+ Hits         3081     3084       +3     
- Misses       4900     4912      +12     
  Partials      148      148              

@g11tech g11tech force-pushed the koptsync branch 2 times, most recently from 72976e7 to 20b8273 Compare December 2, 2021 10:46
@g11tech g11tech marked this pull request as ready for review December 10, 2021 12:41
@g11tech g11tech changed the title kintsugi optimistic sync optimistic sync - part 1: track head's execution status and publish/contribute/attest only on non syncing head Dec 10, 2021
@dapplion dapplion changed the title optimistic sync - part 1: track head's execution status and publish/contribute/attest only on non syncing head optimistic sync p1 - attest only validated headtrack Dec 13, 2021
@dapplion dapplion changed the title optimistic sync p1 - attest only validated headtrack optimistic sync p1 - attest only validated head Dec 13, 2021
dapplion
dapplion previously approved these changes Dec 15, 2021
@dapplion dapplion merged commit b06c013 into ChainSafe:master Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimistic sync: Track Execution status and don't publish/produce on a syncing head
2 participants