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

JSON roundtripping of ConwayGenesis fails on a particular file #4782

Open
smelc opened this issue Dec 4, 2024 · 4 comments
Open

JSON roundtripping of ConwayGenesis fails on a particular file #4782

smelc opened this issue Dec 4, 2024 · 4 comments

Comments

@smelc
Copy link

smelc commented Dec 4, 2024

Dear ledger team, thank you very much for your hard work!

While working on create-testnet-data (the command to generate testnet configurations in cardano-cli), I observed in a test (in this PR) that decoding a conway-genesis.json file was causing some data to be lost: the delegators fields in the initialDReps list.

Here is how to reproduce it using cabal repl:

> cabal repl cardano-ledger-conway:lib:cardano-ledger-conway
> import Data.Aeson
> import Data.Maybe
> x :: ConwayGenesis StandardCrypto <- fromJust <$> decodeFileStrict "/home/churlin/dev/cardano-ledger-2/conway-genesis.json"
> encodeFile "/home/churlin/dev/cardano-ledger-2/conway-genesis-roundtrip.json" x

When comparing the input file conway-genesis.json (to the left) with the file obtained after decoding and reencoding (conway-genesis-roundtrip.json), the value within the delegators fields have been lost:

image

Let me know if I can be of any help to solve this! Note that the input file is not generated manually, but programmatically, here: https://github.com/IntersectMBO/cardano-cli/blob/5e1e4f2ef30da7ffe02642a89870f990bd2d2216/cardano-cli/src/Cardano/CLI/EraBased/Run/Genesis/CreateTestnetData.hs#L369

@lehins
Copy link
Collaborator

lehins commented Dec 5, 2024

@smelc That is the expected behavior. Because it is when the ConwayGenesis applied the delegations in the "delegs" field that will populate DRep delegations.

I do understand however how this can be confusing and undesirable for ConwayGenesis.json file to not roundtrip, so I think the best way to solve this will be to prevent "delegators" field from showing up in the genesis file altogether.

@smelc
Copy link
Author

smelc commented Dec 6, 2024

Because it is when the ConwayGenesis applied the delegations in the "delegs" field that will populate DRep delegations.

@lehins> Can you elaborate on what is the master data here? I'm asking because in CLI we are actually populating delegs correctly but were leaving delegators empty (i.e. we generate the file on the right in my screenshot). I assumed that we were doing it wrong when I saw the JSON, which is why I changed the generation to produce the file on the left. Can you confirm that we should indeed populate both delegs AND delegators?

I do understand however how this can be confusing and undesirable for ConwayGenesis.json file to not roundtrip, so I think the best way to solve this will be to prevent "delegators" field from showing up in the genesis file altogether.

Sounds reasonable to me 👍

@lehins
Copy link
Collaborator

lehins commented Dec 6, 2024

Can you elaborate on what is the master data here?

delegs is the master data and delegators will be populated from it internally.

I'm asking because in CLI we are actually populating delegs correctly but were leaving delegators empty (i.e. we generate the file on the right in my screenshot). I assumed that we were doing it wrong when I saw the JSON

So, you were actually doing it correctly before.

Can you confirm that we should indeed populate both delegs AND delegators?

No, you do not need to populate both. All you need is the delegs field. delegators filed in the DRepState is an internal field needed for the ledger rules and that is why it is ignored in the FromJSON instance.

@smelc
Copy link
Author

smelc commented Dec 9, 2024

Thanks @lehins for the additional answers, very much appreciated 🙏 I believe you can close the issue 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants