From 7a62fc487751694a4499d1439956ad4faab1d86d Mon Sep 17 00:00:00 2001 From: DanS Date: Tue, 25 Aug 2026 07:59:13 +0200 Subject: [PATCH] wallet: break the sweep/consolidation/autoshield deadlock A successful-but-incomplete sweep round deliberately returns with fSweepRunning still set and nextSweep unadvanced, as a "keep draining next block" baton. Every early return in RunSaplingSweep, though, leaves that baton set without re-dispatching -- and RunSaplingConsolidation, which is gated on fSweepRunning, then returns without advancing nextConsolidation. So the "consolidation is within 5 blocks" blackout at the top of RunSaplingSweep never lifts: sweep waits on consolidation, consolidation waits on sweep, and neither runs again. That much is pre-existing. What is new is that autoshield now shares the gate -- RunAutoShieldCoinbase returns early on fSweepRunning || fConsolidationRunning -- so a wedged sweep silently disables coinbase shielding too, with z_autoshieldstatus reporting autoshield true, running false, and no reason. Only honour the baton while a sweep operation is genuinely in flight: if the operation for saplingSweepOperationId is absent or has reached a terminal state, drop the stale flag and let the checks below decide afresh. The drain model is unchanged -- nextSweep is still unadvanced, so the next block re-dispatches. Also report the deferral in z_autoshieldstatus, so mutual exclusion with sweep or consolidation reads as a deferral rather than an unexplained idle. Co-Authored-By: Claude Opus 5 (1M context) --- src/wallet/rpcwallet.cpp | 6 ++++++ src/wallet/wallet.cpp | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 4a136331c..08f3117a7 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3408,6 +3408,12 @@ UniValue z_autoshieldstatus(const UniValue& params, bool fHelp, const CPubKey& m why = "disabled by -autoshield=0"; } else if (pwalletMain->IsLocked()) { why = "wallet is locked; rounds are skipped until it is unlocked"; + } else if (pwalletMain->fSweepRunning || pwalletMain->fConsolidationRunning) { + // Autoshield is mutually exclusive with sweep and consolidation. Without + // this the RPC reports autoshield=true, running=false and an empty + // reason while no round can actually start. + why = strprintf("deferred while %s is running; rounds resume when it finishes", + pwalletMain->fSweepRunning ? "z_sweep" : "sapling consolidation"); } else if (pwalletMain->autoShieldAddress.empty()) { why = ""; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 81df81e1e..7a78591f9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -592,6 +592,31 @@ void CWallet::RunSaplingSweep(int blockHeight) { // masked an unsynchronized mutation.) cs_wallet is recursive, so this is // safe even on any path that already holds it. LOCK(cs_wallet); + + // Stale-baton guard. A successful-but-incomplete sweep round deliberately + // returns with fSweepRunning still set and nextSweep unadvanced (see + // AsyncRPCOperation_sweep::main), as a "continue draining next block" baton. + // But every early return below leaves that baton set WITHOUT re-dispatching, + // and RunSaplingConsolidation -- which is gated on fSweepRunning -- then + // returns without advancing nextConsolidation, so the "consolidation is + // within 5 blocks" blackout at the top of this function never lifts. That + // is a self-sustaining three-way deadlock: sweep waits on consolidation, + // consolidation waits on sweep, and autoshield shares the same gate, so a + // wedged sweep silently disables coinbase shielding forever. + // Only honour the baton while a sweep operation genuinely is in flight. + if (fSweepRunning) { + std::shared_ptr sweepQueue = getAsyncRPCQueue(); + std::shared_ptr inFlightSweep = + (sweepQueue != nullptr) ? sweepQueue->getOperationForId(saplingSweepOperationId) : nullptr; + bool inFlight = (inFlightSweep != nullptr) && + (inFlightSweep->isReady() || inFlightSweep->isExecuting()); + if (!inFlight) { + LogPrintf("%s: clearing stale fSweepRunning at blockHeight=%d (no sweep operation in flight)\n", + __func__, blockHeight); + fSweepRunning = false; + } + } + if (!fSweepEnabled) { return; }