wallet: take note heights from the wallet's depth, not tx_height()

For minDepth > 1, GetFilteredNotes located each wallet tx's block with
tx_height(), which calls GetTransaction and re-reads the tx from disk
through the txindex. It did this once per wallet tx, transparent ones
included, while holding LOCK2. z_listunspent and z_listreceivedbyaddress
did it once more per returned note. Without -txindex, GetTransaction can
only find a tx that still has unspent transparent outputs. tx_height()
then returned 0 for every fully shielded tx, and the tx was skipped. So
on a -txindex=0 node, sweep and consolidation (minDepth 11) and any
minconf >= 2 call never saw those notes.

The wallet already has the depth, and for an active-chain tx
GetDepthInMainChain is chainActive.Height() - height + 1. The new
HeightFromDepth() inverts that (0 when unconfirmed or off the active
chain) and replaces tx_height() at all three sites. The height only
feeds the == 0 skip and hush_dpowconfs. Both sides hold cs_main for the
whole call, so the tip can't move between the walk and the RPC tail.

On a -txindex node the results are identical: the txindex block and
wtx.hashBlock are the same block. The extended equivalence test now
also covers minDepth 2/11/12/22. On -txindex=0 nodes, fully shielded
notes now show up for minDepth > 1. With -zsweep or -consolidation
enabled, those notes will now be swept and consolidated, so init logs
that once. MinDepthAboveOneUsesWalletHeight pins the new behaviour and
shows that the old path returned nothing without a txindex.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/gtest/test_wallet_notecache.cpp

@@ -20,6 +20,7 @@
 #include "zcash/zip32.h"
 
 int32_t hush_dpowconfs(int32_t height, int32_t numconfs);
+int tx_height(const uint256 &hash);
 
 using namespace libzcash;
 
@@ -191,7 +192,7 @@ void ExpectSameEntries(const std::vector<SaplingNoteEntry>& want, const std::vec
 // TearDown, because every gtest suite runs in the same process.
 class WalletNoteCache : public ::testing::Test {
 protected:
-    static const int TIP_HEIGHT = 30;
+    static constexpr int TIP_HEIGHT = 30;
 
     CWallet wallet;
     SaplingPaymentAddress paA, paB, paC;    // A and C spendable, B viewing key only
@@ -361,10 +362,39 @@ TEST_F(WalletNoteCache, FixtureExercisesEveryFilter) {
     }
 }
 
-// The walk returns exactly what the 69bb035e7 loop returned, in the same order, for every filter
-// combination.
+// The walk returns exactly what the 69bb035e7 loop returned on a -txindex node, in the same order,
+// for every filter combination. minDepth 11/12/22 sit on the fixture's depth-11 and depth-21 txs.
 TEST_F(WalletNoteCache, FilteredNotesMatchLegacy) {
-    ExpectEquivalent({0, 1});
+    ExpectEquivalent({0, 1, 2, 11, 12, 22});
+}
+
+// minDepth > 1 (sweep and consolidation use 11) locates each tx's block from its depth. The old
+// tx_height() re-read the tx through the txindex, and without one (as here, and on a -txindex=0
+// node) it never found a fully shielded tx, so nothing was returned at all.
+TEST_F(WalletNoteCache, MinDepthAboveOneUsesWalletHeight) {
+    // Unspent notes at depth >= 11: tx3's 3 (depth 11) and tx4's 1 (depth 21); tx6 (12) is spent.
+    EXPECT_EQ(4u, Filtered({}, 11, INT_MAX, true, false, false).size());
+    EXPECT_EQ(5u, Filtered({}, 11, INT_MAX, false, false, false).size());
+    EXPECT_EQ(1u, Filtered({}, 12, INT_MAX, true, false, false).size()) << "depth 11 is below minDepth 12";
+    EXPECT_EQ(3u, Filtered({}, 11, 11, true, false, false).size()) << "depth 21 is above maxDepth 11";
+    EXPECT_EQ(0u, Filtered({}, 22, INT_MAX, false, false, false).size());
+    for (const SaplingNoteEntry& e : Filtered({}, 2, INT_MAX, false, false, false))
+        EXPECT_GE(e.confirmations, 2) << "unmined and shallower txs stay out";
+
+    std::set<PaymentAddress> none;
+    std::vector<SaplingNoteEntry> legacy;
+    LegacyFilteredNotes(wallet, legacy, none, 11, INT_MAX, true, false, false, tx_height);
+    EXPECT_EQ(0u, legacy.size()) << "the old code found nothing here without a txindex";
+}
+
+TEST_F(WalletNoteCache, HeightFromDepth) {
+    LOCK(cs_main);
+    EXPECT_EQ(TIP_HEIGHT, HeightFromDepth(1));
+    EXPECT_EQ(TIP_HEIGHT - 10, HeightFromDepth(11));
+    EXPECT_EQ(0, HeightFromDepth(0)) << "mempool";
+    EXPECT_EQ(0, HeightFromDepth(-1)) << "not in the chain or mempool";
+    for (const auto& d : depthOf)
+        EXPECT_EQ(minedHeight.at(d.first), HeightFromDepth(wallet.mapWallet.at(d.first).GetDepthInMainChain()));
 }
 
 // Two notes recorded under A's viewing key that it can't actually decrypt (real outputs to someone

src/init.cpp

@@ -2504,6 +2504,14 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
             }
         }
 
