Skip to content
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

Expose get_event_data, get_function_info utilities #3268

Closed
wants to merge 3 commits into from

Conversation

reedsa
Copy link
Contributor

@reedsa reedsa commented Mar 5, 2024

What was wrong?

Closes #1596
Relates to #3036

get_event_data, get_function_info utilities were inaccessible for public use even though many people have the need to use these functions without instantiating a Contract.

Further refactoring to be covered in #3279

How was it fixed?

Expose these functions via web3.contract.utils.

Todo:

Cute Animal Picture

Screen Shot 2024-03-05 at 2 50 20 PM

@reedsa reedsa force-pushed the expose-contract-utils branch 3 times, most recently from 470a26b to 83ca21c Compare March 7, 2024 17:55
@reedsa reedsa requested review from fselmo, pacrob and kclowes March 7, 2024 18:00
reedsa added a commit to reedsa/web3.py that referenced this pull request Mar 7, 2024
@pacrob
Copy link
Contributor

pacrob commented Mar 8, 2024

Overall great, my only question is how to obtain an event_abi, as this is the first I've heard that term. Looks like it's obtained using find_matching_event_abi, so maybe we need to expose that too? Not sure how deep the rabbit hole will go 🐰

reedsa added a commit to reedsa/web3.py that referenced this pull request Mar 11, 2024
reedsa added a commit to reedsa/web3.py that referenced this pull request Mar 11, 2024
reedsa added a commit to reedsa/web3.py that referenced this pull request Mar 11, 2024
reedsa added a commit to reedsa/web3.py that referenced this pull request Mar 11, 2024
reedsa added a commit to reedsa/web3.py that referenced this pull request Mar 11, 2024
@reedsa
Copy link
Contributor Author

reedsa commented Mar 12, 2024

There are now two additional utilities I made public to assist with the event_abi. A similar utility exists to extract a function_abi with it's name and argument signature, but those can be made public in a subsequent refactor (#3279).

find_matching_event_abi - Looks in the contract ABI for the event that correlates with the provided name.
construct_event_filter_params - Builds the keccak signatures from the provided filters and event ABI

Build filter params using `construct_event_filter_params`
Find ABIs for events and functions with `find_matching_event_abi`, `find_matching_fn_abi`
Fix imports
@fselmo
Copy link
Collaborator

fselmo commented Mar 12, 2024

Not to throw a wrench in the spokes, but before we make this public via an API, do you think this is worth a re-consideration: #2489 ?

@fselmo
Copy link
Collaborator

fselmo commented Mar 12, 2024

Not to throw a wrench in the spokes, but before we make this public via an API, do you think this is worth a re-consideration: #2489 ?

On second thought, if we can handle this strictness in the codec itself this should remain unchanged. See my comment here. That might be a more elegant solution than passing strict into every method that uses eth_abi. Although if it makes sense to here, since it is a utility method, then we can consider adding a strict kwarg to get_event_data too.

Thoughts?

@reedsa
Copy link
Contributor Author

reedsa commented Mar 13, 2024

I like the idea of putting this within the codec itself as it seems much more flexible. I think if we decide to add more contract utility functions the flag could be required for other functions that deal with events.

It looks like the decode issue could happen for any kind of log, so if that is true it would be easier to maintain in the codec.

Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is okay as a first step to expose these methods, but I think we could really improve on this API. For example, it might be nice to return an object for get_function_info instead of a tuple. It also feels clunky to be passing around the codec. Feel free to take or leave, but I think if we put these methods on the web3 instance instead, we don't need to pass the codec through. I'm curious too if you have any other ideas about where we might make the API better.


Inspect a contract function from an ABI as follows:

.. code-block:: python
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These code blocks would be really good candidates for doctests. I think there are some examples of doctests in this file, but let me know if you need more direction than that!

@reedsa
Copy link
Contributor Author

reedsa commented Mar 19, 2024

@kclowes Great ideas, I also think we can do better with these. I see several functions that deal with contract_abi's which might be useful for public consumption.

In addition to get_event_data, get_function_info and subsequent related functions I've exposed in this PR, many of the ABI helpers for functions and events seem useful too, namely those in _utils/abi.py, _utils/events.py.

I think the eth_utils.abi helpers are a little out of place too but I dont think it's worth moving those at this point.

I think we can expose the same functionality of the function and event abi helpers but with descriptive names. After an initial think through those candidates, I came up with the following:

# get all functions in a contract
fns = w3.get_contract_functions(contract_abi: ABI)

# ABIFunction
fn = fns[0]
fn_name = fn["name"]

fn_abi = w3.get_contract_function_abi(contract_abi, fn_name, fn_args, fn_kwargs)
fn_arguments = w3.get_contract_function_arguments(fn_abi, fn_args, fn_kwargs)
fn_selector = w3.get_contract_function_selector(fn_abi)
fn_signature = w3.get_contract_function_signature(fn_abi)

# get all events in a contract
events = w3.get_contract_events(contract_abi)

# ABIEvent
event = events[0]
event_name = event["event"]

event_abi = w3.get_contract_event_abi(contract_abi, event_name, event_arg_names)
event_argument_names = w3.get_contract_event_argument_names(event_abi)
event_inputs = w3.get_contract_event_inputs(event_abi, indexed=True)
event = w3.get_contract_event_log(event_abi, log_entry)

@kclowes
Copy link
Collaborator

kclowes commented Mar 20, 2024

Good ideas! I like those! This is what Ethers exposes too, for reference.

@reedsa
Copy link
Contributor Author

reedsa commented May 17, 2024

Closing this, proceeding in #3401

@reedsa reedsa closed this May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move get_event_data and get_function_info from web3._utils modules to Public API
4 participants