Merge main, and keep the checksum filter on the number field

The branch predated #4, and rewriting enter_word_number around the scrolled
field dropped the wordset the letter keyboard narrows by: the signature lost
`allowed` and the body called the unnarrowed seedtool_word_number. On a
mnemonic's last word that leaves the number pad accepting words the letters
refuse - the exact asymmetry #4 exists to prevent, and worse than having
neither, since the reader who used the number pad has been told nothing.

Nothing caught it. The self-test proves seedtool_word_number_in exact against
all 2048 words, but that proves the function; the screen quietly not calling it
leaves every test green. So the filter is back in three places and pinned in a
fourth:

- the ring is built from seedtool_next_digits_in, so a digit that cannot begin
  an allowed word is never scrollable to in the first place;
- OK appears only when seedtool_word_number_in says the digits already name an
  allowed word;
- the confirmation gate narrows too. The ring should already have made a
  disallowed number untypeable, but this is where a number becomes a word, and
  a gate inheriting its guarantee from the screen upstream is one bug away from
  accepting what that screen was supposed to refuse - the same "verified rather
  than inherited" the address index gets in browse_addresses;
- a source-grep test asserts enter_word_number takes the set and calls neither
  plain function, proved by putting the unnarrowed call back and watching it
  fail by name.

The box geometry was arithmetic checked by eye. Four _Static_asserts now hold
it: the widest field against the display, and the arrows against the boxes,
the title and the footer. The row is centred, so boxes that do not fit run off
both edges where fill_rect clips them silently - a field that looks wrong on
the device and fails nowhere. SEEDTOOL_DIGIT_BOXES_MAX names what the renderer
can draw, and the app asserts a word number's digits fit inside it, so the two
ends are held together rather than agreeing by coincidence. Proved by widening
a box to 60px and watching the build fail.

Verified with the host self-test and 50 Python tests.

Author: oroderico

.claude/skills/audit/SKILL.md