+        // Sweep and consolidation pick notes with GetFilteredNotes at minDepth > 1. Older versions
+        // located each note's block by re-reading its tx through the txindex, so on a -txindex=0 node
+        // every fully shielded tx was skipped and these never moved those notes. Now they will.
+        if (!fTxIndex && (pwalletMain->fSweepEnabled || pwalletMain->fSaplingConsolidationEnabled)) {
+            LogPrintf("%s: -txindex=0 with -zsweep/-consolidation: shielded notes that versions before this "
+                      "fix never saw on such nodes are now included in sweeps and consolidations\n", __func__);
+        }
+
         //Set Automatic Coinbase Shielding (default ON, conditional: self-guards
         //on nodes where it cannot act - no owned coinbase, external mineraddress,
         //or locked wallet). Closes the transparent-coinbase leak for miners.

src/wallet/rpcwallet.cpp

@@ -4309,7 +4309,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp, const CPubKey& mypk)
         for (auto & entry : saplingEntries) {
             UniValue obj(UniValue::VOBJ);
 
-            int nHeight   = tx_height(entry.op.hash);
+            int nHeight   = HeightFromDepth(entry.confirmations);
             int dpowconfs = hush_dpowconfs(nHeight, entry.confirmations);
 
             // Only return notarized results when minconf>1
@@ -4642,7 +4642,7 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp, const CPubK
     if (boost::get<libzcash::SaplingPaymentAddress>(&zaddr) != nullptr) {
         for (SaplingNoteEntry & entry : saplingEntries) {
             UniValue obj(UniValue::VOBJ);
-            int nHeight   = tx_height(entry.op.hash);
+            int nHeight   = HeightFromDepth(entry.confirmations);
             int dpowconfs = hush_dpowconfs(nHeight, entry.confirmations);
             // Only return notarized results when minconf>1
             if (nMinDepth > 1 && dpowconfs == 1)

src/wallet/wallet.cpp

@@ -76,7 +76,6 @@ bool fPayAtLeastCustomFee = true;
 CBlockIndex *hush_chainactive(int32_t height);
 extern int32_t HUSH_LOADINGBLOCKS;
 int32_t hush_dpowconfs(int32_t height,int32_t numconfs);
-int tx_height( const uint256 &hash );
 bool fTxDeleteEnabled = false;
 bool fTxConflictDeleteEnabled = false;
 int fDeleteInterval = DEFAULT_TX_DELETE_INTERVAL;
@@ -5715,6 +5714,16 @@ bool CMerkleTx::AcceptToMemoryPool(bool fLimitFree, bool fRejectAbsurdFee)
     return ::AcceptToMemoryPool(mempool, state, *this, fLimitFree, NULL, fRejectAbsurdFee);
 }
 
+// Replaces tx_height() on the wallet's note paths. tx_height() re-read the tx from disk through
+// GetTransaction, once per wallet tx (or per returned note) under LOCK2, and on a -txindex=0 node it
+// never found a fully shielded tx, so those notes were silently skipped. The depth already locates
+// the block: GetDepthInMainChain is chainActive.Height() - height + 1 for an active-chain tx.
+int HeightFromDepth(int nDepth)
+{
+    AssertLockHeld(cs_main);
+    return nDepth > 0 ? chainActive.Height() - nDepth + 1 : 0;
+}
+
 /**
  * Find notes in the wallet filtered by payment address, min depth and ability to spend.
  * These notes are decrypted and added to the output parameter vector, outEntries.
@@ -5773,7 +5782,7 @@ void CWallet::GetFilteredNotes(
         // cs_main is held, so the depth can't change during the walk.
         const int nDepth = wtx.GetDepthInMainChain();
         if (minDepth > 1) {
-            int nHeight    = tx_height(wtx.GetHash());
+            int nHeight    = HeightFromDepth(nDepth);
             if ( nHeight == 0 ) {
                 continue;
             }

src/wallet/wallet.h

@@ -366,6 +366,11 @@ struct SaplingNoteEntry
     int confirmations;
 };
 
+/** Height of the active-chain block holding a wallet tx that is nDepth deep (as GetDepthInMainChain
+ *  returns it, and SaplingNoteEntry::confirmations holds it), or 0 when the tx is unconfirmed or not
+ *  in the active chain. Caller holds cs_main. */
+int HeightFromDepth(int nDepth);
+
 /** A transaction with a merkle branch linking it to the block chain. */
 class CMerkleTx : public CTransaction
 {