fix: rebase miner signing dependency installer changes#6883
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! |
jaxint
left a comment
There was a problem hiding this comment.
Thanks for the contribution! 🎉
jaxint
left a comment
There was a problem hiding this comment.
Thanks for this contribution! Great work.
szx19970521
left a comment
There was a problem hiding this comment.
Reviewed install-miner.sh and miners/checksums.sha256.
Two specific observations:
-
The PR adds a checksum for
macos/fingerprint_checks.py, butinstall-miner.shstill always downloads and verifieslinux/fingerprint_checks.pyasfingerprint_checks.pyfor every platform. If macOS should use its platform-specific helper, the script should choose aFINGERPRINT_FILEper platform and verify that same path; if the Linux helper is intentionally shared, the new macOS checksum entry is misleading/dead metadata. -
The new required downloads for
requirements-miner.txtandminer_crypto.pyusecurl -sSLwithout-f. In checksum-enabled installs a missing file will probably fail at checksum verification, but with--skip-checksumor dry-run-like paths the installer can keep going with an HTML/404 payload and fail later atpip install -r, which is harder to diagnose. Usingcurl -fsSLfor these required artifacts would make missing release paths fail immediately like the checksum download already does.
Positive note: downloading dependencies from the platform requirements file is the right direction; it keeps installer behavior aligned with the miner package instead of hardcoding only requests.
I received RTC compensation for this review.
jaxint
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
jaxint
left a comment
There was a problem hiding this comment.
Nice work! Thanks for contributing. 👍
Summary
maininstall-miner.shdownloading/verifying the platform requirements fileminer_crypto.pybefore installing dependenciesminers/checksums.sha256for the current rebased treeThis is an alternate PR for the fix discussed on #6705, because the original PR branch is owned by another fork and I cannot push the rebase there.
Validation
python -m py_compile miners/linux/rustchain_linux_miner.py miners/linux/miner_crypto.py miners/linux/fingerprint_checks.py miners/macos/fingerprint_checks.pyminers/checksums.sha256miners/linux/requirements-miner.txt, thenimport requests, nacl.signinggit diff --check origin/main...HEADNot run:
bash -n install-miner.sh, because the local Windows environment did not havebash.exeavailable.