Client: (WIP) rework CLI startup logic using RpcConfig (#3983)#4013
Client: (WIP) rework CLI startup logic using RpcConfig (#3983)#4013ether-wan wants to merge 5 commits into
Conversation
| const opts: any = { | ||
| rpcCors, | ||
| if (rpcConfig.transport === 'ws') { | ||
| const wsOpts: any = { |
There was a problem hiding this comment.
Is it possible to add typing for this instead of casting to any?
|
|
||
| export type RPCNamespace = (typeof RPCNamespace)[keyof typeof RPCNamespace] | ||
|
|
||
| export const RPCTransport = { |
There was a problem hiding this comment.
For the keys of this mapping and similar ones, we can switch to using lowercase key names, like here.
| public static readonly DEFAULT_WS_ENGINE_ADDR = '0.0.0.0' | ||
| public static readonly DEFAULT_RPC_PORT = 8545 | ||
| public static readonly DEFAULT_ENGINE_PORT = 8551 | ||
| public static readonly DEFAULT_WS_PORT = 0 |
There was a problem hiding this comment.
the default websocket port is 8546 and other defaults should be likely made consistent with any defaults that already exist in getArgs
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
This seems to be heading in the right direction, but make sure to work in small chunks and check the CI to catch any issues or bugs early and respond to them iteratively. |
Thank you! I'll look into it asap |
|
|
||
| import { helpRPC, startRPCServers } from './startRPC.ts' | ||
| import { generateClientConfig, getArgs } from './utils.ts' | ||
| import { generateClientConfig, generateRpcConfigs, getArgs } from './utils.ts' |
There was a problem hiding this comment.
generateRpcConfigs -> generateRpcConfig (so: without the s)
to align with generateClientConfig
There was a problem hiding this comment.
(also further down the line there are a few Configs, can you please also rename consistently? Thanks! (we are a bit picky on naming things 🙂 ))
4 participants
This PR starts addressing #3983 by refactoring the client CLI
Description
RpcConfigclass for RPC endpoints (eth/engine, http/ws)generateRpcConfigs()func to generate all server configurations from CLI argsstartRPCServers()to use these configs instead of cli argsrun()function to improve testability:run()now handles only CLI argument parsing, config generation, and dispatchstartClientAndServers()function performs the client and RPC server startupThis is still a work in progress — if the current structure looks good, I’ll follow up with unit tests for the new helpers and config logic in a separate commit!