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

.circleci/config.yml

@@ -47,18 +47,32 @@ jobs:
           command: |
             pushd ./scripts/emulator
             set +e                                   # don’t exit on first failure
-            docker-compose up --build firmware-unit
             docker-compose up --build python-keepkey
             set -e
 
             # Collect JUnit / pytest XML results
             mkdir -p ../../test-reports
-            docker cp "$(docker-compose ps -q firmware-unit)":/kkemu/test-reports/. ../../test-reports/
             docker cp "$(docker-compose ps -q python-keepkey)":/kkemu/test-reports/. ../../test-reports/
             popd
 
-            # Fail job if either container reported non-zero status
-            [ "$(cat test-reports/python-keepkey/status)$(cat test-reports/firmware-unit/status)" = "00" ] || exit 1
+            # Fail the job on this repo's OWN result.
+            #
+            # The firmware's C++ firmware-unit suite used to run here and gated
+            # this job. No change in THIS repository can affect firmware C++, and
+            # the firmware repo already runs that suite in its own CI, so all it
+            # did was fail python-keepkey for reasons no python change caused: a
+            # token-table change cannot go green here until the matching firmware
+            # change reaches the branch this clones, which is a release away.
+            #
+            # Read the status file defensively -- it is written by the container,
+            # and a crash before it exists must FAIL rather than silently pass an
+            # empty-string comparison.
+            STATUS_FILE=test-reports/python-keepkey/status
+            if [ ! -f "$STATUS_FILE" ]; then
+              echo "no status file at $STATUS_FILE -- the suite did not finish"
+              exit 1
+            fi
+            [ "$(cat "$STATUS_FILE")" = "0" ] || exit 1
 
       - store_test_results:
           path: test-reports

.github/workflows/ci.yml

@@ -4,7 +4,7 @@
 # and runs the full python integration test suite against it.
 #
 # Stage 1: GATE (seconds)
-#   └─ lint        basic Python syntax check
+#   └─ lint        Python syntax + deterministic protocol contract tests
 #
 # Stage 2: TEST (gated by Stage 1)
 #   └─ integration full pytest suite against emulator
@@ -13,9 +13,15 @@ name: CI
 
 on:
   push:
-    branches: [master, develop, 'feature/**', 'fix/**', 'hotfix/**']
+    branches: [master, develop, reconcile/upstream-sync, 'feature/**', 'fix/**', 'hotfix/**']
   pull_request:
-    branches: [master, develop]
+    branches: [master, develop, reconcile/upstream-sync]
+
+# One run per ref: a new push supersedes the old instead of both burning a
+# runner to completion.
+concurrency:
+  group: ci-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
 
 jobs:
   # ═══════════════════════════════════════════════════════════