@@ -0,0 +1,194 @@
+---
+name: audit
+description: Audit this Origo firmware tree for quality, quantity and security. Runs the project's own gates - host self-test, Python tests, QR smoke test, sanitized build, firmware build and ELF audit - then reports the numbers they emit and examines the invariants none of them cover. Use when asked to audit, review the health of, or check the state of the repository, before a release, or after a run of changes. Not a substitute for /code-review or /security-review, which cover generic ground this deliberately does not repeat.
+---
+
+# Auditing Origo
+
+Origo carries more self-verification than most projects its size: an ELF audit
+that proves the linked surface, a host self-test with thirty-odd named checks,
+a Python suite that includes structural greps of the C, a sanitized build, a
+fuzzer, and a byte-for-byte reproducibility check. All of it is pass/fail and
+none of it reports a trend.
+
+This audit exists for the space around those gates. Its value is not in
+re-proving what they already prove — it is in reading the numbers they emit
+before those numbers become failures, and in looking at the claims the project
+makes about itself that nothing mechanical checks.
+
+## Ground rules
+
+**Never re-derive what a tool already proves.** Run the tool, read its output,
+move on. Time spent hand-checking something `audit_origo_elf.py` already
+guarantees is time not spent on the parts nothing guarantees.
+
+**Exclude `jade/` from every repo-wide search.** It is an untracked reference
+copy of Blockstream Jade — over a thousand files that will drown any grep and
+produce findings about code that is not part of this project. Use
+`--exclude-dir=jade` or restrict paths to `main/ host/ tools/ tests/`.
+
+**Separate measured from inferred.** A number read from a tool's output and a
+conclusion drawn by reading code are different kinds of claim, and the report
+should not blur them. Cite `file:line` for anything found by reading.
+
+**Report what is unproven, not just what is wrong.** "This invariant is stated
+in the README and nothing enforces it" is a finding. Say what would enforce it.
+
+## Stage 1 — run the gates
+
+Cheapest first, so a failure surfaces before an expensive build.
+
+```sh
+./tools/run-simulator.sh --self-test
+python3 -m unittest discover -s tests
+cc -std=c11 -Wall -Wextra -Werror -Imain main/qrcode.c tests/qrcode_smoke.c \
+  -o /tmp/origo-qrcode-smoke && /tmp/origo-qrcode-smoke
+cmake -S host -B build-sanitize -DCMAKE_BUILD_TYPE=Debug \
+  -DCMAKE_C_COMPILER=clang -DORIGO_SANITIZE=ON \
+  && cmake --build build-sanitize --parallel \
+  && ./build-sanitize/origo-simulator --self-test
+```
+
+`unittest discover` rather than pytest: it is what CI runs, and the audit
+should fail the way CI fails.
+
+Then the firmware build and ELF audit. **Use the pinned IDF**, not whatever
+`idf.py` is on PATH:
+
+```sh
+bash -c 'source ~/esp-idf-v5.5.4/export.sh >/dev/null 2>&1 && cd /home/cmpc/origo \
+  && idf.py build \
+  && python3 tools/audit_origo_elf.py build/origo.elf --map build/origo.map \
+       --bin build/origo.bin --nm xtensa-esp32-elf-nm'
+```
+
+The default `~/esp/esp-idf` has drifted to 6.0.2 while `dependencies.lock` pins
+5.5.4; building with it fails a toolchain check before reaching any project
+code. If the build fails that way, say so as an environment finding rather than
+a code finding — and do not "fix" it by letting the lock file be rewritten.
+
+Report each gate as pass or fail with the line that says so. A failure here
+ends the audit: everything below assumes a tree that builds.
+
+## Stage 2 — quantity
+
+### Firmware and memory
+
+The ELF audit prints the binary size on success and fails only at the ceiling
+(`--max-bin-size`, default 295 KiB = 302,080 bytes, `tools/audit_origo_elf.py`).
+Between those two states it says nothing, so report the headroom explicitly:
+
+- bytes, and percentage of the ceiling
+- bytes remaining
+- the delta against the previous commit's binary when one is available
+
+A build sitting above 95% of the ceiling is a finding in its own right, even
+while it passes: the project has already refused a feature on size grounds
+(deflate, ~2 KiB, rejected because the decompressor is a component the audit
+forbids), so remaining space is a design constraint and not just a number.
+
+Then measure what nothing measures:
+
+```sh
+bash -c 'source ~/esp-idf-v5.5.4/export.sh >/dev/null 2>&1 && cd /home/cmpc/origo \
+  && idf.py size && idf.py size-components'
+```
+
+`.bss`, `.data` and IRAM have no budget and no automated report anywhere, yet
+`main/seedtool_app.c` reasons carefully about `.bss` cost in several comments.
+Record the section breakdown so successive audits can compare it.
+
+### Check coverage
+
+Inventory what actually enforces what, and where the gaps are:
+
+- `_Static_assert` sites in `main/` — compile-time invariants
+- source-grep pins in `tests/test_origo_verify.py` — invariants a compiler
+  cannot see, pinned by asserting on the C source text
+- named checks in `host/origo_simulator.c`'s `self_test()`
+
+Then the part that matters: **claims the project states in prose that nothing
+checks.** Read README's Safety boundaries, `SECURITY.md`, `CONTRIBUTING.md` and
+the header comments in `main/`, and for each concrete claim ask what would fail
+if it stopped being true. Known members of this set, to re-check rather than
+rediscover:
+
+- "every screen that holds seed material wipes its buffers when it is left"
+- the partition table having no NVS, PHY-data or OTA slot
+- `CONFIG_ESP_WIFI_ENABLED` / `CONFIG_BT_ENABLED` being absent from the
+  generated `sdkconfig` entirely, rather than set to `n`
+- the logo being "the only picture in the firmware"
+- the RNG being stubbable without changing any generated mnemonic
+
+### Code volume
+
+Lines per file, and the largest functions. `main/seedtool_app.c` holds every
+screen and is the file that grows; report concentration and growth rather than
+absolute size, which on its own means little. A function that has doubled since
+the last audit is worth a look even if it is still short.
+
+## Stage 3 — security
+
+Origo-specific. Generic vulnerability classes belong to `/security-review`.
+
+**Secret wiping.** The stated boundary is that every screen holding seed
+material wipes its buffers on the way out, and `seedtool_zero` is called around
+a hundred times to that end — with no test verifying a single one of them. For
+each buffer holding a mnemonic, passphrase, seed or raw entropy, trace every
+exit: success, cancel, error, and timeout. The error and timeout paths are
+where real gaps have been found before, twice, so weight them accordingly. A
+buffer in `.bss` deserves more suspicion than one on the stack, since it
+persists.
+
+**RNG confinement.** `tests/test_origo_verify.py` greps three files for RNG
+calls and proves absence there. Check that no new path reaches the RNG, and
+note that no test actually stubs the RNG and re-derives a mnemonic — which is
+precisely the claim the README's opening makes.
+
+**The unchecked structural claims.** Read `partitions.csv` and confirm it still
+holds only the factory app. Grep the generated `sdkconfig` for the WiFi and
+Bluetooth symbols. Check whether a second image asset has appeared beside the
+logo. These take a minute each and none of them is covered by CI.
+
+**Fuzzer**, time-boxed. CI omits it deliberately — coverage-guided fuzzing has
+no natural stopping point — so it only ever runs when someone chooses to run
+it. The recipe and the last clean run are recorded in the header of
+`tests/fuzz_parsers.c`. Report executions reached and any finding.
+
+## Stage 4 — quality
+
+Judge against the project's own stated conventions, in `CONTRIBUTING.md`, not
+against generic taste: the `seedtool_`/`SEEDTOOL_` prefix on public names,
+snake_case throughout, and comments that justify *why* rather than restating
+*what*.
+
+**Comment and documentation staleness is a first-class finding here.** Comments
+in this codebase carry design reasoning — why a buffer is sized the way it is,
+why a screen is ordered the way it is — so a comment that has outlived its code
+is misinformation rather than clutter, and it is the failure mode this project
+is most prone to, because the reasoning is dense and the code around it moves.
+Cross-check comments against the code they describe, and README and
+CONTRIBUTING against the commands and structures they document. A documented
+build command that contradicts CI is the worked example of the genre.
+
+Also confirm the dependency posture holds: `tools/make_logo.py` remains the one
+place Pillow is used, and the firmware, verifier and tests stay dependency-free.
+
+## Report
+
+Order findings by severity. For each: what it is, `file:line`, what proves it
+— or, when nothing does, what would.
+
+End with the measured numbers in a fixed shape, so two audits can be compared
+at a glance:
+
+```
+firmware      N bytes (P% of ceiling, R bytes free), delta vs <ref>
+sections      .bss N, .data N, IRAM N
+gates         self-test / tests N passed / QR smoke / sanitized / ELF audit
+coverage      N static asserts, N source-grep pins, N self-test checks
+unenforced    N stated invariants with nothing checking them
+```
+
+Say plainly when a stage was skipped and why. An audit that quietly omits the
+firmware build because the toolchain drifted is worse than one that says so.

