zec
/
librustzcash
mirror of https://github.com/zcash/librustzcash.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Make `from_height` and `limit` parameters to `scan_cached_blocks` non-optional.

This commit is contained in:
Kris Nuttycombe 2023-06-26 10:12:59 -06:00
parent faccf56f04
commit 98147e0d7d
7 changed files with 363 additions and 118 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
zcash_client_backend/CHANGELOG.md
View File

@ -36,7 +36,8 @@ and this library adheres to Rust's notion of
and its signature has changed; it now subsumes the removed `WalletRead::get_all_nullifiers`.
- `WalletRead::get_target_and_anchor_heights` now takes its argument as a `NonZeroU32`
- `chain::scan_cached_blocks` now takes a `from_height` argument that
permits the caller to control the starting position of the scan range.
permits the caller to control the starting position of the scan range
In addition, the `limit` parameter is now required.
- A new `CommitmentTree` variant has been added to `data_api::error::Error`
- `data_api::wallet::{create_spend_to_address, create_proposed_transaction,
shield_transparent_funds}` all now require that `WalletCommitmentTrees` be

1
zcash_client_backend/src/data_api.rs
View File

@ -494,7 +494,6 @@ pub trait WalletWrite: WalletRead {
/// Updates the state of the wallet database by persisting the provided block information,
/// along with the note commitments that were detected when scanning the block for transactions
/// pertaining to this wallet.
#[allow(clippy::type_complexity)]
fn put_block(
&mut self,
block: ScannedBlock<sapling::Nullifier>,

124
zcash_client_backend/src/data_api/chain.rs
View File

@ -45,7 +45,7 @@
//! // At this point, the cache and scanned data are locally consistent (though not
//! // necessarily consistent with the latest chain tip - this would be discovered the
//! // next time this codepath is executed after new blocks are received).
//! scan_cached_blocks(&network, &block_source, &mut db_data, None, None)
//! scan_cached_blocks(&network, &block_source, &mut db_data, BlockHeight::from(0), 10)
//! # }
//! # }
//! ```
@ -60,9 +60,11 @@ use crate::{
data_api::{NullifierQuery, WalletWrite},
proto::compact_formats::CompactBlock,
scan::BatchRunner,
scanning::{add_block_to_runner, scan_block_with_runner},
scanning::{add_block_to_runner, check_continuity, scan_block_with_runner},
};
use super::BlockMetadata;
pub mod error;
use error::Error;
@ -141,8 +143,8 @@ pub fn scan_cached_blocks<ParamsT, DbT, BlockSourceT>(
params: &ParamsT,
block_source: &BlockSourceT,
data_db: &mut DbT,
from_height: Option<BlockHeight>,
limit: Option<u32>,
from_height: BlockHeight,
limit: u32,
) -> Result<(), Error<DbT::Error, BlockSourceT::Error>>
where
ParamsT: consensus::Parameters + Send + 'static,
@ -178,61 +180,81 @@ where
.map(|(tag, ivk)| (tag, PreparedIncomingViewingKey::new(&ivk))),
);
// Start at either the provided height, or where we synced up to previously.
let (scan_from, mut prior_block_metadata) = match from_height {
Some(h) => {
// if we are provided with a starting height, obtain the metadata for the previous
// block (if any is available)
(
Some(h),
if h > BlockHeight::from(0) {
data_db.block_metadata(h - 1).map_err(Error::Wallet)?
} else {
None
},
)
}
None => {
let last_scanned = data_db.block_fully_scanned().map_err(Error::Wallet)?;
last_scanned.map_or_else(|| (None, None), |m| (Some(m.block_height + 1), Some(m)))
}
let mut prior_block_metadata = if from_height > BlockHeight::from(0) {
data_db
.block_metadata(from_height - 1)
.map_err(Error::Wallet)?
} else {
None
};
block_source.with_blocks::<_, DbT::Error>(scan_from, limit, |block: CompactBlock| {
add_block_to_runner(params, block, &mut batch_runner);
Ok(())
})?;
let mut continuity_check_metadata = prior_block_metadata;
block_source.with_blocks::<_, DbT::Error>(
Some(from_height),
Some(limit),
|block: CompactBlock| {
// check block continuity
if let Some(scan_error) = check_continuity(&block, continuity_check_metadata.as_ref()) {
return Err(Error::Scan(scan_error));
}
continuity_check_metadata = continuity_check_metadata.as_ref().map(|m| {
BlockMetadata::from_parts(
block.height(),
block.hash(),
block
.chain_metadata
.as_ref()
.map(|m| m.sapling_commitment_tree_size)
.unwrap_or_else(|| {
m.sapling_tree_size()
+ u32::try_from(
block.vtx.iter().map(|tx| tx.outputs.len()).sum::<usize>(),
)
.unwrap()
}),
)
});
add_block_to_runner(params, block, &mut batch_runner);
Ok(())
},
)?;
batch_runner.flush();
block_source.with_blocks::<_, DbT::Error>(scan_from, limit, |block: CompactBlock| {
let scanned_block = scan_block_with_runner(
params,
block,
&dfvks,
&sapling_nullifiers,
prior_block_metadata.as_ref(),
Some(&mut batch_runner),
)
.map_err(Error::Scan)?;
block_source.with_blocks::<_, DbT::Error>(
Some(from_height),
Some(limit),
|block: CompactBlock| {
let scanned_block = scan_block_with_runner(
params,
block,
&dfvks,
&sapling_nullifiers,
prior_block_metadata.as_ref(),
Some(&mut batch_runner),
)
.map_err(Error::Scan)?;
let spent_nf: Vec<&sapling::Nullifier> = scanned_block
.transactions
.iter()
.flat_map(|tx| tx.sapling_spends.iter().map(|spend| spend.nf()))
.collect();
sapling_nullifiers.retain(|(_, nf)| !spent_nf.contains(&nf));
sapling_nullifiers.extend(scanned_block.transactions.iter().flat_map(|tx| {
tx.sapling_outputs
let spent_nf: Vec<&sapling::Nullifier> = scanned_block
.transactions
.iter()
.map(|out| (out.account(), *out.nf()))
}));
.flat_map(|tx| tx.sapling_spends.iter().map(|spend| spend.nf()))
.collect();
prior_block_metadata = Some(*scanned_block.metadata());
data_db.put_block(scanned_block).map_err(Error::Wallet)?;
Ok(())
})?;
sapling_nullifiers.retain(|(_, nf)| !spent_nf.contains(&nf));
sapling_nullifiers.extend(scanned_block.transactions.iter().flat_map(|tx| {
tx.sapling_outputs
.iter()
.map(|out| (out.account(), *out.nf()))
}));
prior_block_metadata = Some(*scanned_block.metadata());
data_db.put_block(scanned_block).map_err(Error::Wallet)?;
Ok(())
},
)?;
Ok(())
}

105
zcash_client_backend/src/scanning.rs
View File

@ -21,7 +21,7 @@ use zcash_primitives::{
zip32::{sapling::DiversifiableFullViewingKey, AccountId, Scope},
};
use crate::data_api::{BlockMetadata, ScannedBlock};
use crate::data_api::{BlockMetadata, ScannedBlock, ShieldedProtocol};
use crate::{
proto::compact_formats::CompactBlock,
scan::{Batch, BatchRunner, Tasks},
@ -116,17 +116,29 @@ pub enum ScanError {
/// the current chain tip.
PrevHashMismatch { at_height: BlockHeight },
/// The block height field of the proposed new chain tip is not equal to the height of the
/// previous chain tip + 1. This variant stores a copy of the incorrect height value for
/// reporting purposes.
/// The block height field of the proposed new block is not equal to the height of the previous
/// block + 1.
BlockHeightDiscontinuity {
previous_tip: BlockHeight,
prev_height: BlockHeight,
new_height: BlockHeight,
},
/// The size of the Sapling note commitment tree was not provided as part of a [`CompactBlock`]
/// being scanned, making it impossible to construct the nullifier for a detected note.
SaplingTreeSizeUnknown { at_height: BlockHeight },
/// The note commitment tree size for the given protocol at the proposed new block is not equal
/// to the size at the previous block plus the count of this block's outputs.
TreeSizeMismatch {
protocol: ShieldedProtocol,
at_height: BlockHeight,
given: u32,
computed: u32,
},
/// The size of the note commitment tree for the given protocol was not provided as part of a
/// [`CompactBlock`] being scanned, making it impossible to construct the nullifier for a
/// detected note.
TreeSizeUnknown {
protocol: ShieldedProtocol,
at_height: BlockHeight,
},
}
impl fmt::Display for ScanError {
@ -137,11 +149,14 @@ impl fmt::Display for ScanError {
"The parent hash of proposed block does not correspond to the block hash at height {}.",
at_height
),
ScanError::BlockHeightDiscontinuity { previous_tip, new_height } => {
write!(f, "Block height discontinuity at height {}; next height is : {}", previous_tip, new_height)
ScanError::BlockHeightDiscontinuity { prev_height, new_height } => {
write!(f, "Block height discontinuity at height {}; next height is : {}", prev_height, new_height)
}
ScanError::SaplingTreeSizeUnknown { at_height } => {
write!(f, "Unable to determine Sapling note commitment tree size at height {}", at_height)
ScanError::TreeSizeMismatch { protocol, at_height, given, computed } => {
write!(f, "The the {:?} note commitment tree size provided by a compact block did not match the expected size at height {}; given {}, expected {}", protocol, at_height, given, computed)
}
ScanError::TreeSizeUnknown { protocol, at_height } => {
write!(f, "Unable to determine {:?} note commitment tree size at height {}", protocol, at_height)
}
}
}
@ -224,6 +239,46 @@ pub(crate) fn add_block_to_runner<P, S, T>(
}
}
pub(crate) fn check_continuity(
block: &CompactBlock,
prior_block_metadata: Option<&BlockMetadata>,
) -> Option<ScanError> {
if let Some(prev) = prior_block_metadata {
if block.height() != prev.block_height() + 1 {
return Some(ScanError::BlockHeightDiscontinuity {
prev_height: prev.block_height(),
new_height: block.height(),
});
}
if block.prev_hash() != prev.block_hash() {
return Some(ScanError::PrevHashMismatch {
at_height: block.height(),
});
}
if let Some(given) = block
.chain_metadata
.as_ref()
.map(|m| m.sapling_commitment_tree_size)
{
let computed = prev.sapling_tree_size()
+ u32::try_from(block.vtx.iter().map(|tx| tx.outputs.len()).sum::<usize>())
.unwrap();
if given != computed {
return Some(ScanError::TreeSizeMismatch {
protocol: ShieldedProtocol::Sapling,
at_height: block.height(),
given,
computed,
});
}
}
}
None
}
#[tracing::instrument(skip_all, fields(height = block.height))]
pub(crate) fn scan_block_with_runner<
P: consensus::Parameters + Send + 'static,
@ -237,26 +292,13 @@ pub(crate) fn scan_block_with_runner<
prior_block_metadata: Option<&BlockMetadata>,
mut batch_runner: Option<&mut TaggedBatchRunner<P, K::Scope, T>>,
) -> Result<ScannedBlock<K::Nf>, ScanError> {
let mut wtxs: Vec<WalletTx<K::Nf>> = vec![];
let mut sapling_note_commitments: Vec<(sapling::Node, Retention<BlockHeight>)> = vec![];
if let Some(scan_error) = check_continuity(&block, prior_block_metadata) {
return Err(scan_error);
}
let cur_height = block.height();
let cur_hash = block.hash();
if let Some(prev) = prior_block_metadata {
if cur_height != prev.block_height() + 1 {
return Err(ScanError::BlockHeightDiscontinuity {
previous_tip: prev.block_height(),
new_height: cur_height,
});
}
if block.prev_hash() != prev.block_hash() {
return Err(ScanError::PrevHashMismatch {
at_height: cur_height,
});
}
}
// It's possible to make progress without a Sapling tree position if we don't have any Sapling
// notes in the block, since we only use the position for constructing nullifiers for our own
// received notes. Thus, we allow it to be optional here, and only produce an error if we try
@ -281,11 +323,14 @@ pub(crate) fn scan_block_with_runner<
}
})
.or_else(|| prior_block_metadata.map(|m| m.sapling_tree_size()))
.ok_or(ScanError::SaplingTreeSizeUnknown {
.ok_or(ScanError::TreeSizeUnknown {
protocol: ShieldedProtocol::Sapling,
at_height: cur_height,
})?;
let compact_block_tx_count = block.vtx.len();
let mut wtxs: Vec<WalletTx<K::Nf>> = vec![];
let mut sapling_note_commitments: Vec<(sapling::Node, Retention<BlockHeight>)> = vec![];
for (tx_idx, tx) in block.vtx.into_iter().enumerate() {
let txid = tx.txid();

123
zcash_client_sqlite/src/chain.rs
View File

@ -330,7 +330,14 @@ mod tests {
insert_into_cache(&db_cache, &cb);
// Scan the cache
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
1,
)
.unwrap();
// Create a second fake CompactBlock sending more value to the address
let (cb2, _) = fake_compact_block(
@ -344,7 +351,14 @@ mod tests {
insert_into_cache(&db_cache, &cb2);
// Scan the cache again
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height() + 1,
1,
)
.unwrap();
}
#[test]
@ -381,7 +395,14 @@ mod tests {
insert_into_cache(&db_cache, &cb2);
// Scan the cache
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
2,
)
.unwrap();
// Create more fake CompactBlocks that don't connect to the scanned ones
let (cb3, _) = fake_compact_block(
@ -405,7 +426,13 @@ mod tests {
// Data+cache chain should be invalid at the data/cache boundary
assert_matches!(
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None),
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height() + 2,
2
),
Err(_) // FIXME: check error result more closely
);
}
@ -444,7 +471,14 @@ mod tests {
insert_into_cache(&db_cache, &cb2);
// Scan the cache
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
2,
)
.unwrap();
// Create more fake CompactBlocks that contain a reorg
let (cb3, _) = fake_compact_block(
@ -468,7 +502,13 @@ mod tests {
// Data+cache chain should be invalid inside the cache
assert_matches!(
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None),
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height() + 2,
2
),
Err(_) // FIXME: check error result more closely
);
}
@ -516,7 +556,14 @@ mod tests {
insert_into_cache(&db_cache, &cb2);
// Scan the cache
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
2,
)
.unwrap();
// Account balance should reflect both received notes
assert_eq!(
@ -551,7 +598,14 @@ mod tests {
);
// Scan the cache again
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
2,
)
.unwrap();
// Account balance should again reflect both received notes
assert_eq!(
@ -586,7 +640,14 @@ mod tests {
0,
);
insert_into_cache(&db_cache, &cb1);
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
1,
)
.unwrap();
assert_eq!(
get_balance(&db_data.conn, AccountId::from(0)).unwrap(),
value
@ -617,8 +678,8 @@ mod tests {
&tests::network(),
&db_cache,
&mut db_data,
Some(sapling_activation_height() + 2),
None
sapling_activation_height() + 2,
1
),
Ok(_)
);
@ -629,8 +690,8 @@ mod tests {
&tests::network(),
&db_cache,
&mut db_data,
Some(sapling_activation_height() + 1),
Some(1),
sapling_activation_height() + 1,
1,
)
.unwrap();
assert_eq!(
@ -699,7 +760,14 @@ mod tests {
insert_into_cache(&db_cache, &cb);
// Scan the cache
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
1,
)
.unwrap();
// Account balance should reflect the received note
assert_eq!(
@ -720,7 +788,14 @@ mod tests {
insert_into_cache(&db_cache, &cb2);
// Scan the cache again
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height() + 1,
1,
)
.unwrap();
// Account balance should reflect both received notes
assert_eq!(
@ -761,7 +836,14 @@ mod tests {
insert_into_cache(&db_cache, &cb);
// Scan the cache
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
1,
)
.unwrap();
// Account balance should reflect the received note
assert_eq!(
@ -787,7 +869,14 @@ mod tests {
);
// Scan the cache again
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height() + 1,
1,
)
.unwrap();
// Account balance should equal the change
assert_eq!(

6
zcash_client_sqlite/src/wallet.rs
View File

@ -65,11 +65,9 @@
//! - `memo` the shielded memo associated with the output, if any.
use rusqlite::{self, named_params, OptionalExtension, ToSql};
use std::collections::HashMap;
use std::convert::TryFrom;
use std::{
collections::HashMap,
io::{self, Cursor},
};
use std::io::{self, Cursor};
use zcash_client_backend::data_api::ShieldedProtocol;
use zcash_primitives::{

119
zcash_client_sqlite/src/wallet/sapling.rs
View File

@ -571,7 +571,14 @@ pub(crate) mod tests {
0,
);
insert_into_cache(&db_cache, &cb);
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
1,
)
.unwrap();
// Verified balance matches total balance
let (_, anchor_height) = db_data
@ -598,7 +605,14 @@ pub(crate) mod tests {
)
.0;
insert_into_cache(&db_cache, &cb);
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height() + 1,
1,
)
.unwrap();
// Verified balance does not include the second note
let (_, anchor_height2) = db_data
@ -651,7 +665,14 @@ pub(crate) mod tests {
.0;
insert_into_cache(&db_cache, &cb);
}
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height() + 2,
8,
)
.unwrap();
// Second spend still fails
assert_matches!(
@ -681,11 +702,18 @@ pub(crate) mod tests {
&dfvk,
AddressType::DefaultExternal,
value,
11,
10,
)
.0;
insert_into_cache(&db_cache, &cb);
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height() + 10,
1,
)
.unwrap();
// Second spend should now succeed
assert_matches!(
@ -730,7 +758,14 @@ pub(crate) mod tests {
0,
);
insert_into_cache(&db_cache, &cb);
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
1,
)
.unwrap();
assert_eq!(
get_balance(&db_data.conn, AccountId::from(0)).unwrap(),
value
@ -788,7 +823,14 @@ pub(crate) mod tests {
.0;
insert_into_cache(&db_cache, &cb);
}
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height() + 1,
41,
)
.unwrap();
// Second spend still fails
assert_matches!(
@ -821,7 +863,14 @@ pub(crate) mod tests {
)
.0;
insert_into_cache(&db_cache, &cb);
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height() + 42,
1,
)
.unwrap();
// Second spend should now succeed
create_spend_to_address(
@ -865,7 +914,14 @@ pub(crate) mod tests {
0,
);
insert_into_cache(&db_cache, &cb);
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
1,
)
.unwrap();
assert_eq!(
get_balance(&db_data.conn, AccountId::from(0)).unwrap(),
value
@ -938,7 +994,14 @@ pub(crate) mod tests {
.0;
insert_into_cache(&db_cache, &cb);
}
scan_cached_blocks(&network, &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&network,
&db_cache,
&mut db_data,
sapling_activation_height() + 1,
42,
)
.unwrap();
// Send the funds again, discarding history.
// Neither transaction output is decryptable by the sender.
@ -971,7 +1034,14 @@ pub(crate) mod tests {
0,
);
insert_into_cache(&db_cache, &cb);
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
1,
)
.unwrap();
// Verified balance matches total balance
let (_, anchor_height) = db_data
@ -1030,7 +1100,14 @@ pub(crate) mod tests {
0,
);
insert_into_cache(&db_cache, &cb);
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
1,
)
.unwrap();
// Verified balance matches total balance
let (_, anchor_height) = db_data
@ -1103,7 +1180,14 @@ pub(crate) mod tests {
insert_into_cache(&db_cache, &cb);
}
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
11,
)
.unwrap();
// Verified balance matches total balance
let total = Amount::from_u64(60000).unwrap();
@ -1225,7 +1309,14 @@ pub(crate) mod tests {
0,
);
insert_into_cache(&db_cache, &cb);
scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
scan_cached_blocks(
&tests::network(),
&db_cache,
&mut db_data,
sapling_activation_height(),
1,
)
.unwrap();
assert_matches!(
shield_transparent_funds(