-
Notifications
You must be signed in to change notification settings - Fork 60
chore(l1,l2): ordered genesis files #2713
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
Lines of code reportTotal lines added: Detailed view
|
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.
not sure I agree with this approach, now we're mixing fork specific configs like "muirGlacierBlock" with other configs like "depositContractAddress". What is the standard across Ethereum? I have never seen it be alphabetically ordered keys
31fe9fe
to
14cc4dc
Compare
crates/common/Cargo.toml
Outdated
@@ -13,7 +13,9 @@ tracing.workspace = true | |||
tinyvec = "1.6.0" | |||
ethereum-types.workspace = true | |||
serde.workspace = true | |||
serde_json.workspace = true | |||
# Preserve order is used to serialize the Genesis struct |
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.
why do we have to change a dependency in common
for tooling? I thought we wanted to decouple them
crates/common/types/genesis.rs
Outdated
} | ||
} | ||
|
||
fn sort_config(genesis_map: &mut Map<String, Value>) -> Result<Map<String, Value>, String> { |
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.
can't we move this code to the tooling part
**Motivation** Some changes (probably #2713) undid the fixed changes for the load test genesis file, this PR restores it.
Motivation
Ordered genesis files make it easy to diff with one another.
Description
Closes #2706.