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>

src/rpc/server.h

@@ -369,6 +369,7 @@ extern UniValue z_gettotalbalance(const UniValue& params, bool fHelp, const CPub
 extern UniValue z_mergetoaddress(const UniValue& params, bool fHelp, const CPubKey& mypk); // in rpcwallet.cpp
 extern UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk); // in rpcwallet.cpp
 extern UniValue z_sweepstatus(const UniValue& params, bool fHelp, const CPubKey& mypk); // in rpcwallet.cpp
+extern UniValue z_autoshieldstatus(const UniValue& params, bool fHelp, const CPubKey& mypk); // in rpcwallet.cpp
 extern UniValue z_consolidationstatus(const UniValue& params, bool fHelp, const CPubKey& mypk); // in rpcwallet.cpp
 extern UniValue z_shieldcoinbase(const UniValue& params, bool fHelp, const CPubKey& mypk); // in rpcwallet.cpp
 extern UniValue z_getoperationstatus(const UniValue& params, bool fHelp, const CPubKey& mypk); // in rpcwallet.cpp

src/wallet/asyncrpcoperation_autoshieldcoinbase.cpp

@@ -22,6 +22,18 @@ extern std::string randomSietchZaddr();
 
 // Serialized-size estimates for one spent input (kept in sync with rpcwallet.cpp)
 static const size_t AUTOSHIELD_CTXIN_DUST_SIZE = 148;
+// Every autoshield tx carries THREE Sapling OutputDescriptions -- the change
+// note to destZaddr plus the two Sietch dummies -- at ~948 bytes each. Reserving
+// 2000 for "header + sietch outputs" was ~900 bytes short before a single input
+// was counted, so a large enough round could build a tx over MAX_TX_SIZE.
+static const size_t AUTOSHIELD_SAPLING_OUTPUT_SIZE = 948;
+static const size_t AUTOSHIELD_TX_OVERHEAD = (3 * AUTOSHIELD_SAPLING_OUTPUT_SIZE) + 256;
+// Hard cap on inputs per round, mirroring z_shieldcoinbase's
+// SHIELD_COINBASE_DEFAULT_LIMIT. The byte estimate alone is not a safe bound:
+// with a P2PKH coinbase (-mineraddress) the 148-byte figure is exact rather than
+// conservative, so an under-estimate translates directly into an oversize tx.
+// The remainder is simply shielded on the next round.
+static const size_t AUTOSHIELD_MAX_INPUTS = 400;
 static const size_t AUTOSHIELD_CTXIN_P2SH_SIZE = 400;
 // Expire unmined autoshield txs after this many blocks, so a tx cannot straddle
 // a network-upgrade activation.
