Skip to content

test(7.15): consolidated Python harness, bindings, and device coverage - #197

Open
BitHighlander wants to merge 135 commits into
masterfrom
reconcile/upstream-sync
Open

test(7.15): consolidated Python harness, bindings, and device coverage#197
BitHighlander wants to merge 135 commits into
masterfrom
reconcile/upstream-sync

Conversation

@BitHighlander

@BitHighlander BitHighlander commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Purpose

This is the single master-targeting Python release PR for firmware 7.15 / RC18. It consolidates the original #197 work, all seven commits from #199, and the review/test commits that previously had no PR. No history was rewritten; reconcile/upstream-sync was advanced normally.

Contents

  • regenerated 7.15 protocol bindings;
  • Hive sign-message and sign-operations coverage;
  • clearsign v2 and identity-icon contract coverage;
  • Maya signature recovery and release-report coverage;
  • Hive phase-2/3 screenshot selection;
  • Osmosis client-path fixes, wire-review invariants, denomination binding, and corrected report mapping;
  • Ethereum typed-hash policy-gate coverage;
  • host and exact-firmware regression coverage for RC18 clearsign persistence;
  • Zcash ZIP-32 seed-fingerprint helpers and device tests;
  • complete RC18 PCZT host flow for transparent outputs, transparent inputs, and Orchard actions;
  • an RC18 report catalog aligned with the regular-with-Orchard plus bitcoin-only release shape.

RC18 Zcash contract

The regular/full RC18 firmware includes Orchard privacy. Only the bitcoin-only build compiles non-Bitcoin features out; there is no separate Zcash artifact.

zcash_sign_pczt implements the exact firmware 7.15 conversation:

  • sends all transaction header fields and transparent component counts;
  • streams transparent outputs, then transparent inputs, then Orchard actions only when requested;
  • requires every Orchard action to declare is_spend explicitly;
  • rejects host-supplied transparent sighashes;
  • rejects duplicate, missing, or out-of-range device requests;
  • reads the deferred transparent signature response before the final PCZT response;
  • treats Orchard signatures as a compact list: one 64-byte RedPallas signature for each real spend, in ascending action order; dummy shield/change actions produce no device signature.

Deterministic scripted-flow tests cover all-dummy shield, mixed deshield, private Orchard send, compact signature ordering, malformed requests, signature-count mismatches, and preflight rejection.

Protocol provenance

Validation

  • exact-head Python CI run 30218738108 is green on addc0242847cd7be9c402980498b321b344bef34;
  • deterministic RC18 PCZT contract tests pass locally and in CI;
  • the report catalog validator resolves every 7.15 entry against the exact firmware JUnit artifact;
  • the corrected report regenerates as 306 tests: 303 passed, 3 skipped, 0 failed, 0 pending;
  • Orchard is represented accurately as part of the regular product with 21/21 cataloged checks, not as withheld/default-off;
  • firmware run 30217661980 proved the restored crypto fork projection green before this report-only Python repin;
  • firmware run 30218813159 pins this exact Python head and is fully green as the final dependency-graph rehearsal;
  • that exact run collected 593 Python tests (581 passed, 12 skipped, 0 failed), passed the 187-test screenshot selection, and captured 897 OLED PNGs;
  • its generated PDF reports 303/306 passed, 3 skipped, 0 failed, and Zcash Orchard 21/21 in the regular product.

Review and merge gates

  • singular master-targeting branch contains all intended Python release work;
  • deterministic RC18 PCZT contract tests are mandatory in GitHub CI;
  • current Python GitHub Actions CI is green;
  • release report matches the actual Orchard product shape and current test names;
  • device-protocol add 0x swap signing tests #112 human-reviewed and merged;
  • repin this PR to the resulting canonical device-protocol master commit and verify generated bindings;
  • human approval and merge of this PR;
  • repin firmware #320 to the canonical dependency merge commits and rerun final exact-head CI.

Do not merge this PR until the protocol review and canonical repin are complete.

BitHighlander and others added 30 commits April 28, 2026 19:11
Same firmware as the standalone UDP kkemu binary, loaded in-process via
ctypes. Lets python-keepkey exercise the firmware contract that the
keepkey-vault FFI path imposes — most importantly, the caller-driven
polling model (no daemon thread to call kkemu_poll for you).

- keepkeylib/transport_dylib.py: DylibState (process-wide singleton over
  ctypes-loaded libkkemu) + DylibTransport (one per iface 0/1).
  Pumps kkemu_poll on every read/write so the firmware actually makes
  forward progress on caller turns.
- tests/config.py: KK_TRANSPORT=dylib KK_DYLIB=/path/to/libkkemu.dylib
  routes the same fixture to the FFI transport instead of UDP.
- tests/test_dylib_confirm_flow.py: regression for the confirm-flow
  contract (Initialize, WipeDevice, LoadDevice, GetAddress). Skipped
  unless KK_TRANSPORT=dylib so it won't break the default UDP run.

Reproduces the keepkey-vault hang deterministically: Initialize round-
trips fine, wipe_device hangs because confirm_helper busy-loops on a
ButtonAck the dylib silently consumed but never delivered. Caught in
~10s, no electrobun / bun stack required.

Run:
  cd tests && KK_TRANSPORT=dylib KK_DYLIB=.../libkkemu.dylib \
    PYTHONPATH=..:../keepkeylib python3 -m pytest \
    test_dylib_confirm_flow.py -v
