android: ask for the wallet passphrase, and stop printing RPC errors #150

Merged

sirius merged 2 commits from onionwire/t_9ef8561c-wallet-manager-ask-for-the-passphrase-an into onionwire/t_67879b98-wallet-actually-open-the-wallet-file-and 2026-09-23 00:41:47 +00:00

Conversation 1 Commits 2 Files changed 4 +721 -21

sirius commented 2026-09-22 17:29:11 +00:00

Owner

Copy link

Stacked on #149 (onionwire/t_67879b98-wallet-actually-open-the-wallet-file-and, still open at the time of writing: git rev-list --count origin/main..origin/onionwire/t_67879b98-... = 1). base is that branch, not main — the passphrase-taking open does not exist on main.

The bug

Tapping a saved wallet looked like it did nothing. The child was restarted onto the file with no wallet open, so every read answered the daemon's -13 No wallet file, and the screen printed that string at the user. The parent card (#149) makes the core actually open the file and seal the passphrase; this is the phone side for a wallet that predates it, which has no stored passphrase and needs one typed once.

Correction in b4f8bb8 (second pass)

An earlier revision of this PR claimed the failure filter ran "at every site that renders a wallet failure in both wallet screens". That was wrong, and the reviewer reproduced it. The filter sat on WalletChildCard's sticky error line, but the same field of the same record is rendered 185 lines above: WalletScreen.kt's toInput() copied the SDK's lastError verbatim, Text(walletChildLine(input)) printed it (WalletChildStatusText.kt:293-320 appends it; shortSendReason only cuts at ": "), and walletChildStatusDescription put it in the screen-reader sentence too. In exactly the state this card exists for — a live child with no wallet file open — the user was shown No wallet file.

