Tracking the non-blocking review findings from the tri-brain review of #6839 (the #6798 attestation pipe-signing fix). #6839 itself is correct and merges the Linux+Windows fix; these are hardening items that should not block it.
1. Test the inline fallback path, not just the helper
tests/test_attestation_signing_6798.py only imports and exercises miners/signing_helpers.py:build_pipe_sign_message. But setup_miner.py does not distribute signing_helpers.py — installs created by the setup script always take the _SIGNING_HELPERS = False branch and use the inline "{}|{}|{}|{}".format(...) constructor in each miner. That inline path produces byte-identical output today, but it is the path real users hit and it is not directly tested. Add a test that drives the actual Miner.attest() (or at least the inline branch) and asserts the bytes equal the server's reconstruction (miner_id|miner|nonce|commitment).
2. Distribute the shared helper, or drop it
Either add miners/signing_helpers.py to setup_miner.py's download set so installs use the shared (delimiter-checked) builder, or accept that the inline fallback is the real implementation and move the delimiter-safety check into the inline path too. Right now the helper's |-in-field guard only protects the (rarely-used) helper path.
3. Don't silently swallow signing errors
Both miners do except Exception: pass # Fall through unsigned. This is not a regression (pre-fix, signed attestations were 100% rejected, so unsigned-fallback is the same-or-better outcome), but a silent pass hides real signing bugs. Log the exception at WARNING so a broken keypair/helper is diagnosable.
4. macOS miner coverage (investigate)
The Mac miners (macos/rustchain_mac_miner_v2.4.py, v2.5.py) were intentionally not touched by #6839. They currently submit unsigned attestations (sha512 legacy pseudo-sig only, server-accepted) rather than Ed25519 — so they are not affected by #6798. If/when Mac miners adopt Ed25519 attestation signing, they must use the same pipe-string format. No action needed now; noting so it isn't forgotten.
Filed from tri-brain (Codex 5.5 + Grok + GPT-OSS) review; items 1-3 are SHOULD-FIX, item 4 is informational.
Tracking the non-blocking review findings from the tri-brain review of #6839 (the #6798 attestation pipe-signing fix). #6839 itself is correct and merges the Linux+Windows fix; these are hardening items that should not block it.
1. Test the inline fallback path, not just the helper
tests/test_attestation_signing_6798.pyonly imports and exercisesminers/signing_helpers.py:build_pipe_sign_message. Butsetup_miner.pydoes not distributesigning_helpers.py— installs created by the setup script always take the_SIGNING_HELPERS = Falsebranch and use the inline"{}|{}|{}|{}".format(...)constructor in each miner. That inline path produces byte-identical output today, but it is the path real users hit and it is not directly tested. Add a test that drives the actualMiner.attest()(or at least the inline branch) and asserts the bytes equal the server's reconstruction (miner_id|miner|nonce|commitment).2. Distribute the shared helper, or drop it
Either add
miners/signing_helpers.pytosetup_miner.py's download set so installs use the shared (delimiter-checked) builder, or accept that the inline fallback is the real implementation and move the delimiter-safety check into the inline path too. Right now the helper's|-in-field guard only protects the (rarely-used) helper path.3. Don't silently swallow signing errors
Both miners do
except Exception: pass # Fall through unsigned. This is not a regression (pre-fix, signed attestations were 100% rejected, so unsigned-fallback is the same-or-better outcome), but a silentpasshides real signing bugs. Log the exception at WARNING so a broken keypair/helper is diagnosable.4. macOS miner coverage (investigate)
The Mac miners (
macos/rustchain_mac_miner_v2.4.py,v2.5.py) were intentionally not touched by #6839. They currently submit unsigned attestations (sha512 legacy pseudo-sig only, server-accepted) rather than Ed25519 — so they are not affected by #6798. If/when Mac miners adopt Ed25519 attestation signing, they must use the same pipe-string format. No action needed now; noting so it isn't forgotten.Filed from tri-brain (Codex 5.5 + Grok + GPT-OSS) review; items 1-3 are SHOULD-FIX, item 4 is informational.