CONTRIBUTING.md

@@ -34,11 +34,20 @@ exactly what CI (`.github/workflows/build.yml`) checks:
 # plus the RNG-absence and pure-function checks.
 python3 -m unittest discover -s tests -v
 
-# QR encoder, built standalone with warnings as errors.
-cc -std=c11 -Wall -Wextra -Werror -DLOCK_VERSION=5 -Imain \
+# QR encoder, built standalone with warnings as errors. No -DLOCK_VERSION:
+# the firmware does not define it either, so qrcode.h's version guards are
+# preprocessed out here exactly as they are there.
+cc -std=c11 -Wall -Wextra -Werror -Imain \
   main/qrcode.c tests/qrcode_smoke.c -o /tmp/origo-qrcode-smoke
 /tmp/origo-qrcode-smoke
 
+# The same self-test again, instrumented: the out-of-bounds access and the
+# undefined behaviour that reading the code did not think to look for.
+cmake -S host -B build-sanitize -DCMAKE_BUILD_TYPE=Debug \
+  -DCMAKE_C_COMPILER=clang -DORIGO_SANITIZE=ON
+cmake --build build-sanitize --parallel
+./build-sanitize/origo-simulator --self-test
+
 # Firmware build, twice, independently audited and then compared
 # byte-for-byte: a non-reproducible build is treated as a failure.
 idf.py -B build-a -D SDKCONFIG=build-a/sdkconfig build

README.md

@@ -302,7 +302,7 @@ never padded with blank rows. Only labels are listed. Values meant to be transcr
 instead, split by what fits the display.
 
 The last row of every list with somewhere to return to is the way out of it —
-`Back`, `Done / erase`, `[delete]` — and a rule is drawn above it so it is not
+`Back`, `Erase and restart`, `[delete]` — and a rule is drawn above it so it is not
 read as one more choice. That row is always last, in every such list, so
 leaving a screen is always in the same place. The one list with nowhere to
 return to is the main menu itself: there is no `Back` row there, and the
@@ -320,6 +320,21 @@ digits that still lead to a word number are reachable, and the number is shown a
 the word it means, to be confirmed, before the next word is asked for. In both
 cases deleting past the start of a word steps back to the previous one.
 
+Restoring a mnemonic narrows the **last** word further still. That word carries
+the checksum, so most of the wordlist cannot end a given eleven or twenty-three:
+its eleven bits are the leftover entropy bits followed by the checksum bits,
+which leaves 128 of the 2048 words possible for a 12-word mnemonic and only 8
+for a 24-word one. The keyboard offers those and nothing else — so a word misread
+off a metal plate is not typeable in the first place, rather than being reported
+as `Invalid checksum` after the whole mnemonic has been entered. Few enough
+remain for a 24-word restore that entry lists them outright instead of asking
+for a letter. The narrowing is exact in both directions: everything it excludes
+genuinely fails validation, and no correct seed becomes harder to enter. It
+applies to both entry methods and to the last word on the review screen, where
+it is rebuilt from the words currently entered — but not to "Complete checksum",
+whose 11 or 23 words have no checksum in them yet, and not to the backup quiz,
+which asks what the reader wrote down.
+
 A word number is one-based: the position in a printed BIP39 English wordlist,
 where `abandon` is 1 and `zoo` is 2048. It is one more than the zero-based index
 the encoding itself uses, so a list numbered from zero must be read with that in
