Reset Orchard wallet state in `CWallet::ClearNoteWitnessCache`

The general invariant in the wallet is that `CWallet::ChainTip` is only
called with sequential (either connecting or disconnecting) blocks. The
one exception to this is when starting `zcashd` with `-reindex`, which
creates a discontinuity: the node jumps back to the genesis block, and
`ThreadNotifyWallets` will similarly start notifying the wallet of the
entire chain again.

In Bitcoin Core, this behaviour was fine: there was no persistent cached
state that couldn't just be overwritten during the re-notification. For
Zcash however, wallets need to additionally maintain witnesses for notes
that are spendable, and these witnesses can generally only be amended by
sequential blocks.

For Sprout and Sapling, the discontinuity was handled by checking if a
reindex was occurring during `CWallet::InitLoadWallet`, and clearing the
witness caches in `CWallet::ClearNoteWitnessCache` if so. The witnesses
would then be rebuilt as the reindexed chain was re-connected during
`ActivateBestChain`.

The Orchard wallet stores its witnesses in a different structure on the
Rust side, so it wasn't being cleared at the same time. This meant that
when a full reindex was performed in one go, the sequentiality invariant
would be broken once `ThreadNotifyWallets` reached NU5 activation, and
the node would crash with a failed assertion (the issue at hand).

However, reindexing Zcash takes a long time, and has been historically
buggy for various reasons (e.g. crashing due to OOM). And due to a quirk
of how the `-rescan` behaviour is implemented, if a reindexing node is
restarted during its `ActivateBestChain` phase, on restart the node will
almost always trigger a rescan due to the wallet chain locator not
containing any hashes that match the on-start chain tip. And the first
thing that the rescan logic does is check whether the start of the
rescan is before NU5 activation, and reset the Orchard wallet if so.

We now reset the Orchard wallet unconditionally at the same time as we
clear the Sprout and Sapling witness caches. This additionally clears
spentness information that the Orchard wallet is storing, but that is
rebuilt during the reindex.

Closes zcash/zcash#5736.
Closes zcash/zcash#6004.
This commit is contained in:
Jack Grigg 2023-01-31 00:06:13 +00:00
parent 0d1e1ad438
commit 204950191c
1 changed files with 5 additions and 0 deletions

View File

@ -2550,6 +2550,11 @@ void CWallet::ClearNoteWitnessCache()
}
}
nWitnessCacheSize = 0;
// This resets spentness information in addition to the Orchard note witness
// caches, which is fine because it will be recovered during the reindex or
// rescan that called `ClearNoteWitnessCache()`.
orchardWallet.Reset();
}
template<typename NoteDataMap>