diff --git a/docs/developer/api.md b/docs/developer/api.md index e4c20bf..1819106 100644 --- a/docs/developer/api.md +++ b/docs/developer/api.md @@ -116,7 +116,7 @@ documentation and enforcement update. installed as `codex32[gui]` and started by `codex32-gui`. It is a client of the surface above and of the private Core adapter; nothing in `src/codex32/` imports it, and the base install keeps its property of having no third-party runtime -dependency. It carries its own budget of 2,000 logical review lines, separate +dependency. It carries its own budget of 2,050 logical review lines, separate from the 5,200 above. Its own boundaries are documented in [`gui.md`](gui.md) and enforced by `tests/test_gui_boundaries.py`. diff --git a/docs/developer/gui.md b/docs/developer/gui.md index b0b0692..413ea47 100644 --- a/docs/developer/gui.md +++ b/docs/developer/gui.md @@ -19,7 +19,7 @@ cryptography, entropy source, socket, or file storage. Review `reading.py`, `wallet_setup.py`, and `work.py` first. Their behavior is covered without a display. `tools/gui_walkthrough.py` exercises the real GTK -screens under Xvfb. `tests/test_gui_boundaries.py` enforces a separate 2,000 +screens under Xvfb. `tests/test_gui_boundaries.py` enforces a separate 2,050 logical-line GUI budget. ## Security boundaries @@ -35,8 +35,8 @@ logical-line GUI budget. `codex32._bitcoin_core`. 5. **Page secrets are cleared.** `_forget_when_gone` clears card or entry text when an `AdwNavigationView` page leaves the stack. -6. **One worker at a time.** `work.run` serializes background jobs, returns on the - GTK thread, and drops callbacks for pages that are gone. +6. **One worker at a time.** `work.run` and the low-priority wallet poll share one + gate, return on the GTK thread, and drop callbacks for pages that are gone. 7. **Accessibility is opt-in.** `__init__.py` defaults `GTK_A11Y` to `none` before GTK loads; an operator can override it. 8. **Labels never select wallets.** Choice rows use position and disable markup; @@ -110,7 +110,6 @@ wallet. `_Answer.tell` also rejects terminal-only “press Ctrl-C” waits. ## Known limits - Long cards scroll horizontally because entry uses one `Gtk.Entry`. -- Empty-wallet lists refresh on request, not continuously. - Widget construction is covered by `tools/gui_walkthrough.py`, not pytest. - On X11, selecting entry text briefly exposes it through the primary selection. - Enabling GTK accessibility exposes GUI text to other processes on that bus. diff --git a/src/codex32_gui/pages.py b/src/codex32_gui/pages.py index 4621912..fbfdd1c 100644 --- a/src/codex32_gui/pages.py +++ b/src/codex32_gui/pages.py @@ -12,7 +12,7 @@ from dataclasses import dataclass from typing import Literal -from gi.repository import Adw, Gtk +from gi.repository import Adw, GLib, Gtk from codex32 import ( ConfirmationResult, @@ -46,6 +46,7 @@ (0, 1, "One card"), ) CREATE_WALLET = "Create a new wallet" +WALLET_REFRESH_MS = 1000 NO_CAMERA = ( "Do not photograph this and do not type it into any website, chat or password manager. " "Paper and pen only." @@ -751,32 +752,72 @@ def _wallet_page( restoring: bool, ) -> Adw.NavigationPage: """Name the wallet that will hold the keys. The library confirms that name again.""" - group = Adw.PreferencesGroup(title="Empty wallets Bitcoin Core has ready") - rows = [(item.name, "Empty, encrypted" if item.encrypted else "Empty, not encrypted") for item in found] - rows.append((CREATE_WALLET, "codex32 asks Bitcoin Core for a blank wallet, with a passphrase you choose")) - buttons = _radio_group(group, rows) + holder = Gtk.Box(orientation=Gtk.Orientation.VERTICAL) + current = found + buttons: list[Gtk.CheckButton] = [] def go() -> None: # By position, so that a wallet named like the create row is still reachable. - index = _selected(buttons) - if index == len(found): + index = next((i for i, choice in enumerate(buttons) if choice.get_active()), -1) + if index < 0: + return + if index == len(current): view.push(_new_wallet_page(view, core, secret, timestamp, restoring)) return - chosen = found[index] + chosen = current[index] if chosen.locked: view.push(_unlock_page(view, core, secret, chosen, timestamp, restoring)) return _import(view, core, secret, chosen.name, "", timestamp, restoring) + continue_button = _button("Continue", go, style="suggested-action") + + def render(updated: tuple[wallet_setup.Wallet, ...]) -> None: + nonlocal buttons, current + preserve = bool(buttons) + selected = next((i for i, choice in enumerate(buttons) if choice.get_active()), -1) + chosen = current[selected].name if 0 <= selected < len(current) else None + creating = preserve and selected == len(current) + if (old := holder.get_first_child()) is not None: + holder.remove(old) + current = updated + group = Adw.PreferencesGroup(title="Empty wallets Bitcoin Core has ready") + rows = [ + (item.name, "Empty, encrypted" if item.encrypted else "Empty, not encrypted") for item in current + ] + rows.append( + (CREATE_WALLET, "codex32 asks Bitcoin Core for a blank wallet, with a passphrase you choose") + ) + buttons = _radio_group(group, rows) + for choice in buttons: + choice.connect( + "toggled", + lambda _choice: continue_button.set_sensitive(any(item.get_active() for item in buttons)), + ) + holder.append(group) + if preserve: + target = ( + len(current) + if creating + else next((i for i, item in enumerate(current) if item.name == chosen), None) + ) + if target is None: + buttons[0].set_active(False) + else: + buttons[target].set_active(True) + continue_button.set_sensitive(any(choice.get_active() for choice in buttons)) + + render(found) + content = _column( _title( "Which wallet should hold your keys?", f"Bitcoin Core {wallet_setup.version_text(core)} is running on {wallet_setup.network(core)}.", ), - group, + holder, _note( "Only empty wallets are listed, so no wallet you already use can be overwritten. You may also " - "create one in Bitcoin Core yourself and check again." + "create one in Bitcoin Core yourself; it will appear here automatically." ), *( ( @@ -790,15 +831,26 @@ def go() -> None: else () ), ) - return _page( + page = _page( "Wallet", content, - actions=_actions( - _button("Check again", lambda: _wallets(view, core, secret, timestamp)), - _button("Continue", go, style="suggested-action"), - ), + actions=_actions(continue_button), ) + def refreshed(outcome: tuple[wallet_setup.Wallet, ...] | Exception) -> None: + if view.get_visible_page() is page and not isinstance(outcome, Exception) and outcome != current: + render(outcome) + + def refresh() -> bool: + if not work.showing(view, page): + return False + if view.get_visible_page() is page: + work.poll(view, page, lambda: wallet_setup.eligible(core), refreshed) + return True + + GLib.timeout_add(WALLET_REFRESH_MS, refresh) + return page + def _new_wallet_page( view: Adw.NavigationView, diff --git a/src/codex32_gui/work.py b/src/codex32_gui/work.py index 3bcfa94..474dd54 100644 --- a/src/codex32_gui/work.py +++ b/src/codex32_gui/work.py @@ -14,8 +14,7 @@ "That operation stopped in a way this program did not expect. If it was writing to Bitcoin " "Core, check there what state the wallet is in before trying again." ) -_BUSY = "Another operation is still finishing. Try again in a moment." -_running = False +_gate = threading.Lock() def showing(view: Adw.NavigationView, page: Adw.NavigationPage) -> bool: @@ -35,50 +34,59 @@ def run[Result]( work: Callable[[], Result], done: Callable[[Result | Exception], None], ) -> None: - """Run one blocking library call off the main loop and deliver its result back. + """Queue one blocking library call off the main loop and deliver its result back. `correct` runs for up to ten seconds and every Bitcoin Core call waits on a - subprocess, so neither may run on the main loop. The page that starts an - operation shows a spinner and cannot be left, so only one is ever in flight, - and a result for a page that is gone anyway is dropped. The result is - delivered from `finally`, so a failure no screen anticipated still releases - the program instead of leaving it spinning. + subprocess, so neither may run on the main loop. Background jobs share one + gate, so a wallet-list poll cannot overlap an import. A result for a page + that is gone is dropped. The thread is deliberately not a daemon. `BitcoinCore.initialize` locks an unlocked wallet again from a `finally`, and Python does not run `finally` blocks in daemon threads while the interpreter is shutting down, so closing the window during an import would otherwise leave that wallet open. """ - global _running - if _running: - GLib.idle_add(_busy, view, page, done) - return - _running = True + _start(view, page, work, done, claimed=False, daemon=False) + + +def poll[Result]( + view: Adw.NavigationView, + page: Adw.NavigationPage, + work: Callable[[], Result], + done: Callable[[Result | Exception], None], +) -> bool: + """Start one low-priority poll, or skip it while another job owns the gate.""" + if not _gate.acquire(blocking=False): + return False + _start(view, page, work, done, claimed=True, daemon=True) + return True + + +def _start[Result]( + view: Adw.NavigationView, + page: Adw.NavigationPage, + work: Callable[[], Result], + done: Callable[[Result | Exception], None], + *, + claimed: bool, + daemon: bool, +) -> None: def worker() -> None: outcome: Result | Exception = RuntimeError(_UNFINISHED) + if not claimed: + _gate.acquire() try: outcome = work() except (CodexError, BitcoinCoreError, OSError, TypeError, ValueError) as error: outcome = error finally: + _gate.release() GLib.idle_add(deliver, outcome) def deliver(outcome: Result | Exception) -> bool: - global _running - _running = False if showing(view, page): done(outcome) return False - threading.Thread(target=worker).start() - - -def _busy[Result]( - view: Adw.NavigationView, - page: Adw.NavigationPage, - done: Callable[[Result | Exception], None], -) -> bool: - if showing(view, page): - done(RuntimeError(_BUSY)) - return False + threading.Thread(target=worker, daemon=daemon).start() diff --git a/tests/test_gui_boundaries.py b/tests/test_gui_boundaries.py index cc7c5eb..b79044f 100644 --- a/tests/test_gui_boundaries.py +++ b/tests/test_gui_boundaries.py @@ -33,7 +33,7 @@ } ) CORE_ADAPTER = "codex32._bitcoin_core" -BUDGET = 2000 +BUDGET = 2050 def _package() -> Path: @@ -114,3 +114,24 @@ def test_the_gui_keeps_its_own_size_budget() -> None: for path in _modules() } assert sum(counts.values()) < BUDGET, counts + + +def test_read_only_poll_threads_do_not_keep_the_process_alive() -> None: + source = (_package() / "work.py").read_text() + tree = ast.parse(source) + functions = {node.name: node for node in tree.body if isinstance(node, ast.FunctionDef)} + + def daemon_argument(name: str) -> bool: + calls = [ + node + for node in ast.walk(functions[name]) + if isinstance(node, ast.Call) and isinstance(node.func, ast.Name) and node.func.id == "_start" + ] + assert len(calls) == 1 + values = {keyword.arg: keyword.value for keyword in calls[0].keywords} + value = values["daemon"] + assert isinstance(value, ast.Constant) and isinstance(value.value, bool) + return value.value + + assert daemon_argument("run") is False + assert daemon_argument("poll") is True diff --git a/tools/gui_walkthrough.py b/tools/gui_walkthrough.py index a9bc3b6..8c68ba9 100644 --- a/tools/gui_walkthrough.py +++ b/tools/gui_walkthrough.py @@ -23,6 +23,7 @@ from gi.repository import Adw, Gdk, GLib, Gtk +from codex32 import MasterSeed, parse_codex32 from codex32_gui import app, pages, reading, wallet_setup SHARE_A = "MS12NAMEA320ZYXWVUTSRQPNMLKJHGFEDCAXRPP870HKKQRM" @@ -144,6 +145,11 @@ def do_activate(self) -> None: self.completion_gate, self.preflight, self.network, + self.wallet_poll_start, + self.wallet_poll_update, + self.wallet_poll_retries, + self.wallet_poll_disappears, + self.wallet_poll_stops, self.letters, self.basis, self.second_card, @@ -485,6 +491,90 @@ def network(self) -> bool: self.view.replace([pages.home(self.view)]) return True + def wallet_poll_start(self) -> bool: + seed = parse_codex32(SECRET_S) + check("the wallet poll uses a master seed", isinstance(seed, MasterSeed), type(seed).__name__) + if not isinstance(seed, MasterSeed): + return True + self.wallet_polls = 0 + self.wallet_fail_once = False + self.wallet_include_zeta = True + self.wallet_zeta = wallet_setup.Wallet("zeta", False, False) + + def eligible(_core: Any) -> tuple[wallet_setup.Wallet, ...]: + self.wallet_polls += 1 + if self.wallet_fail_once: + self.wallet_fail_once = False + raise wallet_setup.BitcoinCoreError("wallet disappeared during refresh") + wallets = (wallet_setup.Wallet("alpha", False, False),) + return wallets + ((self.wallet_zeta,) if self.wallet_include_zeta else ()) + + wallet_setup.eligible = eligible # type: ignore[assignment] + wallet_setup.version_text = lambda _core: "32.0.0" # type: ignore[assignment] + wallet_setup.network = lambda _core: "signet" # type: ignore[assignment] + page = pages._wallet_page(self.view, _Stub(), seed, (self.wallet_zeta,), 0, False) + self.view.replace([pages.home(self.view), page]) + return True + + def wallet_poll_update(self) -> bool: + if self.wallet_polls == 0: + return False + page = self.page() + listed = rows(page) + check( + "a newly empty wallet appears without a button press", + [row.get_title() for row in listed] == ["alpha", "zeta", pages.CREATE_WALLET], + [row.get_title() for row in listed], + ) + zeta = next(row for row in listed if row.get_title() == "zeta") + check("refresh preserves the selected wallet by name", zeta.get_activatable_widget().get_active()) + check("manual wallet refresh is no longer needed", button(page, "Check again") is None) + self.wallet_fail_once = True + self.wallet_retry_poll = self.wallet_polls + return True + + def wallet_poll_retries(self) -> bool: + if self.wallet_polls == self.wallet_retry_poll: + return False + check( + "a transient refresh failure leaves the wallet chooser in place", + self.page().get_title() == "Wallet", + ) + self.wallet_include_zeta = False + self.wallet_removal_poll = self.wallet_polls + return True + + def wallet_poll_disappears(self) -> bool: + if self.wallet_polls == self.wallet_removal_poll: + return False + page = self.page() + listed = rows(page) + check( + "a selected wallet that stops being eligible disappears", + [row.get_title() for row in listed] == ["alpha", pages.CREATE_WALLET], + [row.get_title() for row in listed], + ) + check( + "a disappearing selected wallet does not select a different destination", + not any(row.get_activatable_widget().get_active() for row in listed), + ) + check( + "Continue is disabled until the operator chooses again", + not button(page, "Continue").get_sensitive(), + ) + listed[0].get_activatable_widget().set_active(True) + check("choosing again re-enables Continue", button(page, "Continue").get_sensitive()) + self.wallet_poll_count = self.wallet_polls + self.wallet_poll_left = GLib.get_monotonic_time() + self.view.replace([pages.home(self.view)]) + return True + + def wallet_poll_stops(self) -> bool: + if GLib.get_monotonic_time() - self.wallet_poll_left < 1_300_000: + return False + check("wallet polling stops after leaving the page", self.wallet_polls == self.wallet_poll_count) + return True + def letters(self) -> bool: page = self.page() if page.get_title() != "codex32":