Decoded transactions should refer to classes/entities rather than standards
raulk opened this issue · 4 comments
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.
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.
@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.
@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?
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.