Contract Events APIs #3472

reedsa · 2024-08-30T23:00:21Z

What was wrong?

Contract function classes provide functions for extracting contract components. Contract event API methods do not yet exist. The first set of methods for find_events_by_identifier and get_event_by_identifier should be added.

Contract Event APIs should be provided, namely all_events, find_events_by_name and get_event_by_name.

How was it fixed?

Implement functions for Contract classes to find many events or getting a single event by identifier, find_events_by_identifier and get_event_by_identifier.

Additional Event APIs are now available (all_events, find_events_by_name, get_event_by_name, find_events_by_signature, get_event_by_signature, find_events_by_selector, get_event_by_selector, find_events_by_topic, and get_event_by_topic)

Todo:

Clean up commit history
Add or update documentation related to these changes
Add entry to the release notes

Cute Animal Picture

fselmo

This will be a great addition to get in. Currently as it stands, if I have an ABI that has both a Deposit event and a Deposited event, and I try to use contract.get_event_by_name() with either "Deposit" or "Deposited", I can get neither event. Instead, I get:

Web3ValueError: Could not find any event with matching name

I don't think this is desired behavior. Those are not ambiguous but distinct names. Can we add a test for something like this and make sure it passes? I saw a few nits in message wording, feel free to take or leave those as I see it in the function messages as well. I'll take another pass here once we get that test in.

fselmo · 2024-10-11T17:31:25Z

web3/contract/utils.py

