feat: implement extended commands for relay cli#5188
feat: implement extended commands for relay cli#5188natanasow wants to merge 11 commits into5131-implement-base-commands-for-relay-clifrom
Conversation
Signed-off-by: nikolay <n.atanasow94@gmail.com>
…mplement-extended-commands-for-relay-cli # Conflicts: # bin/hiero-relay.js
Signed-off-by: nikolay <n.atanasow94@gmail.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
…mplement-extended-commands-for-relay-cli # Conflicts: # bin/hiero-relay.js
Signed-off-by: nikolay <n.atanasow94@gmail.com>
Signed-off-by: nikolay <n.atanasow94@gmail.com>
…mplement-extended-commands-for-relay-cli
jasuwienas
left a comment
There was a problem hiding this comment.
Looks great overall, I just have a few suggestions/questions.
bin/hiero-relay.md
Outdated
| **Start relay with both http and ws servers with specific subdomains** | ||
|
|
||
| ```bash | ||
| hiero-relay -n testnet -r --rpc-ws-enabled true --rpc-http-api eth net debug --rpc-ws-api web3 |
There was a problem hiding this comment.
I'm unable to make it work locally.
I'm running the relay with -n testnet -r --rpc-http-api eth commands, but I'am still able to query debug_getRawTransaction
There was a problem hiding this comment.
I DMed you, but I'll drop it here for visibility to other reviewers.
Make sure you delete your local .standalone, then run npm run build && node scripts/build-standalone.js before testing these extended options.
Signed-off-by: nikolay <n.atanasow94@gmail.com>
Signed-off-by: nikolay <n.atanasow94@gmail.com>
Signed-off-by: nikolay <n.atanasow94@gmail.com>
Signed-off-by: nikolay <n.atanasow94@gmail.com>
quiet-node
left a comment
There was a problem hiding this comment.
Clean work but some concerns
|
|
||
| const subdomain = method.split('_')[0] ?? null; | ||
| if (!RELAY_RPC_WS_API.has(subdomain)) { | ||
| return jsonRespError(null, spec.MethodNotFound(subdomain), requestDetails.requestId); | ||
| } | ||
|
|
There was a problem hiding this comment.
This subdomain check calls method.split('_') before validateJsonRpcRequest runs. If a WS client sends a request without a method field, method is undefined and this throws a TypeError. Would it make sense to move the subdomain check after validateJsonRpcRequest (which already guards against missing/invalid method fields) to avoid the crash.
| const subdomain = request.method.split('_')[0] ?? null; | ||
| if (!RELAY_RPC_HTTP_API.has(subdomain)) { | ||
| return jsonRespError(request.id, spec.MethodNotFound(subdomain), requestId); | ||
| } | ||
|
|
||
| try { |
There was a problem hiding this comment.
spec.MethodNotFound(subdomain) in this logic for both HTTP and WS passes only the subdomain prefix (e.g., "eth") instead of the full method name (e.g., "eth_call"). This produces misleading errors like "Method eth not found". The existing verifySupportedMethod path correctly uses request.method. I think we should use request.method here too for consistency
|
|
||
| const subdomain = method.split('_')[0] ?? null; | ||
| if (!RELAY_RPC_WS_API.has(subdomain)) { | ||
| return jsonRespError(null, spec.MethodNotFound(subdomain), requestDetails.requestId); |
There was a problem hiding this comment.
This hardcodes null as the response ID. Every other jsonRespError call in this file uses request.id || null, and the HTTP equivalent correctly uses request.id. Per JSON-RPC 2.0 spec, the error response must echo the request's id for client correlation. Let's change it to request.id || null to be compliant
| **Start relay with ws server only:** | ||
|
|
||
| ```bash | ||
| hiero-relay -n testnet -r --rpc-http-enabled false --rpc-ws-enabled |
There was a problem hiding this comment.
Correct me if I'm wrong but but yargs boolean options don't seem to work this way. It parses as the flag being present (true) plus a positional arg "false".
So with yargs type: 'boolean', --rpc-http-enabled false parses as:
argv['rpc-http-enabled']=true(the flag is present, so true)argv._=['false'](false becomes a positional argument)
To disable a boolean flag in yargs, users must use --no-rpc-http-enabled. The documented example won't disable the HTTP server as intended, and the validation guard becomes unreachable when following the docs. Can you double check and make sure?
| RELAY_RPC_HTTP_API: { | ||
| type: 'strArray', | ||
| required: false, | ||
| defaultValue: ['eth', 'debug', 'net', 'web3', 'txpool', 'trace', 'admin'], | ||
| }, | ||
| RELAY_RPC_WS_API: { | ||
| type: 'strArray', | ||
| required: false, | ||
| defaultValue: ['eth', 'debug', 'net', 'web3', 'txpool', 'trace', 'admin'], | ||
| }, | ||
| REQUEST_ID_IS_OPTIONAL: { |
There was a problem hiding this comment.
Nit: This new config keysuse a RELAY_ prefix, but the related keys RPC_HTTP_ENABLED / RPC_WS_ENABLED from PR #5074 don't. Since these are all part of the same RPC transport feature family, could we align the naming? Dropping the RELAY_ prefix (RPC_HTTP_API, RPC_WS_API) would be consistent and non-breaking.
| if (!argv['rpc-http-enabled'] && !argv['rpc-ws-enabled']) { | ||
| parser.showHelp(); | ||
| console.log('\nError: At least one transport must be enabled (--rpc-http-enabled or --rpc-ws-enabled)'); | ||
| return; | ||
| } | ||
| const readOnlyEnvs = CliHelper.populateEnvBaseOnReadOnlyOption(argv); |
There was a problem hiding this comment.
This transport validation guard here is placed after the --config-file early return. Users who start the relay with -c ./my.env where both transports are disabled get no CLI warning then the child process spawns and only fails at runtime via logger.fatal. Would it make sense to parse the .env file and checking transport config before spawning, so both paths give the same immediate feedback.
Description
Implement the following CLI options:
--json-pretty-print-enable - true/false
--rpc-http-enabled - true/false -
RELAY_RPC_HTTP_ENABLED--rpc-ws-enabled - true/false -
RELAY_RPC_WS_ENABLED--rpc-http-api -
RELAY_RPC_HTTP_APIwhat subdomain (ETH, DEBUG, ADMIN, TRACE are enabled)-- rpc-ws-api -
RELAY_RPC_WS_APIwhat subdomain (ETH, DEBUG, ADMIN, TRACE are enabled)Related issue(s)
Fixes #5132
Testing Guide
Changes from original design (optional)
N/A
Additional work needed (optional)
N/A
Checklist