Thank you for the introduction @SamWilsn!
Nice to meet you @simonDos, we are looking forward to your feedback! 
Thanks for this proposal, I’ve been working with NFTs owned by NFTs in a project of my own recently, and I came across this while researching what other people are doing in this area. I’ve got a few thoughts on EIP 6059.
Firstly, just a typo, this should be “supply chains”?
One concrete example of this can be drawn from supply trains use case.
I like the general principle that an NFT from one contract can be the owner of an NFT from a different contract, so I’m pleased to see this spec using that approach. In contrast, EIP-6150 seems to expect that relationships only occur within the same contract, and that limits composability between contracts unaware of each other.
I feel like EIP-6059 is on the right track, but has some implicit assumptions about the type of nesting being modelled by the spec. I think you could get to a more general spec, which is also simpler.
A lot of the spec is describing how to model an ordered list of child NFTs owned by a parent, and a way to add children to the list using a two-step propose-and-accept method, with a secondary list of pending children.
I feel that this part of the spec is describing one approach to solve a general problem. My own use case doesn’t map well to an ordered list of children, as my children are more like object properties with distinct and clear semantic relationships to the parent. Representing them as a list would introduce the problem of validating that the cardinality of the relationships is correct, and of searching the list to find the child of a particular type. The propose-approve addition model is not something I need, so supporting it would feel unnatural.
I feel that a lot of the spec deals with how to create and modify hierarchies of NFTs. Construction of the hierarchy is something that is fairly application-specific, as every application will have their own requirements for what combinations of things are valid and what permissions are required.
For me, there’s more benefit in standardising how to read/interpret a hierarchy than write a hierarchy, as reading can be done by any number of parties, and these parties need standardisation to agree on how to interpret the hierarchy. Whereas writing is a limited-scope operation by a specific party, and the details of how something was created don’t matter for others, so long as they can interpret the result. Standardising writing imposes constraints on how an application can work, whereas standardising reading enables applications to interoperate in ways they otherwise couldn’t.
I think there’s potential to make NFTs more composable, extensible and enable emergent behaviour using NFTs owned by NFTs.
For example, eip-4907 provides a way for NFTs to be rented to 3rd parties, without giving the 3rd party the right to transfer the NFT. But it expects that an NFT implement the spec. i.e. as well as modelling their own domain, it expects an NFT contract to also model the rental domain, which feels like an unnecessary merging of separate concerns, and limits compatibility with existing contracts that are unaware of the spec. This also doesn’t scale — we can’t expect every NFT contract to be re-written to implement a new interface in order to work with new standards (especially considering the immutable, single-identity nature of NFTs).
The same could be achieved without needing the rented NFT to know about the rental spec by using a wrapper rental NFT contract that owns the borrowed NFTs, and transferring ownership of the rental wrapper to the borrower. This allows the rental wrapper to implement arbitrary conditions on the rental (rather than the fixed time duration model used by the 4907). e.g. the wrapper could remain active as long as a superfluid stream paying into it remained solvent. The wrapper would be a proper bearer asset that can be re-sold by the borrower. And the rights of the owner and borrower are protected by the logic of the wrapper rental contract, such that the owner can’t rug-pull the renter by selling the rented NFT, and the borrower can’t transfer the rented NFT.
What’s needed is a standard way to represent semantic information about the relationship between the parent and child NFTs. (e.g. in this example, so that 3rd parties can understand that ownership of the rental NFT implies the owner has this rental agreement with the wrapped NFT.) And more generally, an NFT may not own another NFT but could still express information about it. This feels very much like the problem that Linked Data and the Semantic Web tries to solve — it wouldn’t be practical to create an EIP and specific interface for every domain in which an NFT can express a relationship with another, but it would be practical standardise a way to express such relationships generically, a bit like how EIP-165 or ERC-1820 generically define contract interface compatibility.
Sorry if this is a bit abstract, but I can point to some concrete things for this EIP.
The way ownerOf() is specified breaks compatibility with 721. In my opinion, it should report the contract address that owns the NFT, not the account that owns it transitively. Instead of changing the semantics of ownerOf() the spec should define a new function should for the indirect owner.
However, I’m not sure that such a function is needed. Because 721 already defines ownerOf() which can point to the owning contract, the only thing missing is a standard way to ask the owning contract which of its tokens owns the child NFT.
Similarly, 721’s Transfer event already communicates the transfer of ownership to the parent NFT contract, but an event listener needs to know which tokenId is the owner. Rather than having the child emit a new NestTransfer event, the parent contract could emit a Receive event, containing the tokenId that now owns the child.
This approach would allow regular 721 contracts to interoperate, without needing them to know that their owner is an NFT.
The spec mentions INestable and nestMint which are not defined in the spec.
On the security side of things, an issue I’ve considered in my own nested ownership situation is with selling the nested NFTs, or more generally, allowing other contracts to use them safely. The problem is that if a hierarchy can be modified by its current owner, the owner could list the NFT for sale, and then front-run a sale transaction by removing a child NFT that the buyer was expecting to own after the sale.
In theory this front-running and modification could be detected as part of the contract executing the sale transaction, but in practice NFT marketplaces won’t be aware of NFT nesting, so they need to rely on a hierarchy being immutable.
I address this by making the parent NFT’s tokenId a content address/hash of the child NFTs. Changing the children changes the tokenId, (burns and mints a new parent), so its not possible to be rug-pulled when buying such an NFT hierarchy, as the sale’s transfer will fail because the sold token has been burnt.
Rather than using content addressing, sequential IDs that aren’t re-used could also work, but content-addressing has the property that the same conceptual NFT can be brought back into existence by combining the same children as before.
I think at least some warning of this kind of issue should be added to the security section, even if a general remediation isn’t possible.
2 Likes
First of all, thanks a lot for such a detailed review! There are some very interesting points, I hope I’ll cover them all.
Firstly, just a typo, this should be “supply chains”?
Good catch, we’ll fix.
My own use case doesn’t map well to an ordered list of children
There’s no particular order needed to keep track of the children in the spec. It needs to be a list though, we consider it important to be able to get the full list from other contracts.
The propose-approve addition model is not something I need, so supporting it would feel unnatural.
We strongly believe this is a key component of this EIP, since not including it opens the doors for spamming. You could auto-accept the children directly on add, we’ve done this already in some implementations where we mark some trusted collections which get accepted directly.
I feel that a lot of the spec deals with how to create and modify hierarchies of NFTs. Construction of the hierarchy is something that is fairly application-specific, as every application will have their own requirements for what combinations of things are valid and what permissions are required. (…) For me, there’s more benefit in standardising how to read/interpret a hierarchy than write a hierarchy (…)
Due to the propose-approve pattern, the accept and reject functions are needed. Besides that, we actually need the addChild function there because it is the entry point to create such hierarchy. We also need a way to transfer children out, since only the parent NFT should be able to manage them. So while I agree interfaces should focus on how to read rather than write, it seems unavoidable to have these minimal writing functions.
The way ownerOf() is specified breaks compatibility with 721. In my opinion, it should report the contract address that owns the NFT, not the account that owns it transitively. Instead of changing the semantics of ownerOf() the spec should define a new function should for the indirect owner. (…) However, I’m not sure that such a function is needed. Because 721 already defines ownerOf() which can point to the owning contract, the only thing missing is a standard way to ask the owning contract which of its tokens owns the child NFT.
Current implementation of ownerOf is in line with ERC721 as it signals who controls the token, be it nested or not. We feel that modifying such behavior, by returning the parent token’s contract address, might break the backwards compatibility.
Let’s use an example of access pass NFT for this. If the access pass is nested, the access would be granted to the parent token, not the root owner as intended.Additionally this would create the need for two additional methods; one for retrieving the root owner and another that would signal whether the token is nested or not and into which token it is nested.
Similarly, 721’s Transfer event already communicates the transfer of ownership to the parent NFT contract, but an event listener needs to know which tokenId is the owner. Rather than having the child emit a new NestTransfer event, the parent contract could emit a Receive event, containing the tokenId that now owns the child.
A Receive event would need to have the information about the previous tokenId owner or we wouldn’t have the full picture. This information is already known when we emit the NestTransfer, but not on the addChild call.
This approach would allow regular 721 contracts to interoperate, without needing them to know that their owner is an NFT.
We don’t expect a regular 721 to be interoperable with this interface. It simply cannot be done since those have no way to be transfererd into a Nestable token identifying the id of the parent.
The spec mentions INestable and nestMint which are not defined in the spec.
Thanks for noticing, we will fix soon.
On the security side of things, an issue I’ve considered in my own nested ownership situation is with selling the nested NFTs, or more generally, allowing other contracts to use them safely. The problem is that if a hierarchy can be modified by its current owner, the owner could list the NFT for sale, and then front-run a sale transaction by removing a child NFT that the buyer was expecting to own after the sale. (…)
We are well aware of this problem and agree that it should be clearly mentioned on the spec. Thanks for bringing it up!
Please let me know if I left something out.
2 Likes
Thanks for your reply, I appreciate you taking the time! I think I have a better understanding of your intentions and goals for this now. I can see it’s necessary to define the transfer mechanism for what you’re doing with it. Particularly when designing this kind of interface for new NFTs that are explicitly aware of nesting.
Just to explain my approach, I use an approve-and-spend interaction, like you’d use to have a contract use an ERC20. E.g. The owner of an NFT to be wrapped approves the parent contract to operate its NFT. It then calls a function on the parent contract, which can then transfer the caller’s NFT to itself. It has the same UX problems (2 separate transactions) as ERC20s with regular EOAs though.
1 Like
This was the key area that stood out to me as not ideal for this ERC, and why I hesitate to integrate it: the concept of any token collection being “spam” is easily handled at the UI layer rather than the blockchain layer. Token collections are separate contracts, and only appear “in your wallet” if the UI layer knows of the collection contract and enumerates it. There are already robust solutions for curated token set lists that different authorities produce that wallets/galleries/tools can adhere to. Giving users the ability to ignore/remove a token from their visual wallet in the UI is gas-less and doesn’t affect legitimate tokens being transferred in.
Dealing with spam tokens by forcing EVERY transfer to EVERY parent token to need a second transaction (gas cost to the receiver) is a worse experience for every legitimate/wanted token transfer, for a benefit that seems like it could be achieved other ways. Hence, compared to a system like the (abandoned) EIP998, I think this system is overly-costly to the legitimate end users.
This ERC is already in final, but the behavior you see as problematic can easily be solved by having an auto accept mechanism for collections you specify and using it on the _afterAddChild hook. This has been used already in a few collections. Alternatively, you could just that that for all if it fits your use case better.
Here’s a snippet:
mapping(address => bool) private _autoAcceptCollection;
function setAutoAcceptCollection(
address collection
) public virtual onlyOwner {
_autoAcceptCollection[collection] = true;
}
function _afterAddChild(
uint256 tokenId,
address childAddress,
uint256 childId,
bytes memory
) internal override {
// Auto accept children if they are from known collections
if (_autoAcceptCollection[childAddress]) {
_acceptChild(
tokenId,
_pendingChildren[tokenId].length - 1,
childAddress,
childId
);
}
}
RE 998. It is in deed abandoned, and we did consider it before proposing our own EIP. One of the issues we found isthat it tried to solve different problems in a same EIP with 4 implementations. But the real stopper was that we discovered huge security risks when doing bottom up approaches.
This EIP works well to make any NFT from the spec available as child or parent with a single interface, always giving custody to parent. These kind of problems can not be seen easily from just the interface, that’s why we worked for several months on testing this internally and producing a stable sample implementation before proposing for feedback.
If you’re interested, you can find a basic implementation of this ERC on the assets folder, or we also provide a convinient NPM package, very similar to what OpenZeppeling does: @rmrk-team/evm-contracts - npm. It contains core implementations for this and other ERCs (5773 and 6220), with extensions and 3 ready to use implementations of each, for either lazy minting or preminting with ERC20 or native token. Repo is public so feel free to contribute!
1 Like
Yes, logic can be added to short-circuit the approval step, but by having the propose-accept infrastructure be part of the standard and not an implementation choice adds bloat to any contract that wishes to not use it. My intent is to express feedback as a developer evaluating which standard(s) to implement in new projects; that requirement makes me more likely to skip this one in favor of an alternate “nestable” standard.
Additionally, as I’m evaluating how implementing this standard for projects that have existing token collections, I’m concerned that within the EIP-6059 definition, it sets up a requirement to not use zero as a token identifier, but also claims backward-compatibility with ERC-721:
ID MUST never be a
0value, as this proposal uses0values do signify that the token/destination is not an NFT.
The Nestable token standard has been made compatible with ERC-721 in order to take advantage of the robust tooling available for implementations of ERC-721 and to ensure compatibility with existing ERC-721 infrastructure.
These two statements are at odds with each other. The ERC-721 standard only specifies that identifiers be uint256 values:
NFT Identifiers
Every NFT is identified by a unique
uint256ID inside the ERC-721 smart contract. … While some ERC-721 smart contracts may find it convenient to start with ID 0 and simply increment by one for each new NFT, callers SHALL NOT assume that ID numbers have any specific pattern to them, and MUST treat the ID as a “black box”.
This includes zero as a valid identifier. All token collections that include a token with an identifier of zero can still be a compliant ERC-721 token (assuming it adheres to all the other requirements of that standard). By my understanding of the ERC-6059 standard, any ERC-721 token that has an identifier of zero would be unable to be set as the parent or child of any other token (attempting to add a child to token ID #0 would incorrectly try to assign the child to the ERC-721 token contract itself). Is that correct? If so, I feel it is inaccurate to claim EIP-6059 is “compatible with ERC-721” (and to make claims like “This EIP works well to make any NFT from the spec available as child or parent”), as it’s only compatible with some ERC-721 tokens.
I’ve put a lot of thought into this, trying to see if we could have made the propose-accept only optional, or as an extension. It would remove the need for many methods but there would be a problem in the ChildTransfered event and the transferChild function, since they need the pending flag there. Not being able to easily extend it to propose-accept would have been a big issue for us.
On the other hand, the current ERC allows for an easy implementation which bypasses this pattern. Maybe we could add a note and even a sample implementation on how that would work. Which is basically having addChild send children directly to active array and emitting both ChildProposed and ChildAccepted. And have empty implementations for functions which manage pending children.
The spec here means 6059, and in deed, any 6069 implementation can work as either parent or child to any other 6059. We don’t pretend to be able to receive or send to existing 721s, that will not work and I don’t think we claim such ability.
This is compatible in the sense that any functionality that’s out there for 721s will also work for tokens implementing 6059. Not allowing for ID 0 does not change this backwards compatibility for all existing tools build for 721.
If you want to use this standard there are 2 cases:
- You are creating a new collection. In which case, you can simply start from 1, we don’t see how this could be problematic.
- You want to add this functionality to your existing collection. For this, you can use a wrapper which locks 721s and mints 6059. We have such tool and what we do is to map ID 0 (if existing) to max supply. This is annoyance, yes, but preventing the use of ID 0 simplified the code, reduced possibility of errors (since a mapping would default to Id). Additionally the gas and size usage was reduced by not having such checks. It was a trade-off and we feel we made the right choice.
1 Like
Okay, that clarifies things; the intended audience for this standard seems to be primarily aimed at new creations, and ones that won’t be interacting with ERC721 tokens as children.
I reread the “Backwards Compatibility” section and realized it does have a more full description of what the intention here is (EIP6059 works with older tooling), but to me the crux of the issue is I don’t think that is the definition of “backwards compatible”, which lead to my confusion.
I most commonly have seen that term used to indicate “old things can work in the new tool” (e.g. What is Backward Compatible (Backward Compatibility)? “software system that can successfully use interfaces and data from earlier versions of the system or with other systems”). Therefore the intention of “ERC6059 tokens can masquerade as ERC721 tokens for tools that don’t know about ERC6059” is a fine goal, but is going the “wrong way around” (“new thing works in the old tool”) so I believe should be labeled something different than “Backwards Compatible”.
I think this is a very valid point. We will update that section to be more clear about compatibility. Thanks!
1 Like
How does this compare/relate to:
I guess it is orthogonal to EIP-6551 since it has to specifically with nested NFTs, but your proposal does seem to overlap substantially with EIP-998.
As someone who builds on-chain games, simple ownership is not enough to convey semantic information about how an NFT owns other NFTs. Instead, our solution (still in early stages, but one gaining adoption) is to use an external registry contract which takes ownership of children/items: contracts/InventoryFacet.sol at develop · G7DAO/contracts · GitHub
In terms of actionable critique - I would like to understand if you looked at ERC-998 and why that fell short of your needs.
EIP998 is abandoned, and the author of EIP6059 did already comment on it here; they did read it and draw inspiration from it, but didn’t directly mirror its structure.
EIP6551 is another way to achieve a similar result, but EIP6551 aims to be completely external to the “parent” token (and hence even existing tokens or non-ERC721-tokens could implement), while EIP6059 aims for new token collections, as a way to have child-token-ownership “baked in” (aiming to have EIP6059 be a new “base standard” like ERC721 that wallets and apps would implement, expecting that additional functionality beyond basic ownership/transfer functionality).
1 Like
Exactly.
@stoicdev0 took some time, to prepare a really nice comparison on Twitter. Let me share it here:
https://twitter.com/stoicdev0/status/1657236277891067904
We use ERC-6059 in the ERC-6220, which seems to solve some issues that you are solving in the InventoryFacet. I didn’t take too deep of a dive, but you might find it useful 
1 Like
Thank you @MidnightLightning and @ThunderDeliverer .
I didn’t realize that ERC998 was abandoned but it has certainly been under draft for a long time. Thanks for clarifying.
Didn’t know about ERC-6220, either. Thank you for pointing it out.
1 Like
The function nestTransferFrom() declared in EIP and in attached implementation mismatch. The parameter data is missing.
The EIP6059’s function ownerOf() returns the root owner actually. It is not just a design decision. Third party tools, indexers, that track only ERC721 events will find a different owner than is returned by ownerOf(). Other platforms, marketplaces, wallets, can track ERC721 events and synchronize with blockchain by calling ownerOf() and balanceOf() occasionally. This provides a clear inconsistency. The test that fails.
it('Transfer event does not point to the owner', async function () {
// preparation
await parent.mint(alice.address, 1);
await child.mint(alice.address, 2);
await child.connect(alice).nestTransferFrom(alice.address, parent.address, 2, 1, '0x');
// verification
expect(await child.ownerOf(2)).to.be.eq(alice.address);
const aliceAddressSerialized = ethers.utils.hexZeroPad(alice.address.toLowerCase(), 32);
const transferTopic = ethers.utils.id('Transfer(address,address,uint256)');
const tokenIdSerialized = ethers.utils.hexZeroPad('0x02', 32);
const logs = await ethers.provider.getLogs({address: child.address, fromBlock: 1, toBlock: 'latest', topics: [transferTopic, null, null, tokenIdSerialized]});
const lastOwner = logs[logs.length - 1].topics[2];
expect(lastOwner).to.be.eq(aliceAddressSerialized);
});
There is another problem with the function balanceOf(). An address that is an owner of a token in terms of ownerOf() should have positive balance, from ERC721 perspective. The test that fails.
it('the owner has a balance', async function () {
// preparation
await parent.mint(alice.address, 1);
await child.mint(alice.address, 2);
await child.connect(alice).nestTransferFrom(alice.address, parent.address, 2, 1, '0x');
await parent.connect(alice).acceptChild(1, 0, child.address, 2);
// verification
expect(await child.ownerOf(2)).to.be.eq(alice.address);
expect(await child.balanceOf(alice.address)).to.be.gt(0);
});
So EIP6059 is not 100% compatible with ERC721. Some 3rd party tools can run into inconsistency.
And please check the function burn(). It puts the root owner instead of the direct owner in the Transfer event, this is different than in other places.
@SamWilsn can you advise on how to proceed here? The interface ID in the proposal takes the missing parameter into account, but the parameter itself is not there in the specification. What would be the best way to address this?
Can we open another PR to fix this issue or is there another way of going about it since the proposal is in Final status?
The least contentious approach would be to make a new EIP with a new number. This has the downside that EIP-6059 will stick around in Final forever, but would end up with the most clear document in the end.
You could also make a small change to EIP-6059 to fix the issue in such a way that contracts that would’ve been valid today are still valid. From my understanding, calling a Solidity function with extra calldata is fine, so the backwards compatible change would be to add the parameter and something like this to the comment:
@param data Additional data with no specified format, which should be sent in call to `to`
The key bit is the “should”, which means implementations are free to ignore data.
2 Likes
Thank you for your suggestion!
We decided to open a PR to update the current proposal. We feel like this is the best course of action since having two virtually the same proposals makes little sense to us. This way, we end up with a complete proposal and allow for implementers to continue using a standardized way of nesting non-fungible tokens.
1 Like