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-0054? | Cardano Smart NFTs #263

Merged
merged 10 commits into from
Jul 19, 2022
Merged

CIP-0054? | Cardano Smart NFTs #263

merged 10 commits into from
Jul 19, 2022

Conversation

kieransimkin
Copy link
Contributor

I've received a lot of feedback on this proposal both on the forum and in direct communication with various NFT creators. I've redrafted this proposal based on that feedback - I'm now moving on to create an initial implementation at Artifct - I will of course continue to take feedback and update the document as the implementation progresses.

@kieransimkin kieransimkin changed the title CIP-0054? - Cardano Smart NFTs CIP-0054? | Cardano Smart NFTs May 18, 2022
Authors: Kieran Simkin
Comments-URI: https://forum.cardano.org/t/cip-draft-cardano-smart-nfts/100470
Status: Draft
Type: Standards
Copy link
Member

Choose a reason for hiding this comment

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

This one would be a "Process" track as it does not entail any changes in the core Cardano components (no ledger or consensus changes; it is purely built upon the current system capabilities).

Copy link

Choose a reason for hiding this comment

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

Support of the mutable NFT part is necessary for recording system eternal changes and will allow to chase use cases like gaming (build your character, add items to stash, work your stats) as well as multi-verse cases (transfer your character and play in another game; interactions with multi-verse market places, etc.). You can also chase cases as Cardano Weather Station, Cardano Seismic Activity, Cardano Financial Markets, New Google Maps layers from Cardano NFTs (Helium internet coverage, Favourite fruit prices in your surroundings,...). And many many many more!

@rphair
Copy link
Collaborator

rphair commented Jun 20, 2022

(comment made prematurely)

@michaelpj
Copy link
Contributor

Just continuing my prophet-of-doom role to point out that CIP-25 still has security problems.


## Abstract

This CIP specifies a standard for an API which should be provided to Javascript NFTs, it also defines some additions to the 721 metadata standard defined in [CIP-0025](https://github.com/cardano-foundation/CIPs/tree/master/CIP-0025) which allow an NFT to specify it would like to receive certain current information from the blockchain.
Copy link
Member

Choose a reason for hiding this comment

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

Anything below this paragraph seems to belong more the "Motivation" than the abstract.

Created: 2022-05-18
License: CC-BY-4.0
---
# Cardano “Smart NFTs”
Copy link
Member

Choose a reason for hiding this comment

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

Please no title in the markdown 🙏, the title already is part of the front-matter data.


This combination of functionality enables many exciting new possibilities with on-chain NFTs.

## Description:
Copy link
Member

@KtorZ KtorZ Jun 28, 2022

Choose a reason for hiding this comment

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

Please rename as Specification (no colon)

Copy link
Collaborator

Choose a reason for hiding this comment

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

i.e. no "colon" (:) 😁

Copy link
Member

Choose a reason for hiding this comment

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

Whoops, not sure where my mind was.


```
{
"721": {
Copy link
Member

Choose a reason for hiding this comment

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

This proposal should use new key or, a new version number for 721. But either way, there needs to be a way to programmatically tell the expected structure of the metadata.


"uses": {
"transactions": <string | array>,
"tokens": <string | array>,
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest to avoid double types when designing API as much as possible. If there's only one item, then an array of one item is just fine and makes it much easier for code dealing with that structure later on.

Copy link
Member

Choose a reason for hiding this comment

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

Also, consider providing a CDDL specification for that on-chain format, rather than JSON. JSON is only how on-chain metadata are typically rendered, but they are actual stored as binary objects following in CBOR.


```
"uses": {
"transactions": "own"
Copy link
Member

Choose a reason for hiding this comment

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

What would be the semantic of "own"? How would a transaction identify as "own"? Would that refer to any transaction that has moved this NFT around (even if it didn't change wallets but was just carried as change)? Would that include the minting transaction? etc..

"asset1frc5wn889lcmj5y943mcn7tl8psk98mc480v3j"
]
}
```
Copy link
Member

Choose a reason for hiding this comment

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

Both examples with the asset id and the address are heavily user-facing / JSON-specific. In principle, this metadata would be stored on-chain and I would advise against storing these things are text strings. Both addresses and asset ids have well-defined on-chain binary representation, use them!

Also, regarding asset fingerprints, these are not meant to be global unique identifiers. They are fine in the context of a single wallet but they should not be used as id in a global setup. Rather, use the asset id (i.e. policy id + asset name). Incidentally, it would be okay to however display the asset fingerprint to the user, so long as it is stored as an asset id.


To enable modifier tokens - (that is, a token which you can hold alongside a Smart NFT which changes the Smart NFT's appearance or behaviour in some way); we provide a way for the Smart NFT to monitor the tokens held by a specific address. Similarly to the “transactions” key, the “tokens” key will also accept either a string or array.

In this case we define the “own” keyword to mean the tokens held by the same stake key that currently holds the Smart NFT itself.
Copy link
Member

Choose a reason for hiding this comment

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

What about stake part not directly associated to a single stake key? For instance, a stake part captured by a native monetary script as a combination of three keys? Or simply, an address without stake part? Would those be simply excluded from the 'own' keyword?


It would also be nice to be able to retrieve data that has been previously committed to the blockchain, separately to the NFT which wishes to access it. This would be useful for retrieving oracle data such as current Ada price quotes – as well as for allowing an NFT to import another NFT’s data.

Further to this - for on-chain programatically generated NFTs, it makes sense to mint the code to render the NFT as one token, and then have the individual NFTs contain only the input variables for that code. This CIP specifies an additional metadata option which specifies that an NFT should be rendered by another token - this will massively reduce code duplication in on-chain NFTs.
Copy link
Member

Choose a reason for hiding this comment

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

side-note: I'd love to see some example of existing JavaScript NFT in introduction to build a mental picture of those :)

"transactions": "own",
"renderer": "asset1frc5wn889lcmj5y943mcn7tl8psk98mc480v3j"
}
```
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure to understand the rationale for storing the rendering code on-chain 🤔 ? It seems to me like a misuse of a blockchain as a database (which it is not!). If the goal is to enforce that only a certain code is executed for rendering an certain NFT, I'd suggest to instead store a hash of that code and have client applications select the right rendering code based on that.

If the issue is to keep the client as dumb as possible then, I'd suggest to store the code on a system meant to store files such as IPFS, and only record a link / hash to that file on the Cardano blockchain.

@mangelsjover mangelsjover added the State: Waiting for Author Proposal showing lack of documented progress by authors. label Jun 28, 2022
@rphair
Copy link
Collaborator

rphair commented Jul 1, 2022

today's IO Dev Digest has linked back to this thread, calling for "community feedback" - https://mailchi.mp/iohk/dev-april-digest-675582

@mangelsjover mangelsjover added the State: Last Check Review favourable with disputes resolved; staged for merging. label Jul 13, 2022
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.

confirmed OK based on today's meeting

@SebastienGllmt
Copy link
Contributor

You may be interested in the changes introduced by Vasil as they allow things like CIP66(#294) and CIP68 (#299)

@rphair rphair merged commit bc16d56 into cardano-foundation:master Jul 19, 2022
@rphair rphair removed State: Last Check Review favourable with disputes resolved; staged for merging. State: Waiting for Author Proposal showing lack of documented progress by authors. labels May 2, 2023
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.

8 participants