@@ -258,6 +270,27 @@ bool AsyncRPCOperation_autoshieldcoinbase::main_impl() {
     libzcash::SaplingPaymentAddress destZaddr;
     std::string destStr;
     std::vector<ShieldCoinbaseUTXO> inputs;
+
+    // Proof building below runs WITHOUT cs_wallet (deliberately, so wallet RPCs
+    // are not stalled), which leaves a multi-second window in which a manual
+    // z_shieldcoinbase or z_sendmany over the same miner address would re-select
+    // these same coinbase outputs. AvailableCoins honours IsLockedCoin, so lock
+    // them for the duration exactly as z_shieldcoinbase does. RAII because there
+    // are several early returns between here and commit, and a leaked lock would
+    // silently exclude those coins from every future round.
+    struct ScopedCoinLocks {
+        std::vector<COutPoint> locked;
+        ~ScopedCoinLocks() {
+            // A destructor is noexcept by default; letting the lock acquisition
+            // escape would turn a contended mutex into std::terminate.
+            try {
+                if (locked.empty()) return;
+                LOCK2(cs_main, pwalletMain->cs_wallet);
+                // UnlockCoin takes a non-const reference (upstream signature).
+                for (COutPoint& op : locked) pwalletMain->UnlockCoin(op);
+            } catch (...) {}
+        }
+    } coinLocks;
     CAmount shieldedValue      = 0;
     unsigned int max_tx_size   = MAX_TX_SIZE_AFTER_SAPLING;
 
@@ -280,7 +313,7 @@ bool AsyncRPCOperation_autoshieldcoinbase::main_impl() {
         // AvailableCoins with fOnlySpendable already excludes immature coinbase
         // (< COINBASE_MATURITY) and outputs we don't own, so external
         // -mineraddress / pool coinbase naturally yields zero inputs.
-        size_t estimatedTxSize = 2000; // header + sietch outputs headroom
+        size_t estimatedTxSize = AUTOSHIELD_TX_OVERHEAD;
         std::vector<COutput> vecOutputs;
         pwalletMain->AvailableCoins(vecOutputs, true, NULL, false, true);
         for (const COutput& out : vecOutputs) {
@@ -293,6 +326,11 @@ bool AsyncRPCOperation_autoshieldcoinbase::main_impl() {
             }
             size_t increase = (boost::get<CScriptID>(&address) != nullptr)
                                   ? AUTOSHIELD_CTXIN_P2SH_SIZE : AUTOSHIELD_CTXIN_DUST_SIZE;
+            if (inputs.size() >= AUTOSHIELD_MAX_INPUTS) {
+                LogPrintf("%s: reached per-round input cap (%d); deferring remaining coinbase to next round\n",
+                          opid, (int)AUTOSHIELD_MAX_INPUTS);
+                break;
+            }
             if (estimatedTxSize + increase >= max_tx_size) {
                 // Size-safe batch; the remainder is shielded next round.
                 LogPrintf("%s: reached per-tx size cap; deferring remaining coinbase to next round\n", opid);
@@ -305,6 +343,12 @@ bool AsyncRPCOperation_autoshieldcoinbase::main_impl() {
             inputs.push_back(utxo);
             shieldedValue += out.tx->vout[out.i].nValue;
         }
+
+        for (const ShieldCoinbaseUTXO& t : inputs) {
+            COutPoint outpt(t.txid, t.vout);
+            pwalletMain->LockCoin(outpt);
+            coinLocks.locked.push_back(outpt);
+        }
     }
 
     CAmount fee = pwalletMain->autoShieldFee;

src/wallet/rpcwallet.cpp

@@ -3345,6 +3345,83 @@ UniValue z_sweepstatus(const UniValue& params, bool fHelp, const CPubKey& mypk)
       return ret;
 }
 
+UniValue z_autoshieldstatus(const UniValue& params, bool fHelp, const CPubKey& mypk)
+{
+    if (!EnsureWalletIsAvailable(fHelp))
+        return NullUniValue;
+
+    if (fHelp || params.size() > 0)
+        throw runtime_error(
+            "z_autoshieldstatus\n"
+            "\nReport the state of automatic coinbase shielding: whether it is on, where it sends,\n"
+            "and -- when it is off -- why.\n"
+            "\nResult:\n"
+            "{\n"
+            "  \"autoshield\" : true|false,          (boolean) whether auto-shielding is enabled\n"
+            "  \"running\" : true|false,             (boolean) whether a round is in flight\n"
+            "  \"next_autoshield\" : n,              (numeric) height of the next round\n"
+            "  \"autoshieldinterval\" : n,           (numeric) blocks between rounds\n"
+            "  \"autoshieldaddress\" : \"zaddr\",     (string) resolved destination; empty until first resolved\n"
+            "  \"autoshieldfee\" : n,                (numeric) fee in puposhis\n"
+            "  \"autoshieldminutxos\" : n,           (numeric) minimum matured coinbase utxos per round\n"
+            "  \"hdseedorigin\" : n,                 (numeric) 0 unrecorded, 1 created, 2 restored, 3 retrofit, 4 unknown\n"
+            "  \"hdseedorigin_desc\" : \"...\",       (string) readable form of hdseedorigin\n"
+            "  \"seed_recoverable\" : true|false,    (boolean) whether a seed phrase can be exported\n"
+            "  \"disabled_reason\" : \"...\"          (string) why auto-shielding is not running, if it is not\n"
+            "}\n"
+            "\nExamples:\n"
+            + HelpExampleCli("z_autoshieldstatus", "")
+            + HelpExampleRpc("z_autoshieldstatus", "")
+        );
+
+    LOCK2(cs_main, pwalletMain->cs_wallet);
+
+    UniValue ret(UniValue::VOBJ);
+    ret.push_back(Pair("autoshield", pwalletMain->fAutoShieldEnabled));
+    ret.push_back(Pair("running", pwalletMain->fAutoShieldRunning));
+    ret.push_back(Pair("next_autoshield", pwalletMain->nextAutoShield));
+    ret.push_back(Pair("autoshieldinterval", pwalletMain->autoShieldInterval));
+    ret.push_back(Pair("autoshieldaddress", pwalletMain->autoShieldAddress));
+    ret.push_back(Pair("autoshieldfee", pwalletMain->autoShieldFee));
+    ret.push_back(Pair("autoshieldminutxos", pwalletMain->autoShieldMinUtxos));
+
+    int origin = pwalletMain->hdSeedOrigin;
+    std::string desc;
+    switch (origin) {
+        case CWallet::HDSEED_ORIGIN_CREATED:  desc = "created on an empty wallet"; break;
+        case CWallet::HDSEED_ORIGIN_RESTORED: desc = "restored from -mnemonic/-hdseed"; break;
+        case CWallet::HDSEED_ORIGIN_RETROFIT: desc = "retrofitted onto a pre-existing wallet"; break;
+        case CWallet::HDSEED_ORIGIN_UNKNOWN:  desc = "predates provenance recording"; break;
+        default:                              desc = "not yet recorded"; break;
+    }
+    ret.push_back(Pair("hdseedorigin", origin));
+    ret.push_back(Pair("hdseedorigin_desc", desc));
+    ret.push_back(Pair("seed_recoverable", pwalletMain->IsMnemonicSeed()));
+
+    // Say why it is off. A silent "false" is exactly what made the destination
+    // un-inspectable in the first place.
+    std::string why = "";
+    if (!pwalletMain->fAutoShieldEnabled) {
+        if (origin != CWallet::HDSEED_ORIGIN_CREATED && origin != CWallet::HDSEED_ORIGIN_RESTORED)
+            why = "HD seed origin is not known-recoverable; back the seed up and pass -autoshield=1";
+        else
+            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 = "";
+    }
+    ret.push_back(Pair("disabled_reason", why));
+
+    return ret;
+}
+
 UniValue z_listreceivedaddress(const UniValue& params, bool fHelp,const CPubKey&)
 {
   if (!EnsureWalletIsAvailable(fHelp))
@@ -6349,6 +6426,7 @@ static const CRPCCommand commands[] =
     { "wallet",             "z_gettotalbalance",        &z_gettotalbalance,        false },
     { "wallet",             "z_mergetoaddress",         &z_mergetoaddress,         false },
     { "wallet",             "z_sweepstatus",            &z_sweepstatus,            true  },
+    { "wallet",             "z_autoshieldstatus",       &z_autoshieldstatus,       true  },
     { "wallet",             "z_consolidationstatus",    &z_consolidationstatus,    true  },
     { "wallet",             "z_sendmany",               &z_sendmany,               false },
     { "wallet",             "z_shieldcoinbase",         &z_shieldcoinbase,         false },

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<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;
     }

src/wallet/walletdb.cpp

@@ -34,6 +34,15 @@
 #include <boost/scoped_ptr.hpp>
 #include <boost/thread.hpp>
 
+// Out-of-line definitions for CHDChain's in-class static constants. These are
+// only initialised in the class body, so any ODR use -- binding one to a const
+// reference, which is exactly what gtest's EXPECT_*/ASSERT_* macros do -- needs
+// a definition or the link fails. hush-gtest hit this on VERSION_HD_MNEMONIC.
+const int CHDChain::VERSION_HD_BASE;
+const int CHDChain::VERSION_HD_TRANSPARENT;
+const int CHDChain::VERSION_HD_MNEMONIC;
+const int CHDChain::CURRENT_VERSION;
+
 using namespace std;
 
 static uint64_t nAccountingEntryNumber = 0;