Merge pull request #6069 from str4d/prune-witnesses-for-definitely-spent-notes

wallet: Prune witnesses for notes spent more than 100 blocks ago
This commit is contained in:
str4d 2022-07-14 20:19:49 +01:00 committed by GitHub
commit e75213ff65
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 161 additions and 33 deletions

View File

@ -2537,7 +2537,38 @@ void CWallet::ClearNoteWitnessCache()
}
template<typename NoteDataMap>
void CopyPreviousWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize)
static void UpdateSpentHeightAndMaybePruneWitnesses(NoteDataMap& noteDataMap, int indexHeight, const uint256& nullifier)
{
for (auto& item : noteDataMap) {
auto* nd = &(item.second);
// 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
// enough that we will never unspend it. Either way, we can skip the
// spentness check and pruning.
if (nd->witnesses.empty()) continue;
// 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
// 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.
if (nd->nullifier.has_value() && nd->nullifier.value() == nullifier) {
nd->spentHeight = indexHeight;
}
// 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
// we won't roll back more than WITNESS_CACHE_SIZE blocks due to checks
// elsewhere in the code.
if (nd->spentHeight.has_value() && nd->spentHeight.value() + WITNESS_CACHE_SIZE < indexHeight) {
nd->witnesses.clear();
nd->witnessHeight = -1;
}
}
}
template<typename NoteDataMap>
static void CopyPreviousWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize)
{
for (auto& item : noteDataMap) {
auto* nd = &(item.second);
@ -2549,8 +2580,12 @@ void CopyPreviousWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t nW
// have been decremented, and we are incrementing the blocks
// immediately after.
assert(nWitnessCacheSize >= nd->witnesses.size());
// Witnesses being incremented should always be either -1
// (never incremented or decremented) or one below indexHeight
// `witnessHeight` should only be in one of two cases:
// - -1, indicating that this note does not need to track witnesses.
// This may be because the note is not mined, or because the note
// was spent long enough ago that its witness cache was cleared.
// - The height prior to the current height, indicating that this
// note is being actively incremented.
assert((nd->witnessHeight == -1) || (nd->witnessHeight == indexHeight - 1));
// Copy the witness for the previous block if we have one
if (nd->witnesses.size() > 0) {
@ -2564,7 +2599,7 @@ void CopyPreviousWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t nW
}
template<typename NoteDataMap>
void AppendNoteCommitment(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize, const uint256& note_commitment)
static void AppendNoteCommitment(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize, const uint256& note_commitment)
{
for (auto& item : noteDataMap) {
auto* nd = &(item.second);
@ -2578,7 +2613,7 @@ void AppendNoteCommitment(NoteDataMap& noteDataMap, int indexHeight, int64_t nWi
}
template<typename OutPoint, typename NoteData, typename Witness>
void WitnessNoteIfMine(std::map<OutPoint, NoteData>& noteDataMap, int indexHeight, int64_t nWitnessCacheSize, const OutPoint& key, const Witness& witness)
static void WitnessNoteIfMine(std::map<OutPoint, NoteData>& noteDataMap, int indexHeight, int64_t nWitnessCacheSize, const OutPoint& key, const Witness& witness)
{
if (noteDataMap.count(key) && noteDataMap[key].witnessHeight < indexHeight) {
auto* nd = &(noteDataMap[key]);
@ -2609,12 +2644,25 @@ void WitnessNoteIfMine(std::map<OutPoint, NoteData>& noteDataMap, int indexHeigh
template<typename NoteDataMap>
void UpdateWitnessHeights(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize)
static void UpdateWitnessHeights(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize)
{
for (auto& item : noteDataMap) {
auto* nd = &(item.second);
// At this point, we can be in one of three cases:
// - Notes with a witnessHeight greater than indexHeight are not updated
// (as this is a rescan).
// - All newly and actively witnessed notes will have a witness height
// below indexHeight and at least one witness, for which we need to
// set the note's witnessHeight accurately.
// - 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
// guaranteed to have no witnesses and a witnessHeight of -1.
if (nd->witnessHeight < indexHeight) {
nd->witnessHeight = indexHeight;
if (nd->witnesses.empty()) {
assert(nd->witnessHeight == -1);
} else {
nd->witnessHeight = indexHeight;
}
// Check the validity of the cache
// See comment in CopyPreviousWitnesses about validity.
assert(nWitnessCacheSize >= nd->witnesses.size());
@ -2630,6 +2678,41 @@ void CWallet::IncrementNoteWitnesses(
bool performOrchardWalletUpdates)
{
LOCK(cs_wallet);
// Read the block from disk if we don't already have it.
const CBlock* pblock {pblockIn};
CBlock block;
if (!pblock) {
if (!ReadBlockFromDisk(block, pindex, consensus)) {
throw std::runtime_error(
strprintf("Can't read block %d from disk (%s)", pindex->nHeight, pindex->GetBlockHash().GetHex()));
}
pblock = &block;
}
// Update the wallet's note maps for spentness changes.
for (const CTransaction& tx : pblock->vtx) {
if (!tx.vJoinSplit.empty() || !tx.vShieldedSpend.empty()) {
for (std::pair<const uint256, CWalletTx>& wtxItem : mapWallet) {
// Sprout
for (const auto& jsdesc : tx.vJoinSplit) {
for (const uint256& nullifier : jsdesc.nullifiers) {
::UpdateSpentHeightAndMaybePruneWitnesses(
wtxItem.second.mapSproutNoteData, pindex->nHeight, nullifier);
}
}
// Sapling
for (const auto& spend : tx.vShieldedSpend) {
::UpdateSpentHeightAndMaybePruneWitnesses(
wtxItem.second.mapSaplingNoteData, pindex->nHeight, spend.nullifier);
}
}
}
}
// 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
// which we can append this block's commitments.
for (std::pair<const uint256, CWalletTx>& wtxItem : mapWallet) {
::CopyPreviousWitnesses(wtxItem.second.mapSproutNoteData, pindex->nHeight, nWitnessCacheSize);
::CopyPreviousWitnesses(wtxItem.second.mapSaplingNoteData, pindex->nHeight, nWitnessCacheSize);
@ -2646,13 +2729,6 @@ void CWallet::IncrementNoteWitnesses(
nWitnessCacheSize += 1;
}
const CBlock* pblock {pblockIn};
CBlock block;
if (!pblock) {
ReadBlockFromDisk(block, pindex, consensus);
pblock = &block;
}
for (const CTransaction& tx : pblock->vtx) {
auto hash = tx.GetHash();
bool txIsOurs = mapWallet.count(hash);
@ -2716,7 +2792,7 @@ void CWallet::IncrementNoteWitnesses(
}
template<typename NoteDataMap>
void DecrementNoteWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize)
static void DecrementNoteWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize)
{
for (auto& item : noteDataMap) {
auto* nd = &(item.second);
@ -2726,16 +2802,31 @@ void DecrementNoteWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t n
// See comment below (this would be invalid if there were a
// prior decrement).
assert(nWitnessCacheSize >= nd->witnesses.size());
// Witnesses being decremented should always be either -1
// (never incremented or decremented) or equal to the height
// of the block being removed (indexHeight)
// `witnessHeight` should only be in one of two cases:
// - -1, indicating that this note does not need to track witnesses.
// This may be because the note is not mined, or because the note
// was spent long enough ago that its witness cache was cleared.
// - The current height, indicating that this note is being actively
// decremented.
assert((nd->witnessHeight == -1) || (nd->witnessHeight == indexHeight));
if (nd->witnesses.size() > 0) {
nd->witnesses.pop_front();
}
// indexHeight is the height of the block being removed, so
// the new witness cache height is one below it.
nd->witnessHeight = indexHeight - 1;
if (nd->witnesses.empty()) {
// We are in one of three cases:
// - We weren't tracking witnesses, so we continue to not do so.
// - The note has been unmined (and we popped the last witnees
// we were tracking), so we stop tracking witnesses.
// - A rollback greater than nWitnessCacheSize has occurred, in
// which case CWallet::DecrementNoteWitnesses will fail an
// assertion after this function returns (as expected, because
// wallet assumptions are broken and we cannot progress).
nd->witnessHeight = -1;
} else {
// indexHeight is the height of the block being removed, so
// the new witness cache height is one below it.
nd->witnessHeight = indexHeight - 1;
}
}
// Check the validity of the cache
// Technically if there are notes witnessed above the current
@ -4322,7 +4413,12 @@ void CWallet::WitnessNoteCommitment(std::vector<uint256> commitments,
while (pindex) {
CBlock block;
ReadBlockFromDisk(block, pindex, Params().GetConsensus());
if (!ReadBlockFromDisk(block, pindex, Params().GetConsensus())) {
// CWallet::WitnessNoteCommitment is only called from the deprecated RPC
// methods `zc_raw_receive` and `zc_raw_joinsplit`.
throw std::runtime_error(
strprintf("Can't read block %d from disk (%s)", pindex->nHeight, pindex->GetBlockHash().GetHex()));
}
for (const CTransaction& tx : block.vtx)
{
@ -4448,7 +4544,10 @@ int CWallet::ScanForWalletTransactions(
ShowProgress(_("Rescanning..."), std::max(1, std::min(99, (int)((Checkpoints::GuessVerificationProgress(chainParams.Checkpoints(), pindex, false) - dProgressStart) / (dProgressTip - dProgressStart) * 100))));
CBlock block;
ReadBlockFromDisk(block, pindex, consensus);
if (!ReadBlockFromDisk(block, pindex, consensus)) {
throw std::runtime_error(
strprintf("Can't read block %d from disk (%s)", pindex->nHeight, pindex->GetBlockHash().GetHex()));
}
for (CTransaction& tx : block.vtx)
{
if (AddToWalletIfInvolvingMe(consensus, tx, &block, pindex->nHeight, fUpdate)) {

View File

@ -262,21 +262,31 @@ public:
std::list<SproutWitness> witnesses;
/**
* Block height corresponding to the most current witness.
* The height of the most recently-witnessed block for this note.
*
* When we first create a SproutNoteData in CWallet::FindMySproutNotes, this is set to
* -1 as a placeholder. The next time CWallet::ChainTip is called, we can
* determine what height the witness cache for this note is valid for (even
* if no witnesses were cached), and so can set the correct value in
* CWallet::IncrementNoteWitnesses and CWallet::DecrementNoteWitnesses.
* Set to -1 if the note is unmined, or if the note was spent long enough
* ago that we will never unspend it.
*/
int witnessHeight;
SproutNoteData() : address(), nullifier(), witnessHeight {-1} { }
/**
* (memory only) Block height at which this note was observed to be spent.
*
* This is used to prune the list of witnesses once we are guaranteed to
* never be unspending the note. If the node is restarted in the window
* between detecting the spend and pruning the witnesses (or before the
* pruning is serialized to disk), then the spentness will likely not be
* re-detected until a rescan is performed (meaning that this note's
* witnesses will continue to be updated, which is only a performance
* rather than a correctness issue).
*/
std::optional<int> spentHeight;
SproutNoteData() : address(), nullifier(), witnessHeight {-1}, spentHeight() { }
SproutNoteData(libzcash::SproutPaymentAddress a) :
address {a}, nullifier(), witnessHeight {-1} { }
address {a}, nullifier(), witnessHeight {-1}, spentHeight() { }
SproutNoteData(libzcash::SproutPaymentAddress a, uint256 n) :
address {a}, nullifier {n}, witnessHeight {-1} { }
address {a}, nullifier {n}, witnessHeight {-1}, spentHeight() { }
ADD_SERIALIZE_METHODS;
@ -309,15 +319,34 @@ public:
* We initialize the height to -1 for the same reason as we do in SproutNoteData.
* See the comment in that class for a full description.
*/
SaplingNoteData() : witnessHeight {-1}, nullifier() { }
SaplingNoteData() : witnessHeight {-1}, nullifier(), spentHeight() { }
SaplingNoteData(libzcash::SaplingIncomingViewingKey ivk) : ivk {ivk}, witnessHeight {-1}, nullifier() { }
SaplingNoteData(libzcash::SaplingIncomingViewingKey ivk, uint256 n) : ivk {ivk}, witnessHeight {-1}, nullifier(n) { }
std::list<SaplingWitness> witnesses;
/**
* The height of the most recently-witnessed block for this note.
*
* Set to -1 if the note is unmined, or if the note was spent long enough
* ago that we will never unspend it.
*/
int witnessHeight;
libzcash::SaplingIncomingViewingKey ivk;
std::optional<uint256> nullifier;
/**
* (memory only) Block height at which this note was observed to be spent.
*
* This is used to prune the list of witnesses once we are guaranteed to
* never be unspending the note. If the node is restarted in the window
* between detecting the spend and pruning the witnesses (or before the
* pruning is serialized to disk), then the spentness will likely not be
* re-detected until a rescan is performed (meaning that this note's
* witnesses will continue to be updated, which is only a performance
* rather than a correctness issue).
*/
std::optional<int> spentHeight;
ADD_SERIALIZE_METHODS;
template <typename Stream, typename Operation>