one click deprecated, snapshot added in readme and updated scripts#241
one click deprecated, snapshot added in readme and updated scripts#241anunay-xin wants to merge 6 commits intoXinFinOrg:masterfrom
Conversation
📝 WalkthroughWalkthroughREADME reworked to a Docker-first guide, adding a snapshot-based Method 4 and marking the one‑click installer as deprecated. Multiple start/up/down scripts now prefer Docker Compose v2 (CLI) and node startup flags migrated from legacy RPC options to HTTP/WS flag names; minor env and bootstrap updates applied. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 5
🤖 Fix all issues with AI agents
In `@mainnet/start-node.sh`:
- Around line 78-91: The script is using unrecognized hyphenated flags (e.g.,
--http-addr, --http-port, --http-api, --ws-addr, etc.) when ENABLE_RPC is true;
update the args assembly in mainnet/start-node.sh (the args array and the
ENABLE_RPC branch) to use the correct flag style supported by the XDC binary and
apply the same changes to the testnet and devnet start-node.sh scripts: either
convert the hyphenated flags to legacy RPC flags (e.g., --rpc, --rpcaddr,
--rpcport, --rpcapi, --ws, --wsaddr/--wsport as supported) or to modern
dot-notation (e.g., --http.addr, --http.port, --http.api, --ws.addr, --ws.port),
and verify by checking the XDC binary help before choosing which form to use.
In `@README.md`:
- Line 142: Update the README line that currently reads "**Step 1: Download
[XinFin One-Click Installer]... for Windows, Linux, and Mac OS and Install on
your local machine.**" by replacing "Mac OS" with the correct Apple branding
"macOS" (i.e., "Windows, Linux, and macOS"); ensure the exact phrase in the
README is updated and preserve surrounding formatting and links.
- Line 259: Remove the confusing placeholder by replacing the note text that
contains "/your/xdcchain/directory" so it only shows the concrete path
"XinFin-Node/mainnet/xdcchain" (i.e., delete the placeholder and parentheses or
reword to "Make sure to use the actual path, e.g. XinFin-Node/mainnet/xdcchain")
to avoid duplication and confusion.
- Line 6: Replace the incomplete fragment "should be facing internet directly"
in README.md with a complete sentence that includes a clear subject and
articles; for example, change it to "The servers should be facing the internet
directly" or, if more specific, "The application servers should face the
internet directly" so the sentence reads grammatically complete and unambiguous.
- Around line 138-140: Replace the "Apple Mac" entry in the README list with the
correct OS name "macOS" (update the list item that currently reads "Apple Mac"
to "macOS") so the entries read: "macOS", "Windows", "Linux - Ubuntu" for
consistency and accuracy.
🧹 Nitpick comments (8)
README.md (6)
14-21: Add language specifiers to code blocks.The code blocks are missing language specifiers, which helps with syntax highlighting and improves readability.
✨ Proposed improvement
-``` +```bash git clone https://github.com/XinFinOrg/XinFin-Node.gitEnter
XinFin-Nodedirectory
-+bash
cd XinFin-Node
34-38: Add language specifier to code block.The code block is missing a language specifier for better syntax highlighting.
✨ Proposed improvement
-``` +```bash cd mainnet # testnet cp env.example .env nano .env</details> --- `45-47`: **Add language specifiers to shell command blocks.** Multiple code blocks containing shell commands are missing language specifiers, which would improve syntax highlighting and readability. <details> <summary>✨ Proposed improvement for all shell command blocks</summary> Add `bash` or `shell` language identifier to all code blocks containing shell commands: ```diff -``` +```bash sudo docker-compose -f docker-compose.yml up -dApply the same change to code blocks at lines 55-57, 59-62, 68-71, and 79-82. </details> Also applies to: 55-57, 59-62, 68-71, 79-82 --- `200-202`: **Add language specifiers to code blocks in Method 4.** Multiple code blocks in the new snapshot method are missing language specifiers, which would improve syntax highlighting and documentation quality. <details> <summary>✨ Proposed improvement</summary> Add `bash` language identifier to all code blocks: ```diff -``` +```bash sudo su -c "bash <(wget -qO- https://raw.githubusercontent.com/XinFinOrg/XinFin-Node/master/setup/bootstrap.sh)" rootApply this change to all code blocks at lines 200-202, 205-211, 215-217, 220-222, 225-234, 238-241, 250-252, and 255-257. For the `.env` configuration block at lines 245-247, use `bash` or `env` as the language specifier. </details> Also applies to: 205-211, 215-217, 220-222, 225-234, 238-241, 245-247, 250-252, 255-257 --- `3-3`: **Consider removing closing hashes from headings for consistency.** The markdown linter flags that these headings use closing hashes (`### Title ###`), which is the "atx_closed" style. The more common and recommended style is "atx" (without closing hashes: `### Title`). While both are valid, consistency improves maintainability. <details> <summary>✨ Proposed improvement</summary> ```diff -### Method 1:- Setup XinFin Masternode Docker ### +### Method 1:- Setup XinFin Masternode Docker -### Method 3:- Setup XinFin's XDC Masternode One-click Installer(Deprecated) ### +### Method 3:- Setup XinFin's XDC Masternode One-click Installer(Deprecated) -### Method 4:- Setup XinFin Masternode Using Snapshot ### +### Method 4:- Setup XinFin Masternode Using SnapshotAlso applies to: 131-131, 193-193
230-230: Endpoint is valid and operational; consider adding a brief security reminder.The endpoint
https://rpc.xdc.network/downloadis accessible and returns valid bash script content (HTTP 200, 26KB). The bootstrap pattern is reasonable and matches the documented workflow.Consider adding a brief note encouraging users to review the script content before piping to bash, especially for users unfamiliar with this pattern. Even though the current documentation contextualizes this within a tmux session (suggesting user review), an explicit security reminder would be helpful for clarity and good practice.
mainnet/docker-up.sh (1)
3-12: Good approach for Docker Compose v2 detection with a minor robustness gap.The v2 plugin detection logic is sound. One observation: the
elsebranch assumesdocker-compose(standalone) is installed, but doesn't verify it. If neitherdocker compose(v2) nordocker-compose(standalone) is available, line 12 will fail with an unhelpful "command not found" error.Consider adding a guard in the fallback:
Suggested improvement
if command -v docker >/dev/null 2>&1 && docker compose version >/dev/null 2>&1; then COMPOSE="docker compose" else - COMPOSE="docker-compose" - echo "Using legacy docker-compose. Upgrade to docker compose is recommended." + if command -v docker-compose >/dev/null 2>&1; then + COMPOSE="docker-compose" + echo "Using legacy docker-compose. Upgrade to docker compose is recommended." + else + echo "Error: Neither 'docker compose' (v2 plugin) nor 'docker-compose' (standalone) found. Please install Docker Compose." >&2 + exit 1 + fi fiThis pattern applies equally to all six docker-up/docker-down scripts in this PR (mainnet, testnet, devnet).
mainnet/start-node.sh (1)
78-91: Unset environment variables will pass empty strings to flags.If
ENABLE_RPCistruebut variables likeRPC_ADDR,RPC_PORT,RPC_API, etc. are not set, they expand to empty strings (e.g.,--http-addr ""), which may cause unexpected behavior. Consider providing sensible defaults (e.g.,0.0.0.0,8545,eth,net,web3).This is pre-existing behavior, but since you're already updating these flags, it would be a good time to add defaults.
Example: adding defaults for RPC env vars
# if ENABLE_RPC is true, add RPC related parameters if echo "${ENABLE_RPC}" | grep -iq "true"; then + RPC_ADDR="${RPC_ADDR:-0.0.0.0}" + RPC_PORT="${RPC_PORT:-8545}" + RPC_API="${RPC_API:-eth,net,web3}" + RPC_CORS_DOMAIN="${RPC_CORS_DOMAIN:-*}" + RPC_VHOSTS="${RPC_VHOSTS:-*}" + WS_ADDR="${WS_ADDR:-0.0.0.0}" + WS_PORT="${WS_PORT:-8555}" + WS_API="${WS_API:-eth,net,web3}" + WS_ORIGINS="${WS_ORIGINS:-*}" args+=( --http
anunay-xin
changed the title
…avoid ERROR: client version 1.30 is too old.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
setup/bootstrap.sh (1)
49-69:⚠️ Potential issue | 🔴 Critical
docker compose(v2 plugin) is used but never installed — only standalone v1 is installed.Line 51 downloads the standalone
docker-composev1.25.0 binary, which provides thedocker-composecommand. Line 69 now invokesdocker compose(the v2 plugin subcommand), which requires thedocker-compose-pluginpackage — never installed by this script. On a fresh machine this will fail.Either install the v2 plugin and use
docker compose, or keep the standalone binary and usedocker-compose. The v2 plugin approach is recommended since standalone v1 is deprecated:Proposed fix: install v2 plugin instead of standalone v1
- echo "Installing Docker-Compose" - - curl -L "https://github.com/docker/compose/releases/download/1.25.0/docker-compose-$(uname -s)-$(uname -m)" -o /usr/local/bin/docker-compose - - chmod +x /usr/local/bin/docker-compose - sleep 5 - echo "Docker Compose Installed successfully" + echo "Installing Docker Compose Plugin" + + sudo apt-get install docker-compose-plugin -y + echo "Docker Compose Plugin installed successfully"
… files; add RPC namespaces section
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mainnet/env.example (1)
9-9:⚠️ Potential issue | 🟡 Minor
dbwas not removed fromRPC_APIin this example file.
WS_API(line 16) haddbremoved, butRPC_APIstill includes it. This is inconsistent withmainnet/.envwheredbwas removed from both. Since thedbnamespace is being dropped, this should be updated too.📝 Proposed fix
-RPC_API=db,eth,net,txpool,web3,XDPoS +RPC_API=eth,net,txpool,web3,XDPoS
🤖 Fix all issues with AI agents
In `@mainnet/.env`:
- Line 11: Update the RPC_API setting in env.example to match the actual mainnet
.env by removing the "db" token so RPC_API is "eth,net,txpool,web3,XDPoS";
locate the RPC_API entry (identifier RPC_API) in env.example and replace the
value that currently includes "db,eth,net,txpool,web3,XDPoS" with the value used
in mainnet .env to keep the files consistent for cp env.example .env users.
In `@README.md`:
- Around line 239-241: The README currently uses "rm -rf" for removing files
"XDC/nodekey" and "XDC/transactions.rlp"; change those commands to use "rm -f"
instead (or keep "rm -rf" only for directory removals) so single files are
removed safely and errors aren't masked—update the instances referencing
XDC/nodekey and XDC/transactions.rlp accordingly.
- Around line 291-312: The README's "Removed Namespaces" section is missing the
`db` namespace which this PR removes from the `RPC_API` and `WS_API` defaults;
update the "Removed Namespaces" list to include `db` alongside `personal`, e.g.,
add a short entry like "`db` - database storage access methods (removed from
RPC_API and WS_API defaults)", and ensure the term `db` matches the exact
namespace string used in the .env changes so readers can correlate the removal
with the `RPC_API` and `WS_API` updates.
- Around line 224-234: Replace the insecure "curl | bash" pattern with explicit
download-and-verify steps: instruct users to first download the installer to a
local file (e.g., using curl or wget), verify the file integrity/signature
(e.g., provide guidance to check a published SHA256 checksum or use GPG
signature verification), inspect the script before running, then execute it
locally (e.g., with bash or by making it executable). Update the README example
block that currently shows "curl -fsSL https://rpc.xdc.network/download | bash"
to the safer sequence and add a note recommending verification for masternode
operators handling funds.
🧹 Nitpick comments (2)
README.md (2)
3-3: Heading style inconsistency: closed ATX headings.Lines 3, 131, and 193 use closed ATX headings (
### ... ###) while other headings in the file don't. Markdownlint flags this (MD003). Consider using open ATX style consistently.📝 Proposed fix
-### Method 1:- Setup XinFin Masternode Docker ### +### Method 1: Setup XinFin Masternode Docker -### Method 3:- Setup XinFin's XDC Masternode One-click Installer(Deprecated) ### +### Method 3: Setup XinFin's XDC Masternode One-click Installer (Deprecated) -### Method 4:- Setup XinFin Masternode Using Snapshot ### +### Method 4: Setup XinFin Masternode Using SnapshotAlso applies to: 131-131, 193-193
46-46: Update docker-compose commands to Docker Compose v2 syntax.Docker Compose v1 (
docker-compose) was deprecated when v2 reached GA (April 2022) and support ended after June 2023. Update the following instances to use the v2 CLI plugin syntax:Lines 46, 56, 70, 81
-sudo docker-compose -f docker-compose.yml up -d +sudo docker compose -f docker-compose.yml up -d-sudo docker-compose -f docker-compose.yml down +sudo docker compose -f docker-compose.yml down
|
|
||
| # RPC API setting | ||
| RPC_API=db,eth,net,txpool,web3,XDPoS | ||
| RPC_API=eth,net,txpool,web3,XDPoS |
There was a problem hiding this comment.
Inconsistency: db removed from RPC_API here but still present in env.example.
mainnet/env.example line 9 still has RPC_API=db,eth,net,txpool,web3,XDPoS while this file has db removed. The example file should match to avoid confusion when users run cp env.example .env.
🤖 Prompt for AI Agents
In `@mainnet/.env` at line 11, Update the RPC_API setting in env.example to match
the actual mainnet .env by removing the "db" token so RPC_API is
"eth,net,txpool,web3,XDPoS"; locate the RPC_API entry (identifier RPC_API) in
env.example and replace the value that currently includes
"db,eth,net,txpool,web3,XDPoS" with the value used in mainnet .env to keep the
files consistent for cp env.example .env users.
1 participant
Summary by CodeRabbit
Documentation
Chores
Bug Fixes