Skip to content

Conversation

@BuilderFred
Copy link
Contributor

Motivation

To lower the barrier to entry for new miners, this PR adds a robust, cross-platform installer script (install.sh) that automates environment setup and persistence.

Features

  • Auto-Detection: Identifies OS (Ubuntu, Debian, Fedora, macOS) and architecture.
  • Sandbox Environment: Sets up a Python virtual environment to avoid system pollution.
  • Persistence: Automatically configures systemd (Linux) or launchd (macOS) to ensure the miner starts on boot and restarts on failure.
  • User Friendly: Supports interactive wallet setup or command-line flags.
  • Clean Removal: Includes an --uninstall flag for easy cleanup.

Fixes Scottcjn/rustchain-bounties#4.

@Scottcjn
Copy link
Owner

Scottcjn commented Feb 2, 2026

@BuilderFred A few things:

  1. This overwrites samuel-asleep's installer that merged to main 30 minutes ago (PR Add production-ready one-command miner installer with virtualenv isol… #6). Completely replacing another contributor's merged work isn't great practice — especially when the previous PR went through multiple review rounds.

  2. If your installer is genuinely better, consider one of these approaches:

    • Add it as a separate file (install-v2.sh or install-advanced.sh) rather than replacing install.sh
    • Or identify specific improvements and submit them as incremental patches to the existing installer
  3. Technical question: Your version uses git clone of the entire repo, while samuel-asleep's downloads individual miner files. The git clone approach means the user gets 100+ MB of repo instead of just the miner script. That's a tradeoff worth discussing.

  4. Testing: Has this been tested with an actual miner attestation to https://50.28.86.131? The bounty requires on-chain verification — the miner installed by your script must successfully attest.

Not rejecting this — but it needs a clear case for why it should replace rather than complement the existing installer. What does yours do that the current one doesn't?

@Scottcjn
Copy link
Owner

Scottcjn commented Feb 2, 2026

Update: We're accepting both installer approaches. The more options for new miners, the better. Reviewing for merge now.

@samuel-asleep Your original installer is valued — this is an alternative approach, not a replacement of your contribution. Both earned their bounties.

@Scottcjn
Copy link
Owner

Scottcjn commented Feb 2, 2026

Quick review notes (non-blocking — merging to keep momentum):

Issues to fix in a follow-up:

  1. read -p won't work in curl | bash piped context — needs read -p ... < /dev/tty (same bug samuel-asleep fixed in PR Add production-ready one-command miner installer with virtualenv isol… #6)
  2. Line 124 logic bug: command -v git >/dev/null 2>&1 || sudo apt-get update && sudo apt-get install -y git — the && chains wrong without braces
  3. Drops PowerPC support — samuel-asleep's version had PPC G4/G5/POWER8 platform detection and per-platform miner downloads. This one only handles Linux + macOS.
  4. --wallet flag parsing on lines 205-208 checks $2 twice redundantly

Good additions:

  • --uninstall flag
  • venv isolation
  • systemd + launchd auto-start
  • Cleaner structure

These can be fixed in a follow-up commit. Merging now.

@Scottcjn
Copy link
Owner

Scottcjn commented Feb 2, 2026

@BuilderFred This PR has a merge conflict with the current install.sh on main (from PR #6).

Two options:

  1. Rebase on main and resolve the conflict: git fetch origin && git rebase origin/main
  2. Rename your installer to install-alt.sh or install-v2.sh so both versions coexist

Either way works — just push the fix and we'll merge.

@Scottcjn
Copy link
Owner

Scottcjn commented Feb 2, 2026

@BuilderFred — This PR has a merge conflict with main (samuel-asleep's installer merged as PR #6).

Can you rebase on main? Two options:

  1. Rebase and rename to install-v2.sh so both installers coexist
  2. Rebase and keep install.sh — but you'll need to resolve the conflict with the existing file

Option 1 is cleaner since samuel-asleep's installer is already live. Your version has nice additions (venv setup, systemd/launchd persistence, uninstall flag) that complement the existing one.

Also heads up on two bugs from the earlier review:

  • read -p won't work when piped via curl | bash (stdin is the script)
  • Line 124: command -v git >/dev/null 2>&1 || sudo apt-get update && sudo apt-get install -y git — needs grouping with { } or the && chains unconditionally

Let me know if you need help with the rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BOUNTY] Build RustChain miner installer script (Linux + Mac)

2 participants