ethereum / EIPs

The Ethereum Improvement Proposal repository
https://eips.ethereum.org/
Creative Commons Zero v1.0 Universal
12.83k stars 5.24k forks source link

ERC-998 Composable Non-Fungible Token Standard #998

Closed mattlockyer closed 2 years ago

mattlockyer commented 6 years ago

title: ERC-998 Composable Non-Fungible Token Standard authors: Matt Lockyer mattdlockyer@gmail.com; Nick Mudge nick@perfectabstractions.com; Jordan Schalm jordan.schalm@gmail.com status: Draft type: Standards Track category: ERC created: 2018-04-15 requires: 721, 20

Update

The latest code is in the EIP and reference implementation

EIP eip-998.md

Reference Implementation https://github.com/mattlockyer/composables-998

Reading Original Medium post: Introducing Crypto Composables

Crypto Composables - Use Cases and Applications

Abstract

A standard extension for any non-fungible token to own another non-fungible ERC-721 or fungible ERC-20 tokens. Transferring the token composition means transferring the entire hierarchy of items. For example, a cryptokitty may own a scratching post and a feeding dish; the dish may contain some amount of fungible “chow” tokens. If I sell the cryptokitty, I sell all of the belongings of the cryptokitty.

Specification After several discussions with the community we settled on providing these 4 extensions to a standard ERC-721 NFT in order to support composable functionality:

ERC998ERC721 top-down composable tokens that receive, hold and transfer ERC721 tokens ERC998ERC20 top-down composable tokens that receive, hold and transfer ERC20 tokens ERC998ERC721 bottom-up composable tokens that attach themselves to other ERC721 tokens. ERC998ERC20 bottom-up composable tokens that attach themselves to ERC721 tokens.

For more details on the reference implementation, visit the EIP.

Summary

I’ve presented a new standard for composable non-fungible tokens, ERC-998, that allows for the ownership of ERC-998, ERC-721 and ERC-20 tokens. This allows standard assets to be composed into complex compositions and traded using a single transfer. The additional functionality that can be added to a non-fungible token based on the presence of specific child tokens is open for exploration. I believe that standard interfaces for non-fungible and fungible assets to be composed and act upon one another is an exciting new area for the Ethereum community to explore and I look forward to seeing the work.

LATEST: EIP with reference implementation code

eip-998.md

mattlockyer commented 6 years ago

It was pointed out to me by @jeffwalsh that adding an ERC-20 token required it's own method for incrementing the balance rather than replacing it.

He's also implemented an example using a basic ERC-721 in this fiddle. Many thanks Jeff.

Physes commented 6 years ago

You might want to check out Delegated Non-Fungible Tokens: #994 which already sets out the architecture of parent/child NFTs. Whereas a single NFT is considered a "deed", a tree of NFTs could be considered a "zone".

mryellow commented 6 years ago

sets out the architecture of parent/child NFTs

// maps child delegate token to parent ID
mapping(uint256 => uint256) private sub;

sub[_delegate] == _tokenId, sub[newTokenId] = _tokenId

Not too different from the proposed mapping:

// which asset owns which other assets at which address
mapping(uint256 => mapping(address => uint256)) children;

However not dealing with the same contract with parents and children being simply related key to key as is the case with #994.

sub[newTokenId] = _tokenId as described works as it's always the child having a single parent. Where children[_tokenID][_childContract] = _index looks to fail as it will over-write the previous child token entry when a new child is added.

Could it be isChild[_tokenId][_childContract][_childId] = true is appropriate mapping instead?

