Release 7.15.0 — Zcash Orchard, native macOS emulator, 7.14 carryover - #226
Closed
BitHighlander wants to merge 129 commits into
Closed
Release 7.15.0 — Zcash Orchard, native macOS emulator, 7.14 carryover#226BitHighlander wants to merge 129 commits into
BitHighlander wants to merge 129 commits into
Conversation
Remove QTUM, BNB, ZIL, GTO, IOST, CMT, MCO, and ODEM from tokens.def. These tokens were listed as ERC-20 entries in the legacy coin table but their contract addresses no longer exist in the generated uniswap_tokens table (they migrated to their own L1 chains or were delisted). This causes Coins.TableSanity unit test to fail because tokenByTicker() finds them in coins[] but tokenByChainAddress() cannot locate them in tokens[], returning UnknownToken. The native chain entries for QTUM, BNB, etc. in coins.def are unaffected (they have has_contract_address=false). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4-stage fail-fast pipeline: gate (lint, secrets, submodules) → build (emulator + ARM cross-compile) → test (unit + Python integration) → publish (manual DockerHub push). Downloadable firmware artifacts with version-tagged naming. CircleCI config removed.
- Fix gitleaks: use free CLI instead of paid gitleaks-action v2 - Add release.yml: tag-triggered build + draft GitHub Release - Add release/* and hotfix/* branch patterns to CI triggers - Add GIT-FLOW.md documenting the branching model
5 benign hits in full-repo scan: U2F dev key, curve25519 README examples, bootloader public signing keys, and 2014 test wallet. All reviewed and confirmed non-sensitive.
Adds cppcheck as a Stage 1 GATE job alongside lint-format and secret-scan. Scans lib/, include/keepkey/, and tools/ with project-aware include paths and defines. Outputs inline PR annotations and a severity summary table. Warn-only until existing findings are triaged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Match CircleCI approach: build emulator from source via docker-compose instead of pre-built artifact. Restore tokens.def to match upstream develop after revert.
docker compose up --build firmware-unit leaves the kkemu container running with dirty emulator state. The subsequent python-keepkey tests connect to this stale emulator, causing test_sign_uni_approve_liquidity_ETH to fail with "Signing cancelled by user". Adding docker compose down between the two runs ensures python tests get a fresh emulator instance.
actions/checkout defaults to the merge ref for pull_request events, which fails for fork PRs when the merge commit isn't ready. Use the PR head SHA directly to avoid auth failures.
CircleCI runs firmware-unit then python-keepkey without tearing down in between. The docker compose down was destroying the kkemu emulator and firmware-unit container, making test report extraction fail.
python-keepkey has a nested submodule (ethereum-lists) that contains the token database used to generate ethereum_tokens.def at build time. Without --recursive, the token table is empty/partial, causing Coins.TableSanity and ethereum signing tests to fail. CircleCI uses --init --recursive globally; match that behavior.
docker-compose v1 (CircleCI) returns all containers with ps -q. docker compose v2 (GH Actions) only returns running containers. Need -a flag to include exited containers for docker cp.
Third-party action fails with 401 on fork PRs. Use built-in actions/upload-artifact instead — test XML is still downloadable.
feat(ci): migrate from CircleCI to GitHub Actions
- Point device-protocol and python-keepkey submodules to upstream master (includes BIP-85, Solana, Tron, TON wire IDs and proto definitions) - Add nanopb .options files for Solana, Tron, TON (field size constraints) - Add Bip85Mnemonic.mnemonic max_size:241 to messages.options - Update lib/transport/CMakeLists.txt with new proto sources, options, headers, and protoc compilation commands - Fix CI: use pre-installed clang-format instead of apt-get install (eliminates 3-minute timeout on GitHub runners) - Update Zcash transparent branch ID from Sapling to NU6
chore: update submodules + nanopb options for 7.14.0 proto chains
- actions/checkout v4 → v6 - actions/upload-artifact v4 → v7 - actions/download-artifact v4 → v8 - actions/cache v4 → v5 - docker/setup-buildx-action v3 → v4 - docker/login-action v3 → v4 Node.js 20 actions forced to Node.js 24 starting June 2, 2026 and removed September 16, 2026.
…tions chore: upgrade GitHub Actions to latest versions
152 files reformatted with `clang-format --style=file` to eliminate all lint-format CI warnings.
fix: resolve all clang-format CI lint warnings
--output-file writes cppcheck's default format while --template controls stdout — the two don't combine. When all findings are suppressed the output file is never created, failing artifact upload. Drop --output-file and pipe stdout through tee to cppcheck_report.txt so the file always exists (even if empty) and contains the same templated annotations used for GitHub PR warnings.
- Remove invalid wildcard suppressions (*:deps/*, *:*.pb.c) that caused "Failed to add suppression. No id." — cppcheck never ran - Simplify unknownMacro suppression (file glob not needed) - Fix grep patterns to match only ::warning annotation lines, not stray cppcheck stderr that polluted the report file - Use parameter defaults to prevent arithmetic errors on empty grep
cppcheck treats blank lines as suppressions with an empty ID, causing "Failed to add suppression. No id." — stripped to bare IDs only.
- models.def syntaxError: X-macro pattern cppcheck can't parse - pb_decode.c memleakOnRealloc: third-party nanopb code The original wildcard suppressions (*:deps/*, *:*.pb.c) never worked — cppcheck doesn't accept * as a suppression ID. It errored out on every CI run before scanning any code. These file-specific suppressions use valid cppcheck syntax.
- Remove continue-on-error: errors now fail the job - Suppress known false-positive errors (uninitvar, bufferAccessOutOfBounds, subtractPointers, ctunullpointer) with file-specific entries for triage - Warnings/style/performance findings produce annotations but don't block - Verified locally: 0 errors, 0 suppression failures
- Add suppressions for errors seen on ubuntu CI cppcheck version: syntaxError (global — X-macros in variant.h and models.def), preprocessorErrorDirective (pb.h), returnDanglingLifetime (tiny-json.c), comparePointers (blupdater — different ID than local subtractPointers) - Print filtered findings in CI logs for easier debugging - Verified locally: 0 errors, suppressions clean
fix(ci): ensure cppcheck_report.txt artifact is always created
- Multi-phase CI pipeline with OLED screenshot capture and PDF report generation - DebugLink canvas→layout bit-packing for 256x64 screenshots (fsm_msg_debug.h, DEBUG_LINK only) - nanopb DebugLinkState.layout max_size 1024→2048 (full display = 2048 bytes at 1bpp) - Docker emulator healthcheck (/health endpoint + service_healthy condition) - Fail-fast gates: CI errors on zero screenshots, conftest hook enforces capture - Zoo screenshot tooling (capture, report, smoke test, emulator wait) - Version bump 7.10.0→7.14.0 (release target metadata) - Pin python-keepkey to screenshot/report-enabled version with requires_message() gating 19 files changed, zero production firmware code modified.
The seed_fingerprint binding (ZIP-32 §6.1) is post-7.14.0 functionality.
Tests in python-keepkey gate themselves with:
self.requires_firmware("7.15.0")
so they correctly skipped on the previous CI run because CMakeLists
still reported 7.14.0. Bumping the project VERSION to 7.15.0 unblocks
end-to-end emulator validation of the binding flow:
- GetOrchardFVK returns seed_fingerprint
- DisplayAddress accepts/rejects expected_seed_fingerprint
- SignPCZT rejects on mismatch before any signing crypto runs
- Cross-check: device-derived fp == python-helper-derived fp
This bumps only the project VERSION reported by the firmware itself
and used in version-gated tests. It does not tag a release, change
build artifacts, or affect any other consumer.
Documents what's actually required to make the strong attestation claim
("this UA is spendable by the device") that the current ZcashDisplayAddress
flow does NOT make.
The current flow verifies the host-supplied (ak, nk, rivk) matches the
device-derived FVK and then displays the host-supplied address string
unchanged. A malicious host that knows the FVK can substitute any UA. The
device's OLED carries no cryptographic binding to "spendable."
This document specifies:
- Threat model and what we can / cannot promise
- The full cryptographic recipe for on-device UA derivation:
ivk via Sinsemilla, d_j via FF1-AES256, g_d via Pallas SWU + GroupHash,
pk_d via existing scalar mult, F4Jumble + bech32m UA encoding
- New proto messages: ZcashGetUnifiedAddress / ZcashUnifiedAddress
- Five implementation phases with effort estimates totalling 14-21 dev-days
(call it 4-5 weeks calendar with review + hardware verification)
- Cross-language test strategy using orchard Rust crate as the oracle
- Open design questions on diversifier-index UX, multi-receiver UAs, dk
exposure on the FVK response, and bech32m vs raw-receiver display
- Migration path that keeps the existing FVK-match flow alive but downgrades
its UX copy to match what it actually proves
Implementation is not started. This is the blueprint.
feat: derive and display Orchard unified addresses
…emulator-runtime # Conflicts: # deps/python-keepkey
…-runtime feat(emulator): libkkemu shared library + native macOS build
The device now derives the Orchard-only Unified Address itself (Sinsemilla + SWU hash-to-curve, default diversifier index 0). Asking the host for a UA + FVK and verifying via FVK match is strictly weaker than device-derived display — it leaves non-Orchard receivers in a UA unverified — and was dropped. - fsm_msg_zcash.h: remove host_supplied_address branch (-101 / +13). ZcashDisplayAddress always derives + displays the device's own UA; optional expected_seed_fingerprint check is preserved. - include/.../messages-zcash.options: drop sizing for removed fields. - deps/device-protocol → 870b8ea: reserve fields 3-6 in ZcashDisplayAddress (address, ak, nk, rivk). - deps/python-keepkey → 954aed2: drop FVK kwargs from helper + display-address tests; the "wrong FVK rejected" case is gone since there is no host-supplied FVK to mismatch.
The full-address confirm step was missing the notification icon, so the user saw no click cue and the screen looked stuck. The QR step also showed a 160px-truncated copy of the address that was unreadable and redundant. - layout_zcash_address_text_notification: now first. Title at the top via desc, full UA in 3 lines (TRANSACTION_WIDTH - 25 leaves room for the right-column icons), confirm icons restored. - layout_zcash_address_notification: now second. Title + QR + icons, truncated address text dropped. - confirm_zcash_address: text layout first, QR layout second; both receive the same desc so the title stays visible.
…path Two security fixes from the PR review: 1. Zcash signing state was not torn down by the common abort paths. fsm_msgZcashSignPCZT sets zcash_signing.active = true and stores derived Orchard keys; fsm_msgZcashPCZTAction only gates on that flag. Initialize, Cancel, and ClearSession aborted other signers but left Zcash state intact, so a host could resume streaming PCZTAction or TransparentInput messages against an already-approved session after a "cancel". Add zcash_signing_abort() (memzero of the whole static struct), wire it into fsm_msgInitialize, fsm_msgCancel, fsm_msgClearSession, and replace 21 ad-hoc `active=false [+ memzero keys]` cleanup sites with the same call so every error path leaves the session in the same fully-zeroed state. 2. ZcashSignPCZT and ZcashGetOrchardFVK accepted any address_n_count >= 3, stripped address_n[2] for the account, and silently fell back to account 0 if neither account nor a valid path was supplied. A malformed host path could therefore sign or export an FVK for an unintended account. Mirror the strict m/32'/133'/account' check already present in ZcashDisplayAddress: require exactly 3 hardened components or an explicit account field, otherwise reject with Failure_SyntaxError.
Picks up the transparent-shielding flow fix in zcash_sign_pczt (n_transparent_inputs propagated, TransparentInput phase driven first, 0xFF completion sentinel handled) and the tightened test_transparent_shielding_* assertions (no more try/except masking).
feat(zcash): full Orchard support
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cuts firmware 7.15.0 for review. 129 commits accumulated on
developsince the lastmastermerge (3d8dec49).Headline features
Zcash Orchard — full on-device support (PR feat(zcash): full Orchard support #220 / commit
3bc5bda3)ZcashGetOrchardFVKreturns Full Viewing Key + ZIP-32 §6.1 seed_fingerprintZcashSignPCZT/ZcashPCZTAction— streamed Orchard shielded signing (RedPallas)ZcashTransparentInput/ZcashTransparentSig— hybrid transparent→Orchard shieldingZcashDisplayAddress— derives Orchard-only Unified Address on-device (Sinsemilla + SWU hash-to-curve), shows full UA + QR with two-step physical confirm. Host-supplied UA / FVK-match attestation flow was dropped.deps/device-protocol→BitHighlander/device-protocol@2352346(now also hasfeat/zcash-display-derive-onlyon top with reserved fields)deps/python-keepkey→BitHighlander/python-keepkey@1ed0894deps/crypto/trezor-firmware→feature/zcash-crypto(Pallas + RedPallas + Sinsemilla primitives)Native macOS ARM64 emulator (PR feat(emulator): libkkemu shared library + native macOS build #217) —
libkkemu.dylibfor host-side test harness; ~30s build vs. minutes-long Docker.7.14.0 carryover (already on develop, lands here in this release)
BIP-85, EVM clear-signing prep, Solana (keepkey#420), TRON+TON (keepkey#414), Lynx coin (keepkey#411), THORChain
depositWithExpiryselector (keepkey#410), nanopb oneof memleak (keepkey#412), 7.14.0 transport prep (keepkey#383). Many CI hardening / cppcheck / lint / upgrade-actions PRs.Security review (PR #220 follow-up — already merged)
Three findings from the post-merge review were addressed before this release:
Initialize/Cancel/ClearSession. Addedzcash_signing_abort()(memzero of the static state); wired into all three handlers; replaced 21 ad-hocactive=false [+ memzero keys]cleanup sites infsm_msg_zcash.hwith the same call.python-keepkeyzcash_sign_pcztnow propagatesn_transparent_inputs, drives Phase 3 (transparent) before Phase 2 (Orchard), handles the0xFFcompletion sentinel; tests assert real signatures instead of swallowing exceptions.ZcashSignPCZTandZcashGetOrchardFVKaccepted anyaddress_n_count >= 3, silently defaulting to account 0 on malformed paths. Now enforce strictm/32'/133'/account'(3 hardened components) matchingZcashDisplayAddress.Test plan
makebuilds device firmware (build-arm-firmwarealready green in feat(zcash): full Orchard support #220 /release/7.15.0)makebuilds emulator (build-emulatoralready green)python-integration-testsgreen against the emulator (already green; transparent shielding tests now actually exercise the flow)python-dylib-testsgreenZcashGetOrchardFVKfor accounts 0, 1, 2 — match python-keepkey helper outputZcashDisplayAddressshows full UA + title on screen 1, QR on screen 2; both confirm steps gate on physical buttonZcashSignPCZTshielded-only against a Zashi-built PCZT — broadcast + confirm receiptZcashPCZTActionis rejected with "Not in Zcash signing mode"Outstanding for follow-ups (NOT blocking this release)
docs/coin-integration/zcash-on-device-ua.md— still describes the now-removed FVK-match flow as the shipped state.@bithighlander/device-protocolnpm package afterfeat/zcash-display-derive-onlymerges to fork master, so downstream wallets (hdwallet, vault) get the trimmedZcashDisplayAddressproto.