diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 35497ef65..3f36d51a6 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -9,8 +9,11 @@ and this library adheres to Rust's notion of ### Added - `zcash_client_backend::data_api`: + - `BlockMetadata::orchard_tree_size`. - `TransparentInputSource` - `SaplingInputSource` + - `ScannedBlock::sapling_tree_size`. + - `ScannedBlock::orchard_tree_size`. - `wallet::propose_standard_transfer_to_address` - `wallet::input_selection::SaplingInputs` - `wallet::input_selection::ShieldingSelector` has been @@ -23,6 +26,12 @@ and this library adheres to Rust's notion of ### Changed - `zcash_client_backend::data_api`: + - Arguments to `BlockMetadata::from_parts` have changed to include Orchard. + - `BlockMetadata::sapling_tree_size` now returns an `Option` instead of + a `u32` for consistency with Orchard. + - Arguments to `ScannedBlock::from_parts` have changed. + - `ScannedBlock::metadata` has been renamed to `to_block_metadata` and now + returns an owned value rather than a reference. - `ShieldedProtocol` has a new variant for `Orchard`, allowing for better reporting to callers trying to perform actions using `Orchard` before it is fully supported. @@ -69,7 +78,7 @@ and this library adheres to Rust's notion of `min_confirmations` argument as `u32` instead of `NonZeroU32`. - The `wallet::input_selection::InputSelector::DataSource` associated type has been renamed to `InputSource`. - - The signature of `wallet:input_selection::InputSelector::propose_transaction` + - The signature of `wallet:input_selection::InputSelector::propose_transaction` has been altered such that it longer takes `min_confirmations` as an argument, instead taking explicit `target_height` and `anchor_height` arguments. This helps to minimize the set of capabilities that the @@ -101,6 +110,7 @@ and this library adheres to Rust's notion of - The fields of `ReceivedSaplingNote` are now private. Use `ReceivedSaplingNote::from_parts` for construction instead. Accessor methods are provided for each previously public field. +- `zcash_client_backend::scanning::ScanError` has a new variant, `TreeSizeInvalid`. - The following fields now have type `NonNegativeAmount` instead of `Amount`: - `zcash_client_backend::data_api`: - `error::Error::InsufficientFunds.{available, required}` diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index c623de26d..b8751b2a7 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -409,8 +409,8 @@ pub trait WalletRead { pub struct BlockMetadata { block_height: BlockHeight, block_hash: BlockHash, - sapling_tree_size: u32, - //TODO: orchard_tree_size: u32 + sapling_tree_size: Option, + orchard_tree_size: Option, } impl BlockMetadata { @@ -418,12 +418,14 @@ impl BlockMetadata { pub fn from_parts( block_height: BlockHeight, block_hash: BlockHash, - sapling_tree_size: u32, + sapling_tree_size: Option, + orchard_tree_size: Option, ) -> Self { Self { block_height, block_hash, sapling_tree_size, + orchard_tree_size, } } @@ -437,11 +439,17 @@ impl BlockMetadata { self.block_hash } - /// Returns the size of the Sapling note commitment tree as of the block that this - /// [`BlockMetadata`] describes. - pub fn sapling_tree_size(&self) -> u32 { + /// Returns the size of the Sapling note commitment tree for the final treestate of the block + /// that this [`BlockMetadata`] describes, if available. + pub fn sapling_tree_size(&self) -> Option { self.sapling_tree_size } + + /// Returns the size of the Orchard note commitment tree for the final treestate of the block + /// that this [`BlockMetadata`] describes, if available. + pub fn orchard_tree_size(&self) -> Option { + self.orchard_tree_size + } } /// The subset of information that is relevant to this wallet that has been @@ -449,8 +457,11 @@ impl BlockMetadata { /// /// [`CompactBlock`]: crate::proto::compact_formats::CompactBlock pub struct ScannedBlock { - metadata: BlockMetadata, + block_height: BlockHeight, + block_hash: BlockHash, block_time: u32, + sapling_tree_size: u32, + orchard_tree_size: u32, transactions: Vec>, sapling_nullifier_map: Vec<(TxId, u16, Vec)>, sapling_commitments: Vec<(sapling::Node, Retention)>, @@ -458,16 +469,23 @@ pub struct ScannedBlock { impl ScannedBlock { /// Constructs a new `ScannedBlock` + #[allow(clippy::too_many_arguments)] pub fn from_parts( - metadata: BlockMetadata, + block_height: BlockHeight, + block_hash: BlockHash, block_time: u32, + sapling_tree_size: u32, + orchard_tree_size: u32, transactions: Vec>, sapling_nullifier_map: Vec<(TxId, u16, Vec)>, sapling_commitments: Vec<(sapling::Node, Retention)>, ) -> Self { Self { - metadata, + block_height, + block_hash, block_time, + sapling_tree_size, + orchard_tree_size, transactions, sapling_nullifier_map, sapling_commitments, @@ -476,12 +494,12 @@ impl ScannedBlock { /// Returns the height of the block that was scanned. pub fn height(&self) -> BlockHeight { - self.metadata.block_height + self.block_height } /// Returns the block hash of the block that was scanned. pub fn block_hash(&self) -> BlockHash { - self.metadata.block_hash + self.block_hash } /// Returns the block time of the block that was scanned, as a Unix timestamp in seconds. @@ -489,13 +507,14 @@ impl ScannedBlock { self.block_time } - /// Returns the metadata describing the state of the note commitment trees as of the end of the - /// scanned block. - /// - /// The metadata returned from this method is guaranteed to be consistent with what is returned - /// by [`Self::height`] and [`Self::block_hash`]. - pub fn metadata(&self) -> &BlockMetadata { - &self.metadata + /// Returns the size of the Sapling note commitment tree as of the end of the scanned block. + pub fn sapling_tree_size(&self) -> u32 { + self.sapling_tree_size + } + + /// Returns the size of the Orchard note commitment tree as of the end of the scanned block. + pub fn orchard_tree_size(&self) -> u32 { + self.orchard_tree_size } /// Returns the list of transactions from the block that are relevant to the wallet. @@ -524,6 +543,16 @@ impl ScannedBlock { pub fn into_sapling_commitments(self) -> Vec<(sapling::Node, Retention)> { self.sapling_commitments } + + /// Returns the [`BlockMetadata`] corresponding to the scanned block. + pub fn to_block_metadata(&self) -> BlockMetadata { + BlockMetadata { + block_height: self.block_height, + block_hash: self.block_hash, + sapling_tree_size: Some(self.sapling_tree_size), + orchard_tree_size: Some(self.orchard_tree_size), + } + } } /// A transaction that was detected during scanning of the blockchain, diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index e4353fc26..18a5cad74 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -147,17 +147,16 @@ use std::ops::Range; use zcash_primitives::{ - block::BlockHash, consensus::{self, BlockHeight}, sapling::{self, note_encryption::PreparedIncomingViewingKey}, zip32::Scope, }; use crate::{ - data_api::{BlockMetadata, NullifierQuery, WalletWrite}, + data_api::{NullifierQuery, WalletWrite}, proto::compact_formats::CompactBlock, scan::BatchRunner, - scanning::{add_block_to_runner, check_continuity, scan_block_with_runner, ScanningKey}, + scanning::{add_block_to_runner, scan_block_with_runner, ScanningKey}, }; pub mod error; @@ -326,46 +325,10 @@ where None }; - 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)); - } - - if block.height() == BlockHeight::from(0) { - // We can always derive a valid `continuity_check_metadata` for the - // genesis block, even if the block source doesn't have - // `sapling_commitment_tree_size`. So briefly set it to a dummy value that - // ensures the `map` below produces the correct genesis block value. - assert!(continuity_check_metadata.is_none()); - continuity_check_metadata = Some(BlockMetadata::from_parts( - BlockHeight::from(0), - BlockHash([0; 32]), - 0, - )); - } - 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::(), - ) - .unwrap() - }), - ) - }); - add_block_to_runner(params, block, &mut batch_runner); Ok(()) @@ -415,7 +378,7 @@ where .map(|out| (out.account(), *out.nf())) })); - prior_block_metadata = Some(*scanned_block.metadata()); + prior_block_metadata = Some(scanned_block.to_block_metadata()); scanned_blocks.push(scanned_block); Ok(()) diff --git a/zcash_client_backend/src/scanning.rs b/zcash_client_backend/src/scanning.rs index 88597da91..4203faa92 100644 --- a/zcash_client_backend/src/scanning.rs +++ b/zcash_client_backend/src/scanning.rs @@ -7,7 +7,7 @@ use std::fmt::{self, Debug}; use incrementalmerkletree::{Position, Retention}; use subtle::{ConditionallySelectable, ConstantTimeEq, CtOption}; use zcash_note_encryption::batch; -use zcash_primitives::consensus::BlockHeight; +use zcash_primitives::consensus::{BlockHeight, NetworkUpgrade}; use zcash_primitives::{ consensus, sapling::{ @@ -167,6 +167,14 @@ pub enum ScanError { protocol: ShieldedProtocol, at_height: BlockHeight, }, + + /// We were provided chain metadata for a block containing note commitment tree metadata + /// that is invalidated by the data in the block itself. This may be caused by the presence + /// of default values in the chain metadata. + TreeSizeInvalid { + protocol: ShieldedProtocol, + at_height: BlockHeight, + }, } impl ScanError { @@ -178,6 +186,7 @@ impl ScanError { BlockHeightDiscontinuity { .. } => true, TreeSizeMismatch { .. } => true, TreeSizeUnknown { .. } => false, + TreeSizeInvalid { .. } => false, } } @@ -189,6 +198,7 @@ impl ScanError { BlockHeightDiscontinuity { new_height, .. } => *new_height, TreeSizeMismatch { at_height, .. } => *at_height, TreeSizeUnknown { at_height, .. } => *at_height, + TreeSizeInvalid { at_height, .. } => *at_height, } } } @@ -211,6 +221,9 @@ impl fmt::Display for ScanError { TreeSizeUnknown { protocol, at_height } => { write!(f, "Unable to determine {:?} note commitment tree size at height {}", protocol, at_height) } + TreeSizeInvalid { protocol, at_height } => { + write!(f, "Received invalid (potentially default) {:?} note commitment tree size metadata at height {}", protocol, at_height) + } } } } @@ -292,7 +305,7 @@ pub(crate) fn add_block_to_runner( } } -pub(crate) fn check_continuity( +fn check_hash_continuity( block: &CompactBlock, prior_block_metadata: Option<&BlockMetadata>, ) -> Option { @@ -309,24 +322,6 @@ pub(crate) fn check_continuity( 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::()) - .unwrap(); - if given != computed { - return Some(ScanError::TreeSizeMismatch { - protocol: ShieldedProtocol::Sapling, - at_height: block.height(), - given, - computed, - }); - } - } } None @@ -345,41 +340,102 @@ pub(crate) fn scan_block_with_runner< prior_block_metadata: Option<&BlockMetadata>, mut batch_runner: Option<&mut TaggedBatchRunner>, ) -> Result, ScanError> { - if let Some(scan_error) = check_continuity(&block, prior_block_metadata) { + if let Some(scan_error) = check_hash_continuity(&block, prior_block_metadata) { return Err(scan_error); } let cur_height = block.height(); let cur_hash = block.hash(); - // 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 - // to use it. `block.sapling_commitment_tree_size` is expected to be correct as of the end of - // the block, and we can't have a note of ours in a block with no outputs so treating the zero - // default value from the protobuf as `None` is always correct. - let mut sapling_commitment_tree_size = block - .chain_metadata - .as_ref() - .and_then(|m| { - if m.sapling_commitment_tree_size == 0 { - None - } else { - let block_note_count: u32 = block - .vtx - .iter() - .map(|tx| { - u32::try_from(tx.outputs.len()).expect("output count cannot exceed a u32") - }) - .sum(); - Some(m.sapling_commitment_tree_size - block_note_count) - } - }) - .or_else(|| prior_block_metadata.map(|m| m.sapling_tree_size())) - .ok_or(ScanError::TreeSizeUnknown { - protocol: ShieldedProtocol::Sapling, - at_height: cur_height, - })?; + let initial_sapling_tree_size = prior_block_metadata.and_then(|m| m.sapling_tree_size()); + let mut sapling_commitment_tree_size = initial_sapling_tree_size.map_or_else( + || { + block.chain_metadata.as_ref().map_or_else( + || { + // If we're below Sapling activation, or Sapling activation is not set, the tree size is zero + params + .activation_height(NetworkUpgrade::Sapling) + .map_or_else( + || Ok(0), + |sapling_activation| { + if cur_height < sapling_activation { + Ok(0) + } else { + Err(ScanError::TreeSizeUnknown { + protocol: ShieldedProtocol::Sapling, + at_height: cur_height, + }) + } + }, + ) + }, + |m| { + let sapling_output_count: u32 = block + .vtx + .iter() + .map(|tx| tx.outputs.len()) + .sum::() + .try_into() + .expect("Sapling output count cannot exceed a u32"); + + // The default for m.sapling_commitment_tree_size is zero, so we need to check + // that the subtraction will not underflow; if it would do so, we were given + // invalid chain metadata for a block with Sapling outputs. + m.sapling_commitment_tree_size + .checked_sub(sapling_output_count) + .ok_or(ScanError::TreeSizeInvalid { + protocol: ShieldedProtocol::Sapling, + at_height: cur_height, + }) + }, + ) + }, + Ok, + )?; + + let initial_orchard_tree_size = prior_block_metadata.and_then(|m| m.orchard_tree_size()); + let mut orchard_commitment_tree_size = initial_orchard_tree_size.map_or_else( + || { + block.chain_metadata.as_ref().map_or_else( + || { + // If we're below Orchard activation, or Orchard activation is not set, the tree size is zero + params.activation_height(NetworkUpgrade::Nu5).map_or_else( + || Ok(0), + |orchard_activation| { + if cur_height < orchard_activation { + Ok(0) + } else { + Err(ScanError::TreeSizeUnknown { + protocol: ShieldedProtocol::Orchard, + at_height: cur_height, + }) + } + }, + ) + }, + |m| { + let orchard_action_count: u32 = block + .vtx + .iter() + .map(|tx| tx.actions.len()) + .sum::() + .try_into() + .expect("Orchard action count cannot exceed a u32"); + + // The default for m.orchard_commitment_tree_size is zero, so we need to check + // that the subtraction will not underflow; if it would do so, we were given + // invalid chain metadata for a block with Orchard actions. + m.orchard_commitment_tree_size + .checked_sub(orchard_action_count) + .ok_or(ScanError::TreeSizeInvalid { + protocol: ShieldedProtocol::Orchard, + at_height: cur_height, + }) + }, + ) + }, + Ok, + )?; let compact_block_tx_count = block.vtx.len(); let mut wtxs: Vec> = vec![]; @@ -428,9 +484,15 @@ pub(crate) fn scan_block_with_runner< .map(|spend| spend.account()) .collect(); + // We keep track of the number of outputs and actions here because tx.outputs + // and tx.actions end up being moved. + let tx_outputs_len = + u32::try_from(tx.outputs.len()).expect("Sapling output count cannot exceed a u32"); + let tx_actions_len = + u32::try_from(tx.actions.len()).expect("Orchard action count cannot exceed a u32"); + // Check for incoming notes while incrementing tree and witnesses let mut shielded_outputs: Vec> = vec![]; - let tx_outputs_len = u32::try_from(tx.outputs.len()).unwrap(); { let decoded = &tx .outputs @@ -548,11 +610,35 @@ pub(crate) fn scan_block_with_runner< } sapling_commitment_tree_size += tx_outputs_len; + orchard_commitment_tree_size += tx_actions_len; + } + + if let Some(chain_meta) = block.chain_metadata { + if chain_meta.sapling_commitment_tree_size != sapling_commitment_tree_size { + return Err(ScanError::TreeSizeMismatch { + protocol: ShieldedProtocol::Sapling, + at_height: cur_height, + given: chain_meta.sapling_commitment_tree_size, + computed: sapling_commitment_tree_size, + }); + } + + if chain_meta.orchard_commitment_tree_size != orchard_commitment_tree_size { + return Err(ScanError::TreeSizeMismatch { + protocol: ShieldedProtocol::Orchard, + at_height: cur_height, + given: chain_meta.orchard_commitment_tree_size, + computed: orchard_commitment_tree_size, + }); + } } Ok(ScannedBlock::from_parts( - BlockMetadata::from_parts(cur_height, cur_hash, sapling_commitment_tree_size), + cur_height, + cur_hash, block.time, + sapling_commitment_tree_size, + orchard_commitment_tree_size, wtxs, sapling_nullifier_map, sapling_note_commitments, @@ -630,7 +716,7 @@ mod tests { /// single spend of the given nullifier and a single output paying the given address. /// Returns the CompactBlock. /// - /// Set `initial_sapling_tree_size` to `None` to simulate a `CompactBlock` retrieved + /// Set `initial_tree_sizes` to `None` to simulate a `CompactBlock` retrieved /// from a `lightwalletd` that is not currently tracking note commitment tree sizes. fn fake_compact_block( height: BlockHeight, @@ -639,7 +725,7 @@ mod tests { dfvk: &DiversifiableFullViewingKey, value: NonNegativeAmount, tx_after: bool, - initial_sapling_tree_size: Option, + initial_tree_sizes: Option<(u32, u32)>, ) -> CompactBlock { let to = dfvk.default_address().1; @@ -700,14 +786,15 @@ mod tests { cb.vtx.push(tx); } - cb.chain_metadata = initial_sapling_tree_size.map(|s| compact::ChainMetadata { - sapling_commitment_tree_size: s + cb - .vtx - .iter() - .map(|tx| tx.outputs.len() as u32) - .sum::(), - ..Default::default() - }); + cb.chain_metadata = + initial_tree_sizes.map(|(initial_sapling_tree_size, initial_orchard_tree_size)| { + compact::ChainMetadata { + sapling_commitment_tree_size: initial_sapling_tree_size + + cb.vtx.iter().map(|tx| tx.outputs.len() as u32).sum::(), + orchard_commitment_tree_size: initial_orchard_tree_size + + cb.vtx.iter().map(|tx| tx.actions.len() as u32).sum::(), + } + }); cb } @@ -755,7 +842,8 @@ mod tests { Some(&BlockMetadata::from_parts( BlockHeight::from(0), BlockHash([0u8; 32]), - 0, + Some(0), + Some(0), )), batch_runner.as_mut(), ) @@ -775,7 +863,7 @@ mod tests { Position::from(1) ); - assert_eq!(scanned_block.metadata().sapling_tree_size(), 2); + assert_eq!(scanned_block.sapling_tree_size(), 2); assert_eq!( scanned_block .sapling_commitments() @@ -810,7 +898,7 @@ mod tests { &dfvk, NonNegativeAmount::const_from_u64(5), true, - Some(0), + Some((0, 0)), ); assert_eq!(cb.vtx.len(), 3); @@ -886,7 +974,7 @@ mod tests { &dfvk, NonNegativeAmount::const_from_u64(5), false, - Some(0), + Some((0, 0)), ); assert_eq!(cb.vtx.len(), 2); let vks: Vec<(&AccountId, &SaplingIvk)> = vec![]; diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index de48d6bb8..5ef5f4e3f 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -222,15 +222,15 @@ impl, P: consensus::Parameters> WalletRead for W } fn block_metadata(&self, height: BlockHeight) -> Result, Self::Error> { - wallet::block_metadata(self.conn.borrow(), height) + wallet::block_metadata(self.conn.borrow(), &self.params, height) } fn block_fully_scanned(&self) -> Result, Self::Error> { - wallet::block_fully_scanned(self.conn.borrow()) + wallet::block_fully_scanned(self.conn.borrow(), &self.params) } fn block_max_scanned(&self) -> Result, Self::Error> { - wallet::block_max_scanned(self.conn.borrow()) + wallet::block_max_scanned(self.conn.borrow(), &self.params) } fn suggest_scan_ranges(&self) -> Result, Self::Error> { @@ -295,7 +295,12 @@ impl, P: consensus::Parameters> WalletRead for W &self, min_confirmations: u32, ) -> Result, Self::Error> { - wallet::get_wallet_summary(self.conn.borrow(), min_confirmations, &SubtreeScanProgress) + wallet::get_wallet_summary( + self.conn.borrow(), + &self.params, + min_confirmations, + &SubtreeScanProgress, + ) } fn get_memo(&self, note_id: NoteId) -> Result, Self::Error> { @@ -429,7 +434,7 @@ impl WalletWrite for WalletDb ( block.height(), Position::from( - u64::from(block.metadata().sapling_tree_size()) + u64::from(block.sapling_tree_size()) - u64::try_from(block.sapling_commitments().len()).unwrap(), ), ) @@ -451,7 +456,7 @@ impl WalletWrite for WalletDb block.height(), block.block_hash(), block.block_time(), - block.metadata().sapling_tree_size(), + block.sapling_tree_size(), block.sapling_commitments().len().try_into().unwrap(), )?; diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index aa624c542..7642f019d 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -675,10 +675,14 @@ impl TestState { min_confirmations: u32, f: F, ) -> T { - let binding = - get_wallet_summary(&self.wallet().conn, min_confirmations, &SubtreeScanProgress) - .unwrap() - .unwrap(); + let binding = get_wallet_summary( + &self.wallet().conn, + &self.wallet().params, + min_confirmations, + &SubtreeScanProgress, + ) + .unwrap() + .unwrap(); f(binding.account_balances().get(&account).unwrap()) } @@ -721,7 +725,13 @@ impl TestState { } pub(crate) fn get_wallet_summary(&self, min_confirmations: u32) -> Option { - get_wallet_summary(&self.wallet().conn, min_confirmations, &SubtreeScanProgress).unwrap() + get_wallet_summary( + &self.wallet().conn, + &self.wallet().params, + min_confirmations, + &SubtreeScanProgress, + ) + .unwrap() } } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index eaa45485f..7e461ec7c 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -553,8 +553,9 @@ impl ScanProgress for SubtreeScanProgress { /// /// `min_confirmations` can be 0, but that case is currently treated identically to /// `min_confirmations == 1` for shielded notes. This behaviour may change in the future. -pub(crate) fn get_wallet_summary( +pub(crate) fn get_wallet_summary( conn: &rusqlite::Connection, + params: &P, min_confirmations: u32, progress: &impl ScanProgress, ) -> Result, SqliteClientError> { @@ -569,7 +570,7 @@ pub(crate) fn get_wallet_summary( wallet_birthday(conn)?.expect("If a scan range exists, we know the wallet birthday."); let fully_scanned_height = - block_fully_scanned(conn)?.map_or(birthday_height - 1, |m| m.block_height()); + block_fully_scanned(conn, params)?.map_or(birthday_height - 1, |m| m.block_height()); let summary_height = (chain_tip_height + 1).saturating_sub(std::cmp::max(min_confirmations, 1)); let sapling_scan_progress = progress.sapling_scan_progress( @@ -946,10 +947,11 @@ pub(crate) fn get_target_and_anchor_heights( } } -fn parse_block_metadata( - row: (BlockHeight, Vec, Option, Vec), +fn parse_block_metadata( + params: &P, + row: (BlockHeight, Vec, Option, Vec, Option), ) -> Result { - let (block_height, hash_data, sapling_tree_size_opt, sapling_tree) = row; + let (block_height, hash_data, sapling_tree_size_opt, sapling_tree, orchard_tree_size_opt) = row; let sapling_tree_size = sapling_tree_size_opt.map_or_else(|| { if sapling_tree == BLOCK_SAPLING_FRONTIER_ABSENT { Err(SqliteClientError::CorruptedData("One of either the Sapling tree size or the legacy Sapling commitment tree must be present.".to_owned())) @@ -975,16 +977,26 @@ fn parse_block_metadata( Ok(BlockMetadata::from_parts( block_height, block_hash, - sapling_tree_size, + Some(sapling_tree_size), + if params + .activation_height(NetworkUpgrade::Nu5) + .iter() + .any(|nu5_activation| &block_height >= nu5_activation) + { + orchard_tree_size_opt + } else { + Some(0) + }, )) } -pub(crate) fn block_metadata( +pub(crate) fn block_metadata( conn: &rusqlite::Connection, + params: &P, block_height: BlockHeight, ) -> Result, SqliteClientError> { conn.query_row( - "SELECT height, hash, sapling_commitment_tree_size, sapling_tree + "SELECT height, hash, sapling_commitment_tree_size, sapling_tree, orchard_commitment_tree_size FROM blocks WHERE height = :block_height", named_params![":block_height": u32::from(block_height)], @@ -993,21 +1005,24 @@ pub(crate) fn block_metadata( let block_hash: Vec = row.get(1)?; let sapling_tree_size: Option = row.get(2)?; let sapling_tree: Vec = row.get(3)?; + let orchard_tree_size: Option = row.get(4)?; Ok(( BlockHeight::from(height), block_hash, sapling_tree_size, sapling_tree, + orchard_tree_size, )) }, ) .optional() .map_err(SqliteClientError::from) - .and_then(|meta_row| meta_row.map(parse_block_metadata).transpose()) + .and_then(|meta_row| meta_row.map(|r| parse_block_metadata(params, r)).transpose()) } -pub(crate) fn block_fully_scanned( +pub(crate) fn block_fully_scanned( conn: &rusqlite::Connection, + params: &P, ) -> Result, SqliteClientError> { if let Some(birthday_height) = wallet_birthday(conn)? { // We assume that the only way we get a contiguous range of block heights in the `blocks` table @@ -1020,7 +1035,7 @@ pub(crate) fn block_fully_scanned( // block rows, which is a combined case of the "gaps and islands" and "greatest N per group" // SQL query problems. conn.query_row( - "SELECT height, hash, sapling_commitment_tree_size, sapling_tree + "SELECT height, hash, sapling_commitment_tree_size, sapling_tree, orchard_commitment_tree_size FROM blocks INNER JOIN ( WITH contiguous AS ( @@ -1039,27 +1054,30 @@ pub(crate) fn block_fully_scanned( let block_hash: Vec = row.get(1)?; let sapling_tree_size: Option = row.get(2)?; let sapling_tree: Vec = row.get(3)?; + let orchard_tree_size: Option = row.get(4)?; Ok(( BlockHeight::from(height), block_hash, sapling_tree_size, sapling_tree, + orchard_tree_size )) }, ) .optional() .map_err(SqliteClientError::from) - .and_then(|meta_row| meta_row.map(parse_block_metadata).transpose()) + .and_then(|meta_row| meta_row.map(|r| parse_block_metadata(params, r)).transpose()) } else { Ok(None) } } -pub(crate) fn block_max_scanned( +pub(crate) fn block_max_scanned( conn: &rusqlite::Connection, + params: &P, ) -> Result, SqliteClientError> { conn.query_row( - "SELECT blocks.height, hash, sapling_commitment_tree_size, sapling_tree + "SELECT blocks.height, hash, sapling_commitment_tree_size, sapling_tree, orchard_commitment_tree_size FROM blocks JOIN (SELECT MAX(height) AS height FROM blocks) blocks_max ON blocks.height = blocks_max.height", @@ -1069,17 +1087,19 @@ pub(crate) fn block_max_scanned( let block_hash: Vec = row.get(1)?; let sapling_tree_size: Option = row.get(2)?; let sapling_tree: Vec = row.get(3)?; + let orchard_tree_size: Option = row.get(4)?; Ok(( BlockHeight::from(height), block_hash, sapling_tree_size, sapling_tree, + orchard_tree_size )) }, ) .optional() .map_err(SqliteClientError::from) - .and_then(|meta_row| meta_row.map(parse_block_metadata).transpose()) + .and_then(|meta_row| meta_row.map(|r| parse_block_metadata(params, r)).transpose()) } /// Returns the block height at which the specified transaction was mined, diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 970e9e33d..4ac7a2ad5 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -466,7 +466,7 @@ pub(crate) mod tests { assert_eq!(st.get_spendable_balance(account, 1), value); assert_eq!( - block_max_scanned(&st.wallet().conn) + block_max_scanned(&st.wallet().conn, &st.wallet().params) .unwrap() .unwrap() .block_height(),