Merge #2887: [Wallet] Some sapling wallet improvements

74f7f94 Don't generate shield addressed that have already been used (Alessandro Rezzi)
5efedea Filter Sapling data only when the right flag is set (Alessandro Rezzi)

Pull request description:

  first commit: filter sapling credit/debit only when the appropriate flag has been set in the filter
  second commit: in the qt wallet don't generate shield addresses that have already been used

ACKs for top commit: 74f7f94
  Fuzzbawls:
    utACK 74f7f94
  Liquid369:
    tACK 74f7f94

Tree-SHA512: 6f75ee1c35f693d7ecf8872bc470876550961cc7108b56ee09b745adf6c595bd2271aff5efe5274eabdef10f17774327d0041f678a8d3cccffc897a421fe43e0

src/qt/addresstablemodel.cpp

@@ -618,7 +618,7 @@ QString AddressTableModel::getAddressToShow(bool isShielded) const
 
     for (auto it = wallet->NewAddressBookIterator(); it.IsValid(); it.Next()) {
         const auto addrData = it.GetValue();
-
+        CWDestination x = *it.GetDestKey();
         if (!isShielded) {
             if (addrData.purpose == AddressBook::AddressBookPurpose::RECEIVE) {
                 const auto &address = *it.GetCTxDestKey();
@@ -627,10 +627,9 @@ QString AddressTableModel::getAddressToShow(bool isShielded) const
                 }
             }
         } else {
-            // todo: add shielded address support to IsUsed
             if (addrData.purpose == AddressBook::AddressBookPurpose::SHIELDED_RECEIVE) {
                 const auto &address = *it.GetShieldedDestKey();
-                if (IsValidPaymentAddress(address) && IsMine(*wallet, address)) {
+                if (IsValidPaymentAddress(address) && IsMine(*wallet, address) && !wallet->IsUsed(address)) {
                     return QString::fromStdString(KeyIO::EncodePaymentAddress(address));
                 }
             }

src/sapling/saplingscriptpubkeyman.cpp

@@ -869,6 +869,11 @@ isminetype SaplingScriptPubKeyMan::IsMine(const CWalletTx& wtx, const SaplingOut
 
 CAmount SaplingScriptPubKeyMan::GetCredit(const CWalletTx& tx, const isminefilter& filter, const bool fUnspent) const
 {
+    // If we are not filtering shield data, return
+    if (!(filter & ISMINE_WATCH_ONLY_SHIELDED || filter & ISMINE_SPENDABLE_SHIELDED)) {
+        return 0;
+    }
+
     if (!tx.tx->IsShieldedTx() || tx.tx->sapData->vShieldedOutput.empty()) {
         return 0;
     }
@@ -889,15 +894,21 @@ CAmount SaplingScriptPubKeyMan::GetCredit(const CWalletTx& tx, const isminefilte
             (fUnspent && IsSaplingSpent(*noteData.nullifier))) {
             continue; // only unspent
         }
-        // todo: check if we can spend this note or not. (if not, then it's a watch only)
-
-        nCredit += noteData.amount ? *noteData.amount : 0;
+        // If we are filtering watch only or we have spend authority add the amount
+        if ((filter & ISMINE_WATCH_ONLY_SHIELDED) || (noteData.address && HaveSpendingKeyForPaymentAddress(*noteData.address))) {
+            nCredit += noteData.amount ? *noteData.amount : 0;
+        }
     }
     return nCredit;
 }
 
 CAmount SaplingScriptPubKeyMan::GetDebit(const CTransaction& tx, const isminefilter& filter) const
 {
+    // If we are not filtering shield data, return
+    if (!(filter & ISMINE_WATCH_ONLY_SHIELDED || filter & ISMINE_SPENDABLE_SHIELDED)) {
+        return 0;
+    }
+
     if (!tx.IsShieldedTx() || tx.sapData->vShieldedSpend.empty()) {
         return 0;
     }
@@ -914,9 +925,12 @@ CAmount SaplingScriptPubKeyMan::GetDebit(const CTransaction& tx, const isminefil
             auto nit = wtx.mapSaplingNoteData.find(op);
             assert(nit != wtx.mapSaplingNoteData.end());
             const auto& nd = nit->second;
-            assert(nd.IsMyNote());        // todo: Add watch only check.
+            assert(nd.IsMyNote());
             assert(static_cast<bool>(nd.amount));
-            nDebit += *(nd.amount);
+            // If we are filtering watch only or we have spend authority add the amount
+            if ((filter & ISMINE_WATCH_ONLY_SHIELDED) || (nd.address && HaveSpendingKeyForPaymentAddress(*nd.address))) {
+                nDebit += *(nd.amount);
+            }
             if (!Params().GetConsensus().MoneyRange(nDebit))
                 throw std::runtime_error("SaplingScriptPubKeyMan::GetDebit() : value out of range");
         }

src/wallet/wallet.cpp

@@ -1480,6 +1480,23 @@ bool CWallet::IsUsed(const CTxDestination address) const
     return false;
 }
 
+bool CWallet::IsUsed(const libzcash::SaplingPaymentAddress address) const
+{
+    LOCK(cs_wallet);
+    if (!::IsMine(*this, address)) {
+        return false;
+    }
+
+    for (const auto& it : mapWallet) {
+        const CWalletTx& wtx = it.second;
+        for (const auto& txout : wtx.mapSaplingNoteData) {
+            if (txout.second.address && *txout.second.address == address)
+                return true;
+        }
+    }
+    return false;
+}
+
 CAmount CWallet::GetDebit(const CTxIn& txin, const isminefilter& filter) const
 {
     {

src/wallet/wallet.h

@@ -1143,6 +1143,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
     bool CreateBudgetFeeTX(CTransactionRef& tx, const uint256& hash, CReserveKey& keyChange, CAmount fee);
 
     bool IsUsed(const CTxDestination address) const;
+    bool IsUsed(const libzcash::SaplingPaymentAddress address) const;
 
     isminetype IsMine(const CTxIn& txin) const;
     CAmount GetDebit(const CTxIn& txin, const isminefilter& filter) const;