aptos-labs / aptos-core

Aptos is a layer 1 blockchain built to support the widespread use of blockchain through better technology and user experience.
https://aptosfoundation.org
Other
6.17k stars 3.64k forks source link

Implement an NFT module and user space #86

Closed davidiw closed 2 years ago

davidiw commented 2 years ago

We need a simple NFT module for aptos-framework, what does that entail?

A good starting spot is actually our NFT and NFTGallery in experimental: https://github.com/aptos-labs/aptos-core/blob/main/aptos-move/framework/experimental/sources/NFTGallery.move https://github.com/aptos-labs/aptos-core/blob/main/aptos-move/framework/experimental/sources/NFT.move

There's a lot of cruft in this code though

  1. We can't store too large a collection on a single user account, so we should either not create explicit collections -- this can be an event based concept or we should have a cap -- where only indexers know the relationship of a collection
  2. So many public functions that are inaccessible, if it isn't used, we should delete it -- apparently whoever wrote it thought we can call functions from userspace :(
  3. It isn't entirely clear how it works and how to publish new NFTs -- e.g., documentation is missing

Key requirements:

Proposal 1)

User points to one or more NFT collections, where each collection points to a set of NFTs.

Issue) an adversary could notice that a user is attempting to create a bunch of new accounts and try to claim those accounts beforehand by submitting the same data hash. This could result in lost resources, so it may be good to have the namespace be generated by something less easy to compute.

magnum6actual commented 2 years ago

Now that the unbridled masses are in here poking around (like me), this becomes a very important topic. Lots of us showing up here expecting to do the kind of stuff we've done elsewhere. I'll likely start building on the NFT.move code.

Would probably be good to have ERC721 metadata json compatibility for future cross chain applications.

magnum6actual commented 2 years ago

From: https://eips.ethereum.org/EIPS/eip-721

This is the “ERC721 Metadata JSON Schema” referenced above.


{
    "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."
        }
    }
}
davidiw commented 2 years ago

@magnum6actual , please! Have fun! Submit a PR :)

CapCap commented 2 years ago

I will throw in my 2c: having built indexers at scale, I really like the solana metadata format: https://docs.metaplex.com/token-metadata/specification it's close enough to the eth one that it's not that big a jump, but allows for more flexibility in terms of assets, etc. Not saying it's perfect, but I think we can take the best of all of em ;-)

davidiw commented 2 years ago

@CapCap @magnum6actual good point about the data inside. The intent of this project is to nail down how we may layout the data and not necessarily what is the right data within an NFT / NFT Collection. We should have a longer thread on that and make sure it makes sense for the community and involve the community before establishing an Aptos standard.

magnum6actual commented 2 years ago

Yeah - first step needs to be discussion leading to (quick?) adoption of standard, then the coding becomes easy. Needs to be as lightweight as possible. I'm just beginning to grasp the power of the Aptos "resource" concept, so we should ensure we fully leverage that capability.

It would be good to separate the business logic from the implementation standard. For example, the metaplex standard is built around the idea of a "first sale" then ongoing royalties from "secondary sales". We're building a novel utility NFT concept and in our use case, "first sale" would be late in the life-cycle of the NFT and is significantly less meaningful than first sale of the typical PFP NFT. So we have to jump through some hoops to make the metaplex standard work for our purpose. I'm too early in learning Aptos to make specific recommendations, but I'm thinking actions on transfer of NFT ownership could be handled by emitting events rather than hard-coded Busines logic on who gets what royalties.

More thoughts later as I get my situational awareness up to speed.

davidiw commented 2 years ago

@magnum6actual , I'm not a big fan of explicit on-chain royalties and such because they can be difficult to enforce in practice. In many other languages, there's nothing that stops the users from routing transactions through smart contracts that would eliminate any funnel back royalties or even using a cost of 0 on the network but the transfer being cash offline. They also result in bloat of the smart contract. Of course, it is still early, and maybe we or someone else comes up with logic to lock down the route NFTs can take on-chain to make at least on-chain royalties unavoidable.

The issue with events is that the yare not emitted within the confines of the VM, they are external only. So we'd need explicit logic to handle royalties.

magnum6actual commented 2 years ago

We're thinking the same direction. Hard coded structures for royalties just muck things up. Even with the rigid standard from Metaplex - the royalties are completely unenforceable. They are only paid because marketplaces like Magic Eden (Solana) choose to comply. There's nothing in code that can control it.

Rather than putting out a hard coded royalty standard for voluntary compliance, maybe it's as simple as a standard for a call back function to be executed on a sale that takes the sales price as an input and returns a list of accounts and amounts to be distributed to each out of the sale price. That gives the issuer freedom/creativity to do things like 50% royalties in the first 30 days (to discourage flipping) and decrease it over time or royalty to the artist and a separate stream to a charity. All that could be baked in the code of the NFT itself and the only thing required from a standards standpoint is the function interface.

As I get to know Move/Aptos better - how it should look is starting to shape up in my mind. I think it should perhaps be a resource with two components - one defined with a handful of standard fields and then a second that is just a generic and the developer can put whatever they want in there. From Aptos, something like:

module Aptos::NFT

struct NFTStandard has store {
      name: ASCII::string,
      description: ASCII::string,
      previewImageURL: ASCII::string,
      actionsOnSaleScriptURL: ASCII::string, // thinking this could be the end point to a REST api call; need to think it through more
      standardField3: <type>,
      standardField4: <type>,
     etc.
}

and the user would build on it like:

module MySuperCoolNFT::Season1

use Aptos::NFT::NFTStandard as AptosNFT;

struct NFT<Standard: store, User: store> has key {
      standard: Standard,
      user: User
}

struct UsersNFTStruct has store {
     couldBeAnything: <type>,
     couldBeAnything: <type>,
}

let standard = AptosNFT { all the required data..... };
let user = UsersNFTStruct {all their data....};
move_to<NFT<AptosNFT>,<UsersNFTStruct>>(&account, <NFT<AptosNFT>,<UsersNFTStruct>> {standard, user});

I don't know if that makes sense yet as I'm still very much on the front end of learning Move, but I think it gives you the beginnings of a very light standard architecture with maximum flexibility/creativity for the user. More that can/should be around that - but that's where I'd start.

davidiw commented 2 years ago

I'd like to imagine there are a set of collectibles or token that folks want to standardize around to make the life of indexers easier. Folks are free to add their own Ts. I might include a little more in the "NFT" core like name, description, url, and collection. Then maybe create an Empty inner T for MVP. Then creative folks can extend that but need to write code to do so.

In terms of collectibles, the current state is that Move doesn't natively support blobbed data from being combined together short of serializing and deserializing it into blobs -- not even sure if that would work.

We're exploring other alternatives, but our ideal is to have as much pure move as possible.

davidiw commented 2 years ago

@magnum6actual , here's a proof of concept: https://github.com/aptos-labs/aptos-core/pull/296

We discussed it yesterday and felt overall okay with the underlying logic -- not necessarily the fields, but at least the organization of the data (e.g., collection, gallery, token)

Would love to hear your thoughts.

davidiw commented 2 years ago

MVP is in code!

NoelJacob commented 2 years ago

Any changes on address generation?

Issue) an adversary could notice that a user is attempting to create a bunch of new accounts and try to claim those accounts beforehand by submitting the same data hash. This could result in lost resources, so it may be good to have the namespace be generated by something less easy to compute.

Another similar network solved this by adding randomness and storing address of nft items in collection contract. It can be done atleast for collection not minting all nfts at once.

davidiw commented 2 years ago

We use 32-byte addresses derived from public keys. This is very secure :)