test: consent to runtime provider for metadata integration vectors - #67
test: consent to runtime provider for metadata integration vectors#67BitHighlander wants to merge 3 commits into
Conversation
|
Expanded validation: the complete host suite against the staged native firmware passes with KEEPKEY_RUNTIME_PROVIDER=1: 444 passed, 36 skipped, two EOS vector subtests passed (103.40 seconds). The focused metadata module passes all 20 tests. Production CI remains required; native generator/packing limitations are recorded in the firmware manifest. |
There was a problem hiding this comment.
🟡 Changes recommended
The new runtime provider fixture hard-codes protobuf wire/field constants and overwrites the global wire-ID mapping without collision checks, which can cause fragile or silent failures as the message set evolves.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the EVM clear-signing integration tests to support staged firmware that requires AdvancedMode plus explicit user-consented runtime provider registration before accepting EVM metadata.
Changes:
- Adds an opt-in test mode gated by
KEEPKEY_RUNTIME_PROVIDER=1to enable AdvancedMode, load a runtime signing provider into slot 3, and reset screenshot capture before the test action. - Introduces a new
tests/runtime_provider.pyfixture that dynamically defines and registers a minimal protobuf message type used to load the clearsign signer pubkey.
File summaries
| File | Description |
|---|---|
| tests/test_msg_ethereum_clear_signing.py | Conditionally enables AdvancedMode, loads the runtime provider, and clears setup screenshots when KEEPKEY_RUNTIME_PROVIDER=1. |
| tests/runtime_provider.py | New fixture module that defines a narrow dynamic protobuf message and uses it to load the clearsign signer identity via ButtonRequest → Success flow. |
Review details
Suppressed comments (1)
tests/runtime_provider.py:17
- Registering the custom wire message uses a bare
117and unconditionally overwrites any existing mapping entry. If the audited enum is extended later (or another fixture registers the same ID), this will silently change behavior. Introduce a named constant and fail fast if the wire ID is already registered.
mapping.map_class_to_type[LoadSigner] = 117
mapping.map_type_to_class[117] = LoadSigner
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Round 1 body-only disposition: 6dfd3a5 checks both mapping directions for the named runtime-provider wire ID before registration, preventing silent collisions. The existing 20 metadata tests pass with the guarded fixture. |
There was a problem hiding this comment.
🟡 Changes recommended
The new env-gated setup path should handle missing optional fixture dependencies more gracefully and includes a small import-clarity cleanup to avoid redundant imports.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tests/test_msg_ethereum_clear_signing.py:18
- Now that os is imported at module scope, the fallback ImportError handler no longer needs to re-import os; keeping it duplicates imports and makes it less clear which imports are actually required in that branch.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
|
Round 2 body-only import cleanup addressed in 4da83a2. The optional-dependency skip suggestion is declined because the explicit runtime-provider CI mode requires real signing; missing dependencies must fail the gate. |
The staged firmware requires AdvancedMode and a user-confirmed runtime provider before accepting EVM metadata. The legacy integration fixture assumed a built-in test signer, causing the required OLED phase to fail before the full suite ran.
When KEEPKEY_RUNTIME_PROVIDER=1, the fixture enables AdvancedMode, requires ButtonRequest then Success while loading the actual vector signing key into slot 3, and clears setup screenshots before the tested action. Default baseline behavior stays unchanged. Existing signature and malformed-payload expectations remain intact.
Validation: all 20 tests in test_msg_ethereum_clear_signing.py pass against the staged native emulator. CI will enable this mode starting at firmware F04. This dependency stacks on #66 and remains unmerged; full production CI and Copilot review remain required.