Skip to content

[baobao] Fix Linux miner checksums for PR #6864#6876

Closed
mkcash wants to merge 1 commit into
Scottcjn:mainfrom
mkcash:fix-linux-miner-checksums
Closed

[baobao] Fix Linux miner checksums for PR #6864#6876
mkcash wants to merge 1 commit into
Scottcjn:mainfrom
mkcash:fix-linux-miner-checksums

Conversation

@mkcash
Copy link
Copy Markdown

@mkcash mkcash commented Jun 5, 2026

Fixes #6864 (Code review bounty #73)

Implemented via NVIDIA AI.

Updates miner checksums to match the new Linux miner artifact SHA256:

  • miners/checksums.sha256: Updated hash
  • setup_miner.py: Updated LINUX_MINER_SHA256 pin

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Non-doc PRs have a BCOS-L1 or BCOS-L2 label
  • Doc-only PRs are exempt from BCOS tier labels when they only touch docs/**, *.md, or common image/PDF files
  • New code files include an SPDX license header
  • You've tested your changes against the live node

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!

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/XS PR: 1-10 lines labels Jun 5, 2026
@zqleslie
Copy link
Copy Markdown
Contributor

zqleslie commented Jun 5, 2026

Review of PR #6876

The checksum hash update is correct — it matches the new Linux miner artifact SHA256 from PR #6864 (a0e85f8b...). However, there's a blocking issue in miners/checksums.sha256:

Bug: The Linux miner path was changed from linux/rustchain_linux_miner.py to just rustchain_linux_miner.py (missing the linux/ directory prefix). This is inconsistent with all other entries in the file (which use linux/ and macos/ subdirectory paths) and will break the checksum verification flow.

Compare with sibling PR #6877 which correctly preserves linux/rustchain_linux_miner.py.

Required fix: Change line 1 of miners/checksums.sha256 to:

a0e85f8bddaf3db183a200a015307e1e069ae3a1e656d8ba530b90b8f374d39c  linux/rustchain_linux_miner.py

Tests to run after fixing:

  • python -m pytest tests/test_install_miner_checksums.py tests/test_setup_miner_downloads.py -q
  • bash scripts/check_fetchall.sh

The setup_miner.py change is correct — the LINUX_MINER_SHA256 pin matches the new artifact hash.

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! 👍

Copy link
Copy Markdown
Contributor

@JesusMP22 JesusMP22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: Fix Linux miner checksums for PR #6864

Summary: Companion PR to #6877, fixing Linux miner checksums for the changes introduced in PR #6864.

What I like:

  • Consistent approach with the companion PR #6877
  • Platform-specific fixes show attention to cross-platform compatibility

Suggestions:

  1. Consider consolidating checksum logic to reduce divergence between platforms
  2. Add integration tests that verify checksums on both Linux and non-Linux platforms
  3. Document any platform-specific checksum differences

Security considerations:

  • Same considerations as #6877: checksum integrity is critical for miner trust
  • Platform-specific code paths should have equal security scrutiny

Verdict: ✅ Good companion fix. Cross-platform consistency matters.

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thanks for contributing.

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! 🎉

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution! Great work.

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Thanks for contributing. 👍

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! 🙏

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Thanks for contributing.

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great work.

@Scottcjn
Copy link
Copy Markdown
Owner

Scottcjn commented Jun 5, 2026

Closing — this targets the already-closed #6864, and the hash it pins (a0e85f…) does not match the actual miners/linux/rustchain_linux_miner.py on main (which is correctly pinned as 96c165…). Applying this would break the checksum check, not fix it. Main's manifest is already self-consistent — nothing to fix here. 🦞

@Scottcjn Scottcjn closed this Jun 5, 2026
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! This PR has been reviewed.

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

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/XS PR: 1-10 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants