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

ERC1155Supply, an extension that keep track of totalSupply for ERC1155 tokens #2536

Closed
Amxx opened this issue Feb 24, 2021 · 8 comments
Closed
Labels
contracts Smart contract code. idea

Comments

@Amxx
Copy link
Collaborator

Amxx commented Feb 24, 2021

🧐 Motivation
ERC1155 supports a middle-ground between ERC20 (fungible) and ERC721 (unfungible) in which token are fungible within the same tokenId. Depending on the sum of all balances for a tokenId, the corresponding token can be seen as fungible or nonfungible. However, there is no on-onchain mechnism to keep track of this.

📝 Details
A totalSupply(uint256 tokenId), similar to ERC20's totalSupply, but with a "per tokenid" result be provided. This would be usefull to distinguish between fongible and nonfongible assets. It also adds a exists(uint256 tokenId) function that returns weither the totalSupply is not null. This would be equivalent to checking weither the corresponding token exists.

@Amxx Amxx added contracts Smart contract code. idea labels Feb 24, 2021
@frangio
Copy link
Contributor

frangio commented Feb 24, 2021

This is similar to #2185, except that PR also offered a way to register a token id as existent even if it had no supply. That PR was motivated by the fact that OpenSea seems to support an exists(uint tokenId) function, so I feel that we should do that here as well. It helps to have that concrete motivation to add this functionality.

@Amxx
Copy link
Collaborator Author

Amxx commented Feb 25, 2021

My proposal includes a exists(uint256 tokenId) function (I edited it to be more explicit). I did not, however, imagined having an additional uint256 → bool mapping that would overload the exist function.

@frangio
Copy link
Contributor

frangio commented Feb 25, 2021

Yeah, I don't think the mapping is necessary, it adds to storage costs and it could be implemented by users by overriding exists.

@KaiRo-at
Copy link
Contributor

KaiRo-at commented Apr 2, 2021

A totalSupply(uint256 tokenId), similar to ERC20's totalSupply, but with a "per tokenid" result be provided.

In our Crypto stamp presale token contracts, we have an assetSupply(tokenId) for that, and a totalSupply() that gives the total amount of all tokens in the whole contract. The latter is interesting for some use cases (like ours), but could be problematic in gas usage in some cases as well. Just wanted to mention it as food for thought.

@Amxx
Copy link
Collaborator Author

Amxx commented Apr 2, 2021

well, the beauty of function overloading and hooks makes it that you could, on the same contract, have a totalSupply(uint256) and a totalSupply(). The first one would return the supply of a particular tokenid, the second one for all token ids.

I really think we should not include this second option in our codebase (as you said it yourself, it only apply to some specific usecases). But I'm confident our extension (which is optional) would not break your interface.

@SaulMoonves
Copy link

The user could mistakenly or purposefully do two large-quantity mints of the same fungible ID to different addresses and have the derived total supply exceed what can be held in a uint256. You would not notice until one of the recipients did a transfer to the other and the quantity overflowed. I quickly looked at the standard and maybe I missed it, but I didn't see anything that said you couldn't mint more of an existing id, which would prevent this undesirable scenario.

@frangio
Copy link
Contributor

frangio commented Jul 13, 2021

We forgot to close this issue which was implemented in #2697.


@SaulMoonves The scenario you describe seems true, but is a problem only for vanilla ERC1155, and in fact ERC1155Supply would prevent this scenario as the mint would overflow directly. I'd suggest raising this problem over at ethereum/EIPs#1155.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contracts Smart contract code. idea
Projects
None yet
Development

No branches or pull requests

5 participants
@frangio @KaiRo-at @Amxx @SaulMoonves and others