Cached defs will fail with recursive call patterns

[alex-miller-0]

We initially implemented decoder caching in #53, but there is an issue that will present itself with recursive definitions, namely that all nested ABIs remain associated to a single parent selector. The purpose of e.g. multicall is to map a single contract call (multicall def) to a set of nested contract calls (nested defs). If one instance of multicall is cached with nested defs A and B, but then a second request is made to multicall with defs A and C nested, the keyring will still pull up multicall from the cache, but the request will fail to decode on the Lattice and, I believe, will be automatically rejected by firmware.

Unfortunately there is no simple solution to this issue, since tx.input does not innately map to def -- you need an ABI definition for this. For recursive defs, you need multiple ABI definitions.

I believe the only way to really handle this is to cache select ABI data as well. We need some way to sanity check that the cached def will actually decode tx.input. Unless we want to build a new decoder in TS (we could), the only other solution would be to cache ABI definitions themselves (just for the function(s) being called).

In any event, we cannot release v0.12.1 as it will result in big problems for people using these patterns. We may want to revert #53 and push a new v0.12.2.

[douglance]

In the past, I've done similar caching in the data layer. If we move caching into the data layer between our service and etherscan, 4byte, etc. then all calls to block explorers that use the same route, would return the same result instantly (assuming the call is in the cache).

We don't have to worry about any other logic that operates against the cache, because we know that that interface is static. The data returned never changes, so we don't have to worry about breaking the cache.

[alex-miller-0]

Just had the thought that we can just reconstruct a function def string using the def returned from fetchCalldataDecoders and then use that to instantiate an Interface on ethers, then attempt to decode tx.input with that result.

https://docs.ethers.io/v5/api/utils/abi/interface/

Specifically something like this:

let def = getCachedDef(tx)
const canonicalTypesArr = convertDefToCanonicalTypesArr(def) // produces e.g. ['uint256', 'string']
const coder = new AbiCoder();
try {
  coder.decode(canonicalTypesArr, '0x' + tx.input.toString('hex'))
} catch (err) {
  // I think this throws an error?
  def = null
}

if (!def) {
  def = await fetchCalldataDecoder(...)
}

Edit: I think we already have this function in src/__test__/utils/ethers.ts: convertDecoderToEthers. If that works, we should just move it to utils.ts and export it as a public method.

[alex-miller-0]

If we move caching into the data layer between our service and etherscan, 4byte, etc.

That's fine but we don't currently have a service that can do this and we don't have SDK functionality to handle one. It also doesn't really solve the problem in this issue because we still need some way to map a def to something we can use to attempt decoding of tx.input.

[douglance]

we don't currently have a service

Sorry--"service" is the wrong term. I meant the SDK in this case.

we still need some way to map a def to something we can use to attempt decoding of tx.input.

I'm confused. the process as I understand is:

1. fetch ABI data from https://api.etherscan.io/{address}
2. parse result and if there is nested abi data, then fetch those nested contracts
3. use tx.input to transform fetched data into what the Lattice expects.

The fetching of data doesn't accept any params or other options. It's just a simple GET to the routes, and the data fetched never changes. So if those fetches are cached, then the system will treat it as if it did fetch them. It would give the same result as if the cache didn't exist.

Where am I not understanding?