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

Check init sync before getting payload attributes #13479

Merged
merged 3 commits into from
Jan 17, 2024
Merged

Conversation

potuz
Copy link
Contributor

@potuz potuz commented Jan 17, 2024

This PR adds a helper to forkchoice to return the delay of the latest imported block. It also adds a helper with an heuristic to check if the node is during init sync. If the highest imported node was imported with a delay of less than an epoch then the node is considered in regular sync. If on the other hand, in addition the highest imported node is more than two epochs old, then the node is considered in init Sync.

The helper to check this only uses forkchoice and therefore requires a read lock. There are four paths to call this

  1. During regular block processing, we defer a function to send the
    second FCU call with attributes. This function may not be called at
    all if we are not regularly syncing
  2. During regular block processing, we check in the path
    postBlockProces->getFCUArgs->computePayloadAttributes the payload
    attributes if we are syncing a late block. In this case forkchoice is already locked and we add a call in getFCUArgs to return early if not regularly syncing
  3. During handling of late blocks on lateBlockTasks we simply return
    early if not in regular sync (This is the biggest change as it takes
    a longer FC lock for lateBlockTasks)
  4. On Attestation processing, in UpdateHead, we are already locked so we
    just add a check to not update head on this path if not regularly
    syncing.

What type of PR is this?

Uncomment one line below and remove others.

Bug fix
Feature
Documentation
Other

What does this PR do? Why is it needed?

Which issues(s) does this PR fix?

Fixes #

Other notes for review

This PR adds a helper to forkchoice to return the delay of the latest
imported block. It also adds a helper with an heuristic to check if the
node is during init sync. If the highest imported node was imported with
a delay of less than an epoch then the node is considered in regular
sync. If on the other hand, in addition the highest imported node is
more than two epochs old, then the node is considered in init Sync.

The helper to check this only uses forkchoice and therefore requires a
read lock. There are four paths to call this

1) During regular block processing, we defer a function to send the
   second FCU call with attributes. This function may not be called at
all if we are not regularly syncing
2) During regular block processing, we check in the path
   `postBlockProces->getFCUArgs->computePayloadAttributes` the payload
attributes if we are syncing a late block. In this case forkchoice is
already locked and we add a call in `getFCUArgs` to return early if not
regularly syncing
3) During handling of late blocks on `lateBlockTasks` we simply return
   early if not in regular sync (This is the biggest change as it takes
a longer FC lock for lateBlockTasks)
4) On Attestation processing, in UpdateHead, we are already locked so we
   just add a check to not update head on this path if not regularly
syncing.
@potuz potuz requested a review from a team as a code owner January 17, 2024 14:52
@potuz potuz added this pull request to the merge queue Jan 17, 2024
Merged via the queue into develop with commit 79bb7ef Jan 17, 2024
17 checks passed
@potuz potuz deleted the check_syncing branch January 17, 2024 16:00
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.

2 participants