With -checknotecache=1 (debug help), GetFilteredNotes decrypts every
note it returns again. If the cached plaintext differs from the fresh
decryption, it logs the note and replaces the cache entry with the
fresh value. It costs the full decryption the cache exists to avoid, so
it is off by default; it is there for validating the cache on a real
wallet, or for diagnosing a suspected cache problem in the field.
CheckNoteCacheRepairsADifference: by default a tampered cache entry is
returned as is (the cache is trusted); with the option it is repaired
and the fresh value is returned.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The startup warm-up, or a lazy fill after -notecachewarmup=0, decrypts
the whole wallet: ~0.5ms per note, ~6s serial for 12k notes. Each
decryption is independent: it reads its own output and writes only its
own note's cachedPlaintext. So FillSaplingNoteCache now splits batches
of 128 or more notes round-robin across -witnessbuildthreads (default:
hardware_concurrency) workers, at least 64 notes per thread. It follows
BuildWitnessCache's Phase 2 pattern:
- the caller keeps cs_wallet held throughout;
- workers take no lock and assert none; under DEBUG_LOCKORDER a fresh
thread has no lock stack;
- per-shard fill counters are summed after join;
- nothing escapes a worker: a throwing decryption leaves that note
unfilled, like one that doesn't decrypt;
- if the OS refuses a thread, the remaining shards run inline.
librustzcash is already called concurrently by BuildWitnessCache's
workers, and libsodium is thread-safe after sodium_init.
ParallelFillMatchesSerial: 300 notes across two keys and diversified
addresses decrypt to exactly the same cache with 1, 4 and 7 threads.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The plaintext cache is in memory only, so a restart empties it. Without
a warm-up, the first balance call after a restart decrypts the whole
wallet under LOCK2(cs_main, cs_wallet): ~6s for a 12k-note wallet. That
is the observed failure: restart, the GUI connects, and the node looks
stuck.
CWallet::WarmSaplingNoteCache decrypts every note whose plaintext isn't
cached, spent ones included, because z_listreceivedbyaddress
(ignoreSpent=false) is called once per address by the GUI's history
scan.
init runs it at the end of the wallet block, just before
SetBroadcastTransactions. At that point RPC is still in warmup (it
shows "Decrypting wallet notes...") and the wallet notification
thread, whose startup witness rebuild can hold cs_wallet for a long
time, has not started yet. It takes cs_wallet only. -notecachewarmup=0
(debug help) skips it, which leaves the lazy fill in GetFilteredNotes
to do the work.
Unlike GetFilteredNotes, the warm-up never asserts. A note that doesn't
decrypt under its ivk, or names an output its tx lacks, is counted in
the summary line and listed under -debug=zrpcunsafe. A corrupt note
therefore can't stop the node from starting, or from serving the
key-export RPCs allowed during warmup; GetFilteredNotes still asserts
on it, as before.
Tests:
- WarmUpFillsEveryNote: spent, locked, unconfirmed and stale-block
notes included; walks afterwards decrypt nothing.
- WarmUpSkipsBadNotes: undecryptable and out-of-range notes are skipped
without aborting.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GetFilteredNotes, the walk behind every balance and note-listing RPC,
sweep, consolidation, z_sendmany and z_mergetoaddress, decrypted every
note it looked at on every call: ~0.5ms of Jubjub arithmetic per note,
under LOCK2(cs_main, cs_wallet). A note's plaintext is fixed by its
output (the txid commits to the ciphertext) and the ivk that decrypts
it, so it never needs to be decrypted twice.
SaplingNoteData gains an in-memory-only
boost::optional<SaplingNoteCache> {note, memo}; the address is the
note's (d, pk_d). It follows the witnessRootValidated precedent:
- never serialized, and left out of operator== and
SaplingNoteDataIdentical, so filling it never makes a tx look changed
and get rewritten (the replay rewrite storm 69bb035e7 removed);
- FindMySaplingNotes fills it from the decryption that found the note
(both the spending-key and the viewing-key-only loop);
- UpdatedNoteData's replace branch keeps a plaintext the wallet already
had, for the same outpoint and ivk only;
- GetFilteredNotes now runs in two passes: pass 1 applies the tx,
spent and locked filters and collects the notes that pass; whatever
lacks a plaintext (everything after a restart, until the startup
warm-up in a later commit) is decrypted once via the new
FillSaplingNoteCache; pass 2 applies the address and spending-key
filters and emits entries in the same order as before.
Spentness, spendability, lock state and depth are still decided on every
call; only the plaintext is cached.
The A0 equivalence test passes unchanged (warm). New tests:
- ColdCacheMatchesLegacy: the same grid with the cache dropped before
every call;
- CachedPlaintextMatchesFreshDecrypt: eager and lazy fills;
- WarmPassDoesNotDecrypt: a fill counter shows a warm walk decrypts
nothing, and spent or locked notes only when the caller asks for
them;
- CacheNeverSerialized: byte-identical SER_DISK/NETWORK/GETHASH output,
empty after a load;
- UpdatedNoteDataAndTheCache: no rewrite on a cache-only difference,
carried on replace, never across another ivk;
- SpendabilityNotCached;
- FindMySaplingNotesFillsCache.
Memory: ~616 bytes per note (the memo is 512), ~8MB for a 13k-note
wallet, next to ~100KB per note of cached witnesses.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SaplingNotePlaintext::decrypt(ciphertext, ivk, epk, cmu) derives
pk_d = [ivk]g_d to check the note commitment, then throws it away. The
wallet then calls ivk.address(d), which derives pk_d again, and
plaintext.note(ivk), which derives it a third time. Each derivation is a
Jubjub scalar multiplication, ~115us, measured. Including the key
agreement and the commitment check, the whole sequence is ~775us per
note.
decrypt_with_note() runs the same decryption and checks and returns the
note built from that pk_d. ivk_to_pkd succeeding already implies a
valid diversifier, which is all ivk.address() checks on top. It gets its
own name rather than a decrypt() overload: an overload taking a fifth
argument by non-const reference would capture calls meant for the
(epk, esk, pk_d, cmu) variant. The cmu check stays.
GetFilteredNotes and FindMySaplingNotes' spending-key loop use it. That
saves two of the three derivations per note in every balance walk, and
one per found note during scans and rescans (trial decryption of
outputs that aren't ours still fails at the AEAD, as before).
FindMySaplingNotes also no longer copies each OutputDescription.
New tests: MatchesDecryptThenNote checks decrypt_with_note field for
field against decrypt() + note(ivk), over default and diversified
addresses, a wrong ivk and a wrong cmu. FindMySaplingNotes pins what
discovery returns: notes per key, diversified addresses to register,
foreign outputs ignored. Both pass before and after the switch.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Take LOCK2(cs_main, cs_wallet) before adding keys in SetUp: both
AddSaplingZKey and AddSaplingIncomingViewingKey assert cs_wallet is
held, which aborts a DEBUG_LOCKORDER build.
- TIP_HEIGHT becomes a namespace-scope constexpr. As a static class
member it was ODR-used by EXPECT_EQ without a definition, which fails
to link as C++11 at -O0.
- TearDown restores the previous chainActive tip and erases only the
mapBlockIndex entries the fixture inserted.
- Add a tx mined only in a stale side block. The legacy tx_height model
returns that block's height, as a txindex can, and the tx must still
be excluded at every minDepth.
- The encrypted variant asserts HaveSaplingSpendingKey directly, and a
new test unlocks the keystore and checks GetSaplingSpendingKey (the
other loop the crypter change touched).
- The legacy-loop comment now says its two LogPrintf lines are dropped.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CCryptoKeyStore::HaveSaplingSpendingKey and GetSaplingSpendingKey scan
mapCryptedSaplingSpendingKeys with `for (auto entry : ...)`. That copies
each entry (an extended full viewing key plus its encrypted-secret
vector) for every key visited, on every call. GetFilteredNotes'
requireSpendingKey check makes one such call per note, and the history
RPCs make one per output per address: ~22ns per key per call, measured
(11us per note on a 1024-key wallet). Iterate by const reference.
The equivalence test gains a locked, encrypted-keystore variant,
LockedWalletMatchesLegacy, so spendability coming from the encrypted
map is covered too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
For minDepth > 1, GetFilteredNotes located each wallet tx's block with
tx_height(), which calls GetTransaction and re-reads the tx from disk
through the txindex. It did this once per wallet tx, transparent ones
included, while holding LOCK2. z_listunspent and z_listreceivedbyaddress
did it once more per returned note. Without -txindex, GetTransaction can
only find a tx that still has unspent transparent outputs. tx_height()
then returned 0 for every fully shielded tx, and the tx was skipped. So
on a -txindex=0 node, sweep and consolidation (minDepth 11) and any
minconf >= 2 call never saw those notes.
The wallet already has the depth, and for an active-chain tx
GetDepthInMainChain is chainActive.Height() - height + 1. The new
HeightFromDepth() inverts that (0 when unconfirmed or off the active
chain) and replaces tx_height() at all three sites. The height only
feeds the == 0 skip and hush_dpowconfs. Both sides hold cs_main for the
whole call, so the tip can't move between the walk and the RPC tail.
On a -txindex node the results are identical: the txindex block and
wtx.hashBlock are the same block. The extended equivalence test now
also covers minDepth 2/11/12/22. On -txindex=0 nodes, fully shielded
notes now show up for minDepth > 1. With -zsweep or -consolidation
enabled, those notes will now be swept and consolidated, so init logs
that once. MinDepthAboveOneUsesWalletHeight pins the new behaviour and
shows that the old path returned nothing without a txindex.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GetFilteredNotes copied every CWalletTx by value before any filter ran,
then copied each SaplingNoteData by value again. Both copies deep-copy
the note's witness list (up to WITNESS_CACHE_SIZE witnesses, ~100KB per
note on a mainnet wallet): ~40us per note per pass, measured. Spent and
locked notes were also fully decrypted (~0.6ms of Jubjub arithmetic per
note) before the spent and locked checks threw them away. -deletetx is
off by default, so spent notes accumulate.
- iterate mapWallet and mapSaplingNoteData by const reference;
- skip txs with no notes of ours before CheckFinalTx;
- read GetDepthInMainChain once per tx instead of 2+N times (cs_main is
held, so it can't change during the walk);
- run the spent and locked checks before the decryption;
- z_listreceivedbyaddress no longer copies the whole CWalletTx per
entry to read its time, and z_sendmany's sort comparator takes its
entries by reference.
The filters have no side effects, so the result set and its order are
unchanged; the A0 equivalence test passes unchanged. One deliberate
difference: a spent or locked note whose ciphertext no longer decrypts
under its ivk is now skipped instead of tripping the decrypt assert. The
new SpentAndLockedNotesAreNotDecrypted test pins this (it aborts on the
old ordering).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds test_wallet_notecache.cpp: a wallet with Sapling notes at several
depths (spent by a mined tx, spent by an unmined tx, locked, viewing-key
only, never mined, plus txs carrying no wallet notes), mined into a fake
active chain. The fixture undoes everything it puts into the process-wide
chain state (chainActive, mapBlockIndex, pcoinsTip) in TearDown, since all
gtest suites share one process.
LegacyFilteredNotes is the 69bb035e7 loop verbatim, with tx_height()
passed in so the minDepth > 1 branch can be modelled on a -txindex node.
FilteredNotesMatchLegacy compares the real walk against it entry by entry
and in order (z_sendmany sorts the result with an unstable sort) over
minDepth {0,1} x maxDepth x ignoreSpent x requireSpendingKey x
ignoreLocked x address filters. FixtureExercisesEveryFilter checks that
each filter really removes something, so the comparison can't pass
vacuously.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DanSandClaude Opus 5.5