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

💥 ERC-2981 (NFT Royalty Standard) Implementation #120

Closed
pcaversaccio opened this issue May 30, 2023 · 10 comments · Fixed by #138
Closed

💥 ERC-2981 (NFT Royalty Standard) Implementation #120

pcaversaccio opened this issue May 30, 2023 · 10 comments · Fixed by #138
Assignees
Labels
feature 💥 New feature or request
Milestone

Comments

@pcaversaccio
Copy link
Owner

@victor-ego proposes to implement ERC-2981 (NFT royalty standard) for 🐍 snekmate's ERC721 contract.

This issue is opened to gather the discussion around whether such an implementation is of a value-add.

Personally, I have the following view:

  • EIP-2981 only defines a way to show/signal royalty information BUT does not enforce the payment actually. Thus, you're actually relying on marketplaces to voluntarily pay the royalties. Also, I got the feeling that this standard is not yet widely supported (still an open issue on OpenSea: EIP 2981 Support ProjectOpenSea/opensea-js#482). So am not sure about the overall value-add and acceptance.

Let me know your thoughts.

@pcaversaccio pcaversaccio added the feature 💥 New feature or request label May 30, 2023
@pcaversaccio pcaversaccio added this to the 0.0.2 milestone May 30, 2023
@pcaversaccio pcaversaccio self-assigned this May 30, 2023
@pcaversaccio pcaversaccio modified the milestones: 0.0.2, 0.0.3 Jun 1, 2023
@WoodsFiend
Copy link

WoodsFiend commented Jun 1, 2023

I believe that the value add is massive. The ways that NFTs can be designed and use cases drastically changes when you can customize royalty receivers, but it needs to be universally adopted to be viable.

I am working on an ERC-1155 contract where anyone can be an NFT creator and then earn royalties from their creations. With opensea this is not possible without an insurmountable amount of off-chain work to manage creators and sales on opensea. It also requires that creators trust the contract owner to pay them, which completely goes against the purpose of trustless smart contracts.

Creators should only have a trust relationship with marketplaces to pay royalties, not contract owners.

This has many business applications, my above example could be for any type of project. Where their is a single contract with hundreds or thousands of creators using it. This allows for the development of potentially massive specialized platforms built on-top of NFTs. The division of creators and contract owners is the next step to make the NFT ecosystem grow. The average creator doesn't have the time, knowledge, or money to get a custom contract designed. They need the tech side to provide smart contract platforms that they can easily use with very little knowledge.

@pcaversaccio
Copy link
Owner Author

Creators should only have a trust relationship with marketplaces to pay royalties, not contract owners.

I would even go further - the entire beauty of smart contracts should be trustlessness. Thus nobody should trust anyone by design. If it's not possible to enforce that trustlessness in the code, either the code or the standard interface is the bug and not the feature imo.

My issue is that I don't want to have dead code inside 🐍 snekmate's ERC721 contract. Also, we want max composability and smart contract creators might want other future standards to be implemented.

What I could think of is the following: We create a directory common in the token directory (similar to what OpenZeppelin is doing) and put there an ERC2981 implementation as a reference. In the code itself, we add proper comments on how integration into ERC721 and ERC1155 could look like (e.g. that you must include the ERC165 identifier and also must clear the royalty information for the token if burned). Like that we provide a reference implementation and anyone can integrate it, but the main ERC721 contract remains untouched. Thoughts?

@schuma7
Copy link

schuma7 commented Jun 2, 2023

In my opinion ERC2981 was definitely a step in the right direction and it has also found decent adoption by some of the largest marketplaces. At the same time it also falls short in certain critical areas.

As @pcaversaccio mentioned it’s only a standard for royalty signalling and as such it doesn’t actually enforce anything. It’s up to the marketplaces to support royalty payouts or not. On OpenSea the situation is even more complicated as signalled royalties are only enforced if a contract also implements the blacklisting of zero-fee marketplaces that do not respect creator royalties.

Furthermore, the deliberately simple design of ERC-2981 only allows you to define a single recipient. This limitation has prompted players like Manifold to introduce their own solutions that enable creators to split royalties between multiple recipients.

I generally think that the current solution as presented by ERC2981 is just not complete and future-proof enough to warrant an integration into the core ERC721 contract. I think providing a reference implementation for creators that want to deliberately adopt ERC2981 would be a great compromise in this case.

@WoodsFiend
Copy link

@schuma7 Its pretty easy to add a PaymentSplitter to 2981 to allow multiple recipients. I agree it shouldn't be in the core ERC721 contract though. Developers need the choice to implement it, but marketplaces should abide by it.

@pcaversaccio I wouldn't be concerned about ERC2981 being dead code, there are many marketplaces that do implement it already such as Rarible. Since you can't control which marketplace a user will use it is a good net to catch those that do support it. My plan is to implement it and set the royalties for 2981 at 5% and all other unsupported marketplaces at 10% in an attempt to dissuade selling on marketplace like opensea. On top of that I plan to use the non-2981 compliant royalties to fund a DAO since it would be a royal pain to try to distribute the royalties to the correct creators.

The entire intention of my plan is to make owners and creators dislike marketplaces that don't integrate EIP-2981 and put pressure on them to do so.

@pcaversaccio
Copy link
Owner Author

pcaversaccio commented Jun 3, 2023

@pcaversaccio I wouldn't be concerned about ERC2981 being dead code, there are many marketplaces that do implement it already such as Rarible.

When I referred to dead code, I meant that being part of my ERC721 contract it's most probably going to be dead code. Vyper doesn't allow for inheritance and will allow for library modules (most probably) starting with Vyper 0.4.0. That being said, my current ERC721 contract entails mostly a feature-complete set of extensions that the majority use, and thus people can simply copy+paste the contract for deployment (or you simply pip install my contracts directly locally 😃). If ERC2981 is included in the core contract, my concern is that many folks don't need it and must refactor them themselves. Thus, my suggestion about the reference implementation which is not part of the core contract. This is also in line with my long-term vision (and was my underlying motivation to build 🐍 snekmate) of having a good extension system with feature-complete contracts that can be imported once libraries are live. Like that, we can achieve max composability while retaining ergonomic separation (IMHO) of different contracts.

@victor-ego
Copy link

Consider a scenario where renowned street artist Banksy decides to release an NFT collection on a marketplace that supports royalty payments. However, platforms like OpenSea, which are currently not enforcing royalties (even though they did a few months ago when testing this ERC721 experiment!), will be unable to facilitate the buying/selling of these NFTs. This is due to the integrated royalty enforcement mechanism in the ERC721 contract of Banksy's NFTs, making royalty payments mandatory.

One might speculate if OpenSea would risk missing out on the trading volume of such high-profile collections due to their non-compliance with royalty payments. The answer, in my opinion, highlights the importance of taking a stand to push marketplaces towards respecting artist royalties.

As artists and developers, while we're yet to claim victory in enforcing royalty payments across all marketplaces, one proactive step could be the creation of a common directory. This initiative could provide an alternative approach to enforce royalty payments, supporting artists in monetizing their work.

Working alongside @fubuloubu, we're in the process of integrating these royalty functionalities into the contracts. We aim to build a trustless system that doesn't rely solely on the goodwill of marketplaces to apply a suggested 10% royalty, but rather enforces it on a contractual level. This represents a significant step towards a more equitable NFT ecosystem, where artists' rights to residual earnings are universally recognized and respected.

A current summary of progress made so far:

  1. Royalty Amount: The contract contains a variable minRoyaltyAmount that stores a fixed minimum royalty amount to be paid in each transaction.

  2. Floor Price Oracle: A mechanism has been created to update this minRoyaltyAmount based on previous transactions. If the contract receives an amount greater than minRoyaltyAmount + threshold, the minRoyaltyAmount is increased by a certain proportion. If the amount received is less than minRoyaltyAmount + threshold, then minRoyaltyAmount is decreased by a certain proportion. This creates a sort of oracle to adjust the floor price based on recent transaction activity.

  3. Balance Tracking: The contract keeps track of the balance after each transaction using lastBalance. This is used in the aforementioned oracle to measure the amount sent in the last transaction.

  4. Royalty Enforcing: The contract has a mechanism to enforce royalty payments. Before each transaction, it checks if the contract balance has increased by at least minRoyaltyAmount since the last transaction. If not, the transaction is reverted.

  5. Withdrawal Function: There's a function to allow the contract owner to withdraw their funds from the contract.

  6. ERC721 Compliance: The contract remains compliant with the ERC721 standard, while adding on these additional features.

  7. RoyaltyInfo Function: This function returns the address of the recipient of the royalty and the calculated royalty amount for a given sale price.

  8. ERC2981 Interface Checking: The contract includes a function to check if a given contract supports the ERC2981 royalty standard.

