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) <noreply@anthropic.com>
This commit is contained in:
@@ -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 = "";
|
||||
}
|
||||
|
||||
@@ -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<AsyncRPCQueue> sweepQueue = getAsyncRPCQueue();
|
||||
std::shared_ptr<AsyncRPCOperation> 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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user