diff --git a/src/app.cpp b/src/app.cpp index dba3728..6fea693 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -363,9 +363,12 @@ bool App::init() // On upgrade (version mismatch), re-save to persist new defaults + current version if (settings_->needsUpgradeSave()) { DEBUG_LOGF("[INFO] Wallet upgraded — re-saving settings with new defaults\n"); - settings_->save(); + settings_->save(); // no writer attached yet: synchronous settings_->clearUpgradeSave(); } + // From here on settings_->save() is written by file_writer_ off the UI thread (flushed by main() + // at exit); a save after a fund/security-critical setter still waits for the disk (Settings::save). + settings_->setFileWriter(&file_writer_); // Lite builds with a linked SDXL backend own a lite wallet controller that drives // real create/open/restore through the bridge. Full-node and unlinked-lite builds @@ -575,7 +578,9 @@ bool App::init() address_book_.load(); // Load the wallet-files metadata index (wallets.json); populated after each wallet load. Local - // only, missing file is fine. + // only, missing file is fine. Its saves are queued on file_writer_ (they follow every scan pass + // whose metadata changed, so they shouldn't fsync on the UI thread). + wallet_index_.setFileWriter(&file_writer_); wallet_index_.load(); // If this wallet build bundles a newer daemon than the one already installed, offer to update it diff --git a/src/app.h b/src/app.h index 16fbc6e..87919a5 100644 --- a/src/app.h +++ b/src/app.h @@ -36,6 +36,7 @@ #include "services/wallet_security_controller.h" #include "services/wallet_security_workflow.h" #include "util/async_task_manager.h" +#include "util/coalescing_writer.h" #include "util/fetch_backoff.h" #include "util/pool_stats_service.h" #include "wallet/wallet_capabilities.h" @@ -161,6 +162,12 @@ public: */ void shutdown(); + /** + * @brief Wait (bounded, 3 s) for queued settings.json / wallets.json writes to land on disk. + * main() calls it around shutdown(): _Exit() skips ~App(), so nothing else would. + */ + void flushPendingWrites() { file_writer_.flush(std::chrono::milliseconds(3000)); } + /** * @brief Check if app should exit (render loop can stop) * Enforces minimum 1-second display of shutdown screen so user sees feedback. @@ -871,6 +878,7 @@ private: // Saved connection credentials (needed to open the fast-lane connection) rpc::ConnectionConfig saved_config_; + util::CoalescingWriter file_writer_; // background atomic writer for settings.json / wallets.json std::unique_ptr settings_; std::unique_ptr lite_wallet_; // lite builds w/ linked backend // Pending send_tab callback for an in-flight lite send (delivered in update() once the diff --git a/src/config/settings.cpp b/src/config/settings.cpp index 8793d3d..95c769d 100644 --- a/src/config/settings.cpp +++ b/src/config/settings.cpp @@ -15,6 +15,7 @@ #include #include +#include "../util/coalescing_writer.h" #include "../util/logger.h" #include "../util/platform.h" @@ -657,8 +658,13 @@ bool Settings::save(const std::string& path) // Atomic + durable: write to a temp file, fsync, then rename over the real file. // A crash mid-write can no longer truncate settings.json (which would silently // reset every preference on the next launch). Owner-only (0600) — it carries the - // lite-server list and address metadata. - return util::Platform::writeFileAtomically(path, j.dump(4), /*restrictPermissions=*/true); + // lite-server list and address metadata. With a writer attached the same atomic write + // runs on its background thread (queued, latest-wins); after a critical setter + // (durable_next_save_) this waits until the file is on disk, as the old save did. + const bool ok = util::persistFileAtomically(writer_, durable_next_save_, path, j.dump(4), + /*restrictPermissions=*/true); + if (ok) durable_next_save_ = false; + return ok; } catch (const std::exception& e) { DEBUG_LOGF("Failed to save settings: %s\n", e.what()); return false; diff --git a/src/config/settings.h b/src/config/settings.h index 8ec5709..28533c4 100644 --- a/src/config/settings.h +++ b/src/config/settings.h @@ -12,6 +12,7 @@ #include namespace dragonx { +namespace util { class CoalescingWriter; } namespace config { /** @@ -39,10 +40,16 @@ public: /** * @brief Save settings to default location - * @return true if saved successfully + * With a file writer attached (the App's), the JSON snapshot is taken here, on the caller's thread, + * and the atomic write runs on the writer's thread: returns true once queued. After a fund/security- + * critical setter (see durable_next_save_) it waits until the file is on disk, like the old save(). + * @return true if saved (or queued) successfully */ bool save(); + /** @brief Route saves through a background writer (not owned; must outlive this). Null = synchronous. */ + void setFileWriter(util::CoalescingWriter* writer) { writer_ = writer; } + /** * @brief Save settings to specific path * @param path Path to settings file @@ -344,7 +351,7 @@ public: // First-run wizard bool getWizardCompleted() const { return wizard_completed_; } - void setWizardCompleted(bool v) { wizard_completed_ = v; } + void setWizardCompleted(bool v) { wizard_completed_ = v; durable_next_save_ = true; } // Whether the one-time "back up your seed phrase" reminder has already been shown. bool getSeedBackupReminded() const { return seed_backup_reminded_; } @@ -366,7 +373,7 @@ public: // observed to be actually encrypted. Lets a quit/crash/failed-connect before it applies be detected // and surfaced (W2-2). NEVER stores the passphrase — only the fact that encryption was requested. bool getEncryptionPending() const { return encryption_pending_; } - void setEncryptionPending(bool v) { encryption_pending_ = v; } + void setEncryptionPending(bool v) { encryption_pending_ = v; durable_next_save_ = true; } // Bundled-daemon size we last prompted to install (see App::renderDaemonUpdatePrompt). Lets the // "a newer node is bundled — update?" prompt fire once per wallet version, never re-nagging. @@ -377,31 +384,31 @@ public: // the datadir; defaults to the daemon's own default. Used at launch to know which wallet we're // on before connect + to scope per-wallet data. std::string getActiveWalletFile() const { return active_wallet_file_; } - void setActiveWalletFile(const std::string& v) { active_wallet_file_ = v; } + void setActiveWalletFile(const std::string& v) { active_wallet_file_ = v; durable_next_save_ = true; } // Pending "migrate to a seed wallet" state (Phase 1 created the wallet; a later sweep/adopt // step consumes it). dest = the new wallet's sweep-target z-address; tempDir = its datadir. bool getSeedMigrationPending() const { return seed_migration_pending_; } - void setSeedMigrationPending(bool v) { seed_migration_pending_ = v; } + void setSeedMigrationPending(bool v) { seed_migration_pending_ = v; durable_next_save_ = true; } std::string getSeedMigrationDest() const { return seed_migration_dest_; } - void setSeedMigrationDest(const std::string& v) { seed_migration_dest_ = v; } + void setSeedMigrationDest(const std::string& v) { seed_migration_dest_ = v; durable_next_save_ = true; } std::string getSeedMigrationTempDir() const { return seed_migration_temp_dir_; } - void setSeedMigrationTempDir(const std::string& v) { seed_migration_temp_dir_ = v; } + void setSeedMigrationTempDir(const std::string& v) { seed_migration_temp_dir_ = v; durable_next_save_ = true; } // The sweep transaction id, persisted once the sweep is submitted — non-empty means the // migration is past the sweep, so a resume goes to the confirm/adopt stage (not sweep again). std::string getSeedMigrationSweepTxid() const { return seed_migration_sweep_txid_; } - void setSeedMigrationSweepTxid(const std::string& v) { seed_migration_sweep_txid_ = v; } + void setSeedMigrationSweepTxid(const std::string& v) { seed_migration_sweep_txid_ = v; durable_next_save_ = true; } // W3-3: the async sweep operation id, persisted while the sweep is in flight (before it resolves // to a txid). Lets a resume re-poll a mid-sweep interruption instead of dropping the txid. Cleared // in the same write that persists the txid, so the txid always outranks it (see [[decideSeedMigrationResume]]). std::string getSeedMigrationSweepOpid() const { return seed_migration_sweep_opid_; } - void setSeedMigrationSweepOpid(const std::string& v) { seed_migration_sweep_opid_ = v; } + void setSeedMigrationSweepOpid(const std::string& v) { seed_migration_sweep_opid_ = v; durable_next_save_ = true; } // Set TRUE synchronously when a sweep is broadcast; survives the stale-opid resume fallback so the // Sweep step knows a ~0 legacy balance may be an UNCONFIRMED in-flight sweep (funds not yet safe) // and must NOT short-circuit to adopt/swap wallet.dat. Cleared on confirmed-adopt / reset / a // failed submit. See [[decideSeedMigrationResume]] and App::renderSeedMigrationDialog (fund-safety). bool getSeedMigrationSweepSubmitted() const { return seed_migration_sweep_submitted_; } - void setSeedMigrationSweepSubmitted(bool v) { seed_migration_sweep_submitted_ = v; } + void setSeedMigrationSweepSubmitted(bool v) { seed_migration_sweep_submitted_ = v; durable_next_save_ = true; } // Security — auto-lock timeout (seconds; 0 = disabled) int getAutoLockTimeout() const { return auto_lock_timeout_; } @@ -413,7 +420,7 @@ public: // Security — PIN unlock enabled bool getPinEnabled() const { return pin_enabled_; } - void setPinEnabled(bool v) { pin_enabled_ = v; } + void setPinEnabled(bool v) { pin_enabled_ = v; durable_next_save_ = true; } // Daemon — keep running in background when closing the app bool getKeepDaemonRunning() const { return keep_daemon_running_; } @@ -581,6 +588,12 @@ public: private: std::string settings_path_; + util::CoalescingWriter* writer_ = nullptr; // null -> synchronous saves (tests, tools, early startup) + // Armed by the fund/security-critical setters (seed-migration stage, encryption_pending, pin_enabled, + // active_wallet_file, wizard_completed): the next save() waits until the file is on disk before it + // returns, so e.g. the sweep-submitted marker is durable before the sweep is broadcast. Cleared by a + // successful save. A NEW setter for crash-critical state must arm it too. + bool durable_next_save_ = false; // Settings values std::string theme_ = "dragonx"; diff --git a/src/data/wallet_index.cpp b/src/data/wallet_index.cpp index 9250de3..b09ae79 100644 --- a/src/data/wallet_index.cpp +++ b/src/data/wallet_index.cpp @@ -9,6 +9,7 @@ #include #include +#include "../util/coalescing_writer.h" #include "../util/logger.h" #include "../util/platform.h" @@ -109,7 +110,8 @@ bool WalletIndex::save() j["extraFolders"] = extra_folders_; // No secrets here (file names + coarse metadata) but keep it owner-only for consistency. - if (!util::Platform::writeFileAtomically(file_path_, j.dump(2), /*restrictPermissions=*/true)) { + if (!util::persistFileAtomically(writer_, /*waitForDisk=*/false, file_path_, j.dump(2), + /*restrictPermissions=*/true)) { DEBUG_LOGF("Could not write wallet index: %s\n", file_path_.c_str()); return false; } diff --git a/src/data/wallet_index.h b/src/data/wallet_index.h index 434c198..93aa1fd 100644 --- a/src/data/wallet_index.h +++ b/src/data/wallet_index.h @@ -8,6 +8,7 @@ #include namespace dragonx { +namespace util { class CoalescingWriter; } namespace data { /** @@ -41,9 +42,12 @@ struct WalletIndexEntry { class WalletIndex { public: bool load(); - bool save(); + bool save(); // queued on the writer when one is attached (returns true once queued) static std::string getDefaultPath(); + /** @brief Route save() through a background writer (not owned; must outlive this). Null = synchronous. */ + void setFileWriter(util::CoalescingWriter* writer) { writer_ = writer; } + const std::vector& entries() const { return entries_; } const std::vector& extraFolders() const { return extra_folders_; } @@ -64,6 +68,7 @@ private: std::vector entries_; std::vector extra_folders_; std::string file_path_; + util::CoalescingWriter* writer_ = nullptr; // null -> synchronous writes }; } // namespace data diff --git a/src/main.cpp b/src/main.cpp index 4d72467..f20e33e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2357,8 +2357,14 @@ int main(int argc, char* argv[]) _Exit(0); }).detach(); + // Land the queued settings/wallets.json writes (incl. the window size saved above) before the joins + // in shutdown(); bounded (3 s) so a wedged disk can't eat the watchdog's budget. + app.flushPendingWrites(); // Final cleanup (daemon already stopped by beginShutdown) app.shutdown(); + // shutdown()'s fallback branch saves settings again; land that too, BEFORE the single-instance lock + // is released below so a relaunch can't read a stale settings.json. _Exit() skips ~App(). + app.flushPendingWrites(); #ifdef DRAGONX_USE_DX11 Shutdown(window, dx); #else diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index 5ad27d2..4a05b58 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -8138,6 +8138,69 @@ void testAtomicFileWrite() removeTree(dir); } +#ifndef _WIN32 +// C269: Settings + WalletIndex saves through the App's background writer, under a temp $HOME (as +// testWalletIndex does; Windows' config dir can't be redirected). The HOME redirect is what keeps a +// developer's real settings.json untouched (getConfigDir() uses only $HOME/.config); XDG_CONFIG_HOME is +// redirected too purely as a precaution. A queued save lands after flush(); a critical setter +// makes the next save wait for the disk (readable with NO flush), and an older queued snapshot can't land +// after it. +void testSettingsBackgroundSave() +{ + using ms = std::chrono::milliseconds; + const char* oldHome = std::getenv("HOME"); + const std::string savedHome = oldHome ? oldHome : ""; + const char* oldXdg = std::getenv("XDG_CONFIG_HOME"); + const bool hadXdg = oldXdg != nullptr; + const std::string savedXdg = hadXdg ? oldXdg : ""; + const fs::path tmp = makeTempDir(); + setenv("HOME", tmp.string().c_str(), 1); + setenv("XDG_CONFIG_HOME", (tmp / ".config").string().c_str(), 1); + { + dragonx::util::CoalescingWriter w; // declared first: outlives s and idx + dragonx::config::Settings s; + s.setFileWriter(&w); + s.setUIOpacity(0.55f); + EXPECT_TRUE(s.save()); // queued + s.setUIOpacity(0.65f); + EXPECT_TRUE(s.save()); // coalesces over / follows the first + EXPECT_TRUE(w.flush(ms(5000))); + { + dragonx::config::Settings back; + EXPECT_TRUE(back.load()); + EXPECT_NEAR(back.getUIOpacity(), 0.65, 1e-6); + } + // A critical setter makes the next save wait for the disk: readable with NO flush, and the + // queued older snapshot can't land after it. + s.setUIOpacity(0.70f); + EXPECT_TRUE(s.save()); // queued + s.setPinEnabled(true); + EXPECT_TRUE(s.save()); // durable + { + dragonx::config::Settings back; + EXPECT_TRUE(back.load()); + EXPECT_TRUE(back.getPinEnabled()); + EXPECT_NEAR(back.getUIOpacity(), 0.70, 1e-6); + } + EXPECT_EQ(w.pendingCount(), (size_t)0); + + dragonx::data::WalletIndex idx; + idx.setFileWriter(&w); + idx.load(); + dragonx::data::WalletIndexEntry e; e.fileName = "wallet.dat"; e.displayName = "Main"; + idx.upsert(e); + EXPECT_TRUE(idx.save()); // queued + EXPECT_TRUE(w.flush(ms(5000))); + dragonx::data::WalletIndex idx2; + EXPECT_TRUE(idx2.load()); + EXPECT_TRUE(idx2.find("wallet.dat") != nullptr); + } + if (!savedHome.empty()) setenv("HOME", savedHome.c_str(), 1); else unsetenv("HOME"); + if (hadXdg) setenv("XDG_CONFIG_HOME", savedXdg.c_str(), 1); else unsetenv("XDG_CONFIG_HOME"); + removeTree(tmp); +} +#endif + #ifndef _WIN32 // C218: openUrl/openFolder must return without waiting for the opener. Fake xdg-open/open that sleep 3 s // are the ONLY entries on PATH (posix_spawnp keeps searching after EACCES, so on a noexec temp dir an @@ -12665,6 +12728,7 @@ int main() testCoalescingWriterOrdering(); testAtomicFileWrite(); #ifndef _WIN32 + testSettingsBackgroundSave(); testOpenerDoesNotBlock(); #endif testHushChatCrypto();