feat(reset): drive and verify on-device dice-entropy collection - #210
Merged
Conversation
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.
BitHighlander
added a commit
to BitHighlander/keepkey-firmware
that referenced
this pull request
Aug 4, 2026
device-protocol -> cf308fd5e (merge of keepkey/device-protocol#119), python-keepkey -> a8e41261c (merge of keepkey/python-keepkey#210). Both previously pointed at the pre-merge branch commits; the old pins remain ancestors of their base branches, so this is a pin move rather than a content change. The pyk merge also brings in the taproot confirmation-sequence commit that branch had and this one did not -- test_msg_signtx_taproot.py and test_msg_resetdevice.py both pass against kkemu built from these pins (11/11), and the ARM full build is unchanged at .text 616,604.
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
1 participant
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.
What
Host-side support and emulator coverage for the on-device dice-entropy ResetDevice flow (device-protocol keepkey/device-protocol#119, firmware PR follows):
ResetDevice.dice_entropy,DebugLinkDecision.input,DebugLinkState.dice_digest,ButtonRequest_DiceRoll); device-protocol submodule bumped to342174d.debuglink.press_input(text)— chunked synthetic roll injection ('1'-'6', 'u' undo). Each chunk must fit the firmware's 40-charmax_size, which in turn keeps the decoded nanopb struct inside the 64-byte tiny-message buffer.debuglink.read_dice_digest().Test
test_msg_resetdevice.py::test_reset_device_dice(version-gated to 7.15.0, SKIPs on older firmware): asserts theButtonRequest_DiceRollannouncement, injects 99 rolls in 40-char chunks with undo churn while simulating the same append/undo rules host-side, asserts the device digest equals sha256 of exactly the expected roll string, then completes the flow and asserts the post-mix internal entropy still yields the documentedsha256(internal || external)mnemonic.Verified against kkemu built from the firmware branch: 5/5 in
test_msg_resetdevice.py(all four pre-existing tests untouched and green), digest-assert mutation check fails as expected.