async/wallet: close the loose ends around automated operations

Six small defects, all found by an audit of the automated-operation path and all
verified present before changing anything.

AsyncRPCQueue::addOperation returned void and silently dropped the operation when
the queue was closed or finishing. Every caller assumed success: the three
schedulers left their running flag set with nothing in flight (which then blocks
every later round), and z_sendmany / z_shieldcoinbase / z_mergetoaddress returned
an opid for work that would never run -- z_shieldcoinbase and z_mergetoaddress
having already locked their selected coins in the constructor. It now returns
bool; the schedulers release the flag and log, and the three RPCs raise an error
instead of handing back an opid. Coin locks are memory-only, so a refusal at
shutdown reclaims them with the process; the lie about success was the defect.

Nothing ever removed finished automated operations from the queue's map.
popOperationForId is reached only from z_getoperationresult, so on a node running
autoshield every 25 blocks the map grew by one entry per round forever. The
schedulers now pop the operation they just cancelled. The worker already handles
a missing id ("cannot find operation in map, may have been removed",
asyncrpcqueue.cpp), and it releases lock_ before calling main(), so popping under
cs_wallet introduces no lock cycle.

The autoshield operation built its transaction against targetHeight_, the
enqueue-time height, while SetExpiryHeight and the network-upgrade straddle guard
both used tipHeight. Since the builder's height selects the consensus branch id,
the guard was checking a height the transaction was not signed against -- it could
not prevent the failure it exists to prevent. Now tipHeight throughout.

cancel() in the sweep, consolidation and autoshield operations set CANCELLED
unconditionally, dropping the base class's guard entirely. The schedulers cancel
the previous operation when they enqueue the next, so a round that had already
SUCCEEDED got its result relabelled as cancelled. Restored a narrower guard: still
cancellable while READY or EXECUTING (the base class refuses the latter, which
would defeat cancellation here), but a terminal state is left alone.

CommitAutomatedTx dumped the whole transaction to stderr on every commit,
duplicating the LogPrintf that CommitTransaction does one call later. ToString()
emits a line per input, so with the 400-input autoshield cap that was tens of KB
of stderr per round. Removed.

Also corrected a comment that credited the immature-coinbase exclusion to
fOnlySpendable (the argument is fOnlyConfirmed; the exclusion is unconditional in
AvailableCoins), and noted that AUTOSHIELD_CTXIN_P2SH_SIZE is a byte size that
merely happens to share the value 400 with the input cap.

Verified on an isolated regtest chain, 8/8: nine consecutive autoshield rounds
succeed after the builder-height change; the operation map stays at 1 entry across
all nine (it grew one per round before); stderr totals 1608 bytes for the whole
run with zero CommitAutomatedTx dumps, while debug.log still records all nine
commits via CommitTransaction; no round is relabelled cancelled; funds shield
correctly and no coin locks leak.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

src/wallet/wallet.cpp

@@ -651,11 +651,21 @@ void CWallet::RunSaplingSweep(int blockHeight) {
     std::shared_ptr<AsyncRPCOperation> lastOperation = q->getOperationForId(saplingSweepOperationId);
     if (lastOperation != nullptr) {
         lastOperation->cancel();
+        // Drop it from the queue's map as well. Nothing else ever removes these:
+        // popOperationForId is only reached from z_getoperationresult, so on a node
+        // running this every interval the map grew without bound.
+        q->popOperationForId(saplingSweepOperationId);
     }
     pendingSaplingSweepTxs.clear();
     std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sweep(blockHeight + 5));
     saplingSweepOperationId = operation->getId();
-    q->addOperation(operation);
+    if (!q->addOperation(operation)) {
+        // Queue is closing (shutdown). Release the flag we just set, or it stays
+        // set with no operation in flight and blocks every later round.
+        LogPrintf("%s: async queue is not accepting operations; skipping this round\n", __func__);
+        fSweepRunning = false;
+        return;
+    }
 }
 
 void CWallet::RunSaplingConsolidation(int blockHeight) {
@@ -699,11 +709,21 @@ void CWallet::RunSaplingConsolidation(int blockHeight) {
     std::shared_ptr<AsyncRPCOperation> lastOperation = q->getOperationForId(saplingConsolidationOperationId);
     if (lastOperation != nullptr) {
         lastOperation->cancel();
+        // Drop it from the queue's map as well. Nothing else ever removes these:
+        // popOperationForId is only reached from z_getoperationresult, so on a node
+        // running this every interval the map grew without bound.
+        q->popOperationForId(saplingConsolidationOperationId);
     }
     pendingSaplingConsolidationTxs.clear();
     std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_saplingconsolidation(blockHeight + 5));
     saplingConsolidationOperationId = operation->getId();
-    q->addOperation(operation);
+    if (!q->addOperation(operation)) {
+        // Queue is closing (shutdown). Release the flag we just set, or it stays
+        // set with no operation in flight and blocks every later round.
+        LogPrintf("%s: async queue is not accepting operations; skipping this round\n", __func__);
+        fConsolidationRunning = false;
+        return;
+    }
 }
 
 // Periodically drain matured transparent coinbase into a wallet-owned Sapling
@@ -754,16 +774,28 @@ void CWallet::RunAutoShieldCoinbase(int blockHeight) {
     std::shared_ptr<AsyncRPCOperation> lastOperation = q->getOperationForId(saplingAutoShieldOperationId);
     if (lastOperation != nullptr) {
         lastOperation->cancel();
+        // Drop it from the queue's map as well. Nothing else ever removes these:
+        // popOperationForId is only reached from z_getoperationresult, so on a node
+        // running this every interval the map grew without bound.
+        q->popOperationForId(saplingAutoShieldOperationId);
     }
     std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_autoshieldcoinbase(blockHeight + 5));
     saplingAutoShieldOperationId = operation->getId();
-    q->addOperation(operation);
+    if (!q->addOperation(operation)) {
+        // Queue is closing (shutdown). Release the flag we just set, or it stays
+        // set with no operation in flight and blocks every later round.
+        LogPrintf("%s: async queue is not accepting operations; skipping this round\n", __func__);
+        fAutoShieldRunning = false;
+        return;
+    }
 }
 
 bool CWallet::CommitAutomatedTx(const CTransaction& tx) {
   CWalletTx wtx(this, tx);
   CReserveKey reservekey(pwalletMain);
-  fprintf(stderr,"%s: %s\n",__func__,tx.ToString().c_str());
+  // No tx dump here: CommitTransaction already LogPrintf's the same wtx.ToString(),
+  // and ToString() emits a line per vin, so with the 400-input autoshield cap this
+  // printed tens of KB to stderr on every automated round.
   return CommitTransaction(wtx, reservekey);
 }