-
Notifications
You must be signed in to change notification settings - Fork 157
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
Reorganize Cardano.Ledger.Shelley
and ShelleyMA
era modules
#2021
Conversation
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.
Looks great, thanks Damian!
I'm a little uncertain about pulling e.g. ValidateScript
to the top, but this is my own fault for making these things era-based, rather than using the specific Script type. So this is perhaps something to address in the future, but I think that this PR takes the best approach to these things as they are defined now.
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.
LGTM
import Shelley.Spec.Ledger.MetaData (MetaData (MetaData), MetaDataHash (MetaDataHash), ValidateMetadata (hashMetadata, validateMetadata), validMetaDatum) | ||
import Shelley.Spec.Ledger.Scripts (MultiSig) | ||
import Shelley.Spec.Ledger.Tx (TxBody, ValidateScript (hashScript, validateScript), hashMultiSigScript, validateNativeMultiSigScript) |
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.
There are a few lines that I would wrap.
|
||
instance CryptoClass.Crypto c => ValidateMetadata (ShelleyEra c) where | ||
hashMetadata = MetaDataHash . hashWithSerialiser toCBOR | ||
validateMetadata (MetaData m) = all validMetaDatum m |
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.
Probably unrelated to this PR, but I saw a deepseq
here in some other implementation. Is that intentional?
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.
That was in the Shelley-MA package, related to the new optional scripts. The idea there is that we want to verify that scripts included in the metadata are valid. Scripts are valid by construction, so this check comes down to making sure they're fully deserialised.
module Cardano.Ledger.Shelley.Constraints where | ||
|
||
import Cardano.Ledger.Compactible (Compactible (..)) | ||
import Cardano.Ledger.Core (AnnotatedData, ChainData, Metadata, Script, SerialisableData, TxBody, 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.
Rather long 🙂
…lley`. - Define `Core` instances in the `ShelleMA` era module.
69568ba
to
a260d53
Compare
bors r+ |
Visible changes: * IntersectMBO/cardano-ledger#1993 * IntersectMBO/cardano-ledger#2021
Visible changes: * IntersectMBO/cardano-ledger#1993 * IntersectMBO/cardano-ledger#2021 One of the changes in Allegra over Shelley is the notion of transaction auxiliary data. We retrospectively define tx metadata to be part of the auxiliary data where in the Shelley era this consists only of the existing tx metadata. But from the Allegra era the auxiliary data contains both the tx metadata and also auxiliary scripts. So where we previously hashed the tx metadata, we now hash the auxiliary data. Take the opportunity to refactor the tx metadata representation and conversion functions to fit better with the new scheme. Also take the opportunity to tidy up the imports in the Query module since we have some very similarly-named type classes. Co-Authored by: Duncan Coutts <[email protected]>
Visible changes: * IntersectMBO/cardano-ledger#1993 * IntersectMBO/cardano-ledger#2021 One of the changes in Allegra over Shelley is the notion of transaction auxiliary data. We retrospectively define tx metadata to be part of the auxiliary data where in the Shelley era this consists only of the existing tx metadata. But from the Allegra era the auxiliary data contains both the tx metadata and also auxiliary scripts. So where we previously hashed the tx metadata, we now hash the auxiliary data. Take the opportunity to refactor the tx metadata representation and conversion functions to fit better with the new scheme. Also take the opportunity to tidy up the imports in the Query module since we have some very similarly-named type classes. Co-authored-by: Duncan Coutts <[email protected]>
2149: Update dependencies r=dcoutts a=mrBliss Visible changes: * IntersectMBO/cardano-ledger#1993 * IntersectMBO/cardano-ledger#2021 Co-authored-by: Thomas Winant <[email protected]>
Visible changes: * IntersectMBO/cardano-ledger#1993 * IntersectMBO/cardano-ledger#2021 One of the changes in Allegra over Shelley is the notion of transaction auxiliary data. We retrospectively define tx metadata to be part of the auxiliary data where in the Shelley era this consists only of the existing tx metadata. But from the Allegra era the auxiliary data contains both the tx metadata and also auxiliary scripts. So where we previously hashed the tx metadata, we now hash the auxiliary data. Take the opportunity to refactor the tx metadata representation and conversion functions to fit better with the new scheme. Also take the opportunity to tidy up the imports in the Query module since we have some very similarly-named type classes. Co-authored-by: Duncan Coutts <[email protected]>
Visible changes: * IntersectMBO/cardano-ledger#1993 * IntersectMBO/cardano-ledger#2021 One of the changes in Allegra over Shelley is the notion of transaction auxiliary data. We retrospectively define tx metadata to be part of the auxiliary data where in the Shelley era this consists only of the existing tx metadata. But from the Allegra era the auxiliary data contains both the tx metadata and also auxiliary scripts. So where we previously hashed the tx metadata, we now hash the auxiliary data. Take the opportunity to refactor the tx metadata representation and conversion functions to fit better with the new scheme. Also take the opportunity to tidy up the imports in the Query module since we have some very similarly-named type classes. Co-authored-by: Duncan Coutts <[email protected]>
2149: Update dependencies r=dcoutts a=mrBliss Visible changes: * IntersectMBO/cardano-ledger#1993 * IntersectMBO/cardano-ledger#2021 Co-authored-by: Thomas Winant <[email protected]> Co-authored-by: Duncan Coutts <[email protected]> Co-authored-by: Luke Nadur <[email protected]>
Visible changes: * IntersectMBO/cardano-ledger#1993 * IntersectMBO/cardano-ledger#2021 One of the changes in Allegra over Shelley is the notion of transaction auxiliary data. We retrospectively define tx metadata to be part of the auxiliary data where in the Shelley era this consists only of the existing tx metadata. But from the Allegra era the auxiliary data contains both the tx metadata and also auxiliary scripts. So where we previously hashed the tx metadata, we now hash the auxiliary data. Take the opportunity to refactor the tx metadata representation and conversion functions to fit better with the new scheme. Also take the opportunity to tidy up the imports in the Query module since we have some very similarly-named type classes. Co-authored-by: Duncan Coutts <[email protected]>
Visible changes: * IntersectMBO/cardano-ledger#1993 * IntersectMBO/cardano-ledger#2021 One of the changes in Allegra over Shelley is the notion of transaction auxiliary data. We retrospectively define tx metadata to be part of the auxiliary data where in the Shelley era this consists only of the existing tx metadata. But from the Allegra era the auxiliary data contains both the tx metadata and also auxiliary scripts. So where we previously hashed the tx metadata, we now hash the auxiliary data. Take the opportunity to refactor the tx metadata representation and conversion functions to fit better with the new scheme. Also take the opportunity to tidy up the imports in the Query module since we have some very similarly-named type classes. Co-authored-by: Duncan Coutts <[email protected]>
2149: Update dependencies r=dcoutts a=mrBliss Visible changes: * IntersectMBO/cardano-ledger#1993 * IntersectMBO/cardano-ledger#2021 Co-authored-by: Thomas Winant <[email protected]> Co-authored-by: Duncan Coutts <[email protected]> Co-authored-by: Luke Nadur <[email protected]>
Move
ShelleyEra
and associated definitions toCardano.Ledger.Shelley
.Define
Core
instances in theShelleMA
era module.