diff --git a/CMakeLists.txt b/CMakeLists.txt index dfe0d6e..aaca831 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -981,7 +981,7 @@ if(BUILD_TESTING) src/wallet/lite_bridge_runtime.cpp src/wallet/lite_client_bridge.cpp src/wallet/lite_connection_service.cpp - src/wallet/lite_wallet_controller.cpp + src/wallet/lite_wallet_controller.cpp src/wallet/lite_result_parsers.cpp src/wallet/lite_sync_service.cpp src/wallet/lite_wallet_gateway.cpp diff --git a/src/ui/pages/settings_page.cpp b/src/ui/pages/settings_page.cpp index cae6222..40ca07f 100644 --- a/src/ui/pages/settings_page.cpp +++ b/src/ui/pages/settings_page.cpp @@ -236,6 +236,24 @@ static void evaluateLiteLifecycleRequestFromPageState(App* app) { break; } + // Wipe ALL secret material when leaving this function, on every path (real execution, + // validation-only fallback, or early return): the UI char buffers AND the std::string + // copies inside `input.request.*Request`. The controller wipes its own by-value request + // copy, but these page-local copies are separate; leaving them would defeat the wipe. + struct LiteSecretScrubber { + wallet::LiteWalletLifecycleUiExecutionInput& in; + ~LiteSecretScrubber() { + sodium_memzero(s_settingsState.lite_lifecycle_passphrase, + sizeof(s_settingsState.lite_lifecycle_passphrase)); + sodium_memzero(s_settingsState.lite_restore_seed, + sizeof(s_settingsState.lite_restore_seed)); + wallet::secureWipeLiteSecret(in.request.createRequest.passphrase); + wallet::secureWipeLiteSecret(in.request.openRequest.passphrase); + wallet::secureWipeLiteSecret(in.request.restoreRequest.seedPhrase); + wallet::secureWipeLiteSecret(in.request.restoreRequest.passphrase); + } + } liteSecretScrubber{input}; + // When a linked lite backend is present, execute the operation for real through the // App-owned controller. Otherwise fall back to the validation-only adapter. if (auto* lite = app->liteWallet()) { @@ -252,11 +270,7 @@ static void evaluateLiteLifecycleRequestFromPageState(App* app) { break; } - // Secrets have been consumed; wipe the UI buffers (and the request copies) so they - // do not linger in memory after the attempt. - sodium_memzero(s_settingsState.lite_lifecycle_passphrase, sizeof(s_settingsState.lite_lifecycle_passphrase)); - sodium_memzero(s_settingsState.lite_restore_seed, sizeof(s_settingsState.lite_restore_seed)); - + // (Secret wiping is handled unconditionally by liteSecretScrubber at function exit.) s_settingsState.lite_lifecycle_summary = result.bridgeResponseRedacted; if (result.walletReady) { s_settingsState.lite_lifecycle_status = "Wallet ready"; diff --git a/src/wallet/lite_wallet_controller.cpp b/src/wallet/lite_wallet_controller.cpp index 71d5e4f..9148067 100644 --- a/src/wallet/lite_wallet_controller.cpp +++ b/src/wallet/lite_wallet_controller.cpp @@ -41,11 +41,13 @@ void applyLiteRefreshModelToWalletState(const LiteWalletAppRefreshModel& model, if (model.hasAddresses) { // Per-address balances from unspent notes/utxos (when the notes command succeeded). - // Sum the value of outputs at each address that are neither spent nor unconfirmed-spent. + // Sum the value of confirmed, unspent outputs at each address: skip anything spent, + // unconfirmed-spent, or still pending (notes/utxos from an unconfirmed received tx) so + // the per-address figures match the confirmed balance the wallet treats as available. std::unordered_map perAddressZatoshis; if (model.hasSpendableOutputs) { for (const auto& output : model.spendableOutputs) { - if (output.spent || output.unconfirmedSpent) continue; + if (output.spent || output.unconfirmedSpent || output.pending) continue; perAddressZatoshis[output.address] += output.valueZatoshis; } } @@ -155,8 +157,7 @@ void LiteWalletController::onLifecycleResult(const LiteWalletLifecycleResult& re void LiteWalletController::startSync() { - if (syncLaunched_) return; - syncLaunched_ = true; + if (syncLaunched_.exchange(true)) return; syncStarted_ = true; // The backend `sync` command is a blocking, uninterruptible full chain scan, so run it on // a detached thread. Capture shared refs (not the controller) so it is safe to outlive us. @@ -183,8 +184,12 @@ std::optional LiteWalletController::refreshModel() model.hasSyncStatus = true; model.sync.walletHeight = syncResult.syncStatus.syncedBlocks; model.sync.chainHeight = syncResult.syncStatus.totalBlocks; - model.sync.progress = syncResult.syncStatus.progress; - model.sync.complete = syncResult.syncStatus.complete; + // syncDone_ is authoritative: the detached sync thread is still running, so we are NOT + // complete regardless of what syncstatus reports. The backend briefly returns the idle + // shape ({"syncing":"false"} -> parser progress=1.0, complete=true) before the scan + // starts publishing in-progress status; don't surface that as a misleading 100%/done. + model.sync.complete = false; + model.sync.progress = syncResult.syncStatus.complete ? 0.0 : syncResult.syncStatus.progress; return model; } diff --git a/src/wallet/lite_wallet_controller.h b/src/wallet/lite_wallet_controller.h index 3a24d4a..d77af15 100644 --- a/src/wallet/lite_wallet_controller.h +++ b/src/wallet/lite_wallet_controller.h @@ -125,6 +125,14 @@ private: // The bridge is shared (not just owned) so the detached, uninterruptible sync thread can // safely outlive the controller: it holds a ref, so the underlying bridge is destroyed // (and litelib_shutdown called) only once BOTH the controller and a running sync release it. + // + // Concurrent execute() is intentionally NOT guarded by a C++ mutex here. The design relies + // on overlapping calls (the detached sync thread runs the long `sync` while the refresh + // worker polls `syncstatus`, and the UI thread may call `new`). litelib serializes wallet + // access internally: LightClient.wallet is an Arc> (do_new_address takes + // write(), do_balance takes read(), do_sync_internal takes read()/write() and writes + // sync_status separately), so concurrent execute() calls cannot corrupt state. A coarse + // mutex would instead serialize sync against syncstatus polling and defeat the design. std::shared_ptr bridge_; LiteWalletLifecycleService lifecycle_; LiteWalletGateway gateway_; @@ -136,7 +144,7 @@ private: // Detached background sync (backend `sync` is a blocking, uninterruptible full scan). std::thread syncThread_; - bool syncLaunched_ = false; + std::atomic syncLaunched_{false}; // startSync() guard (set on the main thread) std::shared_ptr> syncDone_ = std::make_shared>(false); // Joinable background refresh worker (fast iterations: syncstatus, plus data once synced). diff --git a/src/wallet/lite_wallet_lifecycle_service.cpp b/src/wallet/lite_wallet_lifecycle_service.cpp index 009f0c9..85ebc3a 100644 --- a/src/wallet/lite_wallet_lifecycle_service.cpp +++ b/src/wallet/lite_wallet_lifecycle_service.cpp @@ -7,8 +7,6 @@ #include #include -#include - namespace dragonx { namespace wallet { @@ -384,11 +382,13 @@ LiteWalletLifecycleResult LiteWalletLifecycleService::bridgeResult( result.ok = true; result.bridgeAccepted = true; - // The bridge already classifies "Error:"-prefixed responses as failures (ok=false). - // A successful init returns a JSON document (seed info for create, wallet info for - // open/restore); treat a well-formed JSON response as a genuinely ready wallet rather - // than discarding it. (Richer field extraction lands with the sync slice.) - result.walletReady = nlohmann::json::accept(bridgeCall.value); + // The bridge already classifies "Error:"-prefixed responses (and null/empty returns) as + // failures (ok=false), so reaching here means the backend genuinely succeeded. The success + // payloads are NOT uniformly JSON: create/restore return a seed object + // ({"seed":..,"birthday":..}), but open (litelib_initialize_existing) returns the bare + // string "OK". A JSON-validity test would therefore wrongly mark a successful open as + // not-ready, so treat any non-empty success response as a ready wallet. + result.walletReady = !bridgeCall.value.empty(); result.status = successStatus; return result; } diff --git a/tests/fake_lite_backend.h b/tests/fake_lite_backend.h index f54f949..08cfc2a 100644 --- a/tests/fake_lite_backend.h +++ b/tests/fake_lite_backend.h @@ -51,13 +51,19 @@ inline char* liteFakeDup(const char* s) } inline bool liteFakeWalletExists(const char*) { return g_liteFakeWalletExists; } -inline char* liteFakeInitNew(bool, const char*) { return liteFakeDup("{\"result\":\"created\"}"); } +// Match the real litelib_* return shapes so tests exercise the production walletReady path: +// create/restore return a seed object ({"seed":..,"birthday":..}); open returns the bare +// string "OK" (NOT JSON) — see litelib_initialize_existing. +inline char* liteFakeInitNew(bool, const char*) +{ + return liteFakeDup("{\"seed\":\"fake seed phrase words\",\"birthday\":0}"); +} inline char* liteFakeInitFromPhrase(bool, const char*, const char*, unsigned long long, unsigned long long, bool) { - return liteFakeDup("{\"result\":\"restored\"}"); + return liteFakeDup("{\"seed\":\"fake seed phrase words\",\"birthday\":0}"); } -inline char* liteFakeInitExisting(bool, const char*) { return liteFakeDup("{\"result\":\"opened\"}"); } +inline char* liteFakeInitExisting(bool, const char*) { return liteFakeDup("OK"); } inline char* liteFakeExecute(const char* command, const char* args) { // new-address generation returns a JSON array with the new address (type from args: zs/R). diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index dafe436..2d941d7 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -4458,7 +4458,10 @@ void testLiteWalletControllerLifecycle() const auto liteCaps = makeWalletCapabilities(WalletBuildKind::Lite, /*embeddedDaemon*/ false, /*liteBackendLinked*/ true); const LiteConnectionSettings conn = defaultLiteConnectionSettings(); - // create -> wallet ready, walletOpen() true, persist callback fires once, no leak + // create -> wallet ready, walletOpen() true, persist callback fires once. + // (No alloc==freed leak assert here: a ready wallet launches the detached sync thread + + // refresh worker, which are still in flight at end-of-scope and legitimately hold owned + // strings. The leak/double-free invariant is checked in the thread-free bridge test.) { dragonx::test::resetLiteFakeCounters(); int persistCount = 0; @@ -4474,7 +4477,22 @@ void testLiteWalletControllerLifecycle() EXPECT_TRUE(result.operation == LiteWalletLifecycleOperation::CreateNew); EXPECT_TRUE(controller.walletOpen()); EXPECT_EQ(persistCount, 1); - EXPECT_EQ(dragonx::test::g_liteFakeAlloc, dragonx::test::g_liteFakeFreed); + } + + // open existing -> ready. Regression guard: the real backend's open path + // (litelib_initialize_existing) returns the bare string "OK", which is NOT valid JSON, so + // walletReady must key off a non-empty success response, not JSON validity. + { + dragonx::test::resetLiteFakeCounters(); + dragonx::test::g_liteFakeWalletExists = true; + LiteWalletController controller(liteCaps, conn, LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi())); + LiteWalletOpenRequest req; + req.passphrase = "hunter2"; + const auto result = controller.openWallet(req); + EXPECT_TRUE(result.ok); + EXPECT_TRUE(result.walletReady); + EXPECT_TRUE(result.operation == LiteWalletLifecycleOperation::OpenExisting); + EXPECT_TRUE(controller.walletOpen()); } // restore-from-seed round-trips to ready