tests: cover SSKR generation on the two-button devices - #144
Merged
aido merged 1 commit intoAug 4, 2026
Merged
Conversation
test_sskr_128bit.py and test_sskr_256bit.py generate shares on the touch devices and skip on Nano X and Nano S+, so this half of the feature had no end-to-end coverage there: neither the two menus that pick the share count and the threshold (src/bagl/ux_sskr.c) nor generate_sskr() behind them. What that path runs is not screen code. It reaches bolos_ux_bip39_to_sskr_convert() and through it the Shamir split, its randomness, and the CBOR and ByteWords encoding of every share. None of that is device-specific, but the arguments it is called with are: the two-button screens compute them in their own file, from their own context fields, and a mistake there is invisible to the touch tests. The shares cannot be asserted -- the share-set identifier is drawn at random, so two runs of the same split produce different ByteWords. What is fixed is everything in front of it: the CBOR tag aido#6.40309 and the byte-string header of a 21-byte shard, which is what a 12-word seed always produces, and which is the "tuna next keep gyro" the touch tests already assert. Changing one byte of that tag in bolos_ux_bip39_to_sskr_convert() turns this test red, so it is reading generated output rather than a screen that happens to say the right thing. The verdict is asserted before the generation menus are opened, both lines of it. Only the match flow offers to generate, so this is what makes the test mean "generation from a phrase the device agreed with". Two helpers were missing, and neither is a wrapper around navigate_until_text(): choose_in_flow() walks a bounded UX_FLOW. The verdict flows are three fixed steps with no loop, so a right click on the last one changes nothing -- navigate_until_text() reports that as a stuck screen and times out. choose_in_carousel() walks a UX_STEP_MENULIST. Those put several entries on the screen and select one of them, so "3 is on the screen" is not the condition to stop on: on the first screen of the share-count menu it is true while "1" is the entry selected, and validating there picks the wrong one. Same reason enter_letter() below it tests the position rather than the text.
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
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.
Share generation on the two-button devices had no end-to-end coverage.
test_sskr_128bit.pyandtest_sskr_256bit.pygenerate shares on the touch devices and skip on Nano X and Nano S+, so neither the two menus that pick the share count and the threshold (src/bagl/ux_sskr.c) norgenerate_sskr()behind them was ever reached there.That path is not screen code. It goes through
bolos_ux_bip39_to_sskr_convert()to the Shamir split, its randomness, and the CBOR and ByteWords encoding of every share. None of that is device-specific, but the arguments it is called with are: the two-button screens compute them in their own file, from their own context fields, and a mistake there is invisible to the touch tests.The shares themselves cannot be asserted — the share-set identifier is drawn at random, so two runs of the same split produce different ByteWords. What is fixed is everything in front of it: the CBOR tag #6.40309 and the byte-string header of a 21-byte shard, which is what a 12-word seed always produces, and which is the
tuna next keep gyrothe touch tests already assert. Changing one byte of that tag inbolos_ux_bip39_to_sskr_convert()turns this test red, so it is reading generated output rather than a screen that happens to say the right thing.The verdict is asserted before the generation menus are opened, both of its lines. Only the match flow offers to generate, which is what makes this "generation from a phrase the device agreed with" rather than just "generation".
Two helpers
Neither is a wrapper around
navigate_until_text(), for the reasonnano.pyexists at all.choose_in_flow()walks a boundedUX_FLOW. The verdict flows are three fixed steps with no loop, so a right click on the last one changes nothing —navigate_until_text()cannot tell that from a screen that has stopped responding, and reports the second.choose_in_carousel()walks aUX_STEP_MENULIST. Those put several entries on the screen and select one of them, so "3 is on the screen" is not the condition to stop on: on the first screen of the share-count menu it is true while "1" is the entry that would be validated. Same reasonenter_letter()beside it tests the position rather than the text.Verification
Not covered
The successful path only. The error paths on these devices — an invalid phrase, invalid shares, a wrong CRC, too few shares, a duplicated share, a threshold the scheme refuses — remain uncovered, as does the rest of the suite that still skips there.