Handling Dynamic Data Sources with unknown ABIS

[davekaj]

Right now we can't run dynamic data sources for reading symbol() and decimal() and name() for all the tokens on Uniswap.

This is because Uniswap does not do any checking to see what contracts are stored in its Factory contract as a "Uniswap Exchange." This has led to the fact that the Graph Node will crash when trying to read symbol and decimal.

The feature I am requesting is this: have functionality in the mappings to call into a contract and see if a function exists. It might look something like this:

import {functionExists} from '@graphprotocol/graph-ts'
import ......

export function handleNewExchange(event: NewExchange): void {
  let id = event.params.exchange.toHex()
  let exchange = new Exchange(id)
  let erc20contract = ERC20.bind(event.params.token)

  if (erc20contract.functionExists("symbol")) {
    exchange.decimal = erc20contract.symbol()
  } else {
    exchange.decimal = null
  }
}

This is causing a blocker on Uniswap as I can't index all the exchanges for their symbol and decimal and these are important for the data, otherwise it gets confusing what you are looking at. I could hard code them all, but thats over 250 exchanges, and it isn't a long term solution.

[Jannis]

First thoughts on this:

• erc20contract.symbol() wouldn't work if the ERC20 ABI doesn't have symbol. We can't generate code for functions that don't exist.

• functionExists("symbol") won't be good enough, because symbol may have different signatures in different contracts. It would have to be .hasFunction("symbol()"), so a check that includes the full function signature.

• I'm not sure yet how we'd even detect whether a contract on chain has a certain function. Is that even possible? If not, how about a different approach:

  let result = erc20contract.tryCall("symbol", []) // Parameters are Value[]
  if (result !== null) 
    exchange.symbol = result.toString()
  }

  The tryCall function would have to return a low-level Value because we have no type hints without knowing the ABI.

• There is another option: Generate try<FunctionName> variants for all functions in all ABIs, add an ABI that includes a definition for symbol(), and then do

  let contract = WithSymbol.bind(event.address)
  exchange.symbol = contract.trySymbol() // returns `null | string`

@leodasvacas Since we've both worked on the TS API a bunch, what do you think?

[leoyvens]

@Jannis I searched a bit, to implement functionExists we'd need to somehow get the ABI, which doesn't seem possible in this case. A alternative would be do to bytecode analysis somehow, which would be complicated and probably unreliable. Which leaves us with your suggestion of trying to call the method.

My concern with trying to call is whether we can know if the call failed for a deterministic reason. If we cannot tell transient failures from a missing method failure, that API would bring up the same determinism issue we have with IPFS. But it's probably possible to tell if the web3 error was a network failure or an error returned by the ethereum node, which should be good enough for us.

I don't think codegen bloat is a concern for us, so my preference is for generating try variants for all calls.

[davekaj]

Can we push this up the list and get it done this week? The plan right now is to show the updated Uniswap subgraph to them next Monday. Let's figure this out tomorrow morning!

[leoyvens]

I did a quick test to see what sort of error eth_call returns, and it simply returns 0x on an invalid call. The reasons for this can be non-deterministic. I don't see any tools we can use to reliably and deterministically tell if a contract implements a given method.

@davekaj we need a source of truth for which contracts implement these interfaces, be it hardcoded, in IPFS or on-chain. Ideally Uniswap would have interfaces for those methods and mandate that the exchanges implement ERC-165, but that doesn't seem to be the case.

[davekaj]

Yeah, I see your point.

It would be useful if Uniswap had used ERC-165.

Is it possible to still try and call the function, and if it fails - we just leave it out, even though it is non-deterministic? I am wondering for this reason:

• I would guess that 277/280 of the contracts DO have symbol(). Whether or not it fails, or succeeds, I can write the mappings to handle both. The problem is the graph node just crashes.
• I could get a source of truth for IPFS or On-chain, or hardcode the 280 myself, but then it wont be able to dynamically add contracts. I just want to run that function call, and if it doesn't return a value, not have the graph node crash, so I can handle it

Is this possible? Or can we not bring in this non-determinism into the graph node?

[leoyvens]

@davekaj The only precedent for successful, non-deterministic indexing is ipfs.cat. To decide for another exception here, a wider discussion would be necessary.

[davekaj]

Okay cool, so the question now becomes:

• Can we have this wider discussion this week and come up with a solution that works before Monday when presenting to uniswap. (Also dependant on how long it takes us to implement what we decide)
• Or do I just work around it for now. I can work around it - it would make it the subgraph only slightly less good. But it would still be the best way to get data on Uniswap out there so I would be okay releasing it.

I am going to relay this back in the engineering chat so we can decide quickly

[davekaj]

Actually I've changed my mind - the subgraph will still be very good, its just a small implementation detail. I can work around it, and then a small update can be pushed in the future. We have pushed small updates to Compound so it is normal for this to happen.

So I am going to say we do not have to figure this out by monday, but we still have to figure it out - the sooner the better though!

[NoahZinsmeister]

👋🏻 noah from uniswap here, just read through the thread, please let me know if there's anything i can do to help on my end!

[leoyvens]

initially I thought we couldn't solve this since the Geth RPC response is too generic, but depending on what Parity responds it might be possible to solve this.

[leoyvens]

It turns out that, at least for Parity, the response for a missing method is the same as when the call reverts, so this was the same as #1139 which is fixed. Yay!