Remove `RawAddress`

This adds a new `AddrSet` type for use in note retrieval
as a filter, in place of a heterogeneous list of `RawAddress`
values. `RawAddress` will complicate the handling of addresses
within the wallet after the addition of unified addresses,
because it does not contain transparent receiver types, and
if we retain this polymorphism it means a lot of invalid states
are represented in places we don't want them to be. It's better
to figure out what types of addresses we're working with as soon
as possible after parsing, and use monomorphic calls from there
on in.

qa/rpc-tests/wallet_shieldingcoinbase.py

@@ -151,7 +151,7 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework):
             results = self.nodes[1].z_listunspent(1, 999, False, [myzaddr])
         except JSONRPCException as e:
             errorString = e.error['message']
-        assert_equal("Invalid parameter, spending key for address does not belong to wallet" in errorString, True)
+        assert_equal("Invalid parameter, spending key for an address does not belong to the wallet.", errorString)
 
         # Verify that debug=zrpcunsafe logs params, and that full txid is associated with opid
         initialized_line = check_node_log(self, 0, myopid + ": z_sendmany initialized", False)

src/miner.h

@@ -45,25 +45,12 @@ public:
         return addr;
     }
     std::optional<MinerAddress> operator()(const libzcash::UnifiedAddress &addr) const {
-        auto recipient = RecipientForPaymentAddress()(addr);
-        if (recipient.has_value()) {
-            // This looks like a recursive call, but we are actually calling
-            // ExtractMinerAddress with a different type:
-            // - libzcash::PaymentAddress has a libzcash::UnifiedAddress
-            //   alternative, which invokes this method.
-            // - RecipientForPaymentAddress() returns libzcash::RawAddress,
-            //   which does not have a libzcash::UnifiedAddress alternative.
-            //
-            // This works because std::visit does not require the visitor to
-            // solely match the std::variant, only that it can handle all of
-            // the variant's alternatives.
-            return std::visit(ExtractMinerAddress(), recipient.value());
-        } else {
-            // Either the UA only contains unknown shielded receivers (unlikely that we
-            // wouldn't know about them), or it only contains transparent receivers
-            // (which are invalid).
-            return std::nullopt;
+        for (const auto& receiver: addr) {
+            if (std::holds_alternative<libzcash::SaplingPaymentAddress>(receiver)) {
+                return std::get<libzcash::SaplingPaymentAddress>(receiver);
+            }
         }
+        return std::nullopt;
     }
 };
 

src/rust/src/zip339_ffi.rs

@@ -63,7 +63,7 @@ pub extern "C" fn zip339_free_phrase(phrase: *const c_char) {
     if !phrase.is_null() {
         unsafe {
             // It is correct to cast away const here; the memory is not actually immutable.
-            CString::from_raw(phrase as *mut c_char);
+            drop(CString::from_raw(phrase as *mut c_char));
         }
     }
 }

src/wallet/asyncrpcoperation_sendmany.cpp

@@ -935,7 +935,11 @@ bool AsyncRPCOperation_sendmany::find_unspent_notes() {
     std::vector<SaplingNoteEntry> saplingEntries;
     // TODO: move this to the caller
     auto zaddr = KeyIO(Params()).DecodePaymentAddress(fromaddress_);
-    pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, zaddr, mindepth_);
+    std::optional<AddrSet> noteFilter = std::nullopt;
+    if (zaddr.has_value()) {
+        noteFilter = AddrSet::ForPaymentAddresses(std::vector({zaddr.value()}));
+    }
+    pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noteFilter, mindepth_);
 
     // If using the TransactionBuilder, we only want Sapling notes.
     // If not using it, we only want Sprout notes.

src/wallet/rpcwallet.cpp

@@ -1987,7 +1987,7 @@ UniValue settxfee(const UniValue& params, bool fHelp)
     return true;
 }
 
-CAmount getBalanceZaddr(std::optional<libzcash::RawAddress> address, int minDepth = 1, int maxDepth = INT_MAX, bool ignoreUnspendable=true);
+CAmount getBalanceZaddr(std::optional<libzcash::PaymentAddress> address, int minDepth = 1, int maxDepth = INT_MAX, bool ignoreUnspendable=true);
 
 UniValue getwalletinfo(const UniValue& params, bool fHelp)
 {
@@ -2241,8 +2241,6 @@ UniValue z_listunspent(const UniValue& params, bool fHelp)
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Maximum number of confirmations must be greater or equal to the minimum number of confirmations");
     }
 