@@ -345,24 +360,50 @@ screen in the entry path may depend on the device RNG.
 
 An optional printable-ASCII passphrase of at most 100 characters is typed on a
 four-page keyboard covering the whole printable range, entered twice, and exists
-for that derivation session only.
+for that derivation session only. It is set from the wallet viewer's Derivation
+screen rather than asked for on the way in: a session begins with none, and the
+screen states which of the two is in force rather than leaving it to be assumed.
+Changing or clearing it re-derives the master fingerprint, since that
+fingerprint is a function of it — so the fingerprint is also the check that the
+passphrase in force is the intended one.
 
 ## Restore and inspect
 
 Existing mnemonics are validated before derivation; a bad checksum blocks the
-address viewer entirely. For a valid mnemonic the viewer shows the master
-fingerprint, then one wallet type at a time — Native SegWit (BIP84) or Taproot
-(BIP86) — chosen up front, so the account key and its addresses are always read
-for the type just picked rather than interleaved with the other one's. Each
-type shows:
-
-- its watch-only account key at `m/84'/0'/0'` or `m/86'/0'/0'`, titled with its
-  key origin such as `[73c5da0a/84'/0'/0']`. Native SegWit can be shown as
-  either standard BIP32 `xpub` or, on request, its SLIP-132 `zpub` — the same
-  78 bytes with the four version bytes swapped, since libwally itself
-  serialises only the plain BIP32 versions and has no notion of SLIP-132.
-  Taproot is `xpub` only: SLIP-132 defines no taproot version prefix, so there
-  is no `zpub`-equivalent to offer it as;
+address viewer entirely. For a valid mnemonic the viewer's own menu is titled
+with the master fingerprint — `Wallet @73c5da0a` — rather than spending a row
+on showing it. It is an identity rather than an action, and being read without
+being asked for is what makes it a check: it is a function of the passphrase in
+force and moves the moment that does, so a reader who knows their wallet's
+fingerprint sees at a glance whether the device is deriving that wallet.
+
+A **Derivation** screen holds the three things that decide what everything else
+derives, ordered by how deep each one cuts: the optional passphrase, which
+decides the seed itself and so every key the device can produce; the wallet
+type — Native SegWit (BIP84) or Taproot (BIP86) — which picks a path from that
+seed; and the account index (`m/type'/0'/account'`, 0 through 999), which picks
+a branch of that path. All three last for the whole viewing session rather than
+one visit to a screen, so checking account 2 under both types means setting it
+once. The passphrase
+can be changed or removed at any point without leaving the session, its own row
+says which of the two is in force, and the fingerprint in the title is
+re-derived whenever it changes. One type is in force at a time, so an
+account key and its addresses are always read for the type currently set
+rather than interleaved with the other one's — the derivation path shown with
+every value names it, so what is on screen says which type produced it rather
+than leaving it to be remembered. The wallet menu itself then offers, for
+whatever is set:
+
+- **Extended public key**, holding every way the account's watch-only key at
+  `m/84'/0'/0'` or `m/86'/0'/0'` leaves the device — they are the same 78 bytes
+  three ways, so they sit together rather than as separate rows that look
+  unrelated while carrying identical risk. As `xpub`, titled with its key
+  origin such as `[73c5da0a/84'/0'/0']`; as SLIP-132's `zpub`, the same 78
+  bytes with the four version bytes swapped, since libwally serialises only the
+  plain BIP32 versions and has no notion of SLIP-132 — Native SegWit only,
+  since SLIP-132 defines no taproot prefix, and under Taproot that row is
+  absent rather than present and refusing; or as the output descriptor
+  described below;
 - its mainnet addresses for indices 0 through 99, on whichever branch is asked
   for: `Addresses` opens a Receive/Change choice first, and the list that
   follows is `m/84'/0'/0'/0/i` or `m/86'/0'/0'/0/i` for receive,
@@ -378,7 +419,8 @@ type shows:
   account already costs and keeps the cache the size it was. The chain step
   sits below the account key, so only the cheap half of the derivation
   actually repeats;
-- its BIP380 output descriptor — script type, key origin, account key and an
+- its BIP380 output descriptor, the third entry under Extended public key —
+  script type, key origin, account key and an
   8-character checksum in the one string a watch-only wallet imports directly,
   as `wpkh([73c5da0a/84'/0'/0']xpub.../<0;1>/*)#hpg6d6w2` or its `tr()`
   equivalent. The chain step is BIP389's multipath `<0;1>`, so the single