flow-nft icon indicating copy to clipboard operation
flow-nft copied to clipboard

Improvements to the NFT Standard

Open joshuahannan opened this issue 3 years ago • 21 comments

Issue To Be Solved

We recently had a discussion during Flow Developer Office hours about improvements to the NFT standard. There is some consensus that the current NFT standard is lacking in some areas and could use either:

  • A complete re-design and brand new standard

OR

  • Improvements to the existing standard

The first option would be nice because we could start from scratch and do whatever we want, but getting projects to migrate to the new standard and managing the existence of two standards at the same time would be quite difficult, so there is definitely an incentive to try to stick with the old standard if possible.

We will explore both possibilities within this issue and connected issues.

Notes from the conversation are here. The notes are still very rough and need a lot more detail. I will need help prioritizing and creating issues for each important improvement.

None of these are set in stone, they are simply just ideas for potential improvements. They all need a lot of discussion before we move forward with any of them.

Suggest A Solution

  • Maybe add a totalMinted field in addition to totalSupply: https://github.com/onflow/flow-nft/issues/11

  • Standard for storage and collection public paths

    • naming of the paths themselves and including the paths, addresses, and potentially types in the interface
    • may require an update to Cadence to allow more sophisticated path names
  • removing the id field and using uuid instead

  • remove total supply field

    • not necessarily needed
  • make ownedNFTs let instead of var

  • creating a transfer method that allows to withdraw and deposit in a single call and emit a transfer event

  • batch withdraw, batch deposit, and batch transfer

  • getName method

    • doesn't require a field, can use a custom function or struct
  • Generics

    • REALLY big change to Cadence, likely not possible any time soon, if at all
  • Default methods

    • we could structure the code to avoid having to write boilerplate code, especially having to force-cast the NFT in the
  • Requiring that a certain event is emitted in a function

    • requires an update to Cadence
  • Using the uuid for the key in the ownedNFTs dictionary could be more useful

  • being able to support multiple NFT types in a single contract

    • How would a single collection handle that, or would it be multiple collections
  • Accounts could all be initialized with a standard NFT collection so that there is always a place to store an NFT even if the one for a specific contract has been unlinked or not created

    • If the link is public, people could spam and flood an account's storage with NFTs (BAD)
    • could have a field in the collection to only allow up to a certain amount of NFTs to be deposited, or require those who deposit to the collection to help pay for the storage required
  • Need an NFTMinted or NFT Destroyed event: https://github.com/onflow/flow-nft/issues/34

  • Capability Index to a specific NFT in a collection to give the capability owner the ability to withdraw a single NFT from a collection : https://github.com/onflow/flow-nft/issues/36

  • Add getDefaultStoragePath() functions to FT+NFT interfaces

  • Add a nonce to NFT standard: #63

joshuahannan avatar Aug 23 '21 15:08 joshuahannan

