-
Notifications
You must be signed in to change notification settings - Fork 724
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
Yaml version of mainnet config #3269
Conversation
6d060c4
to
b9287a9
Compare
b9287a9
to
ec6ad20
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.
Can you include in the commit message why there are code changes in the second commit and not just comments in a config file.
ec6ad20
to
791f49e
Compare
Done! |
791f49e
to
30516b7
Compare
There is a windows failure in CI still. |
9464e40
to
eff9e3c
Compare
@@ -167,7 +167,8 @@ test-suite cardano-cli-test | |||
main-is: cardano-cli-test.hs | |||
type: exitcode-stdio-1.0 | |||
|
|||
build-depends: bech32 >= 1.1.0 | |||
build-depends: aeson | |||
, bech32 >= 1.1.0 |
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.
Random question, is the bech32 version bounds necessary?
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 don't have the background for why there is a lower bound here, but it is unchanged from before.
projectBase <- H.note =<< H.evalIO . IO.canonicalizePath =<< H.getProjectBase | ||
result <- H.evalIO $ runExceptT $ initialLedgerState $ projectBase </> "configuration/cardano/mainnet-config.json" | ||
case result of | ||
Right (_, _) -> return () | ||
Left e -> H.failWithCustom GHC.callStack Nothing (T.unpack (renderInitialLedgerStateError e)) | ||
|
||
hprop_configMainnetYaml :: Property | ||
hprop_configMainnetYaml = H.propertyOnce $ 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.
So we're testing the equivalence of yaml and aeson decoding?
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.
We have a YAML config file that contains helpful comments and a JSON config file which is generated by nix. They are meant to have the same config, so the test ensures they never diverge.
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!
##### Update system parameters ##### | ||
|
||
# This protocol version number gets used by block producing nodes as part | ||
# of the system for agreeing on and synchronising protocol updates. | ||
LastKnownBlockVersion-Major: 0 | ||
LastKnownBlockVersion-Minor: 2 | ||
LastKnownBlockVersion-Major: 3 |
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 would be useful to link to: https://github.com/input-output-hk/cardano-node/blob/master/cardano-node/src/Cardano/Node/Protocol/Cardano.hs#L199 for the maximum protocol version. I'm not sure where the max block version exists though.
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.
Done!
…on and add tests to ensure consistency between the two.
eff9e3c
to
ae0d335
Compare
bors merge |
Build succeeded: |
Introduces a YAML version of the Mainnet configuration file that is better documented than the JSON version. This includes a test to ensure the YAML and JSON versions agree.
To see how the configuration file used by mainnet currently compares to how it was back in the Byron era, see this diff: b9287a9