WIP Split Crypto into standard Crypto and HeaderCrypto #3262
Conversation
This is still the work in progress and is rough at edges - Split Crypto class into Crypto (DSIGN, ADDRHASH) and HeaderCrypto (KES, VRF) - Move HeaderCrypto into Cardano.Protocol - Use PoolStakeVRF and GenesisVRF as a proxy to VRF in ledger code - Parametrize TPrao protcol code with Crypto and HeaderCrypto Many functions are parametrized by Era and HeaderCrypto as Era implicitly implies Crypto (Crypto Era). TODO: Get examples and tests compiled
| @@ -156,22 +163,22 @@ defaultShelleyLedgerExamples mkWitnesses mkAlonzoTx value txBody auxData transla | |||
| -------------------------------------------------------------------------------} | |||
|
|
|||
| exampleShelleyLedgerBlock :: | |||
| forall era. | |||
| (EraSegWits era, ShelleyBasedEra' era) => | |||
| forall era hcrypto. | |||
There was a problem hiding this comment.
We've switched everywhere in the repo from crypto type parameter to c, at least that is the way it is now on master, so I think it would be best to name it hc
| forall era hcrypto. | |
| forall era hc. |
| MaxKESEvolutionsUnsupported | ||
| sgMaxKESEvolutions | ||
| (totalPeriodsKES (Proxy @(KES (Crypto era)))) | ||
| -- TODO: This is the only place where KES is required and that too for a |
There was a problem hiding this comment.
The whole validateGenesis function can be moved into cardano-protocol-praos, since it is not used anywhere in ledger.
| -- where | ||
| -- poolKeys = exampleKeys @c @hc @'StakePool |
There was a problem hiding this comment.
| -- where | |
| -- poolKeys = exampleKeys @c @hc @'StakePool |
| @@ -271,7 +284,7 @@ tickChainState | |||
| Origin -> NeutralNonce | |||
| At (LastAppliedBlock {labHash}) -> hashHeaderToNonce labHash | |||
| } | |||
| lv = either (error . show) id $ futureLedgerView testGlobals chainNes slotNo | |||
| -- lv = either (error . show) id $ futureLedgerView testGlobals chainNes slotNo | |||
There was a problem hiding this comment.
| -- lv = either (error . show) id $ futureLedgerView testGlobals chainNes slotNo |
| @@ -245,7 +253,7 @@ instance CC.Crypto c => GetLedgerView (ConwayEra c) where | |||
| $ TRC ((), ss, slot) | |||
|
|
|||
| -- | Data required by the Transitional Praos protocol from the Shelley ledger. | |||
| data LedgerView crypto = LedgerView | |||
| data LedgerView crypto hcrypto = LedgerView | |||
There was a problem hiding this comment.
Why do you need hcrypto in the LedgerView? It does not seem to being used.
There was a problem hiding this comment.
Not required. It is something that needs cleanup.
- ALl the ledger code is separated from header crypto (KES/VRF) - Proxy VRFs are PoolStakeVRF and GenesisVRF
Format as per the style
- Use Data.Proxy instead of a locally defined data type
- Use HeaderCrypto to separately parametrise HeaderBody - The ledger Era is only bound to standard Crypto - The consensus protocol is parametrised by Crypto and HeaderCrypto Note that - Only ouroboros-consensus-protocol is compiled - ouroboros-consenus-cardano libarry is WIP, and only Block.hs is changed to reflect the fact that it is possible to use a separate HeaderCrypto for an era (and define the the cardano eras) parametrized by different crypto version. Each version reflecting a unique combination of standard Crypto and Header Crypto. This repo is compiled against ledger branch (see IntersectMBO/cardano-ledger#3262). You might have to change cabal.project to compile against git repository
|
@lehins I am taking this over this PR from @yogeshsajanikar, as this is part of IntersectMBO/ouroboros-network#4151. Would you have some time to help me understand better what's the situation? I think I understand the overall goal of this work (eg. support evolving crypto primitives over eras) but it's not clear to me what's been done, what remains to be done, what's the deadline, whether there are intermediate steps we could do, etc. |
@abailly-iohk I'll be happy to chat about it, however I am not too sure about the progress on this work or what the actual time line is. Considering that this PR is using 1.1 release branch as base this work is definitely outdated. |
|
The changes are done to the release branch as the main purpose was to 1 Split the crypto into two parts, header and body Hence I changed release branch against which consensus code was compiled. Initially I targeted the ledger master branch, but the porting changes required in consensus were non-trivial. The current status of the change is
Having said that I agree that work is needed to make it work against master. But most of the changes are mechanical. |
|
@yogeshsajanikar Thanks for your input, we have a chat today with @lehins to sort out this branch issue and move this PR forward. |
|
Had a shot at rebasing this issue on master and it's not trivial (for me), as discussed with @lehins I will wait for him to merge related changes to ledger's master codebase before trying again. |
|
Can I help? I have a some free time next week. |
|
@yogeshsajanikar Sure, would you be available on Monday morning CET for a call? |
|
Yes. Please feel free to schedule a meeting @abailly-iohk |
|
Hope you're available, just setup a meeting today at 10:30 CET |
|
@lehins Do you have a branch we could attempt to rebase on? |
|
@abailly-iohk The one I have is quite unstable right this moment, however I expect this to change either today or tomorrow. So, very soon I'll have a reviewable PR, which could be used as a base branch if need be. |
|
@lehins Any news on this front? |
|
@abailly-iohk Sorry, forgot to let you know. I did have a reviewable PR last week. As of today it has been merged, so IntersectMBO/ouroboros-network#4349 is fully compatible with ledger master. I'll be doing ledger releases to CHaPs today and merging the above PR |
|
No problem @lehins , so this means we can rebase this PR on master? |
Correct |
|
@yogeshsajanikar Are you still available to do the rebase? Otherwise, I can take care of it, no worries. |
|
Yes, I can rebase it. Hope it's okay if I tackle it tomorrow?
On Wednesday, 22 February, 2023 at 08:46:36 pm IST, Arnaud Bailly ***@***.***> wrote:
@yogeshsajanikar Are you still available to do the rebase? Otherwise, I can take care of it, no worries.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
@yogeshsajanikar Tomorrow is totally fine, thanks for your help |
|
Started working on this. |
|
Continuing work today afternoon. |
|
@yogeshsajanikar I guess you did not have time to work on this? That's totally fine, I propose to take it over if you don't mind as I need it to implement the relevant stuff in consensus. |
ghost
mentioned this pull request
|
This PR appears stale. I am going to close it for now. |
Description
VRF and KES have new optimized implementations that need to be used for Conway era. This is the first step to use separate KES/VRF from Crypto such that it can be parametrized separately. Also since consensus is the primary consumer of the VRF/KES and ledger uses them for the bookkeeping, this PR moves HeaderCrypto into TPraos protocol.
This branch is based on a #lehins/no-vrf-usage-in-ledger (ported on the ledger release) to get conensus code compiled.
Checklist
fourmolu(which can be run withscripts/fourmolize.sh