wallet: fix sweep-scheduler wedge and unsynchronized driver mutation
The zaddr-sweep op cleared fSweepRunning/nextSweep only on the sweepComplete success path (inside main_impl), so a cancelled or throwing sweep left fSweepRunning stuck true. Since fSweepRunning now also gates consolidation and the default-on autoshield, a persistently failing sweep (e.g. a corrupt-witness note) would wedge all three background ops for the session. Move the scheduler bookkeeping into main() so it runs on every terminal state (success/failure/exception/cancel), guarded by op id. Preserve the intended "keep draining every block until swept" model: on a successful-but-incomplete round the flag stays set and nextSweep is not advanced; on completion OR on failure/exception the flag is released and nextSweep backs off one interval, so a failing sweep no longer retries every block or wedges the other ops. Also fix RunSaplingSweep to take cs_wallet itself (was AssertLockHeld, a no-op in release builds) since ChainTip does not hold it there and the driver mutates scheduler state + enqueues -- matching RunSaplingConsolidation and RunAutoShieldCoinbase. sweepComplete is recorded via a new member; saplingSweepOperationId made public. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -27,8 +27,17 @@ AsyncRPCOperation_sweep::AsyncRPCOperation_sweep(int targetHeight, bool fromRpc)
|
|||||||
AsyncRPCOperation_sweep::~AsyncRPCOperation_sweep() {}
|
AsyncRPCOperation_sweep::~AsyncRPCOperation_sweep() {}
|
||||||
|
|
||||||
void AsyncRPCOperation_sweep::main() {
|
void AsyncRPCOperation_sweep::main() {
|
||||||
if (isCancelled())
|
if (isCancelled()) {
|
||||||
|
// Only the current op owns the scheduler flag; a stale/cancelled op must
|
||||||
|
// not clear it out from under a freshly-enqueued successor.
|
||||||
|
if (pwalletMain) {
|
||||||
|
LOCK(pwalletMain->cs_wallet);
|
||||||
|
if (getId() == pwalletMain->saplingSweepOperationId) {
|
||||||
|
pwalletMain->fSweepRunning = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
return;
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
set_state(OperationStatus::EXECUTING);
|
set_state(OperationStatus::EXECUTING);
|
||||||
start_execution_clock();
|
start_execution_clock();
|
||||||
@@ -64,6 +73,23 @@ void AsyncRPCOperation_sweep::main() {
|
|||||||
set_state(OperationStatus::FAILED);
|
set_state(OperationStatus::FAILED);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Scheduler bookkeeping, done here so it runs on success AND failure AND
|
||||||
|
// exception (main_impl's terminal code is skipped when it throws). Only the
|
||||||
|
// current op mutates scheduler state. Preserves the "keep draining every
|
||||||
|
// block until swept" model: on a successful-but-incomplete round we leave
|
||||||
|
// fSweepRunning set and nextSweep unadvanced so the next block continues.
|
||||||
|
// On completion OR on failure/exception we release fSweepRunning and back
|
||||||
|
// off one interval — critically, a persistently failing sweep no longer
|
||||||
|
// leaves fSweepRunning stuck true and wedges consolidation + autoshield.
|
||||||
|
if (pwalletMain) {
|
||||||
|
LOCK2(cs_main, pwalletMain->cs_wallet);
|
||||||
|
if (getId() == pwalletMain->saplingSweepOperationId && (!success || sweepComplete_)) {
|
||||||
|
int tipHeight = (chainActive.Tip() != NULL) ? chainActive.Tip()->GetHeight() : targetHeight_;
|
||||||
|
pwalletMain->nextSweep = pwalletMain->sweepInterval + tipHeight;
|
||||||
|
pwalletMain->fSweepRunning = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
std::string s = strprintf("%s: Sweep operation finished. (status=%s", getId(), getStateAsString());
|
std::string s = strprintf("%s: Sweep operation finished. (status=%s", getId(), getStateAsString());
|
||||||
if (success) {
|
if (success) {
|
||||||
s += strprintf(", success)\n");
|
s += strprintf(", success)\n");
|
||||||
@@ -314,10 +340,11 @@ bool AsyncRPCOperation_sweep::main_impl() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (sweepComplete) {
|
// Record whether the wallet is fully swept; the scheduler bookkeeping
|
||||||
pwalletMain->nextSweep = pwalletMain->sweepInterval + chainActive.Tip()->GetHeight();
|
// (advancing nextSweep / clearing fSweepRunning) is done in main() so it
|
||||||
pwalletMain->fSweepRunning = false;
|
// also runs on the failure/exception/cancel paths and cannot wedge the
|
||||||
}
|
// shared fSweepRunning flag (which now also gates consolidation + autoshield).
|
||||||
|
sweepComplete_ = sweepComplete;
|
||||||
|
|
||||||
LogPrintf("%s: Created %d transactions with total output amount=%s, status=%d\n", getId(), numTxCreated, FormatMoney(amountSwept), (int)status);
|
LogPrintf("%s: Created %d transactions with total output amount=%s, status=%d\n", getId(), numTxCreated, FormatMoney(amountSwept), (int)status);
|
||||||
setSweepResult(numTxCreated, amountSwept, sweepTxIds);
|
setSweepResult(numTxCreated, amountSwept, sweepTxIds);
|
||||||
|
|||||||
@@ -34,6 +34,10 @@ public:
|
|||||||
private:
|
private:
|
||||||
int targetHeight_;
|
int targetHeight_;
|
||||||
bool fromRPC_;
|
bool fromRPC_;
|
||||||
|
// Set by main_impl(): true iff there was nothing left to sweep this round.
|
||||||
|
// Read by main() to decide scheduler bookkeeping. Defaults false so an
|
||||||
|
// exception (which skips main_impl's assignment) is treated as "not done".
|
||||||
|
bool sweepComplete_ = false;
|
||||||
|
|
||||||
bool main_impl();
|
bool main_impl();
|
||||||
|
|
||||||
|
|||||||
@@ -585,7 +585,13 @@ void CWallet::RunSaplingSweep(int blockHeight) {
|
|||||||
if (blockHeight == 0)
|
if (blockHeight == 0)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
AssertLockHeld(cs_wallet);
|
// Take cs_wallet ourselves: ChainTip (the notify-thread caller) does NOT
|
||||||
|
// hold it here, and we mutate fSweepRunning/nextSweep/saplingSweepOperationId
|
||||||
|
// and enqueue below. Matches RunSaplingConsolidation/RunAutoShieldCoinbase.
|
||||||
|
// (The old AssertLockHeld(cs_wallet) was a no-op in release builds and thus
|
||||||
|
// masked an unsynchronized mutation.) cs_wallet is recursive, so this is
|
||||||
|
// safe even on any path that already holds it.
|
||||||
|
LOCK(cs_wallet);
|
||||||
if (!fSweepEnabled) {
|
if (!fSweepEnabled) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -786,7 +786,6 @@ private:
|
|||||||
std::vector<CTransaction> pendingSaplingConsolidationTxs;
|
std::vector<CTransaction> pendingSaplingConsolidationTxs;
|
||||||
|
|
||||||
std::vector<CTransaction> pendingSaplingSweepTxs;
|
std::vector<CTransaction> pendingSaplingSweepTxs;
|
||||||
AsyncRPCOperationId saplingSweepOperationId;
|
|
||||||
|
|
||||||
void AddToTransparentSpends(const COutPoint& outpoint, const uint256& wtxid);
|
void AddToTransparentSpends(const COutPoint& outpoint, const uint256& wtxid);
|
||||||
void AddToSaplingSpends(const uint256& nullifier, const uint256& wtxid);
|
void AddToSaplingSpends(const uint256& nullifier, const uint256& wtxid);
|
||||||
@@ -831,6 +830,9 @@ public:
|
|||||||
int rescanStartHeight = 0;
|
int rescanStartHeight = 0;
|
||||||
|
|
||||||
int nextSweep = 0;
|
int nextSweep = 0;
|
||||||
|
// Id of the in-flight sweep op; read by the op to confirm it is still the
|
||||||
|
// current one before mutating scheduler state.
|
||||||
|
AsyncRPCOperationId saplingSweepOperationId;
|
||||||
int amountSwept = 0;
|
int amountSwept = 0;
|
||||||
int amountConsolidated = 0;
|
int amountConsolidated = 0;
|
||||||
int sweepInterval = 10;
|
int sweepInterval = 10;
|
||||||
|
|||||||
Reference in New Issue
Block a user