-
Notifications
You must be signed in to change notification settings - Fork 720
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
cardano-testnet: fix that genesis files generated by create-testnet-data were ignored #5732
Conversation
70b39b9
to
980b943
Compare
H.rewriteJsonFile @Value (tempAbsPath </> defaultGenesisFilepath ShelleyEra) $ \o -> o | ||
& L.key "maxLovelaceSupply" . L._Integer .~ 10_000_000_000_000_000 |
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.
I tried to get rid of this rewrite, but it makes the Treasury Growth
fail, so something is still wrong somewhere. I will look into it in another PR. I don't want to block this one.
genesisByronDir <- H.createDirectoryIfMissing $ tempAbsPath </> "byron" | ||
|
||
files <- H.listDirectory tempAbsPath | ||
forM_ files $ \file -> do | ||
H.note file | ||
forM_ files H.note | ||
|
||
H.renameFile (tempAbsPath </> "byron-gen-command" </> "genesis.json") (genesisByronDir </> "genesis.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.
I tried to make the byron genesis handled like the others (i.e. to end up at byron-genesis.json
), but it makes the node refuse to start, because of a hash mismatch. I will solve this later on, because I didn't want to block this PR.
70e541e
to
9c8cade
Compare
…ata were ignored Simplify structure of directory generated in cardano-testnet: put genesis files at the top-level instead of within shelley/
9c8cade
to
6df5731
Compare
6df5731
to
9bea1a3
Compare
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.
Nice work 💪
inputGenesisAlonzoFp = tempAbsPath </> genesisInputFilepath AlonzoEra | ||
inputGenesisConwayFp = tempAbsPath </> genesisInputFilepath ConwayEra | ||
|
||
-- We write the genesis files to disk, to pass them to create-testnet-data. |
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.
These files shouldn't be modified after they are created. create-testnet-data
should create the final genesis files we want to use and then we create a configuration file based on those genesis files.
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.
Yep agreed, we should remove the last rewrites that remain later on 👍
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 you update the comment so it's obvious this isn't the behaviour we want?
defaultGenesisFilepath era = "shelley" </> ("genesis." <> eraToString era <> ".json") | ||
defaultGenesisFilepath era = | ||
-- This path is actually generated by create-testnet-data. Don't change it. | ||
eraToString era <> "-genesis.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.
👍
defaultDRepVkeyFp | ||
:: Int -- ^ The DRep's index (starts at 0) | ||
-> FilePath | ||
defaultDRepVkeyFp n = "drep-keys" </> ("drep" <> show n) </> "drep.vkey" |
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.
👍
=> (MonadTest m, MonadIO m, HasCallStack) | ||
=> TmpAbsolutePath | ||
-> AnyCardanoEra -- ^ The era used for generating the hard fork configuration toggle | ||
-> m LBS.ByteString | ||
createConfigYaml (TmpAbsolutePath tempAbsPath) era = GHC.withFrozenCallStack $ do | ||
createConfigJson (TmpAbsolutePath tempAbsPath) era = GHC.withFrozenCallStack $ do |
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.
Let's use pretty JSON here. Makes debugging a little easier.
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.
Good point, I'm myself always reformatting those JSON files before inspecting them 😆
-- 50 second epochs | ||
-- Epoch length should be "10 * k / f" where "k = securityParam, f = activeSlotsCoeff" | ||
H.rewriteJsonFile @Value inputGenesisShelleyFp $ \o -> o | ||
-- TODO: remove rho and tau adjustment after https://github.com/IntersectMBO/cardano-api/pull/425 gets | ||
-- integrated with newer cardano-api into node | ||
& L.key "protocolParams" . L.key "rho" . L._Number .~ 0.1 |
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.
💪
@@ -93,49 +93,6 @@ hprop_drep_retirement = H.integrationRetryWorkspace 2 "drep-retirement" $ \tempA | |||
drepSKeyFp :: Int -> FilePath | |||
drepSKeyFp n = tempAbsPath' </> "drep-keys" </> ("drep" <> show n) </> "drep.skey" | |||
|
|||
-- Create Drep registration certificates |
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.
Do we still test this work flow?
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.
No we don't, but I'll readd such a test shortly afterwards.
Follow-up tracked here: #5742
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! Only one concern: #5732 (comment)
9bea1a3
to
45d1b8c
Compare
Description
create-testnet-data
were not passed to the nodes spawned in clusterscreate-testnet-data
new features of creating DReps; which is used in this PR to remove custom inializationcardano-testnet
. It changes from:. ├── alonzo-genesis.json ├── byron │ └── genesis.json ├── byron.genesis.spec.json ├── configuration.yaml ├── conway-genesis.json ├── genesis.alonzo.spec.json ├── genesis.conway.spec.json └── shelley ├── genesis.alonzo.json ├── genesis.conway.json └── genesis.shelley.json
to
. ├── alonzo-genesis.json ├── byron │ └── genesis.json ├── byron.genesis.spec.json ├── configuration.yaml ├── conway-genesis.json ├── genesis.alonzo.spec.json ├── genesis.conway.spec.json └── shelley-genesis.json
Checklist
Note on CI
If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.