Test Conway CDDL using Cuddle #4437
Merged
Conversation
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
teodanciu
reviewed
Jul 2, 2024
eras/conway/impl/test/Test/Cardano/Ledger/Conway/Binary/CddlSpec.hs
Outdated
Show resolved
Hide resolved
teodanciu
reviewed
Jul 2, 2024
teodanciu
reviewed
Jul 2, 2024
teodanciu
reviewed
Jul 2, 2024
teodanciu
reviewed
Jul 2, 2024
teodanciu
reviewed
Jul 2, 2024
teodanciu
reviewed
Jul 2, 2024
teodanciu
reviewed
Jul 2, 2024
lehins
requested changes
Jul 2, 2024
There was a problem hiding this comment.
This looks great!!! I am loving it!!! ❤️
Looks like you've used an older CDDL spec as reference, could you please go through and compare to an up-to-date version and make necessary adjustments?
I haven't gone through the whole PR, but as soon as all current comments are addressed I'll do a thorough review.
This PR pretty much takes care of #4125 and #4126
There are more to go: #4461 and #4462 and then some other ideas that we've discussed
eras/conway/impl/test/Test/Cardano/Ledger/Conway/Binary/CddlSpec.hs
Outdated
Show resolved
Hide resolved
|
Note that I have #4461 addressed, but planned to include that in a subsequent PR to avoid inflating this one. |
This commit introduces a re-implementation of the Conway CDDL specification using 'cuddle', a Haskell library for defining and validating CDDL. This allows us to generate and validate test cases without resorting to using the external `cddl` library.
- Fix protocol_param_update definition - Fix indices for certificates. - Inputs to a transaction should be inputs, not indices. - Index is sized - Fix the alternatives in credential. - Fix tags for constr. - script_ref has a tag - post_alonzo_transaction_output is a map - voter and gov_action_id are refs, not keywords - Appropriate size the index tag in redeemers - Both sides of the redeemers map are arrays, not maps
In particular, updating to this version prevents generating CBOR Maps with duplicate keys, which can result in parsing errors
Updated with all changes to conway.cddl up to 78b32d5
This is the final set of changes to the Conway CDDL spec made via comparison with the original cddl files for Conway. As of this commit, the remaining changes with the original cddl are as follows: - Various fields in crypto were defined with /= and are now defined with =. This does not affect the semantics, being instead a convention as per https://datatracker.ietf.org/doc/html/rfc8610#section-3.9 - The index keys in auxiliary_data are written as if they were value and not type keys: ``` 39,43c30,34 < / #6.259({? 0 : metadata < , ? 1 : [* native_script] < , ? 2 : [* plutus_v1_script] < , ? 3 : [* plutus_v2_script] < , ? 4 : [* plutus_v3_script]}) --- > / #6.259({? 0 => metadata > , ? 1 => [* native_script] > , ? 2 => [* plutus_v1_script] > , ? 3 => [* plutus_v2_script] > , ? 4 => [* plutus_v3_script]}) ``` This is a simpler formulation with identical semantics. - The alternatives in 'certificate', 'relay', 'native_script' and 'gov_action' are encoded using type1 choice semantics ('/') rather than group choice semantics ('//'). In the context, this is an equivalent formulation. - nonempty_oset has been removed, pending explanation of what it means. nonempty_set is used instead. - In various places, null is replaced with nil. They are synonyms. - The names of type parameters are changed; typically a is replaced with a0. - A few rules using references as the bounds in ranges are now inlined. This will be fixed once input-output-hk/cuddle#29 is addressed. - `distinct` is not currently possible to implement as a generic function, since `sized` is currently specialised to work on values. input-output-hk/cuddle#31 addresses. Until this is fixed, distinct<bytes> is instead distinct_bytes. - tstr is replaced with text. They are synonyms. - The keys in the 'pool_params' and 'script_n_of_k' groups are lost. This is due to input-output-hk/cuddle#32. However, since pool_params is included into an array, this does not alter the semantics, only the documentation. - datum_hash is of type datum_hash, not hash32. This seems an overlooked item in the original cddl.
7 tasks
This was referenced Aug 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces cuddle, a Haskell library for declaring and validating CDDL documents.
This is then used to reflect the Conway CDDL specification (in eras/conway/impl/src/Cardano/Ledger/Conway/CDDL.hs) using a DSL known as Huddle.
libs/cardano-ledger-binary/testlib/Test/Cardano/Ledger/Binary/Cuddle.hs introduces utilities to run CDDL conformance tests using cuddle. These are currently based on a Huddle declaration, but it would be equally possible to use a native CDDL specification combined with cuddle's CBOR generators.
eras/conway/impl/test/Test/Cardano/Ledger/Conway/Binary/CddlSpec.hs then introduces a new set of tests based in the Huddle conway specification. The existing test suite is left untouched for now.
Checklist
.cabalandCHANGELOG.mdfiles according to theversioning process.
.cabalfiles for all affected packages are updated. If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)CHANGELOG.mdfor the affected packages. New section is never added with the code changes. (See RELEASING.md)fourmolu(usescripts/fourmolize.sh)scripts/cabal-format.sh)hie.yamlhas been updated (usescripts/gen-hie.sh)