From 53a10e149d0eb8d53196129da9070d76882a09f0 Mon Sep 17 00:00:00 2001 From: DanS Date: Sun, 7 Jun 2026 14:17:17 -0500 Subject: [PATCH] fix(rpc): detect mid-session disconnects and stop blocking the UI thread The connection state machine never tore down on a lost connection: refresh-loop RPC errors were swallowed, rpc_->isConnected() stayed true after a daemon crash/restart/socket drop, and the UI showed stale balances with no reconnect. Several operations also ran synchronous curl straight from ImGui handlers. - Add handleLostConnection(): after N consecutive cycles where BOTH core RPCs fail (warmup excluded, so no reconnect loop), disconnect so update()'s reconnect branch re-enters tryConnect(). - Move banPeer/unbanPeer/clearBans and key export/import onto the worker thread (import requests a rescan that could freeze the UI for the curl timeout). - Run the block-info dialog's two chained RPCs on the worker thread (+ guard the getblockhash result type). - Detect daemon warmup via the JSON-RPC -28 code (new RpcError carrying the code; message text preserved so 401/warmup string-matching is unaffected), and widen CONNECTTIMEOUT to 10s for remote/TLS hosts (2s localhost). Co-Authored-By: Claude Opus 4.8 --- src/app.h | 7 + src/app_network.cpp | 186 ++++++++++++++++++--------- src/rpc/rpc_client.cpp | 64 ++++++--- src/rpc/rpc_client.h | 16 +++ src/ui/windows/block_info_dialog.cpp | 26 +++- 5 files changed, 221 insertions(+), 78 deletions(-) diff --git a/src/app.h b/src/app.h index ff677d6..885f7da 100644 --- a/src/app.h +++ b/src/app.h @@ -574,6 +574,9 @@ private: bool encryption_state_prefetched_ = false; // suppress duplicate getwalletinfo on connect bool rescan_status_poll_in_progress_ = false; bool opid_poll_in_progress_ = false; + // Consecutive Core-refresh cycles where BOTH core RPCs failed → likely a dead + // connection. After kCoreFailuresBeforeDisconnect, tear down and reconnect. + int consecutive_core_failures_ = 0; // Pending z_sendmany operation tracking bool send_progress_active_ = false; @@ -692,6 +695,10 @@ private: void tryConnect(); void onConnected(); void onDisconnected(const std::string& reason); + // Tear down a connection that died mid-session (daemon crash / restart / dropped + // socket) so update()'s reconnect branch re-enters tryConnect(). Unlike onDisconnected + // alone, this also rpc_->disconnect()s so rpc_->isConnected() actually flips to false. + void handleLostConnection(const std::string& reason); void applyDefaultBanlist(); // Private methods - data refresh diff --git a/src/app_network.cpp b/src/app_network.cpp index 0c3ec53..b658c14 100644 --- a/src/app_network.cpp +++ b/src/app_network.cpp @@ -493,6 +493,12 @@ void App::onDisconnected(const std::string& reason) last_tx_block_height_ = -1; pending_opids_.clear(); pending_send_info_.clear(); + // Resolve any deferred send callbacks so their UI doesn't spin forever on disconnect. + for (auto& entry : pending_send_callbacks_) { + if (entry.second) entry.second(false, reason); + } + pending_send_callbacks_.clear(); + consecutive_core_failures_ = 0; send_progress_active_ = false; send_submissions_in_flight_ = 0; network_refresh_.resetJobs(); @@ -510,6 +516,15 @@ void App::onDisconnected(const std::string& reason) } } +void App::handleLostConnection(const std::string& reason) +{ + DEBUG_LOGF("[Connection] %s — tearing down for reconnect\n", reason.c_str()); + // Flip the main client's connected_ flag so update()'s else-branch re-enters + // tryConnect(). onDisconnected() alone only tears down the fast lane. + if (rpc_) rpc_->disconnect(); + onDisconnected(reason); +} + // ============================================================================ // Data Refresh — Tab-Aware Prioritized System // @@ -1034,6 +1049,25 @@ void App::refreshCoreData() try { NetworkRefreshService::applyCoreRefreshResult(state_, result, std::time(nullptr)); applyPendingSendBalanceDeltas(true); + + // 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 + // with -28 there legitimately, and counting it would cause a reconnect loop. + constexpr int kCoreFailuresBeforeDisconnect = 3; + if (!state_.warming_up) { + if (!result.balanceOk && !result.blockchainOk) { + if (++consecutive_core_failures_ >= kCoreFailuresBeforeDisconnect && + state_.connected) { + consecutive_core_failures_ = 0; + handleLostConnection("Lost connection to daemon"); + return; // state torn down — skip the rest of this callback + } + } else { + consecutive_core_failures_ = 0; + } + } + // Auto-shield transparent funds if enabled if (result.balanceOk && settings_ && settings_->getAutoShield() && state_.transparent_balance > 0.0001 && !state_.sync.syncing && @@ -1573,28 +1607,58 @@ void App::stopPoolMining() void App::banPeer(const std::string& ip, int duration_seconds) { - if (!state_.connected || !rpc_) return; - - rpc_->setBan(ip, "add", [this](const json&) { - refreshPeerInfo(); - }, nullptr, duration_seconds); + if (!state_.connected || !rpc_ || !worker_) return; + // Run on the worker thread — these are called straight from the Peers tab's ImGui + // handlers, and rpc_->call() blocks on synchronous curl under curl_mutex_. + worker_->post([this, ip, duration_seconds]() -> rpc::RPCWorker::MainCb { + std::string err; + try { + rpc::RPCClient::TraceScope trace("Peers / Ban"); + rpc_->call("setban", {ip, "add", duration_seconds}); + } catch (const std::exception& e) { + err = e.what(); + } + return [this, err]() { + if (!err.empty()) ui::Notifications::instance().error("Ban failed: " + err); + else refreshPeerInfo(); + }; + }); } void App::unbanPeer(const std::string& ip) { - if (!state_.connected || !rpc_) return; - - rpc_->setBan(ip, "remove", [this](const json&) { - refreshPeerInfo(); + if (!state_.connected || !rpc_ || !worker_) return; + worker_->post([this, ip]() -> rpc::RPCWorker::MainCb { + std::string err; + try { + rpc::RPCClient::TraceScope trace("Peers / Unban"); + rpc_->call("setban", {ip, "remove"}); + } catch (const std::exception& e) { + err = e.what(); + } + return [this, err]() { + if (!err.empty()) ui::Notifications::instance().error("Unban failed: " + err); + else refreshPeerInfo(); + }; }); } void App::clearBans() { - if (!state_.connected || !rpc_) return; - - rpc_->clearBanned([this](const json&) { - state_.banned_peers.clear(); + if (!state_.connected || !rpc_ || !worker_) return; + worker_->post([this]() -> rpc::RPCWorker::MainCb { + std::string err; + try { + rpc::RPCClient::TraceScope trace("Peers / Clear bans"); + rpc_->call("clearbanned", nlohmann::json::array()); + } catch (const std::exception& e) { + err = e.what(); + } + return [this, err]() { + if (!err.empty()) { ui::Notifications::instance().error("Clear bans failed: " + err); return; } + state_.banned_peers.clear(); + refreshPeerInfo(); + }; }); } @@ -1881,33 +1945,35 @@ void App::invalidateAddressValidationCache() void App::exportPrivateKey(const std::string& address, std::function callback) { - if (!state_.connected || !rpc_) { + if (!state_.connected || !rpc_ || !worker_) { if (callback) callback(""); return; } - - auto keyKind = services::WalletSecurityController::classifyAddress(address); - if (keyKind == services::WalletSecurityController::KeyKind::Shielded) { - // Z-address: use z_exportkey - rpc::RPCClient::TraceScope trace("Settings / Export private key"); - rpc_->z_exportKey(address, [callback](const json& result) { - if (callback) callback(result.get()); - }, [callback](const std::string& error) { - DEBUG_LOGF("Export z-key error: %s\n", error.c_str()); - ui::Notifications::instance().error("Key export failed: " + error); - if (callback) callback(""); - }); - } else { - // T-address: use dumpprivkey - rpc::RPCClient::TraceScope trace("Settings / Export private key"); - rpc_->dumpPrivKey(address, [callback](const json& result) { - if (callback) callback(result.get()); - }, [callback](const std::string& error) { - DEBUG_LOGF("Export t-key error: %s\n", error.c_str()); - ui::Notifications::instance().error("Key export failed: " + error); - if (callback) callback(""); - }); - } + + const bool shielded = services::WalletSecurityController::classifyAddress(address) + == services::WalletSecurityController::KeyKind::Shielded; + const char* method = shielded ? "z_exportkey" : "dumpprivkey"; + // Run on the worker thread — z_exportkey/dumpprivkey block on synchronous curl and + // are invoked straight from the export dialog (UI thread). + worker_->post([this, method, address, callback]() -> rpc::RPCWorker::MainCb { + std::string key; + std::string err; + try { + rpc::RPCClient::TraceScope trace("Settings / Export private key"); + key = rpc_->call(method, {address}).get(); + } catch (const std::exception& e) { + err = e.what(); + } + return [callback, key, err]() { + if (!err.empty()) { + DEBUG_LOGF("Export key error: %s\n", err.c_str()); + ui::Notifications::instance().error("Key export failed: " + err); + if (callback) callback(""); + } else if (callback) { + callback(key); + } + }; + }); } void App::exportAllKeys(std::function callback) @@ -1961,34 +2027,36 @@ void App::exportAllKeys(std::function callback) void App::importPrivateKey(const std::string& key, std::function callback) { - if (!state_.connected || !rpc_) { + if (!state_.connected || !rpc_ || !worker_) { if (callback) callback(false, "Not connected"); return; } - - auto keyKind = services::WalletSecurityController::classifyPrivateKey(key); - - if (keyKind == services::WalletSecurityController::KeyKind::Shielded) { - rpc::RPCClient::TraceScope trace("Settings / Import private key"); - rpc_->z_importKey(key, true, [this, callback](const json& result) { + + const bool shielded = services::WalletSecurityController::classifyPrivateKey(key) + == services::WalletSecurityController::KeyKind::Shielded; + // Run on the worker thread — import requests a full rescan (rescan=true), so the + // synchronous curl call can take many seconds; never block the UI thread on it. + worker_->post([this, key, shielded, callback]() -> rpc::RPCWorker::MainCb { + std::string err; + try { + rpc::RPCClient::TraceScope trace("Settings / Import private key"); + if (shielded) rpc_->call("z_importkey", {key, "yes"}); // rescan + else rpc_->call("importprivkey", {key, "", true}); // label "", rescan + } catch (const std::exception& e) { + err = e.what(); + } + return [this, shielded, err, callback]() { + if (!err.empty()) { + if (callback) callback(false, err); + return; + } invalidateAddressValidationCache(); refreshAddresses(); if (callback) callback(true, services::WalletSecurityController::importSuccessMessage( - services::WalletSecurityController::KeyKind::Shielded)); - }, [callback](const std::string& error) { - if (callback) callback(false, error); - }); - } else { - rpc::RPCClient::TraceScope trace("Settings / Import private key"); - rpc_->importPrivKey(key, true, [this, callback](const json& result) { - invalidateAddressValidationCache(); - refreshAddresses(); - if (callback) callback(true, services::WalletSecurityController::importSuccessMessage( - services::WalletSecurityController::KeyKind::Transparent)); - }, [callback](const std::string& error) { - if (callback) callback(false, error); - }); - } + shielded ? services::WalletSecurityController::KeyKind::Shielded + : services::WalletSecurityController::KeyKind::Transparent)); + }; + }); } void App::backupWallet(const std::string& destination, std::function callback) diff --git a/src/rpc/rpc_client.cpp b/src/rpc/rpc_client.cpp index 1f18433..6f3a973 100644 --- a/src/rpc/rpc_client.cpp +++ b/src/rpc/rpc_client.cpp @@ -6,6 +6,7 @@ // All calls are blocking; run on RPCWorker threads, never on main thread. #include "rpc_client.h" +#include "connection.h" #include "../config/version.h" #include "../util/base64.h" @@ -170,7 +171,10 @@ bool RPCClient::connect(const std::string& host, const std::string& port, curl_easy_setopt(impl_->curl, CURLOPT_HTTPHEADER, impl_->headers); curl_easy_setopt(impl_->curl, CURLOPT_WRITEFUNCTION, WriteCallback); curl_easy_setopt(impl_->curl, CURLOPT_TIMEOUT, 30L); - curl_easy_setopt(impl_->curl, CURLOPT_CONNECTTIMEOUT, 1L); // localhost — fails fast if not listening + // Localhost fails fast if nothing is listening; a remote/TLS daemon needs a larger + // budget for the TCP + TLS handshake over real network latency (1s would spuriously fail). + const long connectTimeout = Connection::isLocalHost(host) ? 2L : 10L; + curl_easy_setopt(impl_->curl, CURLOPT_CONNECTTIMEOUT, connectTimeout); // Test connection with getinfo try { @@ -191,7 +195,12 @@ bool RPCClient::connect(const std::string& host, const std::string& port, // it just hasn't finished initializing yet. Mark as connected+warmup // so the wallet can show the UI instead of a blocking overlay. std::string msg = e.what(); - bool isWarmup = (msg.find("Loading") != std::string::npos || + // Warmup is JSON-RPC error code -28 (RPC_IN_WARMUP) — the robust signal. Fall back + // to message substrings for any path that didn't carry the numeric code. + int code = 0; + if (const auto* re = dynamic_cast(&e)) code = re->code; + bool isWarmup = (code == -28) || + (msg.find("Loading") != std::string::npos || msg.find("Verifying") != std::string::npos || msg.find("Activating") != std::string::npos || msg.find("Rewinding") != std::string::npos || @@ -281,23 +290,36 @@ json RPCClient::call(const std::string& method, const json& params) // (insufficient funds, bad params, etc.) with a valid JSON body. // Parse the body first to extract the real error message. if (http_code != 200) { + int errCode = 0; try { json response = json::parse(response_data); - if (response.contains("error") && !response["error"].is_null()) { - std::string err_msg = response["error"]["message"].get(); - throw std::runtime_error(err_msg); + if (response.contains("error") && response["error"].is_object()) { + if (response["error"].contains("code") && response["error"]["code"].is_number_integer()) + errCode = response["error"]["code"].get(); + if (response["error"].contains("message") && response["error"]["message"].is_string()) + throw RpcError(errCode, response["error"]["message"].get()); + // message missing/non-string — keep the detail instead of a bare HTTP code + throw RpcError(errCode, "RPC error: " + response["error"].dump()); } } catch (const json::exception&) { // Body wasn't valid JSON — fall through to generic HTTP error } - throw std::runtime_error("RPC error: HTTP " + std::to_string(http_code)); + throw RpcError(errCode, "RPC error: HTTP " + std::to_string(http_code)); } json response = json::parse(response_data); - + if (response.contains("error") && !response["error"].is_null()) { - std::string err_msg = response["error"]["message"].get(); - throw std::runtime_error("RPC error: " + err_msg); + int errCode = 0; + std::string err_msg; + if (response["error"].is_object()) { + if (response["error"].contains("code") && response["error"]["code"].is_number_integer()) + errCode = response["error"]["code"].get(); + if (response["error"].contains("message") && response["error"]["message"].is_string()) + err_msg = response["error"]["message"].get(); + } + if (err_msg.empty()) err_msg = response["error"].dump(); + throw RpcError(errCode, "RPC error: " + err_msg); } return response["result"]; @@ -340,20 +362,32 @@ json RPCClient::call(const std::string& method, const json& params, long timeout curl_easy_getinfo(impl_->curl, CURLINFO_RESPONSE_CODE, &http_code); if (http_code != 200) { + int errCode = 0; try { json response = json::parse(response_data); - if (response.contains("error") && !response["error"].is_null()) { - std::string err_msg = response["error"]["message"].get(); - throw std::runtime_error(err_msg); + if (response.contains("error") && response["error"].is_object()) { + if (response["error"].contains("code") && response["error"]["code"].is_number_integer()) + errCode = response["error"]["code"].get(); + if (response["error"].contains("message") && response["error"]["message"].is_string()) + throw RpcError(errCode, response["error"]["message"].get()); + throw RpcError(errCode, "RPC error: " + response["error"].dump()); } } catch (const json::exception&) {} - throw std::runtime_error("RPC error: HTTP " + std::to_string(http_code)); + throw RpcError(errCode, "RPC error: HTTP " + std::to_string(http_code)); } json response = json::parse(response_data); if (response.contains("error") && !response["error"].is_null()) { - std::string err_msg = response["error"]["message"].get(); - throw std::runtime_error("RPC error: " + err_msg); + int errCode = 0; + std::string err_msg; + if (response["error"].is_object()) { + if (response["error"].contains("code") && response["error"]["code"].is_number_integer()) + errCode = response["error"]["code"].get(); + if (response["error"].contains("message") && response["error"]["message"].is_string()) + err_msg = response["error"]["message"].get(); + } + if (err_msg.empty()) err_msg = response["error"].dump(); + throw RpcError(errCode, "RPC error: " + err_msg); } return response["result"]; diff --git a/src/rpc/rpc_client.h b/src/rpc/rpc_client.h index 69baaff..3ad26b1 100644 --- a/src/rpc/rpc_client.h +++ b/src/rpc/rpc_client.h @@ -9,6 +9,7 @@ #include #include #include +#include #include namespace dragonx { @@ -18,6 +19,21 @@ using json = nlohmann::json; using Callback = std::function; using ErrorCallback = std::function; +/** + * @brief A JSON-RPC error carrying the daemon's numeric error code. + * + * what() preserves the exact human-readable message (so existing string matching + * still works); `code` exposes the JSON-RPC error code — notably -28 (RPC_IN_WARMUP) + * for a daemon still starting up. Derives from std::runtime_error, so every existing + * `catch (const std::exception&)` continues to handle it unchanged. + */ +class RpcError : public std::runtime_error { +public: + RpcError(int errorCode, const std::string& message) + : std::runtime_error(message), code(errorCode) {} + int code = 0; +}; + /** * @brief JSON-RPC client for dragonxd * diff --git a/src/ui/windows/block_info_dialog.cpp b/src/ui/windows/block_info_dialog.cpp index 8e1e19d..e533c60 100644 --- a/src/ui/windows/block_info_dialog.cpp +++ b/src/ui/windows/block_info_dialog.cpp @@ -5,6 +5,7 @@ #include "block_info_dialog.h" #include "../../app.h" #include "../../rpc/rpc_client.h" +#include "../../rpc/rpc_worker.h" #include "../../util/i18n.h" #include "../notifications.h" #include "../schema/ui_schema.h" @@ -124,14 +125,31 @@ void BlockInfoDialog::render(App* app) } if (material::StyledButton(TR("block_get_info"), ImVec2(0,0), S.resolveFont(closeBtn.font))) { - if (rpc && rpc->isConnected()) { + if (rpc && rpc->isConnected() && app->worker()) { s_loading = true; s_error.clear(); s_has_data = false; s_pending_app = app; - - // Use getBlock(height) which uses UnifiedCallback - rpc->getBlock(s_height, handleBlockResponseUnified); + + // Run the two chained RPCs (getblockhash → getblock) on the worker thread; + // doing them inline froze the UI for two round-trips. Guard the hash type. + int height = s_height; + app->worker()->post([rpc, height]() -> rpc::RPCWorker::MainCb { + json block; + std::string error; + try { + rpc::RPCClient::TraceScope trace("Explorer / Block info"); + auto hashResult = rpc->call("getblockhash", {height}); + if (!hashResult.is_string()) { + error = "unexpected getblockhash result"; + } else { + block = rpc->call("getblock", {hashResult.get()}); + } + } catch (const std::exception& e) { + error = e.what(); + } + return [block, error]() { handleBlockResponseUnified(block, error); }; + }); } }