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

feat: switch from http to ws connection (eth execution client) #212

Merged
merged 3 commits into from
Sep 25, 2023

Conversation

Vid201
Copy link
Member

@Vid201 Vid201 commented Sep 21, 2023

Some other things in this PR:

  • switch from anyhow to eyre
  • update bundler spec tests to latest commit
  • update geth version from to v1.12.0

@Vid201 Vid201 changed the title feat: switch from http to ws, switch from anyhow to eyre feat: switch from http to ws connection (eth execution client) Sep 21, 2023
@Vid201 Vid201 force-pushed the feat/ws branch 2 times, most recently from 4ef3de7 to 0fe411d Compare September 21, 2023 13:07
@Vid201 Vid201 requested a review from zsluedem September 21, 2023 15:57
@Vid201 Vid201 linked an issue Sep 21, 2023 that may be closed by this pull request
Copy link
Collaborator

@zsluedem zsluedem left a comment

Choose a reason for hiding this comment

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

What if the websocket connection is lost because of network issue? Do you have plans to address that?Does ethers ws handle that internally?

The problem with websocket is that we need to provide a way when the connection is lost. If we don't have that, when the situation happens, the bundler would just crash and got no more data from websocket. As for normal http request, if one of the http request is lost, it would just got one task crash(maybe this task would crash the whole bundler. It is possible. But http leave codes easier to write)


/// Ethereum execution client proxy HTTP RPC endpoint
#[clap(long)]
pub eth_client_proxy_address: Option<String>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am curious whether env variable http_proxy and https_proxy are applied to the lib we used. Do you know that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I am not sure what you mean by that

@Vid201
Copy link
Member Author

Vid201 commented Sep 22, 2023

What if the websocket connection is lost because of network issue? Do you have plans to address that?Does ethers ws handle that internally?

The problem with websocket is that we need to provide a way when the connection is lost. If we don't have that, when the situation happens, the bundler would just crash and got no more data from websocket. As for normal http request, if one of the http request is lost, it would just got one task crash(maybe this task would crash the whole bundler. It is possible. But http leave codes easier to write)

Yes, that is the problem that still needs to be addressed in this PR. At the moment, if the connection is dropped, the bundler doesn't crash (same as HTTP). But if the eth client is up again, the connection is not reestablished. I am planning to make a wrapper around eth_client, that offers a way to do healthcheck and connect if connection is not live. Then the bundler pings eth_client every x seconds. What do you think?

I think websockets will be useful when we subscribe to blocks to check which user operations are included onchain.

@Vid201
Copy link
Member Author

Vid201 commented Sep 24, 2023

What if the websocket connection is lost because of network issue? Do you have plans to address that?Does ethers ws handle that internally?
The problem with websocket is that we need to provide a way when the connection is lost. If we don't have that, when the situation happens, the bundler would just crash and got no more data from websocket. As for normal http request, if one of the http request is lost, it would just got one task crash(maybe this task would crash the whole bundler. It is possible. But http leave codes easier to write)

Yes, that is the problem that still needs to be addressed in this PR. At the moment, if the connection is dropped, the bundler doesn't crash (same as HTTP). But if the eth client is up again, the connection is not reestablished. I am planning to make a wrapper around eth_client, that offers a way to do healthcheck and connect if connection is not live. Then the bundler pings eth_client every x seconds. What do you think?

I think websockets will be useful when we subscribe to blocks to check which user operations are included onchain.

@zsluedem I fixed the reconnection problems (https://github.com/Vid201/silius/blob/feat/ws/Cargo.toml#L43). This is now handled in ethers-rs internally (gakonst/ethers-rs@d9c2baf). When the connection is lost, it tries to reconnect every 2 seconds. When the node becomes online again, it works normally onwards. The behavior is similar now to lost http requests.

Copy link
Collaborator

@zsluedem zsluedem left a comment

Choose a reason for hiding this comment

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

Awesome!~!!

@zsluedem zsluedem merged commit b1841aa into main Sep 25, 2023
2 checks passed
@zsluedem zsluedem deleted the feat/ws branch September 25, 2023 01:32
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.

Connect to the execution client over websockets
2 participants