EIPs

EIPs copied to clipboard

---

eip: 4910 title: Royalty Bearing NFTs description: Extension of the ERC-721 standard for NFTs to correctly define, process, and pay (hierarchical) onchain royalties from NFT sales, going beyond EIP-2981. author: Andreas Freund (@Therecanbeonlyone1969) discussions-to: https://ethereum-magicians.org/t/royalty-bearing-nfts/8453 status: Draft type: Standards Track category: ERC created: 2022-03-14 requires: 721, 165, 2771

---

A critical exception has occurred: Message: pr 4910 is already merged; quitting (cc @alita-moore, @mryalamanchi)

@MicahZoltu implemented all required changes.

@MicahZoltu ... a gentle reminder to merge if it looks ok for you. Thank you!

@lightclient @MicahZoltu ... I cannot add the reference implementation folder under assets/eip-4910. I am getting this error below when I try to push the commit.

error: invalid path 'assets/eip-4910/eip-4910-reference-implementation/'
error: assets/eip-4910/eip-4910-reference-implementation/: cannot add to the index - missing --add option?
fatal: Unable to process path assets/eip-4910/eip-4910-reference-implementation/

Looks like the github action triggered should have the -- add option in the batch script to allow for subfolders to be added. Could that be added?

Of course, I could just add the test javascript files and the blank contracts under assets/eip-4910 but that does not make it easy for other implementers to take the code and the tests and run them easily, as they should be able to do.

Would appreciate a suggestion as to how the above error can be overcome. Thank you!

Your local git client is giving you that error, or CI is giving you that error? There shouldn't be any restrictions on adding subdirectories to the repository, and other users have added them in the past which makes me think this is a local git issue you are running into?

Your local git client is giving you that error, or CI is giving you that error? There shouldn't be any restrictions on adding subdirectories to the repository, and other users have added them in the past which makes me think this is a local git issue you are running into?

I was stupid indeed ... I had forgotten that I still had another .git folder in my folder ... duh ... now it worked. It was indeed a local git issue. @MicahZoltu

@MicahZoltu @lightclient ... just a gentle reminder as to the above. Have your concerns been addressed?

I don't handle ERCs, you'll have to wait for one of the other editors who does handle them to get a chance to take a look. I was just providing some early feedback to help speed things along is all.

I don't handle ERCs, you'll have to wait for one of the other editors who does handle them to get a chance to take a look. I was just providing some early feedback to help speed things along is all.

Thank you so much @MicahZoltu ... greatly appreciated!

I don't handle ERCs, you'll have to wait for one of the other editors who does handle them to get a chance to take a look. I was just providing some early feedback to help speed things along is all.

Thanks @MicahZoltu. Keen to see which Editor(s) tap in. Lots of excitement building around this.

Two overarching comments:

1. The reference implementation is huge, and appears to include a whole build system. Ideally the reference implementation would just be one or maybe two files, and be trimmed down to the absolute minimum required to show how the standard generally works. The reference implementation doesn't have to be production ready, meaning you can skip doing necessary security checks (though I recommend leaving some comments around indicating where security checks would be recommended and care should be taken). For example, you may leave out any reentrancy protection even though a production implementation would want that but it makes the code smaller and easier to read. For dependencies (like ERC-721) you don't need to include an implementation, you can just extend from it and leave it an exercise for the reader to find an ERC-721 implementation.
2. This EIP is enormous. This makes it both very difficult to get reviews on, and thus difficult to get through the standardization process successfully, and it tends to hurt adoption because complicated standards are harder to adopt. Can this EIP be split into smaller EIPs that each specify a single useful stand-alone feature instead?

1. For dependencies (like ERC-721) you don't need to include an implementation, you can just extend from it and leave it an exercise for the reader to find an ERC-721 implementation.

@MicahZoltu I do not understand what you mean here. Can you give an example?

2. This EIP is enormous. This makes it both very difficult to get reviews on, and thus difficult to get through the standardization process successfully, and it tends to hurt adoption because complicated standards are harder to adopt. Can this EIP be split into smaller EIPs that each specify a single useful stand-alone feature instead?

@MicahZoltu Royalties are not only a very important but also a very complex subject. I can remove all the optional features such as delegation and updating of parameters etc. and simply mention it as a sentence to make it smaller. Otherwise, breaking the standard into several pieces does not really make sense since data structures, royalty account structures, royalty payouts, NFT creation, listing/unlisting, and buying are connected. It is virtually impossible to treat them separately. And if you did treat them separately, there would be a lot of dependencies between the different EIPs, making it more confusing than helpful. IMHO.

@MicahZoltu I do not understand what you mean here. Can you give an example?

contract EIP4910 extends EIP721 {
	...
}

And just leave EIP721 undefined in the reference implementation (users can go look at EIP721 if they want, or they can go online and find an implementation of it from a source they trust if they like).

@SamWilsn ... changes as per our discussion. Let me know if this is sufficient. Also, please, discuss with @MicahZoltu what we discussed about the reference implementation.

@MicahZoltu @lightclient @SamWilsn any updates on the PR? Has been 4 months. I have not seen any more editorial comments. The only open issue is the reference implementation. A suggestion as to that topic: If you allowed external links in optional sections such as for a reference implementation, I could just add the GitHub link to the reference implementation and remove all the rest. That might be the easiest interim step until you have finalized the licensing discussion. Just a thought.

Your reference implementation and test cases are... too complete.

@Pandapip1 how can that be?

You have a complete project, with tooling and such. Reference implementations and test cases should be as simple as possible.

You have a complete project, with tooling and such. Reference implementations and test cases should be as simple as possible.

@Pandapip1 In W3C, DIF, OASIS, EEA, and other standards bodies, the more complete and easy to use the reference implementation of a standard is, including as complete a test coverage as possible, the easier for users to adopt it and ensure not to break existing functionality through modifications. And ease of use and making it easy to adopt is the goal, no?

Yes, but having full tooling and using libraries is too complex. The implementations and tests should be as simple (no tooling, no libraries, no package files, etc...) as possible.

Yes, but having full tooling and using libraries is too complex. The implementations and tests should be as simple (no tooling, no libraries, no package files, etc...) as possible.

@Pandapip1 I am sorry, I am daft, but why is that too complex? What is the benefit of not having an OOB reference implementation with full test harness that is easy to run?

The commit cb0d7ff3c9775a1c11aed7bce65f640589ec41d4 (as a parent of eacf81332e7af851402885069dd84a98ab70c5b7) contains errors. Please inspect the Run Summary for details.

I put up a PR with some fixes for the new build system: https://github.com/Therecanbeonlyone1969/EIPs/pull/1

I put up a PR with some fixes for the new build system: Therecanbeonlyone1969#1

@SamWilsn thank you! What is the status of the discussion on including the reference implementation using other licenses than CC0 for the reference implementation (note, the standard itself of course which should be CC), of course)? Can we simplify this somehow? At this point, I am willing to get remove it to get the draft merged and continue the process. We have been on this for over 5 months which is getting borderline, especially, since we are down to discussing the reference implementation.

@MicahZoltu @SamWilsn @Pandapip1 may I have an update, please?