-    std::set<libzcash::RawAddress> zaddrs = {};
-
     bool fIncludeWatchonly = false;
     if (params.size() > 2) {
         fIncludeWatchonly = params[2].get_bool();
@@ -2250,109 +2248,93 @@ UniValue z_listunspent(const UniValue& params, bool fHelp)
 
     LOCK2(cs_main, pwalletMain->cs_wallet);
 
+    std::optional<AddrSet> noteFilter = std::nullopt;
+    std::set<std::pair<libzcash::SproutPaymentAddress, uint256>> sproutNullifiers;
+    std::set<std::pair<libzcash::SaplingPaymentAddress, uint256>> saplingNullifiers;
+
     KeyIO keyIO(Params());
     // User has supplied zaddrs to filter on
     if (params.size() > 3) {
         UniValue addresses = params[3].get_array();
-        if (addresses.size()==0)
+        if (addresses.size() == 0) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, addresses array is empty.");
-
-        // Keep track of addresses to spot duplicates
-        set<std::string> setAddress;
+        }
 
         // Sources
+        std::vector<libzcash::PaymentAddress> sourceAddrs;
         for (const UniValue& o : addresses.getValues()) {
             if (!o.isStr()) {
                 throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected string");
             }
-            string address = o.get_str();
-            auto zaddr = keyIO.DecodePaymentAddress(address);
+
+            auto zaddr = keyIO.DecodePaymentAddress(o.get_str());
             if (!zaddr.has_value()) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, address is not a valid zaddr: ") + address);
+                throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, not a valid Zcash address: ") + o.get_str());
             }
 
-            // We want to return unspent notes corresponding to any receiver within a
-            // Unified Address.
-            for (const auto ra : std::visit(GetRawShieldedAddresses(), zaddr.value())) {
-                bool hasSpendingKey = std::visit(match {
-                    [&](const SaplingPaymentAddress& addr) {
-                        return pwalletMain->HaveSaplingSpendingKeyForAddress(addr);
-                    },
-                    [&](const SproutPaymentAddress& addr) {
-                        return pwalletMain->HaveSproutSpendingKey(addr);
-                    }
-                }, ra);
-
-                // If we don't include watchonly addresses, we must reject any address
-                // for which we do not have the spending key.
-                if (!fIncludeWatchonly && !hasSpendingKey) {
-                    throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address);
-                }
-
-                zaddrs.insert(ra);
-            }
-
-            if (setAddress.count(address)) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, duplicated address: ") + address);
-            }
-            setAddress.insert(address);
+            sourceAddrs.push_back(zaddr.value());
         }
-    }
-    else {
+
+        noteFilter = AddrSet::ForPaymentAddresses(sourceAddrs);
+        sproutNullifiers = pwalletMain->GetSproutNullifiers(noteFilter.value().GetSproutAddresses());
+        saplingNullifiers = pwalletMain->GetSaplingNullifiers(noteFilter.value().GetSaplingAddresses());
+
+        // If we don't include watchonly addresses, we must reject any address
+        // for which we do not have the spending key.
+        if (!fIncludeWatchonly && !pwalletMain->HasSpendingKeys(noteFilter.value())) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for an address does not belong to the wallet."));
+        }
+    } else {
         // User did not provide zaddrs, so use default i.e. all addresses
         std::set<libzcash::SproutPaymentAddress> sproutzaddrs = {};
         pwalletMain->GetSproutPaymentAddresses(sproutzaddrs);
+        sproutNullifiers = pwalletMain->GetSproutNullifiers(sproutzaddrs);
 
         // Sapling support
         std::set<libzcash::SaplingPaymentAddress> saplingzaddrs = {};
         pwalletMain->GetSaplingPaymentAddresses(saplingzaddrs);
-
-        zaddrs.insert(sproutzaddrs.begin(), sproutzaddrs.end());
-        zaddrs.insert(saplingzaddrs.begin(), saplingzaddrs.end());
+        saplingNullifiers = pwalletMain->GetSaplingNullifiers(saplingzaddrs);
     }
 
     UniValue results(UniValue::VARR);
 
-    if (zaddrs.size() > 0) {
-        std::vector<SproutNoteEntry> sproutEntries;
-        std::vector<SaplingNoteEntry> saplingEntries;
-        pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, zaddrs, nMinDepth, nMaxDepth, true, !fIncludeWatchonly, false);
-        auto nullifierSet = pwalletMain->GetNullifiersForAddresses(zaddrs);
+    std::vector<SproutNoteEntry> sproutEntries;
+    std::vector<SaplingNoteEntry> saplingEntries;
+    pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noteFilter, nMinDepth, nMaxDepth, true, !fIncludeWatchonly, false);
 
