Skip to content

Conversation

@aniketd
Copy link
Contributor

@aniketd aniketd commented Oct 30, 2025

Description

Addresses some more of the checklist in #5367 and is the third part of #5194

Checklist

  • Commits in meaningful sequence and with useful messages.
  • Tests added or updated when needed.
  • CHANGELOG.md files updated for packages with externally visible changes.
    NOTE: New section is never added with the code changes. (See RELEASING.md).
  • Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
    versioning process.
  • Version bounds in .cabal files updated when necessary.
    NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
  • Code formatted (use scripts/fourmolize.sh).
  • Cabal files formatted (use scripts/cabal-format.sh).
  • CDDL files are up to date (use scripts/gen-cddl.sh)
  • hie.yaml updated (use scripts/gen-hie.sh).
  • Self-reviewed the diff.

@aniketd aniketd changed the base branch from master to aniketd/more-cddl-deduplication October 31, 2025 12:01
@aniketd aniketd force-pushed the aniketd/even-more-cddl branch from c3b9ed7 to 6fda464 Compare October 31, 2025 12:02
@aniketd aniketd marked this pull request as ready for review October 31, 2025 18:34
@aniketd aniketd requested a review from a team as a code owner October 31, 2025 18:34
Base automatically changed from aniketd/more-cddl-deduplication to master October 31, 2025 20:36
@aniketd aniketd force-pushed the aniketd/even-more-cddl branch from 888e696 to ea10f9d Compare November 3, 2025 16:29
* Rename and move metadata, matadatum and metadatum_label definitions to
  core, along with transaction_index.
* Remove metadata_hash and just use
  auxiliary_data_hash everywhere.
Rename era-name-prefixed definitions with meaningful structural names
that are easier to reuse.
* Make native_script fields era-prefixed as and when they are introduced
* Add smart constructors for script types in core
* Fix script_n_of_k for eras after allegra
@aniketd aniketd force-pushed the aniketd/even-more-cddl branch from ea10f9d to 472c126 Compare November 3, 2025 16:58
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I love it!
Only a couple of minor suggestions

| 1. metadata (raw) - Supported since Shelley
| 2. auxiliary_data_array - Array format, introduced in Allegra
| 3. auxiliary_data_map - Tagged map format, introduced in Alonzo
| Dijkstra adds plutus_v4_script support at index 5
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a zero based index: so element 5 but index 4 😉

Suggested change
| Dijkstra adds plutus_v4_script support at index 5
| Dijkstra adds plutus_v4_script support at index 4

Comment on lines +407 to +421
mkInvalidBefore :: Named Group
mkInvalidBefore =
comment
[str|Timelock validity intervals are half-open intervals [a, b).
|This field specifies the left (included) endpoint a.
|]
$ "invalid_before" =:~ grp [4, a slot]

mkInvalidHereafter :: Named Group
mkInvalidHereafter =
comment
[str|Timelock validity intervals are half-open intervals [a, b).
|This field specifies the right (excluded) endpoint b.
|]
$ "invalid_hereafter" =:~ grp [5, a slot]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency sake we should use the same prefix for CDDL and constructor functions

Suggested change
mkInvalidBefore :: Named Group
mkInvalidBefore =
comment
[str|Timelock validity intervals are half-open intervals [a, b).
|This field specifies the left (included) endpoint a.
|]
$ "invalid_before" =:~ grp [4, a slot]
mkInvalidHereafter :: Named Group
mkInvalidHereafter =
comment
[str|Timelock validity intervals are half-open intervals [a, b).
|This field specifies the right (excluded) endpoint b.
|]
$ "invalid_hereafter" =:~ grp [5, a slot]
mkScriptInvalidBefore :: Named Group
mkScriptInvalidBefore =
comment
[str|Timelock validity intervals are half-open intervals [a, b).
|This field specifies the left (included) endpoint a.
|]
$ "script_invalid_before" =:~ grp [4, a slot]
mkScriptInvalidHereafter :: Named Group
mkScriptInvalidHereafter =
comment
[str|Timelock validity intervals are half-open intervals [a, b).
|This field specifies the right (excluded) endpoint b.
|]
$ "script_invalid_hereafter" =:~ grp [5, a slot]


### `testlib`

* Add CDDL definitions for `script_hash`, `mkScriptPubkey`, `mkScriptAll`, `mkScriptAny`, `mkScriptNOfK`, `mkInvalidBefore`, `mkInvalidHereafter`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

Suggested change
* Add CDDL definitions for `script_hash`, `mkScriptPubkey`, `mkScriptAll`, `mkScriptAny`, `mkScriptNOfK`, `mkInvalidBefore`, `mkInvalidHereafter`
* Add CDDL definitions for `script_hash`, `mkScriptPubkey`, `mkScriptAll`, `mkScriptAny`, `mkScriptNOfK`, `mkScriptInvalidBefore`, `mkScriptInvalidHereafter`

metadata = {* metadatum_label => metadatum}

transaction_metadatum_label = uint
metadatum_label = uint .size 8
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch on fixing the width of this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants