-
Notifications
You must be signed in to change notification settings - Fork 61
Add ERC-2981 NFT Royalty Standard implementation #126
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
Add ERC-2981 NFT Royalty Standard implementation #126
Conversation
Coverage Report
Last updated: Mon, 27 Oct 2025 05:53:40 GMT for commit |
Implements the ERC-2981 standard for NFT royalty information using Compose's diamond storage pattern. This implementation provides: - ERC2981Facet with royaltyInfo() query function - LibERC2981 library with internal setter functions for royalty configuration - Support for both default and per-token royalty settings - IERC2981 interface with custom errors Note: External setter functions are intentionally omitted from the facet to allow maintainers to decide on the appropriate access control pattern (e.g., owner-only, role-based, or public with token ownership checks). Tests will be added once the final contract design and access control approach are confirmed.
4deed08 to
52c9f4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Good job.
Can you please rename the files and facet and library name, etc. Replace ERC2981 in the names of things to Royalty. For example instead of LibERC2981 change it to LibRoyalty. Change ERC2981Facet to RoyaltyFacet. But continue using the standard errors with their current names.
Also can you please put these files in the token folder, and take the Lib file out of the libraries file and delete the libraries file.
Definitely have it in the comments that this is an implementation of ERC-2981.
Introduce LibRoyalty library and RoyaltyFacet contract to implement the ERC-2981 NFT royalty standard.
|
@mudgen @panditdhamdhere Done! I've made all the requested changes: Renaming
File Structure
Documentation
All logic remains 100% identical - only names and file locations changed. |
maxnorm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good. Thanks for the great work @aapsi !
For your previous question, you can leave it like that. Access control isn't define by Compose. Doing that will break our goal of reusable composition
For any custom logic/restriction, the user of Compose will create it's own facet using the core function of the lib while wrapping it with their custom needs.
The diamond will look like this:
royaltyInfo() -> RoyaltyFacet
setDefaultRoyalty() -> CustomFacet
By using the lib in their custom facets, the users are playing with the same storage location as the RoyaltyFacet from Compose
|
@maxnorm Thank you for the response!! This solidifies my understanding of the Compose and it's goal. |
|
You can also refer to the new section Understanding Facets and Libraries in the README that explain it in more details Thanks to you for your great contribution! |
…entation Add ERC-2981 NFT Royalty Standard implementation
Add ERC-2981 NFT Royalty Standard Implementation
Summary
This PR implements the ERC-2981 NFT Royalty Standard using Compose's diamond storage pattern. The implementation provides a read-only facet with the standard
royaltyInfo()function, along with a comprehensive library containing setter functions for configuring royalties.What's Included
royaltyInfo(uint256 _tokenId, uint256 _salePrice)query functionsetDefaultRoyalty()- Set default royalty for all tokensdeleteDefaultRoyalty()- Remove default royalty configurationsetTokenRoyalty()- Set per-token royalty (overrides default)resetTokenRoyalty()- Reset token to use default royaltyImplementation Details
✅ Follows Compose Conventions:
keccak256("compose.erc2981")✅ ERC-2981 Compliant:
royaltyInfo()function(address receiver, uint256 royaltyAmount)as specifiedQuestion for Maintainers ( @mudgen @maxnorm @panditdhamdhere @Haroldwonder ): Access Control Pattern
The library provides internal setter functions, but the facet intentionally does not expose these as external functions yet. This allows the maintainers to decide on the appropriate access control pattern.
the relavant issue is : Add ERC-2981 royalties facet #42