mattlockyer / composables-998

An implementation and documentation repo for developing the ERC-998 standard for Ethereum.
MIT License
105 stars 64 forks source link

Very confusing naming/design with TopDown and BottomUp #28

Open vernon99 opened 6 years ago

vernon99 commented 6 years ago

Why was 998 the contract split and renamed in two? Top down and bottom up. Doesn't make much sense to me both conceptually (only the parent should be composable) and in terms of the naming. Parent should be parent. Child should be child and may have a separate extra interface to communicate with the parent. I suggest to rename ERC998ERC721TopDown back to simply ERC998. And have ERC998Child interface if you want to have extra logic for the children, callbacks from the parents, etc. Like look at what ERC721 done with ERC721 and ERC721Receiver.

achadha235 commented 6 years ago

Both interfaces enable token composition through two different designs. Both interfaces can be combined or used independently to enable specific use cases with their own set of advantages and disadvantages. It helps to think about which contract implements the capability for token possession. In the top down case, it is in the possessor and in the other it is within the possession. From a design perspective, I think separating these two ideas makes complete sense.

As for the naming, I agree with you completely - the words "top down" and "bottom up" really give no information about the capabilities of these two interfaces. Nothing about the words "ERC" / "998" / "Top" / "Down" / "Bottom" / "Up" says anything about "token possession". Good interface names communicate the interface's capability in a few clear words. In our case, communicating which contract implements the possession capability is vital.

While the words "parent" and "child" add something more, I think they still miss the target because you can have complex parent-child relationships independent of which interface is used. In other words, child-parent relationships can exist whether you use top-down or bottom-up composition and hence the parent/child terminology does little to disambiguate which case you're talking about. Furthermore, you can also have cases where something is both a parent and a child. Instead of parent / child, I like to think of it in terms of owners / possessions and where this capability is implemented.

My $0.02 here: The Top Down interface should be called ERC998TokenPossessor. It is evident a "token possessor" contract would implement the capability for it to possess tokens. Similarly, the Bottom Up interface should be called ERC998TokenPossession - again it is clearer that a "token possession" contract would implement functionality for it to be possessed by a token. This is also more consistent with the overall idea of NFT Token Possessions. In the hybrid / combined ERC998 case, it is conceptually more meaningful for a contract to be a "token possessor" and a "token possession" simultaneously than being a both top-down/bottom-up or a parent/child.

nathalie-ckc commented 6 years ago

@vernon99, you might also find this write-up by @mudgen to be helpful in explaining top down vs bottom up: https://medium.com/@mudgen/top-down-and-bottom-up-composables-whats-the-difference-and-which-one-should-you-use-db939f6acf1d

vernon99 commented 6 years ago

I think this is a very confusing decision. What you call bottom-up is not really a composable in a true sense. Simply because the information about the ownership is distributed here between multiple token contracts. There's no single point of truth. Basically if you have a container item this item doesn't know anything about its children, the children do know, but independently from each other! That leads to conflicting opinions, much more complicated logic for atomic swaps, batch calls, etc. All the pros for TopDown given in the medium article are solvable within that framework, no need to invent an inverted model here.

I also think it's a bad idea to mix those two things in a single standard. A few days back I pointed to a developer to this standard since I remember the previous version was pretty clear and functional. The guy gets back to me and of course he used the wrong one! Took us some time to rework it. The designs are dramatically different.

mudgen commented 6 years ago

@vernon99 I apologise for the names/designs and code changing a lot on you. I understand how can that can be disorienting and disconcerting.

Here's some data about the design/code changes:

The bottom-up approach solves a problem the top-down approach can't which is enable a regular ERC721 token to own a token. I think this an important capability that people and companies will want. Cryptokitties is already using a bottom-up approach with Kitty Hats.

Everything is solved and there are no major problems if we are only ever dealing just with top-down composables. But I think people will want to use regular ERC721 tokens with composables so we need to design for that and that is where the problems crop up and why we have a top-down approach and a bottom-up approach. Each approach handles regular ERC721 tokens (ERC721 tokens that are not composables) differently.

There are two problems with top-down composables:

  1. A top-down composable cannot be owned by a regular ERC721 token (an ERC721 token that is not a composable).
  2. Any arbitrary function cannot be called on regular ERC721 contracts for ERC721 tokens when they are child tokens of a top-down composable. The reason is because they are directly owned by the top-down composable contract not the user so authenticating msg.sender fails.

If we don't have bottom-up composables then how should we address these problems?

If you are only interested in the top-down approach then you can use that and ignore the bottom-up approach. Anyone can use only one approach or both.

Here is an initial draft of the ERC998 standard: https://github.com/mudgen/EIPs/blob/master/EIPS/eip-998.md

I am hoping that things settle down soon and there aren't any more drastic changes.

mudgen commented 6 years ago

Maybe we can have a poll about the naming for the composables.

rahul-soshte commented 2 years ago

Its a bit confusing even today. Maybe both should have really separate names, its confusing because there is a addition of a naming convention inside a naming convention, and its so big a name, that its a bit difficult to read as well.