wallet/rpc: gather Sapling keys once per history RPC, not per output per address

zsWalletTxJSON (z_listsentbyaddress, z_listreceivedaddress, getalldata)
runs once per wallet tx under LOCK2(cs_main, cs_wallet). For every
shielded output it copied the wallet's address set, and for each address
fetched the spending key, re-derived its full viewing key (an
elliptic-curve operation), and trial-decrypted the output; the sends side
did the same with the ovk. Every received or sent note in a tx with
spends walked the whole wallet in GetNullifiersForAddresses for the
change check. The spends side decrypted every spent note again and
deep-copied its SaplingNoteData (witnesses included). The GUI runs
z_listsentbyaddress over the whole wallet on every launch.

SaplingHistoryKeys, built once per RPC call, holds each address with a
usable spending key (in GetSaplingPaymentAddresses order, with its ivk
and ovk as the spending key gives them, and its ivk as
GetSaplingIncomingViewingKey gives it). It also gives the change check
as a lookup into the wallet's (ivk, nullifier) pairs, collected on first
use. The helpers keep their exact structure and quirks. Each output's
JSON object and change flag still accumulate across the addresses that
decrypt it. Diversified addresses sharing a key still produce one entry
each. An unknown address still checks change under a zero ivk. A locked
wallet still reports no shielded received or sent entries. Only the
repeated work is gone:

- each output is decrypted at most once per distinct key. Once it is
  known which ivk decrypts an output, no other ivk is tried: the cached
  plaintext, or a decryption under the ivk the wallet's note data
  records. Two canonical ivks can't both pass the note commitment check,
  which pins pk_d = [ivk]g_d;
- outCiphertexts are decrypted once per distinct ovk; the t->z shielding
  ovk is derived from the seed once per tx;
- spends use the cached plaintext of the spent note.

Two deliberate behaviour changes. The spends helper looked notes up with
operator[]: it inserted a null-outpoint entry into
mapSaplingNullifiersToNotes for every other wallet's nullifier it met.
After getalldata or z_listsentbyaddress had run, z_viewtransaction of
any tx spending someone else's note (every incoming z->z payment) failed
with map::at. And with an inconsistent nullifier map, it inserted an
empty note into a wallet tx. The helpers now only read the wallet. Also,
if the walletpassphrase timer relocks the wallet during the call, the
whole call reports the keys as of its start; the old per-output lookups
switched over partway.

test_wallet_history.cpp holds 69bb035e7's helpers verbatim (renamed)
and a wallet exercising every branch:
- default and diversified addresses, a viewing-key-only address, and a
  key imported after its note arrived;
- z->z sends with change, and with text, empty and binary memos;
- a send nobody's ovk reads, and a t->z shielding send;
- our spends, someone else's, and a mixed tx.
Every tx x every return type x nine address filters gives byte-identical
JSON: with the plaintext cache warm, cold, and on an encrypted wallet
both locked and unlocked. Mutation-checked: dropping the ivk check from
the uniqueness shortcut fails the test. DoesNotModifyTheWallet pins the
read-only change (and shows that the old helpers did insert the foreign
nullifier).

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

src/Makefile.gtest.include

@@ -18,7 +18,9 @@ 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 \
+	gtest/test_wallet_history.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_history.cpp

