Skip to content

feat(emulator): screen-first confirm gating (reactive, Approach B)#250

Merged
BitHighlander merged 2 commits into
developfrom
feat/emu-reactive-confirm
Jun 17, 2026
Merged

feat(emulator): screen-first confirm gating (reactive, Approach B)#250
BitHighlander merged 2 commits into
developfrom
feat/emu-reactive-confirm

Conversation

@BitHighlander

@BitHighlander BitHighlander commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Emulator: screen-first confirm gating (reactive, Approach B)

The emulator now renders the real firmware OLED confirm frame and holds it before the user approves — gating like a physical KeepKey — instead of auto-approving and replaying the captured frames afterward.

This PR has two commits:

  1. Screen-first confirm gating — the feature.
  2. Concurrency review fixes — addressing a thread-safety review of the poll thread.

How (gating)

Driven by a dylib poll thread (firmware, merged to alpha): confirm_helper blocks in C while the Bun event loop stays live, so the confirm becomes reactive — one DebugLinkDecision per firmware ButtonRequest, gated on the user's click (interactive) or auto-pressed (setup ops). hdwallet's ButtonAck flows through; nothing is pre-written.

  • emulator.ts: dylib thread owns the poll (kkemu_start); drop the JS poll timer + pause/pre-poll machinery.
  • emulator-transport.ts: onButtonRequest hook in readChunk + writeDecision; remove ButtonAck suppression and prewrite/cancel.
  • emulator-window.ts: emuGatedConfirm (interactive | auto) replaces the pre-write dance; remove CONFIRM_COUNTS. Unifies the three confirm wrappers.

Concurrency review fixes (2nd commit)

A thread-safety review of the poll-thread change found three real races, all fixed (firmware: BitHighlander/keepkey-firmware #251 + #252, both merged to alpha):

  • Display capture ring → true lock-free SPSC (atomic indices, single-writer each side, drop-on-full).
  • g_poll_running volatile_Atomic (acquire/release).
  • Save-during-confirm deadlocksaveEmulatorState uses non-blocking kkemu_trylock + event-loop yield (proven: waits through a held confirm then acquires, no freeze).
  • Plus: robust Cancel injection on stop, and retiring the unused emuGetDisplay binding (unsafe in thread mode).

Submodule

modules/keepkey-firmware 455ac104f322fc40 (poll thread #251 + thread-safety fixes #252, both merged to alpha).

Test

  • make test-emu30/30 (24 host-driven + 07-reactive-confirm + 08-thread-safety covering the thread + reactive DLD + the no-deadlock save path).
  • tsc --noEmit clean. Manual: EVM sends gate correctly with the held frame.

Review focus (sensitive signing path)

  • Thread lifecycle / flash-snapshot trylock in libkkemu.c + emulator.ts.
  • The reactive DLD timing in emuGatedConfirm / onButtonRequest.

The emulator now renders the real firmware OLED confirm frame and HOLDS it
BEFORE the user approves — gating like a physical KeepKey — instead of
auto-approving and replaying the captured frames afterward.

Driven by the dylib poll thread (firmware submodule bump): confirm_helper
blocks in C while the Bun event loop stays live, so the confirm becomes
reactive — one DebugLinkDecision per firmware ButtonRequest, gated on the
user's click (interactive) or auto-pressed (setup ops). hdwallet's own
ButtonAck flows through; nothing is pre-written.

- emulator.ts: the dylib thread owns the poll (kkemu_start); flash snapshot
  under kkemu_lock; drop the JS poll timer + pause/resume/emuPollOnce machinery.
- emulator-transport.ts: onButtonRequest hook in readChunk + writeDecision;
  remove ButtonAck suppression and prewrite/cancel.
- emulator-window.ts: emuGatedConfirm (interactive | auto) replaces the
  pre-write confirm dance; remove CONFIRM_COUNTS/getConfirmCount.
- index.ts / engine-controller.ts: both emuConfirmOp wrappers delegate to
  emuGatedConfirm (auto mode).
- tests: 07-reactive-confirm.test.ts drives the thread + reactive DLD end to
  end (loadDevice + showDisplay address; result matches the silent path).
- Makefile: ad-hoc re-sign the rebuilt dylib (linker-signed adhoc is rejected
  by standalone bun on Sequoia).

Net -170 lines in the vault. confirm_sm.c untouched and libkkemu.c is
dylib-only, so real-hardware signing is byte-identical.
@BitHighlander
BitHighlander force-pushed the feat/emu-reactive-confirm branch 3 times, most recently from a82da00 to e3f1859 Compare June 16, 2026 23:51
Host-side half of the thread-safety fixes (firmware: BitHighlander
keepkey-firmware#252; submodule bump a40a7ae1 -> f322fc40).

- saveEmulatorState is now async and takes the firmware lock via kkemu_trylock
  + an event-loop yield instead of a blocking kkemu_lock — the poll thread
  holds the lock across a pending confirm, so a blocking acquire from the
  bridge-bearing event loop would deadlock against the click that releases it.
  Callers (emuGatedConfirm, emulatorSave RPC) await it.
- Drop emuGetDisplay + its FFI binding (kkemu_get_display retired in firmware):
  zero callers, and the underlying canvas read was unsynchronized w.r.t. the
  poll thread.
- tests/emulator/08-thread-safety.test.ts: proves save-during-confirm does NOT
  deadlock (it waits through a held confirm then acquires), and drains the
  frame ring cross-thread under load with no malformed frames.
@BitHighlander
BitHighlander force-pushed the feat/emu-reactive-confirm branch from e3f1859 to 99aee2d Compare June 17, 2026 00:02
@BitHighlander
BitHighlander merged commit 82c37bf into develop Jun 17, 2026
1 check passed
@BitHighlander
BitHighlander deleted the feat/emu-reactive-confirm branch June 17, 2026 18:23
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