disasm: Improve stateMutability, payable, and other modifier metadata reliability

[zilayo]

When using abiFromBytecode many pure/view functions are returning payable: true stateMutability: 'payable' when tested on different addresses.

Similarly, many non-payable (but state changing) functions are returning payable: false stateMutability: 'view'

I haven't been able to narrow down why this occurs yet - it seems to behave differently depending on the bytecode it checks.

It will either return ALL functions as payable: true stateMutability: 'payable' or it will return all functions as payable: false stateMutability: 'view'

For example the 'name()' selector 0x06fdde03.

On some contracts this will return:

{
  type: 'function',
  selector: '0x06fdde03',
  payable: false,
  stateMutability: 'view',
  outputs: [ { type: 'bytes' } ]
}

Whereas on others it will return:

{
  type: 'function',
  selector: '0x06fdde03',
  payable: true,
  stateMutability: 'payable'
}

A state changing function such as 'transfer(address,uint256)' - selector 0xa9059cbb

On some contracts this will return:

{
  type: 'function',
  selector: '0xa9059cbb',
  payable: true,
  stateMutability: 'payable',
  inputs: [ { type: 'bytes' } ]
}

Whereas on others it will return:

{
  type: 'function',
  selector: '0xa9059cbb',
  payable: false,
  stateMutability: 'view',
  outputs: [ { type: 'bytes' } ],
  inputs: [ { type: 'bytes' } ]
}

Example contracts returning all payable: true stateMutability: 'payable' functions:
WETH https://etherscan.io/address/0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2#code
USDT - https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7#code
Uniswap V2 Pairs - https://etherscan.io/address/0xd6c6c8f05856dc0a9dad7e69d0a704481447bf15#code
1Inch - https://etherscan.io/token/0x111111111117dc0aa78b770fa6a738034120c302
BLUR - https://etherscan.io/token/0x5283D291DBCF85356A21bA090E6db59121208b44

Example random contracts returning all payable: false stateMutability: 'view' functions:
https://etherscan.io/address/0x98497389acc3014148352f1a01fb9cf0caf44d91
https://etherscan.io/token/0x54cf61c200985678456824f46d4e5125d6f6b440

Code used to test with the 'name()' function (provider is an ethers JSON RPC Provider):

const bytecode = await provider.getCode(address)

const abi = whatsabi.abiFromBytecode(bytecode)

const nameFunction Only = abi.find((res) => res.selector === "0x06fdde03");

console.log(nameFunction)

[shazow]

Unfortunately the modifier metadata and input/output is still very unreliable, because we only traverse static jumps for now (so any code that gets touched by dynamic jumps is missed). Need to add abstract stack tracing to get dynamic jumps, which will give better data there. That's going to be a while.

For now, best to not rely on that. I'd say it's maybe 30-50% correct, anecdotally.

I figure that's better than not having it? But could be convinced otherwise.

[shazow]

Opened issues #26 and #27 to track blocking work on this.

[zilayo]

Unfortunately the modifier metadata and input/output is still very unreliable, because we only traverse static jumps for now (so any code that gets touched by dynamic jumps is missed). Need to add abstract stack tracing to get dynamic jumps, which will give better data there. That's going to be a while.

For now, best to not rely on that. I'd say it's maybe 30-50% correct, anecdotally.

I figure that's better than not having it? But could be convinced otherwise.

Agreed - having it with some unreliability is better than not having it at all. It likely doesn't have much of an impact for most current use cases.

Awesome package btw - great job!