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

Shelley: choose the right initial nonce #2005

Closed
mrBliss opened this issue Apr 24, 2020 · 8 comments · Fixed by #2289
Closed

Shelley: choose the right initial nonce #2005

mrBliss opened this issue Apr 24, 2020 · 8 comments · Fixed by #2289
Assignees
Labels
consensus issues related to ouroboros-consensus

Comments

@mrBliss
Copy link
Contributor

mrBliss commented Apr 24, 2020

@mrBliss mrBliss added consensus issues related to ouroboros-consensus shelley ledger integration labels Apr 24, 2020
@mrBliss mrBliss added this to the S12 2020-05-07 milestone Apr 24, 2020
@nc6
Copy link
Contributor

nc6 commented Apr 27, 2020

I think the easiest thing to do for this is to seed the nonce with some randomness we can insert in ShelleyGenesis. Then people who know more about crypto can decide how much we care about that randomness.

@mrBliss mrBliss modified the milestones: S14 2020-06-04, S15 2020-06-18 Jun 9, 2020
@mrBliss mrBliss self-assigned this Jun 15, 2020
@mrBliss
Copy link
Contributor Author

mrBliss commented Jun 16, 2020

@JaredCorduan With all these discussions about nonces, I don't know what the final decisions were 😕. Let me go over the places in consensus where we currently pick a nonce so we can check whether we make the right choice.

A fresh Shelley chain

When starting a Shelley chain from scratch, like in the F&F testnet, we call initialShelleyState in consensus with SL.NeutralNonce as the initNonce argument here: https://github.com/input-output-hk/ouroboros-network/blob/c1f65da121805cd3903915ab0ccad576554d3c68/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Node.hs#L175-L186

Looking at initialShelleyState, this means we use that SL.NeutralNonce argument for the following nonces: "Current epoch nonce", "Evolving nonce", and "Candidate nonce". The fourth and final nonce, "Prev epoch hash nonce", is hard-coded in initialShelleyState to be NeutralNonce.

Question 1: Is this correct? The alternative is to create a nonce from the hash of the Shelley genesis config file and pass that to initialShelleyState.

The Byron-to-Shelley hard fork

When transitioning from Byron to Shelley, we have to translate the Byron protocol state to the Shelley protocol state (PrtclState). This happens here: https://github.com/input-output-hk/ouroboros-network/blob/c1f65da121805cd3903915ab0ccad576554d3c68/ouroboros-consensus-cardano/src/Ouroboros/Consensus/Cardano/CanHardFork.hs#L446-L460
At the moment, we use the NeutralNonce for all four nonces.

Question 2: Is this correct? The alternative is again to create a nonce from the hash of the Shelley genesis config file and use that for all four nonces, or just the first three (so not the "Prev epoch hash nonce"). Note that we don't have access to the last Byron hash as we don't track it in the PBftState. Access to the hash of the Shelley genesis config file is possible, as it is something static that we can store in our config data types.

@JaredCorduan
Copy link
Contributor

Thank you for such a clear and organized question @mrBliss !

We can do the same thing for both a fresh Shelley chain and a chain from Byron. We should set "Current epoch nonce", "Evolving nonce", and "Candidate nonce" to the same value X, and the "Prev epoch hash nonce" to the NeutralNonce. I think we should set X to the hash of the Shelley genesis config file (I am confirming this with the cryptographers now, and will let you know).

For a bit of explanation:

At the start of an epoch, the "Current epoch nonce", "Evolving nonce", and "Candidate nonce" will always be the same value (equal to each other, not the same value between epochs :) ). The "Prev epoch hash nonce" does not matter at the start of the epoch, we just store this value so that when a new epoch begins we can use the hash of the last block in the previous epoch to create the next Current epoch nonce. In other words, the "Prev epoch hash nonce" will just be immediately overridden on the next block. So NeutralNonce is great.

@mrBliss
Copy link
Contributor Author

mrBliss commented Jun 16, 2020

Thank you for such a clear and organized question @mrBliss !

We can do the same thing for both a fresh Shelley chain and a chain from Byron. We should set "Current epoch nonce", "Evolving nonce", and "Candidate nonce" to the same value X, and the "Prev epoch hash nonce" to the NeutralNonce. I think we should set X to the hash of the Shelley genesis config file (I am confirming this with the cryptographers now, and will let you know).

Thank you for the clear answer! I'll start implementing this now (I can backtrack if the cryptographers disagree).

For a bit of explanation:

At the start of an epoch, the "Current epoch nonce", "Evolving nonce", and "Candidate nonce" will always be the same value (equal to each other, not the same value between epochs :) ). The "Prev epoch hash nonce" does not matter at the start of the epoch, we just store this value so that when a new epoch begins we can use the hash of the last block in the previous epoch to create the next Current epoch nonce. In other words, the "Prev epoch hash nonce" will just be immediately overridden on the next block. So NeutralNonce is great.

I see. The last part I remembered. Crystal clear now 👍

@nfrisby
Copy link
Contributor

nfrisby commented Jun 17, 2020

