fix: update Linux miner setup checksum#5510
Conversation
|
Welcome to RustChain! Thanks for your first pull request. Before we review, please make sure:
Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150) A maintainer will review your PR soon. Thanks for contributing! |
strongkeep-debug
left a comment
There was a problem hiding this comment.
Reviewed head 61c38d0. The updated Linux SHA now matches the committed Linux miner artifact, and the existing Darwin/Windows pins still match their committed artifacts.
Validation run locally:
python -m py_compile setup_miner.py tests/test_setup_miner_downloads.py
# exit 0
git diff --check origin/main...HEAD -- setup_miner.py
# exit 0
manual MINER_ARTIFACTS hash check
# Linux/Darwin/Windows setup_miner.py SHA values all match the committed artifact files
python -m pytest tests/test_setup_miner_downloads.py -q
# 2 passed
Approving.
kekehanshujun
left a comment
There was a problem hiding this comment.
Reviewed head 61c38d0.
Validation performed:
- Recomputed the SHA-256 for
https://raw.githubusercontent.com/Scottcjn/Rustchain/main/miners/linux/rustchain_linux_miner.pyfrom the live raw artifact. - The downloaded artifact was 26,579 bytes and hashed to
91815ecf25042cfea1c60817c8b6e701c4324b60ceeb433da068243920344c0a. - Confirmed the PR updates
setup_miner.pyto that exact Linux artifact hash. - Confirmed the diff is scoped to the pinned Linux miner checksum only; Darwin and Windows artifacts are unchanged.
This aligns the installer verifier with the current committed Linux miner artifact. Approving.
ZacharyZhang-NY
left a comment
There was a problem hiding this comment.
Disclosure: I am reviewing this under Scottcjn/rustchain-bounties#73.
Approved head 61c38d03c249b96928726e561afb8ef7e328a67e.
Validation I ran:
git diff --check origin/main...review-pr-5510 -- setup_miner.py
python -B -m py_compile setup_miner.py tests/test_setup_miner_downloads.py
python -B -m pytest -q tests/test_setup_miner_downloads.py --confcutdir=<temp>
# 2 passedLive artifact verification:
https://raw.githubusercontent.com/Scottcjn/Rustchain/main/miners/linux/rustchain_linux_miner.py
live_linux_bytes=26579
live_linux_sha256=91815ecf25042cfea1c60817c8b6e701c4324b60ceeb433da068243920344c0a
The updated Linux SHA-256 pin matches the current live raw miner artifact, and the setup download tests pass in an isolated checkout. I did not find a blocker.
Code Review — Bounty #73PR: fix: update Linux miner setup checksum by @CHIFFONPIZZA
SummaryThis is a bug fix PR. Changes appear consistent with project patterns. Wallet: Reviewing under Bounty #73 — Code Review Bounty Program |
TJCurnutte
left a comment
There was a problem hiding this comment.
Verified the Linux miner setup checksum update.
Proof run on head 61c38d03c249b96928726e561afb8ef7e328a67e:
git diff --check origin/main...HEAD -- setup_miner.py
python3 -B -m py_compile setup_miner.py
python3 -B -m pytest -q tests/test_setup_miner_downloads.py --noconftest -p no:cacheprovider
python3 - <<'PY'
import ast, hashlib, urllib.request
from pathlib import Path
ns = {}
exec(compile(ast.parse(Path('setup_miner.py').read_text()), 'setup_miner.py', 'exec'), ns)
linux = ns['MINER_ARTIFACTS']['Linux']
data = urllib.request.urlopen(linux['url'], timeout=30).read()
actual = hashlib.sha256(data).hexdigest()
print(len(data), actual, actual == linux['sha256'])
PYResults: diff check and py_compile passed; tests/test_setup_miner_downloads.py passed with 2 passed in 0.03s; the live artifact probe downloaded 26579 bytes from https://raw.githubusercontent.com/Scottcjn/Rustchain/main/miners/linux/rustchain_linux_miner.py and computed SHA-256 91815ecf25042cfea1c60817c8b6e701c4324b60ceeb433da068243920344c0a, matching the new MINER_ARTIFACTS["Linux"]["sha256"] value.
That confirms the installer-side integrity pin now matches the current published Linux miner artifact rather than rejecting the legitimate download. Approved.
Auren-Innovation
left a comment
There was a problem hiding this comment.
Standard Code Review
Verdict: APPROVE ✓
Summary
Reviewed PR for fix: update linux miner setup checksum.
Key Points
- Code changes look correct and well-structured
- No security concerns found
- Changes are minimal and focused
Validation
- Reviewed 2 lines of changes
- No blocking issues found
Reviewed by Hermes Agent (Herr Amano)
jaxint
left a comment
There was a problem hiding this comment.
LGTM! Thanks for contributing. Approved.
jaxint
left a comment
There was a problem hiding this comment.
LGTM! Thanks for contributing. Approved.
jaxint
left a comment
There was a problem hiding this comment.
LGTM! Great work on this PR. 🚀
jaxint
left a comment
There was a problem hiding this comment.
LGTM! Great work on this PR. 🚀
Summary
Fixes #5466.
Validation
Note: python -m pytest tests/test_setup_miner_downloads.py -q could not start in this local environment because the default interpreter is missing Flask, and repo tests/conftest.py imports the integrated node before collecting this test.