feat: add dedicated websocket cli parameter

[atanmarko]

Description

Add sequencer command line argument --subnet-websocket-endpoint to explicitly provided subnet jsonrpc websocket endpoint. Usable when subnet endpoint could not be derived from http/https endpoint in a standard way (what is done by default), e.g. when custom cloud websocket endpoints are used.

Fixes TP-749

PR Checklist:

• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added or updated tests that comprehensively prove my change is effective or that my feature works

[codecov]

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (fe8c952) 60.83% compared to head (79c2b57) 60.80%.

❗ Current head 79c2b57 differs from pull request most recent head e00f852. Consider uploading reports for the commit e00f852 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #337      +/-   ##
==========================================
- Coverage   60.83%   60.80%   -0.03%     
==========================================
  Files         226      226              
  Lines       12333    12339       +6     
==========================================
  Hits         7503     7503              
- Misses       4830     4836       +6     

Files	Coverage Δ
...tes/topos/src/components/sequencer/commands/run.rs	16.66% <ø> (ø)
crates/topos/src/config/sequencer.rs	0.00% <ø> (ø)
crates/topos/src/components/node/services.rs	20.00% <0.00%> (-0.21%)	⬇️
crates/topos/src/components/sequencer/mod.rs	0.00% <0.00%> (ø)
crates/topos-sequencer/src/lib.rs	0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[hadjiszs]

I think the initial idea is more to make it straight and not flexible, simply subnet_http_jsonrpc_endpoint and subnet_ws_jsonrpc_endpoint given explicitly, without any attempt to infer anything

Here the current patch is nice because pretty backward compatible with what we have now, but the concern is that's the type of weird logic that are not nice when they adds up I think

[dvdplm]

I think we should stop inferring things automatically, like @hadjiszs suggests.

Also, wrt naming: JSON-RPC is the protocol and http/ws are the available transports. With that in mind I think subnet_websocket_endpoint is a poor name. I suggest we rename the options to subnet_jsonrpc_http and subnet_jsonrpc_ws and make both required (no Option<String>).

I am a bit concerned about our naming of env vars; I think they should be consistently prefixed with TOPOS_, but that's something for a different PR. :)

[atanmarko]

I think we should stop inferring things automatically, like @hadjiszs suggests.

Also, wrt naming: JSON-RPC is the protocol and http/ws are the available transports. With that in mind I think subnet_websocket_endpoint is a poor name. I suggest we rename the options to subnet_jsonrpc_http and subnet_jsonrpc_ws and make both required (no Option<String>).

I am a bit concerned about our naming of env vars; I think they should be consistently prefixed with TOPOS_, but that's something for a different PR. :)

Right I am aware of that. I didn't want to break things with the tools team setup (to rename the subnet_jsonrpc_endpoint) and add topos to the env variable. Lets break everything then.

@dvdplm In 90% of the cases http and websocket endpoints are related and easily deducible. If we start requiring everything we will have very soon 10 parameters required to run our node

[atanmarko]

@dvdplm @hadjiszs Updated param names to subnet_jsonrpc_http and subnet_jsonrpc_ws. I am strongly against making websocket argument required, as first time we needed it in a year was recently when Monir was dealing with the external subnets. Anyway lets leave that decision for some other PR.

[dvdplm]

I am strongly against making websocket argument required, as first time we needed it in a year was recently when Monir was dealing with the external subnets. Anyway lets leave that decision for some other PR.

Ok, maybe you're right. We'll discuss it elsewhere. :)