I'm not sure if this is already supported by the above discussion, but the tests should be able to twiddle the initial Shelley nonce independently for the sake of statistical testing and similar. So it'd be good to parameterize the initial nonce selection at least out to the interface that testing should use.

@mrBliss
Copy link
Contributor Author

mrBliss commented Jun 18, 2020

I'm still awaiting a decision on how to compute the genesis hash in IntersectMBO/cardano-ledger#1558 (comment).

However, I can already parameterise the code (ConsensusConfig for TPraos) over the initial nonce (prep for #2235). After a decision about the computation of the genesis hash is made, it will then just be a matter of passing the right nonce to the ConsensusConfig.

@mrBliss mrBliss linked a pull request Jun 18, 2020 that will close this issue
mrBliss added a commit that referenced this issue Jun 26, 2020
Fixes #2005.

This nonce will be used to construct Shelley's initial `PrtclState`, both when
starting a fresh Shelley chain and when forking from Byron to Shelley (when
translating the Byron `ConsensusState` to the Shelley one).

We store the initial nonce in the `TPraosParams`, which is part of the
`ConsensusConfig` for `TPraos`. We need it here, at run-time, because we need
it when translating the Byron `ConsensusState` to the Shelley one.

`protocolInfoShelley` and `protocolInfoCardano` (as well as
`ProtocolReadTPraos` and `ProtocolCardano`) now take a `Nonce` argument that
will be used as the initial nonce. Typically the `Nonce` passed to these
functions (constructors) should be derived from the hash of the Shelley
Genesis config JSON file. Moreover, this allows us to choose a different
initial nonce for testing purposes, as required for #2235.

NOTE: up until now we have used `SL.NeutralNonce` as the initial nonce for
Shelley. When a different nonce is picked in `cardano-node`, i.e., one derived
from the hash of the Shelley Genesis config JSON file, it would cause a hard
fork.
@mrBliss
Copy link
Contributor Author

mrBliss commented Jun 26, 2020

Quoting from IntersectMBO/cardano-ledger#1531 (comment):

I had a discussion with @nc6 and we decided that it's best to just use the Nonce type directly for this, both in cardano-ledger-specs (to avoid confusion as Genesis(Config)Hash would not be related to the existing GenesisHash and genesis in any way) and in ouroboros-consensus (we just need a Nonce for entropy, we also don't care where it comes from). Only in cardano-node, where they have to provide that Nonce to consensus (via ProtocolInfo, is the choice made to use the hash of the Shelley genesis JSON file.

mrBliss added a commit that referenced this issue Jun 26, 2020
Fixes #2005.

This nonce will be used to construct Shelley's initial `PrtclState`, both when
starting a fresh Shelley chain and when forking from Byron to Shelley (when
translating the Byron `ChainDepState` to the Shelley one).

We store the initial nonce in the `TPraosParams`, which is part of the
`ConsensusConfig` for `TPraos`. We need it here, at run-time, because we need
it when translating the Byron `ChainDepState` to the Shelley one.

`protocolInfoShelley` and `protocolInfoCardano` (as well as
`ProtocolRealTPraos` and `ProtocolCardano`) now take a `Nonce` argument that
will be used as the initial nonce. Typically the `Nonce` passed to these
functions (constructors) should be derived from the hash of the Shelley
Genesis config JSON file. Moreover, this allows us to choose a different
initial nonce for testing purposes, as required for #2235.

NOTE: up until now we have used `SL.NeutralNonce` as the initial nonce for
Shelley. When a different nonce is picked in `cardano-node`, i.e., one derived
from the hash of the Shelley Genesis config JSON file, it would cause a hard
fork.
iohk-bors bot added a commit that referenced this issue Jun 26, 2020
2289: Shelley/Cardano: take the initial nonce as a parameter r=mrBliss a=mrBliss

Fixes #2005.

This nonce will be used to construct Shelley's initial `PrtclState`, both when
starting a fresh Shelley chain and when forking from Byron to Shelley (when
translating the Byron `ChainDepState` to the Shelley one).

We store the initial nonce in the `TPraosParams`, which is part of the
`ConsensusConfig` for `TPraos`. We need it here, at run-time, because we need
it when translating the Byron `ChainDepState` to the Shelley one.

`protocolInfoShelley` and `protocolInfoCardano` (as well as
`ProtocolRealTPraos` and `ProtocolCardano`) now take a `Nonce` argument that
will be used as the initial nonce. Typically the `Nonce` passed to these
functions (constructors) should be derived from the hash of the Shelley
Genesis config JSON file. Moreover, this allows us to choose a different
initial nonce for testing purposes, as required for #2235.

NOTE: up until now we have used `SL.NeutralNonce` as the initial nonce for
Shelley. When a different nonce is picked in `cardano-node`, i.e., one derived
from the hash of the Shelley Genesis config JSON file, it would cause a hard
fork.

Co-authored-by: Thomas Winant <thomas@well-typed.com>
@iohk-bors iohk-bors bot closed this as completed in 4bec6b0 Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants