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

WebsocketProviderV2 #3048

Merged
merged 19 commits into from
Jul 24, 2023
Merged

WebsocketProviderV2 #3048

merged 19 commits into from
Jul 24, 2023

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Jul 19, 2023

What was wrong?

Related to Issue #2880

  • The current WebsocketProvider doesn't keep an open connection and doesn't allow for subscription support
  • Missing support for eth_subscribe

How was it fixed?

  • Started as an expirement a few months back. I put together a new version of the Websocket Provider that maintains an open connection and processes responses asynchronously using all the middleware and formatters we're used to.
  • I polished up as much as I could of that hacky version and abstracted a lot of the logic into classes, a transient async response processing cache, and kept going along the same route until it passed all the integration tests. Very open to suggestions on improvements.

Features:

  • Persistent connection async websocket provider support
  • Debug logging where appropriate around the transient async response processing cache and websocket connection
  • eth_subscribe (and eth_unsubscribe) support

Usage (from the docs added):

>>> import asyncio
>>> from web3 import AsyncWeb3
>>> from web3.providers import WebsocketProviderV2

>>> LOG = True  # toggle debug logging
>>> if LOG:
...     import logging
...     logger = logging.getLogger("web3.providers.WebsocketProviderV2")
...     logger.setLevel(logging.DEBUG)
...     logger.addHandler(logging.StreamHandler())

>>> async def ws_v2_subscription_example():
...     async with AsyncWeb3.persistent_websocket(
...         WebsocketProviderV2(f"ws://127.0.0.1:8546")
...     ) as w3:
...         # subscribe to new block headers
...         subscription_id = await w3.eth.subscribe("newHeads")
...
...         unsubscribed = False
...         while not unsubscribed:
...             async for response in w3.listen_to_websocket():
...                 print(f"{response}\n")
...                 # handle responses here
...
...                 if some_condition:
...                     # unsubscribe from new block headers
...                     unsubscribed = await w3.eth.unsubscribe(subscription_id)
...                     break
...
...         # still an open connection, make any other requests and get
...         # responses via send / receive
...         latest_block = await w3.eth.get_block("latest")
...         print(f"Latest block: {latest_block}")
...
...         # the connection closes automatically when exiting the context
...         # manager / `async with` block

>>> asyncio.run(ws_v2_subscription_example())

Todo:

  • Add entry to the release notes
  • Fix failing docs
  • Add documentation for WebsocketProviderV2
  • Fix failing EthereumTesterProvider / eth-tester based core tests

Cute Animal Picture

20230620_141634

- Allow for establishing a ws connection, leaving it open, and asynchronously sending and receiving messages
- Allow for a message stream to endlessly receive messages and pass the responses through result formatters
- Added a simple version for ``eth_subscribe`` to ``AsyncEth``

TODO:

- Still need to pass responses back through the middleware before the formatters are applied
- Refactor everything, it's all a huge mess :P
- Typing
- Docs around new websocket setup
- Docs around ``eth_subscribe`` only being used for persistent websocket connection ... add validation to the method to raise an exception if coming from any other provider maybe?
- Implement the validation to ``eth_subscribe`` that will raise an exception if a provider cannot handle subscriptions + add clear messaging for the user
- Add tests
- Refactor + clean up the transient cache and cache logic
- Move methods out of RequestManager? Allow for the provider to handle most of these cases?
- curry the response formatting for easier passing of each middleware response handler method
- make the cache class for the PersistentConnectionProvider a ``SimpleCache``
- use classes where appropriate rather than dicts for caching
- DRY the duplicate code in the persistent recv() stream and the singular ws.recv() via ws.send() by using just one AsyncGenerator and returning only the __anext__() item in the singular retrieve.
- Some cleanup along the way
- Hacky support for subscriptions with params just to test against another subscription ("logs") that isn't "newHeads"
- Huge cleanup on logic
- Move certain base class properties into the PersistentConnectionProvider class
- Wrap up some of the subscription / caching logic into private methods in the PersistentConnectionProvider
- Add a utility method for seeing if either set in a pair of sets is a subset of the other. This is useful for comparing dict keys with formatters where keys may be missing on either side of the comparison but one is always a subset of the other.
- Use this utility method to map to different formatters for parsing subscription response types.
@fselmo fselmo changed the title Websockets Provider V2 WebsocketProviderV2 Jul 19, 2023
- Create Types for eth_subscription request params and update those in the method signature
- Refactor some of the logic around formatting middleware: curry ``_apply_response_formatters()`` rather than creating a whole new method that is basically a curried version of that
- lint-roll and fix lint issues
- Handle some different caching cases, clear WebsocketProviderV2 cache after disconnecting
- Properly set up logic for eth_unsubscribe
- Pop subscription info from cache if successful unsubscribe call with the subscription id
- Fixed some things along the way. Looks liek a lot of the async setup was a bit muddied. It was necessary to create async versions of fixtures for these tests. As this is only an async provider, it wasn't able to steal any informationa at all from the sync tests since it doesn't define a ``w3`` in its module scope. I believe that's the only reason the other async tests are working.
@fselmo fselmo marked this pull request as ready for review July 19, 2023 16:30
@fselmo fselmo force-pushed the websockets-sandbox branch 2 times, most recently from b5f1d4c to aba9105 Compare July 19, 2023 17:22
@fselmo fselmo requested review from kclowes, pacrob and reedsa July 19, 2023 17:23
@fselmo
Copy link
Collaborator Author

