-
Notifications
You must be signed in to change notification settings - Fork 149
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
Utilities for Contract ABIs #271
base: main
Are you sure you want to change the base?
Conversation
0c6d60f
to
615e3d4
Compare
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.
I like the new test functions, very nice! I didn't go through with a fine tooth comb, but looks good overall!
The only outstanding things I saw are:
- the comment around which exceptions to raise, and where they live
- I left a comment about raising vs returning an empty list. let me know what you think!
1d18a64
to
9e2a2da
Compare
8d65e46
to
b76e640
Compare
03fe5b4
to
6e44918
Compare
6e44918
to
4b6df2c
Compare
1599e85
to
761d1c8
Compare
adfc679
to
854a6a5
Compare
Usage question - I pulled an ABI from web3 (
Same for |
…uments to function ABI 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 may look like a lot of comments but I left mostly nits, some things to discuss, and a few inconsistencies. The code looks great to me, most of it is around arg names, function names, deprecation cycle, and docstring blips.
``filter_abi_by_type`` was renamed from ``filter_by_type``. ``get_normalized_abi_arg_type`` is now called ``get_normalized_abi_component_type``. Renamed ``_abi_inputs_types`` -> ``_abi_input_types``. Updates argument names for utils, ``abi`` -> ``contract_abi``. Docstring updates to match arguments and types accurately. Also replaces ``Union[ABIFunction, ABIError, ABIFallback, ABIReceive]`` with ``ABICallable``.
I think all my concerns were addressed here. Will wait to approve once the |
4943452
to
2bf578f
Compare
What was wrong?
Contract ABI utilities are useful for encoding and decoding contract transactions. Many utilities are implemented in web3.py as private methods.
Related to ethereum/web3.py#3036
These changes will be utilized in web3.py function/event utils:
ethereum/web3.py#3408
ethereum/web3.py#3401
How was it fixed?
Implements a public API for ABI utilities.
Todo:
Clean up commit history
Add or update documentation related to these changes
Add entry to the release notes
Cute Animal Picture