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

Address comments from code review.

This commit is contained in:
Kris Nuttycombe 2023-07-03 17:06:43 -06:00
parent c363e71fa9
commit c13c8c6678
13 changed files with 78 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
zcash_client_backend/CHANGELOG.md
View File

@ -46,7 +46,7 @@ and this library adheres to Rust's notion of
respective `min_confirmations` arguments as `NonZeroU32`
- `data_api::wallet::input_selection::InputSelector::{propose_transaction, propose_shielding}`
now take their respective `min_confirmations` arguments as `NonZeroU32`
- A new `Sync` variant has been added to `data_api::chain::error::Error`.
- A new `Scan` variant has been added to `data_api::chain::error::Error`.
- A new `SyncRequired` variant has been added to `data_api::wallet::input_selection::InputSelectorError`.
- `zcash_client_backend::wallet`:
- `SpendableNote` has been renamed to `ReceivedSaplingNote`.
@ -61,8 +61,7 @@ and this library adheres to Rust's notion of
- Arguments to `zcash_client_backend::scanning::scan_block` have changed. This
method now takes an optional `BlockMetadata` argument instead of a base commitment
tree and incremental witnesses for each previously-known note. In addition, the
return type has now been updated to return a `Result<ScannedBlock, ScanError>`
in the case of scan failure.
return type has now been updated to return a `Result<ScannedBlock, ScanError>`.
### Removed

4
zcash_client_backend/build.rs
View File