@@ -365,6 +373,46 @@ def get_function_by_identifier(
    return fns[0]


+def find_events_by_identifier(


nit: Should we rename this to get_events_by_identifier since the singular is get_event_by_identifier?

There's a convention that follows the find_* used for lookups which return a list of items, and get_* when expecting a single result. The get method takes the resulting list from the find and just returns a single element (if there is not a single result in the list, the get method raises an exception).

fselmo · 2024-10-11T17:32:42Z

web3/contract/utils.py

+    """
+    if len(events) > 1:
+        raise Web3ValueError(
+            f"Found multiple events with matching {identifier}. " f"Found: {events!r}"


nit: use "identifier" in the message for clarity

Suggested change

f"Found multiple events with matching {identifier}. " f"Found: {events!r}"

f"Found multiple events with matching identifier `{identifier}`: {events!r}"

I think identifier is a little confusing because we use abi_element_identifier to represent the name of the ABIFunction or ABIEvent.

In this context, identifier is a string that represents the type of "check" passed in. For example, get_function_by_signature uses find_functions_by_identifier by passing a "check" function that retrieves all functions from the ABI which passes the check. Then the resulting list of ABI components is passed to get_function_by_identifier to ensure there is only one result in the list and returns the ABI component itself. If not, an exception is raised, and the identifier is "signature", indicating the search type.

fselmo · 2024-10-11T17:33:02Z

web3/contract/utils.py

+            f"Found multiple events with matching {identifier}. " f"Found: {events!r}"
+        )
+    elif len(events) == 0:
+        raise Web3ValueError(f"Could not find any event with matching {identifier}")


nit

Suggested change

raise Web3ValueError(f"Could not find any event with matching {identifier}")

raise Web3ValueError(f"Could not find any event with matching identifier `{identifier}`")

fselmo · 2024-10-11T17:34:07Z

web3/contract/utils.py

+    """
+    Check that the provided list of TContractFunction instances contains one element and
+    return it.
+    """
    if len(fns) > 1:
        raise Web3ValueError(
            f"Found multiple functions with matching {identifier}. " f"Found: {fns!r}"


I see now we already phrase it this way with functions... take or leave really. Tiniest of nits.

Suggested change

f"Found multiple functions with matching {identifier}. " f"Found: {fns!r}"

f"Found multiple functions with matching identifier `{identifier}`. Found: {fns!r}"

kclowes

This is looking good to me once the Deposit/Deposited edge case that @fselmo found is cleaned up. Those doctests are super nice, thanks for getting those in. I just left a few comments!

kclowes · 2024-10-11T15:30:51Z

docs/web3.contract.rst

@@ -232,12 +248,13 @@ Each Contract Factory exposes the following methods.

    Returns the transaction hash for the deploy transaction.

-    .. code-block:: python
+    .. doctest:: contractmethods


Nice work with this!

kclowes · 2024-10-11T18:42:32Z

tests/core/contracts/test_extracting_event_data.py

@@ -545,14 +545,14 @@ def test_contract_event_get_logs_sorted_by_log_index(w3, emitter, request_mocker
    ]

    with request_mocker(w3, mock_results={"eth_getLogs": get_logs_response}):
-        logs = emitter.events.LogNoArguments().get_logs()


Do we have any tests to make sure that an event called with parens still passes? I'm fine taking them out here, but it might be good to have an explicit test to make sure it still works since it's documented. And the parens/no parens might be worth removing in v8. It feels antithetical to the zen of python, and I don't think there is a whole lot of value add in being able to call events with both parentheses and without. Although I can see the argument for maintaining consistency with ContractFunctions /shrug

kclowes · 2024-10-11T19:36:34Z

web3/contract/base_contract.py

+        Raises a Web3ValueError if the signature is invalid or if there is no match or
+        more than one is found.
+        """
+        if " " in signature:


Should we just strip out the spaces instead of raising here?

I pulled this logic from get_function_by_signature but it might be better to strip out spaces in the long run. I can strip those in get_event_by_signature for now.

kclowes · 2024-10-11T20:38:41Z

docs/web3.contract.rst

+        >>> contract.get_event_by_name('Approval')
+        <Event Approval(address,address,uint256)>
+
+.. py:classmethod:: Contract.find_events_by_selector(selector)


Does this work with the 4byte selector too or just the whole thing?

I'm not certain I've come across events that are represented by a 4byte selector, so this is implemented to match the whole event signature encoding. If it makes sense to use the 4byte selector I can update this to match the function selector.

Oh, right. This is events. I think we can just leave it as-is. If we get a request to change it later, we can.

Contract utils for `get_event_by_name`, `get_event_by_signature`, `get_event_by_selector`, `get_event_by_topic` Contract utils for `find_events_by_name`, `find_events_by_signature`, `find_events_by_selector`, `find_events_by_topic`

reedsa · 2024-10-18T16:33:48Z

@fselmo I havent reproduced the error you saw but added tests to cover those cases for both functions and events. Please let me know what the ABI was that you used and maybe there's an edge case I'm missing. Thanks!

kclowes

Looks good to me! Just left a nit or two, and some typing suggestions/clarifications 🚀

kclowes · 2024-10-18T17:19:05Z

web3/contract/async_contract.py

+        address: ChecksumAddress,
+        callable_check: Callable[..., Any],
+    ) -> List["AsyncContractEvent"]:
+        return cast(


I think this is a redundant cast

kclowes · 2024-10-18T18:23:54Z

web3/contract/base_contract.py

-        return PropertyCheckingFactory(class_name, (cls,), kwargs)
+    def factory(
+        cls, class_name: str, **kwargs: Any
+    ) -> Union["ContractEvent", "AsyncContractEvent"]:


I wonder if this should be TContractEvent?

I found that using TContractEvent requires one argument for factory to be of that same type.

Ah, okay. Thanks for looking into it!

kclowes · 2024-10-18T18:28:27Z

web3/contract/base_contract.py

@@ -956,6 +1121,26 @@ def get_function_by_identifier(
            "This method should be implemented in the inherited class"
        )

+    @combomethod
+    def find_events_by_identifier(


nit:find/get_functions/events_by_id are all beneath the # Private helpers comment. Consider moving above 🤷

kclowes · 2024-10-18T18:29:27Z

web3/contract/contract.py

+        address: ChecksumAddress,
+        callable_check: Callable[..., Any],
+    ) -> List["ContractEvent"]:
+        return cast(


redundant cast

reedsa force-pushed the contract-event-api branch from 4ae2950 to e1eabd2 Compare August 30, 2024 23:38

reedsa force-pushed the contract-event-api branch from e1eabd2 to 711f58a Compare October 2, 2024 18:26

reedsa changed the title ~~Event Contract APIs~~ find_events_by_identifier for Contract Events Oct 2, 2024

reedsa changed the title ~~find_events_by_identifier for Contract Events~~ Contract Events APIs Oct 2, 2024

reedsa force-pushed the contract-event-api branch from 479723f to 40cc3f9 Compare October 4, 2024 21:01

reedsa marked this pull request as ready for review October 4, 2024 21:01

reedsa force-pushed the contract-event-api branch from 40cc3f9 to 716b756 Compare October 7, 2024 18:56

reedsa added a commit to reedsa/web3.py that referenced this pull request Oct 8, 2024

Newsfragment for ethereum#3472

d4727ed

reedsa force-pushed the contract-event-api branch from 290094b to d4727ed Compare October 8, 2024 20:18

reedsa requested review from fselmo, kclowes and pacrob October 8, 2024 20:23

fselmo reviewed Oct 11, 2024

View reviewed changes

kclowes reviewed Oct 11, 2024

View reviewed changes

reedsa added 4 commits October 18, 2024 09:44

Contract APIs for event elements in a contract ABI

c932fac

Contract utils for `get_event_by_name`, `get_event_by_signature`, `get_event_by_selector`, `get_event_by_topic` Contract utils for `find_events_by_name`, `find_events_by_signature`, `find_events_by_selector`, `find_events_by_topic`

Newsfragment for ethereum#3472

860a8fd

Apply changes from ethereum#3479

fccc9dc

Allow optional parens on events for sync and async contracts

465def7

reedsa force-pushed the contract-event-api branch from 32c0e16 to 465def7 Compare October 18, 2024 15:45

Tests for overlapping function/event names

810529e

reedsa requested review from kclowes and fselmo October 18, 2024 16:33

kclowes approved these changes Oct 18, 2024

View reviewed changes

Corrections per feedback

5ceacde

reedsa merged commit dc02336 into ethereum:main Oct 21, 2024
71 checks passed

slundqui mentioned this pull request Nov 7, 2024

Adhere to new events api in web3py 7.5.0 delvtech/pypechain#149

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contract Events APIs #3472

Contract Events APIs #3472

reedsa commented Aug 30, 2024 •

edited

Loading

fselmo left a comment •

edited

Loading

fselmo Oct 11, 2024

reedsa Oct 17, 2024

fselmo Oct 11, 2024

reedsa Oct 18, 2024

fselmo Oct 11, 2024

fselmo Oct 11, 2024

kclowes left a comment

kclowes Oct 11, 2024

kclowes Oct 11, 2024

kclowes Oct 11, 2024

reedsa Oct 16, 2024

kclowes Oct 11, 2024

reedsa Oct 16, 2024 •

edited

Loading

kclowes Oct 16, 2024

reedsa commented Oct 18, 2024

kclowes left a comment

kclowes Oct 18, 2024

kclowes Oct 18, 2024

reedsa Oct 21, 2024

kclowes Oct 21, 2024

kclowes Oct 18, 2024

kclowes Oct 18, 2024

		@@ -365,6 +373,46 @@ def get_function_by_identifier(
		return fns[0]


		def find_events_by_identifier(

	f"Found multiple events with matching {identifier}. " f"Found: {events!r}"
	f"Found multiple events with matching identifier `{identifier}`: {events!r}"

	raise Web3ValueError(f"Could not find any event with matching {identifier}")
	raise Web3ValueError(f"Could not find any event with matching identifier `{identifier}`")

	f"Found multiple functions with matching {identifier}. " f"Found: {fns!r}"
	f"Found multiple functions with matching identifier `{identifier}`. Found: {fns!r}"

Contract Events APIs #3472

Contract Events APIs #3472

Conversation

reedsa commented Aug 30, 2024 • edited Loading

What was wrong?

How was it fixed?

Todo:

Cute Animal Picture

fselmo left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kclowes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reedsa Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reedsa commented Oct 18, 2024

kclowes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reedsa commented Aug 30, 2024 •

edited

Loading

fselmo left a comment •

edited

Loading

reedsa Oct 16, 2024 •

edited

Loading