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 WS support to cryo #65

Open
libevm opened this issue Sep 21, 2023 · 3 comments
Open

Add WS support to cryo #65

libevm opened this issue Sep 21, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@libevm
Copy link

libevm commented Sep 21, 2023

add ws support pls 🙏

--ws OR --rpc
@0xprames
Copy link

0xprames commented Sep 24, 2023

hey there - C/P'ing my messages from the TG on this

0xprames, [Sep 24, 2023 at 10:22:40 AM]:
yeah im going through the code + your PRs adding RetryClient (both the initial and the final one using ethers)

Looking at the alchemy docs for it rn and I see this w.r.t Websocket transport errors and how clients would do retries (as opposed to Http clients catching a 429 and retrying)

I think the first option you mention makes sense to unblock anyone who NEEDS a WS client asap (just creating a basic WS ethers provider w/o retry)

Looking at the alchemy docs for it rn and I see this w.r.t Websocket transport errors and how clients would do retries (as opposed to Http clients catching a 429 and retrying)

I think the first option you mention makes sense to unblock anyone who NEEDS a WS client asap (just creating a basic WS ethers provider w/o retry)

although im curious as to why a WS connection would be preferred as a user (for the existing Fetcher impl - i.e classic JsonRpc requests)
...

which confirms what Libevm was saying earlier re: no Ws support rn in ethers::RetryClient

I suspect a WsRateLimitRetryPolicy may be a bit more complex to implement given that alchemy wss screencap above.

@0xprames
Copy link

tl;dr (deleted those messages bc I spammed chat)

  • i was asking why users may want WS support given Fetcher is only making jsonrpc request rn (discussed this further in the TG and came to the conclusion that it would be faster)
  • I was saying as per alchemy docs, WS retry may be a bit more complicated than Http retries
  • confirmed what you mentioned in the TG that ethers::RetryClient does not currently support Ws:
    let ws_provider = Provider::new(RetryClient::new(
        Ws::connect(Url::parse(ws_url).unwrap()).await.map_err(|_e|ParseError::ParseError("could not connect to ws_provider".to_string()))?,
        Box::new(HttpRateLimitRetryPolicy),
        args.max_retries,
        args.initial_backoff,
    ))

gives:

error[E0277]: the trait bound `ethers::ethers_providers::HttpRateLimitRetryPolicy: RetryPolicy<WsClientError>` is not satisfied
  --> crates/cli/src/parse/source.rs:22:9
   |
22 |         Box::new(HttpRateLimitRetryPolicy),
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `RetryPolicy<WsClientError>` is not implemented for `ethers::ethers_providers::HttpRateLimitRetryPolicy`
   |
   = help: the trait `RetryPolicy<ethers::ethers_providers::HttpClientError>` is implemented for `ethers::ethers_providers::HttpRateLimitRetryPolicy`
   = note: required for the cast from `ethers::ethers_providers::HttpRateLimitRetryPolicy` to the object type `dyn RetryPolicy<WsClientError>`

@0xprames
Copy link

so i think the solution @ipatka mentioned in that TG makes sense, which is to add a basic WS client without retries, and maybe iterate on that solution (either here or in ethers.rs codebase), adding retries eventually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants