Skip to content

Commit 99c2dad

Browse files
committed
Revert "Merge pull request #10 from oroderico/keyboard-layout-setting"
This reverts commit 2167f9d, reversing changes made to a3560da.
1 parent 2167f9d commit 99c2dad

5 files changed

Lines changed: 57 additions & 165 deletions

File tree

‎README.md‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -344,21 +344,13 @@ and padded to four digits.
344344

345345
### The keyboards
346346

347-
The letter keyboards are QWERTY by default, or alphabetical if Settings says so
348-
— `Keyboard: QWERTY` / `Keyboard: ABC`, for readers who do not touch-type and
349-
would rather not hunt. The number keyboard has no such choice, digits having
350-
only one order. The setting lives in Settings, which is reachable only from the
351-
main menu, so it cannot change while a seed is loaded: no session can have half
352-
a passphrase typed on one arrangement and half on the other. Keys are found by
353-
character rather than by position in any case, so the same presses spell the
354-
same string either way.
355-
356-
The cursor opens on the middle key of the middle row — because on a ring of
357-
thirty keys walked with two buttons the corner is the furthest possible place to
358-
start from. It then keeps its place, so repeating a character is one press rather
359-
than a walk back, and returns to the centre when a keyboard is opened or a symbol
360-
page is turned. When the letter it was resting on stops leading to a word, it
361-
moves to the nearest key that still does.
347+
The keyboards are QWERTY. The cursor opens on the middle key of the middle row —
348+
`g` on the letter keyboards, `5` on the number one — because on a ring of thirty
349+
keys walked with two buttons the corner is the furthest possible place to start
350+
from. It then keeps its place, so repeating a character is one press rather than
351+
a walk back, and returns to the centre when a keyboard is opened or a symbol page
352+
is turned. When the letter it was resting on stops leading to a word, it moves to
353+
the nearest key that still does.
362354

363355
The suggestion order, the initially selected key and where a list has scrolled to
364356
are pure functions of what has been typed and chosen. None is randomised: no

‎host/origo_simulator.c‎

Lines changed: 28 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -291,19 +291,19 @@ static bool layout_is_well_formed(const char* layout)
291291
return row && seedtool_layout_center(layout) < seedtool_layout_keys(layout);
292292
}
293293

294-
/* Every letter exactly once, and exactly one backspace: a row with a letter
295-
* dropped would make some BIP39 words impossible to type. Takes the layout
296-
* rather than naming one, because there are now two arrangements of these keys
297-
* and an alphabetical one typed out by hand is exactly where a missing letter
298-
* would come from. */
299-
static bool word_layout_is_complete(const char* const layout)
294+
static bool layouts_are_complete(void)
300295
{
301296
bool seen[128] = { false };
302-
if (!layout_is_well_formed(layout) || seedtool_layout_keys(layout) != SEEDTOOL_LETTERS + 1) {
297+
298+
if (!layout_is_well_formed(SEEDTOOL_WORD_LAYOUT) || !layout_is_well_formed(SEEDTOOL_WORD_NUMBER_LAYOUT)
299+
|| seedtool_layout_keys(SEEDTOOL_WORD_LAYOUT) != SEEDTOOL_LETTERS + 1
300+
|| seedtool_layout_keys(SEEDTOOL_WORD_NUMBER_LAYOUT) != SEEDTOOL_DIGITS + 2) {
303301
return false;
304302
}
305-
for (size_t i = 0; i < seedtool_layout_keys(layout); ++i) {
306-
const unsigned char key = (unsigned char)seedtool_layout_key(layout, i);
303+
/* Every letter exactly once, and exactly one backspace: a QWERTY row with a
304+
* letter dropped would make some BIP39 words impossible to type. */
305+
for (size_t i = 0; i < seedtool_layout_keys(SEEDTOOL_WORD_LAYOUT); ++i) {
306+
const unsigned char key = (unsigned char)seedtool_layout_key(SEEDTOOL_WORD_LAYOUT, i);
307307
if (key != SEEDTOOL_KEY_BACKSPACE && (key < 'a' || key > 'z')) {
308308
return false;
309309
}
@@ -317,45 +317,6 @@ static bool word_layout_is_complete(const char* const layout)
317317
return false;
318318
}
319319
}
320-
return true;
321-
}
322-
323-
/* The pages of one arrangement must reach all 95 printable ASCII characters
324-
* between them. Checked per arrangement, not over both together: a character
325-
* present only in the QWERTY pages would leave a passphrase containing it
326-
* untypeable for a reader who had switched to alphabetical, which is the same
327-
* failure as dropping it outright for half the users. */
328-
static bool passphrase_pages_are_complete(const char* const* const pages)
329-
{
330-
bool seen[128] = { false };
331-
for (size_t page = 0; page < SEEDTOOL_PASSPHRASE_PAGES; ++page) {
332-
if (!layout_is_well_formed(pages[page])) {
333-
return false;
334-
}
335-
for (size_t i = 0; i < seedtool_layout_keys(pages[page]); ++i) {
336-
seen[(unsigned char)seedtool_layout_key(pages[page], i)] = true;
337-
}
338-
}
339-
for (unsigned char character = 0x20; character <= 0x7e; ++character) {
340-
if (!seen[character]) {
341-
return false;
342-
}
343-
}
344-
return true;
345-
}
346-
347-
static bool layouts_are_complete(void)
348-
{
349-
bool seen[128] = { false };
350-
351-
if (!layout_is_well_formed(SEEDTOOL_WORD_NUMBER_LAYOUT)
352-
|| seedtool_layout_keys(SEEDTOOL_WORD_NUMBER_LAYOUT) != SEEDTOOL_DIGITS + 2) {
353-
return false;
354-
}
355-
/* Both arrangements, or the one that is not the default ships unchecked. */
356-
if (!word_layout_is_complete(SEEDTOOL_WORD_LAYOUT) || !word_layout_is_complete(SEEDTOOL_WORD_LAYOUT_ALPHA)) {
357-
return false;
358-
}
359320
/* The number keyboard's characters, not just its key count. enter_word_number
360321
* indexes reachable[key - '0'] for every key that is neither backspace nor
361322
* accept (seedtool_app.c), so a non-digit smuggled into this layout reads
@@ -377,11 +338,25 @@ static bool layouts_are_complete(void)
377338
return false;
378339
}
379340
}
380-
/* Each arrangement's pages must reach all 95 printable ASCII characters, or
381-
* a passphrase typed on an older build - or under the other setting -
382-
* could not be retyped. Space repeats on every page on purpose. */
383-
return passphrase_pages_are_complete(seedtool_passphrase_layouts)
384-
&& passphrase_pages_are_complete(seedtool_passphrase_layouts_alpha);
341+
/* The passphrase pages together must still reach all 95 printable ASCII
342+
* characters, or a passphrase typed on an older build could not be retyped
343+
* on this one. Space repeats on every page on purpose. */
344+
memset(seen, 0, sizeof(seen));
345+
for (size_t page = 0; page < SEEDTOOL_PASSPHRASE_PAGES; ++page) {
346+
const char* const layout = seedtool_passphrase_layouts[page];
347+
if (!layout_is_well_formed(layout)) {
348+
return false;
349+
}
350+
for (size_t i = 0; i < seedtool_layout_keys(layout); ++i) {
351+
seen[(unsigned char)seedtool_layout_key(layout, i)] = true;
352+
}
353+
}
354+
for (unsigned char character = 0x20; character <= 0x7e; ++character) {
355+
if (!seen[character]) {
356+
return false;
357+
}
358+
}
359+
return true;
385360
}
386361

387362
/* Type `digits` one key at a time and require that the keyboard offered every

‎main/seedtool_app.c‎

Lines changed: 12 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,8 @@ _Static_assert(ADDRESS_SHOWN_ROWS <= ADDRESS_LIST_ROWS, "more address rows are s
4949
#define NAV_FOOTER "Up/Down move BOTH select"
5050
#define ACK_FOOTER "BOTH continue Up/Down back"
5151

52-
/* Word entry keyboard: the letters plus backspace. Which arrangement is in use
53-
* is word_layout()'s business; the key count is the same either way, since
54-
* both hold every letter exactly once. */
52+
/* Word entry keyboard: the letters plus backspace. */
53+
#define WORD_LAYOUT SEEDTOOL_WORD_LAYOUT
5554
#define WORD_KEYS (SEEDTOOL_LETTERS + 1)
5655

5756
/* Word number keyboard, for a backup that records numbers rather than words. */
@@ -86,17 +85,6 @@ const char* const seedtool_passphrase_layouts[PASSPHRASE_PAGES] = {
8685
":;<=>?@\n[\\]^_`~\n{|} \b\t\r",
8786
};
8887

89-
/* Alphabetical: the two letter pages reordered, the digit and symbol pages
90-
* shared with the array above rather than repeated - punctuation has no
91-
* alphabetical order, and two copies of the same page would be two places to
92-
* keep a character from going missing instead of one. */
93-
const char* const seedtool_passphrase_layouts_alpha[PASSPHRASE_PAGES] = {
94-
"abcdefghij\nklmnopqrs \ntuvwxyz\b\t\r",
95-
"ABCDEFGHIJ\nKLMNOPQRS \nTUVWXYZ\b\t\r",
96-
"1234567890\n!\"#$%&'()\n*+,-./ \b\t\r",
97-
":;<=>?@\n[\\]^_`~\n{|} \b\t\r",
98-
};
99-
10088
typedef void (*format_fn)(unsigned value, char* output, size_t output_len);
10189

10290
static uint64_t last_action;
@@ -112,23 +100,6 @@ static bool chord_learned;
112100
* every boot - Origo has no persistence to save them to. */
113101
static bool orientation_flipped;
114102
static unsigned backlight_level = SEEDTOOL_DISPLAY_BRIGHTNESS_DEFAULT;
115-
/* Which arrangement the letter keyboards use. Settings is reachable only from
116-
* the Origo menu, and the wallet has no row that returns there, so this cannot
117-
* change while a seed is loaded: no session can have half a passphrase typed
118-
* on one arrangement and half on the other. Keys are found by character rather
119-
* than position in any case, so the same presses spell the same string either
120-
* way - but the ordering is what makes that a fact about the menu tree rather
121-
* than about the reader being careful. */
122-
static bool alphabetical_keys;
123-
124-
/* The letter keyboards in whichever arrangement is set. The number keyboard
125-
* has no alphabetical form and is not routed through here. */
126-
static const char* word_layout(void) { return alphabetical_keys ? SEEDTOOL_WORD_LAYOUT_ALPHA : SEEDTOOL_WORD_LAYOUT; }
127-
128-
static const char* passphrase_layout(const size_t page)
129-
{
130-
return alphabetical_keys ? seedtool_passphrase_layouts_alpha[page] : seedtool_passphrase_layouts[page];
131-
}
132103

133104
static const char* nav_hint(void) { return chord_learned ? "" : " " NAV_FOOTER; }
134105

@@ -830,7 +801,7 @@ static int enter_word(
830801
size_t stem_len = 0;
831802
/* The cursor opens at the centre and then keeps its place between letters,
832803
* so a word is not retyped from the far corner every time. */
833-
size_t selected = seedtool_layout_center(word_layout());
804+
size_t selected = seedtool_layout_center(WORD_LAYOUT);
834805
char title[24];
835806
(void)snprintf(title, sizeof(title), "Word %u/%u", (unsigned)position, (unsigned)total);
836807

@@ -880,13 +851,13 @@ static int enter_word(
880851
bool enabled[WORD_KEYS] = { false };
881852
(void)seedtool_next_letters_in(allowed, stem, stem_len, letters);
882853
for (size_t i = 0; i < WORD_KEYS; ++i) {
883-
const char key = seedtool_layout_key(word_layout(), i);
854+
const char key = seedtool_layout_key(WORD_LAYOUT, i);
884855
enabled[i] = key == SEEDTOOL_KEY_BACKSPACE || letters[key - 'a'];
885856
}
886857
selected = nearest_enabled(enabled, WORD_KEYS, selected);
887858
bool picked = false;
888859
while (!picked) {
889-
seedtool_display_keyboard(title, stem_len ? stem : "-", word_layout(), enabled, selected, position, total);
860+
seedtool_display_keyboard(title, stem_len ? stem : "-", WORD_LAYOUT, enabled, selected, position, total);
890861
switch (wait_key()) {
891862
case KEY_SELECT:
892863
picked = true;
@@ -905,7 +876,7 @@ static int enter_word(
905876
return -1;
906877
}
907878
}
908-
const char key = seedtool_layout_key(word_layout(), selected);
879+
const char key = seedtool_layout_key(WORD_LAYOUT, selected);
909880
if (key == SEEDTOOL_KEY_BACKSPACE) {
910881
erase = true;
911882
} else if (stem_len < SEEDTOOL_MAX_WORD_LEN) {
@@ -1422,10 +1393,10 @@ static int restore_mnemonic(const size_t count, char* mnemonic, const size_t mne
14221393
static bool enter_passphrase_once(char* output, const size_t output_len)
14231394
{
14241395
size_t used = 0, page = 0;
1425-
size_t selected = seedtool_layout_center(passphrase_layout(0));
1396+
size_t selected = seedtool_layout_center(seedtool_passphrase_layouts[0]);
14261397
output[0] = '\0';
14271398
for (;;) {
1428-
const char* const layout = passphrase_layout(page);
1399+
const char* const layout = seedtool_passphrase_layouts[page];
14291400
const size_t keys = seedtool_layout_keys(layout);
14301401
if (selected >= keys) {
14311402
selected = seedtool_layout_center(layout);
@@ -1454,7 +1425,7 @@ static bool enter_passphrase_once(char* output, const size_t output_len)
14541425
page = (page + 1) % PASSPHRASE_PAGES;
14551426
/* A new page is a new keyboard, so the cursor starts from its centre
14561427
* rather than from wherever the page key happened to sit. */
1457-
selected = seedtool_layout_center(passphrase_layout(page));
1428+
selected = seedtool_layout_center(seedtool_passphrase_layouts[page]);
14581429
} else if (pressed == SEEDTOOL_KEY_BACKSPACE) {
14591430
if (used) {
14601431
output[--used] = '\0';
@@ -2776,24 +2747,16 @@ static void show_settings_menu(void)
27762747
orientation_flipped ? "On" : "Off");
27772748
format_brightness(brightness_fraction, sizeof(brightness_fraction));
27782749
(void)snprintf(brightness_item, sizeof(brightness_item), "Brightness: %s", brightness_fraction);
2779-
char keyboard_item[24];
2780-
(void)snprintf(
2781-
keyboard_item, sizeof(keyboard_item), "Keyboard: %s", alphabetical_keys ? "ABC" : "QWERTY");
2782-
const char* items[] = { orientation_item, brightness_item, keyboard_item, "About", "Back" };
2783-
const int selected = choose_kept("Settings", items, 5, true, &cursor);
2784-
if (selected < 0 || selected == 4) {
2750+
const char* items[] = { orientation_item, brightness_item, "About", "Back" };
2751+
const int selected = choose_kept("Settings", items, 4, true, &cursor);
2752+
if (selected < 0 || selected == 3) {
27852753
return;
27862754
}
27872755
if (selected == 0) {
27882756
orientation_flipped = !orientation_flipped;
27892757
seedtool_display_set_orientation(orientation_flipped);
27902758
} else if (selected == 1) {
27912759
show_brightness();
2792-
} else if (selected == 2) {
2793-
/* Toggled in place: two arrangements, and the row names the one in
2794-
* force, so a chooser would be a screen to say what the label
2795-
* says. Nothing to apply - the keyboards read it when they draw. */
2796-
alphabetical_keys = !alphabetical_keys;
27972760
} else {
27982761
(void)page_text("Safety",
27992762
"No seed is stored. No radio, wallet signing, PIN, OTA or serial RPC. Verify the firmware hash and "

‎main/seedtool_app.h‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,6 @@
66
* prove they cover what they must — a rearranged layout that quietly dropped a
77
* character would otherwise leave a passphrase impossible to retype. */
88
#define SEEDTOOL_WORD_LAYOUT "qwertyuiop\nasdfghjkl\nzxcvbnm\b"
9-
/* The same keys in alphabetical order, offered from Settings for readers who
10-
* do not touch-type and find QWERTY a scatter to hunt through. Same 10/9/8
11-
* shape as the layout above, so it fits the three rows the keyboard is drawn
12-
* in without the renderer knowing either of them apart. */
13-
#define SEEDTOOL_WORD_LAYOUT_ALPHA "abcdefghij\nklmnopqrs\ntuvwxyz\b"
149
/* Accept before Backspace, not after: nearest_enabled (seedtool_app.c) walks
1510
* the keyboard as a circular ring, so whichever of the two sits closer to the
1611
* digit row is what a disabled key's cursor snaps to first. Backspace is
@@ -20,13 +15,6 @@
2015
#define SEEDTOOL_WORD_NUMBER_LAYOUT "1234567890\n\r\b"
2116
#define SEEDTOOL_PASSPHRASE_PAGES 4
2217
extern const char* const seedtool_passphrase_layouts[SEEDTOOL_PASSPHRASE_PAGES];
23-
/* The alphabetical passphrase pages. Only the two letter pages differ; the
24-
* digit and symbol pages are the same strings, since there is no alphabetical
25-
* order for punctuation. Both arrays must still cover every printable ASCII
26-
* character between them - the self-test and the Python suite check each one
27-
* separately, because a page dropped from one arrangement and not the other
28-
* would leave a passphrase typeable only under the setting it was made with. */
29-
extern const char* const seedtool_passphrase_layouts_alpha[SEEDTOOL_PASSPHRASE_PAGES];
3018

3119
void seedtool_run(void);
3220

‎tests/test_origo_verify.py‎

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import importlib.util
22
from pathlib import Path
3-
import string
43
import re
54
import unittest
65

@@ -548,42 +547,17 @@ def test_firmware_emits_the_branch_it_is_tested_for(self):
548547
self.assertFalse(fragment in app, why)
549548

550549
def test_passphrase_keyboards_cover_printable_ascii(self):
551-
# Once per arrangement, not over both together: a character present
552-
# only in the QWERTY pages would leave a passphrase containing it
553-
# untypeable for a reader who had switched the keyboard to
554-
# alphabetical, which is the same failure as dropping it outright for
555-
# half the users. The firmware's own self-test asserts the same thing
556-
# against the compiled arrays; this reads the source, so the two agree
557-
# only if the definition really is what ships.
558550
source = (Path(__file__).parents[1] / "main/seedtool_app.c").read_text()
559-
arrays = {
560-
"seedtool_passphrase_layouts[PASSPHRASE_PAGES] = {": "QWERTY",
561-
"seedtool_passphrase_layouts_alpha[PASSPHRASE_PAGES] = {": "alphabetical",
562-
}
563-
for anchor, which in arrays.items():
564-
self.assertIn(anchor, source, f"the {which} passphrase array is not where this test looks for it")
565-
layouts = source.split(anchor, 1)[1].split("};", 1)[0]
566-
covered = set()
567-
for line in layouts.splitlines():
568-
line = line.strip()
569-
if not line.startswith('"'):
570-
continue
571-
body = line[1 : line.rindex('"')]
572-
covered.update(body.replace("\\n", "").replace("\\b", "").replace("\\t", "").replace("\\r", "")
573-
.replace('\\"', '"').replace("\\\\", "\\"))
574-
self.assertEqual(covered, {chr(c) for c in range(0x20, 0x7F)}, f"the {which} pages do not cover ASCII")
575-
576-
def test_word_keyboards_hold_every_letter(self):
577-
# Both arrangements of the letter keyboard, from the header where they
578-
# are defined. A layout with a letter missing makes some BIP39 words
579-
# impossible to type - the reader would reach for a key that is not
580-
# there and have no way to finish a restore.
581-
header = (Path(__file__).parents[1] / "main/seedtool_app.h").read_text()
582-
for name in ("SEEDTOOL_WORD_LAYOUT", "SEEDTOOL_WORD_LAYOUT_ALPHA"):
583-
match = re.search(rf'#define {name} "([^"]*)"', header)
584-
self.assertIsNotNone(match, f"{name} is not defined as a plain string literal")
585-
keys = match.group(1).replace("\\n", "").replace("\\b", "")
586-
self.assertEqual(sorted(keys), sorted(string.ascii_lowercase), f"{name} is not the 26 letters exactly once")
551+
layouts = source.split("passphrase_layouts[PASSPHRASE_PAGES] = {", 1)[1].split("};", 1)[0]
552+
covered = set()
553+
for line in layouts.splitlines():
554+
line = line.strip()
555+
if not line.startswith('"'):
556+
continue
557+
body = line[1 : line.rindex('"')]
558+
covered.update(body.replace("\\n", "").replace("\\b", "").replace("\\t", "").replace("\\r", "")
559+
.replace('\\"', '"').replace("\\\\", "\\"))
560+
self.assertEqual(covered, {chr(c) for c in range(0x20, 0x7F)})
587561

588562
def test_logo_asset_matches_its_header(self):
589563
# The artwork is generated by an authoring tool that needs Pillow, but

0 commit comments

Comments
 (0)