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

Plumbing for loading Shelley node leader credentials #832

Merged
merged 4 commits into from
Apr 23, 2020

Conversation

dcoutts
Copy link
Contributor

@dcoutts dcoutts commented Apr 22, 2020

The final patch is the one we're after, it does the final bit where we load up all the various files to make the Praos slot leader credentials.

First two patches are more refactoring. Then CLI plumbing. Finally some tweaks to the OpCert read/write code and then putting it all together by using it in the node startup code.

@dcoutts dcoutts changed the title Reduce duplication in NodeCLI / NodeProtocolMode code Plumbing for loading Shelley node leader credentials Apr 23, 2020
@dcoutts dcoutts marked this pull request as ready for review April 23, 2020 00:36
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Once this builds I can have another look but so far 👍



------------------------------------------------------------------------------
-- Errors
--

data ShelleyProtocolInstantiationError =
GenesisReadError !FilePath !String
GenesisReadError !FilePath !String
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

, configFp :: !ConfigYamlFilePath
, validateDB :: !Bool
, shutdownIPC :: !(Maybe Fd)
{ nodeMode :: !NodeProtocolMode
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, way better than my initial attempt.

@dcoutts dcoutts force-pushed the dcoutts/shelley-plumbing-leader-creds branch from 3be9a9e to c817df2 Compare April 23, 2020 08:30
The NodeProtocolMode was splitting the configuration between the mock
and real cases right at the very top level, eliminating any possibility
of sharing between the cases. But in fact almost everything can be
shared as there are currently few differences between the two cases.

This adjusts the types so that most things are shared. This will make
adding more things (for Shelley) rather easier.
KES, VRK and op cert files.

Just CLI flags, not yet used.
For the OpCert we actually need the pair of the op cert and the
corresponding verification key.

Also, we need the op cert to use valid CBOR for its external rep, so we
have to be a little careful with the to/fromCBORGroup stuff. Clarify
this with explicit encoders and decoders.

In all three modules, use local type aliases to simplify the type
signatures.

Also derive Show for the Error types, since we need to embed these
inside bigger error types that themselves derive Show.
Use the VRF, KES and OpCert utils to load them all during the node
startup, if they are specified on the command line.

This is needed for the node to be able to make blocks.
@dcoutts dcoutts force-pushed the dcoutts/shelley-plumbing-leader-creds branch from c817df2 to bcb1589 Compare April 23, 2020 13:43
@Jimbo4350 Jimbo4350 self-requested a review April 23, 2020 13:56
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

LGTM!

@dcoutts
Copy link
Contributor Author

dcoutts commented Apr 23, 2020

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 23, 2020

@iohk-bors iohk-bors bot merged commit 09d800e into master Apr 23, 2020
@iohk-bors iohk-bors bot deleted the dcoutts/shelley-plumbing-leader-creds branch April 23, 2020 14:41
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

Successfully merging this pull request may close these issues.

None yet

3 participants