Skip to content
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

CIP-0025: Added version 2 and cddl #267

Merged
merged 7 commits into from
Jun 28, 2022

Conversation

alessandrokonrad
Copy link
Contributor

No description provided.


metadata_details =
{
name : text,
Copy link
Contributor

Choose a reason for hiding this comment

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

We have to be a little careful with this definition since text here isn't the standard "text" in cbor. Instead, Cardano metadata enforces that all text or byte array has a maximum length. We should probably either define these as a special type that matches the same max length or explicitly allow people to use an array to combine chunks together

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I just fixed it. name should definitely stay below 64 characters. The ones that can exceed 64 characters have the type string / [* string].

@KtorZ KtorZ added the Update Adds content or significantly reworks an existing proposal label May 25, 2022
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

👍 Approving.
@alessandrokonrad perhaps consider moving the CDDL as separate files so they can be linked / downloaded by tools / CI for validation & automation.

Will leave the PR open for a bit, and we'll merge in the next editor bi-weekly at latest.

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

really easy to read with CDDL in separate files 🤓

Copy link
Contributor

@SebastienGllmt SebastienGllmt left a comment

Choose a reason for hiding this comment

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

LGTM

@rphair rphair merged commit 8ddf4f8 into cardano-foundation:master Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Update Adds content or significantly reworks an existing proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants