From 870793433be713553139746b04bf3ea6d815517d Mon Sep 17 00:00:00 2001 From: DanS Date: Sun, 30 Aug 2026 22:45:51 -0500 Subject: [PATCH] fix(sync): stop large-wallet balance polling from starving block connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a fully-shielded (ac_private=1) chain, z_gettotalbalance is O(mapWallet) and holds the daemon's cs_main for its whole duration — ~20s on a ~5k-tx wallet. The Overview refresh polled it every ~2s (twice: minconf 0 and 1), so cs_main was held almost continuously, starving the single block-connection thread: the node connected blocks only in the gaps between polls and could fall further behind the tip than it caught up (observed live: gap growing 58→100 blocks while the GUI was open, one core pegged on GetFilteredNotes, 22 idle, ~17 B/s download). Two hardening changes on top of the existing "skip balance while syncing" guard: - Hysteresis: keep the low-impact sync profile (and balance suppression) for a short settle window after catching up, so a large-wallet scan can't immediately re-starve connection and bounce the node back into syncing. Armed only on the syncing→caught-up edge, so a wallet synced from the start is never throttled at connect (effectivelySyncing()). - Adaptive balance cadence: time each z_gettotalbalance scan and require the next poll to wait at least (cost / 10%), so balance scanning never occupies more than ~10% of wall-clock. Cheap wallets are unaffected (the tab's Core timer stays the cadence); a ~20s scan backs off to ~200s. Wallet mutations (send/shield) force the next poll through so the user's own action updates the balance immediately (balanceRefreshDue()). getblockchaininfo keeps its normal cadence throughout, so sync progress stays live. Build + test_phase4 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app.cpp | 4 +- src/app.h | 10 +++++ src/app_network.cpp | 56 +++++++++++++++++++++--- src/services/network_refresh_service.cpp | 12 ++++- src/services/network_refresh_service.h | 1 + 5 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/app.cpp b/src/app.cpp index ee4d877..9828be1 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -919,7 +919,9 @@ void App::update() // Re-apply the refresh cadence when sync starts/finishes: while syncing we throttle polling to // a low-impact profile so RPC contention doesn't slow block download (see applyRefreshPolicy). - if (state_.sync.syncing != refresh_policy_syncing_) { + // effectivelySyncing() includes the post-sync settle window, so this also reverts to the normal + // per-tab cadence once that window elapses. + if (effectivelySyncing() != refresh_policy_syncing_) { applyRefreshPolicy(current_page_); } diff --git a/src/app.h b/src/app.h index dc1f0fa..93cc22a 100644 --- a/src/app.h +++ b/src/app.h @@ -1102,6 +1102,14 @@ private: bool daemon_start_error_shown_ = false; int daemon_last_seen_crashes_ = 0; // surface each new embedded-daemon crash reason once bool refresh_policy_syncing_ = false; // whether the sync-throttle refresh profile is active + // Sync-settle hysteresis + adaptive balance-poll throttle. Balance polling (z_gettotalbalance) is + // O(mapWallet) and holds the daemon's cs_main, which starves block connection on a large shielded + // wallet — so we keep the low-impact profile briefly after catching up, and back the balance poll + // off in proportion to its own measured cost. See effectivelySyncing() / balanceRefreshDue(). + bool was_core_syncing_ = false; // previous Core-refresh sync state, to detect the caught-up edge + std::time_t sync_settle_until_ = 0; // hold the sync-throttle until this wall-clock time (0 = not settling) + double last_balance_scan_ms_ = 0.0; // measured cost of the last z_gettotalbalance scan + bool force_balance_refresh_ = false; // a wallet mutation forces the next balance poll through the throttle // Auto-clear for secrets copied to the clipboard. Only a hash of the copied secret is kept. std::uint64_t clipboard_secret_hash_ = 0; double clipboard_clear_deadline_ = 0.0; @@ -1439,6 +1447,8 @@ private: void refreshPrice(); void refreshWalletEncryptionState(); void applyRefreshPolicy(ui::NavPage page); + bool effectivelySyncing() const; // syncing, or within the post-sync settle window (hysteresis) + bool balanceRefreshDue() const; // adaptive: enough time elapsed given the last balance-scan cost? bool currentPageNeedsWalletDataRefresh() const; bool shouldRunWalletTransactionRefresh() const; bool shouldRefreshTransactions() const; diff --git a/src/app_network.cpp b/src/app_network.cpp index 17015f8..b05ca64 100644 --- a/src/app_network.cpp +++ b/src/app_network.cpp @@ -837,13 +837,39 @@ void App::applyRefreshPolicy(ui::NavPage page) // While the daemon is syncing, override the per-tab cadence with the low-impact sync profile so // the wallet stops contending for the daemon's cs_main lock (frequent getpeerinfo / per-block // transaction scans / balance polls slow block connection). This makes every tab sync as fast - // as the Console tab does today. Reverts to the per-tab profile once sync finishes. - refresh_policy_syncing_ = state_.sync.syncing; + // as the Console tab does today. effectivelySyncing() keeps this profile on briefly after catching + // up (hysteresis) so a large-wallet scan can't immediately re-starve connection and bounce the + // node back into "syncing". Reverts to the per-tab profile once the settle window passes. + refresh_policy_syncing_ = effectivelySyncing(); network_refresh_.setIntervals(refresh_policy_syncing_ ? services::RefreshScheduler::kSyncProfile : getIntervalsForPage(page)); } +// True while the node is behind, and for a short settle window after it first catches up. The settle +// window is armed only on the syncing→caught-up edge (see the Core refresh callback), so a wallet that +// was synced from the start is never throttled at connect — only a node that just finished catching up. +bool App::effectivelySyncing() const +{ + if (state_.sync.syncing) return true; + if (sync_settle_until_ == 0) return false; // no pending settle → genuinely caught up + return std::time(nullptr) < sync_settle_until_; +} + +// Adaptive throttle: the next balance poll must wait at least (lastScanCost / kBalanceDutyCycle) since +// the last one, so balance scanning can never occupy more than ~kBalanceDutyCycle of wall-clock. A +// cheap wallet (sub-cadence cost) is unaffected — the tab's Core timer stays the real cadence; a ~20s +// scan on a large wallet backs off to roughly every ~200s instead of every 2s, freeing cs_main for +// block connection. A wallet mutation bypasses this via force_balance_refresh_. +bool App::balanceRefreshDue() const +{ + constexpr double kBalanceDutyCycle = 0.10; + if (state_.last_balance_update == 0) return true; // never fetched + if (last_balance_scan_ms_ <= 0.0) return true; // no cost measured yet + const double minInterval = (last_balance_scan_ms_ / 1000.0) / kBalanceDutyCycle; + return std::difftime(std::time(nullptr), state_.last_balance_update) >= minInterval; +} + bool App::currentPageNeedsWalletDataRefresh() const { using NP = ui::NavPage; @@ -1666,9 +1692,15 @@ void App::refreshCoreData() ? fast_rpc_.get() : rpc_.get(); if (!w || !rpc) return; ui::NavPage tracePage = current_page_; - // Skip the balance call while syncing (it's incomplete anyway and takes the wallet lock + - // cs_main). Captured on the main thread to avoid reading state_ off the worker thread. - const bool includeBalance = !state_.sync.syncing; + // Decide whether to include the balance call (z_gettotalbalance — O(mapWallet), holds cs_main). + // Suppress it (a) while syncing or within the post-sync settle window, so it can't starve block + // connection, and (b) unless enough time has elapsed given the LAST scan's measured cost, so a + // large shielded wallet backs off automatically instead of re-scanning every couple of seconds. + // A wallet mutation (send/shield) forces the next poll through so the user's own action updates the + // balance immediately. Captured on the main thread to avoid reading state_ off the worker thread. + const bool includeBalance = !effectivelySyncing() && + (force_balance_refresh_ || balanceRefreshDue()); + if (includeBalance) force_balance_refresh_ = false; auto enqueued = network_refresh_.enqueue(services::NetworkRefreshService::Job::Core, *w, [this, rpc, tracePage, includeBalance]() -> rpc::RPCWorker::MainCb { AppRefreshRpcGateway refreshRpc(*rpc, traceSource(tracePage, "Core refresh")); @@ -1678,6 +1710,19 @@ void App::refreshCoreData() NetworkRefreshService::applyCoreRefreshResult(state_, result, std::time(nullptr)); applyPendingSendBalanceDeltas(true); + // Feed the adaptive balance throttle + sync-settle hysteresis. Record the last scan's + // cost (0 when balance was skipped), and arm the settle window only on the + // syncing→caught-up edge so a wallet synced from the start is never throttled at connect. + if (result.balanceScanMs > 0.0) last_balance_scan_ms_ = result.balanceScanMs; + const bool nowSyncing = state_.sync.syncing; + if (nowSyncing) { + sync_settle_until_ = 0; + } else if (was_core_syncing_) { + constexpr double kSyncSettleSeconds = 8.0; + sync_settle_until_ = std::time(nullptr) + static_cast(kSyncSettleSeconds); + } + was_core_syncing_ = nowSyncing; + // Mid-session connection-loss detection. During normal operation, both core // RPCs failing together means the daemon connection is dead (a busy daemon // fails them individually, not both at once). Warmup is excluded — both fail @@ -5300,6 +5345,7 @@ void App::submitZSendMany(const std::string& from, const std::string& to, double // Force transaction list refresh so the sent tx appears immediately transactions_dirty_ = true; last_tx_block_height_ = -1; + force_balance_refresh_ = true; // the user's own send must update the balance now, past the throttle network_refresh_.markWalletMutationRefresh(); // z_sendmany only returned an opid: the transaction is built/signed/ // broadcast asynchronously by the daemon. Defer the user-facing diff --git a/src/services/network_refresh_service.cpp b/src/services/network_refresh_service.cpp index 39755e5..33d331f 100644 --- a/src/services/network_refresh_service.cpp +++ b/src/services/network_refresh_service.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -294,8 +295,13 @@ NetworkRefreshService::CoreRefreshResult NetworkRefreshService::collectCoreRefre json blockInfo; bool balanceOk = false; bool blockOk = false; + double balanceScanMs = 0.0; if (includeBalance) { + // z_gettotalbalance is O(mapWallet) and holds the daemon's cs_main for its whole duration — + // seconds on a large shielded wallet. Time it so the caller can throttle how often it polls + // (balanceRefreshDue()), keeping balance scans from starving block connection. + const auto balanceStart = std::chrono::steady_clock::now(); try { // DISPLAY total: minconf=0 — includes the user's own pending change so it doesn't crater totalBalance = rpc.call("z_gettotalbalance", json::array({0})); balanceOk = true; @@ -305,6 +311,8 @@ NetworkRefreshService::CoreRefreshResult NetworkRefreshService::collectCoreRefre try { // SPENDABLE total: minconf=1 (confirmed). If absent, spendable degrades to display in apply. spendableBalance = rpc.call("z_gettotalbalance", json::array({1})); } catch (...) {} + balanceScanMs = std::chrono::duration( + std::chrono::steady_clock::now() - balanceStart).count(); } try { @@ -314,7 +322,9 @@ NetworkRefreshService::CoreRefreshResult NetworkRefreshService::collectCoreRefre DEBUG_LOGF("BlockchainInfo error: %s\n", e.what()); } - return parseCoreRefreshResult(totalBalance, spendableBalance, balanceOk, blockInfo, blockOk); + auto result = parseCoreRefreshResult(totalBalance, spendableBalance, balanceOk, blockInfo, blockOk); + result.balanceScanMs = balanceScanMs; + return result; } NetworkRefreshService::MiningRefreshResult NetworkRefreshService::parseMiningRefreshResult( diff --git a/src/services/network_refresh_service.h b/src/services/network_refresh_service.h index 26f2215..514a345 100644 --- a/src/services/network_refresh_service.h +++ b/src/services/network_refresh_service.h @@ -111,6 +111,7 @@ public: std::optional verificationProgress; std::optional longestChain; std::optional notarized; + double balanceScanMs = 0.0; // wall-clock spent in z_gettotalbalance this refresh (0 if balance skipped) }; struct MiningRefreshResult {