Skip to content

feat: Create endpoint manager for websockets #251

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

Open
wants to merge 3 commits into
base: plat-6492-add-network-and-models
Choose a base branch
from

Conversation

shahnami
Copy link
Member

Summary

https://linear.app/openzeppelin-development/issue/PLAT-6659/refactor-rpc-calls-to-use-substrate-client-with-websocket

  • Moves http and websocket related logic into their own folders
  • Creates websocket specific endpoint manager and transport
  • Creates midnight specific websocket client
  • Adds integration tests for websockets

TODO (in follow up PRs):

  • Replace HTTP Midnight client with WS Midnight client entirely
  • Replace midnight_jsonBlock and midnight_decodeEvents RPC calls with substrate client calls
  • Update documentation

Testing Process

Checklist

  • Add a reference to related issues in the PR description.
  • Add unit tests if applicable.
  • Add integration tests if applicable.
  • Add property-based tests if applicable.
  • Update documentation if applicable.

_retry_policy: ExponentialBackoff,
_retry_strategy: Option<TransientErrorRetryStrategy>,
) -> Result<(), anyhow::Error> {
Err(anyhow::anyhow!(
Copy link
Contributor

@NicoMolinaOZ NicoMolinaOZ May 26, 2025

Choose a reason for hiding this comment

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

Do we need this to maintain consistency with other transport implementations?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's right, it implements BlockchainTransport where we only need get_current_url and send_raw_request.

We don't need retry policy logic and update_endpoint_manager because our websocket implementwtion doesn't have a middleware client, as opposed to http implementation, which has the reqwest middleware client. Mainly because of how connections are handled in http vs websockets.

As you can see, we don't actually have a client associated in the endpoint manager for ws to update. It simply manages rotation based on weights. Whereas for http it's a bit more complex, where we can configure different retry strategies based on error codes etc.. I hope that makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense! thanks for the explanation Nami!

Copy link
Contributor

@NicoMolinaOZ NicoMolinaOZ left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants