DragonX/dragonx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 4 Wiki Activity

wallet: note-cache review follow-ups

Browse Source 
- UpdatedNoteData now keeps the incoming plaintext when it keeps the
  wallet's own entries. When it returns "unchanged" (equal note data,
  or SaplingNoteDataIdentical), any entry lacking a plaintext takes the
  incoming one, for the same outpoint and ivk only. The startup rescan
  replays loaded txs with FindMySaplingNotes' freshly decrypted data;
  that plaintext was thrown away, so the warm-up decrypted those notes a
  second time. The cache is outside every comparison, so the function
  still returns false and nothing is rewritten.
- -checknotecache is read once at init into fCheckNoteCache, instead of
  a mapArgs lookup on every GetFilteredNotes call. Its help text and
  comment now say what it checks: every note the walk considers, before
  the address filter, so a corrupt cached address can't drop a note
  unchecked.
- GetFilteredNotes pre-sizes its result only when there's no address
  filter, so a one-address call no longer reserves room for the whole
  wallet.
- WarmSaplingNoteCache returns its counts (notes, filled, no such
  output, undecryptable), and FillSaplingNoteCache records the threads
  it used, so tests can tell the cases apart.
- WarmSaplingNoteCache's comment no longer claims nothing can wait on
  cs_wallet while it runs: the RPCs allowed during warmup can.

Tests:
- ReplayWithPlaintextIsNoUpdate pins the rule that the plaintext cache
  stays out of the "did this tx change?" decision. A rescan replays a
  witnessed note loaded without plaintext, using FindMySaplingNotes'
  data that carries one. That must be no update: no rewrite, and the
  witness and nullifier are kept. The stored entry then gains the
  plaintext. Checked by mutation: with cache presence added to
  SaplingNoteDataIdentical, this test fails and every other test still
  passes.
- ParallelFillMatchesSerial uses 460 notes and asserts the thread count.
  So the 7-thread case really runs 7 threads, with uneven shards
  (460 % 7 != 0). Before, it was capped at 4 and repeated the 4-thread
  case.
- ColdParallelFillMatchesLegacy: a cold GetFilteredNotes over ~270
  candidates fills on 4 threads and matches the 69bb035e7 loop entry
  for entry.
- WarmUpSkipsBadNotes asserts the warm-up's separate counts (1 no such
  output, 2 undecryptable), so removing the out-of-range guard is now
  caught.
- CachedPlaintextMatchesFreshDecrypt checks exact counts.
  CheckNoteCacheRepairsADifference also tampers with a memo, and looks
  entries up by outpoint. The meaningless GetHash check is dropped.
- A ScopedArg / ScopedCheckNoteCache RAII helper restores -args and
  fCheckNoteCache even when an assertion returns early.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
DanS
2026-10-17 08:16:09 -05:00
co-authored by Claude Opus 5.5
parent 23877e1038
commit 190023f85f
4 changed files with 200 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/wallet/wallet.cpp
+41 -16
View File
@@ -78,6 +78,7 @@ CBlockIndex *hush_chainactive(int32_t height);
extern int32_t HUSH_LOADINGBLOCKS;
int32_t hush_dpowconfs(int32_t height,int32_t numconfs);
bool fTxDeleteEnabled = false;
bool fCheckNoteCache = false;
bool fTxConflictDeleteEnabled = false;
int fDeleteInterval = DEFAULT_TX_DELETE_INTERVAL;
unsigned int fDeleteTransactionsAfterNBlocks = DEFAULT_TX_RETENTION_BLOCKS;
@@ -2034,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())
@@ -2053,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
@@ -2093,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;
}
@@ -5782,49 +5801,55 @@ unsigned int CWallet::FillSaplingNoteCache(const std::vector<SaplingNoteToDecryp
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, so nothing waits on cs_wallet meanwhile. Unlike GetFilteredNotes it
// never asserts, so a corrupt note can't stop the node from starting (or from serving the key-export RPCs
// allowed during warmup); GetFilteredNotes still asserts on such a note, as before.
void CWallet::WarmSaplingNoteCache()
// 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;
unsigned int nNotes = 0, nBad = 0;
for (auto& p : mapWallet) {
CWalletTx& wtx = p.second;
for (auto& item : wtx.mapSaplingNoteData) {
nNotes++;
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);
nBad++;
result.noSuchOutput++;
continue;
}
uncached.push_back(SaplingNoteToDecrypt { &item.second, &wtx.vShieldedOutput[item.first.n] });
}
}
const unsigned int nFilled = uncached.empty() ? 0 : FillSaplingNoteCache(uncached);
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());
nBad++;
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__, nFilled, nNotes, nMs,
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. A diagnostic for
// the cache; it costs the full decryption it exists to avoid.
// -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);
@@ -5966,12 +5991,12 @@ void CWallet::GetFilteredNotes(
const unsigned int nFilled = uncached.empty() ? 0 : FillSaplingNoteCache(uncached);
// Pass 2: the filters that need the note's address.
const bool fCheckCache = GetBoolArg("-checknotecache", false);
saplingEntries.reserve(saplingEntries.size() + candidates.size());
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 (fCheckCache)
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);