-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add support for parsing Shelley genesis blocks #331
Conversation
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, just a few minor comments
pub decentralisation_param: Fraction, | ||
pub e_max: u64, | ||
pub extra_entropy: ShelleyGenesisExtraEntropy, | ||
pub key_deposit: u64, |
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.
maybe some of these could be Coin
when relevant? It's still just an alias but it would be consistent with the rest of the protocol param structs.
} | ||
|
||
pub fn parse_genesis_data<R: Read>(json: R) -> Result<config::ShelleyGenesisData, GenesisJSONError> { | ||
let data_value: serde_json::Value = serde_json::from_reader(json)?; |
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 the clone? I don't see it used anywhere else.
ipv6 | ||
)); | ||
}, | ||
_ => panic!("Only single host address relays are supported in cardano-node Relay JSON parsing") |
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.
If we're going to panic here it'd be good to say so in the function comment.
Edit: I read the comment on RelayTypeMap
saying you couldn't even find JSON for other types so maybe this is okay.
#[derive(Serialize, Deserialize, Debug)] | ||
pub struct ShelleyGenesisCredential { | ||
// for some reason there actually is a space in the JSON key emitted by the Haskell node | ||
#[serde(rename = "key hash")] |
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 looks like cardano-node changed it so that both key hash
and keyHash
are accepted, so we have to handle both those possibilities here as well. Not sure if Serde has an option to enable 2 different ways to represent a field in JSON
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.
@SebastienGllmt You can use #[serde(alias = "keyHash")]
It can even be defined multiple times.
example:
#[derive(serde::Serialize, serde::Deserialize, Debug)]
pub struct Foo {
#[serde(alias = "KeyHash")]
#[serde(alias = "keyHash")]
key_hash: String,
}
fn main() {
let jsons = [
"{ \"key_hash\": \"default\" }",
"{ \"keyHash\": \"lowerCamelCase\" }",
"{ \"KeyHash\": \"UpperCamelCase\" }",
"{ \"keyhash\": \"should not parse\" }",
];
for json in jsons {
let foo: Result<Foo, _> = serde_json::from_str(json);
println!("{json} -> {foo:?}");
}
}
The first 3 are all accepted. First by it being the field name, and the next 2 for the alias =
marker.
Running:
{ "key_hash": "default" } -> Ok(Foo { key_hash: "default" })
{ "keyHash": "lowerCamelCase" } -> Ok(Foo { key_hash: "lowerCamelCase" })
{ "KeyHash": "UpperCamelCase" } -> Ok(Foo { key_hash: "UpperCamelCase" })
{ "keyhash": "should not parse" } -> Err(Error("missing field `key_hash`", line: 1, column: 33))
* Add alias for `ShelleyGenesisCredential::keyHash` to account for cardano-node json format * Remove unnecessary clone() * Change `u64` to `Coin` when appropriate in Genesis types * ran cargo fmt
* Add support for parsing Shelley genesis blocks * Genesis keyHash alias + minor fixes * Add alias for `ShelleyGenesisCredential::keyHash` to account for cardano-node json format * Remove unnecessary clone() * Change `u64` to `Coin` when appropriate in Genesis types * ran cargo fmt * Update Genesis parsing for changes in #340 --------- Co-authored-by: rooooooooob <rooooooooob@users.noreply.github.com> Co-authored-by: rooooooooob <raftias@gmail.com>
This is required as part of dcSpark/carp#187
Notably, the Shelley genesis block has an
intialFunds
field and astaking
field which essentially allows having stake pools and shelley-era keys as part of the genesis block for newly spun up networks which is useful for testing