-
-
Notifications
You must be signed in to change notification settings - Fork 6
Add production-ready one-command miner installer with virtualenv isol… #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ation and service management
Scottcjn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review: Miner Installer with Virtualenv Isolation (PR #6)
@samuel-asleep - This is the strongest PR of the batch. Virtualenv isolation, systemd/launchd auto-start, and clean uninstall are all real improvements over the existing installer. The code is clean and well-structured.
Issues to Fix
1. All API endpoint URLs are wrong (BLOCKER)
The PR references three endpoint patterns throughout INSTALL.md, README.md, and the install.sh post-install output that don't exist on the RustChain node:
/wallet/YOUR_WALLET_NAME/balance → 404
/wallet/YOUR_WALLET_NAME/transactions → 404
/miner/YOUR_WALLET_NAME/stats → 404
The actual endpoints are:
# Balance check (the only one that exists)
curl -sk "https://50.28.86.131/wallet/balance?miner_id=YOUR_WALLET_NAME"
# Active miners list
curl -sk https://50.28.86.131/api/miners
# Node health
curl -sk https://50.28.86.131/health
# Current epoch
curl -sk https://50.28.86.131/epochThere is no transaction history endpoint and no per-miner stats endpoint. Fix all references in:
INSTALL.mdlines 177, 183, 189 (and all other occurrences)README.mdlines 56, 61install.shpost-install echo statements
2. config.json is created but never consumed
cat > "$INSTALL_DIR/config.json" << EOF
{
"wallet": "$WALLET_NAME",
"node_url": "$NODE_URL"
}
EOFThe miner uses --wallet CLI arg, not a config file. INSTALL.md tells users to edit config.json to change settings, but the miner ignores it. Either:
- Remove
config.jsoncreation and references - Or have the start script read from it (parse with
python -c "import json; ...")
3. No wallet name validation
Wallet names with spaces, quotes, or shell metacharacters will break the systemd service file and launchd plist since they're interpolated directly:
ExecStart=$venv_python $INSTALL_DIR/rustchain_miner.py --wallet $walletIf wallet is my wallet or $(rm -rf ~), this breaks or worse. Add validation:
if [[ ! "$wallet_name" =~ ^[a-zA-Z0-9_-]+$ ]]; then
echo -e "${RED}[!] Wallet name can only contain letters, numbers, hyphens, and underscores${NC}"
exit 1
fiMinor Issues
4. Heredoc quoting in service files
The systemd service and launchd plist use unquoted heredocs (<< EOF), which means shell variables are expanded at creation time. This is actually correct here since you want the current values baked in, but worth a comment in the script.
5. read -r in piped context
When the script is piped from curl (curl ... | bash), stdin is the curl output, not the terminal. The read -r wallet_name and read -r setup_autostart calls will read from the pipe (which is empty/exhausted), not from the user. This means:
- Without
--wallet, the auto-generated name is always used (user can't type) - Auto-start prompt is always skipped
The fix: redirect reads from /dev/tty:
read -r wallet_name < /dev/tty
read -r setup_autostart < /dev/ttyWhat's Good
- Virtualenv isolation is the right call -
pip install --userpollutes the user's Python - Systemd user services (not system services) - no sudo needed
- Launchd plist with
KeepAliveand-u(unbuffered) for real logging - Clean uninstall removes services, files, and symlinks
--walletflag for non-interactive/scripted installs- Fallback from
python -m venvtovirtualenvmodule - Good error messages with colored output
- INSTALL.md is thorough (troubleshooting section is useful)
Fix items 1, 3, and 5, and this is mergeable. Item 2 is a nice-to-have.
|
@samuel-asleep Here's the fix checklist: Fix List1. Fix all API endpoint URLs (BLOCKER)Every # Balance check (CORRECT)
curl -sk "https://50.28.86.131/wallet/balance?miner_id=YOUR_WALLET_NAME"
# Miners list (CORRECT)
curl -sk https://50.28.86.131/api/miners
# Node health (CORRECT)
curl -sk https://50.28.86.131/healthRemove references to 2. Add wallet name validationPrevent shell injection via wallet names. Before using the wallet name anywhere: if [[ ! "$wallet_name" =~ ^[a-zA-Z0-9_-]+$ ]]; then
echo -e "${RED}[!] Wallet name must be alphanumeric (hyphens and underscores allowed)${NC}"
exit 1
fi3. Fix
|
|
@samuel-asleep Re-reviewed after your second commit. All 4 items from the original fix checklist have been addressed:
New Features (Nice Additions)The second commit also added several improvements beyond the original fixes:
Remaining Nits (Non-Blocking)1. Fake example response in INSTALL.md (Minor)Line 164-170 shows: {
"wallet": "my-miner-wallet",
"balance": 12.456,
"pending": 0.12
}The actual {"miner_id": "my-miner-wallet", "amount_rtc": 12.456, "amount_i64": 12456000}2. Mixed HTTP/HTTPS in README (Minor)The "Network Verification" section now uses 3. No
|
Scottcjn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All 4 fix checklist items addressed. Installer now handles full lifecycle. Ready to merge — only remaining nit is a fake example response in INSTALL.md (wrong field names in the JSON example).
|
@samuel-asleep Sent 5 RTC to wallet curl -s 'https://50.28.86.131/wallet/balance?miner_id=samuel-asleep' -kThis is a welcome bonus — the full 25 RTC bounty (#4 on rustchain-bounties) pays on merge to main. If you want a different wallet name, let us know and we'll transfer. |
Interactive miner dashboard for RustChain with: - Real-time active miners table with architecture badges - Antiquity multiplier display (2.5x for G4, 2.0x for G5) - Balance leaderboard (top 20) - Miner search by wallet ID - Node health monitoring - Auto-refresh every 30 seconds Tech: Pure vanilla JS + Tailwind CSS CDN, no build step. 526 lines of production code. Closes Scottcjn/rustchain-bounties#6
feat: Miner Dashboard Explorer (Bounty #6)
Implements a comprehensive installer for RustChain miners supporting Linux (Ubuntu/Debian/Fedora) and macOS (Intel/Apple Silicon) with virtualenv isolation, persistent service setup, and clean uninstall.
Changes
Installer enhancements (
install.sh)~/.rustchain/venv- dependencies install without touching system Python--uninstallflag removes services, files, virtualenv, and configuration cleanly--wallet WALLET_NAMEargument for non-interactive installationsPlatform support
Documentation
README.mdwith installation examples and service management commandsINSTALL.md(371 lines) covering installation, troubleshooting, service management, and security notesUsage
Technical notes
python -m venvwith fallback tovirtualenvmodule~/.rustchain/miner.logFixes Scottcjn/rustchain-bounties#4