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

Optimize issuing advance engine fcU for prepare payload #4054

Closed
4 tasks
g11tech opened this issue May 24, 2022 · 4 comments · Fixed by #4209
Closed
4 tasks

Optimize issuing advance engine fcU for prepare payload #4054

g11tech opened this issue May 24, 2022 · 4 comments · Fixed by #4209
Assignees
Labels
prio-medium Resolve this some time soon (tm).

Comments

@g11tech
Copy link
Contributor

g11tech commented May 24, 2022

Currently the engine fcU is issued just after a successful block import (#3965), predicting the new head to be the head in the next slot if BN has any beacon proposer. Normally this would work just fine, but this isn't sufficient and the following scenarios needs to be addressed:

  • wait till the last interval of slot before issuing fcU unless the head is already last imported
  • issue the fcU
    • for skipped slot
    • for head change
@twoeths
Copy link
Contributor

twoeths commented Jun 20, 2022

Right now PrecomputeNextEpochTransitionScheduler does not work after the merge due to that. I suppose we should make this work as a separate scheduler like in lighthouse and do precompute next epoch transition in this scheduler too https://github.com/sigp/lighthouse/blob/564d7da656803f5e06e53a303972580be54500bf/beacon_node/beacon_chain/src/proposer_prep_service.rs#L18

@g11tech
Copy link
Contributor Author

g11tech commented Jun 21, 2022

Right now PrecomputeNextEpochTransitionScheduler does not work after the merge due to that. I suppose we should make this work as a separate scheduler like in lighthouse and do precompute next epoch transition in this scheduler too https://github.com/sigp/lighthouse/blob/564d7da656803f5e06e53a303972580be54500bf/beacon_node/beacon_chain/src/proposer_prep_service.rs#L18

my idea is not NOT run PrecomputeNextEpochTransitionScheduler but run PrecomputeNextSlotTransitionScheduler and do an fcU there if not epoch transition else first do epoch transition and then chain fcU to it. what do you think @tuyennhv ?

or we do slot processing on a copy ?

@g11tech
Copy link
Contributor Author

g11tech commented Jun 21, 2022

if you want to take this up @tuyennhv feel free!

@twoeths
Copy link
Contributor

twoeths commented Jun 22, 2022

my idea is not NOT run PrecomputeNextEpochTransitionScheduler but run PrecomputeNextSlotTransitionScheduler and do an fcU there if not epoch transition else first do epoch transition and then chain fcU to it. what do you think @tuyennhv ?

yeah we're on the same page, we only need 1 scheduler, yeah I can take this @g11tech

@twoeths twoeths self-assigned this Jun 22, 2022
@dapplion dapplion added the prio-medium Resolve this some time soon (tm). label Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio-medium Resolve this some time soon (tm).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants