diff --git a/src/gtest/test_wallet_notecache.cpp b/src/gtest/test_wallet_notecache.cpp index 0415bc657..bc2ea9c30 100644 --- a/src/gtest/test_wallet_notecache.cpp +++ b/src/gtest/test_wallet_notecache.cpp @@ -121,8 +121,38 @@ class NoteCacheWallet : public CWallet { public: bool EncryptKeys(CKeyingMaterial& vMasterKeyIn) { return CCryptoKeyStore::EncryptKeys(vMasterKeyIn); } bool Unlock(const CKeyingMaterial& vMasterKeyIn) { return CCryptoKeyStore::Unlock(vMasterKeyIn); } + bool UpdatedNoteData(const CWalletTx& wtxIn, CWalletTx& wtx, bool fSameBlock = true) { + return CWallet::UpdatedNoteData(wtxIn, wtx, fSameBlock); + } + + // Drops every note's in-memory plaintext, as a restart does. + void ClearPlaintextCache() { + for (auto& p : mapWallet) + for (auto& item : p.second.mapSaplingNoteData) + item.second.cachedPlaintext = boost::none; + } + size_t CachedNotes() const { + size_t n = 0; + for (const auto& p : mapWallet) + for (const auto& item : p.second.mapSaplingNoteData) + n += item.second.cachedPlaintext ? 1 : 0; + return n; + } }; +void ExpectCacheMatchesFreshDecrypt(const SaplingNoteData& nd, const OutputDescription& od) +{ + ASSERT_TRUE(static_cast(nd.cachedPlaintext)); + auto fresh = SaplingNotePlaintext::decrypt_with_note(od.encCiphertext, nd.ivk, od.ephemeralKey, od.cm); + ASSERT_TRUE(static_cast(fresh)); + const SaplingNote& cached = nd.cachedPlaintext->note; + EXPECT_TRUE(cached.d == fresh->second.d); + EXPECT_EQ(fresh->second.pk_d, cached.pk_d); + EXPECT_EQ(fresh->second.r, cached.r); + EXPECT_EQ(fresh->second.value(), cached.value()); + EXPECT_TRUE(nd.cachedPlaintext->memo == fresh->first.memo()); +} + // The fake active chain is 0..TIP_HEIGHT; STALE_HEIGHT is a side block forked off it. constexpr int TIP_HEIGHT = 30; constexpr int STALE_HEIGHT = 25; @@ -357,7 +387,8 @@ protected: return entries; } - void ExpectEquivalent(const std::vector& minDepths) { + // cold: drop the plaintext cache before every call, so each one decrypts every note it returns. + void ExpectEquivalent(const std::vector& minDepths, bool cold = false) { const std::vector>> filters = { {"none", {}}, {"A", {paA}}, {"B", {paB}}, {"A+C", {paA, paC}}, }; @@ -368,6 +399,10 @@ protected: for (bool ignoreLocked : {false, true}) for (const auto& filter : filters) { std::string ctx = Describe(minDepth, maxDepth, ignoreSpent, requireSpendingKey, ignoreLocked, filter.first); + if (cold) { + LOCK(wallet.cs_wallet); + wallet.ClearPlaintextCache(); + } ExpectSameEntries(Legacy(filter.second, minDepth, maxDepth, ignoreSpent, requireSpendingKey, ignoreLocked), Filtered(filter.second, minDepth, maxDepth, ignoreSpent, requireSpendingKey, ignoreLocked), ctx); @@ -588,3 +623,150 @@ TEST(WalletNoteCacheDiscovery, FindMySaplingNotes) { EXPECT_TRUE(toAdd.at(addrsA[1]) == ivkA); EXPECT_TRUE(toAdd.at(addrsA[2]) == ivkA); } + +// Every note FindMySaplingNotes finds comes with its plaintext, for both the spending-key loop and the +// viewing-key-only loop. +TEST(WalletNoteCacheDiscovery, FindMySaplingNotesFillsCache) { + NoteCacheWallet wallet; + auto skA = TestKey(0), skB = TestKey(1); + auto addrsA = AddressesOf(skA); + LOCK2(cs_main, wallet.cs_wallet); + ASSERT_TRUE(wallet.AddSaplingZKey(skA, addrsA[0])); + ASSERT_TRUE(wallet.AddSaplingIncomingViewingKey(skB.expsk.full_viewing_key().in_viewing_key(), skB.DefaultAddress())); + CTransaction tx = MakeTx({MakeOutput(addrsA[0], 11), MakeOutput(addrsA[1], 12), MakeOutput(skB.DefaultAddress(), 13)}); + auto notes = wallet.FindMySaplingNotes(tx).first; + ASSERT_EQ(3u, notes.size()); + for (const auto& item : notes) + ExpectCacheMatchesFreshDecrypt(item.second, tx.vShieldedOutput[item.first.n]); +} + +// With the cache dropped before every call (a restart), the walk still returns exactly the old loop's +// entries: the lazy fill decrypts the same notes the same way. +TEST_F(WalletNoteCache, ColdCacheMatchesLegacy) { + ExpectEquivalent({0, 1, 11}, true); +} + +// The cache holds exactly what a fresh decryption gives, wherever it was filled. +TEST_F(WalletNoteCache, CachedPlaintextMatchesFreshDecrypt) { + LOCK2(cs_main, wallet.cs_wallet); + for (int pass = 0; pass < 2; pass++) { + if (pass == 1) { + wallet.ClearPlaintextCache(); + Filtered({}, 0, INT_MAX, false, false, false); // lazy fill of every confirmed note + } + size_t checked = 0; + for (const auto& p : wallet.mapWallet) { + for (const auto& item : p.second.mapSaplingNoteData) { + if (!item.second.cachedPlaintext) + continue; + ExpectCacheMatchesFreshDecrypt(item.second, p.second.vShieldedOutput[item.first.n]); + checked++; + } + } + EXPECT_GE(checked, 9u) << "pass " << pass; + } +} + +// A warm walk decrypts nothing; a cold one decrypts each note it needs exactly once, and spent notes +// only when the caller asks for them. +TEST_F(WalletNoteCache, WarmPassDoesNotDecrypt) { + LOCK2(cs_main, wallet.cs_wallet); + const uint64_t base = wallet.nSaplingNoteCacheFills; + Filtered({}, 1, INT_MAX, true, true, true); + EXPECT_EQ(base, wallet.nSaplingNoteCacheFills) << "FindMySaplingNotes already filled every note"; + + wallet.ClearPlaintextCache(); + EXPECT_EQ(0u, wallet.CachedNotes()); + Filtered({}, 1, INT_MAX, true, false, true); + // tx1 (2), tx3 (3), tx4 (1), tx7 (1): tx2's note is locked and tx6's is spent, so neither is decrypted. + EXPECT_EQ(base + 7, wallet.nSaplingNoteCacheFills); + Filtered({}, 1, INT_MAX, true, false, true); + Filtered({paA}, 0, INT_MAX, true, true, true); + EXPECT_EQ(base + 7, wallet.nSaplingNoteCacheFills) << "warm"; + + Filtered({}, 1, INT_MAX, false, false, false); + EXPECT_EQ(base + 9, wallet.nSaplingNoteCacheFills) << "the locked and the spent note, once"; + Filtered({}, 1, INT_MAX, false, false, false); + EXPECT_EQ(base + 9, wallet.nSaplingNoteCacheFills); +} + +// The cache never reaches wallet.dat, the network or a tx hash: serialization is byte-identical with it +// filled or empty, and a deserialized entry comes back empty. +TEST_F(WalletNoteCache, CacheNeverSerialized) { + LOCK2(cs_main, wallet.cs_wallet); + const uint256 txid = txids[2]; // tx3: three notes + CWalletTx filled = wallet.mapWallet.at(txid); + ASSERT_EQ(3u, filled.mapSaplingNoteData.size()); + CWalletTx empty = filled; + for (auto& item : empty.mapSaplingNoteData) { + ASSERT_TRUE(static_cast(item.second.cachedPlaintext)); + item.second.cachedPlaintext = boost::none; + } + for (int type : {SER_DISK, SER_NETWORK, SER_GETHASH}) { + CDataStream a(type, CLIENT_VERSION), b(type, CLIENT_VERSION); + a << filled; + b << empty; + EXPECT_TRUE(a.str() == b.str()) << "CWalletTx, type " << type; + CDataStream c(type, CLIENT_VERSION), d(type, CLIENT_VERSION); + c << filled.mapSaplingNoteData.begin()->second; + d << empty.mapSaplingNoteData.begin()->second; + EXPECT_TRUE(c.str() == d.str()) << "SaplingNoteData, type " << type; + } + EXPECT_EQ(txid, filled.GetHash()); + + CDataStream ss(SER_DISK, CLIENT_VERSION); + ss << filled; + CWalletTx back; + ss >> back; + ASSERT_EQ(3u, back.mapSaplingNoteData.size()); + for (const auto& item : back.mapSaplingNoteData) + EXPECT_FALSE(static_cast(item.second.cachedPlaintext)); +} + +// UpdatedNoteData: a difference in the cache alone is no update (no rewrite to disk); the replace branch +// keeps a plaintext the wallet already had; and nothing is carried across a different ivk. +TEST_F(WalletNoteCache, UpdatedNoteDataAndTheCache) { + LOCK2(cs_main, wallet.cs_wallet); + const uint256 txid = txids[2]; + CWalletTx& wtx = wallet.mapWallet.at(txid); + const SaplingOutPoint op0(txid, 0), op1(txid, 1); + + // Same note data, but arriving without plaintext: unchanged, and the wallet keeps its cache. + CWalletTx replay = wtx; + for (auto& item : replay.mapSaplingNoteData) + item.second.cachedPlaintext = boost::none; + EXPECT_FALSE(wallet.UpdatedNoteData(replay, wtx)); + EXPECT_TRUE(static_cast(wtx.mapSaplingNoteData.at(op0).cachedPlaintext)); + + // A real update (an extra note) without plaintext for the existing ones: replaced, cache carried. + CWalletTx incoming = replay; + incoming.mapSaplingNoteData.erase(op1); + wtx.mapSaplingNoteData.erase(op1); + incoming.mapSaplingNoteData[op1] = SaplingNoteData(TestKey(2).expsk.full_viewing_key().in_viewing_key()); + EXPECT_TRUE(wallet.UpdatedNoteData(incoming, wtx)); + ExpectCacheMatchesFreshDecrypt(wtx.mapSaplingNoteData.at(op0), wtx.vShieldedOutput[0]); + EXPECT_FALSE(static_cast(wtx.mapSaplingNoteData.at(op1).cachedPlaintext)) << "nothing to carry"; + + // Incoming note data under another ivk: its (absent) plaintext is not replaced by ours. + CWalletTx otherKey = wtx; + for (auto& item : otherKey.mapSaplingNoteData) { + item.second.ivk = TestKey(9).expsk.full_viewing_key().in_viewing_key(); + item.second.cachedPlaintext = boost::none; + } + EXPECT_TRUE(wallet.UpdatedNoteData(otherKey, wtx)); + for (const auto& item : wtx.mapSaplingNoteData) + EXPECT_FALSE(static_cast(item.second.cachedPlaintext)); +} + +// Spendability is decided per call, never cached: a note to a viewing-key-only address joins the +// requireSpendingKey results as soon as its spending key is added, with its plaintext already cached. +TEST_F(WalletNoteCache, SpendabilityNotCached) { + LOCK2(cs_main, wallet.cs_wallet); + auto before = Filtered({paB}, 1, INT_MAX, true, true, false); + EXPECT_EQ(0u, before.size()); + EXPECT_EQ(2u, Filtered({paB}, 1, INT_MAX, true, false, false).size()); + const uint64_t fills = wallet.nSaplingNoteCacheFills; + ASSERT_TRUE(wallet.AddSaplingZKey(TestKey(1), paB)); + EXPECT_EQ(2u, Filtered({paB}, 1, INT_MAX, true, true, false).size()); + EXPECT_EQ(fills, wallet.nSaplingNoteCacheFills); +} diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9cf5161f2..dd2d91556 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2078,6 +2078,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 @@ -2280,6 +2288,7 @@ std::pair 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)); found = true; break; @@ -2288,7 +2297,7 @@ std::pair 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; } @@ -2297,6 +2306,7 @@ std::pair 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; } @@ -5717,6 +5727,22 @@ bool CMerkleTx::AcceptToMemoryPool(bool fLimitFree, bool fRejectAbsurdFee) return ::AcceptToMemoryPool(mempool, state, *this, fLimitFree, NULL, fRejectAbsurdFee); } +unsigned int CWallet::FillSaplingNoteCache(const std::vector& notes) +{ + AssertLockHeld(cs_wallet); + unsigned int nFilled = 0; + for (const SaplingNoteToDecrypt& n : notes) { + 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() }; + nFilled++; + } + } + nSaplingNoteCacheFills += nFilled; + return nFilled; +} + // 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 @@ -5769,10 +5795,19 @@ void CWallet::GetFilteredNotes( 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; + const SaplingNoteData* nd; + int nDepth; + }; + std::vector candidates; + std::vector uncached; for (auto & p : mapWallet) { // 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. - const CWalletTx& wtx = p.second; + // 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()) @@ -5800,13 +5835,13 @@ void CWallet::GetFilteredNotes( } nTxs++; - for (const auto & pair : wtx.mapSaplingNoteData) { + for (auto & pair : wtx.mapSaplingNoteData) { const SaplingOutPoint& op = pair.first; - const SaplingNoteData& nd = pair.second; + SaplingNoteData& nd = pair.second; nNotes++; - // The cheap filters run before the decryption, which costs ~0.5-0.8ms of Jubjub - // arithmetic per note. Spent notes stay in the wallet (-deletetx is off by default). + // 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; @@ -5818,36 +5853,43 @@ void CWallet::GetFilteredNotes( continue; } - // One key agreement and one pk_d derivation; the note and its address come with it. - auto maybe_pt = SaplingNotePlaintext::decrypt_with_note( - wtx.vShieldedOutput[op.n].encCiphertext, - nd.ivk, - wtx.vShieldedOutput[op.n].ephemeralKey, - wtx.vShieldedOutput[op.n].cm); - assert(static_cast(maybe_pt)); - const SaplingNotePlaintext& notePt = maybe_pt->first; - const libzcash::SaplingNote& note = maybe_pt->second; - const libzcash::SaplingPaymentAddress pa(note.d, note.pk_d); + assert(op.n < wtx.vShieldedOutput.size()); + candidates.push_back(Candidate { &op, &nd, nDepth }); + if (!nd.cachedPlaintext) + uncached.push_back(SaplingNoteToDecrypt { &nd, &wtx.vShieldedOutput[op.n] }); + } + } - // skip notes which belong to a different payment address in the wallet - if (!(filterAddresses.empty() || filterAddresses.count(pa))) { + // 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. + 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(c.nd->cachedPlaintext)); + 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; } - - // 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 { - op, pa, note, notePt.memo(), nDepth }); } + + 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 @@ -5855,9 +5897,9 @@ void CWallet::GetFilteredNotes( // counts go to the default log only with -debug=bench too. const int64_t nWalkMs = std::chrono::duration_cast(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 entries=%u walk=%dms\n", + 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, (unsigned int)(saplingEntries.size() - nEntriesBefore), nWalkMs); + 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); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 08327a9b3..dc421fac0 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -301,6 +301,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 memo; +}; + class SaplingNoteData { public: @@ -328,6 +336,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 cachedPlaintext; + ADD_SERIALIZE_METHODS; template @@ -1469,6 +1486,18 @@ public: bool LoadCryptedMnemonicEntropy(const uint256& entropyFp, const std::vector& 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& notes); + /** Notes FillSaplingNoteCache has decrypted since startup (FindMySaplingNotes' fills not counted). + * Guarded by cs_wallet. */ + uint64_t nSaplingNoteCacheFills = 0; + void GetFilteredNotes(std::vector& saplingEntries, std::string address, int minDepth=1,