Unfortunately, I missed the office hours :(

But I think the most important is somehow we need to decide on some basic rules ( like a kind of constitution, and build rules around them ) and accept that we cannot do everything.

For example for me: making ownedNFTs not public doesn't make sense, but there is no way to decide this without days of discussion with cons/pros. But if there was a rule saying that "user can always have to have ability to access resources they own" would be much easier.

Likewise:" Accounts could all be initialized with a standard NFT collection so that there is always a place to store an NFT even if the one for a specific contract has been unlinked or not created"

If there were some rules saying for example "you are responsible for the storage fee of the resource you created" it would be much easier to solve this issue than skip it. (even when storage fees are decided)

Always there is a discussion on minor subjects without some big picture official decisions (rulesets etc), taking a lot of time and energy.

When we are trying to do (or allow to do everything), I am feeling we are doing everything half-baked.

bluesign avatar Aug 23 '21 15:08 bluesign

Firstly I think we need to both work on NFTv2 but more importantly fix metadata/data/views for NFTv1. We should make sure that NFTv1 changes are compatible with v2. Why?

Because this change is urgent. More and more developers and solutions are comming to flow and if we want to be able to like sell all NFTs on the big marketplaces (assuming they come to flow) then we need a standard for v1. We need a standard that makes it possible for existing contracts to work via updating their nft contracts. More on this in #9.

As for NFTv2 I have added most of the suggestions that are possible right now from the above list to my GenericNFT repo https://github.com/bjartek/flow-nfmt/tree/nftv2 What is missing is currently

  • batch withdraw/transfer/deposit (if anybody wants to take a stab prs are welcome)

bjartek avatar Aug 23 '21 21:08 bjartek

I agree, I think making the update to v1 to add the getSchemas/getViews and any other possible upgrades here is probably more important than making a second standard. So lets try to start accelerating those discussions. Is there anyone who wants to go through this list and see if there are any possible improvements to V1 other than the metadata improvement?

joshuahannan avatar Aug 24 '21 19:08 joshuahannan

I just looked through the list and AFAIK nothing else can be added without breaking compatibility.

bjartek avatar Aug 24 '21 20:08 bjartek

Yeah, that is what it looked like to me, but I just wanted to make sure. That makes things easier! So we'll definitely move forward with discussing those improvements first.

joshuahannan avatar Aug 25 '21 16:08 joshuahannan

So we decide on naming of schema/view/metadata.

Then we decide if we should allow multiple instances of a type or not and use a string to differentiate them.

Then we should probably create some predefined structs and put in a seperate contract for convenience.

Or?

bjartek avatar Aug 25 '21 16:08 bjartek

Yeah, I think those are the main things we need to do. I think deciding on a name should be easy. I like view as the name and there seems to be some agreement about that. Then we'll probably need to create multiple proposals with example code for your second point so we can discuss those a bit more. There still seems to be some disagreement in #9 about those kinds of details, so we'll need to resolve those too. Then I think we show those proposals to more people on the Flow team and some of the important NFT marketplaces and projects to get more perspectives on it.

Creating the predefined structs would come later after the main designs have been approved.

joshuahannan avatar Aug 25 '21 16:08 joshuahannan

Agreed.

My code has example on how it works with multiple views of the same type. Somsomebody has to champion the "only-use-type" alternative.

bjartek avatar Aug 25 '21 16:08 bjartek

Are there any plans to add lending functionality for NFTs? I'm thinking of the following use case for NFTs in play-to-earn games/guilds. From https://eips.ethereum.org/EIPS/eip-2615

Suppose Alice owns NFTs and wants to rent out a NFT, and Bob wants to lease a NFT.

1. Alice approves the setting of a tenant-right for the NFT Alice owns.
2. Alice sends a rental listing to the rental contract.
3. Bob fills the rental request, and the right to use the NFT is transferred to Bob. At the same time, the tenant-right is set, and Alice becomes not able to transfer the right to use the NFT.
4. Bob registers rent (anyone can pay the rent).
5. Alice can withdraw the rent from the rental contract.
6. Alice can finish the agreement if the agreement period has ended and the agreement is kept (i.e. rent is paid without delay).
7. Alice can revoke the agreement if the agreement is breached (e.g. rent is not paid on time) and revoke the tenant-right and take over the right to use the NFT.

jonwalch avatar Nov 05 '21 17:11 jonwalch

Just checked this out and look forward to any reasonable improvements to the existing standard or a new standard.

In the case where a complete re-design and brand new standard is produced could these requirements be added:

  1. Migration guide for NFTs using old standard to new standard
  2. Visibility/timeline into when new standard may be deployed to testnet and mainnet

The visibility/timeline would be useful from a business/project decision perspective as maybe it makes sense for new projects that are considering deployment to testnet and mainnet to wait a little extra and use the new standard if the dates were close to coinciding.

Giners avatar Nov 05 '21 17:11 Giners

@jonwalch I don't think that a lending standard belongs in the core NFT standard, but we're happy to provide feedback on one if you would like to give it a shot!

@Giners Yes, we will definitely work with the community on a timeline and migration guide once we start working on this in earnest. We're currently focusing on getting the metadata standard figured out first, then will put our attention to this once that gets included

joshuahannan avatar Nov 08 '21 15:11 joshuahannan

@joshuahannan Thanks for the reply. I'm still in exploration mode. I'll certainly send a lending standard your way once developed. Would love feedback. :smiley:

jonwalch avatar Nov 08 '21 15:11 jonwalch

https://flowty.io

bjartek avatar Nov 08 '21 15:11 bjartek

https://flowty.io

Thanks for sharing @bjartek. They're solving 1/2 of the equation. I'm personally more interested in lending the NFT so the renter can use it. Very common use case for play-to-earn guild scholarships.

jonwalch avatar Nov 08 '21 15:11 jonwalch

I’d like to revisit this topic, specifically because of ownedNFTs field in the Collection.

Recently, I have been consistently butting heads with ownedNFTs being ‘pub’ in the interface. In my opinion, I believe Smart Contract devs should have the ability to decide how they want their NFTs to be passed around for multiple reasons:

  1. They should have the ability to restrict how their NFTs are passed around. Take FLOAT for example, the proof of attendance protocol on Flow. FLOATs are indeed NFTs, but there is a very valid use case where creators of certain events do not want their FLOATs to be traded between users, because that takes away from the “proof of attendance” aspect of the platform.

Even if this wasn’t the case, why is the standard able to tell developers how their NFTs are treated? After all, an NFT is just an asset. If I want my asset to only be passed around based on certain conditions, I should be able to say that in code.

  1. Maintain state. Often times a deposit or withdraw function is what maintains state in Cadence code. For example, upon depositing an NFT, I take some action in code. Same for withdrawals. By requiring ownedNFTs be pub, anyone can bypass those functions not just in terms of security reasons, but state will go out of sync and be problematic for all users.

One argument I see people making is that NFTs should truly be owned by a user. “This is the Cadence way,” as Cadence fits a resource oriented model where they have true control over their assets. I don’t believe in this mindset. While it is great Cadence allows for true ownership, you can still own your assets without having complete utter control over them. After all, making users go through a withdraw or deposit function is often necessary to maintain proper state. And all code is public for everyone to see.

jacob-tucker avatar Apr 07 '22 01:04 jacob-tucker

hey @jacob-tucker, I think Soul-Bound Token https://github.com/onflow/flow/pull/788 is very good idea ( from our bam from Emerald City)

For the NFT part, as we discussed earlier, that design ending up with having central ledger, so I don't think this is the Cadence way.

In my opinion this is the path that will happen.

  • First you need to remove deposit and withdraw and replace them with transfer ( otherwise people can access NFT and send to each other, do other stuff that can lose state sync (putting into custom NFT collection) )
  • Eventually you need to force transfer only between your kind of collection to keep state
  • So user actually never has access to resource. ( This defeats of having resource for composability etc )
  • Basically you end up with central ledger ( or something can be replaced with central ledger )

I think there can be a way, finding a middle ground. But most important part is deciding what are the priorities here.

Tbh I am even in favour of ( next to NFT standard ), a central ledger standard where developer's can have unlimited power over ledger. ( But as I said not sure how it fits with Flow and Cadence )

bluesign avatar Apr 07 '22 06:04 bluesign

Thank you for the response @bluesign

The issues with the described approach is that even if you disabled deposit and withdraw, people can always take directly ownedNFTs, so the transfer can be completely skipped. In addition, I think a soul-bound token is overdoing it for the purposes of the use cases I mentioned (like maintaing state upon certain actions). We shouldn't need (at least I think) to enforce soul-bound if we only want to update state upon deposit/withdraw.

I would also be interested in a ledger, but for the purposes of the standard, I would still argue for allowing developers to have greater flexibility with their designed assets, and changing the access modifier to access(self), unless there is reasoning not to.

jacob-tucker avatar Apr 07 '22 06:04 jacob-tucker

Thanks for commenting! Your concern actually won't be an issue any more after the June spork. I don't know if you've been paying attention to FLIPs, but Cadence has been updated to make it impossible to mutate a public array or dictionary field from outside the scope in which it is defined. So after the June, spork, you won't need to worry about this any more.

Also, we're almost done with our proposal for a reworking of the token standards, which we'll be sharing with the community very soon, which would also address most of these suggestions, as well as some more that we've been aware of.

joshuahannan avatar Apr 07 '22 15:04 joshuahannan

That just made my day. YES! Thank you Josh. Can't wait

jacob-tucker avatar Apr 07 '22 19:04 jacob-tucker

Another point that I feel are worth discussion here is.

  • Should the collection you put an NFT in be seperated from the NFT itself.

Many NFTs today are very simple and do not require and unique methods in a collection. As long as they can use views to expose the state they want then having the boilerplate of a collection in each NFT is not needed.

If we could couple this with a standard NFT collection that every account has by default it would solve a lot of hard problems. This collection should be like the standard NFT collection but then also have some methods to filter on type of nfts it holds aso.

Problems:

  • if each account has a default NFT collection you are potentially vulnerable to DDOS attacks where a blackhat can mint nfts and deposit to users to fill up their storage. Note that this is technically feasible today aswell but the users needs to have an collection added for a common NFT type that is free to mint. Like FLOATS or Rarible NFTs.

Personally I think this adds more value then it causes problems. The problem of a black hat filling up a users storage is something that should be solved seperately. There are many proposed suggestions to this, one is that a tx should pay for the storage it uses in a receiver. This would be a pretty huge change though and does not really work with dapper wallet either.

bjartek avatar Apr 08 '22 07:04 bjartek

Yes, we definitely think that is important @bjartek. We're actually finishing up a proposal for our refactoring of the token standards and your point is a big part of the NFT changes. I'll share it with you soon and we'll probably post it in the forum next week

joshuahannan avatar Apr 08 '22 15:04 joshuahannan