@@ -0,0 +1,816 @@
+// 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 <utf8.h>
+
+#include "chain.h"
+#include "chainparams.h"
+#include "coins.h"
+#include "init.h"
+#include "key_io.h"
+#include "main.h"
+#include "primitives/block.h"
+#include "primitives/transaction.h"
+#include "random.h"
+#include "rpc/server.h"
+#include "script/standard.h"
+#include "utilmoneystr.h"
+#include "wallet/wallet.h"
+#include "wallet/rpchushwallet.h"
+#include "zcash/Address.hpp"
+#include "zcash/Note.hpp"
+#include "zcash/NoteEncryption.hpp"
+#include "zcash/zip32.h"
+
+using namespace std;
+using namespace libzcash;
+
+// The history-RPC helpers (src/wallet/rpchushwallet.cpp) as of 69bb035e7, verbatim apart from the Legacy_
+// prefix and static linkage: the reference the optimized helpers must reproduce byte for byte.
+static void Legacy_zsTxSpendsToJSON(const CWalletTx& wtx, UniValue& spends, CAmount& totalSpends, CAmount& filteredSpends, const std::string& strAddress, bool filterByAddress) {
+
+  LOCK2(cs_main, pwalletMain->cs_wallet);
+
+  //Used to identify incomplete key sets
+  int vinCount = 0;
+  int vinSpendCount = 0;
+  int shieldedSpendCount = 0;
+  int shieldedSpendsSpentCount = 0;
+
+  //Check address
+  bool isTAddress = false;
+  bool isZsAddress = false;
+
+  CTxDestination tAddress = DecodeDestination(strAddress);
+  auto zAddress = DecodePaymentAddress(strAddress);
+  SaplingPaymentAddress zsAddress;
+
+  if (filterByAddress) {
+
+    if (IsValidDestination(tAddress))
+      isTAddress = true;
+
+    if (IsValidPaymentAddress(zAddress)) {
+      if (boost::get<libzcash::SaplingPaymentAddress>(&zAddress) != nullptr) {
+          zsAddress = boost::get<libzcash::SaplingPaymentAddress>(zAddress);
+          isZsAddress = true;
+      }
+    }
+  }
+
+
+  // Transparent Inputs belonging to the wallet
+  UniValue tSpends(UniValue::VARR);
+  if (isTAddress || !filterByAddress) {
+    for (int i = 0; i < wtx.vin.size(); i++) {
+      vinCount++;
+      const CTxIn& txin = wtx.vin[i];
+      UniValue obj(UniValue::VOBJ);
+      CTxDestination address;
+      const CWalletTx* parent = pwalletMain->GetWalletTx(txin.prevout.hash);
+      if (parent != NULL) {
+        const CTxOut& parentOut = parent->vout[txin.prevout.n];
+        ExtractDestination(parentOut.scriptPubKey, address);
+        if(IsMine(*pwalletMain, address)){
+          vinSpendCount++;
+          totalSpends += CAmount(-parentOut.nValue);
+          obj.push_back(Pair("address",EncodeDestination(address)));
+          obj.push_back(Pair("scriptPubKey",HexStr(parentOut.scriptPubKey.begin(), parentOut.scriptPubKey.end())));
+          obj.push_back(Pair("amount",ValueFromAmount(-parentOut.nValue)));
+          obj.push_back(Pair("spendTxid",parent->GetHash().ToString()));
+          obj.push_back(Pair("spendVout",(int)txin.prevout.n));
+          CTxDestination filterAddress = DecodeDestination(strAddress);
+          if (address == tAddress || !filterByAddress) {
+            filteredSpends += CAmount(-parentOut.nValue);
+            tSpends.push_back(obj);
+          }
+        }
+      }
+    }
+  }
+  spends.push_back(Pair("transparentSpends",tSpends));
+
+  // Sapling Inputs belonging to the wallet
+  UniValue zsSpends(UniValue::VARR);
+  if (isZsAddress || !filterByAddress) {
+    for (int i = 0; i < wtx.vShieldedSpend.size(); i++) {
+      shieldedSpendCount++;
+      const SpendDescription& spendDesc = wtx.vShieldedSpend[i];
+      UniValue obj(UniValue::VOBJ);
+
+      SaplingOutPoint op = pwalletMain->mapSaplingNullifiersToNotes[spendDesc.nullifier];
+
+      if (pwalletMain->IsSaplingNullifierFromMe(spendDesc.nullifier)) {
+        const CWalletTx* parent = pwalletMain->GetWalletTx(pwalletMain->mapSaplingNullifiersToNotes[spendDesc.nullifier].hash);
+        const OutputDescription& output = parent->vShieldedOutput[op.n];
+        auto nd = pwalletMain->mapWallet[pwalletMain->mapSaplingNullifiersToNotes[spendDesc.nullifier].hash].mapSaplingNoteData[op];
+        auto pt = libzcash::SaplingNotePlaintext::decrypt(output.encCiphertext,nd.ivk,output.ephemeralKey,output.cm);
+
+        if (pt) {
+          auto note = pt.get();
+          auto pa = nd.ivk.address(note.d);
+          auto address = pa.get();
+          shieldedSpendsSpentCount++;
+          totalSpends += CAmount(-note.value());
+          obj.push_back(Pair("address",EncodePaymentAddress(address)));
+          obj.push_back(Pair("amount", ValueFromAmount(CAmount(-note.value()))));
+          obj.push_back(Pair("spendTxid",parent->GetHash().ToString()));
+          obj.push_back(Pair("spendshieldedOutputIndex",(int)op.n));
+          if (address == zsAddress || !filterByAddress) {
+            filteredSpends += CAmount(-note.value());
+            zsSpends.push_back(obj);
+          }
+        }
+      }
+    }
+  }
+  spends.push_back(Pair("saplingSpends",zsSpends));
+
+ 
+  spends.push_back(Pair("totalSpends",ValueFromAmount(filteredSpends)));
+
+  if (!filterByAddress) {
+    if (filteredSpends != 0 && (vinCount != vinSpendCount || shieldedSpendCount != shieldedSpendsSpentCount)) {
+      spends.push_back(Pair("missingSpendingKeys", true));
+      spends.push_back(Pair("vinCount", vinCount));
+      spends.push_back(Pair("vinSpendCount", vinSpendCount));
+      spends.push_back(Pair("shieldedSpendCount", shieldedSpendCount));
+      spends.push_back(Pair("shieldedSpendsSpentCount", shieldedSpendsSpentCount));
+    } else {
+        spends.push_back(Pair("missingSpendingKeys", false));
+    }
+  }
+}
+
+
+static void Legacy_zsTxReceivedToJSON(const CWalletTx& wtx, UniValue& received, CAmount& totalReceived, const std::string& strAddress, bool filterByAddress) {
+
+  if(fZdebug)
+    fprintf(stderr,"%s: txid=%s\n", __func__, wtx.GetHash().ToString().c_str() );
+
+  LOCK2(cs_main, pwalletMain->cs_wallet);
+
+  //Check address
+  bool isTAddress = false;
+  bool isZsAddress = false;
+
+  CTxDestination tAddress = DecodeDestination(strAddress);
+  auto zAddress = DecodePaymentAddress(strAddress);
+  SaplingPaymentAddress zsAddress;
+
+  if (filterByAddress) {
+
+    if (IsValidDestination(tAddress))
+      isTAddress = true;
+
+    if (IsValidPaymentAddress(zAddress)) {
+      if (boost::get<libzcash::SaplingPaymentAddress>(&zAddress) != nullptr) {
+          zsAddress = boost::get<libzcash::SaplingPaymentAddress>(zAddress);
+          isZsAddress = true;
+      }
+    }
+  }
+
+
+  //Transparent Received txos belonging to the wallet
+  UniValue tReceived(UniValue::VARR);
+  if (isTAddress || !filterByAddress) {
+    for (int i = 0; i < wtx.vout.size(); i++) {
+      const CTxOut& txout = wtx.vout[i];
+      UniValue obj(UniValue::VOBJ);
+      CTxDestination address;
+      ExtractDestination(txout.scriptPubKey, address);
+      if(IsMine(*pwalletMain, address)){
+        obj.push_back(Pair("address",EncodeDestination(address)));
+        obj.push_back(Pair("scriptPubKey",HexStr(txout.scriptPubKey.begin(), txout.scriptPubKey.end())));
+        obj.push_back(Pair("amount",ValueFromAmount(txout.nValue)));
+        obj.push_back(Pair("vout", i));
+        if (address == tAddress || !filterByAddress) {
+          totalReceived += CAmount(txout.nValue);
+          tReceived.push_back(obj);
+        }
+      }
+    }
+  }
+  received.push_back(Pair("transparentReceived",tReceived));
+
+
+  //Sapling Sends belonging to the wallet
+  UniValue zsReceived(UniValue::VARR);
+  if (isZsAddress || !filterByAddress) {
+    for (int i = 0; i < wtx.vShieldedOutput.size(); i++) {
+      const OutputDescription& outputDesc = wtx.vShieldedOutput[i];
+      UniValue obj(UniValue::VOBJ);
+      bool changeTx = false;
+      //Decrypt sapling incoming commitments using IVK
+      std::set<libzcash::SaplingPaymentAddress> addresses;
+      pwalletMain->GetSaplingPaymentAddresses(addresses);
+      for (auto addr : addresses) {
+        libzcash::SaplingExtendedSpendingKey extsk;
+        if (pwalletMain->GetSaplingExtendedSpendingKey(addr, extsk)) {
+          auto pt = libzcash::SaplingNotePlaintext::decrypt(
+                  outputDesc.encCiphertext, extsk.expsk.full_viewing_key().in_viewing_key(), outputDesc.ephemeralKey, outputDesc.cm);
+
+          if (pt) {
+            auto note = pt.get();
+            auto pt_unwrapped = pt.get();
+            auto memo = pt_unwrapped.memo();
+            obj.push_back(Pair("address",EncodePaymentAddress(addr)));
+            obj.push_back(Pair("amount", ValueFromAmount(CAmount(note.value()))));
+            obj.push_back(Pair("memo", HexStr(memo)));
+
+              if (memo[0] <= 0xf4) {
+                  auto end = std::find_if(memo.rbegin(), memo.rend(), [](unsigned char v) { return v != 0; });
+                  std::string memoStr(memo.begin(), end.base());
+                if (utf8::is_valid(memoStr)) {
+                obj.push_back(Pair("memoStr", memoStr));
+            }
+        }
+            obj.push_back(Pair("shieldedOutputIndex",i));
+
+            //Check Change Status
+            if (wtx.vShieldedSpend.size()!=0) {
+              std::set<std::pair<PaymentAddress, uint256>> nullifierSet;
+              nullifierSet = pwalletMain->GetNullifiersForAddresses({addr});
+              BOOST_FOREACH(const SpendDescription& spendDesc, wtx.vShieldedSpend) {
+                if (nullifierSet.count(std::make_pair(addr, spendDesc.nullifier))) {
+                    changeTx = true;
+                }
+              }
+            }
+            obj.push_back(Pair("change",changeTx));
+            if (addr == zsAddress || !filterByAddress) {
+              totalReceived += CAmount(note.value());
+              zsReceived.push_back(obj);
+            }
+          }
+        }
+      }
+    }
+  }
+  received.push_back(Pair("saplingReceived",zsReceived));
+
+  received.push_back(Pair("totalReceived",ValueFromAmount(totalReceived)));
+}
+
+
+static void Legacy_zsTxSendsToJSON(const CWalletTx& wtx, UniValue& sends, CAmount& totalSends, const std::string& strAddress, bool filterByAddress) {
+
+  LOCK2(cs_main, pwalletMain->cs_wallet);
+
+  //Used to identify incomplete key sets
+  int shieldedOutputCount = 0;
+  int shieldedOutputDecryptedCount = 0;
+
+  //Check address
+  bool isTAddress = false;
+  bool isZsAddress = false;
+
+  CTxDestination tAddress = DecodeDestination(strAddress);
+  auto zAddress = DecodePaymentAddress(strAddress);
+  SaplingPaymentAddress zsAddress;
+
+  if (filterByAddress) {
+
+    if (IsValidDestination(tAddress))
+      isTAddress = true;
+
+    if (IsValidPaymentAddress(zAddress)) {
+      if (boost::get<libzcash::SaplingPaymentAddress>(&zAddress) != nullptr) {
+          zsAddress = boost::get<libzcash::SaplingPaymentAddress>(zAddress);
+          isZsAddress = true;
+      }
+    }
+  }
+
+  //All Transparent Sends in the transaction
+
+  UniValue tSends(UniValue::VARR);
+  if (isTAddress || !filterByAddress) {
+    for (int i = 0; i < wtx.vout.size(); i++) {
+      const CTxOut& txout = wtx.vout[i];
+      UniValue obj(UniValue::VOBJ);
+      CTxDestination address;
+      ExtractDestination(txout.scriptPubKey, address);
+      obj.push_back(Pair("address",EncodeDestination(address)));
+      obj.push_back(Pair("scriptPubKey",HexStr(txout.scriptPubKey.begin(), txout.scriptPubKey.end())));
+      obj.push_back(Pair("amount",ValueFromAmount(-txout.nValue)));
+      obj.push_back(Pair("vout", i));
+      if (address == tAddress || !filterByAddress) {
+        totalSends += CAmount(-txout.nValue);
+        tSends.push_back(obj);
+      }
+    }
+  }
+  sends.push_back(Pair("transparentSends",tSends));
+
+  //All Shielded Sends in the transaction
+  UniValue zsSends(UniValue::VARR);
+  if (isZsAddress || !filterByAddress) {
+    for (int i = 0; i < wtx.vShieldedOutput.size(); i++) {
+      const OutputDescription& outputDesc = wtx.vShieldedOutput[i];
+      shieldedOutputCount++;
+      UniValue obj(UniValue::VOBJ);
+      bool changeTx = false;
+
+      //Decrypt sapling outgoing t to z transaction using HDseed
+      if (wtx.vShieldedSpend.size()==0) {
+        HDSeed seed;
+        if (pwalletMain->GetHDSeedForDerivation(seed)) {
+          auto opt = libzcash::SaplingOutgoingPlaintext::decrypt(
+                  outputDesc.outCiphertext,ovkForShieldingFromTaddr(seed),outputDesc.cv,outputDesc.cm,outputDesc.ephemeralKey);
+
+          if (opt) {
+            auto opt_unwrapped = opt.get();
+            auto pt = libzcash::SaplingNotePlaintext::decrypt(
+                    outputDesc.encCiphertext,outputDesc.ephemeralKey,opt_unwrapped.esk,opt_unwrapped.pk_d,outputDesc.cm);
+
+            if (pt) {
+              shieldedOutputDecryptedCount++;
+              auto pt_unwrapped = pt.get();
+              libzcash::SaplingPaymentAddress sentAddr(pt_unwrapped.d, opt_unwrapped.pk_d);
+              obj.push_back(Pair("address",EncodePaymentAddress(sentAddr)));
+              obj.push_back(Pair("amount", ValueFromAmount(CAmount(pt_unwrapped.value()))));
+              obj.push_back(Pair("shieldedOutputIndex",i));
+              obj.push_back(Pair("change",false));
+              if (sentAddr == zsAddress || !filterByAddress) {
+                totalSends += CAmount(-pt_unwrapped.value());
+                zsSends.push_back(obj);
+              }
+            }
+          }
+        }
+
+      //attempt Decryption of Outgoing Sapling using wallet extended spending keys
+      } else {
+        std::set<libzcash::SaplingPaymentAddress> addresses;
+        pwalletMain->GetSaplingPaymentAddresses(addresses);
+        for (auto addr : addresses) {
+          libzcash::SaplingExtendedSpendingKey extsk;
+          if (pwalletMain->GetSaplingExtendedSpendingKey(addr, extsk)) {
+            auto opt = libzcash::SaplingOutgoingPlaintext::decrypt(
+                    outputDesc.outCiphertext,extsk.expsk.full_viewing_key().ovk,outputDesc.cv,outputDesc.cm,outputDesc.ephemeralKey);
+
+            if (opt) {
+              auto opt_unwrapped = opt.get();
+              auto pt = libzcash::SaplingNotePlaintext::decrypt(
+                      outputDesc.encCiphertext,outputDesc.ephemeralKey,opt_unwrapped.esk,opt_unwrapped.pk_d,outputDesc.cm);
+
+              if (pt) {
+                auto pt_unwrapped = pt.get();
+                auto memo = pt_unwrapped.memo();
+                shieldedOutputDecryptedCount++;
+                libzcash::SaplingPaymentAddress sentAddr(pt_unwrapped.d, opt_unwrapped.pk_d);
+                obj.push_back(Pair("address",EncodePaymentAddress(sentAddr)));
+                obj.push_back(Pair("amount", ValueFromAmount(CAmount(-pt_unwrapped.value()))));
+                obj.push_back(Pair("memo", HexStr(memo)));
+
+              if (memo[0] <= 0xf4) {
+                  auto end = std::find_if(memo.rbegin(), memo.rend(), [](unsigned char v) { return v != 0; });
+                  std::string memoStr(memo.begin(), end.base());
+                if (utf8::is_valid(memoStr)) {
+                obj.push_back(Pair("memoStr", memoStr));
+            }
+        }
+                obj.push_back(Pair("shieldedOutputIndex",i));
+
+                //Check Change Status
+                if (wtx.vShieldedSpend.size()!=0) {
+                  std::set<std::pair<PaymentAddress, uint256>> nullifierSet;
+                  nullifierSet = pwalletMain->GetNullifiersForAddresses({sentAddr});
+                  BOOST_FOREACH(const SpendDescription& spendDesc, wtx.vShieldedSpend) {
+                    if (nullifierSet.count(std::make_pair(sentAddr, spendDesc.nullifier))) {
+                        changeTx = true;
+                    }
+                  }
+                }
+                obj.push_back(Pair("change",changeTx));
+                if (sentAddr == zsAddress || !filterByAddress) {
+                  totalSends += CAmount(-pt_unwrapped.value());
+                  zsSends.push_back(obj);
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+  sends.push_back(Pair("saplingSends",zsSends));
+
+  if (shieldedOutputCount != shieldedOutputDecryptedCount) {
+    sends.push_back(Pair("missingSaplingOVK", true));
+  } else {
+    sends.push_back(Pair("missingSaplingOVK", false));
+  }
+
+  sends.push_back(Pair("totalSends",ValueFromAmount(totalSends)));
+}
+
+
+static void Legacy_zsWalletTxJSON(const CWalletTx& wtx, UniValue& ret, const std::string strAddress, bool fBool, const int returnType) {
+
+  LOCK2(cs_main, pwalletMain->cs_wallet);
+
+  //Track total wallet spend and received
+  CAmount totalSpends = 0;
+  CAmount filteredSpends = 0;
+  CAmount totalReceived = 0;
+  CAmount totalSends = 0;
+
+  //Various Univalue to be added to the final transaction
+  UniValue spends(UniValue::VOBJ);
+  UniValue received(UniValue::VOBJ);
+  UniValue sends(UniValue::VOBJ);
+  UniValue tx(UniValue::VOBJ);
+
+  //Begin Compiling the Decrypted Transaction
+  tx.push_back(Pair("txid", wtx.GetHash().ToString()));
+
+  if(fZdebug)
+    fprintf(stderr,"%s: txid=%s\n", __func__, wtx.GetHash().ToString().c_str() );
+
+  if (wtx.IsCoinBase())
+  {
+      tx.push_back(Pair("coinbase", true));
+      if (wtx.GetDepthInMainChain() < 1)
+          tx.push_back(Pair("category", "orphan"));
+      else if (wtx.GetBlocksToMaturity() > 0)
+          tx.push_back(Pair("category", "immature"));
+      else
+          tx.push_back(Pair("category", "generate"));
+  } else {
+    tx.push_back(Pair("coinbase", false));
+    tx.push_back(Pair("category", "standard"));
+  }
+
+  tx.push_back(Pair("blockhash", wtx.hashBlock.GetHex()));
+  tx.push_back(Pair("blockindex", wtx.nIndex));
+  int confirms = wtx.GetDepthInMainChain();
+  if(confirms > 0)
+  {
+      tx.push_back(Pair("blocktime", mapBlockIndex[wtx.hashBlock]->GetBlockTime()));
+  } else {
+      tx.push_back(Pair("blocktime", 0));
+  }
+  tx.push_back(Pair("expiryheight", (int64_t)wtx.nExpiryHeight));
+  tx.push_back(Pair("confirmations", confirms));
+  tx.push_back(Pair("time", wtx.GetTxTime()));
+  tx.push_back(Pair("size", static_cast<uint64_t>(GetSerializeSize(static_cast<CTransaction>(wtx), SER_NETWORK, PROTOCOL_VERSION))));
+
+  //Wallet Conflicts
+  UniValue conflicts(UniValue::VARR);
+  BOOST_FOREACH(const uint256& conflict, wtx.GetConflicts())
+      conflicts.push_back(conflict.GetHex());
+  tx.push_back(Pair("walletconflicts", conflicts));
+
+  // Return Type used to determine what is included in the transaction
+  // 0 Spends, Received and spends
+  // 1 Sends
+  // 2 Received
+  // 3 Spends
+  // TODO - Setup Enum, maybe...
+
+  // Add Spends retrieved earlier
+  if (returnType != 2) {
+    Legacy_zsTxSpendsToJSON(wtx, spends, totalSpends, filteredSpends, strAddress, fBool);
+    if ((!fBool || filteredSpends != 0) && (returnType == 0 || returnType == 1)) {
+      tx.push_back(Pair("spends",spends));
+    }
+  }
+  // Get Received
+  if (returnType == 0 || returnType == 2) {
+    Legacy_zsTxReceivedToJSON(wtx, received, totalReceived, strAddress, fBool);
+    if (!fBool || totalReceived != 0) {
+      tx.push_back(Pair("received",received));
+    }
+  }
+
+  // Get Sends
+  if (returnType == 0 || returnType == 3) {
+    //Only include sends if there are spends that belong to the wallet.
+    if (totalSpends != 0 || fBool) {
+      Legacy_zsTxSendsToJSON(wtx, sends, totalSends, strAddress, fBool);
+    }
+    if (!fBool || totalSends != 0) {
+      tx.push_back(Pair("sends",sends));
+    }
+  }
+
+  if ((returnType == 0 && (!fBool || filteredSpends != 0 || totalReceived != 0 || totalSends != 0))
+   || (returnType == 1 && (!fBool || filteredSpends != 0))
+   || (returnType == 2 && (!fBool || totalReceived != 0))
+   || (returnType == 3 && (!fBool || totalSends != 0))) {
+    ret.push_back(tx);
+  }
+
+}
+
+namespace {
+
+std::array<unsigned char, HUSH_MEMO_SIZE> Memo(const std::string& text)
+{
+    std::array<unsigned char, HUSH_MEMO_SIZE> memo;
+    memo.fill(0);
+    if (text == "empty")            // the "no memo" encoding: 0xF6 then zeros
+        memo[0] = 0xF6;
+    else if (text == "binary")      // starts below 0xF5 but isn't UTF-8
+        memo[0] = 0xC3, memo[1] = 0x28;
+    else
+        memcpy(memo.data(), text.data(), text.size());
+    return memo;
+}
+
+// A real encrypted Sapling output, and when ovk is given, an outCiphertext the sender can decrypt with it
+// (the wallet never checks proofs or value commitments).
+OutputDescription Output(const SaplingPaymentAddress& to, uint64_t value, const std::string& memo,
+                         const boost::optional<uint256>& ovk = boost::none)
+{
+    SaplingNote note(to, value);
+    auto enc = SaplingNotePlaintext(note, Memo(memo)).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;
+    if (ovk) {
+        SaplingOutgoingPlaintext out(note.pk_d, enc->second.get_esk());
+        od.outCiphertext = out.encrypt(*ovk, od.cv, od.cm, enc->second);
+    } else {
+        GetRandBytes(od.outCiphertext.begin(), od.outCiphertext.size());
+    }
+    return od;
+}
+
+CTransaction SaplingTx(const std::vector<OutputDescription>& outputs, const std::vector<uint256>& spentNullifiers,
+                       const std::vector<COutPoint>& vin = {})
+{
+    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);
+    }
+    for (const COutPoint& prev : vin)
+        mtx.vin.push_back(CTxIn(prev));
+    return CTransaction(mtx);
+}
+
+SaplingExtendedSpendingKey Key(uint32_t account)
+{
+    std::vector<unsigned char, secure_allocator<unsigned char>> raw(32, 0x6b);
+    HDSeed seed(raw);
+    return SaplingExtendedSpendingKey::Master(seed).Derive(account | ZIP32_HARDENED_KEY_LIMIT);
+}
+
+}
+
+// A wallet whose txs exercise every branch of the history helpers: notes received on default and diversified
+// addresses, by a viewing-key-only address and by a key imported afterwards; z->z sends with change and memos;
+// a t->z shielding send; spends of our notes and of someone else's. pwalletMain points at it during the test.
+class HistoryWallet : public CWallet {
+public:
+    bool EncryptKeys(CKeyingMaterial& vMasterKeyIn) { return CCryptoKeyStore::EncryptKeys(vMasterKeyIn); }
+    bool Unlock(const CKeyingMaterial& vMasterKeyIn) { return CCryptoKeyStore::Unlock(vMasterKeyIn); }
+};
+
+class WalletHistory : public ::testing::Test {
+protected:
+    HistoryWallet wallet;
+    CWallet* savedWallet = nullptr;
+    SaplingPaymentAddress paA, paA2, paB, paC, paD, paF;
+    CTxDestination taddr;
+    std::map<uint256, int> depthOf;
+    std::vector<uint256> txids;
+    CCoinsView coinsBase;
+    CCoinsViewCache coins{&coinsBase};
+    CCoinsViewCache* savedCoinsTip = nullptr;
+    CBlockIndex* savedTip = nullptr;
+    std::deque<CBlock> blocks;
+    std::deque<CBlockIndex> index;
+    std::vector<uint256> insertedBlocks;
+
+    void SetUp() override {
+        SelectParams(CBaseChainParams::MAIN);
+        savedWallet = pwalletMain;
+        pwalletMain = &wallet;
+        savedCoinsTip = pcoinsTip;
+        pcoinsTip = &coins;
+        LOCK2(cs_main, wallet.cs_wallet);
+        savedTip = chainActive.Tip();
+
+        std::vector<unsigned char, secure_allocator<unsigned char>> raw(32, 0x2d);
+        ASSERT_TRUE(wallet.SetHDSeed(HDSeed(raw)));
+        HDSeed seed;
+        ASSERT_TRUE(wallet.GetHDSeedForDerivation(seed));
+        const uint256 seedOvk = ovkForShieldingFromTaddr(seed);
+
+        auto skA = Key(0), skB = Key(1), skC = Key(2), skD = Key(3), skF = Key(9);
+        paA = skA.DefaultAddress();
+        diversifier_index_t j;
+        j.begin()[0] = 50;
+        paA2 = skA.ToXFVK().Address(j)->second;
+        paB = skB.DefaultAddress();
+        paC = skC.DefaultAddress();
+        paD = skD.DefaultAddress();
+        paF = skF.DefaultAddress();
+        const uint256 ovkA = skA.expsk.full_viewing_key().ovk, ovkC = skC.expsk.full_viewing_key().ovk;
+        ASSERT_TRUE(wallet.AddSaplingZKey(skA, paA));
+        ASSERT_TRUE(wallet.AddSaplingIncomingViewingKey(skA.expsk.full_viewing_key().in_viewing_key(), paA2));
+        ASSERT_TRUE(wallet.AddSaplingIncomingViewingKey(skB.expsk.full_viewing_key().in_viewing_key(), paB));
+        ASSERT_TRUE(wallet.AddSaplingZKey(skC, paC));
+
+        CKey tkey;
+        tkey.MakeNewKey(true);
+        ASSERT_TRUE(wallet.AddKeyPubKey(tkey, tkey.GetPubKey()));
+        taddr = tkey.GetPubKey().GetID();
+
+        const uint256 nfA = uint256S("a1"), nfA2 = uint256S("a2"), nfC = uint256S("c1"), nfForeign = uint256S("ff");
+
+        // T0: transparent funds to our taddr.
+        CMutableTransaction t0;
+        t0.vin.push_back(CTxIn(COutPoint(GetRandHash(), 0)));
+        t0.vout.push_back(CTxOut(10 * COIN, GetScriptForDestination(taddr)));
+        AddTx(CTransaction(t0), 30);
+        // T1: received by A, A2, B (viewing key only), C and someone else; no spends.
+        uint256 t1 = AddTx(SaplingTx({Output(paA, 1000, "for A"), Output(paA2, 2000, "empty"), Output(paB, 3000, "for B"),
+                                      Output(paC, 4000, "binary"), Output(paF, 5000, "not ours")}, {}),
+                           25, {{0, nfA}, {1, nfA2}, {3, nfC}});
+        (void)t1;
+        // T2: A sends to F with change back to A, spending A's note: outCiphertexts under A's ovk.
+        AddTx(SaplingTx({Output(paF, 600, "payment", ovkA), Output(paA, 399, "change", ovkA)}, {nfA}), 20);
+        // T3: t->z shielding of T0's output to A, outCiphertext under the seed's shielding ovk.
+        AddTx(SaplingTx({Output(paA, 9 * COIN, "empty", seedOvk)}, {}, {COutPoint(txids[0], 0)}), 15);
+        // T4: C spends its note: to A2 (another of our keys) and change to C.
+        AddTx(SaplingTx({Output(paA2, 1500, "to a diversified A", ovkC), Output(paC, 2499, "binary", ovkC)}, {nfC}), 10);
+        // T5: someone else's spend paying A.
+        AddTx(SaplingTx({Output(paA, 777, "from a stranger")}, {nfForeign}), 8);
+        // T6: received by D before D's key is in the wallet (an import without rescan), then mined.
+        AddTx(SaplingTx({Output(paD, 4242, "for D, found later")}, {}), 5);
+        // T8: spends one of our notes and one of someone else's (a coinjoin-like tx): missingSpendingKeys.
+        AddTx(SaplingTx({Output(paF, 100, "mixed", ovkA)}, {nfA, uint256S("fe")}), 3);
+        // T9: spends A2's note, but its one output's outCiphertext is under nobody's ovk we know: missingSaplingOVK.
+        AddTx(SaplingTx({Output(paF, 50, "unreadable send")}, {nfA2}), 2);
+        // T7: spends A2's note (A and A2 share A's key); never mined and not in the mempool (depth -1).
+        AddTx(SaplingTx({Output(paF, 2900, "", ovkA)}, {nfA2}), 0);
+        ASSERT_TRUE(wallet.AddSaplingZKey(skD, paD));
+
+        Mine();
+    }
+
+    void TearDown() override {
+        LOCK(cs_main);
+        chainActive.SetTip(savedTip);
+        for (const uint256& h : insertedBlocks)
+            mapBlockIndex.erase(h);
+        pcoinsTip = savedCoinsTip;
+        pwalletMain = savedWallet;
+    }
+
+    uint256 AddTx(const CTransaction& tx, int depth, const std::map<uint32_t, uint256>& nullifiers = {}) {
+        CWalletTx wtx(&wallet, tx);
+        mapSaplingNoteData_t found = wallet.FindMySaplingNotes(tx).first;
+        for (const auto& nf : nullifiers)
+            found.at(SaplingOutPoint(tx.GetHash(), nf.first)).nullifier = nf.second;
+        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();
+    }
+
+    void Mine() {
+        const int tip = 40;
+        for (int h = 0; h <= tip; h++) {
+            blocks.emplace_back();
+            CBlock& block = blocks.back();
+            block.nTime = 1700000000 + 36 * h;
+            block.nNonce = uint256S(strprintf("%x", 0x900 + h));
+            if (h > 0)
+                block.hashPrevBlock = blocks[h - 1].GetHash();
+            for (const auto& d : depthOf)
+                if (tip - 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;
+            auto inserted = mapBlockIndex.insert(std::make_pair(block.GetHash(), &bi));
+            ASSERT_TRUE(inserted.second);
+            insertedBlocks.push_back(block.GetHash());
+            bi.phashBlock = &inserted.first->first;
+        }
+        chainActive.SetTip(&index[tip]);
+        for (const auto& d : depthOf)
+            wallet.mapWallet.at(d.first).SetMerkleBranch(blocks[tip - d.second + 1]);
+    }
+
+    std::string NewJSON(const CWalletTx& wtx, const std::string& addr, bool fBool, int returnType) {
+        UniValue ret(UniValue::VARR);
+        zsWalletTxJSON(wtx, ret, addr, fBool, returnType, GetSaplingHistoryKeys());
+        return ret.write();
+    }
+    std::string LegacyJSON(const CWalletTx& wtx, const std::string& addr, bool fBool, int returnType) {
+        UniValue ret(UniValue::VARR);
+        Legacy_zsWalletTxJSON(wtx, ret, addr, fBool, returnType);
+        return ret.write();
+    }
+    void ExpectMatchesLegacy(bool checkCoverage = true);
+};
+
+// Every tx, every return type, unfiltered and filtered by each kind of address: byte-identical JSON. Run with
+// the wallet's plaintext cache warm (as after the startup warm-up) and cold (every output trial-decrypted).
+class WalletHistoryCold : public WalletHistory {};
+
+TEST_F(WalletHistoryCold, MatchesLegacy) {
+    LOCK2(cs_main, wallet.cs_wallet);
+    for (auto& p : wallet.mapWallet)
+        for (auto& item : p.second.mapSaplingNoteData)
+            item.second.cachedPlaintext = boost::none;
+    ExpectMatchesLegacy();
+}
+
+TEST_F(WalletHistory, MatchesLegacy) {
+    LOCK2(cs_main, wallet.cs_wallet);
+    ExpectMatchesLegacy();
+}
+
+void WalletHistory::ExpectMatchesLegacy(bool checkCoverage) {
+    const std::vector<std::pair<std::string, bool>> filters = {
+        {"*", false}, {"*", true}, {EncodePaymentAddress(paA), true}, {EncodePaymentAddress(paA2), true},
+        {EncodePaymentAddress(paB), true}, {EncodePaymentAddress(paC), true}, {EncodePaymentAddress(paD), true},
+        {EncodePaymentAddress(paF), true}, {EncodeDestination(taddr), true},
+    };
+    std::string all;
+    for (const uint256& txid : txids) {
+        const CWalletTx& wtx = wallet.mapWallet.at(txid);
+        for (int returnType = 0; returnType <= 3; returnType++) {
+            for (const auto& f : filters) {
+                // New first: the legacy spends helper default-inserts into wallet maps.
+                std::string got = NewJSON(wtx, f.first, f.second, returnType);
+                std::string want = LegacyJSON(wtx, f.first, f.second, returnType);
+                EXPECT_EQ(want, got) << "tx " << txid.ToString() << " returnType " << returnType << " filter " << f.first << "/" << f.second;
+                all += got;
+            }
+        }
+    }
+    // The fixture really reaches the interesting branches.
+    if (!checkCoverage)
+        return;
+    for (const char* needle : {"\"change\":true", "\"memoStr\"", "\"saplingSpends\":[{", "\"transparentSpends\":[{",
+                               "\"saplingSends\":[{", "\"saplingReceived\":[{", "\"missingSaplingOVK\":true",
+                               "\"missingSpendingKeys\":true", "for D, found later"}) {
+        EXPECT_NE(std::string::npos, all.find(needle)) << needle;
+    }
+}
+
+// The same comparison on an encrypted wallet, locked (no spending keys or HD seed available: no shielded
+// received or sent entries, as before) and then unlocked.
+class WalletHistoryEncrypted : public WalletHistory {};
+
+TEST_F(WalletHistoryEncrypted, MatchesLegacyLockedAndUnlocked) {
+    LOCK2(cs_main, wallet.cs_wallet);
+    CKeyingMaterial masterKey(32, 0x5e);
+    ASSERT_TRUE(wallet.EncryptKeys(masterKey));
+    ASSERT_TRUE(wallet.IsLocked());
+    ASSERT_TRUE(GetSaplingHistoryKeys().spendable.empty());
+    ExpectMatchesLegacy(false);
+    ASSERT_TRUE(wallet.Unlock(masterKey));
+    ASSERT_FALSE(GetSaplingHistoryKeys().spendable.empty());
+    ExpectMatchesLegacy();
+}
+
+// The history helpers only read the wallet. The old spends helper's operator[] lookups inserted an entry into
+// mapSaplingNullifiersToNotes for every other wallet's nullifier they met, which made a later
+// z_viewtransaction of that tx fail (map::at on a null outpoint).
+TEST_F(WalletHistory, DoesNotModifyTheWallet) {
+    LOCK2(cs_main, wallet.cs_wallet);
+    const size_t nullifierEntries = wallet.mapSaplingNullifiersToNotes.size();
+    std::map<uint256, size_t> noteEntries;
+    for (const auto& p : wallet.mapWallet)
+        noteEntries[p.first] = p.second.mapSaplingNoteData.size();
+
+    for (const uint256& txid : txids)
+        for (int returnType = 0; returnType <= 3; returnType++)
+            NewJSON(wallet.mapWallet.at(txid), "*", false, returnType);
+    EXPECT_EQ(nullifierEntries, wallet.mapSaplingNullifiersToNotes.size());
+    EXPECT_EQ(0u, wallet.mapSaplingNullifiersToNotes.count(uint256S("ff"))) << "someone else's spend";
+    for (const auto& p : wallet.mapWallet)
+        EXPECT_EQ(noteEntries.at(p.first), p.second.mapSaplingNoteData.size());
+
+    // What the old helpers did, for the record.
+    for (const uint256& txid : txids)
+        LegacyJSON(wallet.mapWallet.at(txid), "*", false, 0);
+    EXPECT_EQ(1u, wallet.mapSaplingNullifiersToNotes.count(uint256S("ff")));
+}

src/gtest/test_wallet_notedata.cpp

@@ -4,9 +4,18 @@
 
 #include <gtest/gtest.h>
 
+#include <deque>
+
+#include "chain.h"
+#include "coins.h"
+#include "main.h"
+#include "primitives/block.h"
 #include "primitives/transaction.h"
+#include "random.h"
 #include "uint256.h"
 #include "wallet/wallet.h"
+#include "zcash/Note.hpp"
+#include "zcash/zip32.h"
 
 namespace {
 
@@ -15,6 +24,9 @@ public:
     bool UpdatedNoteData(const CWalletTx& wtxIn, CWalletTx& wtx, bool fSameBlock = true) {
         return CWallet::UpdatedNoteData(wtxIn, wtx, fSameBlock);
     }
+    int VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessOnly) {
+        return CWallet::VerifyAndSetInitialWitness(pindex, witnessOnly);
+    }
 };
 
 libzcash::SaplingIncomingViewingKey TestIvk()
@@ -153,3 +165,108 @@ TEST(WalletNoteData, EmptyIncomingIsNoUpdate) {
     EXPECT_EQ(1u, wtx.mapSaplingNoteData.size());
     EXPECT_EQ(400, wtx.mapSaplingNoteData.at(op0).witnessHeight);
 }
+
+namespace {
+
+// A real encrypted Sapling output to pa (the wallet never checks its proof or value commitment).
+OutputDescription EncryptedOutput(const libzcash::SaplingPaymentAddress& pa, uint64_t value)
+{
+    libzcash::SaplingNote note(pa, value);
+    std::array<unsigned char, HUSH_MEMO_SIZE> memo;
+    memo.fill(0);
+    auto enc = libzcash::SaplingNotePlaintext(note, memo).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;
+}
+
+// A three-block active chain (heights 0-2) whose block 1 holds the given txs, undone on destruction:
+// chainActive, mapBlockIndex and pcoinsTip are process-wide and every gtest suite shares the process.
+struct FakeActiveChain {
+    std::deque<CBlock> blocks;
+    std::deque<CBlockIndex> index;
+    CBlockIndex* savedTip;
+    CCoinsView coinsBase;
+    CCoinsViewCache coins{&coinsBase};
+    CCoinsViewCache* savedCoinsTip;
+
+    explicit FakeActiveChain(const std::vector<CTransaction>& inBlock1) {
+        LOCK(cs_main);
+        savedTip = chainActive.Tip();
+        savedCoinsTip = pcoinsTip;
+        pcoinsTip = &coins;
+        for (int h = 0; h <= 2; h++) {
+            blocks.emplace_back();
+            CBlock& block = blocks.back();
+            block.nTime = 1700000000 + h;
+            block.nNonce = uint256S(strprintf("%x", 0x700 + h));
+            if (h > 0)
+                block.hashPrevBlock = blocks[h - 1].GetHash();
+            if (h == 1)
+                block.vtx = inBlock1;
+            block.hashMerkleRoot = block.BuildMerkleTree();
+            index.emplace_back(block);
+            CBlockIndex& bi = index.back();
+            bi.SetHeight(h);
+            bi.pprev = h > 0 ? &index[h - 1] : nullptr;
+            bi.phashBlock = &mapBlockIndex.insert(std::make_pair(block.GetHash(), &bi)).first->first;
+        }
+        chainActive.SetTip(&index.back());
+    }
+    ~FakeActiveChain() {
+        LOCK(cs_main);
+        chainActive.SetTip(savedTip);
+        for (const CBlock& block : blocks)
+            mapBlockIndex.erase(block.GetHash());
+        pcoinsTip = savedCoinsTip;
+    }
+};
+
+}
+
+// A note found with an incoming viewing key only (z_importviewingkey) can't be spent and never gets
+// a nullifier, because deriving one takes the full viewing key. VerifyAndSetInitialWitness must leave
+// it alone. It used to clear it and reseed its witness from the block on disk on every call, then
+// abort on the missing nullifier the first time the note was confirmed. Here there are no block
+// files, so the old reseed path throws.
+TEST(WalletNoteData, ViewingKeyOnlyNoteIsNotWitnessed) {
+    NoteDataWallet wallet;
+    std::vector<unsigned char, secure_allocator<unsigned char>> raw(32, 0x3c);
+    HDSeed seed(raw);
+    auto sk = libzcash::SaplingExtendedSpendingKey::Master(seed);
+    auto pa = sk.DefaultAddress();
+
+    LOCK2(cs_main, wallet.cs_wallet);
+    ASSERT_TRUE(wallet.AddSaplingIncomingViewingKey(sk.expsk.full_viewing_key().in_viewing_key(), pa));
+    ASSERT_FALSE(wallet.HaveSaplingFullViewingKey(sk.expsk.full_viewing_key().in_viewing_key()));
+
+    CMutableTransaction mtx;
+    mtx.fOverwintered = true;
+    mtx.nVersion = SAPLING_TX_VERSION;
+    mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID;
+    mtx.vShieldedOutput.push_back(EncryptedOutput(pa, 5000));
+    CTransaction tx(mtx);
+
+    CWalletTx wtx(&wallet, tx);
+    mapSaplingNoteData_t found = wallet.FindMySaplingNotes(tx).first;
+    ASSERT_EQ(1u, found.size()) << "the viewing key finds the note";
+    wtx.SetSaplingNoteData(found);
+    ASSERT_TRUE(wallet.AddToWallet(wtx, true, nullptr));
+
+    FakeActiveChain chain({tx});
+    CWalletTx& stored = wallet.mapWallet.at(tx.GetHash());
+    stored.SetMerkleBranch(chain.blocks[1]);
+    ASSERT_EQ(2, stored.GetDepthInMainChain());
+
+    int minHeight = -1;
+    ASSERT_NO_THROW(minHeight = wallet.VerifyAndSetInitialWitness(chainActive.Tip(), false));
+    EXPECT_EQ(chainActive.Height(), minHeight) << "it must not pull the witness rebuild down to its height";
+    const SaplingNoteData& nd = stored.mapSaplingNoteData.at(SaplingOutPoint(tx.GetHash(), 0));
+    EXPECT_TRUE(nd.witnesses.empty());
+    EXPECT_FALSE(static_cast<bool>(nd.nullifier));
+    EXPECT_EQ(-1, nd.witnessHeight);
+}

src/init.cpp

@@ -513,6 +513,10 @@ std::string HelpMessage(HelpMessageMode mode)
     // If this is used incorrectly (-rescanheight too large), then the local wallet may attempt to spend funds which it does not have witness data about
     // which will cause a "missing inputs" error when added to the mempool. Rescanning from correct height will fix this.
     strUsage += HelpMessageOpt("-keepnotewitnesscache", _("Keep partial Sapling Note Witness cache. Must be used with -rescanheight to find missing cache items."));
+    if (showDebug)
+        strUsage += HelpMessageOpt("-notecachewarmup", "Decrypt all wallet notes into memory at startup, so the first balance call after a restart doesn't (default: 1)");
+    if (showDebug)
+        strUsage += HelpMessageOpt("-checknotecache", "On every balance or note-listing call, decrypt each cached note it considers again and log and repair any difference from the cache (slow; default: 0)");
     strUsage += HelpMessageOpt("-rescan", _("Rescan the block chain for missing wallet transactions") + " " + _("on startup"));
     strUsage += HelpMessageOpt("-rescanheight", _("Rescan from specified height when rescan=1 on startup"));
     strUsage += HelpMessageOpt("-salvagewallet", _("Attempt to recover private keys from a corrupt wallet.dat") + " " + _("on startup"));
@@ -2504,6 +2508,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: fully shielded notes, which versions before "
+                      "v1.3.1 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.
@@ -2698,6 +2710,15 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
                 }
             }
         }
+        // Decrypt the wallet's notes into the in-memory plaintext cache now, while RPC is still in warmup
+        // and before the wallet notification thread (started below) can want cs_wallet. Otherwise the
+        // first balance call after a restart decrypts them all under cs_main.
+        fCheckNoteCache = GetBoolArg("-checknotecache", false);
+        if (GetBoolArg("-notecachewarmup", true) && !ShutdownRequested()) {
+            uiInterface.InitMessage(_("Decrypting wallet notes..."));
+            pwalletMain->WarmSaplingNoteCache();
+        }
+
         pwalletMain->SetBroadcastTransactions(GetBoolArg("-walletbroadcast", true));
     } // (!fDisableWallet)
 #else // ENABLE_WALLET

src/rpc/server.cpp

@@ -26,6 +26,7 @@
 #include "util.h"
 #include "util/strencodings.h"
 #include "asyncrpcqueue.h"
+#include <chrono>
 #include <memory>
 #include <univalue.h>
 #include <boost/bind.hpp>
@@ -640,6 +641,30 @@ std::string JSONRPCExecBatch(const UniValue& vReq)
     return ret.write() + "\n";
 }
 
+namespace {
+// Logs an RPC whose wall time reached 500ms, or every RPC under -debug=bench. That time includes
+// waiting for cs_main / cs_wallet behind block connection or another RPC, which the callee's own
+// timers can't see because they start once the locks are held. A long-poll getblocktemplate waits
+// for a new block with cs_main released, so it is only logged under -debug=bench. The method is
+// logged as method=<name>: a bare " <name> " token would match the GUI's " rescan " log heuristic.
+class RPCCallTimer {
+    const std::string& strMethod;
+    const bool fLongPoll;
+    const std::chrono::steady_clock::time_point start;
+public:
+    RPCCallTimer(const std::string& method, const UniValue& params)
+        : strMethod(method),
+          fLongPoll(method == "getblocktemplate" && params.size() > 0 && params[0].isObject() &&
+                    !find_value(params[0], "longpollid").isNull()),
+          start(std::chrono::steady_clock::now()) {}
+    ~RPCCallTimer() {
+        const int64_t nMs = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - start).count();
+        if ((nMs >= 500 && !fLongPoll) || LogAcceptCategory("bench"))
+            LogPrintf("RPC method=%s took %dms\n", strMethod, nMs);
+    }
+};
+}
+
 UniValue CRPCTable::execute(const std::string &strMethod, const UniValue &params) const
 {
     // Find method
@@ -674,6 +699,7 @@ UniValue CRPCTable::execute(const std::string &strMethod, const UniValue &params
     try
     {
         // Execute
+        RPCCallTimer timer(strMethod, params);
         return pcmd->actor(params, false, CPubKey());
     }
     catch (const std::exception& e)

src/wallet/crypter.cpp

@@ -591,7 +591,7 @@ bool CCryptoKeyStore::GetSaplingSpendingKey(const libzcash::SaplingFullViewingKe
         if (!IsCrypted())
             return CBasicKeyStore::GetSaplingSpendingKey(fvk, skOut);
 
-        for (auto entry : mapCryptedSaplingSpendingKeys) {
+        for (const auto& entry : mapCryptedSaplingSpendingKeys) {
             if (entry.first.fvk == fvk) {
                 const std::vector<unsigned char> &vchCryptedSecret = entry.second;
                 return DecryptSaplingSpendingKey(vMasterKey, vchCryptedSecret, entry.first, skOut);

src/wallet/crypter.h

@@ -265,7 +265,7 @@ public:
             LOCK(cs_SpendingKeyStore);
             if (!IsCrypted())
                 return CBasicKeyStore::HaveSaplingSpendingKey(fvk);
-            for (auto entry : mapCryptedSaplingSpendingKeys) {
+            for (const auto& entry : mapCryptedSaplingSpendingKeys) {
                 if (entry.first.fvk == fvk) {
                     return true;
                 }

src/wallet/rpchushwallet.cpp

@@ -16,6 +16,109 @@ using namespace libzcash;
 
 bool EnsureWalletIsAvailable(bool avoidException);
 
+SaplingHistoryKeys GetSaplingHistoryKeys()
+{
+  AssertLockHeld(pwalletMain->cs_wallet);
+  SaplingHistoryKeys keys;
+  std::set<libzcash::SaplingPaymentAddress> addresses;
+  pwalletMain->GetSaplingPaymentAddresses(addresses);
+  for (const auto& addr : addresses) {
+    libzcash::SaplingExtendedSpendingKey extsk;
+    if (pwalletMain->GetSaplingExtendedSpendingKey(addr, extsk)) {
+      const libzcash::SaplingFullViewingKey fvk = extsk.expsk.full_viewing_key();
+      SaplingHistoryKeys::SpendableAddress a;
+      a.addr = addr;
+      a.ivk = fvk.in_viewing_key();
+      a.ovk = fvk.ovk;
+      // Left zero when the wallet doesn't know addr, exactly as GetNullifiersForAddresses does.
+      pwalletMain->GetSaplingIncomingViewingKey(addr, a.walletIvk);
+      keys.spendable.push_back(a);
+    }
+  }
+  return keys;
+}
+
+bool SaplingHistoryKeys::IsNullifierOfAddress(const libzcash::SaplingIncomingViewingKey& addrIvk, const uint256& nullifier) const
+{
+  AssertLockHeld(pwalletMain->cs_wallet);
+  if (!noteNullifiers) {
+    noteNullifiers = std::set<std::pair<libzcash::SaplingIncomingViewingKey, uint256>>();
+    for (const auto& p : pwalletMain->mapWallet) {
+      for (const auto& item : p.second.mapSaplingNoteData) {
+        if (item.second.nullifier)
+          noteNullifiers->insert(std::make_pair(item.second.ivk, *item.second.nullifier));
+      }
+    }
+  }
+  return noteNullifiers->count(std::make_pair(addrIvk, nullifier)) > 0;
+}
+
+namespace {
+
+// One output's decryptions under incoming viewing keys, each key tried at most once. Two distinct ivks can't
+// both decrypt an output: the cm check recomputes pk_d = [ivk]g_d, and ivks are canonical scalars below the
+// order of g_d (ka_agree and ivk_to_pkd reject the rest), so a second one would need a NoteCommit collision.
+// So once it is known which ivk decrypts the output (the wallet's note data records the one FindMySaplingNotes
+// found, with its plaintext normally cached), no other ivk is tried.
+class OutputDecryptions {
+  const OutputDescription& output;
+  const SaplingNoteData* nd;
+  std::map<libzcash::SaplingIncomingViewingKey, boost::optional<SaplingNoteCache>> tried;
+
+  boost::optional<SaplingNoteCache> Decrypt(const libzcash::SaplingIncomingViewingKey& ivk) const {
+    auto both = libzcash::SaplingNotePlaintext::decrypt_with_note(output.encCiphertext, ivk, output.ephemeralKey, output.cm);
+    if (!both)
+      return boost::none;
+    return SaplingNoteCache{ both->second, both->first.memo() };
+  }
+  // The plaintext under the ivk the wallet recorded for this output, decrypting it if it isn't cached.
+  const boost::optional<SaplingNoteCache>& Recorded() {
+    auto it = tried.find(nd->ivk);
+    if (it == tried.end())
+      it = tried.insert(std::make_pair(nd->ivk, nd->cachedPlaintext ? nd->cachedPlaintext : Decrypt(nd->ivk))).first;
+    return it->second;
+  }
+public:
+  OutputDecryptions(const CWalletTx& wtx, uint32_t n) : output(wtx.vShieldedOutput[n]), nd(nullptr) {
+    auto it = wtx.mapSaplingNoteData.find(SaplingOutPoint(wtx.GetHash(), n));
+    if (it != wtx.mapSaplingNoteData.end())
+      nd = &it->second;
+  }
+  const SaplingNoteCache* Get(const libzcash::SaplingIncomingViewingKey& ivk) {
+    auto it = tried.find(ivk);
+    if (it == tried.end()) {
+      boost::optional<SaplingNoteCache> result;
+      if (nd && nd->ivk == ivk) {
+        result = Recorded();
+      } else if (!(nd && Recorded())) {
+        // Not a wallet note, or one whose recorded ivk doesn't decrypt it: try this key.
+        result = Decrypt(ivk);
+      }
+      it = tried.insert(std::make_pair(ivk, result)).first;
+    }
+    return it->second ? &*it->second : nullptr;
+  }
+};
+
+// What an outCiphertext tells the sender holding ovk: the note's plaintext and the address it was sent to.
+boost::optional<std::pair<libzcash::SaplingNotePlaintext, libzcash::SaplingPaymentAddress>> DecryptSent(
+    const OutputDescription& outputDesc, const uint256& ovk)
+{
+  auto opt = libzcash::SaplingOutgoingPlaintext::decrypt(
+          outputDesc.outCiphertext,ovk,outputDesc.cv,outputDesc.cm,outputDesc.ephemeralKey);
+  if (!opt)
+    return boost::none;
+  auto opt_unwrapped = opt.get();
+  auto pt = libzcash::SaplingNotePlaintext::decrypt(
+          outputDesc.encCiphertext,outputDesc.ephemeralKey,opt_unwrapped.esk,opt_unwrapped.pk_d,outputDesc.cm);
+  if (!pt)
+    return boost::none;
+  auto pt_unwrapped = pt.get();
+  return std::make_pair(pt_unwrapped, libzcash::SaplingPaymentAddress(pt_unwrapped.d, opt_unwrapped.pk_d));
+}
+
+}
+
 void zsTxSpendsToJSON(const CWalletTx& wtx, UniValue& spends, CAmount& totalSpends, CAmount& filteredSpends, const std::string& strAddress, bool filterByAddress) {
 
   LOCK2(cs_main, pwalletMain->cs_wallet);
@@ -87,18 +190,29 @@ void zsTxSpendsToJSON(const CWalletTx& wtx, UniValue& spends, CAmount& totalSpen
       const SpendDescription& spendDesc = wtx.vShieldedSpend[i];
       UniValue obj(UniValue::VOBJ);
 
-      SaplingOutPoint op = pwalletMain->mapSaplingNullifiersToNotes[spendDesc.nullifier];
+      // Lookups by find(), not operator[]: that inserted an entry for every other wallet's nullifier it met
-
+      // (and could insert an empty note into a wallet tx).
       if (pwalletMain->IsSaplingNullifierFromMe(spendDesc.nullifier)) {
-        const CWalletTx* parent = pwalletMain->GetWalletTx(pwalletMain->mapSaplingNullifiersToNotes[spendDesc.nullifier].hash);
+        const SaplingOutPoint op = pwalletMain->mapSaplingNullifiersToNotes.at(spendDesc.nullifier);
-        const OutputDescription& output = parent->vShieldedOutput[op.n];
+        const CWalletTx* parent = pwalletMain->GetWalletTx(op.hash);
-        auto nd = pwalletMain->mapWallet[pwalletMain->mapSaplingNullifiersToNotes[spendDesc.nullifier].hash].mapSaplingNoteData[op];
+        auto ndIt = parent->mapSaplingNoteData.find(op);
-        auto pt = libzcash::SaplingNotePlaintext::decrypt(output.encCiphertext,nd.ivk,output.ephemeralKey,output.cm);
+        // The spent note's plaintext, from the wallet's cache when it has it.
+        boost::optional<SaplingNoteCache> pt;
+        if (ndIt != parent->mapSaplingNoteData.end() && op.n < parent->vShieldedOutput.size()) {
+          const SaplingNoteData& nd = ndIt->second;
+          if (nd.cachedPlaintext) {
+            pt = nd.cachedPlaintext;
+          } else {
+            const OutputDescription& output = parent->vShieldedOutput[op.n];
+            auto both = libzcash::SaplingNotePlaintext::decrypt_with_note(output.encCiphertext,nd.ivk,output.ephemeralKey,output.cm);
+            if (both)
+              pt = SaplingNoteCache{ both->second, both->first.memo() };
+          }
+        }
 
         if (pt) {
-          auto note = pt.get();
+          const libzcash::SaplingNote& note = pt->note;
-          auto pa = nd.ivk.address(note.d);
+          const libzcash::SaplingPaymentAddress address(note.d, note.pk_d);
-          auto address = pa.get();
           shieldedSpendsSpentCount++;
           totalSpends += CAmount(-note.value());
           obj.push_back(Pair("address",EncodePaymentAddress(address)));
@@ -132,7 +246,7 @@ void zsTxSpendsToJSON(const CWalletTx& wtx, UniValue& spends, CAmount& totalSpen
 }
 
 
-void zsTxReceivedToJSON(const CWalletTx& wtx, UniValue& received, CAmount& totalReceived, const std::string& strAddress, bool filterByAddress) {
+void zsTxReceivedToJSON(const CWalletTx& wtx, UniValue& received, CAmount& totalReceived, const std::string& strAddress, bool filterByAddress, const SaplingHistoryKeys& keys) {
 
   if(fZdebug)
     fprintf(stderr,"%s: txid=%s\n", __func__, wtx.GetHash().ToString().c_str() );
@@ -188,22 +302,18 @@ void zsTxReceivedToJSON(const CWalletTx& wtx, UniValue& received, CAmount& total
   UniValue zsReceived(UniValue::VARR);
   if (isZsAddress || !filterByAddress) {
     for (int i = 0; i < wtx.vShieldedOutput.size(); i++) {
-      const OutputDescription& outputDesc = wtx.vShieldedOutput[i];
       UniValue obj(UniValue::VOBJ);
       bool changeTx = false;
-      //Decrypt sapling incoming commitments using IVK
+      //Decrypt sapling incoming commitments using IVK: once per key, not once per address
-      std::set<libzcash::SaplingPaymentAddress> addresses;
+      OutputDecryptions decryptions(wtx, i);
-      pwalletMain->GetSaplingPaymentAddresses(addresses);
+      for (const SaplingHistoryKeys::SpendableAddress& a : keys.spendable) {
-      for (auto addr : addresses) {
+        const libzcash::SaplingPaymentAddress& addr = a.addr;
-        libzcash::SaplingExtendedSpendingKey extsk;
+        {
-        if (pwalletMain->GetSaplingExtendedSpendingKey(addr, extsk)) {
+          const SaplingNoteCache* pt = decryptions.Get(a.ivk);
-          auto pt = libzcash::SaplingNotePlaintext::decrypt(
-                  outputDesc.encCiphertext, extsk.expsk.full_viewing_key().in_viewing_key(), outputDesc.ephemeralKey, outputDesc.cm);
 
           if (pt) {
-            auto note = pt.get();
+            const libzcash::SaplingNote& note = pt->note;
-            auto pt_unwrapped = pt.get();
+            auto memo = pt->memo;
-            auto memo = pt_unwrapped.memo();
             obj.push_back(Pair("address",EncodePaymentAddress(addr)));
             obj.push_back(Pair("amount", ValueFromAmount(CAmount(note.value()))));
             obj.push_back(Pair("memo", HexStr(memo)));
@@ -219,10 +329,8 @@ void zsTxReceivedToJSON(const CWalletTx& wtx, UniValue& received, CAmount& total
 
             //Check Change Status
             if (wtx.vShieldedSpend.size()!=0) {
-              std::set<std::pair<PaymentAddress, uint256>> nullifierSet;
-              nullifierSet = pwalletMain->GetNullifiersForAddresses({addr});
               BOOST_FOREACH(const SpendDescription& spendDesc, wtx.vShieldedSpend) {
-                if (nullifierSet.count(std::make_pair(addr, spendDesc.nullifier))) {
+                if (keys.IsNullifierOfAddress(a.walletIvk, spendDesc.nullifier)) {
                     changeTx = true;
                 }
               }
@@ -243,7 +351,7 @@ void zsTxReceivedToJSON(const CWalletTx& wtx, UniValue& received, CAmount& total
 }
 
 
-void zsTxSendsToJSON(const CWalletTx& wtx, UniValue& sends, CAmount& totalSends, const std::string& strAddress, bool filterByAddress) {
+void zsTxSendsToJSON(const CWalletTx& wtx, UniValue& sends, CAmount& totalSends, const std::string& strAddress, bool filterByAddress, const SaplingHistoryKeys& keys) {
 
   LOCK2(cs_main, pwalletMain->cs_wallet);
 
@@ -296,6 +404,13 @@ void zsTxSendsToJSON(const CWalletTx& wtx, UniValue& sends, CAmount& totalSends,
   //All Shielded Sends in the transaction
   UniValue zsSends(UniValue::VARR);
   if (isZsAddress || !filterByAddress) {
+    //The t to z shielding ovk comes from the HD seed: derive it once per tx, not once per output
+    boost::optional<uint256> seedOvk;
+    if (wtx.vShieldedSpend.size()==0 && wtx.vShieldedOutput.size()!=0) {
+      HDSeed seed;
+      if (pwalletMain->GetHDSeedForDerivation(seed))
+        seedOvk = ovkForShieldingFromTaddr(seed);
+    }
     for (int i = 0; i < wtx.vShieldedOutput.size(); i++) {
       const OutputDescription& outputDesc = wtx.vShieldedOutput[i];
       shieldedOutputCount++;
@@ -304,20 +419,14 @@ void zsTxSendsToJSON(const CWalletTx& wtx, UniValue& sends, CAmount& totalSends,
 
       //Decrypt sapling outgoing t to z transaction using HDseed
       if (wtx.vShieldedSpend.size()==0) {
-        HDSeed seed;
+        if (seedOvk) {
-        if (pwalletMain->GetHDSeedForDerivation(seed)) {
+          {
-          auto opt = libzcash::SaplingOutgoingPlaintext::decrypt(
+            auto sent = DecryptSent(outputDesc, *seedOvk);
-                  outputDesc.outCiphertext,ovkForShieldingFromTaddr(seed),outputDesc.cv,outputDesc.cm,outputDesc.ephemeralKey);
 
-          if (opt) {
+            if (sent) {
-            auto opt_unwrapped = opt.get();
-            auto pt = libzcash::SaplingNotePlaintext::decrypt(
-                    outputDesc.encCiphertext,outputDesc.ephemeralKey,opt_unwrapped.esk,opt_unwrapped.pk_d,outputDesc.cm);
-
-            if (pt) {
               shieldedOutputDecryptedCount++;
-              auto pt_unwrapped = pt.get();
+              const libzcash::SaplingNotePlaintext& pt_unwrapped = sent->first;
-              libzcash::SaplingPaymentAddress sentAddr(pt_unwrapped.d, opt_unwrapped.pk_d);
+              const libzcash::SaplingPaymentAddress& sentAddr = sent->second;
               obj.push_back(Pair("address",EncodePaymentAddress(sentAddr)));
               obj.push_back(Pair("amount", ValueFromAmount(CAmount(pt_unwrapped.value()))));
               obj.push_back(Pair("shieldedOutputIndex",i));
@@ -330,26 +439,22 @@ void zsTxSendsToJSON(const CWalletTx& wtx, UniValue& sends, CAmount& totalSends,
           }
         }
 
-      //attempt Decryption of Outgoing Sapling using wallet extended spending keys
+      //attempt Decryption of Outgoing Sapling using wallet extended spending keys: once per ovk, not once per address
       } else {
-        std::set<libzcash::SaplingPaymentAddress> addresses;
+        std::map<uint256, boost::optional<std::pair<libzcash::SaplingNotePlaintext, libzcash::SaplingPaymentAddress>>> sentByOvk;
-        pwalletMain->GetSaplingPaymentAddresses(addresses);
+        for (const SaplingHistoryKeys::SpendableAddress& a : keys.spendable) {
-        for (auto addr : addresses) {
+          {
-          libzcash::SaplingExtendedSpendingKey extsk;
+            auto tried = sentByOvk.find(a.ovk);
-          if (pwalletMain->GetSaplingExtendedSpendingKey(addr, extsk)) {
+            if (tried == sentByOvk.end())
-            auto opt = libzcash::SaplingOutgoingPlaintext::decrypt(
+              tried = sentByOvk.insert(std::make_pair(a.ovk, DecryptSent(outputDesc, a.ovk))).first;
-                    outputDesc.outCiphertext,extsk.expsk.full_viewing_key().ovk,outputDesc.cv,outputDesc.cm,outputDesc.ephemeralKey);
+            const auto& sent = tried->second;
 
-            if (opt) {
+            {
-              auto opt_unwrapped = opt.get();
+              if (sent) {
-              auto pt = libzcash::SaplingNotePlaintext::decrypt(
+                const libzcash::SaplingNotePlaintext& pt_unwrapped = sent->first;
-                      outputDesc.encCiphertext,outputDesc.ephemeralKey,opt_unwrapped.esk,opt_unwrapped.pk_d,outputDesc.cm);
-
-              if (pt) {
-                auto pt_unwrapped = pt.get();
                 auto memo = pt_unwrapped.memo();
                 shieldedOutputDecryptedCount++;
-                libzcash::SaplingPaymentAddress sentAddr(pt_unwrapped.d, opt_unwrapped.pk_d);
+                const libzcash::SaplingPaymentAddress& sentAddr = sent->second;
                 obj.push_back(Pair("address",EncodePaymentAddress(sentAddr)));
                 obj.push_back(Pair("amount", ValueFromAmount(CAmount(-pt_unwrapped.value()))));
                 obj.push_back(Pair("memo", HexStr(memo)));
@@ -365,10 +470,10 @@ void zsTxSendsToJSON(const CWalletTx& wtx, UniValue& sends, CAmount& totalSends,
 
                 //Check Change Status
                 if (wtx.vShieldedSpend.size()!=0) {
-                  std::set<std::pair<PaymentAddress, uint256>> nullifierSet;
+                  libzcash::SaplingIncomingViewingKey sentIvk;
-                  nullifierSet = pwalletMain->GetNullifiersForAddresses({sentAddr});
+                  pwalletMain->GetSaplingIncomingViewingKey(sentAddr, sentIvk);
                   BOOST_FOREACH(const SpendDescription& spendDesc, wtx.vShieldedSpend) {
-                    if (nullifierSet.count(std::make_pair(sentAddr, spendDesc.nullifier))) {
+                    if (keys.IsNullifierOfAddress(sentIvk, spendDesc.nullifier)) {
                         changeTx = true;
                     }
                   }
@@ -397,7 +502,7 @@ void zsTxSendsToJSON(const CWalletTx& wtx, UniValue& sends, CAmount& totalSends,
 }
 
 
-void zsWalletTxJSON(const CWalletTx& wtx, UniValue& ret, const std::string strAddress, bool fBool, const int returnType) {
+void zsWalletTxJSON(const CWalletTx& wtx, UniValue& ret, const std::string strAddress, bool fBool, const int returnType, const SaplingHistoryKeys& keys) {
 
   LOCK2(cs_main, pwalletMain->cs_wallet);
 
@@ -469,7 +574,7 @@ void zsWalletTxJSON(const CWalletTx& wtx, UniValue& ret, const std::string strAd
   }
   // Get Received
   if (returnType == 0 || returnType == 2) {
-    zsTxReceivedToJSON(wtx, received, totalReceived, strAddress, fBool);
+    zsTxReceivedToJSON(wtx, received, totalReceived, strAddress, fBool, keys);
     if (!fBool || totalReceived != 0) {
       tx.push_back(Pair("received",received));
     }
@@ -479,7 +584,7 @@ void zsWalletTxJSON(const CWalletTx& wtx, UniValue& ret, const std::string strAd
   if (returnType == 0 || returnType == 3) {
     //Only include sends if there are spends that belong to the wallet.
     if (totalSpends != 0 || fBool) {
-      zsTxSendsToJSON(wtx, sends, totalSends, strAddress, fBool);
+      zsTxSendsToJSON(wtx, sends, totalSends, strAddress, fBool, keys);
     }
     if (!fBool || totalSends != 0) {
       tx.push_back(Pair("sends",sends));

src/wallet/rpchushwallet.h

@@ -13,10 +13,35 @@ struct balancestruct {
   CAmount immature;
 };
 
+/** The wallet's Sapling keys as the history helpers use them, gathered once per RPC call. The helpers used to
+ *  look them up for every output and every address, re-deriving each full viewing key (an elliptic-curve
+ *  operation), and walked every note in the wallet for each change check. Build it with pwalletMain->cs_wallet
+ *  held, and keep holding it while the helpers run: that keeps the wallet's keys and notes fixed. The one thing
+ *  it can't stop is the walletpassphrase timer relocking the wallet mid-call; the call then reports the keys as
+ *  they were when it started, where the old per-output lookups switched over partway. */
+struct SaplingHistoryKeys {
+    struct SpendableAddress {
+        libzcash::SaplingPaymentAddress addr;
+        libzcash::SaplingIncomingViewingKey ivk;         //!< from its spending key: what received outputs are decrypted with
+        uint256 ovk;                                     //!< from its spending key: what outCiphertexts are decrypted with
+        libzcash::SaplingIncomingViewingKey walletIvk;   //!< GetSaplingIncomingViewingKey(addr), as the change check looked it up
+    };
+    //! Every wallet address whose spending key is available (never while the wallet is locked), in
+    //! GetSaplingPaymentAddresses order.
+    std::vector<SpendableAddress> spendable;
+    //! Whether GetNullifiersForAddresses({addr}) would contain (addr, nullifier), given addr's
+    //! GetSaplingIncomingViewingKey result (zero when the wallet doesn't know addr): nullifier belongs to a
+    //! wallet note received with that ivk. Walks the wallet's notes once, on first use.
+    bool IsNullifierOfAddress(const libzcash::SaplingIncomingViewingKey& addrIvk, const uint256& nullifier) const;
+private:
+    mutable boost::optional<std::set<std::pair<libzcash::SaplingIncomingViewingKey, uint256>>> noteNullifiers;
+};
+SaplingHistoryKeys GetSaplingHistoryKeys();
+
 void zsTxSpendsToJSON(const CWalletTx& wtx, UniValue& spends, CAmount& totalSpends, CAmount& filteredSpends, const std::string& strAddress, bool filterByAddress);
-void zsTxReceivedToJSON(const CWalletTx& wtx, UniValue& received, CAmount& totalReceived, const std::string& strAddress, bool filterByAddress);
+void zsTxReceivedToJSON(const CWalletTx& wtx, UniValue& received, CAmount& totalReceived, const std::string& strAddress, bool filterByAddress, const SaplingHistoryKeys& keys);
-void zsTxSendsToJSON(const CWalletTx& wtx, UniValue& sends, CAmount& totalSends, const std::string& strAddress, bool filterByAddress);
+void zsTxSendsToJSON(const CWalletTx& wtx, UniValue& sends, CAmount& totalSends, const std::string& strAddress, bool filterByAddress, const SaplingHistoryKeys& keys);
-void zsWalletTxJSON(const CWalletTx& wtx, UniValue& ret, const std::string strAddress, bool fBool, const int returnType);
+void zsWalletTxJSON(const CWalletTx& wtx, UniValue& ret, const std::string strAddress, bool fBool, const int returnType, const SaplingHistoryKeys& keys);
 
 
 #endif //HUSH_WALLET_RPCWALLET_H

src/wallet/rpcwallet.cpp

@@ -3032,11 +3032,13 @@ UniValue getalldata(const UniValue& params, bool fHelp,const CPubKey&)
     }
 
     //Create Ordered List
-    map<int64_t,CWalletTx> orderedTxs;
+    // Pointers into mapWallet, not copies (a CWalletTx copy deep-copies every note's witness list).
+    // As before, the first tx inserted for an nOrderPos wins.
+    map<int64_t,const CWalletTx*> orderedTxs;
     for (map<uint256, CWalletTx>::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) {
       const uint256& wtxid = it->first;
       const CWalletTx& wtx = (*it).second;
-      orderedTxs.insert(std::pair<int64_t,CWalletTx>(wtx.nOrderPos, wtx));
+      orderedTxs.insert(std::make_pair(wtx.nOrderPos, &wtx));
 
       unsigned int txType = 0;
       // 0 Unassigend
@@ -3228,10 +3230,11 @@ UniValue getalldata(const UniValue& params, bool fHelp,const CPubKey&)
         }
 
 
+        const SaplingHistoryKeys historyKeys = GetSaplingHistoryKeys();
         uint64_t t = GetTime();
-        for (map<int64_t,CWalletTx>::reverse_iterator it = orderedTxs.rbegin(); it != orderedTxs.rend(); ++it)
+        for (map<int64_t,const CWalletTx*>::reverse_iterator it = orderedTxs.rbegin(); it != orderedTxs.rend(); ++it)
         {
-            const CWalletTx& wtx = (*it).second;
+            const CWalletTx& wtx = *(*it).second;
 
             if (!CheckFinalTx(wtx))
                 continue;
@@ -3248,7 +3251,7 @@ UniValue getalldata(const UniValue& params, bool fHelp,const CPubKey&)
               continue;
             }
 
-            zsWalletTxJSON(wtx, trans, "*", false, 0);
+            zsWalletTxJSON(wtx, trans, "*", false, 0, historyKeys);
             if (trans.size() >= nCount) break;
 
         }
@@ -3515,17 +3518,20 @@ UniValue z_listreceivedaddress(const UniValue& params, bool fHelp,const CPubKey&
         throw runtime_error("Filter must be greater that 0.");
 
     //Created Ordered Transaction Map
-    map<int64_t,CWalletTx> orderedTxs;
+    // Pointers into mapWallet, not copies (a CWalletTx copy deep-copies every note's witness list).
+    // As before, the first tx inserted for an nOrderPos wins.
+    map<int64_t,const CWalletTx*> orderedTxs;
     for (map<uint256, CWalletTx>::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) {
       const CWalletTx& wtx = (*it).second;
-      orderedTxs.insert(std::pair<int64_t,CWalletTx>(wtx.nOrderPos, wtx));
+      orderedTxs.insert(std::make_pair(wtx.nOrderPos, &wtx));
     }
 
 
+    const SaplingHistoryKeys historyKeys = GetSaplingHistoryKeys();
     uint64_t t = GetTime();
     //Reverse Iterate thru transactions
-    for (map<int64_t,CWalletTx>::reverse_iterator it = orderedTxs.rbegin(); it != orderedTxs.rend(); ++it) {
+    for (map<int64_t,const CWalletTx*>::reverse_iterator it = orderedTxs.rbegin(); it != orderedTxs.rend(); ++it) {
-        const CWalletTx& wtx = (*it).second;
+        const CWalletTx& wtx = *(*it).second;
         bool includeTransaction = true;
 
         //Excude transactions with less confirmations than required
@@ -3546,7 +3552,7 @@ UniValue z_listreceivedaddress(const UniValue& params, bool fHelp,const CPubKey&
         }
 
         if (includeTransaction) {
-          zsWalletTxJSON(wtx, ret, params[0].get_str() , true, 2);
+          zsWalletTxJSON(wtx, ret, params[0].get_str() , true, 2, historyKeys);
         }
 
         if (ret.size() > nCount) break;
@@ -3698,17 +3704,20 @@ UniValue z_listsentbyaddress(const UniValue& params, bool fHelp,const CPubKey&)
         throw runtime_error("Filter must be greater that 0.");
 
     //Created Ordered Transaction Map
-    map<int64_t,CWalletTx> orderedTxs;
+    // Pointers into mapWallet, not copies (a CWalletTx copy deep-copies every note's witness list).
+    // As before, the first tx inserted for an nOrderPos wins.
+    map<int64_t,const CWalletTx*> orderedTxs;
     for (map<uint256, CWalletTx>::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) {
       const CWalletTx& wtx = (*it).second;
-      orderedTxs.insert(std::pair<int64_t,CWalletTx>(wtx.nOrderPos, wtx));
+      orderedTxs.insert(std::make_pair(wtx.nOrderPos, &wtx));
     }
 
 
+    const SaplingHistoryKeys historyKeys = GetSaplingHistoryKeys();
     uint64_t t = GetTime();
     //Reverse Iterate thru transactions
-    for (map<int64_t,CWalletTx>::reverse_iterator it = orderedTxs.rbegin(); it != orderedTxs.rend(); ++it) {
+    for (map<int64_t,const CWalletTx*>::reverse_iterator it = orderedTxs.rbegin(); it != orderedTxs.rend(); ++it) {
-      const CWalletTx& wtx = (*it).second;
+      const CWalletTx& wtx = *(*it).second;
 
       if (!CheckFinalTx(wtx))
           continue;
@@ -3731,7 +3740,7 @@ UniValue z_listsentbyaddress(const UniValue& params, bool fHelp,const CPubKey&)
       if (nFilterType == 2 && wtx.GetDepthInMainChain() > nFilter)
           continue;
 
-      zsWalletTxJSON(wtx, ret, "*", false, 0);
+      zsWalletTxJSON(wtx, ret, "*", false, 0, historyKeys);
 
 
       if (ret.size() >= nCount) break;
@@ -4309,7 +4318,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 +4651,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)
@@ -4662,7 +4671,7 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp, const CPubK
             }
             obj.push_back(Pair("outindex", (int)entry.op.n));
             obj.push_back(Pair("rawconfirmations", entry.confirmations));
-            auto wtx = pwalletMain->mapWallet.at(entry.op.hash); //.ToString());
+            const CWalletTx& wtx = pwalletMain->mapWallet.at(entry.op.hash);
             obj.push_back(Pair("time", wtx.GetTxTime()));
 
             obj.push_back(Pair("confirmations", dpowconfs));
@@ -5376,7 +5385,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk)
     // sort notes from largest to smallest, which means
     // we will spend the largest first
     std::sort(saplingEntries.begin(), saplingEntries.end(),
-        [](SaplingNoteEntry i, SaplingNoteEntry j) -> bool {
+        [](const SaplingNoteEntry& i, const SaplingNoteEntry& j) -> bool {
             return i.note.value() > j.note.value();
         });
 

src/wallet/wallet.cpp

@@ -45,6 +45,7 @@
 #include <limits>
 #include <system_error>
 #include <thread>
+#include <chrono>
 #include <atomic>
 #include <mutex>
 #include "zcash/zip32.h"
@@ -76,8 +77,8 @@ 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 fCheckNoteCache = false;
 bool fTxConflictDeleteEnabled = false;
 int fDeleteInterval = DEFAULT_TX_DELETE_INTERVAL;
 unsigned int fDeleteTransactionsAfterNBlocks = DEFAULT_TX_RETENTION_BLOCKS;
@@ -1262,9 +1263,11 @@ void ClearSingleNoteWitnessCache(NoteData* nd)
     nd->witnessRootValidated = false;
 }
 
-int CWallet::SaplingWitnessMinimumHeight(const uint256& nullifier, int nWitnessHeight, int nMinimumHeight)
+// A note without a nullifier is never extended by BuildWitnessCache (its pending-note gate requires one),
+// so it doesn't pull the rebuild's start height down.
+int CWallet::SaplingWitnessMinimumHeight(const boost::optional<uint256>& nullifier, int nWitnessHeight, int nMinimumHeight)
 {
-    if (GetSaplingSpendDepth(nullifier) <= WITNESS_CACHE_SIZE) {
+    if (nullifier && GetSaplingSpendDepth(*nullifier) <= WITNESS_CACHE_SIZE) {
         nMinimumHeight = min(nWitnessHeight, nMinimumHeight);
     }
     return nMinimumHeight;
@@ -1296,6 +1299,14 @@ int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessO
         uint256 blockRoot;
         uint256 witnessRoot;
 
+        //A note found with an incoming viewing key only (z_importviewingkey) can't be spent, so it needs
+        //no witness, and it never gets a nullifier: deriving one takes the full viewing key, which
+        //UpdateSaplingNullifierNoteMapWithTx skips without. Seeding it here meant clearing and reseeding
+        //it from disk on every call (the no-nullifier rule below), then aborting on the missing nullifier
+        //the first time it was confirmed. If its spending key is imported later, the rule below seeds it.
+        if (!HaveSaplingFullViewingKey(nd->ivk))
+          continue;
+
         if (!nd->nullifier)
           ::ClearSingleNoteWitnessCache(nd);
 
@@ -1307,7 +1318,7 @@ int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessO
 
           //Skip Validation when witness root has been validated
           if (nd->witnessRootValidated) {
-            nMinimumHeight = SaplingWitnessMinimumHeight(*item.second.nullifier, nd->witnessHeight, nMinimumHeight);
+            nMinimumHeight = SaplingWitnessMinimumHeight(item.second.nullifier, nd->witnessHeight, nMinimumHeight);
             continue;
           }
 
@@ -1319,12 +1330,12 @@ int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessO
             CBlockIndex* whIndex = chainActive[nd->witnessHeight];
             if (whIndex == NULL) {
               //witnessHeight strictly above the active chain (transient catch-up): cannot validate yet
-              nMinimumHeight = SaplingWitnessMinimumHeight(*item.second.nullifier, nd->witnessHeight, nMinimumHeight);
+              nMinimumHeight = SaplingWitnessMinimumHeight(item.second.nullifier, nd->witnessHeight, nMinimumHeight);
               continue;
             }
             if (nd->witnesses.front().root() == whIndex->hashFinalSaplingRoot) {
               nd->witnessRootValidated = true;
-              nMinimumHeight = SaplingWitnessMinimumHeight(*item.second.nullifier, nd->witnessHeight, nMinimumHeight);
+              nMinimumHeight = SaplingWitnessMinimumHeight(item.second.nullifier, nd->witnessHeight, nMinimumHeight);
               continue;
             }
             //root mismatch on the active chain -> desynced; fall through to rebuild below
@@ -1336,7 +1347,7 @@ int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessO
           blockRoot = pblockindex->hashFinalSaplingRoot;
           if (witnessRoot == blockRoot) {
             nd->witnessRootValidated = true;
-            nMinimumHeight = SaplingWitnessMinimumHeight(*item.second.nullifier, nd->witnessHeight, nMinimumHeight);
+            nMinimumHeight = SaplingWitnessMinimumHeight(item.second.nullifier, nd->witnessHeight, nMinimumHeight);
             continue;
           }
         }
@@ -1388,7 +1399,7 @@ int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessO
         }
         nd->witnessHeight = pblockindex->GetHeight();
         UpdateSaplingNullifierNoteMapWithTx(wtxItem.second);
-        nMinimumHeight = SaplingWitnessMinimumHeight(*item.second.nullifier, nd->witnessHeight, nMinimumHeight);
+        nMinimumHeight = SaplingWitnessMinimumHeight(item.second.nullifier, nd->witnessHeight, nMinimumHeight);
       }
     }
   }
@@ -2024,6 +2035,21 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD
 // True if both maps hold the same notes with identical persisted state (every field SaplingNoteData
 // serializes). SaplingNoteData::operator== ignores the witnesses, so it cannot tell a real change
 // from a redundant update.
+// Copies the decrypted plaintext of each incoming note into the matching (same outpoint and ivk) entry that
+// lacks one. UpdatedNoteData uses it when it keeps the wallet's entries: a replay or rescan has just
+// decrypted the notes (FindMySaplingNotes), and entries loaded from wallet.dat have no plaintext. The cache
+// is in memory only and outside every comparison, so this never makes a tx look changed.
+static void KeepNotePlaintexts(const mapSaplingNoteData_t& incoming, mapSaplingNoteData_t& wallet)
+{
+    for (const mapSaplingNoteData_t::value_type& in : incoming) {
+        if (!in.second.cachedPlaintext)
+            continue;
+        mapSaplingNoteData_t::iterator it = wallet.find(in.first);
+        if (it != wallet.end() && !it->second.cachedPlaintext && it->second.ivk == in.second.ivk)
+            it->second.cachedPlaintext = in.second.cachedPlaintext;
+    }
+}
+
 static bool SaplingNoteDataIdentical(const mapSaplingNoteData_t& a, const mapSaplingNoteData_t& b)
 {
     if (a.size() != b.size())
@@ -2043,8 +2069,10 @@ static bool SaplingNoteDataIdentical(const mapSaplingNoteData_t& a, const mapSap
 bool CWallet::UpdatedNoteData(const CWalletTx& wtxIn, CWalletTx& wtx, bool fSameBlock)
 {
     bool unchangedSaplingFlag = (wtxIn.mapSaplingNoteData.empty() || wtxIn.mapSaplingNoteData == wtx.mapSaplingNoteData);
-    if (unchangedSaplingFlag)
+    if (unchangedSaplingFlag) {
+        KeepNotePlaintexts(wtxIn.mapSaplingNoteData, wtx.mapSaplingNoteData);
         return false;
+    }
 
     auto tmp = wtxIn.mapSaplingNoteData;
     // Ensure we keep any cached witnesses we may already have
@@ -2068,6 +2096,14 @@ bool CWallet::UpdatedNoteData(const CWalletTx& wtxIn, CWalletTx& wtx, bool fSame
         }
     }
 
+    // Keep a decrypted plaintext the wallet already holds (in memory only; a pure function of the
+    // outpoint and ivk) when the incoming note data lacks one.
+    for (const mapSaplingNoteData_t::value_type& nd : wtx.mapSaplingNoteData) {
+        mapSaplingNoteData_t::iterator it = tmp.find(nd.first);
+        if (it != tmp.end() && !it->second.cachedPlaintext && nd.second.cachedPlaintext && it->second.ivk == nd.second.ivk)
+            it->second.cachedPlaintext = nd.second.cachedPlaintext;
+    }
+
     // A replayed or rescanned tx usually carries nothing new; don't rewrite it to disk. Do have
     // VerifyAndSetInitialWitness re-check the kept witnesses against the active chain once (the
     // flag is in memory only): the chain may have moved under them, and it keeps a rescan able to
@@ -2075,6 +2111,7 @@ bool CWallet::UpdatedNoteData(const CWalletTx& wtxIn, CWalletTx& wtx, bool fSame
     if (SaplingNoteDataIdentical(tmp, wtx.mapSaplingNoteData)) {
         for (mapSaplingNoteData_t::value_type& item : wtx.mapSaplingNoteData)
             item.second.witnessRootValidated = false;
+        KeepNotePlaintexts(tmp, wtx.mapSaplingNoteData);
         return false;
     }
 
@@ -2253,21 +2290,24 @@ std::pair<mapSaplingNoteData_t, SaplingIncomingViewingKeyMap> CWallet::FindMySap
 
     // Protocol Spec: 4.19 Block Chain Scanning (Sapling)
     for (uint32_t i = 0; i < nZouts; ++i) {
-        const OutputDescription output = tx.vShieldedOutput[i];
+        const OutputDescription& output = tx.vShieldedOutput[i];
         bool found = false;
         for (auto it = mapSaplingFullViewingKeys.begin(); it != mapSaplingFullViewingKeys.end(); ++it) {
             SaplingIncomingViewingKey ivk = it->first;
-            auto result = SaplingNotePlaintext::decrypt(output.encCiphertext, ivk, output.ephemeralKey, output.cm);
+            auto result = SaplingNotePlaintext::decrypt_with_note(output.encCiphertext, ivk, output.ephemeralKey, output.cm);
             if (result) {
-                auto address = ivk.address(result.get().d);
+                // The note's (d, pk_d) is the address it was sent to, exactly what ivk.address(d) would
-                if (address && mapSaplingIncomingViewingKeys.count(address.get()) == 0) {
+                // derive again: decrypt_with_note already computed pk_d from this ivk and d.
-                    viewingKeysToAdd[address.get()] = ivk;
+                const libzcash::SaplingPaymentAddress address(result->second.d, result->second.pk_d);
+                if (mapSaplingIncomingViewingKeys.count(address) == 0) {
+                    viewingKeysToAdd[address] = ivk;
                 }
                 // We don't cache the nullifier here as computing it requires knowledge of the note position
                 // in the commitment tree, which can only be determined when the transaction has been mined.
                 SaplingOutPoint op {hash, i};
                 SaplingNoteData nd;
                 nd.ivk = ivk;
+                nd.cachedPlaintext = SaplingNoteCache{ result->second, result->first.memo() };
                 noteData.insert(std::make_pair(op, nd));
                 found = true;
                 break;
@@ -2276,7 +2316,7 @@ std::pair<mapSaplingNoteData_t, SaplingIncomingViewingKeyMap> CWallet::FindMySap
         if (!found) {
             for (auto it = mapSaplingIncomingViewingKeys.begin(); it != mapSaplingIncomingViewingKeys.end(); ++it) {
                 SaplingIncomingViewingKey ivk = it-> second;
-                auto result = SaplingNotePlaintext::decrypt(output.encCiphertext, ivk, output.ephemeralKey, output.cm);
+                auto result = SaplingNotePlaintext::decrypt_with_note(output.encCiphertext, ivk, output.ephemeralKey, output.cm);
                 if (!result) {
                     continue;
                 }
@@ -2285,6 +2325,7 @@ std::pair<mapSaplingNoteData_t, SaplingIncomingViewingKeyMap> CWallet::FindMySap
                 SaplingOutPoint op {hash, i};
                 SaplingNoteData nd;
                 nd.ivk = ivk;
+                nd.cachedPlaintext = SaplingNoteCache{ result->second, result->first.memo() };
                 noteData.insert(std::make_pair(op, nd));
                 break;
             }
@@ -5705,6 +5746,137 @@ bool CMerkleTx::AcceptToMemoryPool(bool fLimitFree, bool fRejectAbsurdFee)
     return ::AcceptToMemoryPool(mempool, state, *this, fLimitFree, NULL, fRejectAbsurdFee);
 }
 
+unsigned int CWallet::FillSaplingNoteCache(const std::vector<SaplingNoteToDecrypt>& notes)
+{
+    AssertLockHeld(cs_wallet);
+
+    // Each decryption is independent: it reads its own output and writes only its own note's
+    // cachedPlaintext. So a big batch (a whole wallet after a restart, ~0.5ms a note) is split across
+    // threads the way BuildWitnessCache's Phase 2 is, while this thread keeps cs_wallet held. Workers
+    // must not lock, or even assert a lock: under DEBUG_LOCKORDER a fresh thread has no lock stack.
+    // Spawning costs ~1ms, so small batches stay serial.
+    int nPar = 1;
+    if (notes.size() >= 128) {
+        nPar = (int)GetArg("-witnessbuildthreads", 0);
+        if (nPar <= 0) nPar = (int)std::thread::hardware_concurrency();
+        nPar = std::max(1, std::min(nPar, (int)(notes.size() / 64)));
+    }
+
+    std::vector<unsigned int> filled(nPar, 0);
+    auto worker = [&](int tid) {
+        for (size_t k = (size_t)tid; k < notes.size(); k += (size_t)nPar) {
+            const SaplingNoteToDecrypt& n = notes[k];
+            try {
+                auto result = SaplingNotePlaintext::decrypt_with_note(
+                    n.output->encCiphertext, n.nd->ivk, n.output->ephemeralKey, n.output->cm);
+                if (result) {
+                    n.nd->cachedPlaintext = SaplingNoteCache{ result->second, result->first.memo() };
+                    filled[tid]++;
+                }
+            } catch (...) {
+                // Left unfilled, exactly like a note that doesn't decrypt; callers handle that. Nothing may
+                // escape a std::thread.
+            }
+        }
+    };
+
+    if (nPar <= 1) {
+        worker(0);
+    } else {
+        // If the OS refuses a thread, run the remaining shards here instead of letting the exception
+        // unwind past joinable threads (std::terminate). Each shard still runs exactly once.
+        std::vector<std::thread> threads;
+        threads.reserve(nPar);
+        int t = 0;
+        try {
+            for (; t < nPar; t++) threads.emplace_back(worker, t);
+        } catch (const std::exception& e) {
+            LogPrintf("%s: could only start %d of %d threads (%s); decrypting the rest inline\n", __func__, t, nPar, e.what());
+        }
+        for (int u = t; u < nPar; u++) worker(u);
+        for (std::thread& th : threads) th.join();
+    }
+
+    unsigned int nFilled = 0;
+    for (unsigned int f : filled)
+        nFilled += f;
+    nSaplingNoteCacheFills += nFilled;
+    nSaplingNoteCacheFillThreads = nPar;
+    LogPrint("bench", "%s: decrypted %u of %u notes using %d thread(s)\n", __func__, nFilled, (unsigned int)notes.size(), nPar);
+    return nFilled;
+}
+
+// wallet.dat doesn't store the plaintext, so without this the first balance call after a restart decrypts
+// the whole wallet under cs_main (~0.5ms a note). init runs it before RPC leaves warmup and before the
+// wallet notification thread starts; only the few RPCs allowed during warmup (key export, address
+// listing) can wait on cs_wallet meanwhile. Unlike GetFilteredNotes it never asserts, so a corrupt note
+// can't stop the node from starting (or from serving those RPCs); GetFilteredNotes still asserts on such
+// a note, as before.
+CWallet::SaplingNoteCacheWarmup CWallet::WarmSaplingNoteCache()
+{
+    const std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now();
+    LOCK(cs_wallet);
+    SaplingNoteCacheWarmup result;
+    std::vector<SaplingNoteToDecrypt> uncached;
+    for (auto& p : mapWallet) {
+        CWalletTx& wtx = p.second;
+        for (auto& item : wtx.mapSaplingNoteData) {
+            result.notes++;
+            if (item.second.cachedPlaintext)
+                continue;
+            if (item.first.n >= wtx.vShieldedOutput.size()) {
+                LogPrint("zrpcunsafe", "%s: note %s:%u has no such output\n", __func__, item.first.hash.ToString(), item.first.n);
+                result.noSuchOutput++;
+                continue;
+            }
+            uncached.push_back(SaplingNoteToDecrypt { &item.second, &wtx.vShieldedOutput[item.first.n] });
+        }
+    }
+    result.filled = uncached.empty() ? 0 : FillSaplingNoteCache(uncached);
+    for (const SaplingNoteToDecrypt& n : uncached) {
+        if (!n.nd->cachedPlaintext) {
+            LogPrint("zrpcunsafe", "%s: a note of output %s does not decrypt under its ivk\n", __func__, n.output->cm.ToString());
+            result.undecryptable++;
+        }
+    }
+    const unsigned int nBad = result.noSuchOutput + result.undecryptable;
+    const int64_t nMs = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - start).count();
+    LogPrintf("%s: decrypted %u of %u Sapling notes in %dms%s\n", __func__, result.filled, result.notes, nMs,
+              nBad ? strprintf("; %u could not be decrypted (-debug=zrpcunsafe lists them)", nBad) : "");
+    return result;
+}
+
+// -checknotecache: decrypts a cached note again, and logs and repairs any difference. GetFilteredNotes runs
+// it on every note that passes its tx, spent and locked filters, before the address filter, so a corrupt
+// cached address can't wrongly drop a note unchecked. A diagnostic for the cache: it costs the full
+// decryption the cache exists to avoid.
+static void CheckSaplingNoteCache(const SaplingOutPoint& op, SaplingNoteData& nd, const OutputDescription& output)
+{
+    auto fresh = SaplingNotePlaintext::decrypt_with_note(output.encCiphertext, nd.ivk, output.ephemeralKey, output.cm);
+    if (!fresh) {
+        LogPrintf("%s: note %s:%u has a cached plaintext but no longer decrypts\n", __func__, op.hash.ToString(), op.n);
+        return;
+    }
+    const SaplingNoteCache& cached = *nd.cachedPlaintext;
+    const libzcash::SaplingNote& note = fresh->second;
+    if (!(cached.note.d == note.d && cached.note.pk_d == note.pk_d && cached.note.r == note.r &&
+          cached.note.value() == note.value() && cached.memo == fresh->first.memo())) {
+        LogPrintf("%s: cached plaintext of note %s:%u differs from a fresh decryption; using the fresh one\n",
+                  __func__, op.hash.ToString(), op.n);
+        nd.cachedPlaintext = SaplingNoteCache{ note, fresh->first.memo() };
+    }
+}
+
+// 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.
@@ -5741,79 +5913,126 @@ void CWallet::GetFilteredNotes(
 {
     LOCK2(cs_main, cs_wallet);
 
-    LogPrintf("%s ignoreLocked=%d\n", __func__, ignoreLocked);
+    // Every caller already holds LOCK2, so the lock above never waits and this times the walk alone.
+    // Time spent waiting for the locks shows up in the RPC's own "took" line (rpc/server.cpp).
+    const std::chrono::steady_clock::time_point walkStart = std::chrono::steady_clock::now();
+    const size_t nEntriesBefore = saplingEntries.size();
+    unsigned int nTxs = 0, nNotes = 0, nSpent = 0;
 
+    // Pass 1: the notes that pass every filter not needing the plaintext, in wallet order.
+    struct Candidate {
+        const SaplingOutPoint* op;
+        SaplingNoteData* nd;
+        const OutputDescription* output;
+        int nDepth;
+    };
+    std::vector<Candidate> candidates;
+    std::vector<SaplingNoteToDecrypt> uncached;
     for (auto & p : mapWallet) {
-        CWalletTx wtx = p.second;
+        // By reference: a copy deep-copies every note's witness list (up to WITNESS_CACHE_SIZE
+        // witnesses each), and it was made before any filter ran. Not const: a missing plaintext is
+        // cached onto the wallet's own entry.
+        CWalletTx& wtx = p.second;
+
+        // Most wallet txs (coinbase, sends from other wallets) carry no notes of ours.
+        if (wtx.mapSaplingNoteData.empty())
+            continue;
 
         // Filter the transactions before checking for notes
         if (!CheckFinalTx(wtx) || wtx.GetBlocksToMaturity() > 0)
             continue;
 
+        // 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;
             }
-            int nDepth     = wtx.GetDepthInMainChain();
             int dpowconfs  = hush_dpowconfs(nHeight,nDepth);
             if ( dpowconfs < minDepth || dpowconfs > maxDepth) {
                 continue;
             }
         } else {
-            if ( wtx.GetDepthInMainChain() < minDepth ||
+            if ( nDepth < minDepth || nDepth > maxDepth) {
-                wtx.GetDepthInMainChain() > maxDepth) {
                 continue;
             }
         }
+        nTxs++;
 
         for (auto & pair : wtx.mapSaplingNoteData) {
-            SaplingOutPoint op = pair.first;
+            const SaplingOutPoint& op = pair.first;
-            SaplingNoteData nd = pair.second;
+            SaplingNoteData& nd = pair.second;
-
+            nNotes++;
-            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;
-            }
 
+            // The cheap filters run before any decryption, which costs ~0.5ms of Jubjub arithmetic
+            // per note. Spent notes stay in the wallet (-deletetx is off by default).
             if (ignoreSpent && nd.nullifier && IsSaplingSpent(*nd.nullifier)) {
+                nSpent++;
                 continue;
             }
 
-            // skip notes which cannot be spent
-            if (requireSpendingKey) {
-                libzcash::SaplingIncomingViewingKey ivk;
-                libzcash::SaplingFullViewingKey fvk;
-                if (!(GetSaplingIncomingViewingKey(pa, ivk) &&
-                    GetSaplingFullViewingKey(ivk, fvk) &&
-                    HaveSaplingSpendingKey(fvk))) {
-                    continue;
-                }
-            }
-
             // skip locked notes
-             if (ignoreLocked && IsLockedNote(op)) {
+            if (ignoreLocked && IsLockedNote(op)) {
-                 LogPrintf("%s: skipping locked note %s\n", __func__, op.hash.ToString().substr(0,10).c_str());
+                LogPrint("zrpcunsafe", "%s: skipping locked note %s\n", __func__, op.hash.ToString().substr(0,10).c_str());
-                 continue;
+                continue;
-             }
+            }
 
-            auto note = notePt.note(nd.ivk).get();
+            assert(op.n < wtx.vShieldedOutput.size());
-            saplingEntries.push_back(SaplingNoteEntry {
+            const OutputDescription* output = &wtx.vShieldedOutput[op.n];
-                op, pa, note, notePt.memo(), wtx.GetDepthInMainChain() });
+            candidates.push_back(Candidate { &op, &nd, output, nDepth });
+            if (!nd.cachedPlaintext)
+                uncached.push_back(SaplingNoteToDecrypt { &nd, output });
         }
     }
+
+    // Decrypt what the cache lacks: normally nothing, since FindMySaplingNotes fills new notes and the
+    // startup warm-up fills the ones loaded from wallet.dat.
+    const unsigned int nFilled = uncached.empty() ? 0 : FillSaplingNoteCache(uncached);
+
+    // Pass 2: the filters that need the note's address.
+    if (filterAddresses.empty())
+        saplingEntries.reserve(saplingEntries.size() + candidates.size());
+    for (const Candidate& c : candidates) {
+        // A wallet note that doesn't decrypt under its own ivk is corrupt; as before, that's fatal here.
+        assert(static_cast<bool>(c.nd->cachedPlaintext));
+        if (fCheckNoteCache)
+            CheckSaplingNoteCache(*c.op, *c.nd, *c.output);
+        const SaplingNoteCache& plaintext = *c.nd->cachedPlaintext;
+        const libzcash::SaplingPaymentAddress pa(plaintext.note.d, plaintext.note.pk_d);
+
+        // skip notes which belong to a different payment address in the wallet
+        if (!(filterAddresses.empty() || filterAddresses.count(pa))) {
+            continue;
+        }
+
+        // skip notes which cannot be spent
+        if (requireSpendingKey) {
+            libzcash::SaplingIncomingViewingKey ivk;
+            libzcash::SaplingFullViewingKey fvk;
+            if (!(GetSaplingIncomingViewingKey(pa, ivk) &&
+                GetSaplingFullViewingKey(ivk, fvk) &&
+                HaveSaplingSpendingKey(fvk))) {
+                continue;
+            }
+        }
+
+        saplingEntries.push_back(SaplingNoteEntry {
+            *c.op, pa, plaintext.note, plaintext.memo, c.nDepth });
+    }
+
+    // A slow walk holds cs_main long enough to stall block connection, so it is always logged; a
+    // fast one only with -debug=bench (the balance RPCs run it on every GUI poll). The wallet-size
+    // counts go to the default log only with -debug=bench too.
+    const int64_t nWalkMs = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - walkStart).count();
+    if (LogAcceptCategory("bench")) {
+        LogPrintf("%s: minDepth=%d ignoreSpent=%d requireSpendingKey=%d ignoreLocked=%d txs=%u/%u notes=%u spent=%u decrypted=%u entries=%u walk=%dms\n",
+                  __func__, minDepth, ignoreSpent, requireSpendingKey, ignoreLocked, nTxs, (unsigned int)mapWallet.size(),
+                  nNotes, nSpent, nFilled, (unsigned int)(saplingEntries.size() - nEntriesBefore), nWalkMs);
+    } else if (nWalkMs >= 500) {
+        LogPrintf("%s: minDepth=%d ignoreSpent=%d requireSpendingKey=%d ignoreLocked=%d walk=%dms\n",
+                  __func__, minDepth, ignoreSpent, requireSpendingKey, ignoreLocked, nWalkMs);
+    }
 }
 
 

src/wallet/wallet.h

@@ -62,6 +62,8 @@ extern bool bSpendZeroConfChange;
 extern bool fSendFreeTransactions;
 extern bool fPayAtLeastCustomFee;
 extern bool fTxDeleteEnabled;
+//! -checknotecache: re-decrypt cached Sapling notes in GetFilteredNotes and repair any difference
+extern bool fCheckNoteCache;
 extern bool fTxConflictDeleteEnabled;
 extern int fDeleteInterval;
 extern unsigned int fDeleteTransactionsAfterNBlocks;
@@ -301,6 +303,14 @@ class SproutNoteData
 };
 
 
+/** A Sapling note's decrypted plaintext as the wallet uses it: the note (value, rcm, and the address it
+ *  was sent to as (d, pk_d)) and its memo. */
+struct SaplingNoteCache
+{
+    libzcash::SaplingNote note;
+    std::array<unsigned char, HUSH_MEMO_SIZE> memo;
+};
+
 class SaplingNoteData
 {
 public:
@@ -328,6 +338,15 @@ public:
     // read true and short-circuit the witness self-heal in VerifyAndSetInitialWitness.
     bool witnessRootValidated = false;
 
+    //In Memory Only
+    // The note's decrypted plaintext, so the balance and note-listing calls don't redo the Sapling
+    // decryption (several Jubjub scalar multiplications) for every note on every call. It is a pure
+    // function of (txid, output index, ivk), so nothing invalidates it. Never serialized, and left out
+    // of operator== and SaplingNoteDataIdentical on purpose: a filled and an unfilled entry persist the
+    // same state, so filling one must never make a tx look changed and get rewritten. Filled by
+    // FindMySaplingNotes when the note is found, otherwise by FillSaplingNoteCache.
+    boost::optional<SaplingNoteCache> cachedPlaintext;
+
     ADD_SERIALIZE_METHODS;
 
     template <typename Stream, typename Operation>
@@ -366,6 +385,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
 {
@@ -895,7 +919,7 @@ public:
 
 protected:
 
-    int SaplingWitnessMinimumHeight(const uint256& nullifier, int nWitnessHeight, int nMinimumHeight);
+    int SaplingWitnessMinimumHeight(const boost::optional<uint256>& nullifier, int nWitnessHeight, int nMinimumHeight);
 
     /**
      * pindex is the new tip being connected.
@@ -1464,6 +1488,29 @@ public:
     bool LoadCryptedMnemonicEntropy(const uint256& entropyFp, const std::vector<unsigned char>& vchCryptedSecret);
     
     /* Find notes filtered by payment address, min depth, ability to spend */
+    /** A wallet note whose decrypted plaintext is not cached yet, with the output it decrypts. */
+    struct SaplingNoteToDecrypt {
+        SaplingNoteData* nd;
+        const OutputDescription* output;
+    };
+    /** Decrypts each note into nd->cachedPlaintext and returns how many it filled. A note that doesn't
+     *  decrypt under its ivk is left unfilled. Caller holds cs_wallet. */
+    unsigned int FillSaplingNoteCache(const std::vector<SaplingNoteToDecrypt>& notes);
+    /** Notes FillSaplingNoteCache has decrypted since startup (FindMySaplingNotes' fills not counted), and
+     *  the threads its last call used. Guarded by cs_wallet. */
+    uint64_t nSaplingNoteCacheFills = 0;
+    int nSaplingNoteCacheFillThreads = 0;
+
+    struct SaplingNoteCacheWarmup {
+        unsigned int notes = 0;          //!< notes in the wallet
+        unsigned int filled = 0;         //!< decrypted by this call
+        unsigned int noSuchOutput = 0;   //!< naming an output their tx doesn't have
+        unsigned int undecryptable = 0;  //!< not decrypting under their own ivk
+    };
+    /** Decrypts every wallet note whose plaintext isn't cached yet (all of them after wallet.dat is loaded).
+     *  Never asserts: a note that can't be decrypted is counted, logged and left unfilled. Run at startup. */
+    SaplingNoteCacheWarmup WarmSaplingNoteCache();
+
     void GetFilteredNotes(std::vector<SaplingNoteEntry>& saplingEntries,
                           std::string address,
                           int minDepth=1,

src/zcash/Note.cpp

@@ -115,11 +115,16 @@ boost::optional<SaplingOutgoingPlaintext> SaplingOutgoingPlaintext::decrypt(
     }
 }
 
-boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::decrypt(
+namespace {
+
+// SaplingNotePlaintext::decrypt with an incoming viewing key, also handing back the pk_d = [ivk]g_d
+// it derives to check cmu.
+boost::optional<SaplingNotePlaintext> DecryptWithIvk(
     const SaplingEncCiphertext &ciphertext,
     const uint256 &ivk,
     const uint256 &epk,
-    const uint256 &cmu
+    const uint256 &cmu,
+    uint256 &pk_d
 )
 {
     auto pt = AttemptSaplingEncDecryption(ciphertext, ivk, epk);
@@ -140,7 +145,6 @@ boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::decrypt(
         return boost::none;
     }
 
-    uint256 pk_d;
     if (!librustzcash_ivk_to_pkd(ivk.begin(), ret.d.data(), pk_d.begin())) {
         return boost::none;
     }
@@ -164,6 +168,36 @@ boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::decrypt(
     return ret;
 }
 
+}
+
+boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::decrypt(
+    const SaplingEncCiphertext &ciphertext,
+    const uint256 &ivk,
+    const uint256 &epk,
+    const uint256 &cmu
+)
+{
+    uint256 pk_d;
+    return DecryptWithIvk(ciphertext, ivk, epk, cmu, pk_d);
+}
+
+boost::optional<std::pair<SaplingNotePlaintext, SaplingNote>> SaplingNotePlaintext::decrypt_with_note(
+    const SaplingEncCiphertext &ciphertext,
+    const uint256 &ivk,
+    const uint256 &epk,
+    const uint256 &cmu
+)
+{
+    uint256 pk_d;
+    auto pt = DecryptWithIvk(ciphertext, ivk, epk, cmu, pk_d);
+    if (!pt) {
+        return boost::none;
+    }
+    // ivk_to_pkd succeeding implies d is a valid diversifier, which is all note(ivk) checks on top.
+    SaplingNote note(pt->d, pk_d, pt->value(), pt->rcm);
+    return std::make_pair(*pt, note);
+}
+
 boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::decrypt(
     const SaplingEncCiphertext &ciphertext,
     const uint256 &epk,

src/zcash/Note.hpp

@@ -85,6 +85,17 @@ public:
         const uint256 &cmu
     );
 
+    // decrypt(ciphertext, ivk, epk, cmu) that also returns the note. Building the note needs
+    // pk_d = [ivk]g_d, which the decryption derives anyway to check cmu; note(ivk) would derive it
+    // again (another Jubjub scalar multiplication). A distinct name rather than an overload, so it
+    // can never capture a call meant for the (epk, esk, pk_d, cmu) one.
+    static boost::optional<std::pair<SaplingNotePlaintext, SaplingNote>> decrypt_with_note(
+        const SaplingEncCiphertext &ciphertext,
+        const uint256 &ivk,
+        const uint256 &epk,
+        const uint256 &cmu
+    );
+
     boost::optional<SaplingNote> note(const SaplingIncomingViewingKey& ivk) const;
 
     virtual ~SaplingNotePlaintext() {}