Skip to content
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 Byron genesis parsing #58

Merged
merged 5 commits into from
May 14, 2022
Merged

Add Byron genesis parsing #58

merged 5 commits into from
May 14, 2022

Conversation

SebastienGllmt
Copy link
Contributor

This copies the genesis parsing code from cardano-rust and puts in it CML so that we can parse genesis blocks in projects like oura-postgres-sink. We had to do this because db-sync didn't expose all the information about the genesis block that we needed

Note: I didn't add WASM bindings to any of this since I'm not sure if this is something we'll need from WASM. Things can be exported later if needed

@SebastienGllmt SebastienGllmt self-assigned this May 1, 2022
@SebastienGllmt SebastienGllmt added the enhancement New feature or request label May 1, 2022
@@ -109,7 +109,8 @@ fn main() {
gen_json_schema!(AuxiliaryDataHash);
gen_json_schema!(PoolMetadataHash);
gen_json_schema!(VRFKeyHash);
gen_json_schema!(BlockHash);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI, this was actually a type mistake in the old bindings. Block body hash and block header hash are different things (but coincidentally both happen to be 32 bytes so it didn't cause any issue)

rust/src/address.rs Outdated Show resolved Hide resolved
@@ -135,9 +137,96 @@ macro_rules! define_hash_object {
bech32::to_bech32_from_bytes::<Self>(self.as_ref())
}
}
impl cbor_event::de::Deserialize for $hash_ty {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea about how to add serializable types. Please review @rooooooooob

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually use the trait in utils::Deserialize trait to contain additional error information and such, but that trait is automatically implemented by anything that implements cbor_event::de::Deserialize so this is fine. It will wrap it in DeserializeFailure::CBOR and will contain None for location information (i.e. won't tell you in what part of the CBOR it was found) For simple types like a hash this isn't an issue since there's no member name to tag it with anyway, so you're only losing the struct name being tagged but honestly I don't think is a big deal.

For serialization we directly use the cbor_event trait so this is exactly what you need to do.

Keep in mind that if you want any of these types to directly have to_bytes()/from_bytes() that are was-exposable we need to use those auto-deriving macros in utils but this is perfectly fine for the traits. If they're only called from other rust code just the traits is totally fine and will work with the existing (de)serialization system.

rust/src/chain_crypto/key.rs Show resolved Hide resolved
rust/src/chain_crypto/sign.rs Show resolved Hide resolved
rust/src/genesis/byron/config.rs Outdated Show resolved Hide resolved
rust/src/genesis/byron/config.rs Outdated Show resolved Hide resolved
rust/src/genesis/byron/config.rs Outdated Show resolved Hide resolved
pub fee_policy: fees::LinearFee,
pub avvm_distr: BTreeMap<chain_crypto::PublicKey<Ed25519>, utils::Coin>, // AVVM = Ada Voucher Vending Machine
// TODO: convert to ByronAddress without affecting order
pub non_avvm_balances: BTreeMap<legacy_address::Addr, utils::Coin>,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this gives a different order than what db-sync gives. Not sure which one is correct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the order matter here? Does how it's stored here impact how the hash is computed (order of some map in CBOR?) It could be not important if some order is specified in that part of code and the hashes match up, even if the order here is not what db-sync gives. I don't know enough about the genesis block to really say what should happen though.

Copy link
Contributor

@rooooooooob rooooooooob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't fully review the stuff that I assume was pulled from the old chain tools code (/chain_crypto/,/legacy_address/ and /genesis/*) since it's already a part of old tooling + it's several thousand lines, but I looked at the comments you made and I went through the code that is a part of just CML (e.g. the root-level files in rust/src that were changed) and it looks okay. I'm assuming the brought-in code is fine since you said it works.

@@ -135,9 +137,96 @@ macro_rules! define_hash_object {
bech32::to_bech32_from_bytes::<Self>(self.as_ref())
}
}
impl cbor_event::de::Deserialize for $hash_ty {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually use the trait in utils::Deserialize trait to contain additional error information and such, but that trait is automatically implemented by anything that implements cbor_event::de::Deserialize so this is fine. It will wrap it in DeserializeFailure::CBOR and will contain None for location information (i.e. won't tell you in what part of the CBOR it was found) For simple types like a hash this isn't an issue since there's no member name to tag it with anyway, so you're only losing the struct name being tagged but honestly I don't think is a big deal.

For serialization we directly use the cbor_event trait so this is exactly what you need to do.

Keep in mind that if you want any of these types to directly have to_bytes()/from_bytes() that are was-exposable we need to use those auto-deriving macros in utils but this is perfectly fine for the traits. If they're only called from other rust code just the traits is totally fine and will work with the existing (de)serialization system.

pub fee_policy: fees::LinearFee,
pub avvm_distr: BTreeMap<chain_crypto::PublicKey<Ed25519>, utils::Coin>, // AVVM = Ada Voucher Vending Machine
// TODO: convert to ByronAddress without affecting order
pub non_avvm_balances: BTreeMap<legacy_address::Addr, utils::Coin>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the order matter here? Does how it's stored here impact how the hash is computed (order of some map in CBOR?) It could be not important if some order is specified in that part of code and the hashes match up, even if the order here is not what db-sync gives. I don't know enough about the genesis block to really say what should happen though.

@SebastienGllmt SebastienGllmt merged commit a89b492 into develop May 14, 2022
@SebastienGllmt SebastienGllmt deleted the byron-genesis-parsing branch May 14, 2022 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants