Skip to content
This repository has been archived by the owner on Jan 21, 2022. It is now read-only.

Decoded transactions should refer to classes/entities rather than standards #58

Closed
3 tasks
raulk opened this issue Jul 1, 2018 · 4 comments
Closed
3 tasks

Comments

@raulk
Copy link
Contributor

raulk commented Jul 1, 2018

ethql should refer to classes or entities rather than specific standards. For example, all of ERC20, ERC223, ERC827 relate to tokens.

It is sometimes unfeasible (or hardly worthwhile) to distinguish exactly which standards a given contract implements (although it could be achieved if the contract implements ERC165, or by inspecting the bytecode). Users are generally concerned with the underlying entity.

In the future we might introduce "standards inference" as a functionality, but for now that's not the goal.

Tasks:

  • Remove all schema references to ERC20 and replace them with Token.
  • 'standard' as a field should disappear in favour of 'class' = 'token'.
  • The TX decoder for ERC20 should be renamed to TokenTxDecoder.

In the future, we'll introduce a NonFungibleTokenTxDecoder, in #33.

@pcardune
Copy link
Contributor

pcardune commented Jul 3, 2018

Do graphql interfaces factor into this at all? It seems like it might be hard to bucket multiple ERCs into one abstraction. Either way, I think ethql should expose the underlying ERC spec(s) that the contract implements.

@raulk
Copy link
Contributor Author

raulk commented Jul 3, 2018

@pcardune I've had some doubt about this.

My conclusion in that ERC specs can specify changes in behaviour that are not visible from the outside. For example ERC223 introduces a behaviour change in the transfer function to identify if the beneficiary of a transfer is a contract, in which case it invokes the tokenFallback function on that contract.

This makes it inaccurate for ethql report a contract as ERC20 when it could be implementing ERC223 too. We could consider a type ERC20_ERC223_Transfer to cover the uncertainty, but such naming will scale poorly.

Furthermore, there could be an upcoming standard (let's call it ERCnnn) that introduces a new public function foo(uint256). If in a block we observe a TX for transfer(...) and no calls to foo, we don't know if the underlying contract implements ERCnnn, so we'd have to check the bytecode which is unnecessary overhead in most cases.

Additionally we have ERC777 which uses different naming for all functions (intentionally), but the abstraction at hand is token after all. End users could see this as an implementation detail – and will likely prefer not having to create polymorphic queries to cover both ERC20 and ERC777. WDYT?

I do agree ethql should give users the ability to introspect a contract to determine possible ERC standard(s) it implements. I've come up with an algorithm for this. Was planning to work on it separately, but I could share the details if you want to collaborate.

@raulk
Copy link
Contributor Author

raulk commented Jul 6, 2018

@pcardune ping, just wanted to check if the above ^^ makes sense to you? Will be sending a PR shortly with the change. Would you like to review?

@pcardune
Copy link
Contributor

pcardune commented Jul 6, 2018

I guess the way I think about it is that providing higher level abstractions is a good thing, so long as there is an escape hatch. No matter how careful you are, your higher level abstraction is probably going to miss some detail that clients might need access to. The "escape hatch" is a mechanism for clients to get around the high level abstraction and go straight to the source. For example, solidity exposes an escape hatch by letting you write assembly inside your contracts.

So, 👍 to having a higher level token API that covers all those ERCs. But, there should still be a way to dig under the covers and use the ERC-defined APIs directly, even if that means more complex queries. Perhaps this functionality would only be available to contracts that implement ERC165.

raulk pushed a commit to raulk/ethql that referenced this issue Jul 9, 2018
An entity groups related ERC standards, providing an abstraction on what
kind of concept they refer to. In this way, different token interfaces
can co-exist while making it easy to filter for all token transactions
in a block, regardless of the underlying ERC interface (e.g. ERC20,
ERC777 or ERC20+ERC777).

Resolves Consensys#58.
raulk pushed a commit to raulk/ethql that referenced this issue Jul 9, 2018
An entity groups related ERC standards, providing an abstraction on what
kind of concept they refer to. In this way, different token interfaces
can co-exist while making it easy to filter for all token transactions
in a block, regardless of the underlying ERC interface (e.g. ERC20,
ERC777 or ERC20+ERC777).

Resolves Consensys#58.
@raulk raulk closed this as completed in #65 Jul 11, 2018
raulk pushed a commit that referenced this issue Jul 11, 2018
An entity groups related ERC standards, providing an abstraction on what
kind of concept they refer to. In this way, different token interfaces
can co-exist while making it easy to filter for all token transactions
in a block, regardless of the underlying ERC interface (e.g. ERC20,
ERC777 or ERC20+ERC777).

Resolves #58.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants