From 43fc0090d4ee460ab45c8ba566d5c9b56959ddee Mon Sep 17 00:00:00 2001 From: DanS Date: Sat, 3 Oct 2026 19:52:09 -0500 Subject: [PATCH] fix(wallet): retire tracked sends that only paid the wallet itself A send stays tracked until z_viewtransaction shows an outgoing output. Shields, merges/consolidations and chat note splits pay only the wallet itself, so they never showed one. They stayed tracked for the whole session and forced a full history refresh past the scan throttle on every tab. - A tracked send whose view has no outgoing output is retired once it is mined (1+ confirmations) or conflicted. - Tracked sends are enriched first, and reaching the per-cycle view cap no longer ends the loop, so the cap can't starve them. - Backstop: a tracked send still unsettled 20 blocks after it was added (expired, dropped from the mempool) stops being tracked. - Tracked sends are cleared on disconnect: they belong to that connection's wallet. Co-Authored-By: Claude Opus 5.5 --- src/app.cpp | 1 + src/app.h | 4 + src/app_network.cpp | 29 +++++++ src/services/network_refresh_service.cpp | 37 ++++++++- src/services/network_refresh_service.h | 4 + tests/test_phase4.cpp | 96 ++++++++++++++++++++++++ 6 files changed, 168 insertions(+), 3 deletions(-) diff --git a/src/app.cpp b/src/app.cpp index be2c4e7..f2ecb66 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -1311,6 +1311,7 @@ void App::update() successfulOpids.insert(opid); markPendingSendTransactionSucceeded(opid, txid); send_txids_.insert(txid); + send_txid_heights_.emplace(txid, state_.sync.blocks); invokeSendResultCallback(opid, true, txid); } diff --git a/src/app.h b/src/app.h index d4781d9..73d548c 100644 --- a/src/app.h +++ b/src/app.h @@ -1507,6 +1507,10 @@ private: // even when they don't appear in listtransactions or // z_listreceivedbyaddress. std::unordered_set send_txids_; + // Tip height when each tracked txid was added (TTL backstop, see pruneTrackedSendTxids()). + std::unordered_map send_txid_heights_; + static constexpr int kTrackedSendTtlBlocks = 20; + void pruneTrackedSendTxids(); // First-run wizard state WizardPhase wizard_phase_ = WizardPhase::None; diff --git a/src/app_network.cpp b/src/app_network.cpp index 5f70b19..45f1ef4 100644 --- a/src/app_network.cpp +++ b/src/app_network.cpp @@ -811,6 +811,10 @@ void App::onDisconnected(const std::string& reason) // Clear RPC result caches viewtx_cache_.clear(); + // Tracked sends belong to this connection's wallet: kept across a switch/adopt they made + // z_viewtransaction fail on the next wallet and left phantom pending "send" rows in its History. + send_txids_.clear(); + send_txid_heights_.clear(); confirmed_tx_cache_.clear(); confirmed_tx_ids_.clear(); confirmed_cache_block_ = -1; @@ -2161,6 +2165,7 @@ void App::refreshTransactionData() } NetworkRefreshService::applyTransactionRefreshResult( state_, cacheUpdate, std::move(result), std::time(nullptr)); + pruneTrackedSendTxids(); shielded_history_scan_heights_ = std::move(shieldedScanHeights); storeTransactionHistoryCacheIfAvailable(); shielded_history_scan_cursor_ = nextShieldedScanStartIndex; @@ -2175,6 +2180,30 @@ void App::refreshTransactionData() network_refresh_.resetTxAge(); } +// Backstop for tracked sends that never settle (expired or dropped from the mempool, an RPC error, a +// txid the wallet can't view): stop tracking after kTrackedSendTtlBlocks, since every tracked id keeps +// forcing full history refreshes past the scan throttle. Anything that does confirm later is still +// found and enriched by the untracked history sweep (z_listsentbyaddress). +void App::pruneTrackedSendTxids() +{ + const int tip = state_.sync.blocks; + for (auto it = send_txid_heights_.begin(); it != send_txid_heights_.end();) { + if (!send_txids_.count(it->first)) { // already resolved by a refresh + it = send_txid_heights_.erase(it); + continue; + } + if (it->second <= 0 && tip > 0) it->second = tip; // added before the tip was known + if (tip > 0 && it->second > 0 && tip - it->second > kTrackedSendTtlBlocks) { + DEBUG_LOGF("[tx] stop tracking send %s: unsettled after %d blocks\n", + it->first.c_str(), tip - it->second); + send_txids_.erase(it->first); + it = send_txid_heights_.erase(it); + continue; + } + ++it; + } +} + void App::refreshRecentTransactionData() { if (!worker_ || !rpc_ || !state_.connected) return; diff --git a/src/services/network_refresh_service.cpp b/src/services/network_refresh_service.cpp index 532e0af..96a190c 100644 --- a/src/services/network_refresh_service.cpp +++ b/src/services/network_refresh_service.cpp @@ -1094,8 +1094,21 @@ NetworkRefreshService::TransactionRefreshResult NetworkRefreshService::collectTr if (!cachedView.outgoing_outputs.empty()) knownTxids.insert(txid); } - int viewTxCount = 0; + // Tracked sends go first so they get the per-cycle z_viewtransaction budget ahead of routine + // enrichment, and a spent budget only skips further RPCs: cached rows later in the order are still + // appended (stopping the loop at the cap dropped them from History). + std::vector enrichOrder; + enrichOrder.reserve(knownTxids.size()); for (const auto& txid : knownTxids) { + if (snapshot.sendTxids.count(txid)) enrichOrder.push_back(&txid); + } + for (const auto& txid : knownTxids) { + if (!snapshot.sendTxids.count(txid)) enrichOrder.push_back(&txid); + } + + int viewTxCount = 0; + for (const std::string* txidPtr : enrichOrder) { + const std::string& txid = *txidPtr; bool trackedSend = snapshot.sendTxids.count(txid) > 0; auto cached = snapshot.viewTxCache.find(txid); @@ -1112,7 +1125,7 @@ NetworkRefreshService::TransactionRefreshResult NetworkRefreshService::collectTr if (!trackedSend && snapshot.fullyEnrichedTxids.count(txid)) continue; - if (viewTxCount >= maxViewTransactionsPerCycle) break; + if (viewTxCount >= maxViewTransactionsPerCycle) continue; ++viewTxCount; try { @@ -1163,6 +1176,24 @@ NetworkRefreshService::TransactionRefreshResult NetworkRefreshService::collectTr if (!trackedSend || !entry.outgoing_outputs.empty()) { result.newViewTxEntries[txid] = std::move(entry); + } else { + // A tracked op that only paid the wallet itself never shows an outgoing output, so the + // check above kept it tracked (and forcing history refreshes past the scan throttle on + // every tab) for the rest of the session. While it's unconfirmed the empty result stays + // retryable; once it is mined, or conflicted, the view result is final: cache it and + // stop tracking. Confirmations come from this txid's rows (listtransactions, or the + // gettransaction-backed placeholder) and any raw metadata applied to the entry. + bool mined = entry.confirmations >= 1; + bool conflicted = entry.confirmations < 0; + for (const auto& info : result.transactions) { + if (info.txid != txid) continue; + if (info.confirmations >= 1) mined = true; + else if (info.confirmations < 0) conflicted = true; + } + if (mined || conflicted) { + result.newViewTxEntries[txid] = std::move(entry); + result.resolvedSendTxids.insert(txid); + } } } catch (const std::exception& e) { (void)e; @@ -1512,7 +1543,7 @@ void NetworkRefreshService::applyTransactionRefreshResult(WalletState& state, for (auto& [txid, entry] : result.newViewTxEntries) { bool hasOutgoingOutputs = !entry.outgoing_outputs.empty(); cacheUpdate.viewTxCache[txid] = std::move(entry); - if (hasOutgoingOutputs) cacheUpdate.sendTxids.erase(txid); + if (hasOutgoingOutputs || result.resolvedSendTxids.count(txid)) cacheUpdate.sendTxids.erase(txid); } cacheUpdate.confirmedTxCache.clear(); diff --git a/src/services/network_refresh_service.h b/src/services/network_refresh_service.h index 4f14e14..ce918a3 100644 --- a/src/services/network_refresh_service.h +++ b/src/services/network_refresh_service.h @@ -222,6 +222,10 @@ public: std::vector hushChatMetadata; int blockHeight = -1; TransactionViewCache newViewTxEntries; + // Tracked sends whose view result is final although it shows no outgoing output: the op only + // paid the wallet itself (shield, merge/consolidate, auto-shield, chat note split, a transfer + // between own addresses) and is now mined or conflicted. Applying the result stops tracking them. + std::unordered_set resolvedSendTxids; std::size_t nextShieldedScanStartIndex = 0; std::size_t shieldedAddressesScanned = 0; std::size_t shieldedAddressCount = 0; diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index 997b43b..7e6ba52 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -9302,6 +9302,101 @@ void testWalletIndex() #endif // !_WIN32 } +// Tracked sends that only paid the wallet itself (shield, merge/consolidate, chat note split) show no +// outgoing output in z_viewtransaction. They used to stay tracked for the whole session, forcing a full +// history refresh past the scan throttle on every tab; once mined or conflicted they are now retired. +void testTrackedSendRetirement() +{ + using Refresh = dragonx::services::NetworkRefreshService; + using json = nlohmann::json; + const json ownOnlyView = json{ + {"spends", json::array({json{{"address", "zs-own"}}})}, + {"outputs", json::array({json{{"outgoing", false}, {"address", "zs-own"}, {"value", 1.0}}})}}; + + // Mined: retired and cached. + Refresh::TransactionRefreshSnapshot minedSnapshot; + minedSnapshot.sendTxids = {"self-mined"}; + MockRefreshRpc minedRpc; + minedRpc.addResponse("listtransactions", json::array()); + minedRpc.addResponse("z_viewtransaction", ownOnlyView); + minedRpc.addResponse("gettransaction", json{{"time", 800}, {"confirmations", 3}, {"amount", 0.0}}); + auto mined = Refresh::collectTransactionRefreshResult(minedRpc, minedSnapshot, 400, 4); + EXPECT_EQ(mined.resolvedSendTxids.count("self-mined"), static_cast(1)); + EXPECT_EQ(mined.newViewTxEntries.count("self-mined"), static_cast(1)); + + // Conflicted (negative confirmations): retired too. + Refresh::TransactionRefreshSnapshot conflictSnapshot; + conflictSnapshot.sendTxids = {"self-conflicted"}; + MockRefreshRpc conflictRpc; + conflictRpc.addResponse("listtransactions", json::array()); + conflictRpc.addResponse("z_viewtransaction", ownOnlyView); + conflictRpc.addResponse("gettransaction", json{{"time", 801}, {"confirmations", -1}}); + auto conflicted = Refresh::collectTransactionRefreshResult(conflictRpc, conflictSnapshot, 400, 4); + EXPECT_EQ(conflicted.resolvedSendTxids.count("self-conflicted"), static_cast(1)); + + // Unconfirmed: stays tracked and uncached, so the next cycle retries it. + Refresh::TransactionRefreshSnapshot pendingSnapshot; + pendingSnapshot.sendTxids = {"self-pending"}; + MockRefreshRpc pendingRpc; + pendingRpc.addResponse("listtransactions", json::array()); + pendingRpc.addResponse("z_viewtransaction", ownOnlyView); + pendingRpc.addResponse("gettransaction", json{{"time", 802}, {"confirmations", 0}}); + auto pending = Refresh::collectTransactionRefreshResult(pendingRpc, pendingSnapshot, 400, 4); + EXPECT_TRUE(pending.resolvedSendTxids.empty()); + EXPECT_EQ(pending.newViewTxEntries.count("self-pending"), static_cast(0)); + + // Applying the mined result drops only the retired id. + dragonx::WalletState state; + Refresh::TransactionViewCache viewCache; + std::unordered_set sendTxids{"self-mined", "self-pending"}; + std::vector confirmedCache; + std::unordered_set confirmedIds; + int confirmedBlock = -1; + int lastTxBlock = -1; + Refresh::TransactionCacheUpdate cacheUpdate{viewCache, sendTxids, confirmedCache, confirmedIds, + confirmedBlock, lastTxBlock}; + Refresh::applyTransactionRefreshResult(state, cacheUpdate, std::move(mined), 999); + EXPECT_EQ(sendTxids.count("self-mined"), static_cast(0)); + EXPECT_EQ(sendTxids.count("self-pending"), static_cast(1)); + EXPECT_EQ(viewCache.count("self-mined"), static_cast(1)); + + // A spent z_viewtransaction budget skips further RPCs but still appends cached rows that sort + // after the cutoff (the loop used to stop there and drop them from History). + Refresh::TransactionRefreshSnapshot capSnapshot; + Refresh::TransactionViewCacheEntry cachedSend; + cachedSend.from_address = "zs-from"; + cachedSend.outgoing_outputs.push_back({"zs-dest", 0.5, ""}); + capSnapshot.viewTxCache["z-cached-send"] = cachedSend; + MockRefreshRpc capRpc; + capRpc.addResponse("listtransactions", json::array({ + json{{"txid", "a-recv"}, {"category", "receive"}, {"amount", 1.0}, {"time", 100}, + {"confirmations", 3}, {"address", "R-one"}}, + json{{"txid", "b-recv"}, {"category", "receive"}, {"amount", 2.0}, {"time", 101}, + {"confirmations", 3}, {"address", "R-one"}} + })); + capRpc.addResponse("z_viewtransaction", json{{"spends", json::array()}, {"outputs", json::array()}}); + auto capped = Refresh::collectTransactionRefreshResult(capRpc, capSnapshot, 400, 1); + EXPECT_TRUE(capRpc.methodNames() == std::vector({"listtransactions", "z_viewtransaction"})); + EXPECT_EQ(capRpc.calls[1].params, json::array({"a-recv"})); + EXPECT_TRUE(std::any_of(capped.transactions.begin(), capped.transactions.end(), + [](const dragonx::TransactionInfo& t) { return t.txid == "z-cached-send"; })); + + // Tracked sends get the budget first, whatever their txid sorts as. + Refresh::TransactionRefreshSnapshot orderSnapshot; + orderSnapshot.sendTxids = {"zz-tracked"}; + MockRefreshRpc orderRpc; + orderRpc.addResponse("listtransactions", json::array({ + json{{"txid", "a-recv"}, {"category", "receive"}, {"amount", 1.0}, {"time", 100}, + {"confirmations", 3}, {"address", "R-one"}} + })); + orderRpc.addResponse("z_viewtransaction", json{ + {"spends", json::array({json{{"address", "zs-from"}}})}, + {"outputs", json::array({json{{"outgoing", true}, {"address", "zs-peer"}, {"value", 0.25}}})}}); + orderRpc.addResponse("gettransaction", json{{"time", 900}, {"confirmations", 0}}); + Refresh::collectTransactionRefreshResult(orderRpc, orderSnapshot, 400, 1); + EXPECT_EQ(orderRpc.calls[1].params, json::array({"zz-tracked"})); +} + // The displayed-balance roll must never overshoot, must land on the target after an idle-length // frame, and must survive Reduce Motion and non-finite values (it used to flip between two wrong // values at dt*speed == 2 and diverge to inf/NaN under Reduce Motion's speed of 999). @@ -9360,6 +9455,7 @@ int main() testParseDaemonRescanOutput(); testNetworkRefreshRpcCollectors(); testNetworkRefreshResultModels(); + testTrackedSendRetirement(); testBalanceDisplayLerp(); testOperationStatusPollParsing(); testSecureVaultScope();