This change modifies the implementation of `get_spendable_sapling_notes`
and `select_spendable_sapling_notes` to only return notes at positions
where the associated note commitment tree shard has been fully scanned.
This is slightly more conservative than it needs to be, because
there could be cases where witnesses imported into the tree in the
`shardtree_support` migration cover the complete range of a subtree (and
hence that subtree doesn't need to be re-scanned). However, we can't
detect or depend upon that condition in general without attempting to
create a witness for each note retrieved.
A possible alternative to this approach would be to not bound our query
results on the requested total, and instead attempt to construct a
witness for each note we retrieve, skipping the note if we cannot
construct a witness. However, given that accessing the note commitment
tree can be a costly operation requiring nontrivial deserialization
costs, the more conservative database-oriented approach is perhaps
better.
Prior to the scan-before-sync changes, the wallet was able to assume
that the maximum scanned block height at the time of the spend was
within a few blocks of the chain tip. However, under linear scanning
after the spend-before-sync changes this invariant no longer holds,
resulting in a situation where in linear sync conditions the wallet
could attempt to create transactions with already-past expiry heights.
This change separates the notion of "chain tip" from "max scanned
height", relying upon the `scan_queue` table to maintain the wallet's
view of the consensus chain height and using information from the
`blocks` table only in situations where the latest and/or earliest
scanned height is required.
As part of this change, the `WalletRead` interface is also modified to
disambiguate these concepts.
If `from_height = 0` and `limit > 1`, then the conditional being used to
initialise `continuity_check_metadata` was previously running for every
block, and would therefore panic while processing the block at height 1.
Previously this was not clearly specified, and the implementations in
`zcash_client_sqlite` behaved similarly to when `from_height = None`.
Closeszcash/librustzcash#892.
In general, it is preferable to use globally relevant identifiers where
possible. This PR removes the `WalletRead::TxRef` associated type in
favor of using `TxId` directly for the transaction identifier, and
restricts the use of the `NoteRef` type to those scenarios where the
result of one query is intended to be used directly as the input to
another query.
Closes#834
The `add_checkpoint` method is intended to be idempotent. In the case
that we add a checkpoint at an already-checkpointed block height, we
should only raise an error in the case that the note commitment tree
position or the set of notes spent in the checkpointed block has
changed.
The `shardtree` migration is applied to a database state that was
created via linear scanning, so we have complete wallet information for
those blocks.
We only need to load frontiers into the ShardTree that are close enough
to the wallet's known chain tip to fill `PRUNING_DEPTH` checkpoints, so
that ShardTree's witness generation will be able to correctly handle
anchor depths. Loading frontiers further back than this doesn't add any
useful nodes to the ShardTree (as we don't support rollbacks beyond
`PRUNING_DEPTH`, and we won't be finding notes in earlier blocks), and
hurts performance (as frontier importing has a significant Merkle tree
hashing cost).
Closeszcash/librustzcash#877.
Kris Nuttycombe
Andrew Arnott
Daira Emma Hopwood
Kris Nuttycombe
str4d