-
Notifications
You must be signed in to change notification settings - Fork 906
[Merged by Bors] - Remove CountUnrealized
#4357
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
[Merged by Bors] - Remove CountUnrealized
#4357
Conversation
@michaelsproul do you want to go ahead with a review without the duplicate check TODO? |
I kinda added that todo as I was putting it up, maybe let's wait until I get to it before reviewing. Soz |
Ready for review now assuming CI passes |
Should we also remove the deprecated |
Usually we wait a few releases with the flags retained as no-ops, so people have some time to remove the flags from their setups. On that topic there are a lot of older deprecated flags lying around that we could safely delete now. |
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 great! and tests are passing 🚀
Co-authored-by: Jimmy Chen <jchen.tc@gmail.com>
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.
Nice one, LGTM!
bors r+ |
## Issue Addressed Closes #4332 ## Proposed Changes Remove the `CountUnrealized` type, defaulting unrealized justification to _on_. This fixes the #4332 issue by ensuring that importing the same block to fork choice always results in the same outcome. Finalized sync speed may be slightly impacted by this change, but that is deemed an acceptable trade-off until the optimisation from #4118 is implemented. TODO: - [x] Also check that the block isn't a duplicate before importing
Pull request successfully merged into unstable. Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page.
|
CountUnrealized
CountUnrealized
## Issue Addressed Closes sigp#4332 ## Proposed Changes Remove the `CountUnrealized` type, defaulting unrealized justification to _on_. This fixes the sigp#4332 issue by ensuring that importing the same block to fork choice always results in the same outcome. Finalized sync speed may be slightly impacted by this change, but that is deemed an acceptable trade-off until the optimisation from sigp#4118 is implemented. TODO: - [x] Also check that the block isn't a duplicate before importing
Closes sigp#4332 Remove the `CountUnrealized` type, defaulting unrealized justification to _on_. This fixes the sigp#4332 issue by ensuring that importing the same block to fork choice always results in the same outcome. Finalized sync speed may be slightly impacted by this change, but that is deemed an acceptable trade-off until the optimisation from sigp#4118 is implemented. TODO: - [x] Also check that the block isn't a duplicate before importing
Closes sigp#4332 Remove the `CountUnrealized` type, defaulting unrealized justification to _on_. This fixes the sigp#4332 issue by ensuring that importing the same block to fork choice always results in the same outcome. Finalized sync speed may be slightly impacted by this change, but that is deemed an acceptable trade-off until the optimisation from sigp#4118 is implemented. TODO: - [x] Also check that the block isn't a duplicate before importing
Issue Addressed
Closes #4332
Proposed Changes
Remove the
CountUnrealized
type, defaulting unrealized justification to on. This fixes the #4332 issue by ensuring that importing the same block to fork choice always results in the same outcome.Finalized sync speed may be slightly impacted by this change, but that is deemed an acceptable trade-off until the optimisation from #4118 is implemented.
TODO: