ABI parser #115
ABI parser #115
Conversation
…abi-parsing-bugs
…abi-parsing-bugs
…abi-parsing-bugs
boa/contracts/abi/contract.py
Outdated
| @@ -48,7 +54,7 @@ def marshal_to_python(self, computation, abi_type: list[str]) -> tuple[Any, ...] | |||
| :param computation: the computation object returned by `execute_code` | |||
| :param abi_type: the ABI type of the return value. | |||
| """ | |||
| if computation.is_error: | |||
| if computation.is_error or (abi_type and not computation.output): | |||
There was a problem hiding this comment.
i don't really like the truthy uses here. actually it seems the issue is that abi decoding could fail, right? can we skip this check and then just catch the exception from abi_decode() a few lines down? that would handle other abi decoding errors too, not sure if that's a good thing or not.
There was a problem hiding this comment.
This is to raise proper errors when there is no bytecode deployed in that address.
This test:
If we don't do this here, you get a DecodeError: Error decoding '0x' as '(uint8)' - Value length is not the expected size of 32 bytes - which gives no information to the user.
With this check the error changes to BoaError: (unknown location in <crvusd_abi.json interface at 0x0000000000000000000000000000000000000000 (WARNING: no bytecode at this address!)> crvusd_abi.decimals() -> ['uint8'])
I think using truthy here makes sense, since it doesn't matter whether abi_type/computation.output are None or ''
boa/util/abi.py
Outdated
|
|
||
|
|
||
| def is_abi_encodable(abi_type: str, data: Any) -> bool: | ||
| return is_encodable(abi_type, data) |
There was a problem hiding this comment.
how does is_encodable work?
There was a problem hiding this comment.
Well this function is just a wrapper around the eth_abi function as you can see.
However, I'm looking into their implementation and they just try and catch 😓
So I think it makes sense to stick with eth.codecs and do the try/catch ourselves
boa/contracts/abi/function.py
Outdated
| case multiple: | ||
| raise Exception( | ||
| f"Ambiguous call to {self.name}. " | ||
| f"Arguments can be encoded to multiple overloads: " | ||
| f"{', '.join(f.signature for f in multiple)}." | ||
| ) |
There was a problem hiding this comment.
hmm will it be possible to call functions with multiple overloads?
There was a problem hiding this comment.
In this implementation, it's possible only if there is a single matching overload for which the arguments are encodable.
When there're multiple overloads matching the arguments it's impossible, as this test shows:
titanoboa/tests/unitary/test_abi.py
Line 62 in 2ca7e30
The approach we initially discussed of comparing exact types was not possible since we are calling the ABI from Python, so we don't have the distinct types from the EVM.
There was a problem hiding this comment.
i think it might be useful to be able to force boa to choose one of the functions
There was a problem hiding this comment.
but this is kind of a corner case
boa/contracts/abi/abi_contract.py
Outdated
| :param abi_type: the ABI type of the return value. | ||
| """ | ||
| # when there's no contract in the address, the computation output is empty | ||
| is_missing_output = bool(abi_type and not computation.output) |
There was a problem hiding this comment.
i think we should handle the exception as a general abi decode handler
There was a problem hiding this comment.
and probably not go thru the handle_error machinery because that expects to come from a VM error
tests/unitary/test_abi.py
Outdated
|
|
||
|
|
||
| @pytest.fixture() | ||
| def load_via_abi(): |
There was a problem hiding this comment.
this can just be a utility function, not a fixture
Fixes #106
What I did
How I did it
By writing a new ABI parser
How to verify it
Tests included
Description for the changelog
Cute Animal Picture