cosmos / cosmjs

The Swiss Army knife to power JavaScript based client solutions ranging from Web apps/explorers over browser extensions to server-side clients like faucets/scrapers.
https://cosmos.github.io/cosmjs/
Apache License 2.0
647 stars 332 forks source link

Remove unnecessary status call from connect method #1557

Open livthomas opened 7 months ago

livthomas commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

webmaster128 commented 7 months ago

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.