Merge pull request #706 from zcash/add_limit_validate_chain

Adds `limit` parameter to `validate_chain()`
This commit is contained in:
str4d 2023-01-31 21:01:47 +00:00 committed by GitHub
commit 756f388e00
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 133 additions and 124 deletions

View File

@ -9,20 +9,42 @@ and this library adheres to Rust's notion of
### Changed
- MSRV is now 1.60.0.
- `zcash_client_backend::data_api::wallet::shield_transparent_funds` now
takes a `shielding_threshold` argument that can be used to specify the
minimum value allowed as input to a shielding transaction. Previously
the shielding threshold was fixed at 100000 zatoshis.
- Note commitments now use
`zcash_primitives::sapling::note::ExtractedNoteCommitment` instead of
`bls12_381::Scalar` in the following places:
- The `cmu` field of `zcash_client_backend::wallet::WalletShieldedOutput`.
- `zcash_client_backend::proto::compact_formats::CompactSaplingOutput::cmu`.
- **breaking changes** to `zcash_client_backend::data_api::chain::validate_chain`
- `validate_chain` now requires a non-optional `validate_from` parameter that
indicates the starting point of the `BlockSourceT` validation. An Optional
`limit` can be specified as well. This allows callers to validate smaller
intervals of blocks already present on the provided `BlockSource` shortening
processing times of the function call at the expense of obtaining a partial
result.
- `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. Implementors of `BlockSource` are resposible
of definining how they will fall back to a suitable value when `validate_from`
is `None`.
- When passing a `limit` to the chain validation function, a successful output,
indicates that the chain has been validated on its continuity of heights and hashes
within the limits of the range `[validate_from, validate_from + limit)`.
Callers providing a `limit` argument are responsible for making subsequent calls to
`validate_chain()` in order to complete validating the totality of the block_source.
### Removed
- `zcash_client_backend::data_api`:
- `WalletWrite::remove_unmined_tx` (was behind the `unstable` feature flag).
## [0.6.1] - 2022-12-06
### Added
- `zcash_client_backend::data_api::chain::scan_cached_blocks` now generates

View File

@ -44,7 +44,7 @@
//! // Given that we assume the server always gives us correct-at-the-time blocks, any
//! // errors are in the blocks we have previously cached or scanned.
//! let max_height_hash = db_data.get_max_height_hash().map_err(Error::Wallet)?;
//! if let Err(e) = validate_chain(&network, &block_source, max_height_hash) {
//! if let Err(e) = validate_chain(&block_source, max_height_hash, None) {
//! match e {
//! Error::Chain(e) => {
//! // a) Pick a height to rewind to.
@ -88,7 +88,7 @@ use std::convert::Infallible;
use zcash_primitives::{
block::BlockHash,
consensus::{self, BlockHeight, NetworkUpgrade},
consensus::{self, BlockHeight},
merkle_tree::CommitmentTree,
sapling::{note_encryption::PreparedIncomingViewingKey, Nullifier},
zip32::Scope,
@ -111,7 +111,8 @@ pub trait BlockSource {
type Error;
/// Scan the specified `limit` number of blocks from the blockchain, starting at
/// `from_height`, applying the provided callback to each block.
/// `from_height`, applying the provided callback to each block. If `from_height`
/// is `None` then scanning will begin at the first available block.
///
/// * `WalletErrT`: the types of errors produced by the wallet operations performed
/// as part of processing each row.
@ -119,7 +120,7 @@ pub trait BlockSource {
/// reporting errors related to specific notes.
fn with_blocks<F, WalletErrT, NoteRefT>(
&self,
from_height: BlockHeight,
from_height: Option<BlockHeight>,
limit: Option<u32>,
with_row: F,
) -> Result<(), error::Error<WalletErrT, Self::Error, NoteRefT>>
@ -136,57 +137,52 @@ pub trait BlockSource {
/// provides accurate block information as of the time it was requested.
///
/// Arguments:
/// - `parameters` Network parameters
/// - `block_source` Source of compact blocks
/// - `from_tip` Height & hash of last validated block; if no validation has previously
/// been performed, this will begin scanning from `sapling_activation_height - 1`
/// - `validate_from` Height & hash of last validated block;
/// - `limit` specified number of blocks that will be valididated. Callers providing
/// a `limit` argument are responsible of making subsequent calls to `validate_chain()`
/// to complete validating the remaining blocks stored on the `block_source`. If `none`
/// is provided, there will be no limit set to the validation and upper bound of the
/// validation range will be the latest height present in the `block_source`.
///
/// Returns:
/// - `Ok(())` if the combined chain is valid.
/// - `Ok(())` if the combined chain is valid up to the given height
/// and block hash.
/// - `Err(Error::Chain(cause))` if the combined chain is invalid.
/// - `Err(e)` if there was an error during validation unrelated to chain validity.
///
/// This function does not mutate either of the databases.
pub fn validate_chain<ParamsT, BlockSourceT>(
parameters: &ParamsT,
pub fn validate_chain<BlockSourceT>(
block_source: &BlockSourceT,
validate_from: Option<(BlockHeight, BlockHash)>,
mut validate_from: Option<(BlockHeight, BlockHash)>,
limit: Option<u32>,
) -> Result<(), Error<Infallible, BlockSourceT::Error, Infallible>>
where
ParamsT: consensus::Parameters,
BlockSourceT: BlockSource,
{
let sapling_activation_height = parameters
.activation_height(NetworkUpgrade::Sapling)
.expect("Sapling activation height must be known.");
// The block source will contain blocks above the `validate_from` height. Validate from that
// maximum height up to the chain tip, returning the hash of the block found in the block
// source at the `validate_from` height, which can then be used to verify chain integrity by
// comparing against the `validate_from` hash.
let from_height = validate_from
.map(|(height, _)| height)
.unwrap_or(sapling_activation_height - 1);
let mut prev_height = from_height;
let mut prev_hash: Option<BlockHash> = validate_from.map(|(_, hash)| hash);
block_source.with_blocks::<_, Infallible, Infallible>(from_height, None, move |block| {
let current_height = block.height();
let result = if current_height != prev_height + 1 {
Err(ChainError::block_height_discontinuity(prev_height + 1, current_height).into())
} else {
match prev_hash {
None => Ok(()),
Some(h) if h == block.prev_hash() => Ok(()),
Some(_) => Err(ChainError::prev_hash_mismatch(current_height).into()),
block_source.with_blocks::<_, Infallible, Infallible>(
validate_from.map(|(h, _)| h),
limit,
move |block| {
if let Some((valid_height, valid_hash)) = validate_from {
if block.height() != valid_height + 1 {
return Err(ChainError::block_height_discontinuity(
valid_height + 1,
block.height(),
)
.into());
} else if block.prev_hash() != valid_hash {
return Err(ChainError::prev_hash_mismatch(block.height()).into());
}
}
};
prev_height = current_height;
prev_hash = Some(block.hash());
result
})
validate_from = Some((block.height(), block.hash()));
Ok(())
},
)
}
/// Scans at most `limit` new blocks added to the block source for any transactions received by the
@ -222,19 +218,11 @@ where
BlockSourceT: BlockSource,
DbT: WalletWrite,
{
let sapling_activation_height = params
.activation_height(NetworkUpgrade::Sapling)
.expect("Sapling activation height is known.");
// Recall where we synced up to previously.
// If we have never synced, use sapling activation height to select all cached CompactBlocks.
let mut last_height = data_db
.block_height_extrema()
.map(|opt| {
opt.map(|(_, max)| max)
.unwrap_or(sapling_activation_height - 1)
})
.map_err(Error::Wallet)?;
.map_err(Error::Wallet)?
.map(|(_, max)| max);
// Fetch the UnifiedFullViewingKeys we are tracking
let ufvks = data_db
@ -248,13 +236,21 @@ where
.collect();
// Get the most recent CommitmentTree
let mut tree = data_db
.get_commitment_tree(last_height)
.map(|t| t.unwrap_or_else(CommitmentTree::empty))
.map_err(Error::Wallet)?;
let mut tree = last_height.map_or_else(
|| Ok(CommitmentTree::empty()),
|h| {
data_db
.get_commitment_tree(h)
.map(|t| t.unwrap_or_else(CommitmentTree::empty))
.map_err(Error::Wallet)
},
)?;
// Get most recent incremental witnesses for the notes we are tracking
let mut witnesses = data_db.get_witnesses(last_height).map_err(Error::Wallet)?;
let mut witnesses = last_height.map_or_else(
|| Ok(vec![]),
|h| data_db.get_witnesses(h).map_err(Error::Wallet),
)?;
// Get the nullifiers for the notes we are tracking
let mut nullifiers = data_db.get_nullifiers().map_err(Error::Wallet)?;
@ -290,12 +286,12 @@ where
let current_height = block.height();
// Scanned blocks MUST be height-sequential.
if current_height != (last_height + 1) {
return Err(ChainError::block_height_discontinuity(
last_height + 1,
current_height,
)
.into());
if let Some(h) = last_height {
if current_height != (h + 1) {
return Err(
ChainError::block_height_discontinuity(h + 1, current_height).into(),
);
}
}
let block_hash = BlockHash::from_slice(&block.hash);
@ -366,7 +362,7 @@ where
witnesses.extend(new_witnesses);
last_height = current_height;
last_height = Some(current_height);
Ok(())
},
@ -391,7 +387,7 @@ pub mod testing {
fn with_blocks<F, DbErrT, NoteRef>(
&self,
_from_height: BlockHeight,
_from_height: Option<BlockHeight>,
_limit: Option<u32>,
_with_row: F,
) -> Result<(), Error<DbErrT, Infallible, NoteRef>>

View File

@ -18,8 +18,32 @@ and this library adheres to Rust's notion of
### Changed
- MSRV is now 1.60.0.
- **breaking changes** to `validate_chain`.
- `zcash_client_backend::data_api::chain::validate_chain` now requires a
non-optional `validate_from` parameter that indicates the starting point of
the `BlockSourceT` validation. An Optional `limit` can be specified as
well. 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 has up to which the chain as been
validated on its continuity of heights and hashes. Callers providing a
`limit` aregumente are responsible of subsequent calls to `validate_chain()`
to complete validating the totality of the block_source.
### Removed
- implementation of unstable function `WalletWrite::remove_unmined_tx`,
## [0.4.2] - 2022-12-13
### Fixed
- `zcash_client_sqlite::WalletDb::get_transparent_balances` no longer returns an

View File

@ -28,7 +28,7 @@ pub mod migrations;
/// blocks are traversed up to the maximum height.
pub(crate) fn blockdb_with_blocks<F, DbErrT, NoteRef>(
block_source: &BlockDb,
last_scanned_height: BlockHeight,
last_scanned_height: Option<BlockHeight>,
limit: Option<u32>,
mut with_row: F,
) -> Result<(), Error<DbErrT, SqliteClientError, NoteRef>>
@ -51,7 +51,7 @@ where
let mut rows = stmt_blocks
.query(params![
u32::from(last_scanned_height),
last_scanned_height.map_or(0u32, u32::from),
limit.unwrap_or(u32::max_value()),
])
.map_err(to_chain_error)?;
@ -197,7 +197,7 @@ pub(crate) fn blockmetadb_find_block(
#[cfg(feature = "unstable")]
pub(crate) fn fsblockdb_with_blocks<F, DbErrT, NoteRef>(
cache: &FsBlockDb,
last_scanned_height: BlockHeight,
last_scanned_height: Option<BlockHeight>,
limit: Option<u32>,
mut with_block: F,
) -> Result<(), Error<DbErrT, FsBlockDbError, NoteRef>>
@ -213,16 +213,16 @@ where
.conn
.prepare(
"SELECT height, blockhash, time, sapling_outputs_count, orchard_actions_count
FROM compactblocks_meta
WHERE height > ?
ORDER BY height ASC LIMIT ?",
FROM compactblocks_meta
WHERE height > ?
ORDER BY height ASC LIMIT ?",
)
.map_err(to_chain_error)?;
let rows = stmt_blocks
.query_map(
params![
u32::from(last_scanned_height),
last_scanned_height.map_or(0u32, u32::from),
limit.unwrap_or(u32::max_value()),
],
|row| {
@ -301,43 +301,38 @@ mod tests {
// Add an account to the wallet
let (dfvk, _taddr) = init_test_accounts_table(&db_data);
// Empty chain should be valid
validate_chain(
&tests::network(),
&db_cache,
db_data.get_max_height_hash().unwrap(),
)
.unwrap();
// Empty chain should return None
assert_matches!(db_data.get_max_height_hash(), Ok(None));
// Create a fake CompactBlock sending value to the address
let fake_block_hash = BlockHash([0; 32]);
let fake_block_height = sapling_activation_height();
let (cb, _) = fake_compact_block(
sapling_activation_height(),
BlockHash([0; 32]),
fake_block_height,
fake_block_hash,
&dfvk,
AddressType::DefaultExternal,
Amount::from_u64(5).unwrap(),
);
insert_into_cache(&db_cache, &cb);
// Cache-only chain should be valid
validate_chain(
&tests::network(),
let validate_chain_result = validate_chain(
&db_cache,
db_data.get_max_height_hash().unwrap(),
)
.unwrap();
Some((fake_block_height, fake_block_hash)),
Some(1),
);
assert_matches!(validate_chain_result, Ok(()));
// Scan the cache
let mut db_write = db_data.get_update_ops().unwrap();
scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap();
// Data-only chain should be valid
validate_chain(
&tests::network(),
&db_cache,
db_data.get_max_height_hash().unwrap(),
)
.unwrap();
validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None).unwrap();
// Create a second fake CompactBlock sending more value to the address
let (cb2, _) = fake_compact_block(
@ -350,23 +345,13 @@ mod tests {
insert_into_cache(&db_cache, &cb2);
// Data+cache chain should be valid
validate_chain(
&tests::network(),
&db_cache,
db_data.get_max_height_hash().unwrap(),
)
.unwrap();
validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None).unwrap();
// Scan the cache again
scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap();
// Data-only chain should be valid
validate_chain(
&tests::network(),
&db_cache,
db_data.get_max_height_hash().unwrap(),
)
.unwrap();
validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None).unwrap();
}
#[test]
@ -405,12 +390,7 @@ mod tests {
scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap();
// Data-only chain should be valid
validate_chain(
&tests::network(),
&db_cache,
db_data.get_max_height_hash().unwrap(),
)
.unwrap();
validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None).unwrap();
// Create more fake CompactBlocks that don't connect to the scanned ones
let (cb3, _) = fake_compact_block(
@ -431,11 +411,7 @@ mod tests {
insert_into_cache(&db_cache, &cb4);
// Data+cache chain should be invalid at the data/cache boundary
let val_result = validate_chain(
&tests::network(),
&db_cache,
db_data.get_max_height_hash().unwrap(),
);
let val_result = validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None);
assert_matches!(val_result, Err(Error::Chain(e)) if e.at_height() == sapling_activation_height() + 2);
}
@ -476,12 +452,7 @@ mod tests {
scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap();
// Data-only chain should be valid
validate_chain(
&tests::network(),
&db_cache,
db_data.get_max_height_hash().unwrap(),
)
.unwrap();
validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None).unwrap();
// Create more fake CompactBlocks that contain a reorg
let (cb3, _) = fake_compact_block(
@ -502,11 +473,7 @@ mod tests {
insert_into_cache(&db_cache, &cb4);
// Data+cache chain should be invalid inside the cache
let val_result = validate_chain(
&tests::network(),
&db_cache,
db_data.get_max_height_hash().unwrap(),
);
let val_result = validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None);
assert_matches!(val_result, Err(Error::Chain(e)) if e.at_height() == sapling_activation_height() + 3);
}

View File

@ -723,7 +723,7 @@ impl BlockSource for BlockDb {
fn with_blocks<F, DbErrT, NoteRef>(
&self,
from_height: BlockHeight,
from_height: Option<BlockHeight>,
limit: Option<u32>,
with_row: F,
) -> Result<(), data_api::chain::error::Error<DbErrT, Self::Error, NoteRef>>
@ -904,7 +904,7 @@ impl BlockSource for FsBlockDb {
fn with_blocks<F, DbErrT, NoteRef>(
&self,
from_height: BlockHeight,
from_height: Option<BlockHeight>,
limit: Option<u32>,
with_row: F,
) -> Result<(), data_api::chain::error::Error<DbErrT, Self::Error, NoteRef>>