mapping(uint256 => mapping(address => mapping(uint256 => bool))

mryellow commented 6 years ago

This was an edit, but then grew so I'll post again:

I guess for fungible you want it to be an integer.

mapping(uint256 => mapping(address => mapping(uint256 => uint256))

Where > 0 == true for NFTs. i.e. setting it to 1 is sufficient to be recognised as a child.

Alternatively the whole thing on it's head so that one child has one parent as is the case with #994.

parents[_childContract][_childId] = _tokentId with fungibles using index 0 for _childId

edit: Although that then breaks when you're trying to figure out where a fungible is held.

mattlockyer commented 6 years ago

I see your point about the child tokenId being overwritten, that's a pain.

I had several approaches to this and I knew this wasn't perfect. You are right that a child contract will only be able to have one ERC-721 child. Darn.

A strict requirement of this proposal will be NOT requiring any changes to children that are already ERC-721 or ERC-20, meaning they won't have to upgrade to be a child of a composite.

Another approach was to keep the mapping:

mapping(uint256 => mapping(address => uint256));

And upgrade to a pseudo unique address for ERC-721 children:

// generate a unique address for ERC-721 child tokens
// from _childContract and _childTokenId
address childToken = address(keccak256(_childContract, _childTokenId));
// everything successful and you want to add the child token
// use the unique address
children[_tokenId][childToken] = _childTokenId;

Note: this is actually how the EVM generates smart contract addresses, using the hash of address + nonce as a bytes20 address type.

In this way we have a pseudo unique address for every child token.

We will have to adopt your bookkeeping method 0 means no child and 1 means child exists.

I really wanted to reduce the need for a deep mapping as in your proposed solution, although my unique address generation pushes a bit more computation and gas fees onto the miners and users.

This would have to be calculated to transfer a child token as well, like so:

address childToken = address(keccak256(_childContract, _childTokenId));
// other conditions
require(children[_tokenId][childToken] == 1); //0 is false and 1 is true for non-fungible tokens
// transfer child here if other conditions check out
children[_tokenId][childToken] = 0;

Thoughts on this @mryellow?

mryellow commented 6 years ago

address childToken = address(keccak256(_childContract, _childTokenId));

Interesting, so it's keccak256(_childContract) or just straight _childContract address in the case of a fungible?

I was thinking a lot of the issues go away when dealing with non-fungible or fungibles independently. It's always this bit where you have to start thinking about using "tokenId" index 0 or returning "tokenId" 1 as "true", which all ends up feeling more untidy then was the intention.

Perhaps the neatest solution is to not attempt to deal with them in the same variable, but storing differently once you know what type you're dealing with. Not sure how best to represent anything like that semantically though. Could add a boolean for fungible to any methods where tokenId/value appears to select between them, though that's not exactly neat either.

mattlockyer commented 6 years ago

I think you avoid it when dealing with fungibles. So in the fungible functions, simply use the fungible contract address. There's no need for extra computation in the fungibles functions.

Could be possible to go with a completely new mapping for non-fungibles and deal with them seperately.

mapping(uint256 => mapping(address => bool)) childTokens; // NFTs
mapping(uint256 => mapping(address => uint256)) childBalance; // FTs

And still use the pseudo address construction for dealing with NFTs to ensure a unique address per NFT.

mryellow commented 6 years ago

pseudo address construction

Another thought on this concept, ERC: Ethereum Claims Registry #780 features some prior work which might be informative:

The key parameter is intentionally very generic. The ERC does not impose any restrictions saying that certain values for key should only be used for certain things. Rather this is something that the community is free to experiment with. If there is some emerging standard a new EIP can be created for that. There are also a lot of already existing naming conventions for claims that are in use today and coming up with our own standard might limit the ability for these to be used.

Having a key with a construction recipe for fungibles and non-fungibles which is consistent might be something worth looking at. Perhaps stepping away from address type here is nice or opens up more possibilities.

bytes32 key = bytes32(keccak256(_childContract, _childTokenId));

However would this add anything or is the key always built the same way?

Would anyone ever want to use the key for anything else other than an "address" for a token?

mattlockyer commented 6 years ago

My feeling on this, and why I refer to is at "pseudo address creation" is because it is analogous to how smart contract addresses are generated given the nonce of the smart contract.

So here we are creating an address for every NFT based on the contract address and tokenId of the NFT. This is exactly the same as if every NFT were deployed as a new contract from the main contract.

vernon99 commented 6 years ago

This is a great start, thanks. Like the idea of pseudo-addresses for child tokens. Have a bunch of thoughts. Here're three major points.

  1. Mixed token types in one mapping.

Could be possible to go with a completely new mapping for non-fungibles and deal with them seperately.

As you said, I think this is a preferred solution. I don't think it's a good idea to have one mapping for two different standards. It adds complexity (and fragility), adds unexpected behaviors, makes it harder for the wallets to process two different types. Wallets actually don't know the type beforehand - how would they know what token is that without guessing that through presence of the methods, etc? To me that's the biggest problem that I don't see how to resolve without splitting. Token type needs to be defined very explicitly.

So fungiblePossessions and nonfungiblePossessions may be more appropriate, with comments explicitly saying what standards to they support (ERC20+ERC677 and ERC721 respectively).

  1. Currently it requires two transactions to pass ownership. This is very old-school :) ERC721 supports safeTransferFrom with onERC721Received callback (that needs to be implemented by this standard). ERC20 unfortunately doesn't and ERC223 is not backwards compatible with ERC20. But there's a nice ERC677 addition to ERC20 that keeps it backward compatible while adding transferAndCall and tokenFallback pair similarly to described for 721 above. All the key exchanges already support that and this token should also support the callbacks. This is very critical to avoid two transactions per transfer, a bane of many early tokens.

  2. There has to be a way to enumerate all the children. Otherwise you have the wallets problem again, now for list of possessions. ERC721 actually implementa it, see ERC721Enumerable (which should really not be optional). The way it works is we maintain both the hashing for retrieval/checks (to see if it holds this particular token) but also an array to be able to get a token by index. Then you just iterate till you hit 0. That also gives you numberOfChildren that is missing right now completely from the spec. To avoid loops in the smart contract we will need to smartly update the array and keep indices. See how Decentraland did it in the storage and then in enumerable interface.

Also, more like a suggestion. "children" naming sounds a bit off for ERC20. It's more like stuff that belongs to this token. Maybe possessions would be better, or something like that. Nit.

mg6maciej commented 6 years ago

Hello everybody and thanks @mattlockyer for creating this ERC. I was thinking about something like this for over a month, happy to see it brought to attention of a broader community.

My initial idea was very similar to this, that you embed such functionality as an extension to ERC721, and your ERC998 contract would be an owner of some ERC721 tokens (from the perspective of owned token) while keeping information that the true owner is a token this contract itself keeps track of.

I see some problems with such approach. The most important one being that if you have a chain of tokens from different ERC998 contracts, as a user you will have hard time understanding what contract you need to call to transfer a CryptoKitty that is owned by a DogERC998 token that is owned by PersonERC998 token that you own. I might be wrong here, but my intuition tells me you would have to call PersonERC998 and pass it 2 arrays of all contracts and tokens that are owners (parent, grandparent, ...) of the token to be able to retrieve that poor little Kitty.

Currently I'm thinking this is a perfect example where global / master registry could be used. I started drafting it (code is here) and would like to ask all of you if you also think this might be a good idea. I also agree with all the points @vernon99 mentioned and already partially incorporated them into the codebase. The biggest advantage of using registry is that it will support CryptoKitty being owned by another CryptoKitty.

mattlockyer commented 6 years ago

Thank you everyone for the comments. I have been grinding through an implementation with tests so we have a basis from which to work on finding the most optimal solutions together.

@vernon99 I have address the first 2 concerns for nonfungible "possessions". I like this name.

mapping(uint256 => mapping(address => bool)) nonfungiblePossessions;
  mapping(uint256 => mapping(address => uint256)) fungiblePossessions;

  function bytesToUint(bytes b) internal pure returns (uint256 result) {
    result = 0;
    for (uint256 i = 0; i < b.length; i++) {
      uint256 c = uint256(b[i]);
      if (c >= 48 && c <= 57) {
        result = result * 10 + (c - 48);
      }
    }
  }

  function _nonfungibleAddress(
    address _childContract, uint256 _childTokenId
  ) internal pure returns (address) {
    return address(keccak256(_childContract, _childTokenId));
  }

  /**************************************
  * ERC-721 Non-Fungible Possessions
  **************************************/

  //adding nonfungible possessions
  //receives _data which determines which NFT composable of this contract the possession will belong to
  function onERC721Received(address _from, uint256 _tokenId, bytes _data) public returns(bytes4) {
    //convert _data bytes to uint256, assuming tokens were passed in as string data
    // i.e. tokenId = 5 would be "5" coming from web3 or another contract
    uint256 id = bytesToUint(_data);
    nonfungiblePossessions[id][_nonfungibleAddress(msg.sender, _tokenId)] = true;
    return ERC721_RECEIVED;
  }

  //transfer the ERC-721
  function transferChild(
    address _to,
    uint256 _tokenId,
    address _childContract,
    uint256 _childTokenId
  ) public {
    // require ownership of parent token &&
    // check parent token owns the child token
    // use the 'pseudo address' for the specific child tokenId
    address childToken = _nonfungibleAddress(_childContract, _childTokenId);
    require(_owns(msg.sender, _tokenId));
    require(nonfungiblePossessions[_tokenId][childToken] == true);
    require(
      _childContract.call(
        // if true, transfer the child token
        // not a delegate call, the child token is owned by this contract
        bytes4(keccak256("safeTransferFrom(address,address,uint256)")),
        this, _to, _childTokenId
      )
    );
    // remove the parent token's ownership of the child token
    nonfungiblePossessions[_tokenId][childToken] = false;
  }

This code is working now, it's here if you want to run it yourself.

I have put ERC-20 on hold for a bit, seems relatively trivial once ERC-721 is working. @vernon99 thanks again for the comments on where to look for that implementation.

@mg6maciej thanks for dropping in here. I think the global registry is a fantastic solution to a lot of the needs of developers out there.

I do foresee some issues with functionality, specific implementations that restrict the composability of assets for example. For all basic compositions though, I think it's quite a good approach!

mattlockyer commented 6 years ago

I've made some updates to the comment at the top to reflect the dramatic changes to the originally proposed code.

Thank you @vernon99 @mg6maciej for the feedback. It motivated me to push forward and get a more comprehensive and elegant solution up.

mg6maciej commented 6 years ago

I do foresee some issues with functionality, specific implementations that restrict the composability of assets for example.

Yes, there are things that need to be taken care of in 998. I can think of these cases:

mg6maciej commented 6 years ago

Another thing worth discussing is what kind of events these transfers will fire.

mattlockyer commented 6 years ago

Another thing worth discussing is what kind of events these transfers will fire.

Absolutely.

I think this is a great start.

@mg6maciej What are the thoughts on NFTP as a title for both the non-fungible token possessions and FTP for fungible token possessions?

mg6maciej commented 6 years ago

I'm not that interested in choosing names for anything for now. I think it's too early. I'd be more than happy to change names in my implementation at some point.

I've noticed a potential issue with different implementations of ERC721 tho. There are a couple of ways to interact with them.

If you just use transferFrom, you will basically lock all old ERC721 tokens as children.

7flash commented 6 years ago

It looks that ERC-998 is perfect fit for intellectual property tokens. For example, parent contract could reflect the ownership of education course of author. Children tokens could reflect the copies for students. If there is any issues?

mattlockyer commented 6 years ago

@mg6maciej I'm going to be adding the callbacks and events soon to the implementation

I know you're but syncing on the composables instances + registry will be crucial, let's touch base about this next week.

If anyone has any feedback on how to interop a global composable NFT registry with individual instances of Composable 998 (721 extended) please feel free to chime in!

Also there is a Discord for general #buidl chatter around NFTs here: https://discord.gg/3TtqP2C

crowdedconscious commented 6 years ago

Very interesting work Matt! Following your progress closely - keep it up.

jordanschalm commented 6 years ago

Hi everyone! Myself, @flockonus and a few other folks from the CryptoKitties team had the opportunity to meet @mattlockyer a couple days ago to discuss designs for composable NFTs. We primarily discussed a different design than what is currently in this ERC, which I’ll summarize here to get feedback on from all of you fine NFT enthusiasts!

Current Design Issues

Ownership state is split between child/parent

When a token is owned by a composable “parent” NFT, the complete ownership state is split between the child contract and the parent contract. The child contract (which may not know about ERC998) state says that the token is owned by the parent contract address. The parent contract specifies which NFT within the contract actually owns the child token. Similarly, sometimes methods related to ownership are executed from the child contract and other times they are executed from the parent contract. With the bottom-up approach, all ownership state and associated methods would be implemented in the child.

Implementation Complexity

Basically, the current design requires implementors to maintain a lot of state! Parent contracts would need to track who owns their tokens, which child tokens are owned by the parent contract’s tokens, and reimplement/wrap transfers and the like for these child tokens. With a bottom-up approach, all ownership state and methods would be compartmentalized in the contract responsible for the given token.

Ownership terminology overloading

Ownership in Ethereum is currently strongly defined as a mapping from Ethereum addresses to tokens of some kind. Current contracts rely on this relationship for ownership operations (you transfer an ERC20 token to a native Ethereum address). Since being backwards-compatible with this existing notion of ownership is not possible for composable tokens, we suggested being explicit about the semantic difference between the two notions of ownership by naming them differently. We suggested maintaining the own verb for ERC20/ERC721-style ownership, and using something else (maybe link or attach?) for when an NFT “owns” some other token.

Bottom-up Design Proposal

The current proposal describes ERC721 tokens with logic added so they can own child tokens. The bottom-up design inverts this, instead proposing an extension to ERC721 and ERC20/223 that enables them to be linked/attached to any ERC721 token, in addition to being owned by an Ethereum address (as is the case now).

Similarly to the current proposal, the ownership hierarchy for a particular NFT can be thought of as a tree, with an Ethereum address at the root, NFTs in the middle, and either NFTs or fungible tokens at the leaves. image

Rationale

In addition to the benefits in the next section, the rationale behind this proposal comes from our experience with CryptoKitties. After releasing CryptoKitties, many developers built apps, tools, and tokens that extend the CryptoKitties ecosystem. It doesn’t make much sense for a CryptoKitty to own EOS tokens, but it does make sense (and is super cool!) for a CryptoKitty to own a provably unique cat-hat token (see KittyHats). In general, we see value in NFTs being extended by contextual tokens with semantic meaning, not NFTs being able to own arbitrary tokens. We think the bottom-up approach best fits this ecosystem-extension view of composable tokens.

Benefits

Drawbacks

mryellow commented 6 years ago

We suggested maintaining the own verb for ERC20/ERC721-style ownership, and using something else (maybe link or attach?) for when an NFT “owns” some other token.

Wonder if there are issues similar to "approval" where some "attachment" might be orphaned by a chance in "ownership" at the child level.

Does a cat continue to be attached to a hat after the hat's ownership has changed?

Who can detach the hat?

In general, we see value in NFTs being extended by contextual tokens with semantic meaning, not NFTs being able to own arbitrary tokens.

I'm rather interested by the possibilities when it comes to arbitrary standard tokens rather than specifically crafted tokens setup to be children. Semantics can be re-imagined in new contexts.

i.e. Turn the power on for your city in a dApp game by adding POWR tokens, or covering your enemy in GOO, those tokens contributing value which is then contested and exchanged as part of gameplay. In a non-game contexts this might mean bonded curves and curation market type associations where parent token value is a function of children.

This "composable" approach may be entirely unsuited for such tasks in the end, the proper approach is probably a lot more custodian and based around "deposits", but worth mentioning a use-case for the parent-child ownership style even if I'm not seeing the cleanest way to lay it out.

The current proposal describes ERC721 tokens with logic added so they can own child tokens

Perhaps the best approach is to separate everything. ERC721's go on being themselves, ERC20's do their thing. Then another contract can associate them, perhaps by taking custody and wrapping into a new combined entity.

mudgen commented 6 years ago

I have been looking into the bottom-up approach. Some comments:

Using the bottom-up approach it is possible for new composable-aware ERC721 parent contracts to have onchain enumeration for any token that is transferred to it. This is done by using the safeTransferFrom and the onERC721Received functions to pass into the parent contract the child tokenId. And when the child contract transfers itself out of the parent it should make a call to the parent contract to remove itself.

I really like the idea of doing enumeration off-chain because I don't like the expense and complexity of the bookkeeping on-chain.

I like the idea of using events to track child tokens of parents. A cache of this info (child tokens) could be included in the JSON file returned by the tokenURI for each parent.

There is another issue which is buy/trade/sales verification of state. When selling a parent token the state of the parent token (all the children) can be checked using on-chain enumeration to verify it is all that is expected by the buyer. This verification can be done in the same transaction that the transfer is done. Without this verification it is possible for someone to remove children right before a transfer call and the buyer does not get what he/she paid for.

So my question is this: How to securely do state verification of a parent ERC721 token when selling it if there is no on-chain enumeration?

Perhaps the state verification can be done onchain by passing in an array of child contracts and child token Ids to check for after the transfer function is called. I think that would work.

mryellow commented 6 years ago

Related work, haven't dug into it but have seen this mentioned in relation to "composables".

We describe a specification for a new primitive that facilitates the lowcost, trustless creation, and exchange of a {Set}, a collateralized basket of ERC20 tokens on the Ethereum Blockchain. {Set}s serve as an abstraction for end users who want to think about higher-level token concepts without dealing with the details of specific tokens. {Set}s are a superset of the ERC20 token standard with issue and redeem functionality, allowing for the atomic swap of the {Set}s and their underlying tokens. As an investment, {Set}s are similar to index funds (e.g. S&P 500, DJIA) and exchange traded funds (ETFs) in traditional financial services, allowing users to easily get exposure to a multitude of tokens. As {Set}s are ERC20 tokens and composable, it is possible for one token to represent a limitless number of other tokens.

https://setprotocol.com/pdf/set_protocol_whitepaper.pdf

edit:

https://github.com/SetProtocol/set-protocol-contracts/blob/52260e68651a75adac7e5f1c4add96d8b9c48986/contracts/core/SetToken.sol#L121-L122

They create an ERC20 which has a pre-defined set of addresses and quantities associated with it.

flockonus commented 6 years ago

Does a cat continue to be attached to a hat after the hat's ownership has changed? Who can detach the hat?

@mryellow I believe that's the intention, the hat would keep on belonging to the same cat. Who can detach the item is the current owner of the cat.

So my question is this: How to securely do state verification of a parent ERC721 token when selling it if there is no on-chain enumeration? Perhaps the state verification can be done onchain by passing in an array of child contracts and child token Ids to check for after the transfer function is called. I think that would work.

@mudgen I think so too! If the item in case that is for purchase, has attached items that are important for the buyer then it would make sense that the purchase order would pass an array of address+ids to verify the items are indeed attached to the NFT.

It would work with signed transactions (similar to 0x style) orders as well, if the seller at the time of the sales creation also bundle the promise of the items in the order.

mg6maciej commented 6 years ago

@flockonus @mudgen https://github.com/ethereum/EIPs/issues/1180 proposes a generic solution for validation when selling NFTs.

Using the bottom-up approach it is possible for new composable-aware ERC721 parent contracts to have onchain enumeration for any token that is transferred to it. This is done by using the safeTransferFrom and the onERC721Received functions to pass into the parent contract the child tokenId. And when the child contract transfers itself out of the parent it should make a call to the parent contract to remove itself.

@mudgen I think your assumptions are wrong here. If I understand this bottom-up concept correctly, child token is never directly transferred to its parent, i.e. childContract.ownerOf(someTokenId) != parentContract is true, unless someone just sent a token to a contract using transferFrom and basically burned it.

@jordanschalm @flockonus Are you going to join our next composable call on NFTy Magicians discord? I would like to discuss this concept with you in depth and believe we should schedule a call sooner than planned. Also @jordanschalm I asked you some questions there, but if you want I can move it to GitHub.

mudgen commented 6 years ago

I recently did a lot of work on a reference implementation of ERC998 which can be seen here: https://github.com/mattlockyer/composables-998

I also wrote a draft EIP for ERC998 here: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-998.md But I have not done a pull request yet for this EIP to get it added because I would like more feedback first.

Some feed back on the EIP and the reference implementation would be very helpful.

Also, here is a blog post where I gave an overview of top-down and bottom-up approaches: https://hackernoon.com/top-down-and-bottom-up-composables-whats-the-difference-and-which-one-should-you-use-db939f6acf1d

mudgen commented 6 years ago

An EIP998 draft was recently added: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-998.md

chiro-hiro commented 5 years ago

@mattlockyer @mudgen It's could raise another issue about gas cost since almost the call is external call and increasing the number of token contracts in the long term.

flockonus commented 5 years ago

@mudgen Following The discussion on the channel, and commenting on the issue as requested

screen shot 2018-10-22

Currently there is this structure in the reference contract implementation:

struct TokenOwner {
    address tokenOwner;
    uint256 parentTokenId;
}

So I believe the implied idea of ++ parentTokenId to store is to account for the case where the token Id is actually 0? And at retrieval decrementing and doing the checks. The problem with is is that 0xff..ff would overflow, or fail the check, so it just transfers the problem?

If my assumption is correct I'd suggest we instead have this other struct, that would also cost 2 words (same as now):

struct TokenOwner {
    address tokenOwner;
    bool ownedByToken;
    uint256 parentTokenId;
}

So we don't have to overload meaning into parentTokenId, does it make sense?

mudgen commented 5 years ago

@flockonus Thanks for posting here about this.

Yes, makes sense. Your suggested solution looks fine to me.

flockonus commented 5 years ago

Some thoughts on functions of 998 Bottom up 721:

Simplify token transfer functions

transferFrom() -- as inherited from 721

transferChildToken(uint256 _tokenId, address _parentContract, address _parentTokenId)

Notice the transferChildToken suggestion doesn't have the _from field, defined in it. I believe it's not necessary and requires more logic from the client software that otherwise won't add to benefit. The reason is that the CURRENT from contract & tokenId (if any) can be fetched from the mapping.

I understand ERC721 it requires the _from input. Looking at Openzeppelin reference implementation also don't see the point in that, as it's easily retrieved from storage, and amount to extra checks.

awaldstein commented 5 years ago

Hi-- My focus is on Reimagining Philanthropy with NFTs.

Was a team leader on the Honu the CryptoKitty Charity Auction project with CryptoKitties and Bill Tai of Actai Global and Sir Richard Branson's team at Ocean Elders.

Putting together a presentation for NFT.NYC and exploring how 998 can become an added building block to create a future for this.

Looking for projects, ideas, visionaries who can help me (with attribution of course) draw a picture of a possible future with new tools and protocols.

Contact info. arnold@waldstein.com @awaldstein skype

Thanks!

jinserk commented 5 years ago

HI, The reference implementation is somewhat old solidity version, and got error when I use more than solc 0.5.0. Do you have any plan to upgrade this?

Compiling your contracts...
===========================
Error: CompileError: Error parsing /home/jinserk/works/blockchain/ethereum/pilot-ehs-on-ethereum/erc-998/contracts/ComposableBottomUp.sol: ParsedContract.sol:140:22: ParserError: Location already specified.
        bytes memory calldata;
                     ^------^
Compilation failed. See above.
    at async.whilst.error (/home/jinserk/.nvm/versions/node/v10.15.3/lib/node_modules/truffle/build/webpack:/packages/truffle-compile/profiler.js:366:1)
    at /home/jinserk/.nvm/versions/node/v10.15.3/lib/node_modules/truffle/build/webpack:/packages/truffle-compile/~/async/dist/async.js:969:1
    at next (/home/jinserk/.nvm/versions/node/v10.15.3/lib/node_modules/truffle/build/webpack:/packages/truffle-compile/~/async/dist/async.js:5222:1)
    at Promise.all.then.results (/home/jinserk/.nvm/versions/node/v10.15.3/lib/node_modules/truffle/build/webpack:/packages/truffle-compile/profiler.js:348:1)
Truffle v5.0.8 (core: 5.0.8)
Node v10.15.3

Latest openzeppelin-solidity package has renewal of ERC20 and ERC721 tokens implementation, so it should be also considered if you have update plan.

mudgen commented 5 years ago

@jinserk I don't currently have a plan to upgrade it. I would be happy for someone else to upgrade it.

mudgen commented 5 years ago

@awaldstein Sounds like an awesome presentation!

kivanov82 commented 5 years ago

Not sure what the current status of this EIP is at this moment, but I've done something similar, to be able to trade my NFTs (which wraps ERC20) as a sort of the loot box on OpenSee market. See https://github.com/kivanov82/ERC20Box

mudgen commented 5 years ago

The current status of the EIP is draft and people are using the standard.

niusealeo commented 4 years ago

The current EIP-998 proposal scopes the need for ERC998ERC20 and ERC998ERC721 composable tokens. This is fair and good. However there may be a need to expand the EIP-998 scope to include support for ERC1155 token collections by designing ERC998ERC1155 top-down and bottom-up composable definitions that give ERC721 tokens the top-down ability to hold and transfer ERC1155 token collections and ERC1155 token collections the ability to attach themselves to parent ERC721 tokens. I imagine this would work similarly ERC998ERC20 composables. It would be particularly useful for supply chain / provenance / digital twinning requirements in real world commercial applications especially where mixtures of various chemical fluids (represented by combinations of different fungible tokens), and perhaps also parallel component assemblies, are typically transferred in single transactions in real life. In a typical open-world or dungeon-crawling RPG game situation say, this could be likened to a players ability to search, hold and exchange between multiple different purses, treasure chests, bottles, etc. that each contain various assortments (ERC1155 collections) of the game world's items and currencies. For example, it could also represent alchemical potions that are the result of mixing various combinations of ingredients.

mudgen commented 4 years ago

@viltiki I totally agree with you!

nichanank commented 4 years ago

@viltiki I'm also interested in this development. my project's NFTs currently adhere to both ERC998ERC721 and ER998ERC20 but afaik wouldn't interoperate as-is with Enjin metaverse unfortunately.

tclairet commented 3 years ago

Hi, we developed a lightweight version working with the EIP1155 and the latest Open Zeppelin contracts. It would be very cool if you have time check it out. The repo is available here https://github.com/rocksideio/ERC998-ERC1155-TopDown.

All feedbacks are welcome.

mudgen commented 3 years ago

@tclairet That's great. I'm doing something similar with Aavegotchi.

jamesmorgan commented 3 years ago

Hi 998 people! We are also building out a 1155 flavour of this with a parent 721 which can hold 1155 tokens. However for us the token itself is the owner of the children, so move the parent 721, you also get ownership of the child 1155 tokens.

I have looked at @tclairet repo which looks great, thanks for that its really good to see that.

I have noticed that the method signatures for 998 topdown 721 are not really applicable for topdown 721 -> 1155 token.

Is there a common understand of what a common interface for a 721 -> 1155 top down interface would look like? Or even a more standardised 721 -> 1155 EIP somewhere?

mudgen commented 3 years ago

@jamesmorgan I modeled the 721 -> 1155 top down interface functions from 998. The function signatures can be similar to what ERC998 uses.

Like this:

/// @notice Transfer tokens from owner address to a token
/// @param _from The owner address
/// @param _id ID of the token
/// @param _toContract The ERC721 contract of the receiving token
/// @param _toTokenId The receiving token
/// @param _value The amount of tokens to transfer
function transferToParent(
    address _from,
    address _toContract,
    uint256 _toTokenId,
    uint256 _id,
    uint256 _value
) external;

/// @notice Transfer token from a token to an address
/// @param _fromContract The address of the owning contract
/// @param _fromTokenId The owning token
/// @param _to The address the token is transferred to
/// @param _id ID of the token
/// @param _value The amount of tokens to transfer
function transferFromParent(
    address _fromContract,
    uint256 _fromTokenId,
    address _to,
    uint256 _id,
    uint256 _value
) external;

/// @notice Transfer a token from a token to another token
/// @param _fromContract The address of the owning contract
/// @param _fromTokenId The owning token
/// @param _toContract The ERC721 contract of the receiving token
/// @param _toTokenId The receiving token
/// @param _id ID of the token
/// @param _value The amount tokens to transfer
function transferAsChild(
    address _fromContract,
    uint256 _fromTokenId,
    address _toContract,
    uint256 _toTokenId,
    uint256 _id,
    uint256 _value
) external;

@jamesmorgan remember there is EIP-2535 Diamond Standard if you start hitting the 24K max contract size limit. But it is also good for organizing a lot of functionality you want to keep at the same contract address. And for transparent upgrades.

tak1827 commented 3 years ago

I'd like to use this token as our service POC. we expect to issue parent and child ERC721 from a same ComposableTopDown contract. But, I realized that it's impossible because onERC721Received is external .

(Solidity version upgraded ERC998ERC721TopDown implementation is here: ERC998ERC721TopDown-solidity-v6)

Steps

First, mint 2 ERC721 token from a same contract.

composable_contract.mint(wallet_address) => tokenID_1
composable_contract.mint(wallet_address) => tokenID_2 

Second, safeTransfer 2nd token to a same ComposableTopDown contract.

composable_contract.safeTransferFrom(wallet_address, composable_contract_address, tokenID_2,  tokenID_1_bytes) => fail

Don't you expect this usage?

rnbrady commented 3 years ago

Hi @mattlockyer, nice work on this.

So here we are creating an address for every NFT based on the contract address and tokenId of the NFT. This is exactly the same as if every NFT were deployed as a new contract from the main contract.

Why not just take that approach? It seems desirable to make NFTs first class citizens with addresses. They can then own and be owned by other contracts or tokens. They could also be implemented as minimal proxy contracts that point back to logic in the main contract, minimising gas and storage costs.

On a related not, another capability that I think would improve composability and usability is separation of ownership and control/possession. Any ideas on that gratefully received.

Swader commented 3 years ago

Just to chime in here, we have developed nested NFTs as part of RMRK and our set of art legos, and will take this EIP and its comprehensive discussion into consideration when porting our logic over to EVM. For a breakdown, please see Dawn of the Art Legos. We are also thinking hard how to slap royalties onto this, but that's a separate problem.

mudgen commented 3 years ago

Just to chime in here, we have developed nested NFTs as part of RMRK and our set of art legos, and will take this EIP and its comprehensive discussion into consideration when porting our logic over to EVM. For a breakdown, please see Dawn of the Art Legos. We are also thinking hard how to slap royalties onto this, but that's a separate problem.

That's great. If your contract bytecode exceeds the max contract size you might want to consider using EIP-2535 Diamonds. I understand the challenge of implementing a lot of functionality into an ERC721 contract.

lukasz-glen commented 3 years ago

Hi. Any progress with ERC998ERC1155TopDown? I was going to implement it and encountered this discussion. I see some people thought about it for a while. This would be great to have a standard. So, are there any plans to extend EIP-998 with new interfaces? I also suggest to change naming convention. Repeating the term child can be misleading, it can be unclear what type of child it refers to. Names like safeTransferERC1155() are more transparent.