feat: add NFT standard

[friedger]

This PR adds a SIP for the Non-fungible standard.

It fixes #2

[hstove]

Thanks so much for putting this together @friedger!

I am pretty hesitant to support too many display-oriented functions in this standard. I am much more in favor of a separate trait that does a great job at returning display data for an NFT.

For example, you have a mandatory function for icon-url (uint), but I can imagine many NFTs that wouldn't want this. For example, NFTs that represent ownership of an asset (like a house), or an NFT that denotes membership of something.

Plus, if we wanted to really support easily displaying NFTs, this seems too limited. Why not have name, description, etc.

I would love a standard that does the job, while supporting flexibility and easy implementation. In my opinion, we only need:

get-balance
transfer?
get-owner?

I am curious to hear your opinion on defining standard error codes for transfer? and get-owner?. I believe that could go a long way towards a good experience for clients.

[hstove]

@friedger can you rename the file to sips/sip-009-nft-standard.md? This is in line with the existing SIPs, which I just ported in #4.

[friedger]

There is already a discussion about error types.

For transfer?, we could define an error kind for the native transfer:

{kind: "nft-transfer-failed", code: from-nft-transfer}

[friedger]

@hstove We don't really need get-balance, do we?
However, we need get-last-id to ensure that it is iteratable

[hstove]

@friedger agreed that we don't need get-balance, and get-last-id makes sense too. I think the error handling part is the only thing that might need some tweaking. I'm in favor of structured errors, but regardless of what we decide as the signature, can we come up with the standard error types for transfer?

Another thing that we should do, and make as a best practice for this kind of SIP, is to require an example implementation. A simple repo with the trait and an implementation would be perfect. Bonus points for (simple) JS code, especially to demonstrate error handling.

All opinions my own, of course - would love to get more eyes on this.

[Zk2u]

@hstove example implementations are a great idea. my thinking would be to create a monorepo under the org that holds all implemented SIP examples, though through drafts they would be held by the implementer.

[hstove]

@xelamade Friedger added a reference implementation in this PR: https://github.com/friedger/clarity-smart-contracts/blob/master/contracts/sips/nft-trait.clar

[hstove]

Hey @friedger - have been doing some reading around ERC721 implementations. The standard includes a function tokenURI - defined in Solidity as function tokenURI(uint256 _tokenId) external view returns (string);.

This returns a URL which includes JSON metadata that clients can use to embed information about the NFT. I believe we should follow a similar practice.

What do you think about adding this function:

(metadata-url (uint) (string-ascii 256))

Additionally, it would likely be best to build off of the existing JSON schema used in the ERC721 standard:

{
    "title": "Asset Metadata",
    "type": "object",
    "properties": {
        "name": {
            "type": "string",
            "description": "Identifies the asset to which this NFT represents"
        },
        "description": {
            "type": "string",
            "description": "Describes the asset to which this NFT represents"
        },
        "image": {
            "type": "string",
            "description": "A URI pointing to a resource with mime type image/* representing the asset to which this NFT represents. Consider making any images at a width between 320 and 1080 pixels and aspect ratio between 1.91:1 and 4:5 inclusive."
        }
    }
}

[friedger]

@hstove I like get-metadata-uri or shorter get-meta-uri, similar to get-owner. Optionally, we can support get-meta that returns the meta data stored on-chain.

Is the length long enough? Why 256?

Should we limit the asset meta data to images? I suggest using "uri" and "mime-type".

We discussed immutability, therefore, I suggest to include a hash in the contract (get-asset-hash (uint) (response (buff 64) uint))

(This is all about the meta data and should go in a separate section of the standard and it should be optional.)

[hstove]

Well, if this is a URI, do you think 256 is not long enough? It's not for storing on-chain data - this method must return a resolvable URI (although it could be on http, ipfs, etc). Supporting get-meta is just more tricky, because then we have to specify a very strict format, that can't be easily extended.

For example, the JSON schema I linked is for image-based NFTs, but in ETH there are now more semi-formal standards for other NFTs - like 3d objects, videos, etc.

I also like simply using an off-chain pointer because there is no need to access this data on-chain.

We discussed immutability, therefore, I suggest to include a hash in the contract (get-asset-hash (uint) (response (buff 64) uint))

This is an interesting idea - this would be a hash of the JSON metadata? I like it - it's light enough and clients can choose how they respond to an invalid hash. However, if you wanted immutability, you could just use an IPFS-resolving URI, which are immutable.

(This is all about the meta data and should go in a separate section of the standard and it should be optional.)

Agreed that all the JSON schema stuff should be left out of this strict standard. I'm mainly advocating for the URI method, as that'll be quite important to support a decentralized ecosystem of NFT apps.

[friedger]

Addresses some comments, more to come

[jcnelson]

Looks good to me! Once you feel ready and all the remaining comments are addressed, let's advance this to activation-in-progress. Once the trait definition is on mainnet, can you add its address to the Activation section?

We can always keep working on the text content. However, it looks like the reviewers here feel that the trait definition itself is ready. If so, then let's advance this SIP.

[friedger]

@muneebm I try to incorporate your comments. However, it removes the addition implementation conditions. Is this on purpose? I think the specification must also provide requirements for the contract implementation. These could be tested by application developers.

[friedger]

The trait has been deployed to testnet and mainnet!

[jcnelson]

Awesome! This SIP is now in activation-in-progress status.

[hstove]

Given our earlier discussion and stacksgov/grants-program#73, are we on the same page about only a uint error code for transfer?

[friedger]

@hstove Yes, sip has been updated, new version deployed to mainnet, new beeple nft published.

[Zk2u]

all looks good. ready for launch! 🚀

[314159265359879]

@friedger and @markmhx what do you think about making one more change to this trait?
The current name is nft-trait(nft-trait)
I think if there are more NFT standards in the future you will want to distinguish them from each other. As already suggested by @agraebe for fungible tokens: #25 (review) naming it: sip-010-trait. (the current trait deployment for FT also has sip10, in the name).

Perhaps follow a similar line for this NFT trait: (re)deploying it with the sip-009-trait name?

[friedger]

@314159265359879 The name is not part of the sip, IMHO. You can deploy them using any name and as often as you want. I think in the future, we will have some better referencing to sip trait contracts, e.g. via BNS names.

[friedger]

Should this be merged as the sip is activated?

[jcnelson]

And it's official! Congratulations @friedger! SP2PABAF9FTAJYNFZH93XENAJ8FVY99RRM50D2JG9.nft-trait.nft-trait is now officially a standard 🎉

[jcnelson]

Thanks to everyone who helped review and get this ready!