-
Notifications
You must be signed in to change notification settings - Fork 733
Remove era arguments to cardano-testnet
#6314
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
base: master
Are you sure you want to change the base?
Conversation
671b114
to
9725a00
Compare
let genesis = Api.alonzoGenesisDefaults (toCardanoEra sbe) | ||
defaultAlonzoGenesis :: Either AlonzoGenesisError AlonzoGenesis | ||
defaultAlonzoGenesis = do | ||
let genesis = Api.alonzoGenesisDefaults ConwayEra |
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.
This is fine for now but we should parameterize everything on Era era
instead. Era era
tracks only the latest eras.
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.
Saved in #6317 to keep it in mind
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.
This is fine for now
@Jimbo4350 I'm not sure that it is. In order to reintroduce parametrisation on Era era
again, we'd have to revert this PR basically. Is it possible to do this now and keep the parameterisation?
20c8407
to
46cbb65
Compare
Needs an update of golden files: https://github.com/IntersectMBO/cardano-node/pull/6314/checks?check_run_id=48999840783 |
@@ -197,13 +192,13 @@ createSPOGenesisAndFiles | |||
H.note_ $ "Number of stake delegators: " <> show numStakeDelegators | |||
H.note_ $ "Number of seeded UTxO keys: " <> show numSeededUTxOKeys | |||
|
|||
let era = toCardanoEra sbe | |||
let era = toCardanoEra ConwayEra |
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 think this place may be hard to find, where the era is hardcoded. Also It will be useful to still be able to override this value, since we're starting work on the next era now.
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 see two problems here:
- Conway era is hardcoded in multiple places (as a protocol version too) (also in comments), it should be hardcoded in at most one place if we want to move forward with this change.
- We're losing parametrisation on era. This is especially useful when working two eras at the same time: when one era is on mainnet and the other one is in the works. To restore the parametrisation we'd have to basically revert most of this PR. Can we keep the parametrisation, but just use a default era and not expose it to the user for example?
My hard requirement is nr 1, but let's discuss nr 2.
Starting a testnet in an era different than Conway has not been possible for a while, since the options to do so have been removed from `cardano-cli`. This commit therefore removes those options from `cardano-testnet` as well. This commit also removes leftover golden files.
@carbolymer your comments have been addressed. Basically I only changed the parser, and added a single hardcoded value in |
I don't understand where that's coming from: I did regenerate the golden files, and that one exists: https://github.com/IntersectMBO/cardano-node/pull/6314/files#diff-bcd54a52a6f758e10284e243ba0830249a2d15898ce60d606ceead57c74941e3 |
@nbacquey Ah right, it's green in GHA. We have hardcoded paths in nix code, so you have to update those as well: |
Starting a testnet in an era different than Conway has not been possible for a while, since the options to do so have been removed from
cardano-cli
. This PR therefore removes those options fromcardano-testnet
as well, along wih their respective code paths.This PR also removes leftover golden files.
Checklist
CHANGELOG.md
for affected packagehlint
. See.github/workflows/check-hlint.yml
to get thehlint
versionstylish-haskell
. See.github/workflows/stylish-haskell.yml
to get thestylish-haskell
versionghc-9.6
andghc-9.12