-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Encode ABI Utility #3361
base: main
Are you sure you want to change the base?
Encode ABI Utility #3361
Conversation
95af2f5
to
3f5a4aa
Compare
* Use ABI types from `eth_typing` * Fix reference to `ABIEventParam` and `ABIFunctionParam` types * Import `get_abi_input_types` and `get_abi_output_types` from `eth-utils` * Rename `collapse_if_tuple` -> `get_normalized_abi_arg_type` * Import `get_abi_input_names` and `get_abi_output_names` from `eth_utils`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this needs rebasing and some conflict resolution but my review was requested so I left a first pass of comments. I think we can remove the need for is_async
altogether from the utility method and pass in appropriate normalizers
for each code block instead. I also don't think this is up to date with the latest eth-typing changes so I made some comments on using ABIComponent
and ABICallable
where appropriate. I may have missed some but in general it looks like this needs to be updated to match the current state of things there.
@@ -17,3 +16,43 @@ | |||
from .exception_handling import ( # NOQA | |||
handle_offchain_lookup, | |||
) | |||
|
|||
# from eth_utils import ( # NOQA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be addressed. Are we waiting on anything here?
|
||
|
||
def encode_abi( | ||
function_abi: Union[ABIFunction, ABIConstructor, ABIFallback, ABIReceive], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use the new ABICallable
, right?
if "inputs" not in abi and abi["type"] == "fallback": | ||
return [] | ||
return [arg["name"] for arg in abi["inputs"]] | ||
normalizers = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just have one set of normalizers, instead of data_normalizers
and normalizers
? We can probably take out the is_async
from this utility method and just be able to pass in the data normalizers
. I think that feature let's us achieve everything we want. From within the code block, the sync code can pass in all the normalizers
it needs and the async code can do the same.
:param type: `HexStr` | ||
:param arguments: Arguments used for the transaction request. | ||
:param type: `Any` | ||
:param is_async: Enable async transaction encoder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably remove this if we go with my thought below.
:param data_normalizers: List of custom normalizers to apply to the data. | ||
:param type: `list[Callable[..., Tuple[TypeStr, Any]]]` | ||
:param abi_codec: Codec used for encoding and decoding. | ||
:param type: `ABICodec` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:param type: `ABICodec` | |
:param type: `ABICodec` (optional) |
abi_bytes_to_bytes, | ||
abi_string_to_text, | ||
] | ||
if not w3.eth.is_async: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment about removing the is_async
from the utility method. I think we can just build the appropriate normalizers here in the code block instead of trying to use an is_async
flag for a general public utility method that's trying to encode abi. We can build the right normalizers for each code block and pass it in instead, one appropriate for the sync code block and another that is appropriate for the async code.
@@ -913,7 +889,7 @@ def build_strict_registry() -> ABIRegistry: | |||
|
|||
|
|||
def named_tree( | |||
abi: Iterable[Union[ABIFunctionParams, ABIFunction, ABIEvent, Dict[TypeStr, Any]]], | |||
abi: Iterable[Union[ABIFunctionParam, ABIFunction, ABIEvent, Dict[TypeStr, Any]]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
abi: Iterable[Union[ABIFunctionParam, ABIFunction, ABIEvent, Dict[TypeStr, Any]]], | |
abi: Iterable[Union[ABIComponent, ABIFunction, ABIEvent, Dict[TypeStr, Any]]], |
@@ -926,10 +902,10 @@ def named_tree( | |||
|
|||
|
|||
def _named_subtree( | |||
abi: Union[ABIFunctionParams, ABIFunction, ABIEvent, Dict[TypeStr, Any]], | |||
abi: Union[ABIFunctionParam, ABIFunction, ABIEvent, Dict[TypeStr, Any]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
abi: Union[ABIFunctionParam, ABIFunction, ABIEvent, Dict[TypeStr, Any]], | |
abi: Union[ABIComponent, ABIFunction, ABIEvent, Dict[TypeStr, Any]], |
return [arg for arg in event_abi["inputs"] if arg["indexed"] is True] | ||
|
||
|
||
def exclude_indexed_event_inputs(event_abi: ABIEvent) -> List[ABIEventParams]: | ||
def exclude_indexed_event_inputs(event_abi: ABIEvent) -> List[ABIEventParam]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def exclude_indexed_event_inputs(event_abi: ABIEvent) -> List[ABIEventParam]: | |
def exclude_indexed_event_inputs(event_abi: ABIEvent) -> List[ABIComponent]: |
@@ -155,24 +139,14 @@ def receive_func_abi_exists(contract_abi: ABI) -> List[Union[ABIFunction, ABIEve | |||
return filter_by_type("receive", contract_abi) | |||
|
|||
|
|||
def get_indexed_event_inputs(event_abi: ABIEvent) -> List[ABIEventParams]: | |||
def get_indexed_event_inputs(event_abi: ABIEvent) -> List[ABIEventParam]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def get_indexed_event_inputs(event_abi: ABIEvent) -> List[ABIEventParam]: | |
def get_indexed_event_inputs(event_abi: ABIEvent) -> List[ABIComponent]: |
What was wrong?
Related to Issue #3279
Dependent PRs:
web3.py: #3408, #3401
eth-utils: ethereum/eth-utils#271
How was it fixed?
Exposes
encode_abi
ABI utility.Todo:
Cute Animal Picture