-
Notifications
You must be signed in to change notification settings - Fork 959
Serialize bpo schedule in asending order #7753
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
|
Some required checks have failed. Could you please take a look @pawanjay176? 🙏 |
| // Check that serialization is in ascending order | ||
| let yaml = serde_yaml::to_string(&spec.blob_schedule).expect("should serialize"); | ||
|
|
||
| // Deserialize back to Vec<BlobParameters> to check order |
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.
It's a little weird that a serde round trip result in different ordering. Maybe we should reverse again in deserialize?
It currently don't cause any issue, but if it gets serialized -> deseralized -> serailzied again we'll end up having descending order - might be confusing if this functionality gets used by downstream projects
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.
Ah nevermind, this is done in the constructor
lighthouse/consensus/types/src/chain_spec.rs
Lines 1486 to 1490 in 03d297d
| pub fn new(mut vec: Vec<BlobParameters>) -> Self { | |
| // reverse sort by epoch | |
| vec.sort_by(|a, b| b.epoch.cmp(&a.epoch)); | |
| Self(vec) | |
| } |
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 good, thanks!
N/A Serializes the blob_schedule in ascending order to match other clients. This is needed to keep the output of `eth/v1/config/spec` http endpoint consistent across clients. cc @barnabasbusa
Issue Addressed
N/A
Proposed Changes
Serializes the blob_schedule in ascending order to match other clients. This is needed to keep the output of
eth/v1/config/spechttp endpoint consistent across clients.cc @barnabasbusa