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

Add ens name-to-address support for eth_subscribe method #3066

Merged
merged 2 commits into from
Aug 2, 2023

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Jul 26, 2023

What was wrong?

  • eth_subscribe request for "logs" takes an optional dict arg that can include an address param. This case isn't supported by the usual flow for the rpc-abi request formatters because those parse either sequence args to methods or dict args, but not dict args within list sequences... e.g. ("logs", {"address": "0x...", "topics": ["0x...", "0x..."]})

How was it fixed?

  • Parse this special case differently in the middleware for async ENS name-to-address.

Note: It isn't ideal to not add a test here. However, we don't yet have a good test flow for eth_subscribe. Since this is still in beta, via WebsocketProviderV2, I suggest to test this locally and we can work on eth_subscribe tests separately.

Todo:

Cute Animal Picture

MidJourney's interpretation of "a websocket as an animal":

Screenshot 2023-08-01 at 15 52 38

fselmo added a commit to fselmo/web3.py that referenced this pull request Jul 26, 2023
fselmo added a commit to fselmo/web3.py that referenced this pull request Aug 1, 2023
@fselmo fselmo force-pushed the eth_subscribe-name-to-address-support branch from d8294a3 to d235de2 Compare August 1, 2023 21:44
@fselmo fselmo marked this pull request as ready for review August 1, 2023 21:54
@fselmo fselmo requested review from pacrob and wolovim August 1, 2023 21:54
Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

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

lgtm!

- Add special case for eth_subscribe logs arg for ENS name-to-address middleware
@fselmo fselmo force-pushed the eth_subscribe-name-to-address-support branch from d235de2 to 23a3f12 Compare August 2, 2023 16:00
@fselmo fselmo merged commit 5ee7844 into ethereum:main Aug 2, 2023
84 checks passed
@fselmo fselmo deleted the eth_subscribe-name-to-address-support branch August 2, 2023 16:20
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.

None yet

3 participants