Hey there, thanks for sharing your findings! I’ll address some of your points.
Some of its ideas are helpful, but as-is the specification is unusable
I have to disagree here, dozens of collections are already using this for a wide variety of applications, including art, gaming and reputation.
It doesn’t actually allow token bundling.
A bundle is “a group of things fastened together for convenient handling”. By nesting tokens into another you are effectively creating a bundle. If you look at the proposed implementation and tests, it can be done through nestTransferFrom
or with an implemention of nestMint
, we provide the internal method. Parents and children must implement 7401, so you cannot do it with a BAYC unless you wrap it.
Inconsistent naming
Direct owner can be an EOA or an NFT. Parent is only an NFT.
EOA, smart contract owners and ERC721Receiver
are used incorrectly
Not sure I understand the problem you are pointing out. The sample implementation does this check only on sending. If you want to have an implementation which is also allowed to receive NFTs to no particular token, you can add the IERC721Receiver interface and that’s it. I’ve actually done it a few times. What we do is to actually check when sending a token to another one via transfer or mint, that the destination contract implements IERC7401, otherwise it would get trapped.
Does not implement ERC-721
- In the interface definition, it is missing
is ERC721
.
If I remember correctly, we did this on purpose to avoid linearization issues.
- The
balanceOf
function does not satisfy requriments of ERC-721.
We added this clarification on 7401, it was missing on 6059.
- The
transferFrom
function and Transfer
event do not satisfy requirements of ERC-721.
If you are referring to emitting for children when a parent changes ownership, this is absolutely impractical and expensive. This is the same reason why we have a different behavior of balanceOf
when tokens are nested.
- The transfers do not support payable transfers.
You are right, we missed this.
Accepted and rejected children is unnecessary
We simply disagree here, by design this was an important part of the standard for us proposers. It can easily be bypassed by using a hook after a child is added, but we wanted to have this mechanism by default as it proved to be very useful on the initial protocol implementation on Kusama, which ran for about 2 years before we took it to EVM world.
Tokens can get stuck
If a token is nested too deeply, they can get stuck
In our test implemetation we have very simple way to prevent this. We check for inheritance loops up to a certain number of levels. If there are too many nesting levels we simply revert.
Burning is out of scope
Besides the reasoning of burning being useless, which I don’t really agree as it can be used for different mechanics, it was our intention initially not to have it, but it presented a problem. If you wanted to burn a token with children, we considered the right thing to do to also burn recursively its children. This was not possible if some of them could be not burnable, so we opted to include it. I’m not fan of having it here but I think it was the lesser evil.
Undefined behavior
I agree here in most points, we could have been clearer.
There is no access control
I agree here, we probably felt it was obvious, but we should have clarified in some of the methods, that they should only be done by an approved party or the token owner.
Reordering attacks with indexes
We don’t expect the acceptChild to be called by anybody but token owner or approved, another case were we just assumed it but would have been much better to clarify. That solves the reordering attack.
Inaccurate statements
You are right, it is not an accurate statement as there is an alternative, however it adds cost to every transfer so we would not include it here. This was another hard choice but requiring child indices in some operations was the cheapest solution in terms of gas, so we went for it.
Comparison to existing
If you mention another EIP in your proposal, you cannot finalize it until the other one does, so mentioning it was not an option, at that moment ERC-998 was stagnant even. A quick mention would be that ERC998 splits ability to own and ability to be owned into 2 interfaces. With our proposal there’s no need for this. Furthermore, while developing this, we found that unless the parent-child relationships are done in a parent-governed style, there are security issues which an evil intended child could do, a failure that 998 probably has.
Does not allow selling batch tokens
It can be achieved by either escrowing the parent token, or by storing the expected children on listing. We clarify this in the security considerations.
Too complicated
Unless you provide a sample implementation with good test coverage of this interface you propose, I will be doubtful that this can be achieved in a simpler way. We spent about a year going back and forth with the implications of every step, and then finally we stripped all the non absolutely required methods.
If you still have some interest I invite you to take a look at our implementation which addresses most of your concerns since it implements reasonable gating and checks. Here is an NPM package which contains core, non-opinionated, implementations and also ready to use implementations for this and other EIPs. The package is open source, here is the core implementation of this ERC. We also have a convenient wizard here, OZ style, to easily customize your implementations/
Finally, there are over a hundred collections deployed in our marketplace which are taking advantage of this and other standards we’ve proposed, currently we support Ethereum, Polygon, Base, Moonbeam and Astar. I invite you to take a look and reconsider your position after these clarifications.