Skip to content

test(setup): lock miner service templates to pass --wallet (#5713) - #8317

Open
jjb9707 wants to merge 1 commit into
Scottcjn:mainfrom
jjb9707:test/5713-service-wallet-regression
Open

test(setup): lock miner service templates to pass --wallet (#5713)#8317
jjb9707 wants to merge 1 commit into
Scottcjn:mainfrom
jjb9707:test/5713-service-wallet-regression

Conversation

@jjb9707

@jjb9707 jjb9707 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Problem

miners/linux/rustchain_linux_miner.py takes the wallet only from the --wallet CLI argument (rustchain_linux_miner.py:952, 968) and LocalMiner.__init__ falls back to _gen_wallet() when it is missing. A generated service that merely exports WALLET_NAME / RUSTCHAIN_WALLET into the environment therefore mines under a throwaway wallet — rewards and balance checks never line up with the wallet the user entered during setup.

That was issue #5713 and PR #5711 fixed the templates, but the issue is still open and nothing guards the fix. Two contributors have since re-opened work against it (#7880, closed unmerged), so the regression surface is real.

Verified current state on main (@ 5a9d6a8)

Generated unit Wallet passed? Location
setup.sh systemd ExecStart yes setup.sh:327
setup.sh launchd ProgramArguments yes setup.sh:358-359
scripts/install.sh systemd ExecStart yes scripts/install.sh:383
scripts/install.sh launchd ProgramArguments yes scripts/install.sh:421-422

Change

One new test file, tests/test_miner_service_wallet_5713.py (no production code touched). Five assertions:

The helpers match on MINER_SCRIPT / MINER_FILENAME / rustchain_linux_miner.py rather than on fixed variable names, so renaming a shell variable will not silently void the guard.

Verification

Ran the tests against the real main copies of both scripts, then re-ran them with --wallet stripped from all four templates (the pre-#5711 shape), then restored:

① real main files            5 PASS / 0 FAIL
② --wallet stripped          0 PASS / 5 FAIL   <- guard fires, test is not vacuous
③ restored                   5 PASS / 0 FAIL

python -m py_compile tests/test_miner_service_wallet_5713.py -> OK.

Closes #5713

…5713)

The generated systemd units and launchd plists in setup.sh and
scripts/install.sh were corrected in PR Scottcjn#5711 to pass the configured
wallet on the command line, but nothing guards that.

rustchain_linux_miner.py reads the wallet only from --wallet and falls
back to LocalMiner._gen_wallet() otherwise, so an environment-only
template silently mines under a throwaway wallet.

Adds five assertions covering all four generated units.

Closes Scottcjn#5713
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

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) tests Test suite changes size/M PR: 51-200 lines labels Sep 1, 2026
@jjb9707

jjb9707 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Reminder: upstream main is currently green (tip 5a9d6a8a, CI passing). This PR's bot checks (label / welcome / size-label) are all success, and it is mergeable with no conflicts. The CI workflow was not auto-triggered on the fork branch (expected GitHub behavior for fork PRs). Requesting a maintainer (@Scottcjn) to trigger CI and review for merge. Thanks!

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/M PR: 51-200 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Miner service templates ignore the configured wallet unless --wallet is passed

1 participant