gtest: pin GetFilteredNotes behaviour before optimizing it

Adds test_wallet_notecache.cpp: a wallet with Sapling notes at several
depths (spent by a mined tx, spent by an unmined tx, locked, viewing-key
only, never mined, plus txs carrying no wallet notes), mined into a fake
active chain. The fixture undoes everything it puts into the process-wide
chain state (chainActive, mapBlockIndex, pcoinsTip) in TearDown, since all
gtest suites share one process.

LegacyFilteredNotes is the 69bb035e7 loop verbatim, with tx_height()
passed in so the minDepth > 1 branch can be modelled on a -txindex node.
FilteredNotesMatchLegacy compares the real walk against it entry by entry
and in order (z_sendmany sorts the result with an unstable sort) over
minDepth {0,1} x maxDepth x ignoreSpent x requireSpendingKey x
ignoreLocked x address filters. FixtureExercisesEveryFilter checks that
each filter really removes something, so the comparison can't pass
vacuously.

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

src/Makefile.gtest.include

@@ -18,7 +18,8 @@ hush_gtest_SOURCES = \
 	gtest/test_wallet_notedata.cpp \
 	gtest/test_net_closesocket.cpp \
 	gtest/test_fd_defense.cpp \
-	gtest/test_wallet_dbenv.cpp
+	gtest/test_wallet_dbenv.cpp \
+	gtest/test_wallet_notecache.cpp
 
 hush_gtest_CPPFLAGS = $(AM_CPPFLAGS) -DMULTICORE -fopenmp -DBINARY_OUTPUT -DCURVE_ALT_BN128 -DSTATIC $(BITCOIN_INCLUDES)
 hush_gtest_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)

src/gtest/test_wallet_notecache.cpp

