-
Notifications
You must be signed in to change notification settings - Fork 931
Final changes for fusaka-devnet-2
#7655
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
Conversation
jimmygchen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good actually @ethDreamer !
I've made a PR #7722 for the cleanup and test fixes.
* Clean up and add more code comments on BPO forks. * Fix tests. * Fix tests. * Fix moar tests. * Fix moar tests. * Ensure the same `ChainSpec` is used throughout the tests. * Refactor `ForkContext` to avoid recomputing fork digests and using cached values. * Fix failing tests due to mismatching chainspec and block slots. * Review comments - adjust code style and add docs. * make sure all forks are covered in tests --------- Co-authored-by: Pawan Dhananjay <[email protected]>
|
The sync failures are only on fulu. I'll be handling them in #7726 Gonna merge this one when the remaining checks pass |
pawanjay176
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
This pull request has been removed from the queue for the following reason: Pull request #7655 has been dequeued. The pull request could not be merged. This could be related to an activated branch protection or ruleset rule that prevents us from merging. (details: 1 review requesting changes and 1 approving review by reviewers with write access.). You can check the last failing draft PR here: #7730. You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. |
Closes sigp#7467. This PR primarily addresses [the P2P changes](ethereum/EIPs#9840) in [fusaka-devnet-2](https://fusaka-devnet-2.ethpandaops.io/). Specifically: * [the new `nfd` parameter added to the `ENR`](ethereum/EIPs#9840) * [the modified `compute_fork_digest()` changes for every BPO fork](ethereum/EIPs#9840) 90% of this PR was absolutely hacked together as fast as possible during the Berlinterop as fast as I could while running between Glamsterdam debates. Luckily, it seems to work. But I was unable to be as careful in avoiding bugs as I usually am. I've cleaned up the things *I remember* wanting to come back and have a closer look at. But still working on this. Progress: * [x] get it working on `fusaka-devnet-2` * [ ] [*optional* disconnect from peers with incorrect `nfd` at the fork boundary](ethereum/consensus-specs#4407) - Can be addressed in a future PR if necessary * [x] first pass clean-up * [x] fix up all the broken tests * [x] final self-review * [x] more thorough review from people more familiar with affected code
Issue Addressed
Closes #7467.
This PR primarily addresses the P2P changes in fusaka-devnet-2. Specifically:
nfdparameter added to theENRcompute_fork_digest()changes for every BPO fork90% of this PR was absolutely hacked together as fast as possible during the Berlinterop as fast as I could while running between Glamsterdam debates. Luckily, it seems to work. But I was unable to be as careful in avoiding bugs as I usually am. I've cleaned up the things I remember wanting to come back and have a closer look at. But still working on this.
Progress:
fusaka-devnet-2nfdat the fork boundary - Can be addressed in a future PR if necessary