Function ABI Utilities #3408
Merged
Function ABI Utilities #3408
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
reedsa
force-pushed
the
abi-function-utils
branch
6 times, most recently
from
6f13b94 to
3a6f31a
Compare
reedsa
pushed a commit
to reedsa/web3.py
that referenced
this pull request
May 29, 2024
This was referenced May 30, 2024
Closed
reedsa
pushed a commit
to reedsa/web3.py
that referenced
this pull request
May 31, 2024
reedsa
force-pushed
the
abi-function-utils
branch
from
179f614 to
24faed3
Compare
reedsa
pushed a commit
to reedsa/web3.py
that referenced
this pull request
May 31, 2024
reedsa
force-pushed
the
abi-function-utils
branch
from
24faed3 to
88cabbb
Compare
fselmo
reviewed
May 31, 2024
Collaborator
fselmo
left a comment
fselmo
left a comment
There was a problem hiding this comment.
At first pass the code looks good. I just have a big lingering question about the organization of utils methods. I think my inline comments will be clear enough but I was under the impression we were moving things to eth_utils and re-exposing them in a centralized spot where all abi utils can be located, even the in-house web3 ones.
I'll take a second pass once I'm caught up on that.
reedsa
force-pushed
the
abi-function-utils
branch
from
b6c0dc8 to
23da131
Compare
reedsa
pushed a commit
to reedsa/web3.py
that referenced
this pull request
Jun 3, 2024
reedsa
force-pushed
the
abi-function-utils
branch
from
23da131 to
6b7ab3a
Compare
reedsa
pushed a commit
to reedsa/web3.py
that referenced
this pull request
Jun 3, 2024
reedsa
force-pushed
the
abi-function-utils
branch
from
6b7ab3a to
57f2f96
Compare
reedsa
pushed a commit
to reedsa/web3.py
that referenced
this pull request
Jun 3, 2024
reedsa
pushed a commit
to reedsa/web3.py
that referenced
this pull request
Jun 4, 2024
reedsa
force-pushed
the
abi-function-utils
branch
from
57f2f96 to
a0642d5
Compare
reedsa
pushed a commit
to reedsa/web3.py
that referenced
this pull request
Jun 4, 2024
reedsa
force-pushed
the
abi-function-utils
branch
from
a0642d5 to
8a0d5a8
Compare
reedsa
pushed a commit
to reedsa/web3.py
that referenced
this pull request
Jun 4, 2024
reedsa
force-pushed
the
abi-function-utils
branch
from
8a0d5a8 to
83d55dd
Compare
reedsa
force-pushed
the
abi-function-utils
branch
from
b09d81a to
b1b5057
Compare
reedsa
pushed a commit
to reedsa/web3.py
that referenced
this pull request
Jul 26, 2024
reedsa
force-pushed
the
abi-function-utils
branch
from
b1b5057 to
b03eac2
Compare
fselmo
reviewed
Jul 29, 2024
Collaborator
fselmo
left a comment
fselmo
left a comment
There was a problem hiding this comment.
lgtm overall. Some of these are things to think about and discuss before merging, others are minor tweaks / catches.
web3/_utils/contracts.py
Outdated
| fn_identifier: Union[str, Type[FallbackFn], Type[ReceiveFn]], | ||
| contract_abi: Optional[ABI] = None, | ||
| fn_abi: Optional[ABIFunction] = None, | ||
| fn_abi: Optional[ABIElement] = None, |
Collaborator
There was a problem hiding this comment.
Is there a reason we opened this type up? Is that desirable / is there no way to keep it within the function scope as intended? Should this be ABICallable?
Contributor
Author
There was a problem hiding this comment.
I think I recall changing this to encompass constructor, fallback and receive but you are right this should be ABICallable. That will affect a couple parts of this function so I'll get those fixed!
reedsa
force-pushed
the
abi-function-utils
branch
from
b03eac2 to
9bac5d7
Compare
reedsa
force-pushed
the
abi-function-utils
branch
from
9bac5d7 to
cc2d5b7
Compare
fselmo
reviewed
Jul 29, 2024
fselmo
reviewed
Jul 29, 2024
reedsa
force-pushed
the
abi-function-utils
branch
from
cc2d5b7 to
4853df5
Compare
reedsa
force-pushed
the
abi-function-utils
branch
from
4853df5 to
165f20f
Compare
reedsa
force-pushed
the
abi-function-utils
branch
from
165f20f to
689245a
Compare
reedsa
force-pushed
the
abi-function-utils
branch
from
689245a to
42665a1
Compare
reedsa
force-pushed
the
abi-function-utils
branch
from
42665a1 to
ecf6d77
Compare
reedsa
force-pushed
the
abi-function-utils
branch
from
27d55b2 to
be38b81
Compare
fselmo
approved these changes
Jul 31, 2024
Collaborator
fselmo
left a comment
fselmo
left a comment
There was a problem hiding this comment.
lgtm! Just left 1 rename nit.
web3/contract/base_contract.py
Outdated
| args = args or tuple() | ||
| kwargs = kwargs or {} | ||
|
|
||
| fn_info = get_abi_element_info( |
Collaborator
There was a problem hiding this comment.
I think this fn_info could be element_info for clarity, since we left the function filtering logic out of this method.
reedsa
force-pushed
the
abi-function-utils
branch
from
be38b81 to
e6a824c
Compare
added 2 commits
July 31, 2024 10:10
* Filter elements in a contract's ``ABI``. * Extract function and event ``ABI`` attributes from a contract. * Validate arguments passed to a function match the types in the ``ABI`` * Refactor logic and variable naming - Replace arguments named ``fn_name`` with ``abi_element_name`` - Rename `FunctionIdentifier` to `ABIElementIdentifier` - Remove redundant logic and `Optional` types - Integrate new ABI types in ``eth-typing``.
reedsa
force-pushed
the
abi-function-utils
branch
from
e6a824c to
70340eb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was wrong?
Function ABI utilties are needed for some use cases, so exposing them as public methods will allow people to work with ABIs and encode/decode function arguments.
Related to Issue #3036, #1596
Dependent upon ethereum/eth-utils#271.
How was it fixed?
Implement functions for parsing and validating contract data using the ABI spec.
Public ABI methods in
web3.utils.abi:check_if_arguments_can_be_encoded,get_abi_element_info,get_abi_element,get_event_abi,get_event_log_topics,log_topic_to_bytes,Additionally, all
eth_utils.abifunctions are included as part of theweb3.utilsmodule.Todo:
Cute Animal Picture