fix: properly disable RPC#264
Merged
AnilChinchawale merged 9 commits intoXinFinOrg:masterfrom Apr 16, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughStartup scripts, env files, and README now unify RPC and WebSocket configuration: enablement checks use exact Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@devnet/start.sh`:
- Around line 74-91: The ENABLE_RPC check currently uses grep -iq "true" which
allows substring matches (e.g., "untrue") and can incorrectly enable RPC; update
the condition around ENABLE_RPC to perform an exact, case-insensitive comparison
(e.g., match ^true$ or normalize and compare to "true") so only the literal
"true" enables RPC, and keep the existing args population logic (the args array
and the branches that add --rpcaddr/--rpcport/... vs --http=false/--ws=false)
unchanged.
In `@mainnet/start-node.sh`:
- Around line 80-98: The ENABLE_RPC gate currently uses a substring match (grep
-iq "true") which enables RPC for values like "nottrue"; change it to an exact,
case-insensitive boolean check so only "true" enables RPC. Edit the if condition
that guards the args+=( --rpcaddr ... --wsorigins ...) block (the ENABLE_RPC
check in start-node.sh) and replace the grep substring test with an exact match
such as a case-normalized string comparison (for example compare
"${ENABLE_RPC,,}" to "true") or use grep -ix "true" so only the exact value
"true" (any case) enables the RPC block.
In `@testnet/start-apothem.sh`:
- Around line 84-101: The ENABLE_RPC check currently uses grep -iq "true" which
matches substrings (so values like "nottrue" enable RPC); update the conditional
that tests ENABLE_RPC in start-apothem.sh to match the exact literal "true" (for
example use a anchored grep pattern like '^true$' or a shell string equality
check such as [[ "${ENABLE_RPC}" == "true" ]]) so the args array population (the
--rpc*/--ws* flags vs the --http=false/--ws=false branch) only happens when
ENABLE_RPC is exactly "true".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf36674b-6e2e-48da-857f-3b99364b45a5
📒 Files selected for processing (3)
devnet/start.shmainnet/start-node.shtestnet/start-apothem.sh
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
mainnet/start-node.sh (1)
88-88:--store-rewardplacement differs from devnet/start.sh.In this file,
--store-rewardis in both RPC branches (lines 88, 99) but not in the base args. Indevnet/start.sh, it's in the base args AND both branches (causing duplication). Consider moving--store-rewardto the baseargsarray for consistency across all network scripts.Also applies to: 99-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mainnet/start-node.sh` at line 88, The placement of the --store-reward flag should be consistent: move the literal "--store-reward" out of the RPC-specific argument branches and add it to the shared args array so it appears once for all network modes; update the args initialization (the base args array variable) to include "--store-reward" and remove the duplicate occurrences of "--store-reward" from the RPC-specific branches to avoid duplication and ensure parity with devnet/start.sh.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@devnet/start.sh`:
- Line 70: Remove the duplicate --store-reward flag from the conditional
branches and keep it only in the base args array: edit the script where the base
args array is initialized (the args variable) to include --store-reward and then
remove any additional occurrences of --store-reward inside the RPC-enabled
branch and the RPC-disabled branch so the flag is passed exactly once to the XDC
command.
- Line 97: The ENABLE_WS check uses an unanchored grep which allows substring
matches; change the conditional that currently pipes ENABLE_WS into grep -iq
"true" to use an anchored, case-insensitive pattern (same fix applied to
ENABLE_RPC) so only exact true-like values match (e.g., ^true$ or a pattern
matching canonical truth values). Update the if condition that references
ENABLE_WS to use the anchored pattern for exact matching and preserve
case-insensitivity.
In `@mainnet/.env`:
- Around line 10-11: The committed mainnet .env currently enables RPC/WS
(ENABLE_RPC=true and ENABLE_WS=true), which contradicts the secure-by-default
settings; change ENABLE_RPC and ENABLE_WS back to false (ENABLE_RPC=false,
ENABLE_WS=false) in this .env (or remove them so the .env.example defaults
apply) and add a brief inline comment reminding maintainers that enabling RPC/WS
is a deliberate, documented opt-in to avoid accidental exposure.
- Line 6: The repository has a tracked .env file containing a PRIVATE_KEY value;
remove that file from version control and stop tracking secrets by running git
rm --cached on the tracked .env file, commit the removal, and push; ensure the
.gitignore contains the pattern (*.env* or equivalent) so new .env files are
ignored, and keep only env.example under version control to document required
variables.
In `@mainnet/start-node.sh`:
- Line 103: The ENABLE_WS conditional currently uses substring grep which allows
partial matches; update the conditional that tests ENABLE_WS (the if statement
using echo "${ENABLE_WS}" | grep -iq "true") to perform an exact,
case-insensitive match (e.g., use anchored matching like '^true$' with grep -i
or a direct string comparison such as [ "${ENABLE_WS}" = "true" ] or a case
statement) so only the exact boolean "true" triggers the branch.
In `@testnet/.env`:
- Line 6: The repository currently commits a real-seeming PRIVATE_KEY value in
the .env file; remove the sensitive PRIVATE_KEY entry from the committed .env,
add .env to .gitignore, and replace the tracked .env with an env.example (or
update env.example) that documents the PRIVATE_KEY variable name and format;
ensure any references to PRIVATE_KEY in your code or CI use environment
variables loaded at runtime rather than relying on the checked-in .env file
(look for uses of PRIVATE_KEY in config loading or tests to confirm).
In `@testnet/start-apothem.sh`:
- Line 80: The --store-reward flag is being appended multiple times: it's
already in the base argument list and then added again in both RPC branch
argument constructions; remove the duplicate additions so the flag only appears
once (either keep it in the base args and delete the two branch-specific
"--store-reward" entries, or keep it in branches and remove it from base).
Alternatively, when appending in the RPC branch code, guard the append with a
check that the args variable doesn't already contain "--store-reward" to prevent
duplicates (update the branch argument construction logic that currently appends
"--store-reward").
- Line 107: The if-condition checking ENABLE_WS uses substring matching which is
vulnerable; update the conditional around ENABLE_WS to perform an anchored,
case-insensitive exact match (e.g., match only the whole token "true" or
acceptable equivalents like "1") instead of grepping for the substring "true",
so replace the current if that references ENABLE_WS with one that anchors both
start and end for the allowed values to avoid partial matches.
---
Nitpick comments:
In `@mainnet/start-node.sh`:
- Line 88: The placement of the --store-reward flag should be consistent: move
the literal "--store-reward" out of the RPC-specific argument branches and add
it to the shared args array so it appears once for all network modes; update the
args initialization (the base args array variable) to include "--store-reward"
and remove the duplicate occurrences of "--store-reward" from the RPC-specific
branches to avoid duplication and ensure parity with devnet/start.sh.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 642b60e3-57e1-461c-bec7-6473c1de05aa
📒 Files selected for processing (8)
README.mddevnet/.envdevnet/start.shmainnet/.envmainnet/env.examplemainnet/start-node.shtestnet/.envtestnet/start-apothem.sh
wanwiset25
reviewed
Apr 11, 2026
wanwiset25
reviewed
Apr 11, 2026
wanwiset25
reviewed
Apr 11, 2026
wanwiset25
reviewed
Apr 11, 2026
wanwiset25
reviewed
Apr 11, 2026
wanwiset25
reviewed
Apr 11, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
412-412: Clarify the v2.6.8 behavior change to avoid upgrade confusion.Please make this note explicitly mention that
--http/--wsdefaults changed in v2.6.8+, so users understand why migration is required.Suggested wording
-> **Note for Upgrading from v2.6**: If you are upgrading from a version prior to v2.6.8, the old environment variables (`RPC_API`, `WS_API`, `RPC_ADDR`, `WS_ADDR`, `RPC_CORS_DOMAIN`, `RPC_VHOSTS`, `WS_ORIGINS`) are no longer supported. Please update your `.env` file to use the new unified variables listed above. +> **Note for upgrades (v2.6.8 and later)**: Starting in v2.6.8, node defaults for HTTP/WS changed. Legacy env variables (`RPC_API`, `WS_API`, `RPC_ADDR`, `WS_ADDR`, `RPC_CORS_DOMAIN`, `RPC_VHOSTS`, `WS_ORIGINS`) are not used by the current scripts, so update your `.env` to the unified variables above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 412, Update the README note about upgrading from pre-v2.6.8 to explicitly state that as of v2.6.8 the defaults for the command-line flags `--http` and `--ws` changed and that this is why the old environment variables (`RPC_API`, `WS_API`, `RPC_ADDR`, `WS_ADDR`, `RPC_CORS_DOMAIN`, `RPC_VHOSTS`, `WS_ORIGINS`) are no longer supported; instruct users to migrate to the new unified environment variables and/or adjust any service startup flags that referenced the old behavior so their `.env` and invocation (e.g., scripts that pass `--http`/`--ws`) reflect the new defaults in v2.6.8+.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@README.md`:
- Line 412: Update the README note about upgrading from pre-v2.6.8 to explicitly
state that as of v2.6.8 the defaults for the command-line flags `--http` and
`--ws` changed and that this is why the old environment variables (`RPC_API`,
`WS_API`, `RPC_ADDR`, `WS_ADDR`, `RPC_CORS_DOMAIN`, `RPC_VHOSTS`, `WS_ORIGINS`)
are no longer supported; instruct users to migrate to the new unified
environment variables and/or adjust any service startup flags that referenced
the old behavior so their `.env` and invocation (e.g., scripts that pass
`--http`/`--ws`) reflect the new defaults in v2.6.8+.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80b07af9-3132-4afc-8f95-668bdf35e0c7
📒 Files selected for processing (8)
README.mddevnet/.envdevnet/start.shmainnet/.envmainnet/env.examplemainnet/start-node.shtestnet/.envtestnet/start-apothem.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- mainnet/env.example
- mainnet/start-node.sh
- testnet/start-apothem.sh
- devnet/start.sh
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mainnet/start-node.sh`:
- Around line 102-109: The WebSocket branch uses the old ORIGINS variable so
--wsorigins can be empty; update the args construction to use the renamed
ALLOWED_ORIGINS instead of ${ORIGINS} (i.e., replace references to ORIGINS with
ALLOWED_ORIGINS in the ENABLE_WS block that appends --wsorigins to the args
array) so the --wsorigins flag receives the configured ALLOWED_ORIGINS value.
- Around line 90-99: The script references an undefined ORIGINS variable and
passes empty string values to typed RPC/WS flags; replace ORIGINS with the
renamed ALLOWED_ORIGINS where used (e.g. in the --wsorigins "--wsorigins
${ALLOWED_ORIGINS}" occurrence) so WebSocket origin is set correctly, and remove
any empty-valued flags (drop --rpcport "", --rpcapi "", --rpccorsdomain "",
--rpcvhosts "", --wsport "", etc.) from the disable branch—rely only on
--rpc=false and --ws=false to disable services; apply the same fixes to the
testnet and devnet start scripts if present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cd90b74c-3bbf-4fa6-95c9-d8078403f8df
📒 Files selected for processing (7)
README.mddevnet/start.shmainnet/.envmainnet/env.examplemainnet/start-node.shtestnet/.envtestnet/start-apothem.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- mainnet/env.example
- devnet/start.sh
- testnet/start-apothem.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@testnet/start-apothem.sh`:
- Around line 87-91: The script currently hardwires shared RPC flags (--rpcaddr
"${LISTEN_ADDR}", --rpcport "${RPC_PORT}", --rpcapi "${API}", --rpccorsdomain
"${ALLOWED_ORIGINS}", --rpcvhosts "*") which overrides existing legacy RPC_/WS_
env vars and forces --rpcvhosts "*"; change the flag construction so it first
checks for legacy RPC_*/WS_* env vars (e.g. RPC_ADDR, RPC_PORT, RPC_API,
RPCCORSDOMAIN, RPCVHOSTS or WS_* equivalents) and uses those when present,
otherwise fall back to the shared knobs LISTEN_ADDR, RPC_PORT, API,
ALLOWED_ORIGINS; also do not unconditionally add --rpcvhosts "*" when ENABLE_RPC
is true—only add --rpcvhosts if RPCVHOSTS (or legacy equivalent) is set or
empty-string semantics require explicit wildcard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 41fbf517-91b5-4a9b-9009-91d7e04c6953
📒 Files selected for processing (7)
README.mddevnet/start.shmainnet/.envmainnet/env.examplemainnet/start-node.shtestnet/.envtestnet/start-apothem.sh
✅ Files skipped from review due to trivial changes (1)
- devnet/start.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- mainnet/start-node.sh
- mainnet/env.example
Comment on lines
+87
to
+91
| --rpcaddr "${LISTEN_ADDR}" | ||
| --rpcport "${RPC_PORT}" | ||
| --rpcapi "${RPC_API}" | ||
| --rpccorsdomain "${RPC_CORS_DOMAIN}" | ||
| --rpcvhosts "${RPC_VHOSTS}" | ||
| --rpcapi "${API}" | ||
| --rpccorsdomain "${ALLOWED_ORIGINS}" | ||
| --rpcvhosts "*" |
There was a problem hiding this comment.
Preserve legacy RPC/WS overrides instead of hardwiring one shared policy.
This now ignores existing RPC_* / WS_* settings and also forces --rpcvhosts "*" when RPC is enabled. An upgraded node that keeps its older .env silently loses prior bind/API/vhost restrictions, which is a security and compatibility regression unrelated to the ENABLE_RPC=false fix. Please keep the shared knobs as defaults, but fall back to the legacy variables when they are still present.
Possible compatibility-friendly shape
+rpc_addr="${LISTEN_ADDR:-${RPC_ADDR:-0.0.0.0}}"
+rpc_api="${API:-${RPC_API:-db,eth,net,txpool,web3,XDPoS}}"
+rpc_cors="${ALLOWED_ORIGINS:-${RPC_CORS_DOMAIN:-*}}"
+rpc_vhosts="${RPC_VHOSTS:-*}"
+ws_addr="${LISTEN_ADDR:-${WS_ADDR:-0.0.0.0}}"
+ws_api="${API:-${WS_API:-db,eth,net,txpool,web3,XDPoS}}"
+ws_origins="${ALLOWED_ORIGINS:-${WS_ORIGINS:-*}}"
...
- --rpcaddr "${LISTEN_ADDR}"
+ --rpcaddr "${rpc_addr}"
...
- --rpcapi "${API}"
+ --rpcapi "${rpc_api}"
...
- --rpccorsdomain "${ALLOWED_ORIGINS}"
- --rpcvhosts "*"
+ --rpccorsdomain "${rpc_cors}"
+ --rpcvhosts "${rpc_vhosts}"
...
- --wsaddr "${LISTEN_ADDR}"
+ --wsaddr "${ws_addr}"
...
- --wsapi "${API}"
- --wsorigins "${ALLOWED_ORIGINS}"
+ --wsapi "${ws_api}"
+ --wsorigins "${ws_origins}"Also applies to: 108-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testnet/start-apothem.sh` around lines 87 - 91, The script currently
hardwires shared RPC flags (--rpcaddr "${LISTEN_ADDR}", --rpcport "${RPC_PORT}",
--rpcapi "${API}", --rpccorsdomain "${ALLOWED_ORIGINS}", --rpcvhosts "*") which
overrides existing legacy RPC_/WS_ env vars and forces --rpcvhosts "*"; change
the flag construction so it first checks for legacy RPC_*/WS_* env vars (e.g.
RPC_ADDR, RPC_PORT, RPC_API, RPCCORSDOMAIN, RPCVHOSTS or WS_* equivalents) and
uses those when present, otherwise fall back to the shared knobs LISTEN_ADDR,
RPC_PORT, API, ALLOWED_ORIGINS; also do not unconditionally add --rpcvhosts "*"
when ENABLE_RPC is true—only add --rpcvhosts if RPCVHOSTS (or legacy equivalent)
is set or empty-string semantics require explicit wildcard.
wanwiset25
reviewed
Apr 12, 2026
wanwiset25
approved these changes
Apr 12, 2026
anunay-xin
reviewed
Apr 13, 2026
JukLee0ira
force-pushed
the
master
branch
2 times, most recently
from
9be7ac8 to
a9c91a3
Compare
wanwiset25
reviewed
Apr 15, 2026
…emove corresponding environment variables
anunay-xin
approved these changes
Apr 15, 2026
wanwiset25
approved these changes
Apr 15, 2026
AnilChinchawale
approved these changes
Apr 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR refactoring works correctly:
Env var rename: ADDR → removed, ORIGINS → ALLOWED_ORIGINS
Startup flag refactor: --rpc* → --http* format,--ws* → --ws-* format
Simplify disabled state by removing empty args
Refactor Details
1 Modified Env Variables
RPC_ADDR/WS_ADDR2 Modified Startup Flags