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.

Sign up for GitHub

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

Jump to bottom

CIP-0068 | Improve image types and URI descriptions #562

Merged
merged 1 commit into from
Aug 15, 2023
Merged

CIP-0068 | Improve image types and URI descriptions #562

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 35 additions & 22 deletions CIP-0068/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,19 +124,27 @@ files_details =
{
? name : bounded_bytes, ; UTF-8
mediaType : bounded_bytes, ; UTF-8
src : bounded_bytes ; UTF-8
src : uri,
; ... Additional properties are allowed
}

metadata =
{
name : bounded_bytes, ; UTF-8
image : bounded_bytes, ; UTF-8
? mediaType : bounded_bytes, ; UTF-8
rphair marked this conversation as resolved.
Show resolved Hide resolved

; The image URI must point to a resource with media type (mime type) `image/*`
; (for example `image/png`, `image/jpeg`, `image/svg+xml`, etc.)
image : uri,

? description : bounded_bytes, ; UTF-8
? files : [* files_details]
; ... Additional properties are allowed
}

; A valid Uniform Resource Identifier (URI) as a UTF-8 encoded bytestring.
; The URI scheme must be one of `https` (HTTP), `ipfs` (IPFS), `ar` (Arweave) or `data` (on-chain).
; Data URLs (on-chain data) must comply to RFC2397.
uri = bounded_bytes ; UTF-8

; Custom user defined plutus data.
; Setting data is optional, but the field is required
Expand Down Expand Up @@ -204,17 +212,19 @@ metadata =
description : bounded_bytes, ; UTF-8
? ticker: bounded_bytes, ; UTF-8
? url: bounded_bytes, ; UTF-8
? logo: uri,
? decimals: int

; 'logo' does not follow the explanation of the token-registry, it needs to be a valid URI and not a plain bytestring.
; The logo URI must point to a resource with media type (mime type) `image/png`, `image/jpeg` or `image/svg+xml`.
? logo: uri,
Comment on lines 214 to +219
Copy link
Collaborator

Choose a reason for hiding this comment

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

In this section I believe we should keep the naming convention consistent with the Cardano Token Registry to ensure minimal disruption when moving between one standard or the other. Perhaps it is better to move this comment out of the CDDL and use a separate "logo-uri" object that explicitly declares that there is NOT support for data: scheme URIs.


; ... Additional properties are allowed
}

; A URI as a UTF-8 encoded bytestring.
; The URI scheme must be one of `https`, `ipfs` or `data`
; Do not encode plain file payloads as URI.
; 'logo' does not follow the explanation of the token-registry, it needs to be a valid URI and not a plain bytestring.
; Only use the following media types: `image/png`, `image/jpeg`, `image/svg+xml`
uri = bounded_bytes
; A valid Uniform Resource Identifier (URI) as a UTF-8 encoded bytestring.
; The URI scheme must be one of `https` (HTTP), `ipfs` (IPFS), `ar` (Arweave) or `data` (on-chain).
; Data URLs (on-chain data) must comply to RFC2397.
uri = bounded_bytes ; UTF-8
Comment on lines +224 to +227
Copy link
Collaborator

Choose a reason for hiding this comment

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

Drop mention of support for the data: scheme when working with fungible token logo-uri field to be compatible and match the CIP-26 formatting of the Token Registry.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Point of clarification cause I may have misunderstood here, the token registry stores the image in ONLY data: format by default and that is actually an acceptable URI in this case but we just want it to be a "plain text" URI rather than a raw bytestring?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this part was written by @alessandrokonrad & @AndrewWestberg, maybe they can clarify as I believe I just moved things around.

In the token registry, images are base64 encoded PNGs, they are not data URLs, but I think data urls are valid here.

Copy link
Contributor

Choose a reason for hiding this comment

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

@alessandrokonrad can comment on this. My addition was only to add 444 which is a copy paste of parts of 222 and 333.


; Custom user defined plutus data.
; Setting data is optional, but the field is required
Expand Down Expand Up @@ -277,27 +287,28 @@ files_details =
{
? name : bounded_bytes, ; UTF-8
mediaType : bounded_bytes, ; UTF-8
src : bounded_bytes ; UTF-8
src : uri,
; ... Additional properties are allowed
}

metadata =
{
name : bounded_bytes, ; UTF-8
image : bounded_bytes, ; UTF-8
? mediaType : bounded_bytes, ; UTF-8
Copy link
Collaborator

Choose a reason for hiding this comment

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

(potentially requiring version bump to 2 or 3)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed!

Copy link
Contributor Author

@SmaugPool SmaugPool Jul 31, 2023 •
edited

Choose a reason for hiding this comment

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

image is not removed, and mediaType was optional, so I'm not sure of the benefit of bumping the version?

Copy link
Collaborator

@rphair rphair Jul 31, 2023 •
edited

Choose a reason for hiding this comment

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

Because officially now including a mediaType would be erroneous, while it wouldn't have been an error before whether it was missing or present. How such an error would be handled I believe would depend upon the implementor: so we would be giving them that option by bumping the version. See problem reported in #520.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would not be erroneous because additional properties are allowed:

; ... Additional properties are allowed

Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks @SmaugPool - I missed that! 🤓 @Ryun1 we can talk about any other reasons why we might need the version bump @ the CIP meeting today, but it seems like we are OK without one (cc @KtorZ).


; The image URI must point to a resource with media type (mime type) `image/*`
; (for example `image/png`, `image/jpeg`, `image/svg+xml`, etc.)
image : uri,

? description : bounded_bytes, ; UTF-8
? decimals: int,
? files : [* files_details]
; ... Additional properties are allowed
}

; A URI as a UTF-8 encoded bytestring.
; The URI scheme must be one of `https`, `ipfs` or `data`
; Do not encode plain file payloads as URI.
; 'logo' does not follow the explanation of the token-registry, it needs to be a valid URI and not a plain bytestring.
; Only use the following media types: `image/png`, `image/jpeg`, `image/svg+xml`
uri = bounded_bytes
; A valid Uniform Resource Identifier (URI) as a UTF-8 encoded bytestring.
; The URI scheme must be one of `https` (HTTP), `ipfs` (IPFS), `ar` (Arweave) or `data` (on-chain).
; Data URLs (on-chain data) must comply to RFC2397.
uri = bounded_bytes ; UTF-8

; Custom user defined plutus data.
; Setting data is optional, but the field is required
Expand Down Expand Up @@ -354,9 +365,11 @@ To keep metadata compatibility with changes coming in the future, we introduce a

## References

- CIP-0025: https://github.com/cardano-foundation/CIPs/blob/master/CIP-0025
- CIP-0031: https://github.com/cardano-foundation/CIPs/tree/master/CIP-0031
- CIP-0067: https://github.com/cardano-foundation/CIPs/tree/master/CIP-0067
- [CIP 25 - Media NFT Metadata Standard](https://github.com/cardano-foundation/CIPs/blob/master/CIP-0025)
- [CIP 31 - Reference inputs](https://github.com/cardano-foundation/CIPs/tree/master/CIP-0031)
- [CIP 67 - Asset Name Label Registry](https://github.com/cardano-foundation/CIPs/tree/master/CIP-0067)
- [RFC 3986 - Uniform Resource Identifier (URI)](https://www.rfc-editor.org/rfc/rfc3986)
- [RFC 2397 - The "data" URL scheme](https://datatracker.ietf.org/doc/html/rfc2397)

## Copyright

Expand Down