fix: show Windows attestation failures

[ramimbo]

BCOS Checklist (Required For Non-Doc PRs)

• Add a tier label: BCOS-L1
• If adding new code files, include SPDX header near the top (example: # SPDX-License-Identifier: MIT)
• Provide test evidence (commands + output or screenshots)

What Changed

• Surface Windows miner attestation failure details in headless error logs, including challenge exceptions, rejected challenge responses, non-JSON challenge HTTP errors, and submit HTTP status plus JSON code/error/message when available.
• Treat --wallet as a transient runtime override for Windows miner startup instead of saving it over the existing wallet file.
• Refresh the Windows miner setup hash for the changed artifact. I also updated the stale Linux setup hash because the existing setup artifact test validates all platform pins together.

Scope Notes

This is distinct from #5401 / #5411 / #5502, which cover missing successful attest/enroll lifecycle logs. This PR covers failure diagnostics and avoiding accidental wallet overwrite during diagnostic --wallet runs.

This replaces my closed #5571 with a fresh branch from current main after checking the repo and bounty rules.

Fixes #5575.

Testing / Evidence

• RED before initial fix: python3 -m pytest tests/test_windows_attestation_diagnostics.py -q -> 5 failed for the missing diagnostic state/log output and wallet overwrite behavior.
• RED for JSON challenge review follow-up: python3 -m pytest tests/test_windows_attestation_diagnostics.py::test_attest_records_challenge_rejection_details -q -> failed with TypeError when challenge returned HTTP 503 JSON without nonce.
• RED for non-JSON challenge review follow-up: python3 -m pytest tests/test_windows_attestation_diagnostics.py::test_attest_records_non_json_challenge_http_error -q -> failed with challenge request failed: not json instead of HTTP/body diagnostics.
• python3 -m pytest tests/test_windows_attestation_diagnostics.py tests/test_windows_miner_setup_checksum.py tests/test_setup_miner_downloads.py -q -> 11 passed.
• python3 -m py_compile miners/windows/rustchain_windows_miner.py tests/test_windows_attestation_diagnostics.py setup_miner.py -> passed.
• python3 tools/bcos_spdx_check.py --base-ref origin/main -> BCOS SPDX check: OK.
• git diff --check -> exit 0; Git reported the existing CRLF normalization warning for the Windows .bat file.
• curl -sk --max-time 10 https://rustchain.org/health -> ok: true.

RTC payout address: RTC877021895fd29d034f35c87e1b37af8534703792

[ramimbo]

CI note after latest push: all BCOS, build, labeling, welcome, and PoC checks passed on the previous run. The repository-wide test check has been failing during collection before running this PR's focused tests because the CI environment is missing unrelated optional dependencies:

• aiohttp for tests/test_agent_economy_sdk.py
• flask_cors for tests/test_faucet_service_wallet_validation.py
• matplotlib for tests/test_hardware_visualizer.py and tests/test_pse_analyze_results.py

Focused validation for this PR after the latest review follow-up:

python3 -m pytest tests/test_windows_attestation_diagnostics.py tests/test_windows_miner_setup_checksum.py tests/test_setup_miner_downloads.py -q
11 passed in 0.15s

[iamdinhthuan]

Reviewed current head b57750451c88dcec4ad2b3c4ab09ec2b9124bb1c for the Windows attestation diagnostics and wallet override changes.

What I checked:

• The new attestation failure path records challenge exceptions and submit rejections in last_attestation_error, then surfaces that detail through the headless callback without changing the successful attestation path.
• _response_diagnostic() keeps the HTTP diagnostic compact by reporting status plus selected JSON fields (code, error, message) or a bounded body preview.
• --wallet is now treated as a runtime override in both headless and GUI startup; the regression tests prove it does not overwrite the saved wallet file/data.
• The setup hash updates are covered by the checksum/download tests, including the refreshed Windows artifact hash and the stale Linux pin update.

Validation run locally on this head:

python3 -m pytest tests/test_windows_attestation_diagnostics.py tests/test_windows_miner_setup_checksum.py tests/test_setup_miner_downloads.py -q
9 passed
python3 -m py_compile miners/windows/rustchain_windows_miner.py tests/test_windows_attestation_diagnostics.py setup_miner.py
git diff --check origin/main...HEAD -- miners/windows/rustchain_miner_setup.bat miners/windows/rustchain_windows_miner.py setup_miner.py tests/test_windows_attestation_diagnostics.py
python3 tools/bcos_spdx_check.py --base-ref origin/main
BCOS SPDX check: OK

No blocker found from this review. The repository-wide CI test failure appears to be the known unrelated collection dependency issue called out in the PR comment, not a failure in this patch's focused coverage.

[iamdinhthuan]

Follow-up after the earlier approval on current head b57750451c88dcec4ad2b3c4ab09ec2b9124bb1c: I found one attestation failure path that still does not surface the diagnostic this PR is trying to expose.

The new code calls /attest/challenge, immediately reads .json(), and only stores last_attestation_error if that request raises. If the challenge endpoint returns a JSON error response without a nonce, attest() continues with nonce = None and then crashes while building the commitment before last_attestation_error is set.

Direct probe on this head:

challenge_response_status=503
challenge_payload={"code":"ATTEST_BUSY","error":"try later","message":"challenge unavailable"}
raised=TypeError unsupported operand type(s) for +: 'NoneType' and 'str'
last_error=

That means headless users still get an implementation exception instead of a useful attestation diagnostic for challenge-side HTTP/JSON failures. This should return False and set something like challenge rejected: HTTP 503 code=ATTEST_BUSY error=try later message=challenge unavailable, using the same bounded diagnostic helper as submit failures, before computing entropy/commitment.

Validation I ran on the PR head before this follow-up:

python3 -m pytest tests/test_windows_attestation_diagnostics.py tests/test_windows_miner_setup_checksum.py tests/test_setup_miner_downloads.py -q
9 passed
python3 -m py_compile miners/windows/rustchain_windows_miner.py tests/test_windows_attestation_diagnostics.py setup_miner.py
python3 tools/bcos_spdx_check.py --base-ref origin/main
BCOS SPDX check: OK
git diff --check origin/main...HEAD

Suggested regression: mock /attest/challenge to return a non-200 JSON object without nonce, assert attest() returns False, and assert _ensure_ready() emits the compact HTTP/code/error/message diagnostic instead of bubbling a TypeError.

[TJCurnutte]

Approved. I validated the diagnostic path and the installer hash updates at this head.

Proof run at head b57750451c88dcec4ad2b3c4ab09ec2b9124bb1c:

git diff --check origin/main...HEAD -- miners/windows/rustchain_miner_setup.bat miners/windows/rustchain_windows_miner.py setup_miner.py tests/test_windows_attestation_diagnostics.py
python3 -B -m py_compile miners/windows/rustchain_windows_miner.py setup_miner.py tests/test_windows_attestation_diagnostics.py
python3 -B -m pytest -q -o addopts='' --noconftest tests/test_windows_attestation_diagnostics.py

Result: 5 passed in 0.23s.

Additional hash probe:

• miners/windows/rustchain_windows_miner.py SHA-256 = aefeb005650ed6f004eb3a489f708c37bf457810a850d55691c5fec4876af56f
• setup_miner.py Windows SHA and miners/windows/rustchain_miner_setup.bat MINER_SHA256 both match that value.
• setup_miner.py Linux SHA also matches the in-tree Linux miner at 91815ecf25042cfea1c60817c8b6e701c4324b60ceeb433da068243920344c0a.

The useful part here is that Windows attestation failures now preserve the submit/challenge reason in last_attestation_error and surface it through _ensure_ready() instead of collapsing every failure to a generic Attestation failed. The tests also cover the wallet override persistence boundary, so headless/GUI supplied wallets do not silently rewrite the saved wallet file.

[508704820]

Security Review ✅

Reviewed. Windows attestation failure display improvement.

Reviewed by Xeophon - Solana: Lt9nERv6VHsojw15LpFeiaabuphAggzfLF9sM9UXRrZ

[ZacharyZhang-NY]

Reviewed current PR #5576 head ad34527.

Requesting changes for one remaining diagnostics gap: attest() calls challenge_resp.json() before it checks the challenge HTTP status or runs _response_diagnostic(). When /attest/challenge returns a non-JSON HTTP error body, the exception path records only the JSON parser failure and drops the useful HTTP status/body details.

Direct probe on this head:

challenge_response_status=503
challenge_response_body=temporarily unavailable
attest_result=False
last_error=challenge request failed: not json

That means headless users still see Attestation failed: challenge request failed: not json instead of a compact operator diagnostic such as challenge rejected: HTTP 503 body=temporarily unavailable. The submit path already handles non-JSON bodies through _response_diagnostic(); the challenge path should route non-200 responses through the same helper before JSON parsing failures erase the response context, or catch JSON decode errors with the response object still available.

Validation run:

git diff --check origin/main...HEAD -- miners/windows/rustchain_miner_setup.bat miners/windows/rustchain_windows_miner.py setup_miner.py tests/test_windows_attestation_diagnostics.py
python -B -m py_compile miners/windows/rustchain_windows_miner.py setup_miner.py tests/test_windows_attestation_diagnostics.py
python -B -m pytest -q tests/test_windows_attestation_diagnostics.py tests/test_windows_miner_setup_checksum.py tests/test_setup_miner_downloads.py --noconftest
python tools/bcos_spdx_check.py --base-ref origin/main

Result: diff check clean, py_compile clean, focused pytest 10 passed, BCOS SPDX check OK. Hash check also confirms the Windows and Linux artifact SHA-256 values match the updated setup pins.

[ZacharyZhang-NY]

Follow-up review on current head e368a6a.

Approved. The previous non-JSON challenge diagnostic gap is fixed: attest() now checks non-200 challenge responses before JSON parsing and routes them through _response_diagnostic(), so a plain-text HTTP 503 is surfaced as challenge rejected: HTTP 503 body=temporarily unavailable instead of challenge request failed: not json.

The focused regression coverage now includes JSON challenge rejection, non-JSON challenge HTTP error, submit rejection details, headless callback surfacing, and wallet override persistence.

Validation run:

git diff --check origin/main...HEAD -- miners/windows/rustchain_miner_setup.bat miners/windows/rustchain_windows_miner.py setup_miner.py tests/test_windows_attestation_diagnostics.py
python -B -m py_compile miners/windows/rustchain_windows_miner.py setup_miner.py tests/test_windows_attestation_diagnostics.py
python -B -m pytest -q tests/test_windows_attestation_diagnostics.py tests/test_windows_miner_setup_checksum.py tests/test_setup_miner_downloads.py --noconftest
python tools/bcos_spdx_check.py --base-ref origin/main

Result: diff check clean, py_compile clean, focused pytest 11 passed, BCOS SPDX check OK. Hash check confirms the Windows miner SHA-256 matches both setup_miner.py and miners/windows/rustchain_miner_setup.bat, and the Linux pin still matches the in-tree Linux miner.

[HCIE2054]

LGTM!

[minyanyi]

Approved. I reviewed the Windows headless miner attestation diagnostics patch for #5575 and #5401.

What I verified:

• Attestation challenge failures now preserve compact HTTP/JSON diagnostics instead of collapsing to a generic failure.
• Attestation submit rejections such as duplicate hardware now surface the relevant status/code/error/message through the headless callback.
• Successful attestation clears the cached error, so stale diagnostics do not bleed into later runs.
• --wallet overrides are used for the active miner process without overwriting the saved wallet file in headless or GUI mode.
• The updated Windows and Linux artifact hashes match the patched miner scripts in this branch.

Local validation:

• python -m pytest tests\test_windows_attestation_diagnostics.py -q -> 7 passed
• python -m py_compile miners\windows\rustchain_windows_miner.py tests\test_windows_attestation_diagnostics.py setup_miner.py
• git diff --check origin/main...HEAD
• Get-FileHash -Algorithm SHA256 miners\windows\rustchain_windows_miner.py -> 6EC3AAEFB068EA2BB5ADFF7A5C279848FD4B2DF01DE390411C62E954719C22D3
• Get-FileHash -Algorithm SHA256 miners\linux\rustchain_linux_miner.py -> 91815ECF25042CFEA1C60817C8B6E701C4324B60CEEB433DA068243920344C0A

[iamdinhthuan]

Rechecked current head e368a6a after the challenge-diagnostic follow-up. The challenge-side blocker from my earlier changes-requested review is fixed: non-200 challenge responses are handled before JSON parsing and use the same compact diagnostic helper, so plain-text HTTP failures surface as useful operator diagnostics instead of dropping to a JSON parser error or TypeError.

Validation I reran on the current head:

python3 -B -m pytest -q tests/test_windows_attestation_diagnostics.py tests/test_windows_miner_setup_checksum.py tests/test_setup_miner_downloads.py --noconftest
# 11 passed
python3 -B -m py_compile miners/windows/rustchain_windows_miner.py tests/test_windows_attestation_diagnostics.py setup_miner.py
git diff --check origin/main...HEAD -- miners/windows/rustchain_miner_setup.bat miners/windows/rustchain_windows_miner.py setup_miner.py tests/test_windows_attestation_diagnostics.py
python3 tools/bcos_spdx_check.py --base-ref origin/main
# BCOS SPDX check: OK

I also checked the artifact pins: the Windows miner SHA-256 matches both setup_miner.py and the Windows .bat pin, and the Linux miner SHA-256 matches setup_miner.py. No additional bounty claim from me for this follow-up; this just resolves my previous changes-requested finding.

[jaxint]

LGTM! Great work on this PR. 🚀

[HCIE2054]

LGTM!

[kekehanshujun]

I reviewed the Windows attestation diagnostics patch and verified the final Windows miner artifact on the PR branch hashes to the value pinned in setup_miner.py:

6ec3aaefb068ea2bb5adff7a5c279848fd4b2df01de390411c62e954719c22d3

The user-facing attestation error path now preserves challenge/submit rejection details, and the wallet override change avoids overwriting the saved wallet when running one-off headless/GUI commands. The tests cover challenge failures, submit rejections, and wallet override persistence. I do not see a blocker.

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[HCIE2054]

LGTM!

[Scottcjn]

Closing as stale branch — would cause destructive deletions if merged.

Your branch is 155 commits behind current main. Filed during the May 11-13 contributor burst, the codebase has since moved substantially. GitHub's mergeable=clean indicator doesn't detect this; only an explicit git diff main..your-branch --shortstat does.

Bounty credit acknowledged where applicable

Most of the canonical fixes from your work-period have already shipped via other contributors' parallel PRs that landed earlier this week. Specific cases credited via the Codex audit batches:

• Age-scoring (2025 → current_year) — credited in synthesis PR fix(scoring): defensive max(0, ...) age guards + testable current_year param #5660
• Input validation / fix: validate X / fix: reject X — landed via first-poster merges
• Info-disclosure hardening (hide X exception details) — landed via @hungle123-dev first-poster series
• CONTRIBUTING.md per-repo — closed bounty Auto Batch - Divine Features (250 RTC) #1605 with first-poster awards on 2026-05-18

If you want fresh review

Rebase against current main and verify your diff matches the size of your original changes:

git fetch upstream main
git rebase upstream/main
git diff main..HEAD --shortstat

If the deletion count is much higher than what you intended, the branch is still picking up stale assumptions — recreate from a fresh main.

Thanks for the contribution work.