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

[move] Add NFT module to framework + use natural numbers as example #739

Merged
merged 2 commits into from
Mar 11, 2022

Conversation

sblackshear
Copy link
Collaborator

  • Add NFT<T> type, intended to be an ERC721-like standard that we can point folks to. Functions are extensively documented to suggest the intended usage. It's not 100% clear to me that this provides much value over asking folks to define an object with a T field + their own mint and burn functions, but I think this template is a helpful starting point for anyone coming from Eth.
  • Add Num as a silly example of how to use the standard

Copy link
Contributor

@lxfind lxfind left a comment

Choose a reason for hiding this comment

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

Nice!

@@ -0,0 +1,38 @@
module Num {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would NumNFT be a better name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought about it, but then the type that gets minted is NFT<NumNFT>, which seems a bit verbose... We could also have different struct vs module names, but that goes against standard Move conventions, which I would prefer not to do in an example.

- Add `NFT<T>` type, intended to be an ERC721-like standard that we can point folks to. Functions are extensively documented to suggest the intended usage. It's not 100% clear to me that this provides much value over asking folks to define an object with a `T` field + their own `mint` and `burn` functions, but I think this template is a helpful starting point for anyone coming from Eth.
- Add `Num` as a silly example of how to use the standard
@666lcz
Copy link
Contributor

666lcz commented Mar 11, 2022

Add NFT type, intended to be an ERC721-like standard that we can point folks to.

This is a great example! However, I think we should think more thoroughly about what the standard should look like. The most important thing is to make it easy for various wallets, explorers, marketplaces, and games to differentiate(e.g., to tell which move objects are fungible coins, collectibles, or just some random objects) and render these metadata. For example, without a common name and image field, the above-mentioned UIs will have a hard time rendering these assets properly and result in a bad user experience across the ecosystem.

We should also think across different asset classes as well such as semi-fungible tokens and fungible tokens.

@sblackshear
Copy link
Collaborator Author

Add NFT type, intended to be an ERC721-like standard that we can point folks to.

This is a great example! However, I think we should think more thoroughly about what the standard should look like. The most important thing is to make it easy for various wallets, explorers, marketplaces, and games to differentiate(e.g., to tell which move objects are fungible coins, collectibles, or just some random objects) and render these metadata. For example, without a common name and image field, the above-mentioned UIs will have a hard time rendering these assets properly and result in a bad user experience across the ecosystem.

We should also think across different asset classes as well such as semi-fungible tokens and fungible tokens.

+1 to all of this. I'm not the biggest expert on these standards (especially the non-Eth ones), and would love design help/feedback/implementation help on how we should define these standards in Move, to what extent we should strive for compatibility with existing standards vs learning from their mistakes, how this impacts mirroring, etc. I set up some time with you to brainstorm on this tomorrow.

To say a few quick things:
In this first cut, I was only trying to mimic the core functionality of ERC721, not the metadata extension with name, symbol, and tokenURI. I was thinking of adding these via something like

struct Erc721Metadata {
  name: String,
  symbol: String,
  url: Url  // from https://github.com/MystenLabs/sui/pull/726
}

and then either adding Option<Erc721Metadata> to NFT<T>, or asking the programmer to instantiate T with Erc721Metadata. But am not all sure this is the best approach. In particular, I'm wondering whether we want all the JSON stuff in the Eth standard to live on-chain instead of at url (since that's easy in Sui) and whether we should leverage UrlCommitment.

For example, without a common name and image field, the above-mentioned UIs will have a hard time rendering these assets properly and result in a bad user experience across the ecosystem.

On the image part: I was thinking of standardizing this at the level of Url rather than doing something specific to NFT<T> or images. That is: explorers, wallets, etc will know that when they see the URL type, they should fetch the underlying data (which might be an image, text, or something else--we can define that based either on a type stored at the Url or define more specialized on-chain types like JpegUrl). I think if we focus on standardize the fine-grained "building blocks" like Url instead of coarse-grained assets like NFT's, that will make it much easier to build up rich assets, games, etc. that are natively supported by everything on the client side.

make it easy for various wallets, explorers, marketplaces, and games to differentiate(e.g., to tell which move objects are fungible coins, collectibles, or just some random objects

For fungible coins, our standard is Coin<T>. I feel a bit more confident about this one than I do about NFT standards because Diem, StarCoin, and 0L have all done something similar without problems, but definitely open to feedback on this one too!

In general, I think the approach we should take for clients to understand how they should display and interpret Sui objects is that they should use the Move type. Clients will understand a subset of important Move types defined in the framework, and over time that set will grow to include types defined by popular third-party use-cases (and hopefully third-party standards as well!)

Co-authored-by: Chris Li <76067158+666lcz@users.noreply.github.com>
@sblackshear sblackshear merged commit 20fcf96 into MystenLabs:main Mar 11, 2022
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