wallet: fix consolidation-scheduler wedge (ran every block; dead mutual-exclusion)
The Sapling auto-consolidation scheduler never advanced nextConsolidation after init and never set fConsolidationRunning, so once the tip passed the init threshold `-consolidation` dispatched a consolidation op every block instead of once per -consolidationinterval, and every guard that reads fConsolidationRunning (in RunSaplingSweep, and in the new autoshield driver) was dead. Mirror the intended scheduler model: - RunSaplingConsolidation sets fConsolidationRunning=true before dispatch and self-guards with `if (fConsolidationRunning) return;`. - The consolidation op advances nextConsolidation = consolidationInterval + tipHeight and clears fConsolidationRunning on every terminal state (success/failure/exception/cancel), guarded by op id so only the current op mutates scheduler state. - saplingConsolidationOperationId moved to public so the op can read it. Restores the documented once-per-interval cadence and makes the sweep/consolidation/autoshield mutual-exclusion guards effective. Note: the sweep op has the same latent wedge (fSweepRunning/nextSweep are cleared only on the sweepComplete success path, so a cancelled or throwing sweep leaves fSweepRunning stuck true) - left for a follow-up; this commit is the template to port. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -28,8 +28,17 @@ AsyncRPCOperation_saplingconsolidation::AsyncRPCOperation_saplingconsolidation(i
|
||||
AsyncRPCOperation_saplingconsolidation::~AsyncRPCOperation_saplingconsolidation() {}
|
||||
|
||||
void AsyncRPCOperation_saplingconsolidation::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->saplingConsolidationOperationId) {
|
||||
pwalletMain->fConsolidationRunning = false;
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
set_state(OperationStatus::EXECUTING);
|
||||
start_execution_clock();
|
||||
@@ -76,6 +85,21 @@ void AsyncRPCOperation_saplingconsolidation::main() {
|
||||
LogPrintf("%s", s);
|
||||
unlock_notes(); // clean up
|
||||
LogPrint("zrpc", "%s: consolidation input notes unlocked\n", getId());
|
||||
|
||||
// Advance the interval and clear the running flag on EVERY terminal state
|
||||
// (success, failure, exception) so consolidation runs once per interval
|
||||
// instead of every block, and a failed round still lets the next one fire.
|
||||
// Only the CURRENT op does this bookkeeping. This fixes the pre-existing
|
||||
// wedge where nextConsolidation never advanced and fConsolidationRunning
|
||||
// was never set/reset.
|
||||
if (pwalletMain) {
|
||||
LOCK2(cs_main, pwalletMain->cs_wallet);
|
||||
if (getId() == pwalletMain->saplingConsolidationOperationId) {
|
||||
int tipHeight = (chainActive.Tip() != NULL) ? chainActive.Tip()->GetHeight() : targetHeight_;
|
||||
pwalletMain->nextConsolidation = pwalletMain->consolidationInterval + tipHeight;
|
||||
pwalletMain->fConsolidationRunning = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
bool AsyncRPCOperation_saplingconsolidation::main_impl() {
|
||||
|
||||
Reference in New Issue
Block a user