Fixed at the producer instead of the render sites: WalletChildStatus.toInput() now runs lastError through walletFailureText — the one seam where the SDK record becomes the Kotlin model, upstream of WalletChildStatusText.kt (outside this card's file list). It is idempotent on its own replacement texts, so the error card's second pass is stable.

What changed

ui/WalletManagerScreen.kt

• A WalletPassphraseCard is shown when the child reports needs_passphrase and no wallet is open. Masked field (PasswordVisualTransformation), no copy action at all — same rule and same reason as SeedBackupCard. Submit calls the parent's Wire::open_wallet_with_passphrase on the file active_wallet_filename() already reports, so there is no second idea of which wallet is open. It is wiped on success and in onDispose, and the card is not rememberSaveable.
• FLAG_SECURE is set while the composition is up and cleared when it goes. Disclosure: no screen in this app set that flag before this change (grep -rn FLAG_SECURE android/ was empty on the parent tip), so "like the other key-bearing screens" did not hold as written. I added it here because this screen renders a spend key and asks for the passphrase that opens a wallet file; the same gap exists on the wallet screen's seed card, and a follow-up card should decide it app-wide rather than me doing it here.
• walletFailureText(raw) — the display filter. The wallet client keeps only error.message out of the daemon's JSON, so No wallet file is what actually arrives; a failure that never got that far can still arrive as the envelope. Both are replaced with a plain sentence. A bare -13 is matched with lookarounds (-135 is not a code). The core's own refusals (no saved wallet in onionwire-9, node stopped: …, wallet file X is not open — the RPC refused its passphrase (…)) pass through verbatim — they are written for a person and carry the reason. Over-matching only downgrades to a plainer sentence, so the filter fails closed.
• Applied to listError, actionError, keysError and the passphrase error here, and to error / daemonError in the wallet screen's action paths.

ui/WalletScreen.kt

• toInput() filters lastError. This is the fix for the leak above: the status line and its screen-reader sentence are both built from this one WalletChildInput, so nothing the SDK reports can reach either unfiltered. The sticky error line's existing walletFailureText(input?.lastError) stays as the second pass and is stable under it.
• Same filter on the other wallet failure lines (error, daemonError, the wallet-child error flow).
• New line in the child card when the child is running with no wallet file open (walletChildOpenHint): it says whether a wallet is open and where to fix it — "Open it on the Saved wallets screen, where the prompt is" when it needs a passphrase, otherwise "open a saved wallet… or create or import one here". The two walletOpen / needsPassphrase flags are read off the same WalletChildStatus the status line is drawn from; no new state.

WireViewModel.kt

• openWalletWithPassphrase(filename, passphrase). One live refreshWalletChild() probe on success — a one-shot read, not a second poller; the wallet screen still owns the only loop.
• The wallet-child error flow (readWalletChild, startWalletChild, stopWalletChild) now stores the filtered text.

Tests

WalletManagerTest (JVM): 31 new tests, 46 total in the file. Covers the new decision — needs_passphrase && !walletOpen → prompt, open wallet → no prompt (an open wallet wins over a stale flag) — plus the submit gate and its two distinct reasons, the failure-text filter, the open hint, and — new in this pass, the point of the correction — the render path itself: the tests build the real WalletChildStatus record with lastError = "No wallet file", run it through toInput() and assert on walletChildLine(...) / walletChildStatusDescription(...), not on walletFailureText in isolation. Assertions are on pure functions, not on composition.

Mutation-checked: reverting lastError = walletFailureText(lastError) to lastError = lastError fails exactly three of them (the_status_line_never_renders_the_daemons_own_words, a_raw_rpc_envelope_on_the_status_line_is_replaced_too, filtering_the_sdk_record_twice_changes_nothing) and 209 tests completed, 3 failed. With the fix in place the same probe the reviewer ran prints:

INPUT.lastError: [No wallet is open in the wallet child yet. Open one from the list below — …]
LINE: [connecting · remote daemon — No wallet is open in th…]
DESC: [The wallet child is up; its wallet RPC has not answered yet. connecting · remote daemon — No wallet is open in th…]
line contains 'No wallet file': false
desc contains 'No wallet file': false

Full suite on this machine:

rm -rf app/build/test-results/testDebugUnitTest
./gradlew :app:testDebugUnitTest --no-build-cache -x :sdk:cargoBuildAndroid \
  -x :sdk:cargoHostLib -x :sdk:uniffiBindgen -x :sdk:verifyDynamicLibc
209 tests, 0 failures, 0 errors, 14 classes   (XML 2026-09-22T18:07:33Z, run at 18:07:44Z)

No instrumentation or device test is claimed: there is no emulator or device on this machine. A wallet created before #149 opening with a typed passphrase, and the balance / address / seed / keys working afterwards, is NOT verified on a device by this PR. That is the acceptance criterion a reviewer with a device has to run: install, create a wallet (or take one from before), restart, tap it in Saved wallets, type the passphrase, then check Recovery phrase and Private keys on the same screen.

Toolchain check on this machine (not a substitute for CI):

./gradlew :app:assembleDebug
BUILD SUCCESSFUL in 3m 9s
app-debug.apk  40,975,654 bytes  sha256 cc41c2a2c701fae451701867cd2104da1fc56ba3afb01500fa4c3a0c1bffaee0
lib/arm64-v8a/libonionwire_sdk.so  55,217,048 bytes

Scope

git diff --stat against ed29131 (the parent tip this is based on) — only the four allowed files:

 android/app/src/main/java/com/siriusdevops/onionwire/WireViewModel.kt        |  37 ++-
 android/app/src/main/java/com/siriusdevops/onionwire/ui/WalletManagerScreen.kt | 359 ++++++++++-
 android/app/src/main/java/com/siriusdevops/onionwire/ui/WalletScreen.kt      |  66 ++-
 android/app/src/test/java/com/siriusdevops/onionwire/ui/WalletManagerTest.kt | 280 +++++++++
 4 files changed, 721 insertions(+), 21 deletions(-)

No src/**, no crates/**, no manifest, no *.gradle.kts, no new permission, no build-file change, no new dependency. The UniFFI bindings were regenerated locally only to check the surface: Wire.openWalletWithPassphrase(filename, passphrase) and WalletChildStatus.needsPassphrase / .walletOpen are what the parent exposes, and they are what this uses.

No release, no tag.

Stacked on #149 (`onionwire/t_67879b98-wallet-actually-open-the-wallet-file-and`, still open at the time of writing: `git rev-list --count origin/main..origin/onionwire/t_67879b98-...` = 1). **`base` is that branch, not `main`** — the passphrase-taking open does not exist on `main`. ## The bug Tapping a saved wallet looked like it did nothing. The child was restarted onto the file with no wallet open, so every read answered the daemon's `-13 No wallet file`, and the screen printed that string at the user. The parent card (#149) makes the core actually open the file and seal the passphrase; this is the phone side for a wallet that predates it, which has no stored passphrase and needs one typed once. ## Correction in `b4f8bb8` (second pass) An earlier revision of this PR claimed the failure filter ran "at **every** site that renders a wallet failure in both wallet screens". That was wrong, and the reviewer reproduced it. The filter sat on `WalletChildCard`'s sticky error line, but the *same field of the same record* is rendered 185 lines above: `WalletScreen.kt`'s `toInput()` copied the SDK's `lastError` verbatim, `Text(walletChildLine(input))` printed it (`WalletChildStatusText.kt:293-320` appends it; `shortSendReason` only cuts at `": "`), and `walletChildStatusDescription` put it in the screen-reader sentence too. In exactly the state this card exists for — a live child with no wallet file open — the user was shown `No wallet file`. Fixed at the producer instead of the render sites: **`WalletChildStatus.toInput()` now runs `lastError` through `walletFailureText`** — the one seam where the SDK record becomes the Kotlin model, upstream of `WalletChildStatusText.kt` (outside this card's file list). It is idempotent on its own replacement texts, so the error card's second pass is stable. ## What changed **`ui/WalletManagerScreen.kt`** - A `WalletPassphraseCard` is shown when the child reports `needs_passphrase` and no wallet is open. Masked field (`PasswordVisualTransformation`), **no copy action at all** — same rule and same reason as `SeedBackupCard`. Submit calls the parent's `Wire::open_wallet_with_passphrase` on the file `active_wallet_filename()` already reports, so there is no second idea of which wallet is open. It is wiped on success and in `onDispose`, and the card is not `rememberSaveable`. - `FLAG_SECURE` is set while the composition is up and cleared when it goes. **Disclosure: no screen in this app set that flag before this change** (`grep -rn FLAG_SECURE android/` was empty on the parent tip), so "like the other key-bearing screens" did not hold as written. I added it here because this screen renders a spend key and asks for the passphrase that opens a wallet file; the same gap exists on the wallet screen's seed card, and a follow-up card should decide it app-wide rather than me doing it here. - `walletFailureText(raw)` — the display filter. The wallet client keeps only `error.message` out of the daemon's JSON, so `No wallet file` is what actually arrives; a failure that never got that far can still arrive as the envelope. Both are replaced with a plain sentence. A bare `-13` is matched with lookarounds (`-135` is not a code). The core's own refusals (`no saved wallet in onionwire-9`, `node stopped: …`, `wallet file X is not open — the RPC refused its passphrase (…)`) pass through verbatim — they are written for a person and carry the reason. Over-matching only downgrades to a plainer sentence, so the filter fails closed. - Applied to `listError`, `actionError`, `keysError` and the passphrase error here, and to `error` / `daemonError` in the wallet screen's action paths. **`ui/WalletScreen.kt`** - **`toInput()` filters `lastError`.** This is the fix for the leak above: the status line and its screen-reader sentence are both built from this one `WalletChildInput`, so nothing the SDK reports can reach either unfiltered. The sticky error line's existing `walletFailureText(input?.lastError)` stays as the second pass and is stable under it. - Same filter on the other wallet failure lines (`error`, `daemonError`, the wallet-child error flow). - New line in the child card when the child is **running with no wallet file open** (`walletChildOpenHint`): it says whether a wallet is open and where to fix it — "Open it on the Saved wallets screen, where the prompt is" when it needs a passphrase, otherwise "open a saved wallet… or create or import one here". The two `walletOpen` / `needsPassphrase` flags are read off the same `WalletChildStatus` the status line is drawn from; no new state. **`WireViewModel.kt`** - `openWalletWithPassphrase(filename, passphrase)`. One live `refreshWalletChild()` probe on success — a one-shot read, not a second poller; the wallet screen still owns the only loop. - The wallet-child error flow (`readWalletChild`, `startWalletChild`, `stopWalletChild`) now stores the filtered text. ## Tests `WalletManagerTest` (JVM): **31 new tests, 46 total in the file.** Covers the new decision — `needs_passphrase && !walletOpen` → prompt, open wallet → no prompt (an open wallet wins over a stale flag) — plus the submit gate and its two distinct reasons, the failure-text filter, the open hint, and — new in this pass, the point of the correction — **the render path itself**: the tests build the real `WalletChildStatus` record with `lastError = "No wallet file"`, run it through `toInput()` and assert on `walletChildLine(...)` / `walletChildStatusDescription(...)`, not on `walletFailureText` in isolation. Assertions are on pure functions, not on composition. Mutation-checked: reverting `lastError = walletFailureText(lastError)` to `lastError = lastError` fails exactly three of them (`the_status_line_never_renders_the_daemons_own_words`, `a_raw_rpc_envelope_on_the_status_line_is_replaced_too`, `filtering_the_sdk_record_twice_changes_nothing`) and `209 tests completed, 3 failed`. With the fix in place the same probe the reviewer ran prints: ``` INPUT.lastError: [No wallet is open in the wallet child yet. Open one from the list below — …] LINE: [connecting · remote daemon — No wallet is open in th…] DESC: [The wallet child is up; its wallet RPC has not answered yet. connecting · remote daemon — No wallet is open in th…] line contains 'No wallet file': false desc contains 'No wallet file': false ``` Full suite on this machine: ``` rm -rf app/build/test-results/testDebugUnitTest ./gradlew :app:testDebugUnitTest --no-build-cache -x :sdk:cargoBuildAndroid \ -x :sdk:cargoHostLib -x :sdk:uniffiBindgen -x :sdk:verifyDynamicLibc 209 tests, 0 failures, 0 errors, 14 classes (XML 2026-09-22T18:07:33Z, run at 18:07:44Z) ``` No instrumentation or device test is claimed: there is no emulator or device on this machine. **A wallet created before #149 opening with a typed passphrase, and the balance / address / seed / keys working afterwards, is NOT verified on a device by this PR.** That is the acceptance criterion a reviewer with a device has to run: install, create a wallet (or take one from before), restart, tap it in Saved wallets, type the passphrase, then check Recovery phrase and Private keys on the same screen. Toolchain check on this machine (not a substitute for CI): ``` ./gradlew :app:assembleDebug BUILD SUCCESSFUL in 3m 9s app-debug.apk 40,975,654 bytes sha256 cc41c2a2c701fae451701867cd2104da1fc56ba3afb01500fa4c3a0c1bffaee0 lib/arm64-v8a/libonionwire_sdk.so 55,217,048 bytes ``` ## Scope `git diff --stat` against `ed29131` (the parent tip this is based on) — only the four allowed files: ``` android/app/src/main/java/com/siriusdevops/onionwire/WireViewModel.kt | 37 ++- android/app/src/main/java/com/siriusdevops/onionwire/ui/WalletManagerScreen.kt | 359 ++++++++++- android/app/src/main/java/com/siriusdevops/onionwire/ui/WalletScreen.kt | 66 ++- android/app/src/test/java/com/siriusdevops/onionwire/ui/WalletManagerTest.kt | 280 +++++++++ 4 files changed, 721 insertions(+), 21 deletions(-) ``` No `src/**`, no `crates/**`, no manifest, no `*.gradle.kts`, no new permission, no build-file change, no new dependency. The UniFFI bindings were regenerated locally only to check the surface: `Wire.openWalletWithPassphrase(filename, passphrase)` and `WalletChildStatus.needsPassphrase` / `.walletOpen` are what the parent exposes, and they are what this uses. No release, no tag.

sirius added 1 commit 2026-09-22 17:29:11 +00:00

android: ask for the wallet passphrase, and stop printing RPC errors ... 3f84021abb

The wallet manager could not open a wallet it holds no passphrase for: the
child came up pointed at the file with no wallet open, `needs_passphrase`
was true, and every read answered the daemon's `-13 No wallet file`. The
screen rendered that string.

Phone side of t_67879b98:

* WalletManagerScreen asks for the passphrase when the child reports
  `needs_passphrase` and no wallet is open, and submits it through the
  parent's `Wire::open_wallet_with_passphrase` on the file the child is
  already pointed at. Masked field, no copy action (same rule as the seed
  card), wiped on success and on dispose, FLAG_SECURE while the screen is
  up.
* `walletFailureText` filters every user-visible wallet failure: the
  daemon's `No wallet file`, a bare `-13`, and any JSON envelope become a
  plain sentence that says whether a wallet is open and what to do next.
  Applied at every site that renders a wallet error, in both wallet
  screens and the wallet-child error flow in the ViewModel.
* The wallet screen says "no wallet is open in the child" and where to fix
  it, instead of leaving it to surface later as the daemon's own words.

Tests: WalletManagerTest covers the new decisions (prompt shown / not
shown, submit gate and its reasons, the failure-text filter, the open
hint) on the JVM. 203 unit tests green.

sirius added 1 commit 2026-09-22 18:09:52 +00:00

android: filter the wallet child's last_error where the SDK record becomes the model ... b4f8bb809c

The status line renders WalletChildInput.lastError through walletChildLine,
and the same field feeds walletChildStatusDescription for the screen reader.
The filter added in 3f84021 sat on the sticky error card only, so in exactly
the state this card targets — a live child with no wallet file open answering
-13 No wallet file — the raw daemon sentence still reached the line and the
screen-reader sentence.

Move it to the producer: WalletChildStatus.toInput() now runs lastError
through walletFailureText, the one seam where the SDK record becomes the
Kotlin model. Idempotent on its own replacement texts, so the error card's
second pass is stable.

New JVM tests build the SDK record and assert on the rendered line (and its
description), not on walletFailureText in isolation: with the filter removed
they fail on the raw 'No wallet file'.

sirius commented 2026-09-22 18:23:50 +00:00

Author

Owner

Copy link

Second pass: the status-line leak (b4f8bb8)

Fixed the defect the review reproduced. The filter now runs at the producer, WalletChildStatus.toInput() in WalletScreen.kt — the one seam where the SDK record becomes the Kotlin model, upstream of WalletChildStatusText.kt, which is outside this card's file list. walletFailureText is idempotent on its own replacement texts, so the sticky error card's existing pass is stable.

Runtime probe, the same one the review ran, on the fixed build:

INPUT.lastError: [No wallet is open in the wallet child yet. Open one from the list below — …]
LINE: [connecting · remote daemon — No wallet is open in th…]
DESC: [The wallet child is up; its wallet RPC has not answered yet. connecting · remote daemon — No wallet is open in th…]
line contains 'No wallet file': false
desc contains 'No wallet file': false

Six new JVM tests in WalletManagerTest build the real WalletChildStatus record and assert on walletChildLine(...) / walletChildStatusDescription(...) — the render path, not the filter in isolation. Mutation check: reverting lastError = walletFailureText(lastError) to lastError = lastError fails three of them (209 tests completed, 3 failed); with the fix the full suite is 209 tests, 0 failures, 0 errors (XML 2026-09-22T18:07:33Z, run at 18:07:44Z).

The PR body's earlier claim that the filter ran at every render site in both wallet screens was wrong and has been corrected.

CI: android-ci run 585 on b4f8bb8, success — HEAD is now at b4f8bb8, BUILD SUCCESSFUL in 10m 1s, dist/onionwire-0.1.0-android-arm64-v8a.apk 34,276,918 bytes sha256 08b07f16ff059860a5b4372989f1d6dbb79e2bb2a70d3daff990a751c399017e, apksigner verify OK.

## Second pass: the status-line leak (b4f8bb8) Fixed the defect the review reproduced. The filter now runs at the producer, `WalletChildStatus.toInput()` in `WalletScreen.kt` — the one seam where the SDK record becomes the Kotlin model, upstream of `WalletChildStatusText.kt`, which is outside this card's file list. `walletFailureText` is idempotent on its own replacement texts, so the sticky error card's existing pass is stable. Runtime probe, the same one the review ran, on the fixed build: ``` INPUT.lastError: [No wallet is open in the wallet child yet. Open one from the list below — …] LINE: [connecting · remote daemon — No wallet is open in th…] DESC: [The wallet child is up; its wallet RPC has not answered yet. connecting · remote daemon — No wallet is open in th…] line contains 'No wallet file': false desc contains 'No wallet file': false ``` Six new JVM tests in `WalletManagerTest` build the real `WalletChildStatus` record and assert on `walletChildLine(...)` / `walletChildStatusDescription(...)` — the render path, not the filter in isolation. Mutation check: reverting `lastError = walletFailureText(lastError)` to `lastError = lastError` fails three of them (`209 tests completed, 3 failed`); with the fix the full suite is `209 tests, 0 failures, 0 errors` (XML 2026-09-22T18:07:33Z, run at 18:07:44Z). The PR body's earlier claim that the filter ran at every render site in both wallet screens was wrong and has been corrected. CI: `android-ci` run 585 on b4f8bb8, success — `HEAD is now at b4f8bb8`, `BUILD SUCCESSFUL in 10m 1s`, `dist/onionwire-0.1.0-android-arm64-v8a.apk` 34,276,918 bytes sha256 `08b07f16ff059860a5b4372989f1d6dbb79e2bb2a70d3daff990a751c399017e`, `apksigner` verify OK.

sirius merged commit 3ddd370b3d into onionwire/t_67879b98-wallet-actually-open-the-wallet-file-and 2026-09-23 00:41:47 +00:00

sirius referenced this pull request from a commit 2026-09-23 00:41:49 +00:00

Merge pull request 'android: ask for the wallet passphrase, and stop printing RPC errors' (#150) from onionwire/t_9ef8561c-wallet-manager-ask-for-the-passphrase-an into onionwire/t_67879b98-wallet-actually-open-the-wallet-file-and

sirius referenced this pull request 2026-09-23 02:30:23 +00:00

wallet: land the file-name picker fix and the wallet manager on main (from t_67879b98) #153

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

android

ANDROID

  

apk-dev

APK-DEV

  

monero-rpc

MONERO-RPC

  

wallet

WALLET

No labels

android

apk-dev

monero-rpc

wallet

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

sirius/onionwire!150

No description provided.