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

Introduce BurnableERC721Token contract #1126

Closed
1 task done
dougiebuckets opened this issue Jul 28, 2018 · 4 comments
Closed
1 task done

Introduce BurnableERC721Token contract #1126

dougiebuckets opened this issue Jul 28, 2018 · 4 comments
Labels
contracts Smart contract code. feature New contracts, functions, or helpers.

Comments

@dougiebuckets
Copy link
Contributor

🎉 Description

Does it make sense for us to introduce a BurnableERC721Token Contract similar to the BurnableToken contract we have for ERC20s? For ERC20 we simply subtract tokens from the balances mapping in StandardToken whereas for the proposed BurnableERC721Token contract we would need to ensure that the specific token ID or index is eliminated. Pulling from this article the steps might look like:

  • First, we clearApproval(), then remove the token from ownership via removeTokenFrom() and use the Transfer event to alert this change on the front end.

  • Next, we eliminate the metadata associated with that token by deleting what is mapped to that particular token index.

  • Lastly, much like removing a token from ownership, we rearrange our allTokens array so that we replace the _tokenId index with the last token in the array.

  • 📈 This is a feature request.

@nventuro nventuro added feature New contracts, functions, or helpers. contracts Smart contract code. labels Aug 8, 2018
@nventuro
Copy link
Contributor

nventuro commented Aug 8, 2018

I think this makes sense, and is something we might want to include for consistency, but would like to first hear from our resident NFT expert @shrugs.

@shrugs
Copy link
Contributor

shrugs commented Aug 8, 2018

We need it, yes!

We have an implementation stuck in #957 which has unfortunately been put on hold because of a wide variety of blockers that will eventually be resolved, but if you'd like to cherry-pick the burnable parts of that PR into this one (keeping Vittorio's commit authorship), that would be ❤️

@Aniket-Engg
Copy link
Contributor

I think we have a similar contract now in 2.0, so this can be closed.

@frangio
Copy link
Contributor

frangio commented Apr 13, 2021

We have ERC721Burnable now.

@frangio frangio closed this as completed Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contracts Smart contract code. feature New contracts, functions, or helpers.
Projects
None yet
Development

No branches or pull requests

5 participants