Skip to content

test: cover Base deposits and strengthen Zcash rejection checks - #126

Draft
BitHighlander wants to merge 1 commit into
reconcile/upstream-syncfrom
audit/715-base-zcash-review-evidence
Draft

BitHighlander wants to merge 1 commit into
reconcile/upstream-syncfrom
audit/715-base-zcash-review-evidence

Conversation

@BitHighlander

@BitHighlander BitHighlander commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

SOP correction: this fork draft is supplemental test preparation. Firmware staging retains the canonical upstream keepkey/python-keepkey #197 head c1cb6afab2a60f6d20f196940c933bfcfabee106. These changes must be reconciled into that canonical PR branch before release adoption. Human upstream review and upstream dependency master merges precede firmware develop merges; firmware upstreaming comes last.

Two Zcash rejection tests could pass on unrelated failures. Use a complete PCZT spend with a matching-fingerprint signing control, require the exact fingerprint refusal without action streaming or signature release, and check a valid path-only address request before the exact bad-path refusal.

Add the two observed Base THORChain deposit vectors, sign them with AdvancedMode disabled, recover the expected wallet signer, and reject unrelated router addresses and chains. The four-file change affects tests and fixtures; API and runtime behavior are unchanged.

Validation:

  • Base integration: 8 methods / 10 JUnit cases passed on the exact b2 candidate (4e9ba27fda482060b7bf7776006276e5d89bc99f) with the pinned PYK source.
  • Zcash: 3 focused cases passed against integrated firmware 478ec1c32b3925729e16b4899345a1246589d626.
  • Disposable firmware controls bypassing the fingerprint check or rejecting every path-only request fail the corrected tests; the original weak tests still pass. These controls are evidence of test sensitivity, not proposed firmware changes.
  • The combined four-file diff passes targeted Python syntax checks and fixture consistency checks. The unchanged emulator/native suites were not repeated during packaging; fresh CI on this commit has now passed, as recorded below.
  • The workflow's pinned 7.15 firmware 849a75bc6bc62fda701c18fb91a30588fa8055b5 has byte-identical PCZT and Zcash source files to the integrated firmware used for the focused Zcash proof.

Base: c1cb6afab2a60f6d20f196940c933bfcfabee106 (reconcile/upstream-sync). Keep this PR in draft during preparation so the automatic review workflow stays gated.

CI run 38484575571, attempt 2 and CircleCI canonical-smoke passed at 8f73901d0a2262d8df16c28aaf13ee875ab80d6f. Full 7.15: 968 methods passed, 113 skipped and 342 passing subtests; full 7.14.3: 605 passed, 476 skipped and 211 passing subtests. Both BTC boundary rows passed 11/11 with no skips. The three focused Zcash methods and Base replay passed on 7.15, including all eight Base module methods. Successful replay transaction IDs are not individually logged; their coverage follows from the exact two-record fixture and passing loop. The unchanged workflow emits JUnit summary logs but uploads no raw XML; saved job logs retain the counts and case outcomes. Attempt 1 was interrupted by a runner shutdown after the new cases passed; only its failed job was rerun, without changing this head.

The superseded historical firmware candidate for BitHighlander/keepkey-firmware#982 passed with this fork-only Python pin at firmware 3a78bd69e77861ab3d1d95bab5bf267243a4c3a6: full main raw wire XML is 1,000 passed/92 skipped, with all four new/corrected methods passing. Keep the different firmware pins and collection scopes separate.
Firmware #982 now restores canonical upstream pins and the upstream keepkey#197 report URL at d7bc25bf35c0e9110407c0dd17d49290066f7251. This fork commit is no longer adopted by that candidate.

Replay two observed Base THORChain deposits with signer recovery and refusal
controls. Make Zcash fingerprint and account-path tests require the intended
refusal, with valid signing and path-only controls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant