Skip to content

test(recovery): gate per-word BIP-39 validation at 7.15.1, as documented - #213

Closed
BitHighlander wants to merge 1 commit into
reconcile/upstream-syncfrom
fix/bip39-word-gate-matches-docstring
Closed

test(recovery): gate per-word BIP-39 validation at 7.15.1, as documented#213
BitHighlander wants to merge 1 commit into
reconcile/upstream-syncfrom
fix/bip39-word-gate-matches-docstring

Conversation

@BitHighlander

Copy link
Copy Markdown
Contributor

The test's own docstring says the feature it exercises requires firmware
7.15.1+, but the gate admitted 7.15.0. Any 7.15.0 build therefore runs a
test for behaviour that release is not expected to have: entering a word
outside the BIP-39 wordlist returns a CharacterRequest for the next word
rather than a Failure, and the assertion fails.

No behaviour change -- the gate now matches the docstring beside it.

The test's own docstring says the feature it exercises requires firmware
7.15.1+, but the gate admitted 7.15.0. Any 7.15.0 build therefore runs a
test for behaviour that release is not expected to have: entering a word
outside the BIP-39 wordlist returns a CharacterRequest for the next word
rather than a Failure, and the assertion fails.

No behaviour change -- the gate now matches the docstring beside it.
@BitHighlander

Copy link
Copy Markdown
Contributor Author

Closing: this suppresses a real signal rather than fixing a misgated test.

The firmware genuinely lacks per-word BIP-39 validation during cipher recovery -- the handler clears decoded_word at a word boundary and advances without checking the wordlist, so an invalid word returns the next CharacterRequest instead of a Failure. The test is correct to fail; the docstring's "7.15.1+" is what misled me.

The fix belongs in firmware: BitHighlander/keepkey-firmware#390 will carry the per-word validation so the test passes for the right reason.

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