…ntics

The existing test_dylib_confirm_flow covers the caller-driven polling
contract — Initialize / Wipe / LoadDevice / GetAddress — but never asks
the firmware for a layout. Two changes that just landed in the firmware
emulator runtime PR (BitHighlander/keepkey-firmware#217) need functional
coverage that confirm-flow doesn't provide:

1. RINGBUF_CAPACITY in lib/emulator/ringbuf.h was bumped from 32 to
   128. DebugLinkState's 2048-byte `layout` plus the rest of the message
   serializes to ~44 HID reports through the output ring; the previous
   capacity left effective room for 31 reports, so screenshot capture
   truncated mid-layout (msg_debug_write ignores emulatorSocketWrite's
   0-on-full return).

2. fsm_msgDebugLinkGetState in lib/firmware/fsm_msg_debug.h now does a
   single display_refresh() instead of force_animation_start() +
   animate(). The old form overwrote static layouts with stale animation
   frames or no-ops depending on queue state, so screenshots captured
   something different from what the user was seeing.

Both fixes are functionally invisible to the existing test suite. Without
these tests, regressing either change ships green.

This commit adds:

- tests/test_dylib_screenshot.py — four tests:
    * test_layout_round_trip_fits_through_ring   (RINGBUF_CAPACITY)
    * test_layout_repeated_reads_no_truncation   (RINGBUF_CAPACITY)
    * test_layout_stable_across_idle_reads       (canvas semantics)
    * test_layout_features_dont_corrupt_capture  (iface separation)

  Constructs a fresh KeepKeyDebuglinkClient against the dylib singleton
  WITHOUT going through common.KeepKeyTest.setUp — that fixture wipes the
  device on every test and exercises the confirm-flow path that
  test_dylib_confirm_flow is itself a pending regression for. Reading a
  layout doesn't require any of that; we just init and ask DebugLink for
  the home-screen capture.

- tests/config.py — explicit-transport precedence fix:

  Previously HID/WebUSB were always autodetected first. With a real
  KeepKey plugged in, KK_TRANSPORT=dylib was silently overridden — the
  dylib regression suite would either route to hardware or crash on
  hid.pyx. Now the explicit env var (KK_TRANSPORT=dylib) skips hardware
  enumeration entirely, the dylib path runs as requested, and the default
  (no env var set) falls back to the existing UDP behavior.

Verified locally:

  cmake -DKK_EMULATOR=1 -DKK_BUILD_DYLIB=1 -DKK_DEBUG_LINK=ON \
        -DCMAKE_POLICY_VERSION_MINIMUM=3.5 -B build-emu .
  cmake --build build-emu --target kkemulator_dylib

  KK_TRANSPORT=dylib KK_DYLIB=build-emu/lib/libkkemu.dylib \
    PYTHONPATH=keepkeylib:. python -m pytest tests/test_dylib_screenshot.py
  ======================== 4 passed in 0.36s ========================

Out of scope: SignTx + other multi-step flows that go through
confirm_helper. They share the same hang as test_dylib_confirm_flow's
test_load_device_with_auto_confirm — copying the pattern would just
produce a second red regression for the same underlying firmware bug,
not new coverage. Once the confirm-flow regression goes green, signtx
expansion is a follow-up.
…ANSPORT, split confirm-flow setUp

Three findings from review of PR #14:

#1 (High) test_dylib_confirm_flow used common.KeepKeyTest.setUp which calls
   wipe_device() — the same path the file's pending regression is for.
   Hangs in setUp can't be classified by xfail or interrupted by
   pytest-timeout, so test_features_round_trip ("just Initialize") was
   actually wipe + Initialize. Refactored to construct
   KeepKeyDebuglinkClient directly in setUp (matching test_dylib_screenshot's
   pattern), moved wipe + load_device into the one pending test.

   Tried the reviewer-suggested @pytest.mark.xfail(strict=True) +
   @pytest.mark.timeout combo. pytest-timeout (both signal and thread
   methods) cannot interrupt the C-level kkemu_poll busy-loop — the
   hang locks up the entire test runner instead of failing the test.
   Switched to @unittest.skip with explicit rationale documenting
   exactly that, plus the promotion path: when firmware lands the
   confirm fix, drop the skip; if a future change makes kkemu_poll
   GIL-friendly, switch back to xfail+timeout.

#2 (Medium) tests/config.py treated any non-empty KK_TRANSPORT as
   "explicit" and skipped HID/WebUSB autodetect, but only "dylib" was
   actually handled. A typo like KK_TRANSPORT=dyllib silently fell
   through to UDP with hardware disabled. Now scoped to a
   _KNOWN_TRANSPORTS set; unsupported values raise at config import,
   surfacing typos at test collection time. Verified end-to-end:
   `KK_TRANSPORT=dyllib pytest test_msg_signtx.py` now errors on
   collection with the typo'd value in the message.

#3 (Medium/Low) DylibTransport.ready_to_read appended raw frame bytes
   to read_buffer but DylibTransport._pump_one stripped the leading '?'
   HID marker first. Inconsistent stripping corrupts multi-frame message
   reassembly: _read_headers can scan a stray '?' from one chunk into
   the middle of contiguous payload bytes from another, decoding the
   wrong message-type / length.

   Centralised the read+strip into a private _poll_and_stash helper
   shared by both ready_to_read (no sleep) and _pump_one (sleeps on
   miss). Now the buffer always contains continuation+payload bytes
   only; the leading '?' is stripped at the single point of stashing.
   Trailing HID padding zeros from short messages are still tolerated
   by _read_headers' magic-character search.

Verified locally:

  KK_TRANSPORT=dylib KK_DYLIB=build-emu/lib/libkkemu.dylib \
    pytest tests/test_dylib_screenshot.py tests/test_dylib_confirm_flow.py
  ================== 5 passed, 1 skipped in 0.15s ==================
Wires the ZIP-32 §6.1 seed fingerprint binding into the python-keepkey
client to mirror the firmware-side validation.

  device-protocol submodule
    - URL: keepkey/device-protocol -> BitHighlander/device-protocol
      (zcash work pins to fork master while seed_fingerprint sits in
      long-term review for upstream; revert when upstream merges.)
    - pin: d0b8d80 -> 4337c452 (BitHighlander/master with PR #27 merged).
    - messages_zcash_pb2.py regenerated via docker_build_pb.sh
      (kktech/firmware:v8 → libprotoc 3.5.1, the canonical toolchain).
      Selective regen — other pb2 files are intentionally NOT
      regenerated because they currently include content from
      BitHighlander/device-protocol open PRs (#18 SolanaTokenInfo,
      #19 TRON clear-signing, #20 TON clear-signing, #21
      EthereumTxMetadata). Until those merge, regenerating them
      against current master would back out work that the existing
      python-keepkey client relies on.

  keepkeylib/zcash.py (new)
    calculate_seed_fingerprint(seed) -> 32 bytes
      Pure-Python helper. BLAKE2b-256("Zcash_HD_Seed_FP",
      I2LEBSP_8(len) || seed). Matches the firmware C
      implementation byte-for-byte and the keystone3-firmware
      reference vector
        seed = 000102...1f
        fp   = deff604c246710f7176dead02aa746f2fd8d5389f7072556dcb555fdbe5e3ae3

  keepkeylib/client.py
    zcash_display_address  — add expected_seed_fingerprint kwarg
    zcash_sign_pczt         — add expected_seed_fingerprint kwarg
    Both pass through unchanged when the kwarg is None
    (backward compatible).

  tests/test_msg_zcash_seed_fingerprint.py (new)
    Pure-Python helper:
      - reference vector (Keystone3 cross-check)
      - rejects all-zero, all-0xFF, short, long
    Device-backed:
      - GetOrchardFVK returns non-empty seed_fingerprint
      - fingerprint stable across accounts (bound to seed, not account)
      - DisplayAddress: matching expected_seed_fingerprint succeeds,
        response carries seed_fingerprint
      - DisplayAddress: wrong expected_seed_fingerprint rejected
      - DisplayAddress: omitting expected_seed_fingerprint still works
      - SignPCZT: wrong expected_seed_fingerprint rejected before
        any signing crypto runs
Addresses review of PR #15.

Test structure
  Helper tests (no device) move to a dedicated module:
    tests/test_zcash_seed_fingerprint_helper.py
  This module deliberately does NOT import common, transport, or any
  protobuf bindings, so it runs on a stock dev box:
    pytest tests/test_zcash_seed_fingerprint_helper.py
  The previous file inherited common.KeepKeyTest, whose setUp wipes
  the device — pytest -k 'helper' was never actually offline.

Client wrapper coverage
  Device-backed tests now go through the public client helpers
  (self.client.zcash_display_address(... expected_seed_fingerprint=...)
  and self.client.zcash_sign_pczt(... expected_seed_fingerprint=...))
  rather than building raw protobuf messages with self.client.call().
  Confirms the kwarg pass-through end-to-end.

New test
  test_device_fingerprint_matches_python_helper: cross-checks the
  device-computed fingerprint against the python-keepkey helper for
  the same seed (all-allallall mnemonic, empty passphrase). Ties the
  firmware C, python-keepkey helper, and ZIP-32 §6.1 reference vector
  to the same byte-for-byte output.
feat(zcash): seed_fingerprint client + tests
… ≤ 7.14.0)

Pairs the device, signs a 1550-byte EIP-1559 transaction with the
all-all-all test mnemonic, and asserts that ECDSA recovery against the
canonical type-2 pre-image yields the device's own address.

Catches a firmware/ethereum.c ordering bug present in 7.x.0 .. 7.14.0
where the empty access-list byte (0xC0) — which closes the EIP-1559 RLP
body and must be the last byte fed to keccak before signing — was being
hashed inside ethereum_signing_init() right after the initial 1024-byte
data chunk, BEFORE the host had a chance to send the remaining
EthereumTxAck frames. For any tx whose data exceeded the single-chunk
threshold, the resulting pre-image was:

  keccak( ...header...
          || data_len_prefix
          || data[0..1024]
          || 0xC0           (bug: should be after ALL data)
          || data[1024..end] )

The signature was mathematically valid for that mangled hash so RPCs
accepted the broadcast, but the recovered signer was a wrong-but-
deterministic address. The mempool dropped the tx because the recovered
"from" had no balance / wrong nonce. Production symptom: every Uniswap
Universal Router swap, Permit2 batch, and large multicall hung at
"Confirm in wallet."

Single-chunk transactions (<= 1024 bytes) escaped the bug only by
accident — the misplaced 0xC0 happened to land at the end anyway.

Recovery-based assertion (eth-keys, eth-utils.keccak) — works on any
seed, no golden vectors to capture, the test asserts the actual
invariant: "signature recovers to the signer." Fails on broken
firmware, passes on 7.14.1+.

CI: eth-keys added to the existing pip install line; ships a pure-Python
keccak via eth-utils so no native deps are required.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
requires_message("EthereumTxAck") sends an empty EthereumTxAck as a
discovery probe. The firmware (correctly) rejects that with
Failure_UnexpectedMessage because we're not mid-sign, which skips the
test before the actual assertion runs.

requires_firmware("7.2.1") is sufficient — EthereumTxAck has been part
of the protocol since EIP-1559 support landed in 7.2.1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
eth-utils ships keccak via the eth-hash adapter, which auto-selects
between pycryptodome and pysha3 at import time. Without either backend
installed, importing keccak raises:

  ImportError: None of these hashing backends are installed:
  ['pycryptodome', 'pysha3'].

The new EIP-1559 chunked-data regression test imports keccak from
eth_utils to build the canonical type-2 pre-image, so it failed at
import rather than at the recovery assertion. Adding pycryptodome to
the existing pip-install line fixes it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
KeepKeyTest overrides unittest's assertEqual with a 2-arg version
(common.py:104) that doesn't accept the optional msg parameter — passing
one raises:

  TypeError: KeepKeyTest.assertEqual() takes 3 positional arguments
  but 4 were given

Print the regression diagnostic before asserting instead. Pytest captures
stdout on failure, so the divergence (expected vs recovered, canonical
hash, sig values) still surfaces in the failure report.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Upstreaming this test as a permanent regression guard rather than a
one-shot bug catcher. Bumping requires_firmware from 7.2.1 (the version
where EIP-1559 support originally landed) to 7.14.1 (the first version
where the access-list ordering bug is fixed) so CI on broken builds
skips this test instead of flagging a known-broken state as a new
regression.

The header comment already documents the affected range
(7.x.0 .. 7.14.0) and the fix landing in 7.14.1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gnition

- Bump device-protocol submodule to 8f80bcd (adds memo field to RippleSignTx)
- Update messages_ripple_pb2.py with memo field (field 7, optional string)
  compatible with protobuf==3.20.3 (old-format serialized_pb descriptor)
- Add test_sign_with_thorchain_memo in test_msg_ripple_sign_tx.py:
  verifies serialized XRPL tx ends with canonical Memos array binary
  (F9 EA 7D <len> <memo> E1 F1), requires firmware 7.14.2
- Add test_msg_ethereum_thorchain_deposit.py:
  covers legacy deposit() 0x1fece7b4 selector, new depositWithExpiry()
  0x44bc937b selector (requires 7.14.2, no AdvancedMode), and verifies
  non-THORChain addresses are still blocked without AdvancedMode
feat(7.14.2): XRP THORChain memo + EVM depositWithExpiry tests
* feat(hive): add Hive blockchain support

- messages_hive_pb2.py — generated from messages-hive.proto (IDs 1600-1603)
- hive.py — get_public_key / sign_tx client helpers
- mapping.py — register HiveGetPublicKey, HivePublicKey, HiveSignTx, HiveSignedTx wire IDs
- client.py — hive_get_public_key / hive_sign_tx methods on ProtocolMixin

* feat(hive): add HiveGetPublicKeys, HiveSignAccountCreate, HiveSignAccountUpdate

- messages_hive_pb2.py: regenerated from updated proto; now includes all 10
  message types (HiveGetPublicKey/Keys, HivePublicKey/Keys, HiveSignTx/ed,
  HiveSignAccountCreate/ed, HiveSignAccountUpdate/ed). Added role field to
  HiveGetPublicKey.

- mapping.py: register wire IDs 1604-1609 for the six new message types.

- hive.py: add get_public_keys(), sign_account_create(), sign_account_update()
  helpers. get_public_key() gains optional role parameter.

- client.py: add hive_get_public_keys(), hive_sign_account_create(),
  hive_sign_account_update() mixin methods with @expect decorators.
…format

Regenerated using protoc from kktech/firmware:v15 (protobuf 3.17.3).
The previous version used the builder API (protobuf 3.20+) which is
incompatible with the 3.20.3 Python runtime pinned in CI.
Test bugs fixed (mirrors BitHighlander/keepkey-firmware alpha CI fixes):
- ETH THORChain deposit: assertIn(sig_v, [27,28]) -> [37,38] (EIP-155 chain_id=1)
- XRP no-memo check: b'\xf9' -> b'\xf9\xea' (0xF9 appears in DER sigs naturally)
- Zcash FVK validation: skipTest until feature lands in firmware
Covers the full Hive message surface (all 5 firmware handlers) using the
standard 12-word seed (mnemonic12, "alcohol ... aisle"):
- HiveGetPublicKey   — active-role key format + 33-byte raw
- HiveGetPublicKeys  — 4 distinct STM role keys; single/bulk agreement
- HiveSignTx         — transfer (op 2), signature recovers to active key
- HiveSignAccountCreate — account_create (op 9), recovers to owner key + binds
  the 4 device keys and account name into the signed bytes
- HiveSignAccountUpdate — account_update (op 10), recovers to owner key

Account-op tests are self-validating: they recover the signer from the 65-byte
device signature over SHA256(chain_id || serialized_tx) and assert it equals
the device-derived key — exercising the device and validating the attestation
digest (keepkey-vault docs/HIVE-ATTESTATION-DIGEST-SPEC.md). No golden vector
required; recovery is an independent check. Hive was the one alpha-firmware
feature with full firmware+client support and zero test coverage.
Addresses review: substring-presence was too weak — a role swap (both keys
present), a creator rewrite, or an amount change could still pass.

Add a cursor-based Graphene reader matching the firmware append_* layout exactly
(incl. account_update's 0x01 optional-present flags, asset symbol padding, and
the no-wrapper memo_key) and rewrite all three signing tests to parse and assert
each field at its expected position + assert_end() for no trailing bytes:
- transfer: from / to / amount / precision / symbol / memo
- account_create: fee / creator / name / owner|active|posting authority slots / memo_key
- account_update: account / each replacement key in its slot / memo_key

Recovery assertions retained. Parser validated offline against hand-built
firmware-format bytes.
test(hive): vendored SLIP-0048 multi-key + account-op device tests
KeepKeyTest overrides assertEqual(self, lhs, rhs) with no msg parameter, so the
3-arg calls raised TypeError. Verified: all 5 tests pass against the feature/hive
emulator (build-emu/bin/kkemu, fw 7.15.0) — get_public_key(s), sign_tx,
sign_account_create, sign_account_update, with signature recovery + full
serialized_tx field-binding.
test(hive): fix assertEqual signature — all 5 hive tests green on emulator
…ests

Integration-test layer for the firmware Insight clear-signing feature
(keepkey-firmware feat/evm-clear-signing-alpha, PR #257).

signed_metadata.py:
- Fix the key_id/slot footgun: serialize_metadata defaults key_id=3, the
  DEBUG_LINK CI slot whose pubkey == firmware METADATA_PUBKEYS[3] (the test
  signer derives to slot 3, NOT slot 0). Production/Pioneer callers must pass
  key_id=0 explicitly. assert_test_key_matches_slot3() pins this invariant.
- sign_metadata fails loud if `ecdsa` is missing (was a silent zero-signature
  that firmware would reject as MALFORMED, disguising the real cause). Signs the
  identical byte range firmware hashes (version..key_id, excl. sig+recovery).
- Add pure-python keccak256 + EIP-155/EIP-1559 RLP sighash helpers so a metadata
  blob's tx_hash binds the REAL signing digest. Cross-checked against the device:
  recovering an existing erc20-approve signature over eth_sighash_legacy yields
  the test mnemonic's m/44'/60'/0'/0/0 address.

test_msg_ethereum_clear_signing.py:
- All vectors use key_id=3.
- New offline (verified green here, 12/12): slot-3 pubkey assertion, key_id=3
  default, keccak256 known vectors.
- New device-class cases (run on the kkemu/DEBUG_LINK emulator): tx_hash binding
  happy path (signs + recovers correct signer), replay reject (metadata bound to
  tx A, sign tx B → "Metadata does not match signed transaction", no signature),
  AdvancedMode gate (OFF+unknown→reject, ON→sign, native ERC-20 unaffected), and
  cancel-clears-metadata (stale blob not reused).

Offline portion verified with PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python.
Device-class cases require the firmware emulator (libkkemu) + DEBUG_LINK.
… SECTIONS

The feature ships in the 7.15.0 firmware tree, so gate the device tests at
7.15.0 (was 7.15.1, which left them dormant on the current build).

- test setUp: requires_firmware 7.15.1 -> 7.15.0.
- generate-test-report.py SECTIONS 'V' (EVM Clear-Signing) min_firmware
  7.15.1 -> 7.15.0; add V9-V12 mapping the new device-class tests
  (full tx-hash binding happy path, replay reject, AdvancedMode gate,
  cancel-clears-metadata) with OLED screenshot expectations so the
  report-driven Phase-1 capture includes them.

Verified on the containerized kkemu emulator (docker compose, CI-faithful):
all 28 clear-signing tests pass; OLED screenshots captured for the verified
flow (INSIGHT VERIFIED icon + decoded method/contract/args), the replay
reject, and the AdvancedMode gate.
test(insight): EVM clear-signing integration tests + metadata signer
…tract gate)

Covers the firmware Ethereum signing pre-image / clear-sign correctness guards
(firmware PR BitHighlander/keepkey-firmware#255, merged to alpha):
- type=2 without chain_id is rejected (chain_id over-declared the RLP header)
- type=2 with max_fee but no max_priority_fee still signs (priority is a
  mandatory 0x80-encoded field; Stage 1 and Stage 2 must agree)
- type=2 carrying only gas_price, and legacy carrying max_fee_per_gas, rejected
- a contract clear-sign handler selector with calldata streamed beyond the
  initial chunk signs the full data via the generic path instead of confirming
  a prefix (screen-level assertion verified on-device/emulator)
BitHighlander and others added 30 commits August 2, 2026 14:49
Drives the emulator through the full GetAddress path for SPENDTAPROOT --
fsm_msgGetAddress, path_mismatched's m/86' branch, compute_address, the
BIP-86 tweak and bech32m encoding -- none of which the firmware's C unit
tests reach.

Expected values are the three official BIP-86 vectors.  BIP-86 publishes
them against the "abandon abandon ... about" mnemonic, which is exactly
what setup_mnemonic_abandon loads, so these are the spec's constants and
not values our implementation produced.

Verified against a locally built emulator: 1 passed.  Also mutation
checked -- flipping one character of the first expected address makes it
fail, so the assertions are not vacuous.

NOTE: gated at 7.16.0 via TAPROOT_FIRMWARE_VERSION.  develop is currently
7.15.0, so this SKIPS until the project version bumps.  A gate that is
never reached is a test that is silently green forever -- keep the
constant in step with CMakeLists.txt.
Spends a P2TR input on the emulator and compares the 64-byte witness byte
for byte against a signature computed outside the firmware.

The expected value is not a round trip through our own verifier -- that
would pass even if the device committed to the wrong transaction.  It comes
from a standalone Python implementation of BIP-340/341 written from the
specs, keyed from BIP-86's own published xprv for m/86'/0'/0'/0/0, and
self-checked against BIP-86's published internal and output keys before
being used.  BIP-340 signing is deterministic given aux_rand, so equality is
meaningful.

This caught a real bug.  BIP-143 hashes prevouts/sequences/outputs with
DOUBLE sha256 (hasher_sign is HASHER_SHA2D for Bitcoin) while BIP-341
specifies SINGLE sha256, so reusing the BIP-143 accumulators produced a
cryptographically valid signature over the wrong commitment -- exactly the
failure a self-consistent test cannot see.

Includes a synthetic prev-tx fixture in txcache so the test runs offline.
Both taproot tests gated on requires_firmware("7.16.0") while CMakeLists
said 7.15.0, so they skipped -- and which release taproot ships in is still
undecided.  A gate that is never reached is a test that is silently green
forever, which is the failure mode that looks exactly like passing.

Replaces it with requires_taproot(), which asks the device.  The tests now
run whenever the firmware reports the capability, whichever release that
turns out to be, and retargeting the release no longer touches them.

  - common.py: requires_taproot() helper, alongside requires_firmware and
    requires_message
  - regenerated messages_pb2.py for Features.supports_taproot (field 27)
  - device-protocol bumped to the commit adding it

Adds test_taproot_screens.py for Gate-3 OLED capture: the two places a
62-character bech32m address actually reaches the display -- verifying a
receive address, and the p2wsh multisig case that shares the length.  That
capture is what found the address truncation fixed in the firmware repo.
…safety

fix(ci): request reviews safely for fork PRs
test(bitcoin): prove Taproot production paths in release PDF
test(taproot): prove tampered prevout rejection
test(rng): prove the RC23 entropy audit budget
…equence

test(taproot): require physical signing confirmations
Every signing test read the `signature` protobuf field and either
discarded `serialized_tx` or checked a substring of it. `signature` and
`serialized_tx` are separate nanopb fields with independent presence
flags, so a firmware path that populated one and not the other passed
the whole suite -- which is exactly what shipped: the taproot branch
omitted has_serialized_tx and the host silently lost the 66-byte witness
and the 4-byte locktime footer.

The substring check in test_send_p2tr_with_change could not have caught
it either: the change scriptPubKey it looked for is serialized in phase
1, well before any witness, so it survives a truncated suffix.

assertCompleteSegwitTx() parses the transaction per BIP-144 and requires
it to consume exactly len(raw): a segwit marker promises witness data,
so a dropped witness now runs the stream off the end instead of passing
unnoticed. It returns the per-input witness stacks, letting the tests
assert that a key-path spend carries exactly one 64-byte element and
that a legacy input still serializes its empty 0x00 witness.

Each test now also pins the full serialization. Those goldens were
captured from a fixed-firmware emulator run and independently rederived
from the inputs and the existing EXPECTED_* witnesses; both agree.
validate_junit() accepted 'skip' as a waiver. That is right for
build-flag-gated features (bitcoin-only, zcash-privacy), where a skip
genuinely means "not in this build". It is wrong for a capability the
build claims to have: every taproot test opens with requires_taproot(),
so if that capability regressed, all six would skip and the report would
still certify a green run -- coverage it never actually obtained.

MUST_RUN_MODULES lists the modules that must really execute; a skip
there is now a 'skipped-but-required' failure. Verified both ways
against the catalogue: taproot passing validates clean, taproot skipping
produces six failures (B21-B26) where it previously reported success.

B21/B22/B23 prose now states what the tests prove after the serialized-tx
coverage change -- that the full transaction is parsed as BIP-144 and
must consume every byte, so the witness and locktime footer are known to
have reached the host, not just the signature field.
Regenerated bindings for device-protocol feat/dice-entropy
(ResetDevice.dice_entropy, DebugLinkDecision.input,
DebugLinkState.dice_digest, ButtonRequest_DiceRoll). debuglink gains
press_input() (chunked synthetic roll injection; each chunk must fit
the firmware's 40-char max_size) and read_dice_digest().

test_reset_device_dice runs the full flow against the emulator: the
DiceRoll ButtonRequest announcement, injection in 40-char chunks with
undo churn, a host-side simulation of the same append/undo rules, the
device digest matching sha256 of exactly the expected 99-roll string,
and the post-mix internal entropy still producing the documented
sha256(internal || external) mnemonic. Version-gated to 7.15.0.
Regression cover for a host-controllable seed: reset_init aborts left
awaiting_entropy armed from an earlier ResetDevice while zeroing
int_entropy, so a following EntropyAck derived the seed from
sha256(0*32 || host_bytes). The test arms a reset, re-enters with
dice_entropy, cancels, and requires the EntropyAck to fail with 'Not
in Reset mode' with the device still uninitialized.
feat(reset): drive and verify on-device dice-entropy collection
Firmware no longer renders the Internal Entropy screen -- internal entropy
is seed pre-image material, and a host that supplies ext_entropy and reads
that screen once can compute SHA256(shown || ext) and derive the seed.

test_reset_device_pin and test_failed_pin asserted the ButtonRequest for
that screen, so they failed against the new firmware. Rather than dropping
display_random from the request, they keep sending it =True and now assert
the NEXT message is PinMatrixRequest -- which is a direct test of the
compatibility claim: the field stays decodable on the wire and changes
nothing.

Verified 6/6 against an emulator built from the paired firmware branch.
…t's scope

The PDF is the artifact a release review actually reads, and it was quietly
claiming more than it knew. Two defects, one visible consequence.

parse_junit only emitted a 'mod::meth' key when the JUnit classname contained
a dotted test_msg_*/test_sign_*/test_verify_* module. Native gtest suites carry
a bare classname ("Dice", "Storage"), so they produced no such key, and _lookup
has no bare-method fallback by design. CI merged the firmware-unit XMLs into the
report input and every one of the 432 native tests was then structurally
impossible to reference from SECTIONS. Bare classnames are now keyed as
'Suite::Test'.

The header reported "N/N PASSED" against the catalog with nothing saying the
catalog is a subset. A 7.15 RC audit grepped this PDF for feature keywords,
found no hits for dice and PIN KDF, and reported both as having zero coverage.
Both had in fact run green in the same CI run: test_reset_device_dice passed,
and so did all five Dice unit tests and the PIN KDF rewrap tests. The header now
states catalogued-vs-executed and says outright that absence here is not
evidence of absence.

New section K catalogues what that audit went looking for: the dice flow
end-to-end (digest equals SHA256 of exactly the injected rolls, then the
mnemonic is derived from post-mix internal entropy, which is what proves the
rolls reached the seed), the aborted-reset EntropyAck disarm regression, the
five Dice known-answer and independence vectors, and the v16->v19 PIN KDF
rewrap plus storage migration.

Verified against the 7.15.0 RC artifacts from run 31284108490: dice went from 0
to 13 occurrences in the rendered PDF, section K renders 11/11 passed, and
poisoning Dice::MixDependsOnRolls in the merged JUnit turns the header red and
fails --validate-junit, so the entries are wired to real results.

Needs the companion firmware change: the CI trigger validated against the
Python JUnit alone, where every native entry resolves to "missing".
Chasing a rendering defect on the per-output shielded confirm turned up
something worse than a missing screenshot: ZcashSignPCZT is never sent to a
device anywhere in this suite. Every test in test_msg_zcash_sign_pczt drives
a ScriptedTransport with canned responses -- they are offline contract tests
that prove the client builds and orders its messages correctly, and prove
nothing whatsoever about firmware behaviour.

The device-driven Zcash tests cover transparent signing, display-address,
FVK derivation and the seed fingerprint. None of them signs a shielded
output. So the on-device shielded path -- including the confirm screen that
fsm_msg_zcash.h designates as the verification gate for Orchard output
values, since total_amount is "a summary prompt" taken from the host -- has
no automated coverage at all.

The catalog gave no hint of this. The section Z entries read exactly like
the device tests around them, and that is how a screen nobody has ever
rendered sat behind seven green checks.

Say it in the entry instead. No screenshot hint: requesting frames from a
test that cannot reach a device would produce silently zero of them, which
is the same class of empty-but-green evidence this whole pass exists to
remove.

Verified: --screenshot-filter does not select it, and the report still
renders 325 tests and passes --validate-junit against the RC artifacts.
ZcashSignPCZT had never been sent to a device by anything in this suite.
Every test in test_msg_zcash_sign_pczt drives a ScriptedTransport with canned
responses; the device-driven Zcash tests cover transparent signing,
display-address, FVK derivation and the seed fingerprint. So the on-device
shielded path had no automated coverage at all, and seven green checks in
section Z read exactly like device coverage while proving only that the
client serialises its messages in the right order.

That is how a confirm screen which cannot physically fit its amount line
shipped unnoticed. The per-output shielded confirm is the verification gate
for Orchard output values -- total_amount on the summary prompt is taken
straight from the host message -- and a unified address is 106 characters,
three full body rows, against a three-row body. The amount never rendered.

The fixtures are the firmware's own known-answer vectors from
unittests/firmware/zcash.cpp, so the device's cmx recomputation accepts them
without needing a Pallas implementation in Python. The same note under both
pools commits to a different value, which is what makes the pool tests
possible at all.

Four tests:
  - the output review is two screens, and they render differently and
    non-blank (read_layout returns a framebuffer, not text, so the assertions
    are structural rather than OCR)
  - a one-bit change to the recipient breaks the commitment
  - the Orchard commitment is refused when Ironwood is declared
  - the Ironwood commitment for that same note is accepted

Two gates the offline fixtures do not satisfy had to be met for real
firmware: the header digest is recomputed and compared, and for a
shielded-only transaction the verified fee reduces to orchard_value_balance
and must equal the declared fee. Both are computed here rather than canned.

VERIFIED AS A REGRESSION TEST, not just written: run against the shipped
7.15.0 RC emulator (docker image from run 31284108490, the 27970b0c6 build)
it fails with "expected 2 ConfirmOutput screens, got 1", while the
commitment-binding and pool-selection tests pass. It reproduces the defect on
the firmware that has it.

Catalogued as Z22-Z25 with screenshot hints, so the shielded confirm screens
finally appear in the report -- the RC run captured 1037 OLED frames and not
one came from a shielded flow.
The firmware test was renamed PinKdfV16RewrapsToV19AfterCorrectPin ->
PinKdfRewrapsToActiveVersionAfterCorrectPin when the STORAGE_PIN_KDF_V19 gate
went to 0, because it is no longer v19-specific: it now asserts BOTH sides of
the gate, which is what makes it meaningful in the shipping build where v19 is
off. Catalog validation failed against the old name.

Adds K8b for PinUnlocksAfterRebootUnderV17, the end-to-end create/set-PIN/
serialize/reload/unlock/decrypt regression. It belongs in a curated catalog
precisely because every other storage test stays in RAM, and the wallet lockout
it guards lived on the serialize/reboot boundary.
Firmware fix ed6db167 shipped without a test at a chain id that reproduces it.
Every EIP-1559 case in this file uses chain_id 1 or 3 — both single-byte — so
the bug had zero coverage in the file that tests the feature.

hash_rlp_field((uint8_t*)&chain_id, 1) fed only the least-significant byte into
keccak on little-endian ARM. Base (8453 = 0x2105) hashed 0x05 and the signature
recovered to an unrelated address. RLP length was correct; the legacy EIP-155
path was correct; only the EIP-1559 hash was wrong.

A golden r/s needs a device run, so this is a differential: sign one identical
tx under 8453 (0x2105) and 4357 (0x1105). Same low byte AND same RLP length
header, so broken firmware hashes an identical pre-image and — signing being
deterministic — returns the same signature twice, failing the assertion. Fixed
firmware hashes 0x21 0x05 vs 0x11 0x05. No golden value, no new deps (the repo
has ecdsa but no keccak, so recovering the signer address was not available).

Version-gated to 7.15.0 so it SKIPs rather than fails on older firmware.
CI red on #212: test_reset_device_pin and test_failed_pin both failed with
'AssertionError: code: ButtonRequest_ResetDevice'.

Not a bug in b44f1b3 — that commit is correct. Firmware 7.15.0 removed the
Internal Entropy screen (fw 320f0eb5, 'auditable entropy source, on-device
dice, and no entropy display'), because internal entropy is seed pre-image
material: a host that sets display_random and reads the screen once can compute
SHA256(shown || ext) and derive the seed. Confirmed by inspection — reset.c on
the rc29 line contains no display_random references at all, while the pinned
older submodule still has 'if (display_random) {'.

So the assertion 'next message is PinMatrixRequest' is right for 7.15.0+ and
wrong for anything earlier, where that ButtonRequest legitimately still exists.
The tests carried NO version gate, so on CI's older emulator they failed
instead of skipping.

This is the same root cause as test_ethereum_eip_1559_multibyte_chain_id
skipping in the same run: CI's emulator predates 7.15.0. That test was gated
and skipped cleanly; these two were not and went red. Per the house rule that
feature tests must version-gate to SKIP, gate both at 7.15.0.

Worth noting the failure was only visible because these commits finally entered
a PR — they were reachable solely from a fork branch that rc29 pinned directly,
so the breakage rode along unreviewed.
[1] Version gate removed real reset coverage. My requires_firmware("7.15.0")
skipped the WHOLE of test_reset_device_pin / test_failed_pin on the 7.10
emulator — PIN entry, EntropyRequest/Ack, and mnemonic derivation checked
against a locally computed expected value — to silence one assertion. Branch on
the version instead: pre-7.15 acks the Internal Entropy ButtonRequest that
legitimately still exists there, then both paths converge on
PinMatrixRequest. Everything downstream keeps running on every firmware.

[2] Z17 claimed 'ZcashSignPCZT is not sent to a device anywhere in this suite
... no automated coverage at all' while 3bbf996 in the same branch added
exactly that device test. Scoped the claim to THIS module and pointed at Z22.
Notable because the comment above the scope block explains that saying so is
'the difference between evidence and a misleading completeness claim'.

[3] The report said CI 'executed' JUNIT_CENSUS['ran'], which increments for
every collected testcase including skips. The run behind this PR was 613
collected / 252 skipped — the report would have overstated execution by 41%,
and a version-gated test that SKIPs on an old emulator is not evidence the
feature works. Track skipped separately and say 'collected', with the skip
count and why skips happen stated inline.

[4] _capture_button_screens read the framebuffer BEFORE delegating to the
original callback, which is where the render-settle delay lives — so it could
capture a partially drawn or previous screen. Reading after would be worse
(the original presses the button and advances). Settle inside the wrapper
before reading. Unconditional, unlike client.callback_ButtonRequest's
SCREENSHOT-only sleep, because these are structural assertions rather than
screenshot evidence and need a settled layout on every run.

[5] handled in the PR description.
test(7.15): reconcile the branch with what rc29 actually pins, plus the chain_id regression
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