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

NEP: Non-fungible Token Standard #41

Closed
wants to merge 35 commits into from
Closed

Conversation

hal0x2328
Copy link
Contributor

Initial NFT proposal from Splyse/HashPuppies team

@erikzhang
Copy link
Member

Now we have 3 different NFT Standard proposals: #30 , #37 , #41 .
We need to choose one to accept.

@saltyskip
Copy link
Contributor

Have you considered a divisible non fungible token? Easier to explain with an example.

Let's see we want to put a house on the blockchain. There could be one NFT that represents this house, however due to the immense value it holds, houses are split up into pieces in some sort of real estate trust. So each person might own 10% of each house. This allows for high value objects to be tokenized more effectively.

The whole is non fungible even if the the individual pieces of aren't. This also works really well for tokenized art.

If an art piece is worth 50 million dollars, it is hard for own person to own as one non fungible token. However if it is tokenized into 100 fungible parts than it becomes much easier for collective ownership.

I bring this up because NEO seems to be an excellent place to put high value (singular )non fungible items as a single contract, and then tokenizing the individual non fungible entity into smaller (fungible or non fungible) parts. This leads into the process of digitizing real world assets

@erikzhang
Copy link
Member

@saltyskip This sounds interesting. Please make a specific proposal.

@hal0x2328
Copy link
Contributor Author

hal0x2328 commented Apr 23, 2018

@saltyskip An NFT token could have multiple owners currently a using multi-signature address, in the same way a group of owners could own a batch of NEP-5 tokens together. But the percentage of ownership isn't part of the multi-sig standard so that data would not be taken into account.

However with our NFT proposal, the contract owner could serialize the information about owner percentages into the RWData storage and change it any time the token changes hands (i.e. M of N keyholders sign the token over to a new multi-sig address). It would require some coordination however to register new multi-sig ownership hashes during the transfer() event.

@erikzhang
Copy link
Member

A multi-sig address means multiple owners can own one NFT, doesn't mean someone can own a part of one NFT.

@hal0x2328
Copy link
Contributor Author

@erikzhang By virtue of the fact that they together own one NFT, doesn't that mean they each technically own a part of one NFT? The multi-sig method could serve most use cases except where you want to sell your share without the consent of the other owners and cooperation of the token contract (which is what I think you are trying to illustrate with your comment). Yes, that would require a different model where parts of the token have to exist as a unique entity with a single owner.

Perhaps a master meta-token type could be created that is just composed of a list of other tokens and the relative percentages of ownership they represent. But, now you have to consider divisibility of the sub-tokens - what if I want to sell my 10% share to two people, one of whom pays me 80% and the other and 20% of the share, giving them a resulting 8% and 2% stake in the master token? If the token share owner can't create new child tokens at will through the act of transferring them this scheme doesn't work.

Really interesting to think about, would love to add thoughts to the new proposal from @saltyskip when it is added.

@erikzhang
Copy link
Member

@hal0x2328 I have some questions for you to answer.

  1. As a Non-fungible Token Standard, why we need circulation and balanceOf? Since each token is not the same, there should be no need to count the number of tokens in an account?

  2. In the case of a transfer, if we know the tokenid, we can get the owner of it. So there's no need to provide the from parameter for transfer?

  3. As a Non-fungible Token Standard, why we need decimals? Does this mean that each token is still divisible as @saltyskip has said? If so, then the balanceOf method should accept tow parameters: owner and tokenid.

  4. Do we really need RWData and URIData? I guess a lot of tokens may just need ROData. Maybe we can simplify the standard?

@erikzhang
Copy link
Member

erikzhang commented May 10, 2018

In addition, there is a description in the ERC721:

NOTE: Current limitations in Solidity mean that there is no efficient way to return a complete list of an address's NFTs with a single function call. Callers should not assume this method is implemented efficiently (from a gas standpoint) and should strenuously avoid calling this method "on-chain" (i.e. from any non-constant contract function, or from any constant contract function that is likely to be called on-chain).

But in Neo, we have great ways to return the complete list of NFTs. So we really don't need tokenOfOwnerByIndex.

@hal0x2328
Copy link
Contributor Author

hal0x2328 commented May 10, 2018

As a Non-fungible Token Standard, why we need circulation and balanceOf? Since each token is not the same, there should be no need to count the number of tokens in an account?

circulation is a synonym for totalSupply. This was done to maintain compatibility with the NEX ICO template in case external code relies on that operation since it exists in other NEP-5 implementations.

balanceOf would be used in conjunction with tokenOfOwnerById - we need to know how many items exist to iterate over - see the reasoning for including tokenOfOwnerById below and see if it makes sense.

In the case of a transfer, if we know the tokenid, we can get the owner of it. So there's no need to provide the from parameter for transfer?