These features collectively ensure that the contract enforces the payment of royalties on each transaction and that this royalty amount adapts to the transaction history of the contract, providing a form of floor price oracle.

I´m sure there are many things not taken into account of that could be more of a bug than a feature. Will love to hear your thoughts and collaborate together.

@pcaversaccio
Copy link
Owner Author

pcaversaccio commented Jun 6, 2023

@victor-ego thanks for sharing this information. I think what you outline is a nice approach; some feedback:

  • To increase the acceptance of such an approach, I recommend the drafting of an EIP. It is an unfortunate fact that EIPs nowadays serve as a marketing tool that influences acceptance;
  • The price oracle can be easily manipulated if this is desired;
  • "royalties on each transaction" -> I don't think this is the way to go; I would rather see a design where NFTs implement a dedicated buy/sell function that is used by marketplaces to enforce the royalties rather than abusing transferFrom and safeTransferFrom. We need to distinguish the on-chain semantics of a transfer and that's the fundamental issue with on-chain paid royalties;
  • Let's say I have an upgradable contract setup: I could easily overwrite the logic whenever I want. Think about the scenario where you enforce royalty payments as you mentioned; what could happen is that people upgrade the logic for non-enforcement for eg OTC deals and re-implement the previous logic later to be allowed to get listed again;
  • Overall there are multiple attack vectors that would allow to abuse the contract logic imho;

So coming back to the initial topic of this issue: I will release 🐍 snekmate 0.0.2 tomorrow and thereafter we can start working on the ERC2981 implementation. After careful thinking I believe it's better to place the ERC2981 contract in the extensions folder. @victor-ego would you like to collaborate on this contract with me?

@victor-ego
Copy link

@pcaversaccio , I appreciate your insightful feedback and suggestions on our royalty implementation approach.

  1. I completely agree with you regarding drafting an EIP. Although it might have transformed into a marketing tool, it is still the most recognized method to suggest protocol improvements.

  2. On the point of price oracle manipulation, good point. The aim is to maintain the integrity of the price feeds while ensuring they remain beneficial for the artists.

  3. As for the suggestion to introduce a dedicated buy/sell function instead of relying on transferFrom and safeTransferFrom, it's an interesting viewpoint. I concur that this could help delineate the semantics of a transfer better. The broader implication of this change, however, would be a shift in the current paradigm. This could potentially require significant updates from the marketplaces, but the merits certainly make it a worthwhile discussion.

  4. Regarding the upgradable contract setup, it is indeed a valid concern. It's a delicate balance between maintaining the necessary flexibility for contract owners and ensuring the royalty mechanisms aren't bypassed.

  5. I appreciate your recognition of potential attack vectors, and that's precisely why having conversations like these is crucial. We should aim to develop a robust solution that not only benefits artists but also ensures security and integrity.

As for 🐍 snekmate 0.0.2, I'm excited to see its release and the subsequent start on the ERC2981 implementation. I think positioning the ERC2981 contract in the extensions folder is a good idea.

Lastly, I would be absolutely thrilled to collaborate with you on this contract! Let's work together to create a robust, equitable, and universally accepted royalty standard for the NFT ecosystem. 🫡

@pcaversaccio
Copy link
Owner Author

5. We should aim to develop a robust solution that not only benefits artists but also ensures security and integrity.

100%

Lastly, I would be absolutely thrilled to collaborate with you on this contract! Let's work together to create a robust, equitable, and universally accepted royalty standard for the NFT ecosystem. 🫡

Awesome :-D - will reach out on Discord for the next steps. Anyone is invited to continue to share his/her/their thoughts about what can improved wrt on-chain royalties!

@pcaversaccio
Copy link
Owner Author

FYI, I just released 🐍 snekmate 0.0.2.

@pcaversaccio pcaversaccio changed the title 💥 ERC-2981 (NFT Royalty Standard) Implementation 💥 ERC-2981 (NFT Royalty Standard) Implementation Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 💥 New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants