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

Implementing EIP-2981 (ERC721 Royalties) #2789

Closed
htadashi opened this issue Jul 26, 2021 · 13 comments · Fixed by #3012
Closed

Implementing EIP-2981 (ERC721 Royalties) #2789

htadashi opened this issue Jul 26, 2021 · 13 comments · Fixed by #3012

Comments

@htadashi
Copy link
Contributor

htadashi commented Jul 26, 2021

🧐 Motivation

Recently, EIP-2981 went to Final Status. It proposes a "standardized way to retrieve royalty payment information for non-fungible tokens (NFTs) to enable universal support for royalty payments across all NFT marketplaces and ecosystem participants".

While it is possible to implement your own royalty system overriding ERC721/ERC1155's transferFrom functions, I was wondering if it would make sense to support EIP-2981 as an alternative to minimize gas usage in deployment. Plus, EIP-2981 would better support optional royalty payments.

📝 Details

If it makes sense to add EIP-2981 support to OpenZeppelin contracts, I think it could be a nice idea to make an interface ERC721HasRoyalties.sol and IERC1155HasRoyalties.sol to extend ERC721 and ERC1155 contracts.

(P.S.: Since it has been recently finalized, I don't think many marketplaces are adopting EIP-2981 for now, but if this feature makes sense, I can keep track of it 😃)

@shrugs
Copy link
Contributor

shrugs commented Jul 26, 2021

Looking forward to this!

@frangio
Copy link
Contributor

frangio commented Jul 26, 2021

We're interested in this EIP. Has there been support from marketplaces?

Can you elaborate what this EIP would look like as a part of OpenZeppelin Contracts? You mentioned interfaces but I imagine it would also be useful to have an implementation.

@arpu
Copy link

arpu commented Jul 26, 2021

from @benber86 is https://github.com/benber86/nft_royalties_market

@frangio frangio changed the title Implementing EIP-2981 Implementing EIP-2981 (ERC721 Royalties) Jul 28, 2021
@hack3r-0m
Copy link

I am interested in working on this issue since I have been following that EIP closely.

@captainkidd5
Copy link

Would also be very interested in seeing this gain some traction

@frangio
Copy link
Contributor

frangio commented Aug 20, 2021

Are "mainstream" marketplaces planning to adopt this feature?

@hack3r-0m Do you have an idea of what concretely would be added in OpenZeppelin Contracts? What contracts or libraries and what would they look like.

@blmalone
Copy link

blmalone commented Aug 20, 2021

Hi,

This list isn't exhaustive but it's growing.

In addition to this, we've got lots of individual creators basing their royalty information on EIP-2981.

I'm also currently in talks with a few larger marketplaces who're very excited about supporting this. Nifty Gateway being one of these parties.

Implementing EIP-2981 is relatively low effort. You'd want to decide if:

  • You want contract level (global per ERC-721/1155 smart contract) royalty information or
  • Token level royalty information (each token has it's own individual royalty info)
    These are the two generic approaches.

Please see this reference implementation:
https://github.com/dievardump/EIP2981-implementation

Also here's a video that describes the rational:
https://www.youtube.com/watch?v=hTbcw0rhLto (thought I'm sure everyone here already knows why we need this standard)

Happy to answer any questions wrt to implementation. Just let me know.

@htadashi
Copy link
Contributor Author

@frangio Sorry for the late reply - I was waiting to see how marketplaces were going to react. There is a request on OpenSea, but as far I know, they didn't signalled yet support. Maybe @blmalone could convince them to adopt it :)

Regarding what I mentioned about the interface, it was just a vague idea. I was imagining something that would work with a press of a button in OpenZeppelin's contract wizard. @blmalone suggested implementation could be a good basis for that.

@frangio
Copy link
Contributor

frangio commented Sep 15, 2021

Yeah putting royalty info in Contracts Wizard would be nice.

We try to avoid using storage variable if they're not necessary (which is why we have ERC721URIStorage as separate opt in feature). Do you imagine that changing royalty info after deployment would be needed often?

@htadashi
Copy link
Contributor Author

Thanks for the explanation regarding ERC721Storage. I see how that would help in economizing gas costs ⛽

I'd expect that most NFT owners wouldn't change this info after deployment. I did a non-exhaustive search on this contract database and it seems that many contracts expose an external setRoyaltyInfo function for changing the royalty. On the other hand, most of these contracts that are deployed on mainnet seem to call this function just at the start (examples: 1, 2).

@robinportigliatti
Copy link

Hello @JulissaDantes,

Thanks for this implementation, since I was doing it myself for a project... I hope it will be available soon.

@robinportigliatti
Copy link

Hello @JulissaDantes,

Have you thought about making ERC721Royalty.sol Enumerable ? Or maybe I am missing something, another implementation I missed.

Robin,

@Amxx
Copy link
Collaborator

Amxx commented Dec 20, 2021

@robinportigliatti What do you mean by making ERC721Royalty enumerable ?

ERC721Royalty is an ERC721 extension, just like ERC721Enumerable, ERC721Pausable or ERC721Votes. Extensions can be combined, so you can easily inherit all 4 of these extensions if you want an ERC721 contract that is pausable, enumerable, that has the royalty mechanism and that can be used for voting.

While users can opt-in, we don't want to force that. If someone wants ERC721Royalty but doesn't want to bear the high cost of enumerability, we shouldn't force that onto them.

If you want to know more about building a token contract with multiple modules, check out the wizard. Hopefully, it will soon include a toggle for royalties !

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

Successfully merging a pull request may close this issue.

9 participants