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

Merged

sirius merged 8 commits from onionwire/t_67879b98-wallet-actually-open-the-wallet-file-and into main 2026-09-23 02:33:40 +00:00

Conversation 0 Commits 8 Files changed 7 +1122 -54

sirius commented 2026-09-23 02:30:23 +00:00

Owner

Copy link

Carries the wallet-picker fix and the wallet-manager work to main — they are on t_67879b98 but not on main, which is why they are in no release.

Why this PR exists

t_67879b98 was already merged into main (#149, merge 01d3294). #150 and #151 were then merged into that branch (merges 3ddd370, c1fda000, 685ce89), so both PRs read "merged" while main never received a byte of them. v0.19.0 is tagged on main (01d3294, statuses green), so the released APK does not contain the picker fix: create_wallet onionwire-1 -> (code -21, "Cannot create wallet. Already exists.") on a wallet whose file is on disk but absent from the catalogue is still a dead end with no UI escape on the shipped build.

Payload

• src/node.rs, src/wallet_child.rs, tests/wallet.rs — the file-name picker consults the wallet directory, not only the SQLCipher catalogue (wallet_name_taken, wallet_dir()), plus the restart-test harness fix so a child's port is read from the child that dials it (#151, absorbed #152).
• android/app/src/main/java/com/siriusdevops/onionwire/** — wallet manager: ask for the passphrase, stop printing RPC errors, filter last_error where the SDK record becomes the model (#150).

Verification

• This tree carries no CI status at all: a push to a non-main branch runs no workflow here, and #150/#151's merge commits were never built as a tree. This PR is the first build of it (ci is path-filtered on src/**, tests/**; android-ci on android/**).
• The individual commits are green on their own shas (ci / test success on 6edcd65e, run 589).
• Merge order: main only. If this merges, main moves ahead of the v0.19.0 tag, so the wallet fixes reach a release only when a new one is cut (release.yml dispatch, dry_run first, then android-release.yml with the same tag=).

Carries the wallet-picker fix and the wallet-manager work to `main` — they are on `t_67879b98` but **not** on `main`, which is why they are in no release. ## Why this PR exists `t_67879b98` was already merged into `main` (#149, merge `01d3294`). #150 and #151 were then merged *into that branch* (merges `3ddd370`, `c1fda000`, `685ce89`), so both PRs read "merged" while `main` never received a byte of them. `v0.19.0` is tagged on `main` (`01d3294`, statuses green), so the released APK does not contain the picker fix: `create_wallet onionwire-1 -> (code -21, "Cannot create wallet. Already exists.")` on a wallet whose file is on disk but absent from the catalogue is still a dead end with no UI escape on the shipped build. ## Payload - `src/node.rs`, `src/wallet_child.rs`, `tests/wallet.rs` — the file-name picker consults the wallet directory, not only the SQLCipher catalogue (`wallet_name_taken`, `wallet_dir()`), plus the restart-test harness fix so a child's port is read from the child that dials it (#151, absorbed #152). - `android/app/src/main/java/com/siriusdevops/onionwire/**` — wallet manager: ask for the passphrase, stop printing RPC errors, filter `last_error` where the SDK record becomes the model (#150). ## Verification - This tree carries **no CI status at all**: a push to a non-`main` branch runs no workflow here, and #150/#151's merge commits were never built as a tree. This PR is the first build of it (`ci` is path-filtered on `src/**`, `tests/**`; `android-ci` on `android/**`). - The individual commits are green on their own shas (`ci / test` success on `6edcd65e`, run 589). - Merge order: `main` only. If this merges, `main` moves ahead of the `v0.19.0` tag, so the wallet fixes reach a release only when a new one is cut (`release.yml` dispatch, `dry_run` first, then `android-release.yml` with the same `tag=`).

sirius added 8 commits 2026-09-23 02:30:24 +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.

wallet: the file-name picker consults the disk, not only the catalogue ... 59175f6931

next_wallet_filename returned <default>-<n> for the first n the SQLCipher
catalogue did not hold, and never looked at the wallet directory. monero
writes two files per wallet (<name> and <name>.keys) and refuses a create
that would collide with either, so a file whose catalogue row is gone -
Store::delete_wallet keeps the file, and a create whose record_wallet
failed leaves one - meant every attempt proposed the same taken name and
the daemon answered -21 Cannot create wallet. Already exists. for ever.

Add wallet_name_taken (src/wallet_child.rs): it lists the wallet dir and
answers whether <name> or <name>.keys is already there. next_wallet_filename
now needs the candidate free in both places, and Node::wallet_dir() is the
single source of $ONIONWIRE_HOME/wallet, used by the child spawn too.

Fail closed: an unreadable (or missing) wallet dir is an error, not a
guess. Reads only - nothing here deletes a file the catalogue does not
hold, which may be the only copy of a funded wallet.

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'.

tests: write the fake child's argv log with one mv, not append-per-line ... 8d80a4fd55

The wallet-child tests learn a spawned stand-in's port by polling its argv
log, and `fake_bin`/`fake_child_bin` truncated the log and then appended
one line per argument -- so a poller could read a prefix. A truncated port
is a *valid* u16 ("427" out of "42765"), so `wait_for_child_rpc_port`
happily returned a port nothing was listening on and the restart's
`open_wallet` died with `Connection refused` after its 10s budget; the
same prefix read is what `loopback_and_clearnet_remotes_carry_no_proxy...`
printed with its `--daemon-address` value missing.

Both tests failed on the first CI run of this branch (ci / test) and are
green in isolation, which is the signature of this race rather than of a
code defect. The argv now lands in the log with a single `mv`, so a reader
sees either no log or the complete one.

Measured with a tight read loop (argv-race-probe.sh): the append version
was caught mid-write 1/300 reads, the mv version 0/300.

tests: take the restart's port from the child that dials it ... 6edcd65e96

`a_restart_opens_the_stored_wallet_file` failed on the Pi runner twice in a
row — CI run 905 job 1373 and run 901 job 1365 — and passed 3/3 on a 16-core
host:

  open the wallet with a passphrase: "wallet file onionwire-1 is not open —
  the wallet RPC never answered (wallet connect: Connection refused (os error
  111))"

The whole `WALLET_OPEN_WAIT` was burned on refusals, so the mock was bound on
a port the restarted child never dialled. Production is not the suspect:
`WalletChild::start_children` builds the child's argv and the `WalletClient`
URL from the same `pick_port` value, and `start_wallet_child_with` installs
that client in `self.wallet` before `open_child_wallet` dials it, so the client
can never carry the previous child's port.

The harness is the suspect. It learned the restarted child's port from the
*shared* argv log — a file every child writes — and cleared that file first.
A clear is not a synchronisation: on a loaded runner the killed child's write
lands after it, so the poller read the port of a child that was already dead
and bound the mock there. Reproduced deterministically with a stand-in whose
exec is late; with the old read the new test fails with the CI message, byte
for byte.

Each fake child now writes `<log>.<pid>` as well, and the shared copy lands
first, so a pid-named file implies the shared one is complete. A test that
spans two children reads the pid-named file: only that child can write it, so
the port belongs to the child that dials it. `WalletChild::wallet_rpc_pid`
(test-only) supplies the pid.

`open_wallet_blocking` already retries while the RPC is unreachable and gives
up after a bounded wait with a typed `OpenFailure::NotAnswering`, so it cannot
refuse forever against a merely slow child: no production change.

new test: node::tests::a_restart_is_not_given_the_stopped_childs_port

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 ... 3ddd370b3d

Reviewed-on: #150

Merge branch 'onionwire/t_67879b98-wallet-actually-open-the-wallet-file-and' into onionwire/t_81deabb5-wallet-the-file-name-picker-must-consult c1fda00080

Merge pull request 'wallet: the file-name picker must consult the disk, not only the catalogue' (#151) from onionwire/t_81deabb5-wallet-the-file-name-picker-must-consult into onionwire/t_67879b98-wallet-actually-open-the-wallet-file-and ... 685ce89b29

Reviewed-on: #151

sirius merged commit 25e4c67181 into main 2026-09-23 02:33:40 +00:00

sirius referenced this pull request from a commit 2026-09-23 02:33:40 +00:00

Merge pull request 'wallet: land the file-name picker fix and the wallet manager on main (from t_67879b98)' (#153) from onionwire/t_67879b98-wallet-actually-open-the-wallet-file-and into main

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!153

No description provided.