-        for (auto & entry : sproutEntries) {
-            UniValue obj(UniValue::VOBJ);
-            obj.pushKV("txid", entry.jsop.hash.ToString());
-            obj.pushKV("jsindex", (int)entry.jsop.js );
-            obj.pushKV("jsoutindex", (int)entry.jsop.n);
-            obj.pushKV("confirmations", entry.confirmations);
-            bool hasSproutSpendingKey = pwalletMain->HaveSproutSpendingKey(entry.address);
-            obj.pushKV("spendable", hasSproutSpendingKey);
-            obj.pushKV("address", keyIO.EncodePaymentAddress(entry.address));
-            obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value())));
-            std::string data(entry.memo.begin(), entry.memo.end());
-            obj.pushKV("memo", HexStr(data));
-            if (hasSproutSpendingKey) {
-                obj.pushKV("change", pwalletMain->IsNoteSproutChange(nullifierSet, entry.address, entry.jsop));
-            }
-            results.push_back(obj);
+    for (auto & entry : sproutEntries) {
+        UniValue obj(UniValue::VOBJ);
+        obj.pushKV("txid", entry.jsop.hash.ToString());
+        obj.pushKV("jsindex", (int)entry.jsop.js );
+        obj.pushKV("jsoutindex", (int)entry.jsop.n);
+        obj.pushKV("confirmations", entry.confirmations);
+        bool hasSproutSpendingKey = pwalletMain->HaveSproutSpendingKey(entry.address);
+        obj.pushKV("spendable", hasSproutSpendingKey);
+        obj.pushKV("address", keyIO.EncodePaymentAddress(entry.address));
+        obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value())));
+        std::string data(entry.memo.begin(), entry.memo.end());
+        obj.pushKV("memo", HexStr(data));
+        if (hasSproutSpendingKey) {
+            obj.pushKV("change", pwalletMain->IsNoteSproutChange(sproutNullifiers, entry.address, entry.jsop));
         }
+        results.push_back(obj);
+    }
 
-        for (auto & entry : saplingEntries) {
-            UniValue obj(UniValue::VOBJ);
-            obj.pushKV("txid", entry.op.hash.ToString());
-            obj.pushKV("outindex", (int)entry.op.n);
-            obj.pushKV("confirmations", entry.confirmations);
-            bool hasSaplingSpendingKey = pwalletMain->HaveSaplingSpendingKeyForAddress(entry.address);
-            obj.pushKV("spendable", hasSaplingSpendingKey);
-            // TODO: If we found this entry via a UA, show that instead.
-            obj.pushKV("address", keyIO.EncodePaymentAddress(entry.address));
-            obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); // note.value() is equivalent to plaintext.value()
-            obj.pushKV("memo", HexStr(entry.memo));
-            if (hasSaplingSpendingKey) {
-                obj.pushKV("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op));
-            }
-            results.push_back(obj);
+    for (auto & entry : saplingEntries) {
+        UniValue obj(UniValue::VOBJ);
+        obj.pushKV("txid", entry.op.hash.ToString());
+        obj.pushKV("outindex", (int)entry.op.n);
+        obj.pushKV("confirmations", entry.confirmations);
+        bool hasSaplingSpendingKey = pwalletMain->HaveSaplingSpendingKeyForAddress(entry.address);
+        obj.pushKV("spendable", hasSaplingSpendingKey);
+        // TODO: If we found this entry via a UA, show that instead.
+        obj.pushKV("address", keyIO.EncodePaymentAddress(entry.address));
+        obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); // note.value() is equivalent to plaintext.value()
+        obj.pushKV("memo", HexStr(entry.memo));
+        if (hasSaplingSpendingKey) {
+            obj.pushKV("change", pwalletMain->IsNoteSaplingChange(saplingNullifiers, entry.address, entry.op));
         }
+        results.push_back(obj);
     }
 
     return results;
@@ -3053,18 +3035,18 @@ CAmount getBalanceTaddr(std::string transparentAddress, int minDepth=1, bool ign
     return balance;
 }
 
-CAmount getBalanceZaddr(std::optional<libzcash::RawAddress> address, int minDepth, int maxDepth, bool ignoreUnspendable) {
+CAmount getBalanceZaddr(std::optional<libzcash::PaymentAddress> address, int minDepth, int maxDepth, bool ignoreUnspendable) {
     CAmount balance = 0;
     std::vector<SproutNoteEntry> sproutEntries;
     std::vector<SaplingNoteEntry> saplingEntries;
     LOCK2(cs_main, pwalletMain->cs_wallet);
 
-    std::set<libzcash::RawAddress> filterAddresses;
-    if (address) {
-        filterAddresses.insert(address.value());
+    std::optional<AddrSet> noteFilter = std::nullopt;
+    if (address.has_value()) {
+        noteFilter = AddrSet::ForPaymentAddresses(std::vector({address.value()}));
     }
 
-    pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, filterAddresses, minDepth, maxDepth, true, ignoreUnspendable);
+    pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noteFilter, minDepth, maxDepth, true, ignoreUnspendable);
     for (auto & entry : sproutEntries) {
         balance += CAmount(entry.note.value());
     }
@@ -3151,7 +3133,8 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp)
     UniValue result(UniValue::VARR);
     std::vector<SproutNoteEntry> sproutEntries;
     std::vector<SaplingNoteEntry> saplingEntries;
