Previously we used `Uuid::from_fields` to ensure that the various UUID
fields were correctly constructed, but now that we have a CI lint that
checks this automatically, we can consistently use `Uuid::from_u128`
which is easier to prepare from `uuidgen` output.
We don't know at truncation time what the latest chain tip is; the chain
might have reorged to a shorter heavier chain, or the reorg depth might
only be a few blocks. `WalletDb::chain_height` uses the scan queue as
its source of truth, so the `Verify` range we add during truncation
(to prioritise determining whether the rewind was sufficient) can't
extend beyond the block height we know to exist.
The next call to `WalletDb::update_chain_tip` will add additional ranges
beyond this height, which might include a `Verify` range that ends up
merging with the one added during truncation.
The `LEFT OUTER JOIN` was causing the `tx.block IS NULL` check to alias
two cases: an unspent transparent output, and a transparent output spent
in an unmined transaction. The latter only makes sense to include in the
UTXO count if the transaction is expired, and (due to limitations of the
transparent data model in the current wallet) if that expiry won't be
undone by a reorg. We now handle these two cases directly.
Partly reverts 8828276361.
Closeszcash/librustzcash#983.
Co-authored-by: Kris Nuttycombe <kris@nutty.land>
At present, [zcash/zcash-android-wallet-sdk] supports Android API 27,
which bundles SQLite 3.19. SQLite support for the `TRUE` and `FALSE`
constants were introduced in SQLite 3.23, so we cannot currently use
these constants and retain support for Android API 27.
This version support limitation applies only to the `v_transactions`
and `v_tx_outputs` views, which are considered part of the public API
of this crate; other use of more recent SQLite features is fine because
they rely upon the SQLite bundled via our use of the `rusqlite` crate
and feature compatibility is verified via the unit tests of this crate.
This allows us to display additional block information when a
transparent UTXO was provided for a block that we have scanned.
Also, this ensures that memos from a wallet to itself are not
double-counted.
Co-authored-by: str4d <thestr4d@gmail.com>
Under normal usage conditions, the `transactions` table is not currently
populated for transactions involving transparent UTXOs, and so this join
was always resulting in transparent UTXO information being filtered out
from the transaction history.
Fixes [zcash/ZcashLightClientKit#1271]
This adds the `data_api::scanning::spanning_tree` module under
a new `unstable-spanning-tree` feature flag, making it available to
other implementations who want to be able to write their own storage
backends without having to reinvent the spanning tree logic.
This fixes the following bug:
Due to complexities related to non-linear scanning, checkpoints are only
added to the wallet's commitment tree in cases where there are notes
discovered within a scanned block. At present, the `shardtree` API only
makes it possible to add multiple checkpoints of the same tree state
when adding checkpoints at the chain tip, and this functionality is not
used by `zcash_client_backend` because we perform checkpoint insertion
in batches that may contain gaps in the case that multiple blocks
contain no Sapling notes. While it would be possible to fix this by
altering the `shardtree` API to permit explicit insertion of multiple
checkpoints of the same tree state at a given note position, this fix
takes a simpler approach.
Instead of ensuring that a checkpoint exists at every block and
computing the required checkpoint depth directly from the minimum number
of confirmations required when attempting a spend, we alter the
`WalletCommitmentTrees` API to allow internal information of the note
commitment tree to be used to determine this checkpoint depth, given the
minimum number of commitments as an argument. This allows us to select a
usable checkpoint from the sparse checkpoint set that resulted from the
sparse insertion of checkpoints described above.
The `v_sapling_shard_scan_ranges` view pairs every scan range with every
shard range, such that each row shows an overlapping pair.
For the complete shards, this is an overlap check between two ranges,
which the previous query was performing correctly (if verbosely).
For the last incomplete shard, we have a half-open range that needs to
be handled separately. The previous query only handled the case where a
scan range was contained within the last shard, and did not handle the
case where the scan range contained the last shard.
This led to a puzzling bug, where `WalletDb::get_wallet_summary` was
sometimes treating any note received within the last shard as part of
the wallet's pending balance. If the wallet's scan queue contained a
range that encompassed the last incomplete shard, the bug in the
`v_sapling_shard_scan_ranges` view meant that it omitted any mention of
the last shard, which translated into these notes being considered
unmined when joining `sapling_received_notes` against the sub-view
`v_sapling_shards_scan_state`.
The bug was made harder to diagnose due to the previous commit's bug
that was causing scan ranges to not be correctly merged; this resulted
in smaller scan ranges that were more likely to be contained within the
last shard, making it visible in `v_sapling_shard_scan_ranges` and
enabling notes to be detected as mined.
The fixed view uses a simpler query that enables us to handle complete
and incomplete shards together.
Time spent investigating and fixing: 4.5 hours
Co-authored-by: Kris Nuttycombe <kris@nutty.land>
In the sandblasting block ranges, shard trees only cover a few hundred
blocks at most. When scanning block ranges much larger than this, it is
likely that when a note is discovered, its parent shard tree is entirely
contained within the scan range. In this situation, `extended_range`
would be set to `range`, and then because an extended range existed,
ranges with `FoundNote` priority would always be created - that in this
case are empty.
In an effectively-linear-scanning wallet situation, this leads to a
`SpanningTree` being constructed with adjacent `Scanned` ranges,
separated by empty `FoundNote` ranges, which it was unable to merge.
We address this by both preventing generation of the empty `FoundNote`
ranges, and by defensively fixing `SpanningTree::into_vec` to skip empty
ranges.
Co-authored-by: Kris Nuttycombe <kris@nutty.land>
This modifies `update_chain_tip` and `scan_complete` to ensure that
newly created scan ranged do not extend below the wallet birthday
height.
Fixes#947
The previous code was checking that the existing shard range was at
least partially inside the proposed insertion range, but this didn't
handle the case where the proposed insertion range was fully contained
by the existing shard range.
The intent of this API is to provide a single API which returns in a
single call:
* per-account balances, including pending values
* wallet sync progress
Fixes#865Fixes#900
This also removes the zcash_client_sqlite-specific database
initialization procedures in favor of a standardized approach using the
methods available via the data access API.
When `force_rescans` is set to `true` in a call to
`replace_queue_entries`, previously scanned ranges will have their
existing priority overwritten by the scan priority for a provided range;
otherwise, the existing scan priority dominance rule continues to be
enforced. This enables us to require previously scanned ranges be
re-scanned without interfering with higher-priority scan operations.
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.
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.
Previously `extended_range` only covered the extent of the leaves of
all subtrees in which notes were found during a scan. When the scanned
range was large, this was not guaranteed to be contained within the
subtree leaves, causing an assertion failure when an invalid `ScanRange`
was constructed.
The Merkle hashes used for the note commitment trees are domain
separated by level, so when pretending that the subtree roots are leaves
of the cap tree, we need to adjust for their level not being zero.
Closeszcash/librustzcash#874.
Co-authored-by: Sean Bowe <ewillbefull@gmail.com>
This implements a priority queue backed by the wallet database for scan
range ordering. The scan queue is updated on each call to `put_blocks`
or to `update_chain_tip`.
Instead of calling `put_block` for each block scanned,
`scan_cached_blocks` will now defer the block writes until the scan of a
batch is complete and will perform the block writes and note commitment
tree updates all within a single transaction.
This should ordinarily be fine in terms of memory consumption, because
the block data being saved is pruned to only that spend an output
information that is related to transactions in the wallet, which will
normally be sparse enough that the block range size that is appropriate
for a given platform to run within a batch scanner adequately bounds the
memory consumption of this pruned representation.
There are cases where we wish to return informaiton that is relevant to
a specific shielded protocol and `Transparent` is an invalid case. This
is a minor preparatory refactoring that makes this distinction
expressible.
In preparation for out-of-order range-based scanning, it is necessary
to ensure that the size of the Sapling note commitment tree is carried
along through the scan process and that stored blocks are always
persisted with the updated note commitment tree size.
Local chain validation will be performed internal to
`scan_cached_blocks`, and as handling of chain reorgs will need to
change to support out-of-order scanning, the `validate_chain` method
will be superfluous. It is removed in advance of other changes in order
to avoid updating it to reflect the forthcoming changes.
We move thes fields out into a separate BlockMetadata struct to ensure
that future additions to block metadata are structurally separated from
future additions to block data.
`rusqlite` includes a mechanism for creating prepared statements that
automatically caches them and reuses the caches when possible. This
means that it's unnecessary for us to do our own caching, and also
offers a minor performance improvement in that we don't need to eagerly
prepare statements that we may not execute in the lifetime of a given
`WalletDb` object. It also improves code locality, because the prepared
statements are now adjacent in the code to the parameter assignment
blocks that correspond to those statements.
This also updates a number of `put_x` methods to use sqlite upsert
functionality via the `ON CONFLICT` clause, instead of having to perform
separate inserts and updates.
Memos may be absent for both sent and received notes in cases where only
compact block information has been used to populate the wallet database.
This fixes a potential crash in the case that we attempt to decode a
SQLite `NULL` as a byte array.
Fixes#384
(cherry picked from commit d99b4d4d6e)
Memos may be absent for both sent and received notes in cases where only
compact block information has been used to populate the wallet database.
This fixes a potential crash in the case that we attempt to decode a
SQLite `NULL` as a byte array. It does, however, introduce a slight
semantic confusion that will need to be considered in the case of future
updates where a note may not have an associated memo; at present, the
only reason we might not have the memo is that we might not have
retrieved the full transaction information from the chain, but in the
future there might be other possible reasons for this absence.
Fixes#384
This removes the `CommitmentTree`, `IncrementalWitness`, and
`MerklePath` types in favor of equivalent versions available
from the `incrementalmerkletree` crate.
This is in preparation for extraction into the `incrementalmerkletree`
crate, which is not Sapling-specific and therefore cannot hard-code
the depths of these data structures.
This better reflects the semantics of wallet behavior. Also, this
adds a `zcash_client_backend::WalletRead::get_min_unspent_height`
method that replaces the deprecated & removed (and misleadingly
named) `get_rewind_height` method.
This change also settles on `account_value_delta` as the name of the
column in `v_transactions` that describes the transaction's effect on
the value of the associated account.
This removes the path-based dependencies on the `zcash_note_encryption`
crate in favor of using versioned dependencies locally. This better
reflects the future state in which `zcash_note_encryption` is factored
out of the workspace and maintained in a separate repository.
The `halo2_proofs/multicore` flag must be disabled when running wasm
builds; this ensures that we do not accidentally include it as a
transitive dependency when building with `--no-default-features`.
This change makes it possible for wallets using the
`zcash_client_backend::data_api::wallet` module to perform transaction
preparation, including input selection and fee calculation, as an
independent step prior to creating proofs and signatures. This can be
used to improve user experience by making it possible to report the
proposed effects of the transaction to the wallet user (including
privacy implications) prior to authorizing the transaction.
Previously, if a caller wanted to use a block source to perform
scanning from the first available block, they would have to guess
at the block height to start from. Changing this to an optional
argument makes this explicit.
This allows callers to validate smaller intervals of the given
`BlockSourceT` shortening processing times of the function call at the
expense of obtaining a partial result on a given section of interest of
the block source.
`params: &ParamsT` has been removed from the arguments since they were
only needed to fall back to `sapling_activation_height` when `None` as
passed as the `validate_from` argument. Passing `None` as validation
start point on a pre-populated `block_source` would result in an error
`ChainError::block_height_discontinuity(sapling_activation_height - 1, current_height)`
With this new API callers must specify a concrete `validate_from`
argument and assume that `validate_chain` will not take any default
fallbacks to chain `ParamsT`.
The addition of a `limit` to the chain validation function changes the
meaning of its successful output, being now a `BlockHeight, BlockHash)`
tuple indicating the block height and block hash up to which the chain
as been validated on its continuity of heights and hashes. Callers
providing a `limit` aregument are responsible of subsequent calls to
`validate_chain()` to complete validating the remaining blocks stored on
the `block_source`.
Closeszcash/librustzcash#705
The MSRVs of the component crates are left as-is, partly because our
dependencies don't require us to bump them, and partly because those
crates have no pending changes and are relatively stable. We also plan
to split the component crates out into a separate repository, where it
will be easier to have a separate MSRV.
Closeszcash/librustzcash#759.