@@ -34,13 +40,26 @@ jobs:
       - name: Syntax check
         run: python -m py_compile keepkeylib/*.py
 
+      - name: Install contract-test dependencies
+        run: |
+          pip install "protobuf>=3.20,<4" mnemonic ecdsa pytest
+
+      - name: Run deterministic Zcash PCZT contract tests
+        env:
+          PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION: python
+        run: |
+          python -m pytest -q \
+            tests/test_msg_zcash_sign_pczt.py \
+            tests/test_zcash_seed_fingerprint_helper.py
+
       - name: Lint summary
         run: |
           echo "## 🔑 KeepKey python-keepkey — Lint" >> "$GITHUB_STEP_SUMMARY"
           echo "" >> "$GITHUB_STEP_SUMMARY"
           echo "| Check | Status |" >> "$GITHUB_STEP_SUMMARY"
           echo "|-------|--------|" >> "$GITHUB_STEP_SUMMARY"
           echo "| Syntax | ✅ PASS |" >> "$GITHUB_STEP_SUMMARY"
+          echo "| Zcash PCZT contract | ✅ PASS |" >> "$GITHUB_STEP_SUMMARY"
 
   # ═══════════════════════════════════════════════════════════
   # STAGE 2: TEST — pull published emulator, run pytest
@@ -49,26 +68,74 @@ jobs:
   integration:
     needs: [lint]
     runs-on: ubuntu-latest
-    timeout-minutes: 30
+    timeout-minutes: 15
 
-    services:
-      kkemu:
-        image: kktech/kkemu:latest
-        ports:
-          - 11044:11044/udp
-          - 11045:11045/udp
-          - 5000:5000
+    # NO published emulator image. This job BUILDS one from current firmware.
+    #
+    # It used to pull kktech/kkemu:latest -- a floating tag whose image was
+    # five months and six minor versions stale. That single fact caused every
+    # symptom we chased: 80 tests gating on requires_firmware("7.15.0") skipped
+    # silently, and one unskipped test drove a ctime() path that segfaults on
+    # the old image and does not exist in current firmware.
+    #
+    # Publishing a fresher image would only reset that clock. Building from
+    # source removes the class: the emulator under test is, by construction,
+    # the firmware the tests were written against.
 
     steps:
       - uses: actions/checkout@v4
         with:
           submodules: recursive
+          path: python-keepkey
+
+      # python-keepkey is a SUBMODULE of the firmware repo, so the firmware is
+      # where the emulator lives. alpha is the fork's integration branch.
+      - name: Checkout firmware
+        uses: actions/checkout@v4
+        with:
+          repository: BitHighlander/keepkey-firmware
+          ref: alpha
+          path: keepkey-firmware
+
+      # NOT `submodules: recursive`. trezor-firmware carries a micropython
+      # vendor tree whose lib/lwip lives on git.savannah.gnu.org, which serves
+      # dumb HTTP and cannot do the shallow clone actions/checkout requests --
+      # it fails the whole job. The firmware repo's own CI inits exactly these
+      # paths, non-recursively, for the same reason.
+      - name: Init the submodules the emulator build needs
+        working-directory: keepkey-firmware
+        run: |
+          git submodule update --init --depth 1 deps/crypto/trezor-firmware
+          git submodule update --init --depth 1 deps/device-protocol
+          git submodule update --init --depth 1 deps/googletest
+          git submodule update --init --depth 1 deps/qrenc/QR-Code-generator
+          git submodule update --init --depth 1 deps/sca-hardening/SecAESSTM32
+
+      # Test THIS checkout of python-keepkey, not the one the firmware pins.
+      - name: Overlay this python-keepkey onto the firmware tree
+        run: |
+          rm -rf keepkey-firmware/deps/python-keepkey
+          cp -a python-keepkey keepkey-firmware/deps/python-keepkey
+
+      - name: Build the emulator
+        timeout-minutes: 20
+        working-directory: keepkey-firmware
+        run: |
+          docker build -t kkemu-ci -f scripts/emulator/Dockerfile .
+
+      - name: Start the emulator
+        run: |
+          docker run -d --name kkemu \
+            -p 11044:11044/udp -p 11045:11045/udp -p 5000:5000 kkemu-ci
+          sleep 3
+          docker logs kkemu | head -5
 
       - uses: actions/setup-python@v5
         with:
           python-version: '3.11'
 
       - name: Install dependencies
+        working-directory: python-keepkey
         run: |
           pip install --upgrade pip
           pip install "protobuf>=3.20,<4"
@@ -88,20 +155,67 @@ jobs:
             sleep 1
           done
 
+      # "The emulator answered a ping" is not "the emulator is the right
+      # firmware". CI ran a 7.16-era suite against a 7.10.0 image for five
+      # months: 80 tests gate on requires_firmware("7.15.0") and silently
+      # SKIPPED, while one unskipped test drove a code path that segfaults in
+      # 7.10.0 and is already fixed in 7.15 -- which reads as a product failure
+      # but is only a stale image. A floating tag cannot tell you that. This
+      # can, and it fails closed.
+      - name: Assert the emulator is not older than the suite
+        timeout-minutes: 2
+        env:
+          KK_TRANSPORT_MAIN: "127.0.0.1:11044"
+          KK_TRANSPORT_DEBUG: "127.0.0.1:11045"
+          KK_MIN_FW: "7.15.0"
+          KK_UDP_TIMEOUT: "20"
+        working-directory: keepkey-firmware/deps/python-keepkey/tests
+        run: |
+          python - <<'PY'
+          import os, sys
+          sys.path.insert(0, '..')
+          import config
+          from keepkeylib.client import KeepKeyDebuglinkClient
+          c = KeepKeyDebuglinkClient(config.TRANSPORT(*config.TRANSPORT_ARGS,
+                                                      **config.TRANSPORT_KWARGS))
+          c.set_debuglink(config.DEBUG_TRANSPORT(*config.DEBUG_TRANSPORT_ARGS,
+                                                 **config.DEBUG_TRANSPORT_KWARGS))
+          c.init_device()
+          f = c.features
+          got = (f.major_version, f.minor_version, f.patch_version)
+          floor = tuple(int(x) for x in os.environ['KK_MIN_FW'].split('.'))
+          print('emulator firmware %d.%d.%d, floor %s' %
+                (got + (os.environ['KK_MIN_FW'],)))
+          if got < floor:
+              sys.exit('FATAL: the emulator image predates the tests that run '
+                       'against it. Republish kktech/kkemu from current '
+                       'firmware and pin the new digest above.')
+          PY
+
+      # Step-level timeout, deliberately: a JOB-level timeout ends the job as
+      # "cancelled", which reads as an infra blip. A step timeout is a FAILURE.
       - name: Run integration tests
+        timeout-minutes: 8
         env:
           KK_TRANSPORT_MAIN: "127.0.0.1:11044"
           KK_TRANSPORT_DEBUG: "127.0.0.1:11045"
-          PYTHONPATH: "${{ github.workspace }}/keepkeylib:${{ github.workspace }}"
+          PYTHONPATH: "${{ github.workspace }}/keepkey-firmware/deps/python-keepkey"
+          # A crashed emulator now raises instead of blocking in recv() forever.
+          KK_UDP_TIMEOUT: "45"
         run: |
-          cd tests
+          # From the OVERLAID copy, not the standalone checkout: the
+          # storage-version-gate tests assert against lib/firmware/storage.c,
+          # which they find by walking UP. Run them as a sibling of the
+          # firmware and they resolve; run them standalone and they fail
+          # claiming the sources are missing.
+          cd keepkey-firmware/deps/python-keepkey/tests
           pytest -v --junitxml=junit.xml 2>&1 | tee pytest-output.txt
           echo "${PIPESTATUS[0]}" > status
 
       - name: Test summary
         if: always()
         run: |
-          XML="tests/junit.xml"
+          XML="keepkey-firmware/deps/python-keepkey/tests/junit.xml"
           echo "## 🔑 KeepKey python-keepkey — Integration Tests" >> "$GITHUB_STEP_SUMMARY"
           echo "" >> "$GITHUB_STEP_SUMMARY"
 
@@ -148,15 +262,23 @@ jobs:
           echo "---" >> "$GITHUB_STEP_SUMMARY"
           echo "*KeepKey python-keepkey CI*" >> "$GITHUB_STEP_SUMMARY"
 
-      - name: Upload test results
+      # NO check_name. With one, this action publishes a SEPARATE check run
+      # via the Checks API, and its require_tests default of 'false' means an
+      # absent junit.xml -- which is exactly what a killed pytest leaves behind
+      # -- reports conclusion:success with zero duration. That green check sat
+      # on top of a job timing out at 30 minutes for at least six merges.
+      # annotate_only keeps the inline annotations without minting a check.
+      - name: Annotate test results
         uses: mikepenz/action-junit-report@v4
         if: always()
         with:
-          report_paths: tests/junit.xml
-          check_name: Integration Tests
+          report_paths: keepkey-firmware/deps/python-keepkey/tests/junit.xml
+          annotate_only: true
+          require_tests: true
+          fail_on_failure: true
 
       - name: Fail on test failure
         if: always()
         run: |
-          STATUS=$(cat tests/status 2>/dev/null || echo "1")
+          STATUS=$(cat keepkey-firmware/deps/python-keepkey/tests/status 2>/dev/null || echo "1")
           [ "$STATUS" = "0" ] || exit 1

.github/workflows/copilot-review.yml

@@ -1,11 +1,15 @@
 name: Request Copilot Review
 
 on:
-  pull_request:
+  # This workflow never checks out or executes pull-request code. Using the
+  # base-repository context is therefore safe and is required for cross-fork
+  # PRs, whose pull_request GITHUB_TOKEN is always downgraded to read-only.
+  pull_request_target:
     types: [opened, reopened, ready_for_review, synchronize]
 
 jobs:
   request-copilot-review:
+    if: github.event.pull_request.draft == false
     runs-on: ubuntu-latest
     permissions:
       pull-requests: write

build_pb.sh

@@ -3,7 +3,7 @@ CURDIR=$(pwd)
 cd "device-protocol"
 
 echo "Building with protoc version: $(protoc --version)"
-for i in messages messages-ethereum messages-eos messages-nano messages-cosmos messages-ripple messages-binance messages-tendermint messages-thorchain messages-osmosis messages-mayachain messages-solana messages-tron messages-ton messages-zcash types ; do
+for i in messages messages-ethereum messages-eos messages-nano messages-cosmos messages-ripple messages-binance messages-hive messages-tendermint messages-thorchain messages-osmosis messages-mayachain messages-solana messages-tron messages-ton messages-zcash types ; do
     protoc --python_out=$CURDIR/keepkeylib/ -I/usr/include -I. $i.proto
     i=${i/-/_}
     sed -i -Ee 's/^import ([^.]+_pb2)/from . import \1/' $CURDIR/keepkeylib/"$i"_pb2.py

keepkeylib/clearsign_abi.py

@@ -0,0 +1,96 @@
+"""
+Minimal, deterministic Solidity ABI encoder for STATIC types only.
+
+Used to build REAL calldata for the clear-sign flow catalog from a function
+signature + argument values, instead of hand-typing hex (which is how bugs
+get shipped in a signing test suite). Selectors are always derived from
+keccak256(signature) here — never trusted from an external source — so a
+wrong/hallucinated selector fails loudly instead of silently producing a
+plausible-looking but wrong test vector.
+
+Deliberately does NOT support dynamic types (string, bytes, T[], tuples with
+dynamic members) — those need offset/length ABI encoding that's easy to get
+subtly wrong by hand. Calls with dynamic types are hand-built at the call
+site (see clearsign_catalog.py's multicall/handleOps entries) using the
+primitives here (_word/_addr_word) plus an explicit comment that the layout
+is a representative simplification, not a literal captured mainnet tx.
+"""
+
+from .signed_metadata import keccak256
+
+
+def parse_signature(signature):
+    """'supply(address,uint256,address,uint16)' -> ('supply', ['address', 'uint256', 'address', 'uint16'])"""
+    name, rest = signature.split('(', 1)
+    rest = rest.rsplit(')', 1)[0]
+    types = [t.strip() for t in rest.split(',')] if rest.strip() else []
+    return name, types
+
+
+def selector(signature):
+    """4-byte function selector, always computed — never trusted as input."""
+    return keccak256(signature.encode('ascii'))[:4]
+
+
+def _word(value):
+    if isinstance(value, str) and value.startswith('0x'):
+        value = int(value, 16)
+    return int(value).to_bytes(32, 'big')
+
+
+def _addr_word(address):
+    if isinstance(address, str):
+        address = bytes.fromhex(address[2:] if address.startswith('0x') else address)
+    assert len(address) == 20, 'address must be 20 bytes, got %d' % len(address)
+    return b'\x00' * 12 + address
+
+
+def encode_static_args(types, values):
+    """ABI-encode STATIC Solidity types into concatenated 32-byte words.
+    Raises on any dynamic type (string/bytes/arrays) — build those by hand."""
+    assert len(types) == len(values), (
+        'arg count mismatch: %d types, %d values' % (len(types), len(values)))
+    out = bytearray()
+    for typ, val in zip(types, values):
+        if typ == 'address':
+            out += _addr_word(val)
+        elif typ.startswith('uint'):
+            digits = typ[4:]
+            bits = int(digits) if digits else 256
+            n = int(val)
+            assert 0 <= n < (1 << bits), (
+                'value %r out of range for %s' % (val, typ))
+            out += n.to_bytes(32, 'big')
+        elif typ.startswith('int'):
+            # Signed types are NOT unsigned ones with a wider range. intN holds
+            # [-2^(N-1), 2^(N-1)-1] and is encoded two's-complement, sign-
+            # extended to the full word. Treating it as unsigned both rejected
+            # every negative value and silently accepted values at or above
+            # 2^(N-1), which the EVM reads back as NEGATIVE -- calldata that
+            # does not mean what the declared type says.
+            digits = typ[3:]
+            bits = int(digits) if digits else 256
+            n = int(val)
+            lo, hi = -(1 << (bits - 1)), (1 << (bits - 1)) - 1
+            assert lo <= n <= hi, (
+                'value %r out of range for %s (%d..%d)' % (val, typ, lo, hi))
+            out += n.to_bytes(32, 'big', signed=True)
+        elif typ == 'bool':
+            out += (1 if val else 0).to_bytes(32, 'big')
+        elif typ.startswith('bytes') and typ != 'bytes' and not typ.endswith('[]'):
+            n = int(typ[5:])
+            b = val if isinstance(val, (bytes, bytearray)) else bytes.fromhex(
+                val[2:] if val.startswith('0x') else val)
+            assert len(b) == n, 'bytes%d value has wrong length' % n
+            out += b.ljust(32, b'\x00')  # bytesN is left-aligned per ABI spec
+        else:
+            raise ValueError(
+                'dynamic/unsupported type %r — build this call by hand '
+                '(see module docstring)' % typ)
+    return bytes(out)
+
+
+def build_calldata(signature, values):
+    """selector(signature) + ABI-encoded static args, in one call."""
+    _, types = parse_signature(signature)
+    return selector(signature) + encode_static_args(types, values)