diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 999d91860..618bf2c88 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -11,14 +11,15 @@ and this library adheres to Rust's notion of - `impl Eq for zcash_client_backend::zip321::{Payment, TransactionRequest}` - `impl Debug` for `zcash_client_backend::{data_api::wallet::input_selection::Proposal, wallet::ReceivedSaplingNote}` - `zcash_client_backend::data_api`: + - `BlockMetadata` + - `NullifierQuery` for use with `WalletRead::get_sapling_nullifiers` + - `ScannedBlock` + - `ShieldedProtocol` - `WalletRead::{block_metadata, block_fully_scanned, suggest_scan_ranges}` - `WalletWrite::put_block` - `WalletCommitmentTrees` - - `testing::MockWalletDb::new` - - `NullifierQuery` for use with `WalletRead::get_sapling_nullifiers` - - `BlockMetadata` - - `ScannedBlock` - `chain::CommitmentTreeRoot` + - `testing::MockWalletDb::new` - `wallet::input_sellection::Proposal::{min_target_height, min_anchor_height}`: - `zcash_client_backend::wallet::WalletSaplingOutput::note_commitment_tree_position` - `zcash_client_backend::scanning::ScanError` @@ -35,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 @@ -49,6 +51,8 @@ and this library adheres to Rust's notion of now take their respective `min_confirmations` arguments as `NonZeroU32` - 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`. + - The variants of the `PoolType` enum have changed; the `PoolType::Sapling` variant has been + removed in favor of a `PoolType::Shielded` variant that wraps a `ShieldedProtocol` value. - `zcash_client_backend::wallet`: - `SpendableNote` has been renamed to `ReceivedSaplingNote`. - Arguments to `WalletSaplingOutput::from_parts` have changed. diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 7cc945f18..216fc7174 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -372,14 +372,21 @@ pub struct SentTransaction<'a> { pub utxos_spent: Vec, } +/// A shielded transfer protocol supported by the wallet. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum ShieldedProtocol { + /// The Sapling protocol + Sapling, + // TODO: Orchard +} + /// A value pool to which the wallet supports sending transaction outputs. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum PoolType { /// The transparent value pool Transparent, - /// The Sapling value pool - Sapling, - // TODO: Orchard + /// A shielded value pool. + Shielded(ShieldedProtocol), } /// A type that represents the recipient of a transaction output; a recipient address (and, for @@ -487,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, diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index 16d530ac7..49d0f9005 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -45,12 +45,13 @@ //! // 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) //! # } //! # } //! ``` use zcash_primitives::{ + block::BlockHash, consensus::{self, BlockHeight}, sapling::{self, note_encryption::PreparedIncomingViewingKey}, zip32::Scope, @@ -60,9 +61,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 +144,8 @@ pub fn scan_cached_blocks( params: &ParamsT, block_source: &BlockSourceT, data_db: &mut DbT, - from_height: Option, - limit: Option, + from_height: BlockHeight, + limit: u32, ) -> Result<(), Error> where ParamsT: consensus::Parameters + Send + 'static, @@ -178,61 +181,94 @@ 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)); + } - batch_runner.flush(); + if from_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() + }), + ) + }); - 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)?; + add_block_to_runner(params, block, &mut batch_runner); - let spent_nf: Vec<&sapling::Nullifier> = scanned_block - .transactions - .iter() - .flat_map(|tx| tx.sapling_spends.iter().map(|spend| spend.nf())) - .collect(); + Ok(()) + }, + )?; - sapling_nullifiers.retain(|(_, nf)| !spent_nf.contains(&nf)); - sapling_nullifiers.extend(scanned_block.transactions.iter().flat_map(|tx| { - tx.sapling_outputs + batch_runner.flush(); + + 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() - .map(|out| (out.account(), *out.nf())) - })); + .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 + .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(()) - })?; + prior_block_metadata = Some(*scanned_block.metadata()); + data_db.put_block(scanned_block).map_err(Error::Wallet)?; + Ok(()) + }, + )?; Ok(()) } diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 7f3d4ce6a..60ef900ee 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -37,6 +37,8 @@ use crate::{ pub mod input_selection; use input_selection::{GreedyInputSelector, GreedyInputSelectorError, InputSelector}; +use super::ShieldedProtocol; + #[cfg(feature = "transparent-inputs")] use { crate::wallet::WalletTransparentOutput, @@ -550,7 +552,7 @@ where payment.memo.clone().unwrap_or_else(MemoBytes::empty), )?; sapling_output_meta.push(( - Recipient::Unified(ua.clone(), PoolType::Sapling), + Recipient::Unified(ua.clone(), PoolType::Shielded(ShieldedProtocol::Sapling)), payment.amount, payment.memo.clone(), )); @@ -589,7 +591,10 @@ where MemoBytes::empty(), )?; sapling_output_meta.push(( - Recipient::InternalAccount(account, PoolType::Sapling), + Recipient::InternalAccount( + account, + PoolType::Shielded(ShieldedProtocol::Sapling), + ), *amount, change_memo.clone(), )) @@ -610,20 +615,23 @@ where .output_index(i) .expect("An output should exist in the transaction for each shielded payment."); - let received_as = - if let Recipient::InternalAccount(account, PoolType::Sapling) = recipient { - tx.sapling_bundle().and_then(|bundle| { - try_sapling_note_decryption( - params, - proposal.min_target_height(), - &internal_ivk, - &bundle.shielded_outputs()[output_index], - ) - .map(|(note, _, _)| (account, note)) - }) - } else { - None - }; + let received_as = if let Recipient::InternalAccount( + account, + PoolType::Shielded(ShieldedProtocol::Sapling), + ) = recipient + { + tx.sapling_bundle().and_then(|bundle| { + try_sapling_note_decryption( + params, + proposal.min_target_height(), + &internal_ivk, + &bundle.shielded_outputs()[output_index], + ) + .map(|(note, _, _)| (account, note)) + }) + } else { + None + }; SentTransactionOutput::from_parts(output_index, recipient, value, memo, received_as) }); diff --git a/zcash_client_backend/src/scanning.rs b/zcash_client_backend/src/scanning.rs index 8eeb1b57e..d2f419acd 100644 --- a/zcash_client_backend/src/scanning.rs +++ b/zcash_client_backend/src/scanning.rs @@ -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 {:?} 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( } } +pub(crate) fn check_continuity( + block: &CompactBlock, + prior_block_metadata: Option<&BlockMetadata>, +) -> Option { + 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::()) + .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>, ) -> Result, ScanError> { - let mut wtxs: Vec> = vec![]; - let mut sapling_note_commitments: Vec<(sapling::Node, Retention)> = 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> = vec![]; + let mut sapling_note_commitments: Vec<(sapling::Node, Retention)> = vec![]; for (tx_idx, tx) in block.vtx.into_iter().enumerate() { let txid = tx.txid(); diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 478a3bf45..46478fbc9 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -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!( diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 2f65053c4..c3d9b04bd 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -59,7 +59,8 @@ use zcash_client_backend::{ self, chain::{BlockSource, CommitmentTreeRoot}, BlockMetadata, DecryptedTransaction, NullifierQuery, PoolType, Recipient, ScannedBlock, - SentTransaction, WalletCommitmentTrees, WalletRead, WalletWrite, SAPLING_SHARD_HEIGHT, + SentTransaction, ShieldedProtocol, WalletCommitmentTrees, WalletRead, WalletWrite, + SAPLING_SHARD_HEIGHT, }, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, @@ -459,7 +460,10 @@ impl WalletWrite for WalletDb let recipient = if output.transfer_type == TransferType::Outgoing { Recipient::Sapling(output.note.recipient()) } else { - Recipient::InternalAccount(output.account, PoolType::Sapling) + Recipient::InternalAccount( + output.account, + PoolType::Shielded(ShieldedProtocol::Sapling) + ) }; wallet::put_sent_output( diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 6b103cfc3..d70fea082 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -65,9 +65,10 @@ //! - `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::io::Cursor; -use std::{collections::HashMap, io}; +use std::io::{self, Cursor}; +use zcash_client_backend::data_api::ShieldedProtocol; use zcash_primitives::{ block::BlockHash, @@ -116,7 +117,7 @@ pub(crate) fn pool_code(pool_type: PoolType) -> i64 { // implementation detail. match pool_type { PoolType::Transparent => 0i64, - PoolType::Sapling => 2i64, + PoolType::Shielded(ShieldedProtocol::Sapling) => 2i64, } } @@ -1119,7 +1120,11 @@ fn recipient_params( ) -> (Option, Option, PoolType) { match to { Recipient::Transparent(addr) => (Some(addr.encode(params)), None, PoolType::Transparent), - Recipient::Sapling(addr) => (Some(addr.encode(params)), None, PoolType::Sapling), + Recipient::Sapling(addr) => ( + Some(addr.encode(params)), + None, + PoolType::Shielded(ShieldedProtocol::Sapling), + ), Recipient::Unified(addr, pool) => (Some(addr.encode(params)), None, *pool), Recipient::InternalAccount(id, pool) => (None, Some(u32::from(*id)), *pool), } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs b/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs index def63a91d..656281b70 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs @@ -8,7 +8,9 @@ use secrecy::{ExposeSecret, SecretVec}; use uuid::Uuid; use zcash_client_backend::{ - address::RecipientAddress, data_api::PoolType, keys::UnifiedSpendingKey, + address::RecipientAddress, + data_api::{PoolType, ShieldedProtocol}, + keys::UnifiedSpendingKey, }; use zcash_primitives::{consensus, zip32::AccountId}; @@ -229,7 +231,9 @@ impl RusqliteMigration for Migration

{ )) })?; let output_pool = match decoded_address { - RecipientAddress::Shielded(_) => Ok(pool_code(PoolType::Sapling)), + RecipientAddress::Shielded(_) => { + Ok(pool_code(PoolType::Shielded(ShieldedProtocol::Sapling))) + } RecipientAddress::Transparent(_) => Ok(pool_code(PoolType::Transparent)), RecipientAddress::Unified(_) => Err(WalletMigrationError::CorruptedData( "Unified addresses should not yet appear in the sent_notes table." diff --git a/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs index fc3ab7378..7f82cea4a 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs @@ -6,9 +6,10 @@ use rusqlite::{self, named_params}; use schemer; use schemer_rusqlite::RusqliteMigration; use uuid::Uuid; +use zcash_client_backend::data_api::{PoolType, ShieldedProtocol}; use super::add_transaction_views; -use crate::wallet::{init::WalletMigrationError, pool_code, PoolType}; +use crate::wallet::{init::WalletMigrationError, pool_code}; pub(super) const MIGRATION_ID: Uuid = Uuid::from_fields( 0x2aa4d24f, @@ -48,7 +49,7 @@ impl RusqliteMigration for Migration { SELECT tx, :output_pool, output_index, from_account, from_account, value FROM sent_notes", named_params![ - ":output_pool": &pool_code(PoolType::Sapling) + ":output_pool": &pool_code(PoolType::Shielded(ShieldedProtocol::Sapling)) ] )?; diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index a686ec617..5108db19e 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -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(