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

Remove unnecessary status call from connect method #1557

Open
livthomas opened this issue Feb 16, 2024 · 3 comments
Open

Remove unnecessary status call from connect method #1557

livthomas opened this issue Feb 16, 2024 · 3 comments

Comments

@livthomas
Copy link
Contributor

I have just noticed that all Tendermint/Comet clients make an additional call to /status when they are being created by connect method. This is very unfortunate, especially when the client is being created and destroyed very often.

Is it really necessary to make this call? Because from the comment, it looks like the author of this code was not even sure what he was doing. Just because of some failing CI job, all applications around the world using cosmjs library make double calls to RPC nodes.

I refer to the following code:

It looks like this code was added there three years ago whenTendermint34Client class was first implemented. Nobody has ever changed it and it was just copied over to new classes (Tendermint37Client and Comet38Client).

@webmaster128
Copy link
Member

It's a mystery why it's needed by every time I tried to remove it, the CI tests start failing. The way this was designed is more optimized for longer living tendermint clients.

@livthomas
Copy link
Contributor Author

If that code is removed, does it break the client itself or is it just about CI tests failing?

  • If the client doesn't work without making this initial call, what about using a HEAD request or some kind of low-level network call that wouldn't count towards rate limits?
  • If it breaks the CI tests, I would suggest to simply run that code conditionally only in CI.

@webmaster128
Copy link
Member

The first approach is not so easy because the rpc client does not have low level access and could be different transports.

I don't know if it is ONLY a CI issue. I know this is annoying for some users but I prefer to either get the root cause debugged or keep it as is.

I suggest the simplest way for you to fix your use case is using create instead of connect with a manually created HttpClient.

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

No branches or pull requests

2 participants