zecfoundation
/
zcashd
mirror of https://github.com/ZcashFoundation/zcashd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Use references instead of pointers where possible. 

Also, rename WitnessNoteIfMine -> WitnessMyNoteIfNecessary
and apply other suggestions from code review

Co-authored-by: Daira Hopwood <daira@jacaranda.org>
This commit is contained in:
furszy 2021-08-31 23:25:26 -03:00 committed by Kris Nuttycombe
parent 8fb46c794e
commit bed3c9ee3e
1 changed files with 52 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

110
src/wallet/wallet.cpp
View File

@ -2539,30 +2539,28 @@ void CWallet::ClearNoteWitnessCache()
template<typename NoteDataMap> template<typename NoteDataMap>
static void UpdateSpentHeightAndMaybePruneWitnesses(NoteDataMap& noteDataMap, int indexHeight, const uint256& nullifier) static void UpdateSpentHeightAndMaybePruneWitnesses(NoteDataMap& noteDataMap, int indexHeight, const uint256& nullifier)
{ {
for (auto& item : noteDataMap) { for (auto& [k, nd] : noteDataMap) {
auto* nd = &(item.second);
// If the note has no witnesses, then either the note has not been mined // If the note has no witnesses, then either the note has not been mined
// (and thus cannot be spent at this height), or has been spent for long // (and thus cannot be spent at this height), or has been spent for long
// enough that we will never unspend it. Either way, we can skip the // enough that we will never unspend it. Either way, we can skip the
// spentness check and pruning. // spentness check and pruning.
if (nd->witnesses.empty()) continue; if (nd.witnesses.empty()) continue;
// Update spent heights on Sprout and Sapling note data. We know here that // Update spent heights on Sprout and Sapling note data. We know here that
// the block is in the main chain (or else this function wouldn't have been // the block is in the main chain (or else this function wouldn't have been
// called with it), so any nullifier that appears in it is by definition a // called with it), so any nullifier that appears in it is by definition a
// spend. If the note has no nullifier, we can't do a spentness check. // spend. If the note has no nullifier, we can't do a spentness check.
if (nd->nullifier.has_value() && nd->nullifier.value() == nullifier) { if (nd.nullifier.has_value() && nd.nullifier.value() == nullifier) {
nd->spentHeight = indexHeight; nd.spentHeight = indexHeight;
} }
// Prune witnesses for notes spent more than WITNESS_CACHE_SIZE blocks ago, // Prune witnesses for notes spent more than WITNESS_CACHE_SIZE blocks ago,
// so we stop updating their witnesses. This is safe to do because we know // so we stop updating their witnesses. This is safe to do because we know
// we won't roll back more than WITNESS_CACHE_SIZE blocks due to checks // we won't roll back more than WITNESS_CACHE_SIZE blocks due to checks
// elsewhere in the code. // elsewhere in the code.
if (nd->spentHeight.has_value() && nd->spentHeight.value() + WITNESS_CACHE_SIZE < indexHeight) { if (nd.spentHeight.has_value() && nd.spentHeight.value() + WITNESS_CACHE_SIZE < indexHeight) {
nd->witnesses.clear(); nd.witnesses.clear();
nd->witnessHeight = -1; nd.witnessHeight = -1;
} }
} }
} }
@ -2570,51 +2568,50 @@ static void UpdateSpentHeightAndMaybePruneWitnesses(NoteDataMap& noteDataMap, in
template<typename NoteDataMap> template<typename NoteDataMap>
static void CopyPreviousWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize) static void CopyPreviousWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize)
{ {
for (auto& item : noteDataMap) { for (auto& [k, nd] : noteDataMap) {
auto* nd = &(item.second);
// Only increment witnesses that are behind the current height // Only increment witnesses that are behind the current height
if (nd->witnessHeight < indexHeight) { if (nd.witnessHeight < indexHeight) {
// Check the validity of the cache // Check the validity of the cache
// The only time a note witnessed above the current height // The only time a note witnessed above the current height
// would be invalid here is during a reindex when blocks // would be invalid here is during a reindex when blocks
// have been decremented, and we are incrementing the blocks // have been decremented, and we are incrementing the blocks
// immediately after. // immediately after.
assert(nWitnessCacheSize >= nd->witnesses.size()); assert(nWitnessCacheSize >= nd.witnesses.size());
// `witnessHeight` should only be in one of two cases: // `witnessHeight` should only be in one of two cases:
// - -1, indicating that this note does not need to track witnesses. // - -1, indicating that this note does not need to track witnesses.
// This may be because the note is not mined, or because the note // This may be because the note is not mined, or because the note
// was spent long enough ago that its witness cache was cleared. // was spent long enough ago that its witness cache was cleared.
// - The height prior to the current height, indicating that this // - The height prior to the current height, indicating that this
// note is being actively incremented. // note is being actively incremented.
assert((nd->witnessHeight == -1) || (nd->witnessHeight == indexHeight - 1)); assert((nd.witnessHeight == -1) || (nd.witnessHeight == indexHeight - 1));
// Copy the witness for the previous block if we have one // Copy the witness for the previous block if we have one
if (nd->witnesses.size() > 0) { if (nd.witnesses.size() > 0) {
nd->witnesses.push_front(nd->witnesses.front()); nd.witnesses.push_front(nd.witnesses.front());
} }
if (nd->witnesses.size() > WITNESS_CACHE_SIZE) { if (nd.witnesses.size() > WITNESS_CACHE_SIZE) {
nd->witnesses.pop_back(); nd.witnesses.pop_back();
} }
} }
} }
} }
template<typename NoteData> template<typename NoteData>
static void AppendNoteCommitment(NoteData* nd, int indexHeight, int64_t nWitnessCacheSize, const uint256& note_commitment) static void AppendNoteCommitment(NoteData& nd, int indexHeight, int64_t nWitnessCacheSize, const uint256& note_commitment)
{ {
// No empty witnesses can reach here. Before any append, the note must be already witnessed. // No empty witnesses can reach here. Before any append, the note must be already witnessed.
if (nd->witnessHeight < indexHeight && nd->witnesses.size() > 0) { if (nd.witnessHeight < indexHeight && nd.witnesses.size() > 0) {
// Check the validity of the cache // Check the validity of the cache
// See comment in CopyPreviousWitnesses about validity. // See comment in CopyPreviousWitnesses about validity.
assert(nWitnessCacheSize >= nd->witnesses.size()); assert(nWitnessCacheSize >= (int64_t) nd.witnesses.size());
nd->witnesses.front().append(note_commitment); nd.witnesses.front().append(note_commitment);
} }
} }
template<typename NoteData, typename Witness> template<typename NoteData, typename Witness>
static void WitnessNoteIfMine(NoteData* nd, int indexHeight, int64_t nWitnessCacheSize, const Witness& witness) static void WitnessMyNoteIfNecessary(NoteData& nd, int indexHeight, int64_t nWitnessCacheSize, const Witness& witness)
{ {
if (nd->witnessHeight < indexHeight) { if (nd.witnessHeight < indexHeight) {
if (nd->witnesses.size() > 0) { if (!nd.witnesses.empty()) {
// We think this can happen because we write out the // We think this can happen because we write out the
// witness cache state after every block increment or // witness cache state after every block increment or
// decrement, but the block index itself is written in // decrement, but the block index itself is written in
@ -2624,27 +2621,26 @@ static void WitnessNoteIfMine(NoteData* nd, int indexHeight, int64_t nWitnessCac
// doesn't affect existing cached notes because of the // doesn't affect existing cached notes because of the
// NoteData::witnessHeight checks. See #1378 for details. // NoteData::witnessHeight checks. See #1378 for details.
LogPrintf("Inconsistent witness cache state found\n- Cache size: %d\n- Top (height %d): %s\n- New (height %d): %s\n", LogPrintf("Inconsistent witness cache state found\n- Cache size: %d\n- Top (height %d): %s\n- New (height %d): %s\n",
nd->witnesses.size(), nd.witnesses.size(),
nd->witnessHeight, nd.witnessHeight,
nd->witnesses.front().root().GetHex(), nd.witnesses.front().root().GetHex(),
indexHeight, indexHeight,
witness.root().GetHex()); witness.root().GetHex());
nd->witnesses.clear(); nd.witnesses.clear();
} }
nd->witnesses.push_front(witness); nd.witnesses.push_front(witness);
// Set height to one less than pindex so it gets incremented // Set height to one less than pindex so it gets incremented
nd->witnessHeight = indexHeight - 1; nd.witnessHeight = indexHeight - 1;
// Check the validity of the cache // Check the validity of the cache
// See comment in CopyPreviousWitnesses about validity. // See comment in CopyPreviousWitnesses about validity.
assert(nWitnessCacheSize >= nd->witnesses.size()); assert(nWitnessCacheSize >= (int64_t) nd.witnesses.size());
} }
} }
template<typename NoteDataMap> template<typename NoteDataMap>
static void UpdateWitnessHeights(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize) static void UpdateWitnessHeights(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize)
{ {
for (auto& item : noteDataMap) { for (auto& [k, nd] : noteDataMap) {
auto* nd = &(item.second);
// At this point, we can be in one of three cases: // At this point, we can be in one of three cases:
// - Notes with a witnessHeight greater than indexHeight are not updated // - Notes with a witnessHeight greater than indexHeight are not updated
// (as this is a rescan). // (as this is a rescan).
@ -2654,15 +2650,15 @@ static void UpdateWitnessHeights(NoteDataMap& noteDataMap, int indexHeight, int6
// - Any note we are not witnessing because either it hasn't been mined // - Any note we are not witnessing because either it hasn't been mined
// yet or it was spent more than WITNESS_CACHE_SIZE blocks ago, is // yet or it was spent more than WITNESS_CACHE_SIZE blocks ago, is
// guaranteed to have no witnesses and a witnessHeight of -1. // guaranteed to have no witnesses and a witnessHeight of -1.
if (nd->witnessHeight < indexHeight) { if (nd.witnessHeight < indexHeight) {
if (nd->witnesses.empty()) { if (nd.witnesses.empty()) {
assert(nd->witnessHeight == -1); assert(nd.witnessHeight == -1);
} else { } else {
nd->witnessHeight = indexHeight; nd.witnessHeight = indexHeight;
} }
// Check the validity of the cache // Check the validity of the cache
// See comment in CopyPreviousWitnesses about validity. // See comment in CopyPreviousWitnesses about validity.
assert(nWitnessCacheSize >= nd->witnesses.size()); assert(nWitnessCacheSize >= (int64_t) nd.witnesses.size());
} }
} }
} }
@ -2672,7 +2668,7 @@ static void IncrementNoteWitnesses(std::map<OutPoint, NoteData>& noteDataMap,
const std::vector<uint256>& noteCommitments, const std::vector<uint256>& noteCommitments,
const std::vector<uint256>& nullifiers, const std::vector<uint256>& nullifiers,
int chainHeight, int chainHeight,
int prevWitCacheSize, int nPrevWitnessCacheSize,
int nWitnessCacheSize) int nWitnessCacheSize)
{ {
if (noteDataMap.empty()) return; // Nothing to do if (noteDataMap.empty()) return; // Nothing to do
@ -2686,12 +2682,12 @@ static void IncrementNoteWitnesses(std::map<OutPoint, NoteData>& noteDataMap,
// For any notes that still have stored witnesses (and thus are still being // For any notes that still have stored witnesses (and thus are still being
// incremented), copy their previous witness so we have a starting point to // incremented), copy their previous witness so we have a starting point to
// which we can append this block's commitments. // which we can append this block's commitments.
::CopyPreviousWitnesses(noteDataMap, chainHeight, prevWitCacheSize); ::CopyPreviousWitnesses(noteDataMap, chainHeight, nPrevWitnessCacheSize);
// Append new notes commitments. // Append new notes commitments.
for (const auto& noteComm : noteCommitments) { for (const auto& noteComm : noteCommitments) {
for (auto& item : noteDataMap) { for (auto& item : noteDataMap) {
::AppendNoteCommitment(&(item.second), chainHeight, nWitnessCacheSize, noteComm); ::AppendNoteCommitment(item.second, chainHeight, nWitnessCacheSize, noteComm);
} }
} }
@ -2711,10 +2707,8 @@ void CWallet::IncrementNoteWitnesses(
int chainHeight = pindex->nHeight; int chainHeight = pindex->nHeight;
// Set the update cache flag. // Set the update cache flag.
int64_t prevWitCacheSize = nWitnessCacheSize; int64_t nPrevWitnessCacheSize = nWitnessCacheSize;
if (nWitnessCacheSize < WITNESS_CACHE_SIZE) { nWitnessCacheSize = std::min(nWitnessCacheSize + 1, (int64_t) WITNESS_CACHE_SIZE);
nWitnessCacheSize += 1;
}
// Read the block from disk if we don't already have it. // Read the block from disk if we don't already have it.
const CBlock* pblock {pblockIn}; const CBlock* pblock {pblockIn};
@ -2760,7 +2754,7 @@ void CWallet::IncrementNoteWitnesses(
// Append note commitment to the notes belonging to the wallet found in this block. // Append note commitment to the notes belonging to the wallet found in this block.
// This is done here to append only the notes that occur after the witness. // This is done here to append only the notes that occur after the witness.
for (auto& item : inBlockNotesSprout) { for (auto& item : inBlockNotesSprout) {
::AppendNoteCommitment(item.second, pindex->nHeight, nWitnessCacheSize, note_commitment); ::AppendNoteCommitment(*(item.second), pindex->nHeight, nWitnessCacheSize, note_commitment);
} }
// For each note in the transaction that is for this wallet, witness it for the // For each note in the transaction that is for this wallet, witness it for the
@ -2770,7 +2764,7 @@ void CWallet::IncrementNoteWitnesses(
auto ndIt = wtx->mapSproutNoteData.find({hash, i, j}); auto ndIt = wtx->mapSproutNoteData.find({hash, i, j});
if (ndIt != wtx->mapSproutNoteData.end()) { if (ndIt != wtx->mapSproutNoteData.end()) {
SproutNoteData* nd = &ndIt->second; SproutNoteData* nd = &ndIt->second;
::WitnessNoteIfMine(nd, chainHeight, nWitnessCacheSize, frontiers.sprout.witness()); ::WitnessMyNoteIfNecessary(*nd, chainHeight, nWitnessCacheSize, frontiers.sprout.witness());
inBlockNotesSprout.emplace_back(std::make_pair(wtx, nd)); inBlockNotesSprout.emplace_back(std::make_pair(wtx, nd));
} }
} }
@ -2788,7 +2782,7 @@ void CWallet::IncrementNoteWitnesses(
// Append note commitment to the notes belonging to the wallet found in this block. // Append note commitment to the notes belonging to the wallet found in this block.
// This is done here to append only the notes that occur after the witness. // This is done here to append only the notes that occur after the witness.
for (auto& item : inBlockNotesSapling) { for (auto& item : inBlockNotesSapling) {
::AppendNoteCommitment(item.second, chainHeight, nWitnessCacheSize, note_commitment); ::AppendNoteCommitment(*(item.second), chainHeight, nWitnessCacheSize, note_commitment);
} }
// For each note in the transaction that is for this wallet, witness it for the // For each note in the transaction that is for this wallet, witness it for the
@ -2798,7 +2792,7 @@ void CWallet::IncrementNoteWitnesses(
auto ndIt = wtx->mapSaplingNoteData.find({hash, i}); auto ndIt = wtx->mapSaplingNoteData.find({hash, i});
if (ndIt != wtx->mapSaplingNoteData.end()) { if (ndIt != wtx->mapSaplingNoteData.end()) {
SaplingNoteData* nd = &ndIt->second; SaplingNoteData* nd = &ndIt->second;
::WitnessNoteIfMine(nd, chainHeight, nWitnessCacheSize, frontiers.sapling.witness()); ::WitnessMyNoteIfNecessary(*nd, chainHeight, nWitnessCacheSize, frontiers.sapling.witness());
inBlockNotesSapling.emplace_back(std::make_pair(wtx, nd)); inBlockNotesSapling.emplace_back(std::make_pair(wtx, nd));
} }
} }
@ -2827,14 +2821,14 @@ void CWallet::IncrementNoteWitnesses(
noteCommitmentsSprout, noteCommitmentsSprout,
nullifiersSprout, nullifiersSprout,
chainHeight, chainHeight,
prevWitCacheSize, nPrevWitnessCacheSize,
nWitnessCacheSize); nWitnessCacheSize);
// Sapling // Sapling
::IncrementNoteWitnesses(wtx.mapSaplingNoteData, ::IncrementNoteWitnesses(wtx.mapSaplingNoteData,
noteCommitmentsSapling, noteCommitmentsSapling,
nullifiersSapling, nullifiersSapling,
chainHeight, chainHeight,
prevWitCacheSize, nPrevWitnessCacheSize,
nWitnessCacheSize); nWitnessCacheSize);
} }