@ -45,6 +45,10 @@ fn build() -> io::Result<()> {
// Build the gRPC types and client.
tonic_build::configure()
.build_server(false)
.extern_path(
".cash.z.wallet.sdk.rpc.ChainMetadata",
"crate::proto::compact_formats::ChainMetadata",
)
.extern_path(
".cash.z.wallet.sdk.rpc.CompactBlock",
"crate::proto::compact_formats::CompactBlock",

4
zcash_client_backend/proto/compact_formats.proto
View File

@ -10,9 +10,7 @@ option swift_prefix = "";
// Remember that proto3 fields are all optional. A field that is not present will be set to its zero value.
// bytes fields of hashes are in canonical little-endian format.
// BlockMetadata represents information about a block that may not be
// represented directly in the block data, but is instead derived from chain
// data or other external sources.
// ChainMetadata represents information about the state of the chain as of a given block.
message ChainMetadata {
uint32 saplingCommitmentTreeSize = 1; // the size of the Sapling note commitment tree as of the end of this block
uint32 orchardCommitmentTreeSize = 2; // the size of the Orchard note commitment tree as of the end of this block

2
zcash_client_backend/src/data_api.rs
View File

@ -340,7 +340,7 @@ impl<Nf> ScannedBlock<Nf> {
&self.sapling_commitments
}
pub fn take_sapling_commitments(self) -> Vec<(sapling::Node, Retention<BlockHeight>)> {
pub fn into_sapling_commitments(self) -> Vec<(sapling::Node, Retention<BlockHeight>)> {
self.sapling_commitments
}
}

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

@ -92,10 +92,11 @@ pub trait BlockSource {
/// Scans at most `limit` new blocks added to the block source for any transactions received by the
/// tracked accounts.
///
/// If the `from_height` argument is not `None`, then the block source will begin requesting blocks
/// from the provided block source at the specified height; if `from_height` is `None then this
/// will begin scanning at first block after the position to which the wallet has previously
/// fully scanned the chain, thereby beginning or continuing a linear scan over all blocks.
/// If the `from_height` argument is not `None`, then this method block source will begin
/// requesting blocks from the provided block source at the specified height; if `from_height` is
/// `None then this will begin scanning at first block after the position to which the wallet has
/// previously fully scanned the chain, thereby beginning or continuing a linear scan over all
/// blocks.
///
/// This function will return without error after scanning at most `limit` new blocks, to enable
/// the caller to update their UI with scanning progress. Repeatedly calling this function with

4
zcash_client_backend/src/proto/compact_formats.rs generated
View File

@ -1,6 +1,4 @@
/// BlockMetadata represents information about a block that may not be
/// represented directly in the block data, but is instead derived from chain
/// data or other external sources.
/// ChainMetadata represents information about the state of the chain as of a given block.
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct ChainMetadata {

13
zcash_client_backend/src/proto/service.rs generated
View File

@ -1,16 +1,3 @@
/// BlockMetadata represents information about a block that may not be
/// represented directly in the block data, but is instead derived from chain
/// data or other external sources.
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct ChainMetadata {
/// the size of the Sapling note commitment tree as of the end of this block
#[prost(uint32, tag = "1")]
pub sapling_commitment_tree_size: u32,
/// the size of the Orchard note commitment tree as of the end of this block
#[prost(uint32, tag = "2")]
pub orchard_commitment_tree_size: u32,
}
/// A BlockID message contains identifiers to select a block: a height or a
/// hash. Specification by hash is not implemented, but may be in the future.
#[allow(clippy::derive_partial_eq_without_eq)]

5
zcash_client_backend/src/scanning.rs
View File

@ -285,7 +285,7 @@ pub(crate) fn scan_block_with_runner<
at_height: cur_height,
})?;
let block_tx_count = block.vtx.len();
let compact_block_tx_count = block.vtx.len();
for (tx_idx, tx) in block.vtx.into_iter().enumerate() {
let txid = tx.txid();
@ -392,7 +392,8 @@ pub(crate) fn scan_block_with_runner<
{
// Collect block note commitments
let node = sapling::Node::from_cmu(&output.cmu);
let is_checkpoint = output_idx + 1 == decoded.len() && tx_idx + 1 == block_tx_count;
let is_checkpoint =
output_idx + 1 == decoded.len() && tx_idx + 1 == compact_block_tx_count;
let retention = match (dec_output.is_some(), is_checkpoint) {
(is_marked, true) => Retention::Checkpoint {
id: cur_height,

4
zcash_client_sqlite/src/error.rs
View File

@ -9,7 +9,7 @@ use shardtree::ShardTreeError;
use zcash_client_backend::encoding::{Bech32DecodeError, TransparentCodecError};
use zcash_primitives::{consensus::BlockHeight, zip32::AccountId};
use crate::PRUNING_HEIGHT;
use crate::PRUNING_DEPTH;
#[cfg(feature = "transparent-inputs")]
use zcash_primitives::legacy::TransparentAddress;
@ -108,7 +108,7 @@ impl fmt::Display for SqliteClientError {
SqliteClientError::InvalidNoteId =>
write!(f, "The note ID associated with an inserted witness must correspond to a received note."),
SqliteClientError::RequestedRewindInvalid(h, r) =>
write!(f, "A rewind must be either of less than {} blocks, or at least back to block {} for your wallet; the requested height was {}.", PRUNING_HEIGHT, h, r),
write!(f, "A rewind must be either of less than {} blocks, or at least back to block {} for your wallet; the requested height was {}.", PRUNING_DEPTH, h, r),
SqliteClientError::Bech32DecodeError(e) => write!(f, "{}", e),
#[cfg(feature = "transparent-inputs")]
SqliteClientError::HdwalletError(e) => write!(f, "{:?}", e),

8
zcash_client_sqlite/src/lib.rs
View File

@ -82,7 +82,7 @@ pub mod wallet;
/// The maximum number of blocks the wallet is allowed to rewind. This is
/// consistent with the bound in zcashd, and allows block data deeper than
/// this delta from the chain tip to be pruned.
pub(crate) const PRUNING_HEIGHT: u32 = 100;
pub(crate) const PRUNING_DEPTH: u32 = 100;
pub(crate) const SAPLING_TABLES_PREFIX: &str = "sapling";
@ -428,7 +428,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
let block_height = block.height();
let sapling_tree_size = block.metadata().sapling_tree_size();
let sapling_commitments_len = block.sapling_commitments().len();
let mut sapling_commitments = block.take_sapling_commitments().into_iter();
let mut sapling_commitments = block.into_sapling_commitments().into_iter();
wdb.with_sapling_tree_mut::<_, _, SqliteClientError>(move |sapling_tree| {
let start_position = Position::from(u64::from(sapling_tree_size))
- u64::try_from(sapling_commitments_len).unwrap();
@ -640,7 +640,7 @@ impl<P: consensus::Parameters> WalletCommitmentTrees for WalletDb<rusqlite::Conn
let shard_store = SqliteShardStore::from_connection(&tx, SAPLING_TABLES_PREFIX)
.map_err(|e| ShardTreeError::Storage(Either::Right(e)))?;
let result = {
let mut shardtree = ShardTree::new(shard_store, PRUNING_HEIGHT.try_into().unwrap());
let mut shardtree = ShardTree::new(shard_store, PRUNING_DEPTH.try_into().unwrap());
callback(&mut shardtree)?
};
tx.commit()
@ -668,7 +668,7 @@ impl<'conn, P: consensus::Parameters> WalletCommitmentTrees for WalletDb<SqlTran
let mut shardtree = ShardTree::new(
SqliteShardStore::from_connection(self.conn.0, SAPLING_TABLES_PREFIX)
.map_err(|e| ShardTreeError::Storage(Either::Right(e)))?,
PRUNING_HEIGHT.try_into().unwrap(),
PRUNING_DEPTH.try_into().unwrap(),
);
let result = callback(&mut shardtree)?;

105
zcash_client_sqlite/src/wallet.rs
View File

@ -90,7 +90,7 @@ use zcash_client_backend::{
};
use crate::{
error::SqliteClientError, SqlTransaction, WalletCommitmentTrees, WalletDb, PRUNING_HEIGHT,
error::SqliteClientError, SqlTransaction, WalletCommitmentTrees, WalletDb, PRUNING_DEPTH,
};
#[cfg(feature = "transparent-inputs")]
@ -543,11 +543,11 @@ pub(crate) fn block_height_extrema(
fn parse_block_metadata(
row: (BlockHeight, Vec<u8>, Option<u32>, Vec<u8>),
) -> Option<Result<BlockMetadata, SqliteClientError>> {
) -> Result<BlockMetadata, SqliteClientError> {
let (block_height, hash_data, sapling_tree_size_opt, sapling_tree) = row;
let sapling_tree_size = sapling_tree_size_opt.map(Ok).or_else(|| {
let sapling_tree_size = sapling_tree_size_opt.map_or_else(|| {
if sapling_tree == BLOCK_SAPLING_FRONTIER_ABSENT {
None
Err(SqliteClientError::CorruptedData("One of either the Sapling tree size or the legacy Sapling commitment tree must be present.".to_owned()))
} else {
// parse the legacy commitment tree data
read_commitment_tree::<
@ -555,52 +555,50 @@ fn parse_block_metadata(
_,
{ zcash_primitives::sapling::NOTE_COMMITMENT_TREE_DEPTH },
>(Cursor::new(sapling_tree))
.map(|tree| Some(tree.size().try_into().unwrap()))
.map(|tree| tree.size().try_into().unwrap())
.map_err(SqliteClientError::from)
.transpose()
}
})?;
}, Ok)?;
let block_hash = BlockHash::try_from_slice(&hash_data).ok_or_else(|| {
SqliteClientError::from(io::Error::new(
io::ErrorKind::InvalidData,
format!("Invalid block hash length: {}", hash_data.len()),
))
});
})?;
Some(sapling_tree_size.and_then(|sapling_tree_size| {
block_hash.map(|block_hash| {
BlockMetadata::from_parts(block_height, block_hash, sapling_tree_size)
})
}))
Ok(BlockMetadata::from_parts(
block_height,
block_hash,
sapling_tree_size,
))
}
pub(crate) fn block_metadata(
conn: &rusqlite::Connection,
block_height: BlockHeight,
) -> Result<Option<BlockMetadata>, SqliteClientError> {
let res_opt = conn
.query_row(
"SELECT height, hash, sapling_commitment_tree_size, sapling_tree
conn.query_row(
"SELECT height, hash, sapling_commitment_tree_size, sapling_tree
FROM blocks
WHERE height = :block_height",
named_params![":block_height": u32::from(block_height)],
|row| {
let height: u32 = row.get(0)?;
let block_hash: Vec<u8> = row.get(1)?;
let sapling_tree_size: Option<u32> = row.get(2)?;
let sapling_tree: Vec<u8> = row.get(3)?;
Ok((
BlockHeight::from(height),
block_hash,
sapling_tree_size,
sapling_tree,
))
},
)
.optional()?;
res_opt.and_then(parse_block_metadata).transpose()
named_params![":block_height": u32::from(block_height)],
|row| {
let height: u32 = row.get(0)?;
let block_hash: Vec<u8> = row.get(1)?;
let sapling_tree_size: Option<u32> = row.get(2)?;
let sapling_tree: Vec<u8> = row.get(3)?;
Ok((
BlockHeight::from(height),
block_hash,
sapling_tree_size,
sapling_tree,
))
},
)
.optional()
.map_err(SqliteClientError::from)
.and_then(|meta_row| meta_row.map(parse_block_metadata).transpose())
}
pub(crate) fn block_fully_scanned(
@ -608,29 +606,28 @@ pub(crate) fn block_fully_scanned(
) -> Result<Option<BlockMetadata>, SqliteClientError> {
// FIXME: this will need to be rewritten once out-of-order scan range suggestion
// is implemented.
let res_opt = conn
.query_row(
"SELECT height, hash, sapling_commitment_tree_size, sapling_tree
conn.query_row(
"SELECT height, hash, sapling_commitment_tree_size, sapling_tree
FROM blocks
ORDER BY height DESC
LIMIT 1",
[],
|row| {
let height: u32 = row.get(0)?;
let block_hash: Vec<u8> = row.get(1)?;
let sapling_tree_size: Option<u32> = row.get(2)?;
let sapling_tree: Vec<u8> = row.get(3)?;
Ok((
BlockHeight::from(height),
block_hash,
sapling_tree_size,
sapling_tree,
))
},
)
.optional()?;
res_opt.and_then(parse_block_metadata).transpose()
[],
|row| {
let height: u32 = row.get(0)?;
let block_hash: Vec<u8> = row.get(1)?;
let sapling_tree_size: Option<u32> = row.get(2)?;
let sapling_tree: Vec<u8> = row.get(3)?;
Ok((
BlockHeight::from(height),
block_hash,
sapling_tree_size,
sapling_tree,
))
},
)
.optional()
.map_err(SqliteClientError::from)
.and_then(|meta_row| meta_row.map(parse_block_metadata).transpose())
}
/// Returns the block height at which the specified transaction was mined,
@ -704,7 +701,7 @@ pub(crate) fn truncate_to_height<P: consensus::Parameters>(
.map(|opt| opt.map_or_else(|| sapling_activation_height - 1, BlockHeight::from))
})?;
if block_height < last_scanned_height - PRUNING_HEIGHT {
if block_height < last_scanned_height - PRUNING_DEPTH {
if let Some(h) = get_min_unspent_height(conn)? {
if block_height > h {
return Err(SqliteClientError::RequestedRewindInvalid(h, block_height));

4
zcash_client_sqlite/src/wallet/init.rs
View File

@ -21,7 +21,7 @@ use zcash_primitives::{
use zcash_client_backend::{data_api::SAPLING_SHARD_HEIGHT, keys::UnifiedFullViewingKey};
use crate::{error::SqliteClientError, wallet, WalletDb, SAPLING_TABLES_PREFIX};
use crate::{error::SqliteClientError, wallet, WalletDb, PRUNING_DEPTH, SAPLING_TABLES_PREFIX};
use super::commitment_tree::SqliteShardStore;
@ -327,7 +327,7 @@ pub fn init_blocks_table<P: consensus::Parameters>(
_,
{ sapling::NOTE_COMMITMENT_TREE_DEPTH },
SAPLING_SHARD_HEIGHT,
> = ShardTree::new(shard_store, 100);
> = ShardTree::new(shard_store, PRUNING_DEPTH.try_into().unwrap());
shard_tree.insert_frontier_nodes(
nonempty_frontier.clone(),
Retention::Checkpoint {

4
zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs
View File

@ -23,7 +23,7 @@ use crate::{
commitment_tree::SqliteShardStore,
init::{migrations::received_notes_nullable_nf, WalletMigrationError},
},
SAPLING_TABLES_PREFIX,
PRUNING_DEPTH, SAPLING_TABLES_PREFIX,
};
pub(super) const MIGRATION_ID: Uuid = Uuid::from_fields(
@ -103,7 +103,7 @@ impl RusqliteMigration for Migration {
_,
{ sapling::NOTE_COMMITMENT_TREE_DEPTH },
SAPLING_SHARD_HEIGHT,
> = ShardTree::new(shard_store, 100);
> = ShardTree::new(shard_store, PRUNING_DEPTH.try_into().unwrap());
// Insert all the tree information that we can get from block-end commitment trees
{
let mut stmt_blocks = transaction.prepare("SELECT height, sapling_tree FROM blocks")?;