feat: eip-6110 deprecate eth1 data poll #7414
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #7414 +/- ##
============================================
+ Coverage 48.50% 50.25% +1.74%
============================================
Files 602 602
Lines 40355 40375 +20
Branches 2069 2205 +136
============================================
+ Hits 19576 20292 +716
+ Misses 20741 20043 -698
- Partials 38 40 +2 |
Performance Report✔️ no performance regression detected Full benchmark results
|
nflaig
reviewed
Jan 31, 2025
ensi321
changed the title
nflaig
reviewed
Feb 3, 2025
| const finalizedCheckpoint = this.chain.forkChoice.getFinalizedCheckpoint(); | ||
| const fork = this.config.getForkInfoAtEpoch(finalizedCheckpoint.epoch).name; | ||
|
|
||
| if (isForkPostElectra(fork)) { |
There was a problem hiding this comment.
can't we just check the fork based on current slot? it's good to only stop polling if finalized state is electra but I think we shouldn't even call getStateByCheckpoint if it's not even electra fork
There was a problem hiding this comment.
can't we just check the fork based on current slot?
Just checking slot based on clock is not enough. There is a short transition period in Electra where we still need to poll and process eth1 deposits. We can only stop polling when all the eth1 deposits in the queue have been processed
it's good to only stop polling if finalized state is electra but I think we shouldn't even call getStateByCheckpoint if it's not even electra fork
getFinalizedCheckpoint() is a simple call as fcStore.justified.checkpoint is always cached. We can add an additional electra fork check based on clock slot in L253 but this would only avoid calling getFinalizedCheckpoint() in deneb. But we ultimately need to check if finalized state is in Electra, and then further check if finalized state's eth1DepositIndex has reached a certain height in order to stop polling.
There was a problem hiding this comment.
There is a short transition period in Electra where we still need to poll and process eth1 deposits
I know but at the earliest we would run the deposit index check if it's post-electra based on clock slot, and then we can load the state and check the index
Co-authored-by: Nico Flaig <[email protected]>
it's not about EIP inclusion, it's about stop eth1 data polling. I mean if Pectra is released soon with reasonable |
Ah I see what you mean. Yes, if the latest finalized checkpoint has no more new eth1 deposits processed, then the deposit contract snapshot is "frozen". At that point there is no need to maintain the repositories in #7388. I even think at some point we can just hard code a |
wemeetagain
approved these changes
Feb 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Eth1 data poll is not needed anymore in Pectra after all eth1 deposits are processed.
Introduce mechanism to stop the polling.
Note in the case of Lodestar starting up in Electra, Eth1 data poll may run for 1 epoch but no data will actually get polled and then
PrepareNextSlotScheduler.stopEth1Polling()will stop it.Part of #6341