-
Notifications
You must be signed in to change notification settings - Fork 86
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
4386: Introduces property checking the size of Header's VRF Certs for different Eras r=abailly-iohk a=abailly-iohk # Description This PR does 2 things: * An attempt at minimising the "attack surface" of consensus' dependencies on cardano-ledger crypto stuff related to Praos/TPraos, * Introduction of Property to check dispatching of crypto depending on era actually produces the right results. ## Hide Ledger's `PraosCrypto` The idea is to remove direct dependencies from consensus package to cardano-ledger's `TPraos.API` module. There is a duplication of the `PraosCrypto` typeclass between the two components which does seem unnecessary as ultimately, these crypto primitives should end up in the consensus where they are used and relevant. I would have loved to be able to remove the ledger's `PraosCrypto` altogether but of course this is not possible as we use quite a few functions from the ledger that depend on it. So the tiny first step consists in hiding the dependency on `Ledger.PraosCrypto` behind the `Ouroboros.Consensus.Praos.Crypto.PraosCrypto` typeclass (reexported by `Praos` and `TPraos` modules). This simplifies some constraints down the road. The hypothesis this work is based on is that if we manage to locate all dependencies from consensus to ledger for KES and VRF stuff, then introducing a new `Crypto` typeclass and primitives will have a minimal impact over the codebase and could be done independently of the refactoring in the cardano-ledger. ## Add Property to check "dispatching" of VRF per Era The goal of #4150 is to ensure different eras can have different crypto implementations for things such as VRF proofs generation and verification, and KES signing. While #4151 provided solutions to this problem, this PR tries to make the problem manifest through a property that, given an arbitrary header from different eras, verifies the correct crypto is used. This is done simply by verifying the sizes of the VRF certificates which are different between plain Praos and Batch compatible VRF. Co-authored-by: Arnaud Bailly <arnaud.bailly@iohk.io> Co-authored-by: Arnaud Bailly <79840582+abailly-iohk@users.noreply.github.com> Co-authored-by: Joris Dral <joris@well-typed.com>
- Loading branch information
Showing
18 changed files
with
264 additions
and
90 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
98 changes: 98 additions & 0 deletions
98
ouroboros-consensus-cardano-test/test/Test/Consensus/Cardano/Crypto.hs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| {-# LANGUAGE FlexibleContexts #-} | ||
| {-# LANGUAGE FlexibleInstances #-} | ||
| {-# LANGUAGE GADTs #-} | ||
| {-# LANGUAGE LambdaCase #-} | ||
| {-# LANGUAGE MultiParamTypeClasses #-} | ||
| {-# LANGUAGE NamedFieldPuns #-} | ||
| {-# LANGUAGE OverloadedStrings #-} | ||
| {-# LANGUAGE PatternSynonyms #-} | ||
| {-# LANGUAGE RankNTypes #-} | ||
| {-# LANGUAGE ScopedTypeVariables #-} | ||
| {-# LANGUAGE TypeFamilies #-} | ||
| {-# LANGUAGE UndecidableInstances #-} | ||
|
|
||
| {-# OPTIONS_GHC -Wno-incomplete-uni-patterns #-} | ||
|
|
||
| -- | Tests consensus-specific crypto operations in relationship with blocks/headers. | ||
| module Test.Consensus.Cardano.Crypto (tests) where | ||
|
|
||
| import Cardano.Crypto.VRF (sizeCertVRF) | ||
| import Cardano.Crypto.VRF.Praos (certSizeVRF) | ||
| import Data.Function ((&)) | ||
| import Ouroboros.Consensus.Cardano.Block (CardanoHeader, | ||
| StandardCrypto, pattern HeaderAllegra, | ||
| pattern HeaderAlonzo, pattern HeaderBabbage, | ||
| pattern HeaderByron, pattern HeaderConway, | ||
| pattern HeaderMary, pattern HeaderShelley) | ||
| import Ouroboros.Consensus.Shelley.Ledger.Block (Header (..)) | ||
| import Ouroboros.Consensus.Shelley.Protocol.Abstract | ||
| (pTieBreakVRFValue) | ||
| import Ouroboros.Consensus.Shelley.Protocol.Praos () | ||
| import Test.Consensus.Cardano.Generators () | ||
| import Test.QuickCheck (Property, label, property, (===)) | ||
| import Test.Tasty (TestTree, testGroup) | ||
| import Test.Tasty.QuickCheck (testProperty) | ||
|
|
||
| tests :: TestTree | ||
| tests = | ||
| testGroup "Cardano Crypto" [ | ||
| testProperty "era-dependent VRF" prop_VRFCryptoDependsOnBlockEra | ||
| ] | ||
|
|
||
| -- | Check that Babbage and Conway blocks use different VRF crypto. | ||
| -- | ||
| -- This test is based on the following steps: | ||
| -- | ||
| -- 1. generate (forge?) babbage or conway headers | ||
| -- - those should contain different VRF proofs (because they are supposed to | ||
| -- use different algorithms) | ||
| -- - the VRF proof should be invalid for the other era | ||
| -- 2. call some header validation functions that's calling VRF certificate check | ||
| -- 3. assert that different VRF function is called for each era | ||
| -- - the header should be valid | ||
| -- | ||
| -- * why not mock everything? because it does not check that we are | ||
| -- implementing things correctly for Conway we would like to test | ||
| -- the dispatchign induced by this type, to make clear Conway relies | ||
| -- on different crypto primitives | ||
| -- | ||
| -- What needs to change is this type: | ||
| -- | ||
| -- @@ | ||
| -- type CardanoShelleyEras c = | ||
| -- '[ ShelleyBlock (TPraos c) (ShelleyEra c) | ||
| -- , ShelleyBlock (TPraos c) (AllegraEra c) | ||
| -- , ShelleyBlock (TPraos c) (MaryEra c) | ||
| -- , ShelleyBlock (TPraos c) (AlonzoEra c) | ||
| -- , ShelleyBlock (Praos c) (BabbageEra c) | ||
| -- , ShelleyBlock (Praos c) (ConwayEra c) | ||
| -- ] | ||
| -- @@ | ||
| -- | ||
| -- is it enough to test a single specialised crypto function? yes, | ||
| -- because that's a start, but also because using the high level | ||
| -- HFBlock even with a single function would be evidence we are doing | ||
| -- the dispatching right => we don't test the actual crypto functions, | ||
| -- only the "dispatching" logic that requires different instances for | ||
| -- different eras. | ||
| -- | ||
| prop_VRFCryptoDependsOnBlockEra :: CardanoHeader StandardCrypto -> Property | ||
| prop_VRFCryptoDependsOnBlockEra = \case | ||
| HeaderShelley ShelleyHeader {shelleyHeaderRaw} -> | ||
| certVRFHasPraosSize shelleyHeaderRaw & label "Shelley" | ||
| HeaderAllegra ShelleyHeader {shelleyHeaderRaw} -> | ||
| certVRFHasPraosSize shelleyHeaderRaw & label "Allegra" | ||
| HeaderMary ShelleyHeader {shelleyHeaderRaw} -> | ||
| certVRFHasPraosSize shelleyHeaderRaw & label "Mary" | ||
| HeaderAlonzo ShelleyHeader {shelleyHeaderRaw} -> | ||
| certVRFHasPraosSize shelleyHeaderRaw & label "Alonzo" | ||
| HeaderBabbage ShelleyHeader {shelleyHeaderRaw} -> | ||
| certVRFHasPraosSize shelleyHeaderRaw & label "Babbage" | ||
| HeaderConway ShelleyHeader {shelleyHeaderRaw} -> | ||
| -- TODO: this is were we need to change to check we use in the Conway case | ||
| -- Cardano.Crypto.VRF.PraosBatchCompat.certSizevrf | ||
| certVRFHasPraosSize shelleyHeaderRaw & label "Conway" | ||
| HeaderByron _ -> property True & label "Byron" | ||
|
|
||
| where | ||
| certVRFHasPraosSize hdrRaw = sizeCertVRF (pTieBreakVRFValue hdrRaw) === fromIntegral certSizeVRF |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.