From 106fc1ceacf460beedb46ce275898ef4dbaf8601 Mon Sep 17 00:00:00 2001 From: DanS Date: Fri, 9 Oct 2026 13:31:01 -0500 Subject: [PATCH] wallet: filter notes before decrypting them in GetFilteredNotes GetFilteredNotes copied every CWalletTx and every SaplingNoteData (witness list included) by value, and trial-decrypted every note before it applied the address, spent and lock filters. It sits under z_listunspent, z_gettotalbalance, z_getbalance, z_listreceivedbyaddress, z_mergetoaddress and sweep/consolidation, all with cs_main and cs_wallet held, so on a wallet with thousands of notes it is most of the time those RPCs block the block-connect thread. - Read wallet txs and note data by const reference, and skip txs with no Sapling notes before the finality, maturity and depth checks. - Check IsSaplingSpent before decrypting, so spent notes are not decrypted when ignoreSpent is set. - Address pre-filter: when 1-16 filter addresses all have a recorded ivk that is canonical (below 2^251) and actually generates that address, a note whose canonical ivk is none of them cannot pay a filter address and is skipped without decrypting. z_importviewingkey stores (ivk, address) pairs unchecked, hence the verification; an aliased ivk + r_J would pass it, hence the canonical check. Any other case takes the old path. - SaplingNotePlaintext::decrypt_with_pk_d returns the pk_d that decryption already computes, so the note's address and SaplingNote are built without two more scalar multiplications. decrypt() is now a wrapper around it. RPC output is unchanged. The one behaviour difference: a spent note that cannot be decrypted no longer trips assert(maybe_pt) when ignoreSpent is set. Co-Authored-By: Claude Opus 5.5 --- src/wallet/wallet.cpp | 113 ++++++++++++++++++++++++++++++++++-------- src/zcash/Note.cpp | 13 +++++ src/zcash/Note.hpp | 11 ++++ 3 files changed, 117 insertions(+), 20 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 443498cba..dec4fb598 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5629,6 +5629,26 @@ void CWallet::GetFilteredNotes( GetFilteredNotes(saplingEntries, filterAddresses, minDepth, INT_MAX, ignoreSpent, requireSpendingKey); } +namespace { + +// GetFilteredNotes' decrypt-free address pre-filter is only built for at most this many +// filter addresses. Verifying each one costs a Jubjub scalar multiplication, which buys +// nothing when the filter is "every address in the wallet" (z_listunspent, z_getbalances). +// Setting it to 0 disables the pre-filter entirely. +const size_t GETFILTEREDNOTES_PREFILTER_MAX_ADDRESSES = 16; + +// True if ivk < 2^251. A genuine Sapling ivk is CRH_ivk output with its top 5 bits cleared +// (librustzcash_crh_ivk), so it is below 2^251 < r_J (the Jubjub subgroup order). Below r_J, +// pk_d = [ivk] g_d determines ivk uniquely. librustzcash_ivk_to_pkd multiplies by the raw +// 256-bit value without reducing it, so a value >= r_J aliases ivk - r_J. Byte 31 of the +// little-endian representation is the most significant byte. +bool IsCanonicalSaplingIvk(const libzcash::SaplingIncomingViewingKey& ivk) +{ + return (ivk.begin()[31] & 0xF8) == 0; +} + +} // namespace + /** * Find notes in the wallet filtered by payment addresses, min depth, max depth, * if the note is spent, if a spending key is required, and if the notes are locked. @@ -5647,8 +5667,47 @@ void CWallet::GetFilteredNotes( LogPrintf("%s ignoreLocked=%d\n", __func__, ignoreLocked); - for (auto & p : mapWallet) { - CWalletTx wtx = p.second; + // Decrypt-free address pre-filter. A note matches a Sapling filter address a only if + // nd.ivk.address(a.d) == a. If a's recorded ivk r is canonical and provably generates a + // (r.address(a.d) == a), then any other canonical ivk cannot generate a, so a note whose + // canonical nd.ivk is not one of these r can be skipped without decrypting it. Recorded + // pairs are not trusted as-is (z_importviewingkey stores any (ivk, zaddr) pair the user + // supplies), so each one is verified. If any Sapling filter address has no recorded ivk, + // a non-canonical one, or one that does not generate it, the pre-filter is off and every + // note takes the old decrypt path. InvalidEncoding entries never equal a note's address. + std::set prefilterIvks; + bool usePrefilter = !filterAddresses.empty() && + filterAddresses.size() <= GETFILTEREDNOTES_PREFILTER_MAX_ADDRESSES; + if (usePrefilter) { + for (const libzcash::PaymentAddress& fa : filterAddresses) { + const libzcash::SaplingPaymentAddress* sa = boost::get(&fa); + if (sa == nullptr) { + continue; + } + libzcash::SaplingIncomingViewingKey rivk; + if (!GetSaplingIncomingViewingKey(*sa, rivk) || !IsCanonicalSaplingIvk(rivk)) { + usePrefilter = false; + break; + } + auto derived = rivk.address(sa->d); + if (!derived || !(derived.get() == *sa)) { + usePrefilter = false; + break; + } + prefilterIvks.insert(rivk); + } + } + + for (const auto & p : mapWallet) { + // A reference, not a copy: copying a CWalletTx deep-copies every note's witness list. + // Nothing in this loop mutates mapWallet or any CWalletTx, and cs_main + cs_wallet are + // held for the whole function, so the reference stays valid. + const CWalletTx& wtx = p.second; + + // A tx without Sapling note data can never yield an entry (the note loop below runs + // zero times), so skip it before the finality, maturity and depth work. + if (wtx.mapSaplingNoteData.empty()) + continue; // Filter the transactions before checking for notes if (!CheckFinalTx(wtx) || wtx.GetBlocksToMaturity() > 0) @@ -5671,31 +5730,43 @@ void CWallet::GetFilteredNotes( } } - for (auto & pair : wtx.mapSaplingNoteData) { - SaplingOutPoint op = pair.first; - SaplingNoteData nd = pair.second; + for (const auto & pair : wtx.mapSaplingNoteData) { + const SaplingOutPoint& op = pair.first; + const SaplingNoteData& nd = pair.second; - auto maybe_pt = SaplingNotePlaintext::decrypt( - wtx.vShieldedOutput[op.n].encCiphertext, - nd.ivk, - wtx.vShieldedOutput[op.n].ephemeralKey, - wtx.vShieldedOutput[op.n].cm); - assert(static_cast(maybe_pt)); - auto notePt = maybe_pt.get(); - - auto maybe_pa = nd.ivk.address(notePt.d); - assert(static_cast(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))) { + // Address pre-filter (see prefilterIvks above): a canonical ivk that is not one of + // the verified filter ivks cannot generate any filter address, so the + // filterAddresses check after decryption would reject this note anyway. + if (usePrefilter && IsCanonicalSaplingIvk(nd.ivk) && prefilterIvks.count(nd.ivk) == 0) { continue; } + // Spent check before trial decryption: it reads only nd.nullifier and wallet + // state (not the plaintext or the address), so spent notes are never decrypted. if (ignoreSpent && nd.nullifier && IsSaplingSpent(*nd.nullifier)) { continue; } + uint256 pk_d; + auto maybe_pt = SaplingNotePlaintext::decrypt_with_pk_d( + wtx.vShieldedOutput[op.n].encCiphertext, + nd.ivk, + wtx.vShieldedOutput[op.n].ephemeralKey, + wtx.vShieldedOutput[op.n].cm, + pk_d); + assert(static_cast(maybe_pt)); + auto notePt = maybe_pt.get(); + + // Same value as nd.ivk.address(notePt.d).get(): decryption succeeded, so + // librustzcash_ivk_to_pkd(nd.ivk, d) returned true (d is a valid diversifier, which + // is all check_diversifier tests) and wrote pk_d, the value address() recomputes. + SaplingPaymentAddress pa(notePt.d, 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; @@ -5713,7 +5784,9 @@ void CWallet::GetFilteredNotes( continue; } - auto note = notePt.note(nd.ivk).get(); + // Same value as notePt.note(nd.ivk).get(), which is + // SaplingNote(d, nd.ivk.address(d)->pk_d, value, rcm): pa is nd.ivk.address(d). + libzcash::SaplingNote note(notePt.d, pa.pk_d, notePt.value(), notePt.rcm); saplingEntries.push_back(SaplingNoteEntry { op, pa, note, notePt.memo(), wtx.GetDepthInMainChain() }); } diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index 7aa08536f..502d5584e 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -121,6 +121,18 @@ boost::optional SaplingNotePlaintext::decrypt( const uint256 &epk, const uint256 &cmu ) +{ + uint256 pk_d; + return decrypt_with_pk_d(ciphertext, ivk, epk, cmu, pk_d); +} + +boost::optional SaplingNotePlaintext::decrypt_with_pk_d( + const SaplingEncCiphertext &ciphertext, + const uint256 &ivk, + const uint256 &epk, + const uint256 &cmu, + uint256 &pk_d_out +) { auto pt = AttemptSaplingEncDecryption(ciphertext, ivk, epk); if (!pt) { @@ -161,6 +173,7 @@ boost::optional SaplingNotePlaintext::decrypt( return boost::none; } + pk_d_out = pk_d; return ret; } diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index 47bebe740..d42a3cc31 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -77,6 +77,17 @@ public: const uint256 &cmu ); + // Same as decrypt(ciphertext, ivk, epk, cmu). On success it also stores in pk_d_out the + // pk_d = [ivk] g_d that decryption computed, which equals ivk.address(d)->pk_d, so the + // caller does not have to recompute it. pk_d_out is not written on failure. + static boost::optional decrypt_with_pk_d( + const SaplingEncCiphertext &ciphertext, + const uint256 &ivk, + const uint256 &epk, + const uint256 &cmu, + uint256 &pk_d_out + ); + static boost::optional decrypt( const SaplingEncCiphertext &ciphertext, const uint256 &epk,