This parameter could be eliminated, true.

As a Non-fungible Token Standard, why we need decimals? Does this mean that each token is still divisible as @saltyskip has said? If so, then the balanceOf method should accept tow parameters: owner and tokenid.

This is just to maintain compatibility with existing wallets, explorers, etc that expect that method to exist as it does with NEP-5, and might even be a way they use to differentiate (at the time, the supportedStandards proposal was not yet in-the-works). However, since it always returns zero, eliminating would effectively have the same result as long as the NFT implementation returned a False value at the end of execution where no operations match. I've seen some developers returning -1 from contract errors, so keeping this function and forcing Decimals to always return 0 seemed reasonable.

Do we really need RWData and URIData? I guess a lot of tokens may just need ROData. Maybe we can simplify the standard?

We were trying to envision the most common use cases and develop a uniform way to satisfy all of them. In our game, non-fungible assets will not only have "genetics" but will also have transient properties that should be attached to the asset and carry over when the token is transferred. We figure once gaming or other NFT assets become more complex this would be a common requirement, and querying these properties should be done in a uniform way. And URIData would also likely be a pretty globally-needed feature too, we expect that NFT assets are going to need some sort of representation of their uniqueness in image form for human eyes.

But in Neo, we have great ways to return the complete list of NFTs. So we really don't need tokenOfOwnerByIndex.

It seemed to us you still would want a way for your off-chain code to request one token at a time from a subset in a predictable order from the contract instead of retrieving the entire list every time (which eventually might grow quite large), and maintaining the Ethereum method for doing so might make it easier for NFT-using Ethereum dApps to be ported to Neo.

@erikzhang
Copy link
Member

circulation is a synonym for totalSupply. This was done to maintain compatibility with the NEX ICO template in case external code relies on that operation since it exists in other NEP-5 implementations.

We are not going to make NFT standard be compatible with NEP-5. Also, adding methods for compatibility with the NEX ICO template is not the right choice, and we should make this standard more focused on the needs of NFT itself. NEX can modify their templates for NFT standard.

However, since it always returns zero, eliminating would effectively have the same result as long as the NFT implementation returned a False value at the end of execution where no operations match. I've seen some developers returning -1 from contract errors, so keeping this function and forcing Decimals to always return 0 seemed reasonable.

If an error occurs in the contract, the exception should be thrown by the THROW opcode directly. If decimals always returns zero, then it is really not necessary to exist.

And URIData would also likely be a pretty globally-needed feature too, we expect that NFT assets are going to need some sort of representation of their uniqueness in image form for human eyes.

Agree with that. But I don't think RWData would be a common requirement. And perhaps RWData is more suitable for storage offchain.

It seemed to us you still would want a way for your off-chain code to request one token at a time from a subset in a predictable order from the contract instead of retrieving the entire list every time (which eventually might grow quite large)

Perhaps we can use the Iterator APIs to return the token iterator instead of the token list? And, if it's off-chain code, we don't have to care about the size of the data.

and maintaining the Ethereum method for doing so might make it easier for NFT-using Ethereum dApps to be ported to Neo.

I don't think we can port Ethereum dApps easily since there are many difference between NeoVM and EVM.

@erikzhang
Copy link
Member

I've been thinking about @saltyskip proposed divisible non-fungible token. Here are some of my simple ideas:

Divisibility Extension For Non-fungible Token

Methods

  1. decimals()

Methods (if decimals() > 0)

  1. balanceOf(owner, tokenid)
  2. transferPartially(tokenid, to, amount)

Events (if decimals() > 0)

  1. transfer(tokenid, to, amount)

@erikzhang
Copy link
Member

@hal0x2328 @saltyskip @lightszero @RunLyo
I amended this standard to include optional divisibility and remove some non-essential methods.
Please review it.

@saltyskip
Copy link
Contributor

Aren't we moving away from optional methods due to recent proposal of composite smart contracts? If that is the case then we should move the divisibility into a separate proposal in my opinion.

I'm a fan of this because it allows us to establish the simplest possible base layer for the non fungible tokens, and then move into more complex ideas via the composite smart contract

@erikzhang
Copy link
Member

Aren't we moving away from optional methods due to recent proposal of composite smart contracts? If that is the case then we should move the divisibility into a separate proposal in my opinion.

I think this can be an exception because we can easily check decimals() > 0to know whether those divisibility methods are available.

@saltyskip
Copy link
Contributor

saltyskip commented Jun 1, 2018

It seems like we must pick one or the other.

Either
A) Proposals never have optional methods, and instead each one is represented as a composite smart contract.

For example NEP-5 would only consist of required methods. IF it had optional methods then the smart contract would return the supported proposals

[5, 5.1]

This would show that the optional methods are indeed present in the contract

The alternative is B