-    pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, decoded, nMinDepth, false, false);
+    auto noteFilter = AddrSet::ForPaymentAddresses(std::vector({decoded.value()}));
+    pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noteFilter, nMinDepth, INT_MAX, false, false);
 
     std::visit(match {
         [&](const CKeyID& addr) {
@@ -3162,7 +3145,10 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp)
         },
         [&](const libzcash::SproutPaymentAddress& addr) {
             bool hasSpendingKey = pwalletMain->HaveSproutSpendingKey(addr);
-            auto nullifierSet = pwalletMain->GetNullifiersForAddresses({addr});
+            std::set<std::pair<libzcash::SproutPaymentAddress, uint256>> nullifierSet;
+            if (hasSpendingKey) {
+                nullifierSet = pwalletMain->GetSproutNullifiers({addr});
+            }
             for (const SproutNoteEntry& entry : sproutEntries) {
                 UniValue obj(UniValue::VOBJ);
                 obj.pushKV("txid", entry.jsop.hash.ToString());
@@ -3187,7 +3173,10 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp)
         },
         [&](const libzcash::SaplingPaymentAddress& addr) {
             bool hasSpendingKey = pwalletMain->HaveSaplingSpendingKeyForAddress(addr);
-            auto nullifierSet = pwalletMain->GetNullifiersForAddresses({addr});
+            std::set<std::pair<libzcash::SaplingPaymentAddress, uint256>> nullifierSet;
+            if (hasSpendingKey) {
+                nullifierSet = pwalletMain->GetSaplingNullifiers({addr});
+            }
             for (const SaplingNoteEntry& entry : saplingEntries) {
                 UniValue obj(UniValue::VOBJ);
                 obj.pushKV("txid", entry.op.hash.ToString());
@@ -3273,8 +3262,7 @@ UniValue z_getbalance(const UniValue& params, bool fHelp)
         nBalance = getBalanceTaddr(fromaddress, nMinDepth, false);
     } else {
         // TODO: Return an error if a UA is provided (once we support UAs).
-        auto zaddr = std::visit(RecipientForPaymentAddress(), pa.value()).value();
-        nBalance = getBalanceZaddr(zaddr, nMinDepth, INT_MAX, false);
+        nBalance = getBalanceZaddr(pa, nMinDepth, INT_MAX, false);
     }
 
     // inZat
@@ -4069,10 +4057,9 @@ UniValue z_getmigrationstatus(const UniValue& params, bool fHelp) {
     {
         std::vector<SproutNoteEntry> sproutEntries;
         std::vector<SaplingNoteEntry> saplingEntries;
-        std::set<libzcash::RawAddress> noFilter;
         // Here we are looking for any and all Sprout notes for which we have the spending key, including those
         // which are locked and/or only exist in the mempool, as they should be included in the unmigrated amount.
-        pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noFilter, 0, INT_MAX, true, true, false);
+        pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 0, INT_MAX, true, true, false);
         CAmount unmigratedAmount = 0;
         for (const auto& sproutEntry : sproutEntries) {
             unmigratedAmount += sproutEntry.note.value();
@@ -4432,8 +4419,8 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
     bool useAnyUTXO = false;
     bool useAnySprout = false;
     bool useAnySapling = false;
-    std::set<CTxDestination> taddrs = {};
-    std::set<libzcash::RawAddress> zaddrs = {};
+    std::set<CTxDestination> taddrs;
+    std::vector<libzcash::PaymentAddress> zaddrs;
 
     UniValue addresses = params[0].get_array();
     if (addresses.size()==0)
@@ -4468,13 +4455,9 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
             } else {
                 auto zaddr = keyIO.DecodePaymentAddress(address);
                 if (zaddr.has_value()) {
-                    // We want to merge notes corresponding to any receiver within a
-                    // Unified Address.
-                    for (const libzcash::RawAddress& ra : std::visit(GetRawShieldedAddresses(), zaddr.value())) {
-                        zaddrs.insert(ra);
-                        if (std::holds_alternative<libzcash::SaplingPaymentAddress>(ra)) {
-                            isFromNonSprout = true;
-                        }
+                    zaddrs.push_back(zaddr.value());
+                    if (std::holds_alternative<libzcash::SaplingPaymentAddress>(zaddr.value())) {
+                        isFromNonSprout = true;
                     }
                 } else {
                     throw JSONRPCError(RPC_INVALID_PARAMETER, string("Unknown address format: ") + address);
@@ -4651,7 +4634,11 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
         // Get available notes
         std::vector<SproutNoteEntry> sproutEntries;
         std::vector<SaplingNoteEntry> saplingEntries;
-        pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, zaddrs);
+        std::optional<AddrSet> noteFilter =
+            useAnySprout || useAnySapling ?
+                std::nullopt :
+                std::optional(AddrSet::ForPaymentAddresses(zaddrs));
+        pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noteFilter);
 
         // If Sapling is not active, do not allow sending from a sapling addresses.
         if (!saplingActive && saplingEntries.size() > 0) {

src/wallet/wallet.cpp

@@ -684,49 +684,58 @@ void CWallet::SetBestChain(const CBlockLocator& loc)
     SetBestChainINTERNAL(walletdb, loc);
 }
 
-std::set<std::pair<libzcash::RawAddress, uint256>> CWallet::GetNullifiersForAddresses(
-        const std::set<libzcash::RawAddress> & addresses)
-{
-    std::set<std::pair<libzcash::RawAddress, uint256>> nullifierSet;
-    // Sapling ivk -> list of addrs map
-    // (There may be more than one diversified address for a given ivk.)
-    std::map<libzcash::SaplingIncomingViewingKey, std::vector<libzcash::SaplingPaymentAddress>> ivkMap;
-    for (const auto & addr : addresses) {
-        auto saplingAddr = std::get_if<libzcash::SaplingPaymentAddress>(&addr);
-        if (saplingAddr != nullptr) {
-            libzcash::SaplingIncomingViewingKey ivk;
-            this->GetSaplingIncomingViewingKey(*saplingAddr, ivk);
-            ivkMap[ivk].push_back(*saplingAddr);
-        }
-    }
-    for (const auto & txPair : mapWallet) {
-        // Sprout
-        for (const auto & noteDataPair : txPair.second.mapSproutNoteData) {
-            auto & noteData = noteDataPair.second;
-            auto & nullifier = noteData.nullifier;
-            auto & address = noteData.address;
-            if (nullifier && addresses.count(address)) {
-                nullifierSet.insert(std::make_pair(address, nullifier.value()));
-            }
-        }
-        // Sapling
-        for (const auto & noteDataPair : txPair.second.mapSaplingNoteData) {
-            auto & noteData = noteDataPair.second;
-            auto & nullifier = noteData.nullifier;
-            auto & ivk = noteData.ivk;
-            if (nullifier && ivkMap.count(ivk)) {
-                for (const auto & addr : ivkMap[ivk]) {
-                    nullifierSet.insert(std::make_pair(addr, nullifier.value()));
+std::set<std::pair<libzcash::SproutPaymentAddress, uint256>> CWallet::GetSproutNullifiers(
+        const std::set<libzcash::SproutPaymentAddress>& addresses) {
+    std::set<std::pair<libzcash::SproutPaymentAddress, uint256>> nullifierSet;
+    if (!addresses.empty()) {
+        for (const auto& txPair : mapWallet) {
+            for (const auto & noteDataPair : txPair.second.mapSproutNoteData) {
+                auto & noteData = noteDataPair.second;
+                auto & nullifier = noteData.nullifier;
+                auto & address = noteData.address;
+                if (nullifier && addresses.count(address) > 0) {
+                    nullifierSet.insert(std::make_pair(address, nullifier.value()));
                 }
             }
         }
     }
+
+    return nullifierSet;
+}
+
+std::set<std::pair<libzcash::SaplingPaymentAddress, uint256>> CWallet::GetSaplingNullifiers(
+        const std::set<libzcash::SaplingPaymentAddress>& addresses) {
+    std::set<std::pair<libzcash::SaplingPaymentAddress, uint256>> nullifierSet;
+    if (!addresses.empty()) {
+        // Sapling ivk -> list of addrs map
+        // (There may be more than one diversified address for a given ivk.)
+        std::map<libzcash::SaplingIncomingViewingKey, std::vector<libzcash::SaplingPaymentAddress>> ivkMap;
+        for (const auto & addr : addresses) {
+            libzcash::SaplingIncomingViewingKey ivk;
+            this->GetSaplingIncomingViewingKey(addr, ivk);
+            ivkMap[ivk].push_back(addr);
+        }
+
+        for (const auto& txPair : mapWallet) {
+            for (const auto& noteDataPair : txPair.second.mapSaplingNoteData) {
+                auto & noteData = noteDataPair.second;
+                auto & nullifier = noteData.nullifier;
+                auto & ivk = noteData.ivk;
+                if (nullifier && ivkMap.count(ivk) > 0) {
+                    for (const auto & addr : ivkMap[ivk]) {
+                        nullifierSet.insert(std::make_pair(addr, nullifier.value()));
+                    }
+                }
+            }
+        }
+    }
+
     return nullifierSet;
 }
 
 bool CWallet::IsNoteSproutChange(
-        const std::set<std::pair<libzcash::RawAddress, uint256>> & nullifierSet,
-        const libzcash::RawAddress & address,
+        const std::set<std::pair<libzcash::SproutPaymentAddress, uint256>> & nullifierSet,
+        const libzcash::SproutPaymentAddress& address,
         const JSOutPoint & jsop)
 {
     // A Note is marked as "change" if the address that received it
@@ -748,8 +757,9 @@ bool CWallet::IsNoteSproutChange(
     return false;
 }
 
-bool CWallet::IsNoteSaplingChange(const std::set<std::pair<libzcash::RawAddress, uint256>> & nullifierSet,
-        const libzcash::RawAddress & address,
+bool CWallet::IsNoteSaplingChange(
+        const std::set<std::pair<libzcash::SaplingPaymentAddress, uint256>> & nullifierSet,
+        const libzcash::SaplingPaymentAddress& address,
         const SaplingOutPoint & op)
 {
     // A Note is marked as "change" if the address that received it
@@ -760,6 +770,8 @@ bool CWallet::IsNoteSaplingChange(const std::set<std::pair<libzcash::RawAddress,
     // - Notes created by consolidation transactions (e.g. using
     //   z_mergetoaddress).
     // - Notes sent from one address to itself.
+    // FIXME: This also needs to check against the wallet's change address
+    // for the associated unified account when we add UA support
     for (const SpendDescription &spend : mapWallet[op.hash].vShieldedSpend) {
         if (nullifierSet.count(std::make_pair(address, spend.nullifier))) {
             return true;
@@ -5026,29 +5038,48 @@ bool CMerkleTx::AcceptToMemoryPool(bool fLimitFree, bool fRejectAbsurdFee)
     return ::AcceptToMemoryPool(Params(), mempool, state, *this, fLimitFree, NULL, fRejectAbsurdFee);
 }
 
-/**
- * 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.
- */
-void CWallet::GetFilteredNotes(
-    std::vector<SproutNoteEntry>& sproutEntries,
-    std::vector<SaplingNoteEntry>& saplingEntries,
-    std::optional<libzcash::PaymentAddress> address,
-    int minDepth,
-    bool ignoreSpent,
-    bool requireSpendingKey)
-{
-    std::set<libzcash::RawAddress> filterAddresses;
+AddrSet AddrSet::ForPaymentAddresses(const std::vector<libzcash::PaymentAddress>& paymentAddrs) {
+    AddrSet addrs;
+    for (const auto& addr: paymentAddrs) {
+        std::visit(match {
+            [&](const CKeyID& keyId) { },
+            [&](const CScriptID& scriptId) { },
+            [&](const libzcash::SproutPaymentAddress& addr) {
+                addrs.sproutAddresses.insert(addr);
+            },
+            [&](const libzcash::SaplingPaymentAddress& addr) {
+                addrs.saplingAddresses.insert(addr);
+            },
+            [&](const libzcash::UnifiedAddress& uaddr) {
+                for (auto& receiver : uaddr) {
+                    std::visit(match {
+                        [&](const libzcash::SaplingPaymentAddress& addr) {
+                            addrs.saplingAddresses.insert(addr);
+                        },
+                        [&](const auto& other) { }
+                    }, receiver);
+                }
+            },
+        }, addr);
+    }
+    return addrs;
+}
 
-    if (address.has_value()) {
-        for (const auto ra : std::visit(GetRawShieldedAddresses(), address.value())) {
-            filterAddresses.insert(ra);
+bool CWallet::HasSpendingKeys(const AddrSet& addrSet) const {
+    for (const auto& zaddr : addrSet.GetSproutAddresses()) {
+        if (!HaveSproutSpendingKey(zaddr)) {
+            return false;
         }
     }
-
-    GetFilteredNotes(sproutEntries, saplingEntries, filterAddresses, minDepth, INT_MAX, ignoreSpent, requireSpendingKey);
+    for (const auto& zaddr : addrSet.GetSaplingAddresses()) {
+        if (!HaveSaplingSpendingKeyForAddress(zaddr)) {
+            return false;
+        }
+    }
+    return true;
 }
 
+
 /**
  * Find notes in the wallet filtered by payment addresses, min depth, max depth,
  * if the note is spent, if a spending key is required, and if the notes are locked.
@@ -5057,7 +5088,7 @@ void CWallet::GetFilteredNotes(
 void CWallet::GetFilteredNotes(
     std::vector<SproutNoteEntry>& sproutEntries,
     std::vector<SaplingNoteEntry>& saplingEntries,
-    std::set<libzcash::RawAddress>& filterAddresses,
+    const std::optional<AddrSet>& noteFilter,
     int minDepth,
     int maxDepth,
     bool ignoreSpent,
@@ -5087,8 +5118,8 @@ void CWallet::GetFilteredNotes(
             SproutNoteData nd = pair.second;
             SproutPaymentAddress pa = nd.address;
 
-            // skip notes which belong to a different payment address in the wallet
-            if (!(filterAddresses.empty() || filterAddresses.count(pa))) {
+            // skip notes which do not conform to the filter, if supplied
+            if (noteFilter.has_value() && !noteFilter.value().HasSproutAddress(pa)) {
                 continue;
             }
 
@@ -5157,8 +5188,8 @@ void CWallet::GetFilteredNotes(
             assert(static_cast<bool>(maybe_pa));
             auto pa = maybe_pa.value();
 
-            // skip notes which belong to a different payment address in the wallet
-            if (!(filterAddresses.empty() || filterAddresses.count(pa))) {
+            // skip notes which do not conform to the filter, if supplied
+            if (noteFilter.has_value() && !noteFilter.value().HasSaplingAddress(pa)) {
                 continue;
             }
 

src/wallet/wallet.h

@@ -623,7 +623,31 @@ public:
 };
 
 
+class AddrSet {
+private:
+    std::set<libzcash::SproutPaymentAddress> sproutAddresses;
+    std::set<libzcash::SaplingPaymentAddress> saplingAddresses;
 
+    AddrSet() {}
+public:
+    static AddrSet ForPaymentAddresses(const std::vector<libzcash::PaymentAddress>& addrs);
+
+    const std::set<libzcash::SproutPaymentAddress>& GetSproutAddresses() const {
+        return sproutAddresses;
+    }
+
+    const std::set<libzcash::SaplingPaymentAddress>& GetSaplingAddresses() const {
+        return saplingAddresses;
+    }
+
+    bool HasSproutAddress(libzcash::SproutPaymentAddress addr) const {
+        return sproutAddresses.count(addr) > 0;
+    }
+
+    bool HasSaplingAddress(libzcash::SaplingPaymentAddress addr) const {
+        return saplingAddresses.count(addr) > 0;
+    }
+};
 
 class COutput
 {
@@ -1194,9 +1218,20 @@ public:
     void AddPendingSaplingMigrationTx(const CTransaction& tx);
     /** Saves witness caches and best block locator to disk. */
     void SetBestChain(const CBlockLocator& loc);
-    std::set<std::pair<libzcash::RawAddress, uint256>> GetNullifiersForAddresses(const std::set<libzcash::RawAddress> & addresses);
-    bool IsNoteSproutChange(const std::set<std::pair<libzcash::RawAddress, uint256>> & nullifierSet, const libzcash::RawAddress & address, const JSOutPoint & entry);
-    bool IsNoteSaplingChange(const std::set<std::pair<libzcash::RawAddress, uint256>> & nullifierSet, const libzcash::RawAddress & address, const SaplingOutPoint & entry);
+
+    std::set<std::pair<libzcash::SproutPaymentAddress, uint256>> GetSproutNullifiers(
+            const std::set<libzcash::SproutPaymentAddress>& addresses);
+    bool IsNoteSproutChange(
+            const std::set<std::pair<libzcash::SproutPaymentAddress, uint256>> & nullifierSet,
+            const libzcash::SproutPaymentAddress& address,
+            const JSOutPoint & entry);
+
+    std::set<std::pair<libzcash::SaplingPaymentAddress, uint256>> GetSaplingNullifiers(
+            const std::set<libzcash::SaplingPaymentAddress>& addresses);
+    bool IsNoteSaplingChange(
+            const std::set<std::pair<libzcash::SaplingPaymentAddress, uint256>> & nullifierSet,
+            const libzcash::SaplingPaymentAddress& address,
+            const SaplingOutPoint & entry);
 
     DBErrors LoadWallet(bool& fFirstRunRet);
     DBErrors ZapWalletTx(std::vector<CWalletTx>& vWtx);
@@ -1304,19 +1339,13 @@ public:
     /* Set the current encrypted HD seed, without saving it to disk (used by LoadWallet) */
     bool LoadCryptedHDSeed(const uint256& seedFp, const std::vector<unsigned char>& seed);
 
-    /* Find notes filtered by (optional) payment address, min depth, ability to spend */
-    void GetFilteredNotes(std::vector<SproutNoteEntry>& sproutEntries,
-                          std::vector<SaplingNoteEntry>& saplingEntries,
-                          std::optional<libzcash::PaymentAddress> address,
-                          int minDepth=1,
-                          bool ignoreSpent=true,
-                          bool requireSpendingKey=true);
+    bool HasSpendingKeys(const AddrSet& noteFilter) const;
 
     /* Find notes filtered by payment addresses, min depth, max depth, if they are spent,
        if a spending key is required, and if they are locked */
     void GetFilteredNotes(std::vector<SproutNoteEntry>& sproutEntries,
                           std::vector<SaplingNoteEntry>& saplingEntries,
-                          std::set<libzcash::RawAddress>& filterAddresses,
+                          const std::optional<AddrSet>& noteFilter,
                           int minDepth=1,
                           int maxDepth=INT_MAX,
                           bool ignoreSpent=true,

src/zcash/Address.cpp

@@ -71,94 +71,3 @@ uint32_t TypecodeForReceiver::operator()(
 {
     return unknown.typecode;
 }
-
-// ReceiverToRawAddress
-
-std::optional<libzcash::RawAddress> ReceiverToRawAddress::operator()(
-    const CKeyID &p2sh) const
-{
-    return std::nullopt;
-}
-std::optional<libzcash::RawAddress> ReceiverToRawAddress::operator()(
-    const CScriptID &p2sh) const
-{
-    return std::nullopt;
-}
-std::optional<libzcash::RawAddress> ReceiverToRawAddress::operator()(
-    const libzcash::SaplingPaymentAddress &zaddr) const
-{
-    return zaddr;
-}
-std::optional<libzcash::RawAddress> ReceiverToRawAddress::operator()(
-    const libzcash::UnknownReceiver &p2sh) const
-{
-    return std::nullopt;
-}
-
-// RecipientForPaymentAddress
-
-std::optional<libzcash::RawAddress> RecipientForPaymentAddress::operator()(
-    const CKeyID &p2sh) const
-{
-    return std::nullopt;
-}
-std::optional<libzcash::RawAddress> RecipientForPaymentAddress::operator()(
-    const CScriptID &p2sh) const
-{
-    return std::nullopt;
-}
-std::optional<libzcash::RawAddress> RecipientForPaymentAddress::operator()(
-    const libzcash::SproutPaymentAddress &zaddr) const
-{
-    return zaddr;
-}
-std::optional<libzcash::RawAddress> RecipientForPaymentAddress::operator()(
-    const libzcash::SaplingPaymentAddress &zaddr) const
-{
-    return zaddr;
-}
-std::optional<libzcash::RawAddress> RecipientForPaymentAddress::operator()(
-    const libzcash::UnifiedAddress &uaddr) const
-{
-    for (auto& receiver : uaddr) {
-        // Return the first one.
-        return std::visit(ReceiverToRawAddress(), receiver);
-    }
-
-    return std::nullopt;
-}
-
-// GetRawShieldedAddresses
-
-std::set<libzcash::RawAddress> GetRawShieldedAddresses::operator()(
-    const CKeyID &addr) const
-{
-    return {};
-}
-std::set<libzcash::RawAddress> GetRawShieldedAddresses::operator()(
-    const CScriptID &addr) const
-{
-    return {};
-}
-std::set<libzcash::RawAddress> GetRawShieldedAddresses::operator()(
-    const libzcash::SproutPaymentAddress &zaddr) const
-{
-    return {zaddr};
-}
-std::set<libzcash::RawAddress> GetRawShieldedAddresses::operator()(
-    const libzcash::SaplingPaymentAddress &zaddr) const
-{
-    return {zaddr};
-}
-std::set<libzcash::RawAddress> GetRawShieldedAddresses::operator()(
-    const libzcash::UnifiedAddress &uaddr) const
-{
-    std::set<libzcash::RawAddress> ret;
-    for (auto& receiver : uaddr) {
-        auto ra = std::visit(ReceiverToRawAddress(), receiver);
-        if (ra) {
-            ret.insert(*ra);
-        }
-    }
-    return ret;
-}

src/zcash/Address.hpp

@@ -12,9 +12,6 @@
 
 namespace libzcash {
 
-/** Protocol addresses that can receive funds in a transaction. */
-typedef std::variant<SproutPaymentAddress, SaplingPaymentAddress> RawAddress;
-
 class UnknownReceiver {
 public:
     uint32_t typecode;
@@ -163,45 +160,4 @@ public:
     uint32_t operator()(const libzcash::UnknownReceiver &p2pkh) const;
 };
 
-/**
- * Converts the given UA receiver to a protocol address, if it is a shielded receiver.
- */
-class ReceiverToRawAddress {
-public:
-    ReceiverToRawAddress() {}
-
-    std::optional<libzcash::RawAddress> operator()(const libzcash::SaplingPaymentAddress &zaddr) const;
-    std::optional<libzcash::RawAddress> operator()(const CScriptID &p2sh) const;
-    std::optional<libzcash::RawAddress> operator()(const CKeyID &p2pkh) const;
-    std::optional<libzcash::RawAddress> operator()(const libzcash::UnknownReceiver &p2pkh) const;
-};
-
-/**
- * Returns the protocol address that should be used in transaction outputs.
- */
-class RecipientForPaymentAddress {
-public:
-    RecipientForPaymentAddress() {}
-
-    std::optional<libzcash::RawAddress> operator()(const CKeyID &addr) const;
-    std::optional<libzcash::RawAddress> operator()(const CScriptID &addr) const;
-    std::optional<libzcash::RawAddress> operator()(const libzcash::SproutPaymentAddress &zaddr) const;
-    std::optional<libzcash::RawAddress> operator()(const libzcash::SaplingPaymentAddress &zaddr) const;
-    std::optional<libzcash::RawAddress> operator()(const libzcash::UnifiedAddress &uaddr) const;
-};
-
-/**
- * Returns all protocol addresses contained within the given payment address.
- */
-class GetRawShieldedAddresses {
-public:
-    GetRawShieldedAddresses() {}
-
-    std::set<libzcash::RawAddress> operator()(const CKeyID &addr) const;
-    std::set<libzcash::RawAddress> operator()(const CScriptID &addr) const;
-    std::set<libzcash::RawAddress> operator()(const libzcash::SproutPaymentAddress &zaddr) const;
-    std::set<libzcash::RawAddress> operator()(const libzcash::SaplingPaymentAddress &zaddr) const;
-    std::set<libzcash::RawAddress> operator()(const libzcash::UnifiedAddress &uaddr) const;
-};
-
 #endif // ZC_ADDRESS_H_