@@ -0,0 +1,364 @@
+// Copyright (c) 2016-2024 The Hush developers
+// Distributed under the GPLv3 software license, see the accompanying
+// file COPYING or https://www.gnu.org/licenses/gpl-3.0.en.html
+
+#include <gtest/gtest.h>
+
+#include <deque>
+#include <functional>
+
+#include "chain.h"
+#include "coins.h"
+#include "main.h"
+#include "primitives/block.h"
+#include "primitives/transaction.h"
+#include "random.h"
+#include "wallet/wallet.h"
+#include "zcash/Address.hpp"
+#include "zcash/Note.hpp"
+#include "zcash/NoteEncryption.hpp"
+#include "zcash/zip32.h"
+
+int32_t hush_dpowconfs(int32_t height, int32_t numconfs);
+
+using namespace libzcash;
+
+namespace {
+
+typedef std::function<int(const uint256&)> TxHeightFn;
+
+// CWallet::GetFilteredNotes as of 69bb035e7, verbatim except that tx_height() is passed in. This is
+// the reference the optimized walk must reproduce entry for entry and in the same order (z_sendmany
+// sorts the result with an unstable sort, so the order is observable).
+void LegacyFilteredNotes(
+    CWallet& wallet,
+    std::vector<SaplingNoteEntry>& saplingEntries,
+    std::set<PaymentAddress>& filterAddresses,
+    int minDepth,
+    int maxDepth,
+    bool ignoreSpent,
+    bool requireSpendingKey,
+    bool ignoreLocked,
+    const TxHeightFn& tx_height)
+{
+    LOCK2(cs_main, wallet.cs_wallet);
+
+    for (auto & p : wallet.mapWallet) {
+        CWalletTx wtx = p.second;
+
+        // Filter the transactions before checking for notes
+        if (!CheckFinalTx(wtx) || wtx.GetBlocksToMaturity() > 0)
+            continue;
+
+        if (minDepth > 1) {
+            int nHeight    = tx_height(wtx.GetHash());
+            if ( nHeight == 0 ) {
+                continue;
+            }
+            int nDepth     = wtx.GetDepthInMainChain();
+            int dpowconfs  = hush_dpowconfs(nHeight,nDepth);
+            if ( dpowconfs < minDepth || dpowconfs > maxDepth) {
+                continue;
+            }
+        } else {
+            if ( wtx.GetDepthInMainChain() < minDepth ||
+                wtx.GetDepthInMainChain() > maxDepth) {
+                continue;
+            }
+        }
+
+        for (auto & pair : wtx.mapSaplingNoteData) {
+            SaplingOutPoint op = pair.first;
+            SaplingNoteData nd = pair.second;
+
+            auto maybe_pt = SaplingNotePlaintext::decrypt(
+                wtx.vShieldedOutput[op.n].encCiphertext,
+                nd.ivk,
+                wtx.vShieldedOutput[op.n].ephemeralKey,
+                wtx.vShieldedOutput[op.n].cm);
+            assert(static_cast<bool>(maybe_pt));
+            auto notePt = maybe_pt.get();
+
+            auto maybe_pa = nd.ivk.address(notePt.d);
+            assert(static_cast<bool>(maybe_pa));
+            auto pa = maybe_pa.get();
+
+            // skip notes which belong to a different payment address in the wallet
+            if (!(filterAddresses.empty() || filterAddresses.count(pa))) {
+                continue;
+            }
+
+            if (ignoreSpent && nd.nullifier && wallet.IsSaplingSpent(*nd.nullifier)) {
+                continue;
+            }
+
+            // skip notes which cannot be spent
+            if (requireSpendingKey) {
+                libzcash::SaplingIncomingViewingKey ivk;
+                libzcash::SaplingFullViewingKey fvk;
+                if (!(wallet.GetSaplingIncomingViewingKey(pa, ivk) &&
+                    wallet.GetSaplingFullViewingKey(ivk, fvk) &&
+                    wallet.HaveSaplingSpendingKey(fvk))) {
+                    continue;
+                }
+            }
+
+            // skip locked notes
+            if (ignoreLocked && wallet.IsLockedNote(op)) {
+                continue;
+            }
+
+            auto note = notePt.note(nd.ivk).get();
+            saplingEntries.push_back(SaplingNoteEntry {
+                op, pa, note, notePt.memo(), wtx.GetDepthInMainChain() });
+        }
+    }
+}
+
+std::array<unsigned char, HUSH_MEMO_SIZE> MemoFor(uint64_t value)
+{
+    std::array<unsigned char, HUSH_MEMO_SIZE> memo;
+    memo.fill(0);
+    std::string s = "memo-" + std::to_string(value);
+    memcpy(memo.data(), s.data(), s.size());
+    return memo;
+}
+
+// A real encrypted Sapling output to pa. Its proof and value commitment are never checked by the
+// wallet, so they are left empty / random.
+OutputDescription MakeOutput(const SaplingPaymentAddress& pa, uint64_t value)
+{
+    SaplingNote note(pa, value);
+    SaplingNotePlaintext pt(note, MemoFor(value));
+    auto enc = pt.encrypt(note.pk_d);
+    assert(static_cast<bool>(enc));
+    OutputDescription od;
+    od.cv = GetRandHash();
+    od.cm = note.cm().get();
+    od.ephemeralKey = enc->second.get_epk();
+    od.encCiphertext = enc->first;
+    return od;
+}
+
+CTransaction MakeTx(const std::vector<OutputDescription>& outputs, const std::vector<uint256>& spentNullifiers = {})
+{
+    CMutableTransaction mtx;
+    mtx.fOverwintered = true;
+    mtx.nVersion = SAPLING_TX_VERSION;
+    mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID;
+    mtx.vShieldedOutput = outputs;
+    for (const uint256& nf : spentNullifiers) {
+        SpendDescription sd;
+        sd.nullifier = nf;
+        mtx.vShieldedSpend.push_back(sd);
+    }
+    return CTransaction(mtx);
+}
+
+SaplingExtendedSpendingKey TestKey(uint32_t account)
+{
+    std::vector<unsigned char, secure_allocator<unsigned char>> raw(32, 0x5a);
+    HDSeed seed(raw);
+    return SaplingExtendedSpendingKey::Master(seed).Derive(account | ZIP32_HARDENED_KEY_LIMIT);
+}
+
+std::string Describe(int minDepth, int maxDepth, bool ignoreSpent, bool requireSpendingKey, bool ignoreLocked, const std::string& filter)
+{
+    return strprintf("minDepth=%d maxDepth=%d ignoreSpent=%d requireSpendingKey=%d ignoreLocked=%d filter=%s",
+                     minDepth, maxDepth, ignoreSpent, requireSpendingKey, ignoreLocked, filter);
+}
+
+void ExpectSameEntries(const std::vector<SaplingNoteEntry>& want, const std::vector<SaplingNoteEntry>& got, const std::string& ctx)
+{
+    ASSERT_EQ(want.size(), got.size()) << ctx;
+    for (size_t i = 0; i < want.size(); i++) {
+        EXPECT_TRUE(want[i].op == got[i].op) << ctx << " entry " << i;
+        EXPECT_TRUE(want[i].address == got[i].address) << ctx << " entry " << i;
+        EXPECT_TRUE(want[i].note.d == got[i].note.d) << ctx << " entry " << i;
+        EXPECT_EQ(want[i].note.pk_d, got[i].note.pk_d) << ctx << " entry " << i;
+        EXPECT_EQ(want[i].note.r, got[i].note.r) << ctx << " entry " << i;
+        EXPECT_EQ(want[i].note.value(), got[i].note.value()) << ctx << " entry " << i;
+        EXPECT_TRUE(want[i].memo == got[i].memo) << ctx << " entry " << i;
+        EXPECT_EQ(want[i].confirmations, got[i].confirmations) << ctx << " entry " << i;
+    }
+}
+
+}
+
+// A wallet with notes at several depths, spent and unspent, locked and unlocked, spendable and
+// viewing-key-only, plus txs that carry no wallet notes; mined into a fake active chain. Everything it
+// touches in the process-wide chain state (chainActive, mapBlockIndex, pcoinsTip) is undone in
+// TearDown, because every gtest suite runs in the same process.
+class WalletNoteCache : public ::testing::Test {
+protected:
+    static const int TIP_HEIGHT = 30;
+
+    CWallet wallet;
+    SaplingPaymentAddress paA, paB, paC;    // A and C spendable, B viewing key only
+    std::map<uint256, int> depthOf;         // txid -> depth to mine it at (absent: never mined)
+    std::vector<uint256> txids;
+    SaplingOutPoint lockedOp;
+
+    CCoinsView coinsBase;
+    CCoinsViewCache coins{&coinsBase};
+    CCoinsViewCache* savedCoinsTip = nullptr;
+    std::deque<CBlock> blocks;
+    std::deque<CBlockIndex> index;
+    std::map<uint256, int> minedHeight;     // txid -> height of the block holding it
+
+    void SetUp() override {
+        // tx_height()/GetTransaction dereference pcoinsTip; an empty view answers "not found".
+        savedCoinsTip = pcoinsTip;
+        pcoinsTip = &coins;
+
+        auto skA = TestKey(0), skB = TestKey(1), skC = TestKey(2);
+        paA = skA.DefaultAddress();
+        paB = skB.DefaultAddress();
+        paC = skC.DefaultAddress();
+        ASSERT_TRUE(wallet.AddSaplingZKey(skA, paA));
+        ASSERT_TRUE(wallet.AddSaplingIncomingViewingKey(skB.expsk.full_viewing_key().in_viewing_key(), paB));
+        ASSERT_TRUE(wallet.AddSaplingZKey(skC, paC));
+        SaplingPaymentAddress foreign = TestKey(9).DefaultAddress();   // not in the wallet
+
+        LOCK2(cs_main, wallet.cs_wallet);
+        AddTx(MakeTx({MakeOutput(paA, 1000), MakeOutput(paB, 2000)}), 1);
+        uint256 tx2 = AddTx(MakeTx({MakeOutput(paA, 3000)}), 5);
+        AddTx(MakeTx({MakeOutput(paA, 4000), MakeOutput(paC, 5000), MakeOutput(paB, 6000)}), 11);
+        AddTx(MakeTx({MakeOutput(paC, 7000)}), 21);
+        AddTx(MakeTx({MakeOutput(paA, 8000)}), -1);                      // never mined: depth -1
+        AddTx(MakeTx({MakeOutput(foreign, 50)}), 3);                     // no wallet notes at all
+
+        // A note spent by a mined tx, and one whose spending tx never confirmed.
+        const uint256 nfSpent = uint256S("5a1d"), nfPending = uint256S("9e9d");
+        uint256 tx6 = AddTx(MakeTx({MakeOutput(paA, 9000)}), 12);
+        uint256 tx7 = AddTx(MakeTx({MakeOutput(paC, 10000)}), 3);
+        wallet.mapWallet.at(tx6).mapSaplingNoteData.at(SaplingOutPoint(tx6, 0)).nullifier = nfSpent;
+        wallet.mapWallet.at(tx7).mapSaplingNoteData.at(SaplingOutPoint(tx7, 0)).nullifier = nfPending;
+        AddTx(MakeTx({}, {nfSpent}), 2);
+        AddTx(MakeTx({}, {nfPending}), -1);
+
+        lockedOp = SaplingOutPoint(tx2, 0);
+        wallet.LockNote(lockedOp);
+
+        Mine();
+    }
+
+    void TearDown() override {
+        LOCK(cs_main);
+        chainActive.SetTip(nullptr);
+        for (const CBlockIndex& bi : index) {
+            const uint256 hash = bi.GetBlockHash();
+            mapBlockIndex.erase(hash);
+        }
+        pcoinsTip = savedCoinsTip;
+    }
+
+    // Adds tx the way LoadWallet does (no wallet.dat), with the note data FindMySaplingNotes finds.
+    uint256 AddTx(const CTransaction& tx, int depth) {
+        CWalletTx wtx(&wallet, tx);
+        mapSaplingNoteData_t found = wallet.FindMySaplingNotes(tx).first;
+        wtx.SetSaplingNoteData(found);
+        EXPECT_TRUE(wallet.AddToWallet(wtx, true, nullptr));
+        if (depth > 0)
+            depthOf[tx.GetHash()] = depth;
+        txids.push_back(tx.GetHash());
+        return tx.GetHash();
+    }
+
+    // Builds an active chain 0..TIP_HEIGHT and puts each wallet tx into the block at its depth.
+    void Mine() {
+        for (int h = 0; h <= TIP_HEIGHT; h++) {
+            blocks.emplace_back();
+            CBlock& block = blocks.back();
+            block.nTime = 1700000000 + h;
+            block.nNonce = uint256S(strprintf("%x", h + 1));
+            if (h > 0)
+                block.hashPrevBlock = blocks[h - 1].GetHash();
+            for (const auto& d : depthOf) {
+                if (TIP_HEIGHT - d.second + 1 == h)
+                    block.vtx.push_back(wallet.mapWallet.at(d.first));
+            }
+            block.hashMerkleRoot = block.BuildMerkleTree();
+
+            index.emplace_back(block);
+            CBlockIndex& bi = index.back();
+            bi.SetHeight(h);
+            bi.pprev = h > 0 ? &index[h - 1] : nullptr;
+            BlockMap::iterator it = mapBlockIndex.insert(std::make_pair(block.GetHash(), &bi)).first;
+            bi.phashBlock = &it->first;
+        }
+        chainActive.SetTip(&index.back());
+        for (const auto& d : depthOf) {
+            int h = TIP_HEIGHT - d.second + 1;
+            wallet.mapWallet.at(d.first).SetMerkleBranch(blocks[h]);
+            minedHeight[d.first] = h;
+        }
+    }
+
+    // What tx_height() returns on a -txindex node: the height of the block holding the tx, 0 if none.
+    TxHeightFn TxIndexHeight() const {
+        return [this](const uint256& txid) {
+            auto it = minedHeight.find(txid);
+            return it == minedHeight.end() ? 0 : it->second;
+        };
+    }
+
+    std::vector<SaplingNoteEntry> Filtered(const std::set<PaymentAddress>& filter, int minDepth, int maxDepth,
+                                           bool ignoreSpent, bool requireSpendingKey, bool ignoreLocked) {
+        std::set<PaymentAddress> f = filter;
+        std::vector<SaplingNoteEntry> entries;
+        wallet.GetFilteredNotes(entries, f, minDepth, maxDepth, ignoreSpent, requireSpendingKey, ignoreLocked);
+        return entries;
+    }
+
+    std::vector<SaplingNoteEntry> Legacy(const std::set<PaymentAddress>& filter, int minDepth, int maxDepth,
+                                         bool ignoreSpent, bool requireSpendingKey, bool ignoreLocked) {
+        std::set<PaymentAddress> f = filter;
+        std::vector<SaplingNoteEntry> entries;
+        LegacyFilteredNotes(wallet, entries, f, minDepth, maxDepth, ignoreSpent, requireSpendingKey, ignoreLocked,
+                            TxIndexHeight());
+        return entries;
+    }
+
+    void ExpectEquivalent(const std::vector<int>& minDepths) {
+        const std::vector<std::pair<std::string, std::set<PaymentAddress>>> filters = {
+            {"none", {}}, {"A", {paA}}, {"B", {paB}}, {"A+C", {paA, paC}},
+        };
+        for (int minDepth : minDepths)
+        for (int maxDepth : {INT_MAX, 11})
+        for (bool ignoreSpent : {false, true})
+        for (bool requireSpendingKey : {false, true})
+        for (bool ignoreLocked : {false, true})
+        for (const auto& filter : filters) {
+            std::string ctx = Describe(minDepth, maxDepth, ignoreSpent, requireSpendingKey, ignoreLocked, filter.first);
+            ExpectSameEntries(Legacy(filter.second, minDepth, maxDepth, ignoreSpent, requireSpendingKey, ignoreLocked),
+                              Filtered(filter.second, minDepth, maxDepth, ignoreSpent, requireSpendingKey, ignoreLocked),
+                              ctx);
+        }
+    }
+};
+
+// The fixture must exercise every filter, or the equivalence check below could pass vacuously.
+TEST_F(WalletNoteCache, FixtureExercisesEveryFilter) {
+    // Mined, unspent notes: tx1 (2), tx2 (1), tx3 (3), tx4 (1), tx7 (1); tx6's note is spent.
+    EXPECT_EQ(8u, Filtered({}, 1, INT_MAX, true, false, false).size());
+    EXPECT_EQ(9u, Filtered({}, 1, INT_MAX, false, false, false).size()) << "ignoreSpent drops tx6's note";
+    EXPECT_EQ(8u, Filtered({}, 0, INT_MAX, true, false, false).size()) << "a never-mined tx is at depth -1, not 0";
+    EXPECT_EQ(6u, Filtered({}, 1, INT_MAX, true, true, false).size()) << "requireSpendingKey drops B's 2 notes";
+    EXPECT_EQ(7u, Filtered({}, 1, INT_MAX, true, false, true).size()) << "ignoreLocked drops tx2's note";
+    EXPECT_EQ(7u, Filtered({}, 1, 11, true, false, false).size()) << "maxDepth 11 drops tx4";
+    EXPECT_EQ(2u, Filtered({paB}, 1, INT_MAX, true, false, false).size());
+
+    auto onlyC = Filtered({paC}, 1, INT_MAX, true, false, false);
+    ASSERT_EQ(3u, onlyC.size());
+    for (const SaplingNoteEntry& e : onlyC) {
+        EXPECT_TRUE(e.address == paC);
+        EXPECT_TRUE(e.memo == MemoFor(e.note.value()));
+    }
+}
+
+// The walk returns exactly what the 69bb035e7 loop returned, in the same order, for every filter
+// combination.
+TEST_F(WalletNoteCache, FilteredNotesMatchLegacy) {
+    ExpectEquivalent({0, 1});
+}