B) Contracts have optional methods. However in my opinion this defeats the purpose of having composite smart contracts in the first place.

@mwherman2000 maybe you have some thoughts on this matter

@erikzhang
Copy link
Member

erikzhang commented Jun 1, 2018

I think contracts can have optional methods, so long as we can detect their existence in a certain way.

For NFT, we can detect it by checking decimals() > 0.

@mwherman2000
Copy link
Contributor

mwherman2000 commented Jun 1, 2018

@erikzhang I thought we just agreed that NEPs will no longer have optional operations/methods?

Discussion: #40 (comment)

Your agreement: #40 (comment)

There shouldn't be any "backdoors" such as testing for the number of decimals - else people will start creating all sorts of non-standard ways of detecting whether an optional operation/method exists ...which goes against what we've accomplished with NEP-10.

Where do we stand on this?

@mwherman2000
Copy link
Contributor

RE: 5.1

I don't think we should start using decimals in the NEP numbering scheme. I've started a specific discussion here: #50

@erikzhang
Copy link
Member

I thought we just agreed that NEPs will no longer have optional operations/methods?

Any result requires a reason. What are the reasons why we do not allow optional methods? It is because it's difficult for us to know whether these methods exist by NEP numbers. For NFT, we have a very simple way to determine. In this case, splitting it into two NEPs is a very inefficient thing and creates unnecessary troubles in the future when it is used.

@mwherman2000
Copy link
Contributor

RE: "It is because it's difficult for us to know whether these methods exist by NEP numbers."

@erikzhang What is difficult about it?

@WyattMufson
Copy link

I think the goal of having the property be tokenURI is to keep it consistent with the method from Ethereum, but to have it just be one of the properties. @erikzhang?

@WyattMufson
Copy link

Since tokenURI isn't necessarily a URL. It could be an image, data, url, etc

@corollari
Copy link
Contributor

I know, I just wanted to point out that non-official standard and have a discussion about the trade-offs. But I have no strong feelings about it so if you think that changing it is not a good idea then let's keep it the way it is.

@WyattMufson
Copy link

I don't have any particularly strong feelings. I think @erikzhang might, so I'd wait on him

@erikzhang
Copy link
Member

Both tokenURI or external_url are fine.

@WyattMufson
Copy link

@hal0x2328 made this example implementation: https://gist.github.com/hal0x2328/3237fd9f61132cea6b6dd43899947753

@erikzhang
Copy link
Member

erikzhang commented Aug 15, 2019

We also need an implementation with decimals greater than zero.

@WyattMufson
Copy link

The Ryu NFT Collectibles contracts are live on:

@Tommo-L
Copy link

Tommo-L commented Apr 9, 2020

Ready to merge? I know it's not perfect but something is better than nothing. And recently, we want to make NNS as a NFT, we hope we can complete the NFT standard.

@Tommo-L
Copy link

Tommo-L commented Apr 9, 2020

Since we already reference counter in neovm, and we have balanceOf method which return the total amount of owner's NFTs.

// Returns the total amount of NFTs owned by the specified address.
public static BigInteger balanceOf(byte[] owner)

I think we can change tokensOf to tokensPageOf, which is invoked by RPC in most cases.

public static byte[][] tokensPageOf(byte[] owner, int pageNo, int pageSize) // limit the size by a fixed value or reference counter

@Tommo-L
Copy link

Tommo-L commented Apr 9, 2020

We also need an implementation with decimals greater than zero.

Example? We can help write an NFT example in neo-devpack-dotnet.

@steven1227
Copy link
Member

Since we already reference counter in neovm, and we have balanceOf method which return the total amount of owner's NFTs.

// Returns the total amount of NFTs owned by the specified address.
public static BigInteger balanceOf(byte[] owner)

I think we can change tokensOf to tokensPageOf, which is invoked by RPC in most cases.

public static byte[][] tokensPageOf(byte[] owner, int pageNo, int pageSize) // limit the size by a fixed value or reference counter

I agree with that. The tokensPageOf method is better than the enumerators. It decreases the complexity from the sdk/client side.

@corollari
Copy link
Contributor

A counter-point to the idea of changing tokensOf is that, by applying that change, backwards-compatibility is broken with the current smart contracts that have already been deployed.

@gsmachado
Copy link

what's missing to be merged?

@shargon
Copy link
Member

shargon commented Jul 22, 2020

@gsmachado It has conflicts, and I think an implementation should be linked.

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Perhaps we should also modify here in order to explain the use of onPayment, @erikzhang @shargon @igormcoelho.

A similar strategy as #126 should be included here for managing transfer to deployed contracts.

@erikzhang
Copy link
Member

I think this will be replaced by #122.

@erikzhang
Copy link
Member

I will close this since I can't modify the branch. The new PR is in #130.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet