From 3aecffeb2b87ad4b190e2fbbe5bb2155a1776dd2 Mon Sep 17 00:00:00 2001 From: oroderico Date: Sun, 16 Aug 2026 10:53:07 -0400 Subject: [PATCH] Stop Descriptor and the back arrow doing each other's job In Extended public key, choosing "Descriptor" left the menu and taking the back arrow opened the descriptor. Both address types are affected. choose_menu_at returns the arrow as the index one past the last row - the slot the Back row occupied before the arrow replaced it - so a caller checks for `count`. This one checked `count - 1`, which is the last real item, and here that item is "Descriptor": the check caught it and returned, while the arrow's own value fell past every branch to the else that opens the descriptor. It was correct when written. The menu had an explicit "Back" row then, so `count - 1` was that row's index. Lifting Back into the arrow was meant to be transparent for callers - the arrow returns the index the row used to have, so "the checks against its old index still read the same answer" - and it is, for a caller comparing against a literal, which is what Backup, Settings and Derivation each do. This one derived its check from `count`, and `count` itself shrank by one when the row left the array, so the comparison slid off the arrow and onto the last item. Silent, because the result is still a valid index. The other eight menu call sites were read for the same shape; none derives its exit test from `count`, and the two that compute their row count still compare against the right value. Nothing here would have caught it: the self-test covers rendering and geometry, not menu dispatch, and these functions are static in seedtool_app.c and out of the simulator's reach. Adding a check that would is worth doing and is not attempted here. Verified with the host self-test and the 51 Python tests. The dispatch itself was checked by reading every menu call site, not by walking the menu - see above for why this tree has no way to assert it. --- main/seedtool_app.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/main/seedtool_app.c b/main/seedtool_app.c index a0dd822..34775c3 100644 --- a/main/seedtool_app.c +++ b/main/seedtool_app.c @@ -2108,7 +2108,14 @@ static void show_extended_keys(const char* mnemonic, const char* passphrase, con } items[count++] = "Descriptor"; const int selected = choose_menu_at("Extended public key", items, count, &cursor); - if (selected < 0 || (size_t)selected == count - 1) { + /* `count`, not `count - 1`: choose_menu_at returns the arrow as the + * index one past the last row - the slot the Back row used to occupy - + * so `count - 1` is the last real item instead. Here that item is + * "Descriptor", which made the two swap: picking Descriptor left the + * menu, and taking the arrow fell through to the else below and opened + * the descriptor. The other menus compare against a literal that + * already equals count, which is why this was the only one left. */ + if (selected < 0 || (size_t)selected == count) { return; } if (selected == 0) {