fselmo commented Jul 19, 2023

This is ready for an initial review... sorry for the PR size here 😔

cc: @DefiDebauchery

fselmo added a commit to fselmo/web3.py that referenced this pull request Jul 19, 2023
- Add web3.providers.websocket.rst to .gitignore
fselmo added a commit to fselmo/web3.py that referenced this pull request Jul 20, 2023
- Add web3.providers.websocket.rst to .gitignore
- Put back formatters for eth_getCode / remove unnecessary ``compose()``
- Add read-friendly comment splitting Web3 from AsyncWeb3 in main.py
- Use correct class name in docstring
- Friendlier message when exception is raised connecting to websocket endpoint
- Friendlier message for websocket restricted_kwargs; use a merge of default + provided websocket_kwargs with the provided values taking precedence
- Validate "ws://" / "wss://" in websocket endpoint
fselmo added a commit to fselmo/web3.py that referenced this pull request Jul 20, 2023
- Add web3.providers.websocket.rst to .gitignore
- Put back formatters for eth_getCode / remove unnecessary ``compose()``
- Add read-friendly comment splitting Web3 from AsyncWeb3 in main.py
- Use correct class name in docstring
- Friendlier message when exception is raised connecting to websocket endpoint
- Friendlier message for websocket restricted_kwargs; use a merge of default + provided websocket_kwargs with the provided values taking precedence
- Validate "ws://" / "wss://" in websocket endpoint
docs/providers.rst Outdated Show resolved Hide resolved
- Add web3.providers.websocket.rst to .gitignore
- Put back formatters for eth_getCode / remove unnecessary ``compose()``
- Add read-friendly comment splitting Web3 from AsyncWeb3 in main.py
- Use correct class name in docstring
- Friendlier message when exception is raised connecting to websocket endpoint
- Friendlier message for websocket restricted_kwargs; use a merge of default + provided websocket_kwargs with the provided values taking precedence
- Validate "ws://" / "wss://" in websocket endpoint
- Use Dict[str, Any] for websocket_kwargs type
Copy link
Contributor

@reedsa reedsa left a comment

Choose a reason for hiding this comment

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

🚢 Nothing stands out in my mind, lgtm! Will be curious to see adoption feedback and see if there are challenges migrating to v2.

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.

This looks good to me from a code perspective! I'll take some time on Monday to play around with the UI. It'll be awesome to get this in!

web3/providers/async_base.py Show resolved Hide resolved
fselmo added a commit to fselmo/web3.py that referenced this pull request Jul 24, 2023
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.

Left a nit on the docs, but looks great otherwise!

docs/providers.rst Outdated Show resolved Hide resolved
@fselmo fselmo merged commit 75caec1 into ethereum:main Jul 24, 2023
1 check passed
fselmo added a commit that referenced this pull request Jul 24, 2023
fselmo added a commit that referenced this pull request Jul 24, 2023
- Add web3.providers.websocket.rst to .gitignore
- Put back formatters for eth_getCode / remove unnecessary ``compose()``
- Add read-friendly comment splitting Web3 from AsyncWeb3 in main.py
- Use correct class name in docstring
- Friendlier message when exception is raised connecting to websocket endpoint
- Friendlier message for websocket restricted_kwargs; use a merge of default + provided websocket_kwargs with the provided values taking precedence
- Validate "ws://" / "wss://" in websocket endpoint
- Use Dict[str, Any] for websocket_kwargs type
@fselmo fselmo deleted the websockets-sandbox branch July 24, 2023 21:52
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.

4 participants