fix(wallet): surface silent save failures + stale-state cleanups (W5-1, W5-2, W6-1, W6-3)
P2 robustness batch (localized): - W5-1 (Med, lite): persistAfterBroadcast returned false on a persistent post-send/shield save failure, but both callers discarded it and it never logged — completely silent. It now liteLogs the failure (the spent note re-derives on the next sync, so it's a robustness gap, not fund loss). - W5-2 (Med, lite): the post-sync and post-rescan save results (in the detached scan threads) were ignored; both now liteLog on failure. LiteDiagnostics::log is mutex-guarded, so it's safe from those threads. - W6-1 (Med): WalletState::clear() didn't reset mining/pool_mining, so a wallet switch could briefly show the previous wallet's hashrate/blocks. Now reset in clear() (the daemon restarts on switch, so mining genuinely stops). - W6-3 (Low): AddressBook::load() cleared entries_ then threw on the first non-object array element — discarding EVERY contact. It now guards is_object() + per-entry try/catch, skipping and counting malformed entries. Build-clean; ctest 1/1. Remaining P2: W6-2 (refresh-staleness badge — needs UI, overlaps the diagnostics Foundation bundle). See docs/wallet-hardening.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -20,7 +20,7 @@ Status legend: ☐ not started · ◐ in progress · ☑ landed & verified
|
||||
| **P0-B** | W2-2/W4-2, W2-4 | Encryption integrity (never silently unencrypted) | ☑ |
|
||||
| **P1-A** | W3-1, W3-2, W3-4 ✓ · W3-3 ⚑ | Migrate-to-seed correctness (fund-adjacent) | ◐ 3/4 |
|
||||
| **P1-B** | W1-1, W1-2, W1-3, W1-4 ✓ + startup guard | Missing/wrong wallet-file safety | ☑ |
|
||||
| **P2** | W6-2, W5-1, W5-2, W6-1, W6-3 | Stale state & lite save-failure surfacing | ☐ |
|
||||
| **P2** | W5-1, W5-2, W6-1, W6-3 ✓ · W6-2 ☐ | Stale state & lite save-failure surfacing | ◐ 4/5 |
|
||||
| **F** | W7-2, W7-3, W7-4, QoL | Diagnostics foundation + QoL bundle | ☐ |
|
||||
|
||||
---
|
||||
@@ -112,6 +112,12 @@ Land W7-2 first — it unblocks the rest.
|
||||
|
||||
## Progress log
|
||||
|
||||
- **P2 / W5-1 · W5-2 · W6-1 · W6-3 (localized batch)** — ☑ landed:
|
||||
- **W5-1 (Med):** `persistAfterBroadcast` (lite send/shield save) returned false on a persistent save failure but both callers discarded it and it never logged — completely silent. It now `liteLog`s the failure (the note re-derives on next sync, so it's a robustness gap, not fund loss).
|
||||
- **W5-2 (Med):** the post-**sync** and post-**rescan** `save` results (in the detached scan threads) were ignored; both now `liteLog` on failure (`LiteDiagnostics::log` is mutex-guarded, safe from those threads).
|
||||
- **W6-1 (Med):** `WalletState::clear()` didn't reset `mining`/`pool_mining`, so a wallet switch could briefly show the previous wallet's hashrate/blocks. Now reset in `clear()` (the daemon restarts on switch, so mining genuinely stops).
|
||||
- **W6-3 (Low):** `AddressBook::load()` did `entries_.clear()` then threw on the first non-object element — discarding **every** contact. Now it guards `is_object()` + per-entry try/catch, skipping and counting malformed entries.
|
||||
Build-clean; `ctest` 1/1. **Remaining P2:** W6-2 (surface refresh staleness — the timestamps exist in `WalletState`; this needs the UI "updated Xs ago" badge, which overlaps the diagnostics/QoL Foundation bundle).
|
||||
- **P1-B / W1-3 + startup wallet-existence guard** — ☑ landed:
|
||||
- **W1-3 (Med):** `syncedHere` was stamped in the `markOpened` block at bare connect (idHash still empty), letting a freshly-restored wallet skip its needed rescan. It's now stamped only once the identity is verified (idHash non-empty), so it takes effect at the post-address-refresh index update (`updateWalletIndexForActiveWallet` after addresses load), while `lastOpenedEpoch` still records at open.
|
||||
- **Startup guard (the W1-1 launch counterpart):** `App::init` now `exists()`-checks the recorded active wallet before the daemon is configured; a **non-default** active wallet that was moved/deleted between sessions falls back to the default `wallet.dat` with a warning, instead of the daemon silently auto-creating an empty wallet under the missing name. Runs before the PIN-vault init so the vault is scoped to the wallet actually opened.
|
||||
|
||||
@@ -46,20 +46,25 @@ bool AddressBook::load()
|
||||
entries_.clear();
|
||||
|
||||
if (j.contains("entries") && j["entries"].is_array()) {
|
||||
size_t skipped = 0;
|
||||
for (const auto& entry : j["entries"]) {
|
||||
AddressBookEntry e;
|
||||
e.label = entry.value("label", "");
|
||||
e.address = entry.value("address", "");
|
||||
e.notes = entry.value("notes", "");
|
||||
// Legacy entries (no "scope") migrate to "global" so nothing disappears when
|
||||
// multi-wallet scoping lands — a contact you already had stays visible everywhere.
|
||||
e.scope = entry.value("scope", "global");
|
||||
e.avatar = entry.value("avatar", "");
|
||||
|
||||
if (!e.address.empty()) {
|
||||
entries_.push_back(e);
|
||||
}
|
||||
// W6-3: skip (and count) a malformed element rather than letting one bad entry throw and
|
||||
// abort the whole load — which would discard EVERY contact (entries_ was already cleared).
|
||||
if (!entry.is_object()) { ++skipped; continue; }
|
||||
try {
|
||||
AddressBookEntry e;
|
||||
e.label = entry.value("label", "");
|
||||
e.address = entry.value("address", "");
|
||||
e.notes = entry.value("notes", "");
|
||||
// Legacy entries (no "scope") migrate to "global" so nothing disappears when
|
||||
// multi-wallet scoping lands — a contact you already had stays visible everywhere.
|
||||
e.scope = entry.value("scope", "global");
|
||||
e.avatar = entry.value("avatar", "");
|
||||
if (!e.address.empty()) entries_.push_back(e);
|
||||
} catch (const std::exception&) { ++skipped; }
|
||||
}
|
||||
if (skipped > 0)
|
||||
DEBUG_LOGF("Address book: skipped %zu malformed entr%s\n", skipped, skipped == 1 ? "y" : "ies");
|
||||
}
|
||||
|
||||
DEBUG_LOGF("Address book loaded: %zu entries\n", entries_.size());
|
||||
|
||||
@@ -335,6 +335,10 @@ struct WalletState {
|
||||
transactions.clear();
|
||||
peers.clear();
|
||||
bannedPeers.clear();
|
||||
// W6-1: reset node-level mining state too — the daemon restarts on a wallet switch (mining
|
||||
// stops), so leaving the previous wallet's hashrate/blocks would show stale mining stats.
|
||||
mining = MiningInfo{};
|
||||
pool_mining = PoolMiningState{};
|
||||
}
|
||||
|
||||
// Rebuild combined addresses list from z/t lists
|
||||
|
||||
@@ -80,8 +80,10 @@ bool persistAfterBroadcast(LiteClientBridge& bridge)
|
||||
for (int attempt = 0; attempt < 2; ++attempt) {
|
||||
if (bridge.execute("save", "").ok) return true;
|
||||
}
|
||||
// Persistent failure: the spent note will be re-derived from the chain on the next sync,
|
||||
// so this is a robustness gap, not fund loss. (Retry handles the common transient case.)
|
||||
// Persistent failure: the spent note will be re-derived from the chain on the next sync, so this
|
||||
// is a robustness gap, not fund loss. Log it (W5-1) — both callers discard this return, so the
|
||||
// failure was previously completely silent.
|
||||
liteLog("save failed after send/shield — the wallet will re-derive it on the next sync");
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -600,7 +602,8 @@ void LiteWalletController::startSync()
|
||||
// The backend does NOT auto-save after a sync, so persist the freshly-scanned wallet;
|
||||
// otherwise the next launch re-scans from the checkpoint (~30 min). Set `done` only
|
||||
// after the save so a syncComplete() observer sees a fully-persisted wallet.
|
||||
bridge->execute("save", "");
|
||||
if (!bridge->execute("save", "").ok) // W5-2: don't leave a failed post-sync save silent
|
||||
liteLog("save failed after sync — the next launch will re-scan from the checkpoint");
|
||||
}
|
||||
done->store(true);
|
||||
});
|
||||
@@ -631,7 +634,8 @@ bool LiteWalletController::startRescan()
|
||||
// `rescan` clears the wallet's synced block cache and re-downloads/re-scans from the
|
||||
// birthday height — a blocking, uninterruptible full scan, same as `sync`.
|
||||
bridge->execute("rescan", "");
|
||||
bridge->execute("save", ""); // backend doesn't auto-save after a rescan
|
||||
if (!bridge->execute("save", "").ok) // W5-2: don't leave a failed post-rescan save silent
|
||||
liteLog("save failed after rescan — the next launch will re-scan from the checkpoint");
|
||||
}
|
||||
done->store(true);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user