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.
Closeszcash/zcash#5736.
Closeszcash/zcash#6004.
MacOS still ships with a 15-year-old version of Bash so this removes some usage of features that
require newer (only 10 years old) versions.
Fixes#6367.
Co-authored-by: Mohamed AlFalasi <2569171+edubai@users.noreply.github.com>
1000 blocks was selected as a balance between limiting the likely
maximum memory usage of the batch scanner, and avoiding
artificially restricting scanning throughput of small/fast blocks
due to the second-boundary lock synchronization point.
This also removes the `zcashd.wallet.batchscanner.usage.bytes` gague
value that was previously made available when `-prometheusport` was
specified.
Co-authored-by: Jack Grigg <jack@z.cash>
We no longer fetch updates from the mempool unless we have fetched all
updates from the chain, as we would otherwise notify the wallet of
mempool changes for which they have not observed parent transactions
in the chain.
Co-authored-by: Jack Grigg <jack@z.cash>
Wallet rescanning that happens on node startup was previously not
respecting Ctrl-C. We use an explicit check of `ShutdownRequested()`
instead of a `boost::interruption_point` because wallet startup does
not run in a separate thread.
Co-authored-by: Jack Grigg <jack@z.cash>
Jack Grigg
Kris Nuttycombe
Dimitris Apostolou
Alex Morcos
Daira Hopwood
str4d
Kris Nuttycombe
TrellixVulnTeam
Greg Pfeil