-
Notifications
You must be signed in to change notification settings - Fork 721
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
Generate protocolUpdateUtxoCostPerByte in genProtocolParametersUpdate era dependently #5070
Conversation
6794662
to
6bc5c3c
Compare
4c7cc37
to
1bba825
Compare
15feec0
to
d24a256
Compare
-- | ||
-- The Babbage and subsequent eras support such a protocol parameter. | ||
-- | ||
data ProtocolUTxOCostPerByteSupportedInEra era where |
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 actually explicitly avoided using GADTs for ProtocolParameter
fields because it would result in an explosion of type definitions.
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 added GADTs because that was the way we handle era support for things that aren't protocol parameters.
The cost will be (2 x numEras + 5) x numRelevantProtocolPameters
. I don't yet have a good idea how many protocol parameters need this. At least each new protocol parameter will need this.
The ability to determine what protocol parameters are available for what era will be important to generators but I also suspect users of our API will find this useful too.
As for what alternative solutions are available, it will need to be one that allows us to express both the addition and removal of protocol parameters because new eras can and have removed protocol parameters.
If not expressed as types it could be expressed as a runtime value.
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.
For comparison:
- Number of features:
24
- Number of protocol parameters:
25
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've updated PR with an more concise encoding. This means we write less code when adding a new parameter or feature.
We could also convert the existing feature supported in era code to use this new system instead which would reduce the amount of code.
1841f7d
to
bf07a9f
Compare
{-# LANGUAGE GADTs #-} | ||
{-# LANGUAGE StandaloneDeriving #-} | ||
|
||
module Cardano.Api.Features |
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.
This module introduces a new unified type-safe era-based feature flag system.
|
||
data ProtocolParameterUTxOCostPerWord | ||
|
||
data ProtocolParameterUTxOCostPerByte |
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.
Each new feature is represented as an un-exported type.
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.
Maybe DataKinds
would be better here to have a single kind for them?
-- | A representation of a feature that is supported in a given era. | ||
data Feature f where | ||
ProtocolParameterUTxOCostPerWord :: Feature ProtocolParameterUTxOCostPerWord | ||
ProtocolParameterUTxOCostPerByte :: Feature ProtocolParameterUTxOCostPerByte |
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.
The Feature
GADT has a constructor for each feature. This type is exported rather than the individual feature types.
deriving instance Show (Feature f) | ||
|
||
instance ToJSON (Feature f) where | ||
toJSON = toJSON . show |
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.
Unifying the features into one type means we only need type class instances for the Feature
type and not for each feature type.
ProtocolParameterUTxOCostPerWordSupportedInAlonzoEra :: SupportedInEra ProtocolParameterUTxOCostPerWord AlonzoEra | ||
|
||
ProtocolParameterUTxOCostPerByteSupportedInBabbageEra :: SupportedInEra ProtocolParameterUTxOCostPerByte BabbageEra | ||
ProtocolParameterUTxOCostPerByteSupportedInConwayEra :: SupportedInEra ProtocolParameterUTxOCostPerByte ConwayEra |
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.
The SupportedInEra
type allows us to only add code for when a feature is supported in an era. We don't have to add the negative cases.
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's a just a cartesian product of eras and features in a form of a constructor for each, so it will be cumbersome to maintain I think. How about just a type class?
data ByronEra
data ShelleyEra
data AllegraEra
data MaryEra
data AlonzoEra
data BabbageEra
data ConwayEra
-- This can be turned into a single datatype using DataKinds
data ProtocolParameterUTxOCostPerWord
data ProtocolParameterUTxOCostPerByte
data UnknownFeature
data CardanoEra era where
ByronEra :: CardanoEra ByronEra
ShelleyEra :: CardanoEra ShelleyEra
AllegraEra :: CardanoEra AllegraEra
MaryEra :: CardanoEra MaryEra
AlonzoEra :: CardanoEra AlonzoEra
BabbageEra :: CardanoEra BabbageEra
ConwayEra :: CardanoEra ConwayEra
data Feature f where
ProtocolParameterUTxOCostPerWord :: Feature ProtocolParameterUTxOCostPerWord
ProtocolParameterUTxOCostPerByte :: Feature ProtocolParameterUTxOCostPerByte
UnknownFeature :: Feature UnknownFeature
class SupportedInEra f e where
supportedInEra :: Feature f -> CardanoEra e -> Bool
instance {-# OVERLAPPABLE #-} SupportedInEra f e where
supportedInEra _ _ = False
instance ProtocolParameterUTxOCostPerWord `SupportedInEra` ByronEra where
supportedInEra _ _ = True
instance SupportedInEra ProtocolParameterUTxOCostPerByte ByronEra where
supportedInEra _ _ = True
-- >>> supportedInEra ProtocolParameterUTxOCostPerWord ByronEra
-- True
-- >>> supportedInEra ProtocolParameterUTxOCostPerByte ByronEra
-- True
-- >>> supportedInEra UnknownFeature ByronEra
-- False
-- This can be simplified further with a phantom type, not requiring to create a new constructor for each
-- feature, which is already a type
data Feature' f = Feature'
class SupportedInEra' f e where
supportedInEra' :: Feature' f -> CardanoEra e -> Bool
instance {-# OVERLAPPABLE #-} SupportedInEra' f e where
supportedInEra' _ _ = False
instance SupportedInEra' ProtocolParameterUTxOCostPerWord ByronEra where
supportedInEra' _ _ = True
instance SupportedInEra' ProtocolParameterUTxOCostPerByte ByronEra where
supportedInEra' _ _ = True
-- >>> supportedInEra' (Feature' @ProtocolParameterUTxOCostPerByte) ByronEra
-- True
-- >>> supportedInEra' (Feature' @ProtocolParameterUTxOCostPerByte) ByronEra
-- True
-- >>> supportedInEra' (Feature' @UnknownFeature) ByronEra
-- False
we can simplify this even further with {-# LANGUAGE AllowAmbiguousTypes #-}
:
class SupportedInEra'' f e where
supportedInEra'' :: Bool
instance {-# OVERLAPPABLE #-} SupportedInEra'' a b where
supportedInEra'' = False
instance SupportedInEra'' ProtocolParameterUTxOCostPerByte ByronEra where
supportedInEra'' = True
-- >>> supportedInEra'' @UnknownFeature @ByronEra
-- False
-- >>> supportedInEra'' @ProtocolParameterUTxOCostPerByte @ByronEra
-- True
Do we need features to be types?
supportedInEra ProtocolParameterUTxOCostPerWord AlonzoEra = Just ProtocolParameterUTxOCostPerWordSupportedInAlonzoEra | ||
|
||
supportedInEra ProtocolParameterUTxOCostPerByte BabbageEra = Just ProtocolParameterUTxOCostPerByteSupportedInBabbageEra | ||
supportedInEra ProtocolParameterUTxOCostPerByte ConwayEra = Just ProtocolParameterUTxOCostPerByteSupportedInConwayEra |
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.
The supportedInEra
type allows us to only add code for when a feature is supported in an era. We don't have to add the negative cases.
protocolParamMaxValueSize <- Gen.maybe genNat | ||
protocolParamCollateralPercent <- Gen.maybe genNat | ||
protocolParamMaxCollateralInputs <- Gen.maybe genNat | ||
protocolParamUTxOCostPerByte <- sequence $ supportedInEra ProtocolParameterUTxOCostPerByte era $> genLovelace |
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.
An example of how supportedInEra
is used.
e4ea02a
to
b8d9232
Compare
b8d9232
to
726ea6e
Compare
a1392d6
to
dd8817d
Compare
44704ba
to
cc0c164
Compare
cc0c164
to
75827e1
Compare
41ea6c3
to
6d86667
Compare
2fc2c1e
to
75827e1
Compare
Pull request was closed
Depends on #5069