diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 80042ceade..5740dbaea5 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -22,15 +22,19 @@ 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`: + - `AccountBalance` - `AccountBirthday` + - `Balance` - `BlockMetadata` - `NoteId` - `NullifierQuery` for use with `WalletRead::get_sapling_nullifiers` + - `Ratio` - `ScannedBlock` - `ShieldedProtocol` - `WalletCommitmentTrees` + - `WalletSummary` - `WalletRead::{chain_height, block_metadata, block_fully_scanned, suggest_scan_ranges, - get_wallet_birthday, get_account_birthday}` + get_wallet_birthday, get_account_birthday, get_wallet_summary}` - `WalletWrite::{put_blocks, update_chain_tip}` - `chain::CommitmentTreeRoot` - `scanning` A new module containing types required for `suggest_scan_ranges` @@ -113,6 +117,8 @@ and this library adheres to Rust's notion of instead to obtain the wallet's view of the chain tip instead, or `suggest_scan_ranges` to obtain information about blocks that need to be scanned. + - `WalletRead::get_balance_at` has been removed. Use `WalletRead::get_wallet_summary` + instead. - `WalletRead::{get_all_nullifiers, get_commitment_tree, get_witnesses}` have been removed without replacement. The utility of these methods is now subsumed by those available from the `WalletCommitmentTrees` trait. diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 2211318e47..d42b0da95f 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1,9 +1,11 @@ //! Interfaces for wallet data persistence & low-level wallet utilities. -use std::fmt::Debug; -use std::io; -use std::num::NonZeroU32; -use std::{collections::HashMap, num::TryFromIntError}; +use std::{ + collections::{BTreeMap, HashMap}, + fmt::Debug, + io, + num::{NonZeroU32, TryFromIntError}, +}; use incrementalmerkletree::{frontier::Frontier, Retention}; use secrecy::SecretVec; @@ -15,7 +17,10 @@ use zcash_primitives::{ memo::{Memo, MemoBytes}, sapling::{self, Node, NOTE_COMMITMENT_TREE_DEPTH}, transaction::{ - components::{amount::Amount, OutPoint}, + components::{ + amount::{Amount, NonNegativeAmount}, + OutPoint, + }, Transaction, TxId, }, zip32::{AccountId, ExtendedFullViewingKey}, @@ -46,6 +51,155 @@ pub enum NullifierQuery { All, } +/// Balance information for a value within a single pool in an account. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Balance { + /// The value in the account that may currently be spent; it is possible to compute witnesses + /// for all the notes that comprise this value, and all of this value is confirmed to the + /// required confirmation depth. + pub spendable_value: NonNegativeAmount, + + /// The value in the account of shielded change notes that do not yet have sufficient + /// confirmations to be spendable. + pub change_pending_confirmation: NonNegativeAmount, + + /// The value in the account of all remaining received notes that either do not have sufficient + /// confirmations to be spendable, or for which witnesses cannot yet be constructed without + /// additional scanning. + pub value_pending_spendability: NonNegativeAmount, +} + +impl Balance { + /// The [`Balance`] value having zero values for all its fields. + pub const ZERO: Self = Self { + spendable_value: NonNegativeAmount::ZERO, + change_pending_confirmation: NonNegativeAmount::ZERO, + value_pending_spendability: NonNegativeAmount::ZERO, + }; + + /// Returns the total value of funds represented by this [`Balance`]. + pub fn total(&self) -> NonNegativeAmount { + (self.spendable_value + self.change_pending_confirmation + self.value_pending_spendability) + .expect("Balance cannot overflow MAX_MONEY") + } +} + +/// Balance information for a single account. The sum of this struct's fields is the total balance +/// of the wallet. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct AccountBalance { + /// The value of unspent Sapling outputs belonging to the account. + pub sapling_balance: Balance, + + /// The value of all unspent transparent outputs belonging to the account, irrespective of + /// confirmation depth. + /// + /// Unshielded balances are not subject to confirmation-depth constraints, because the only + /// possible operation on a transparent balance is to shield it, it is possible to create a + /// zero-conf transaction to perform that shielding, and the resulting shielded notes will be + /// subject to normal confirmation rules. + pub unshielded: NonNegativeAmount, +} + +impl AccountBalance { + /// Returns the total value of funds belonging to the account. + pub fn total(&self) -> NonNegativeAmount { + (self.sapling_balance.total() + self.unshielded) + .expect("Account balance cannot overflow MAX_MONEY") + } +} + +/// A polymorphic ratio type, usually used for rational numbers. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Ratio { + numerator: T, + denominator: T, +} + +impl Ratio { + /// Constructs a new Ratio from a numerator and a denominator. + pub fn new(numerator: T, denominator: T) -> Self { + Self { + numerator, + denominator, + } + } + + /// Returns the numerator of the ratio. + pub fn numerator(&self) -> &T { + &self.numerator + } + + /// Returns the denominator of the ratio. + pub fn denominator(&self) -> &T { + &self.denominator + } +} + +/// A type representing the potentially-spendable value of unspent outputs in the wallet. +/// +/// The balances reported using this data structure may overestimate the total spendable value of +/// the wallet, in the case that the spend of a previously received shielded note has not yet been +/// detected by the process of scanning the chain. The balances reported using this data structure +/// can only be certain to be unspent in the case that [`Self::is_synced`] is true, and even in +/// this circumstance it is possible that a newly created transaction could conflict with a +/// not-yet-mined transaction in the mempool. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct WalletSummary { + account_balances: BTreeMap, + chain_tip_height: BlockHeight, + fully_scanned_height: BlockHeight, + scan_progress: Option>, +} + +impl WalletSummary { + /// Constructs a new [`WalletSummary`] from its constituent parts. + pub fn new( + account_balances: BTreeMap, + chain_tip_height: BlockHeight, + fully_scanned_height: BlockHeight, + scan_progress: Option>, + ) -> Self { + Self { + account_balances, + chain_tip_height, + fully_scanned_height, + scan_progress, + } + } + + /// Returns the balances of accounts in the wallet, keyed by account ID. + pub fn account_balances(&self) -> &BTreeMap { + &self.account_balances + } + + /// Returns the height of the current chain tip. + pub fn chain_tip_height(&self) -> BlockHeight { + self.chain_tip_height + } + + /// Returns the height below which all blocks have been scanned by the wallet, ignoring blocks + /// below the wallet birthday. + pub fn fully_scanned_height(&self) -> BlockHeight { + self.fully_scanned_height + } + + /// Returns the progress of scanning shielded outputs, in terms of the ratio between notes + /// scanned and the total number of notes added to the chain since the wallet birthday. + /// + /// This ratio should only be used to compute progress percentages, and the numerator and + /// denominator should not be treated as authoritative note counts. Returns `None` if the + /// wallet is unable to determine the size of the note commitment tree. + pub fn scan_progress(&self) -> Option> { + self.scan_progress + } + + /// Returns whether or not wallet scanning is complete. + pub fn is_synced(&self) -> bool { + self.chain_tip_height == self.fully_scanned_height + } +} + /// Read-only operations required for light wallet functions. /// /// This trait defines the read-only portion of the storage interface atop which @@ -157,15 +311,12 @@ pub trait WalletRead { extfvk: &ExtendedFullViewingKey, ) -> Result; - /// Returns the wallet balance for an account as of the specified block height. - /// - /// This may be used to obtain a balance that ignores notes that have been received so recently - /// that they are not yet deemed spendable. - fn get_balance_at( + /// Returns the wallet balances and sync status for an account given the specified minimum + /// number of confirmations, or `Ok(None)` if the wallet has no balance data available. + fn get_wallet_summary( &self, - account: AccountId, - anchor_height: BlockHeight, - ) -> Result; + min_confirmations: u32, + ) -> Result, Self::Error>; /// Returns the memo for a note. /// @@ -747,7 +898,7 @@ pub mod testing { use super::{ chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, BlockMetadata, DecryptedTransaction, NoteId, NullifierQuery, ScannedBlock, SentTransaction, - WalletCommitmentTrees, WalletRead, WalletWrite, SAPLING_SHARD_HEIGHT, + WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }; pub struct MockWalletDb { @@ -853,12 +1004,11 @@ pub mod testing { Ok(false) } - fn get_balance_at( + fn get_wallet_summary( &self, - _account: AccountId, - _anchor_height: BlockHeight, - ) -> Result { - Ok(Amount::zero()) + _min_confirmations: u32, + ) -> Result, Self::Error> { + Ok(None) } fn get_memo(&self, _id_note: NoteId) -> Result, Self::Error> { diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index a69bcce221..950d2e2b30 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -326,7 +326,10 @@ mod tests { use zcash_primitives::{ block::BlockHash, - transaction::{components::Amount, fees::zip317::FeeRule}, + transaction::{ + components::{amount::NonNegativeAmount, Amount}, + fees::zip317::FeeRule, + }, zip32::ExtendedSpendingKey, }; @@ -344,7 +347,7 @@ mod tests { use crate::{ testing::{AddressType, TestBuilder}, - wallet::{get_balance, truncate_to_height}, + wallet::truncate_to_height, AccountId, }; @@ -441,24 +444,21 @@ mod tests { let dfvk = st.test_account_sapling().unwrap(); - // Account balance should be zero - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - Amount::zero() - ); + // Wallet summary is not yet available + assert_eq!(st.get_wallet_summary(0), None); // Create fake CompactBlocks sending value to the address - let value = Amount::from_u64(5).unwrap(); - let value2 = Amount::from_u64(7).unwrap(); - let (h, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); - st.generate_next_block(&dfvk, AddressType::DefaultExternal, value2); + let value = NonNegativeAmount::from_u64(5).unwrap(); + let value2 = NonNegativeAmount::from_u64(7).unwrap(); + let (h, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); + st.generate_next_block(&dfvk, AddressType::DefaultExternal, value2.into()); // Scan the cache st.scan_cached_blocks(h, 2); // Account balance should reflect both received notes assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), + st.get_total_balance(AccountId::from(0)), (value + value2).unwrap() ); @@ -469,7 +469,7 @@ mod tests { // Account balance should be unaltered assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), + st.get_total_balance(AccountId::from(0)), (value + value2).unwrap() ); @@ -479,17 +479,14 @@ mod tests { .unwrap(); // Account balance should only contain the first received note - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - value - ); + assert_eq!(st.get_total_balance(AccountId::from(0)), value); // Scan the cache again st.scan_cached_blocks(h, 2); // Account balance should again reflect both received notes assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), + st.get_total_balance(AccountId::from(0)), (value + value2).unwrap() ); } @@ -505,17 +502,14 @@ mod tests { let dfvk = st.test_account_sapling().unwrap(); // Create a block with height SAPLING_ACTIVATION_HEIGHT - let value = Amount::from_u64(50000).unwrap(); - let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + let value = NonNegativeAmount::from_u64(50000).unwrap(); + let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); st.scan_cached_blocks(h1, 1); - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - value - ); + assert_eq!(st.get_total_balance(AccountId::from(0)), value); // Create blocks to reach SAPLING_ACTIVATION_HEIGHT + 2 - let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); - let (h3, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); + let (h3, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); // Scan the later block first st.scan_cached_blocks(h3, 1); @@ -523,8 +517,8 @@ mod tests { // Now scan the block of height SAPLING_ACTIVATION_HEIGHT + 1 st.scan_cached_blocks(h2, 1); assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - Amount::from_u64(150_000).unwrap() + st.get_total_balance(AccountId::from(0)), + NonNegativeAmount::from_u64(150_000).unwrap() ); // We can spend the received notes @@ -562,35 +556,29 @@ mod tests { let dfvk = st.test_account_sapling().unwrap(); - // Account balance should be zero - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - Amount::zero() - ); + // Wallet summary is not yet available + assert_eq!(st.get_wallet_summary(0), None); // Create a fake CompactBlock sending value to the address - let value = Amount::from_u64(5).unwrap(); - let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + let value = NonNegativeAmount::from_u64(5).unwrap(); + let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); // Scan the cache st.scan_cached_blocks(h1, 1); // Account balance should reflect the received note - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - value - ); + assert_eq!(st.get_total_balance(AccountId::from(0)), value); // Create a second fake CompactBlock sending more value to the address - let value2 = Amount::from_u64(7).unwrap(); - let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value2); + let value2 = NonNegativeAmount::from_u64(7).unwrap(); + let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value2.into()); // Scan the cache again st.scan_cached_blocks(h2, 1); // Account balance should reflect both received notes assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), + st.get_total_balance(AccountId::from(0)), (value + value2).unwrap() ); } @@ -603,38 +591,33 @@ mod tests { .build(); let dfvk = st.test_account_sapling().unwrap(); - // Account balance should be zero - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - Amount::zero() - ); + // Wallet summary is not yet available + assert_eq!(st.get_wallet_summary(0), None); // Create a fake CompactBlock sending value to the address - let value = Amount::from_u64(5).unwrap(); + let value = NonNegativeAmount::from_u64(5).unwrap(); let (received_height, _, nf) = - st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); // Scan the cache st.scan_cached_blocks(received_height, 1); // Account balance should reflect the received note - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - value - ); + assert_eq!(st.get_total_balance(AccountId::from(0)), value); // Create a second fake CompactBlock spending value from the address let extsk2 = ExtendedSpendingKey::master(&[0]); let to2 = extsk2.default_address().1; - let value2 = Amount::from_u64(2).unwrap(); - let (spent_height, _) = st.generate_next_block_spending(&dfvk, (nf, value), to2, value2); + let value2 = NonNegativeAmount::from_u64(2).unwrap(); + let (spent_height, _) = + st.generate_next_block_spending(&dfvk, (nf, value.into()), to2, value2.into()); // Scan the cache again st.scan_cached_blocks(spent_height, 1); // Account balance should equal the change assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), + st.get_total_balance(AccountId::from(0)), (value - value2).unwrap() ); } @@ -648,29 +631,27 @@ mod tests { let dfvk = st.test_account_sapling().unwrap(); - // Account balance should be zero - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - Amount::zero() - ); + // Wallet summary is not yet available + assert_eq!(st.get_wallet_summary(0), None); // Create a fake CompactBlock sending value to the address - let value = Amount::from_u64(5).unwrap(); + let value = NonNegativeAmount::from_u64(5).unwrap(); let (received_height, _, nf) = - st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); // Create a second fake CompactBlock spending value from the address let extsk2 = ExtendedSpendingKey::master(&[0]); let to2 = extsk2.default_address().1; - let value2 = Amount::from_u64(2).unwrap(); - let (spent_height, _) = st.generate_next_block_spending(&dfvk, (nf, value), to2, value2); + let value2 = NonNegativeAmount::from_u64(2).unwrap(); + let (spent_height, _) = + st.generate_next_block_spending(&dfvk, (nf, value.into()), to2, value2.into()); // Scan the spending block first. st.scan_cached_blocks(spent_height, 1); // Account balance should equal the change assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), + st.get_total_balance(AccountId::from(0)), (value - value2).unwrap() ); @@ -679,7 +660,7 @@ mod tests { // Account balance should be the same. assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), + st.get_total_balance(AccountId::from(0)), (value - value2).unwrap() ); } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 318fc16abc..670f5f97bd 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -66,7 +66,7 @@ use zcash_client_backend::{ scanning::{ScanPriority, ScanRange}, AccountBirthday, BlockMetadata, DecryptedTransaction, NoteId, NullifierQuery, PoolType, Recipient, ScannedBlock, SentTransaction, ShieldedProtocol, WalletCommitmentTrees, - WalletRead, WalletWrite, SAPLING_SHARD_HEIGHT, + WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, @@ -88,7 +88,10 @@ pub mod error; pub mod serialization; pub mod wallet; -use wallet::commitment_tree::{self, put_shard_roots}; +use wallet::{ + commitment_tree::{self, put_shard_roots}, + SubtreeScanProgress, +}; #[cfg(test)] mod testing; @@ -243,12 +246,11 @@ impl, P: consensus::Parameters> WalletRead for W wallet::is_valid_account_extfvk(self.conn.borrow(), &self.params, account, extfvk) } - fn get_balance_at( + fn get_wallet_summary( &self, - account: AccountId, - anchor_height: BlockHeight, - ) -> Result { - wallet::get_balance_at(self.conn.borrow(), account, anchor_height) + min_confirmations: u32, + ) -> Result, Self::Error> { + wallet::get_wallet_summary(self.conn.borrow(), min_confirmations, &SubtreeScanProgress) } fn get_memo(&self, note_id: NoteId) -> Result, Self::Error> { @@ -456,6 +458,7 @@ impl WalletWrite for WalletDb block.block_hash(), block.block_time(), block.metadata().sapling_tree_size(), + block.sapling_commitments().len().try_into().unwrap(), )?; for tx in block.transactions() { diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index d16af5a39a..0034ff4bbb 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -14,6 +14,7 @@ use tempfile::NamedTempFile; #[cfg(feature = "unstable")] use tempfile::TempDir; +use zcash_client_backend::data_api::AccountBalance; #[allow(deprecated)] use zcash_client_backend::{ address::RecipientAddress, @@ -25,7 +26,7 @@ use zcash_client_backend::{ input_selection::{GreedyInputSelectorError, InputSelector, Proposal}, propose_transfer, spend, }, - AccountBirthday, WalletWrite, + AccountBirthday, WalletSummary, WalletWrite, }, keys::UnifiedSpendingKey, proto::compact_formats::{ @@ -46,7 +47,10 @@ use zcash_primitives::{ Note, Nullifier, PaymentAddress, }, transaction::{ - components::{amount::BalanceError, Amount}, + components::{ + amount::{BalanceError, NonNegativeAmount}, + Amount, + }, fees::FeeRule, TxId, }, @@ -56,7 +60,10 @@ use zcash_primitives::{ use crate::{ chain::init::init_cache_database, error::SqliteClientError, - wallet::{commitment_tree, init::init_wallet_db, sapling::tests::test_prover}, + wallet::{ + commitment_tree, get_wallet_summary, init::init_wallet_db, sapling::tests::test_prover, + SubtreeScanProgress, + }, AccountId, ReceivedNoteId, WalletDb, }; @@ -65,9 +72,7 @@ use super::BlockDb; #[cfg(feature = "transparent-inputs")] use { zcash_client_backend::data_api::wallet::{propose_shielding, shield_transparent_funds}, - zcash_primitives::{ - legacy::TransparentAddress, transaction::components::amount::NonNegativeAmount, - }, + zcash_primitives::legacy::TransparentAddress, }; #[cfg(feature = "unstable")] @@ -530,6 +535,60 @@ impl TestState { min_confirmations, ) } + + fn with_account_balance T>( + &self, + account: AccountId, + min_confirmations: u32, + f: F, + ) -> T { + let binding = + get_wallet_summary(&self.wallet().conn, min_confirmations, &SubtreeScanProgress) + .unwrap() + .unwrap(); + + f(binding.account_balances().get(&account).unwrap()) + } + + pub(crate) fn get_total_balance(&self, account: AccountId) -> NonNegativeAmount { + self.with_account_balance(account, 0, |balance| balance.total()) + } + + pub(crate) fn get_spendable_balance( + &self, + account: AccountId, + min_confirmations: u32, + ) -> NonNegativeAmount { + self.with_account_balance(account, min_confirmations, |balance| { + balance.sapling_balance.spendable_value + }) + } + + pub(crate) fn get_pending_shielded_balance( + &self, + account: AccountId, + min_confirmations: u32, + ) -> NonNegativeAmount { + self.with_account_balance(account, min_confirmations, |balance| { + balance.sapling_balance.value_pending_spendability + + balance.sapling_balance.change_pending_confirmation + }) + .unwrap() + } + + pub(crate) fn get_pending_change( + &self, + account: AccountId, + min_confirmations: u32, + ) -> NonNegativeAmount { + self.with_account_balance(account, min_confirmations, |balance| { + balance.sapling_balance.change_pending_confirmation + }) + } + + pub(crate) fn get_wallet_summary(&self, min_confirmations: u32) -> Option { + get_wallet_summary(&self.wallet().conn, min_confirmations, &SubtreeScanProgress).unwrap() + } } #[allow(dead_code)] diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index e8b1c0e293..a4bb4846bc 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -68,11 +68,13 @@ use incrementalmerkletree::Retention; use rusqlite::{self, named_params, OptionalExtension, ToSql}; use shardtree::ShardTree; use std::cmp; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::convert::TryFrom; use std::io::{self, Cursor}; use std::num::NonZeroU32; use tracing::debug; +use zcash_client_backend::data_api::{AccountBalance, Balance, Ratio, WalletSummary}; +use zcash_primitives::transaction::components::amount::NonNegativeAmount; use zcash_client_backend::data_api::{ scanning::{ScanPriority, ScanRange}, @@ -106,7 +108,7 @@ use crate::{ }; use crate::{SAPLING_TABLES_PREFIX, VERIFY_LOOKAHEAD}; -use self::scanning::replace_queue_entries; +use self::scanning::{parse_priority_code, replace_queue_entries}; #[cfg(feature = "transparent-inputs")] use { @@ -491,56 +493,269 @@ pub(crate) fn is_valid_account_extfvk( }) } -/// Returns the balance for the account, including all mined unspent notes that we know -/// about. -/// -/// WARNING: This balance is potentially unreliable, as mined notes may become unmined due -/// to chain reorgs. You should generally not show this balance to users without some -/// caveat. Use [`get_balance_at`] where you need a more reliable indication of the -/// wallet balance. -#[cfg(test)] -pub(crate) fn get_balance( - conn: &rusqlite::Connection, - account: AccountId, -) -> Result { - let balance = conn.query_row( - "SELECT SUM(value) FROM sapling_received_notes - INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx - WHERE account = ? AND spent IS NULL AND transactions.block IS NOT NULL", - [u32::from(account)], - |row| row.get(0).or(Ok(0)), - )?; +pub(crate) trait ScanProgress { + fn sapling_scan_progress( + &self, + conn: &rusqlite::Connection, + birthday_height: BlockHeight, + fully_scanned_height: BlockHeight, + chain_tip_height: BlockHeight, + ) -> Result>, SqliteClientError>; +} - match Amount::from_i64(balance) { - Ok(amount) if !amount.is_negative() => Ok(amount), - _ => Err(SqliteClientError::CorruptedData( - "Sum of values in sapling_received_notes is out of range".to_string(), - )), +pub(crate) struct SubtreeScanProgress; + +impl ScanProgress for SubtreeScanProgress { + fn sapling_scan_progress( + &self, + conn: &rusqlite::Connection, + birthday_height: BlockHeight, + fully_scanned_height: BlockHeight, + chain_tip_height: BlockHeight, + ) -> Result>, SqliteClientError> { + if fully_scanned_height == chain_tip_height { + // Compute the total blocks scanned since the wallet birthday + conn.query_row( + "SELECT SUM(sapling_output_count) + FROM blocks + WHERE height >= :birthday_height", + named_params![":birthday_height": u32::from(birthday_height)], + |row| { + let scanned = row.get::<_, Option>(0)?; + Ok(scanned.map(|n| Ratio::new(n, n))) + }, + ) + .map_err(SqliteClientError::from) + } else { + // Compute the number of fully scanned notes directly from the blocks table + let fully_scanned_size = conn.query_row( + "SELECT MAX(sapling_commitment_tree_size) + FROM blocks + WHERE height <= :fully_scanned_height", + named_params![":fully_scanned_height": u32::from(fully_scanned_height)], + |row| row.get::<_, Option>(0), + )?; + + // Compute the total blocks scanned so far above the fully scanned height + let scanned_count = conn.query_row( + "SELECT SUM(sapling_output_count) + FROM blocks + WHERE height > :fully_scanned_height", + named_params![":fully_scanned_height": u32::from(fully_scanned_height)], + |row| row.get::<_, Option>(0), + )?; + + // We don't have complete information on how many outputs will exist in the shard at + // the chain tip without having scanned the chain tip block, so we overestimate by + // computing the maximum possible number of notes directly from the shard indices. + // + // TODO: it would be nice to be able to reliably have the size of the commitment tree + // at the chain tip without having to have scanned that block. + Ok(conn + .query_row( + "SELECT MIN(shard_index), MAX(shard_index) + FROM sapling_tree_shards + WHERE subtree_end_height > :fully_scanned_height + OR subtree_end_height IS NULL", + named_params![":fully_scanned_height": u32::from(fully_scanned_height)], + |row| { + let min_tree_size = row + .get::<_, Option>(0)? + .map(|min| min << SAPLING_SHARD_HEIGHT); + let max_idx = row.get::<_, Option>(1)?; + Ok(fully_scanned_size.or(min_tree_size).zip(max_idx).map( + |(min_tree_size, max)| { + let max_tree_size = (max + 1) << SAPLING_SHARD_HEIGHT; + Ratio::new( + scanned_count.unwrap_or(0), + max_tree_size - min_tree_size, + ) + }, + )) + }, + ) + .optional()? + .flatten()) + } } } -/// Returns the verified balance for the account at the specified height, -/// This may be used to obtain a balance that ignores notes that have been -/// received so recently that they are not yet deemed spendable. -pub(crate) fn get_balance_at( +/// Returns the spendable balance for the account at the specified height. +/// +/// This may be used to obtain a balance that ignores notes that have been detected so recently +/// that they are not yet spendable, or for which it is not yet possible to construct witnesses. +/// +/// `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( conn: &rusqlite::Connection, - account: AccountId, - anchor_height: BlockHeight, -) -> Result { - let balance = conn.query_row( - "SELECT SUM(value) FROM sapling_received_notes - INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx - WHERE account = ? AND spent IS NULL AND transactions.block <= ?", - [u32::from(account), u32::from(anchor_height)], - |row| row.get(0).or(Ok(0)), + min_confirmations: u32, + progress: &impl ScanProgress, +) -> Result, SqliteClientError> { + let chain_tip_height = match scan_queue_extrema(conn)? { + Some((_, max)) => max, + None => { + return Ok(None); + } + }; + + let birthday_height = + 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()); + let summary_height = (chain_tip_height + 1).saturating_sub(std::cmp::max(min_confirmations, 1)); + + let sapling_scan_progress = progress.sapling_scan_progress( + conn, + birthday_height, + fully_scanned_height, + chain_tip_height, + )?; + + // If the shard containing the summary height contains any unscanned ranges that start below or + // including that height, none of our balance is currently spendable. + let any_spendable = conn.query_row( + "SELECT NOT EXISTS( + SELECT 1 FROM v_sapling_shard_unscanned_ranges + WHERE :summary_height + BETWEEN subtree_start_height + AND IFNULL(subtree_end_height, :summary_height) + AND block_range_start <= :summary_height + )", + named_params![":summary_height": u32::from(summary_height)], + |row| row.get::<_, bool>(0), + )?; + + let mut stmt_select_notes = conn.prepare_cached( + "SELECT n.account, n.value, n.is_change, scan_state.max_priority, t.block + FROM sapling_received_notes n + JOIN transactions t ON t.id_tx = n.tx + LEFT OUTER JOIN v_sapling_shards_scan_state scan_state + ON n.commitment_tree_position >= scan_state.start_position + AND n.commitment_tree_position < scan_state.end_position_exclusive + WHERE n.spent IS NULL + AND ( + t.expiry_height IS NULL + OR t.block IS NOT NULL + OR t.expiry_height >= :summary_height + )", )?; - match Amount::from_i64(balance) { - Ok(amount) if !amount.is_negative() => Ok(amount), - _ => Err(SqliteClientError::CorruptedData( - "Sum of values in sapling_received_notes is out of range".to_string(), - )), + let mut account_balances: BTreeMap = BTreeMap::new(); + let mut rows = + stmt_select_notes.query(named_params![":summary_height": u32::from(summary_height)])?; + while let Some(row) = rows.next()? { + let account = row.get::<_, u32>(0).map(AccountId::from)?; + + let value_raw = row.get::<_, i64>(1)?; + let value = NonNegativeAmount::from_nonnegative_i64(value_raw).map_err(|_| { + SqliteClientError::CorruptedData(format!("Negative received note value: {}", value_raw)) + })?; + + let is_change = row.get::<_, bool>(2)?; + + // If `max_priority` is null, this means that the note is not positioned; the note + // will not be spendable, so we assign the scan priority to `ChainTip` as a priority + // that is greater than `Scanned` + let max_priority_raw = row.get::<_, Option>(3)?; + let max_priority = max_priority_raw.map_or_else( + || Ok(ScanPriority::ChainTip), + |raw| { + parse_priority_code(raw).ok_or_else(|| { + SqliteClientError::CorruptedData(format!( + "Priority code {} not recognized.", + raw + )) + }) + }, + )?; + + let received_height = row.get::<_, Option>(4)?.map(BlockHeight::from); + + let is_spendable = any_spendable + && received_height.iter().any(|h| h <= &summary_height) + && max_priority <= ScanPriority::Scanned; + + let is_pending_change = is_change && received_height.iter().all(|h| h > &summary_height); + + let (spendable_value, change_pending_confirmation, value_pending_spendability) = { + let zero = NonNegativeAmount::ZERO; + if is_spendable { + (value, zero, zero) + } else if is_pending_change { + (zero, value, zero) + } else { + (zero, zero, value) + } + }; + + account_balances + .entry(account) + .and_modify(|bal| { + bal.sapling_balance.spendable_value = (bal.sapling_balance.spendable_value + + spendable_value) + .expect("Spendable value cannot overflow"); + bal.sapling_balance.change_pending_confirmation = + (bal.sapling_balance.change_pending_confirmation + change_pending_confirmation) + .expect("Pending change value cannot overflow"); + bal.sapling_balance.value_pending_spendability = + (bal.sapling_balance.value_pending_spendability + value_pending_spendability) + .expect("Value pending spendability cannot overflow"); + }) + .or_insert(AccountBalance { + sapling_balance: Balance { + spendable_value, + change_pending_confirmation, + value_pending_spendability, + }, + unshielded: NonNegativeAmount::ZERO, + }); + } + + #[cfg(feature = "transparent-inputs")] + { + let zero_conf_height = (chain_tip_height + 1).saturating_sub(min_confirmations); + let mut stmt_transparent_balances = conn.prepare( + "SELECT u.received_by_account, SUM(u.value_zat) + FROM utxos u + LEFT OUTER JOIN transactions tx + ON tx.id_tx = u.spent_in_tx + WHERE u.height <= :max_height + AND tx.block IS NULL + GROUP BY u.received_by_account", + )?; + let mut rows = stmt_transparent_balances + .query(named_params![":max_height": u32::from(zero_conf_height)])?; + + while let Some(row) = rows.next()? { + let account = AccountId::from(row.get::<_, u32>(0)?); + let raw_value = row.get(1)?; + let value = NonNegativeAmount::from_nonnegative_i64(raw_value).map_err(|_| { + SqliteClientError::CorruptedData(format!("Negative UTXO value {:?}", raw_value)) + })?; + + account_balances + .entry(account) + .and_modify(|bal| { + bal.unshielded = + (bal.unshielded + value).expect("Unshielded value cannot overflow") + }) + .or_insert(AccountBalance { + sapling_balance: Balance::ZERO, + unshielded: value, + }); + } } + + let summary = WalletSummary::new( + account_balances, + chain_tip_height, + fully_scanned_height, + sapling_scan_progress, + ); + + Ok(Some(summary)) } /// Returns the memo for a received note, if the note is known to the wallet. @@ -828,52 +1043,50 @@ pub(crate) fn block_metadata( pub(crate) fn block_fully_scanned( conn: &rusqlite::Connection, ) -> Result, SqliteClientError> { - // We assume here that the wallet was either initialized via `init_blocks_table`, or - // its birthday is Sapling activation, so the earliest block in the `blocks` table is - // the first fully-scanned block (because it occurs before any wallet activity). - // - // We further assume that the only way we get a contiguous range of block heights in - // the `blocks` table starting with this earliest block, is if all scanning operations - // have been performed on those blocks. This holds because the `blocks` table is only - // altered by `WalletDb::put_blocks` via `put_block`, and the effective combination of - // intra-range linear scanning and the nullifier map ensures that we discover all - // wallet-related information within the contiguous range. - // - // The fully-scanned height is therefore the greatest height in the first contiguous - // range of 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 - FROM blocks - INNER JOIN ( - WITH contiguous AS ( - SELECT height, ROW_NUMBER() OVER (ORDER BY height) - height AS grp - FROM blocks + 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 + // starting with the birthday block, is if all scanning operations have been performed on those + // blocks. This holds because the `blocks` table is only altered by `WalletDb::put_blocks` via + // `put_block`, and the effective combination of intra-range linear scanning and the nullifier + // map ensures that we discover all wallet-related information within the contiguous range. + // + // The fully-scanned height is therefore the greatest height in the first contiguous range of + // 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 + FROM blocks + INNER JOIN ( + WITH contiguous AS ( + SELECT height, ROW_NUMBER() OVER (ORDER BY height) - height AS grp + FROM blocks + ) + SELECT MIN(height) AS group_min_height, MAX(height) AS group_max_height + FROM contiguous + GROUP BY grp + HAVING :birthday_height BETWEEN group_min_height AND group_max_height ) - SELECT MAX(height) AS [fully_scanned_height] - FROM contiguous - GROUP BY grp - ORDER BY height - LIMIT 1 + ON height = group_max_height", + named_params![":birthday_height": u32::from(birthday_height)], + |row| { + let height: u32 = row.get(0)?; + let block_hash: Vec = row.get(1)?; + let sapling_tree_size: Option = row.get(2)?; + let sapling_tree: Vec = row.get(3)?; + Ok(( + BlockHeight::from(height), + block_hash, + sapling_tree_size, + sapling_tree, + )) + }, ) - ON height = fully_scanned_height", - [], - |row| { - let height: u32 = row.get(0)?; - let block_hash: Vec = row.get(1)?; - let sapling_tree_size: Option = row.get(2)?; - let sapling_tree: Vec = 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()) + .optional() + .map_err(SqliteClientError::from) + .and_then(|meta_row| meta_row.map(parse_block_metadata).transpose()) + } else { + Ok(None) + } } /// Returns the block height at which the specified transaction was mined, @@ -1171,6 +1384,7 @@ pub(crate) fn put_block( block_hash: BlockHash, block_time: u32, sapling_commitment_tree_size: u32, + sapling_output_count: u32, ) -> Result<(), SqliteClientError> { let block_hash_data = conn .query_row( @@ -1200,6 +1414,7 @@ pub(crate) fn put_block( hash, time, sapling_commitment_tree_size, + sapling_output_count, sapling_tree ) VALUES ( @@ -1207,19 +1422,22 @@ pub(crate) fn put_block( :hash, :block_time, :sapling_commitment_tree_size, + :sapling_output_count, x'00' ) ON CONFLICT (height) DO UPDATE SET hash = :hash, time = :block_time, - sapling_commitment_tree_size = :sapling_commitment_tree_size", + sapling_commitment_tree_size = :sapling_commitment_tree_size, + sapling_output_count = :sapling_output_count", )?; stmt_upsert_block.execute(named_params![ ":height": u32::from(block_height), ":hash": &block_hash.0[..], ":block_time": block_time, - ":sapling_commitment_tree_size": sapling_commitment_tree_size + ":sapling_commitment_tree_size": sapling_commitment_tree_size, + ":sapling_output_count": sapling_output_count, ])?; Ok(()) @@ -1683,8 +1901,6 @@ mod tests { use crate::{testing::TestBuilder, AccountId}; - use super::get_balance; - #[cfg(feature = "transparent-inputs")] use { incrementalmerkletree::frontier::Frontier, @@ -1704,11 +1920,8 @@ mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - // The account should be empty - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - Amount::zero() - ); + // The account should have no summary information + assert_eq!(st.get_wallet_summary(0), None); // We can't get an anchor height, as we have not scanned any blocks. assert_eq!( @@ -1718,15 +1931,17 @@ mod tests { None ); - // An invalid account has zero balance + // The default address is set for the test account + assert_matches!( + st.wallet().get_current_address(AccountId::from(0)), + Ok(Some(_)) + ); + + // No default address is set for an un-initialized account assert_matches!( st.wallet().get_current_address(AccountId::from(1)), Ok(None) ); - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - Amount::zero() - ); } #[test] diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 98162eebd6..a8ba6a80d2 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -219,7 +219,9 @@ mod tests { time INTEGER NOT NULL, sapling_tree BLOB NOT NULL , sapling_commitment_tree_size INTEGER, - orchard_commitment_tree_size INTEGER)", + orchard_commitment_tree_size INTEGER, + sapling_output_count INTEGER, + orchard_action_count INTEGER)", "CREATE TABLE nullifier_map ( spend_pool INTEGER NOT NULL, nf BLOB NOT NULL, @@ -365,16 +367,15 @@ mod tests { } let expected_views = vec![ - // v_sapling_shard_unscanned_ranges + // v_sapling_shard_scan_ranges format!( - "CREATE VIEW v_sapling_shard_unscanned_ranges AS - WITH wallet_birthday AS (SELECT MIN(birthday_height) AS height FROM accounts) + "CREATE VIEW v_sapling_shard_scan_ranges AS SELECT shard.shard_index, shard.shard_index << 16 AS start_position, (shard.shard_index + 1) << 16 AS end_position_exclusive, IFNULL(prev_shard.subtree_end_height, {}) AS subtree_start_height, - shard.subtree_end_height AS subtree_end_height, + shard.subtree_end_height, shard.contains_marked, scan_queue.block_range_start, scan_queue.block_range_end, @@ -383,18 +384,53 @@ mod tests { LEFT OUTER JOIN sapling_tree_shards prev_shard ON shard.shard_index = prev_shard.shard_index + 1 INNER JOIN scan_queue ON + (scan_queue.block_range_start >= subtree_start_height AND shard.subtree_end_height IS NULL) OR (scan_queue.block_range_start BETWEEN subtree_start_height AND shard.subtree_end_height) OR ((scan_queue.block_range_end - 1) BETWEEN subtree_start_height AND shard.subtree_end_height) OR ( scan_queue.block_range_start <= prev_shard.subtree_end_height AND (scan_queue.block_range_end - 1) >= shard.subtree_end_height - ) - INNER JOIN wallet_birthday - WHERE scan_queue.priority > {} - AND scan_queue.block_range_end > wallet_birthday.height", + )", u32::from(st.network().activation_height(NetworkUpgrade::Sapling).unwrap()), + ), + // v_sapling_shard_unscanned_ranges + format!( + "CREATE VIEW v_sapling_shard_unscanned_ranges AS + WITH wallet_birthday AS (SELECT MIN(birthday_height) AS height FROM accounts) + SELECT + shard_index, + start_position, + end_position_exclusive, + subtree_start_height, + subtree_end_height, + contains_marked, + block_range_start, + block_range_end, + priority + FROM v_sapling_shard_scan_ranges + INNER JOIN wallet_birthday + WHERE priority > {} + AND block_range_end > wallet_birthday.height", priority_code(&ScanPriority::Scanned) ), + // v_sapling_shards_scan_state + "CREATE VIEW v_sapling_shards_scan_state AS + SELECT + shard_index, + start_position, + end_position_exclusive, + subtree_start_height, + subtree_end_height, + contains_marked, + MAX(priority) AS max_priority + FROM v_sapling_shard_scan_ranges + GROUP BY + shard_index, + start_position, + end_position_exclusive, + subtree_start_height, + subtree_end_height, + contains_marked".to_owned(), // v_transactions "CREATE VIEW v_transactions AS WITH diff --git a/zcash_client_sqlite/src/wallet/init/migrations.rs b/zcash_client_sqlite/src/wallet/init/migrations.rs index f5cd2e2ac2..03381ba2cf 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations.rs @@ -12,6 +12,7 @@ mod ufvk_support; mod utxos_table; mod v_sapling_shard_unscanned_ranges; mod v_transactions_net; +mod wallet_summaries; use schemer_rusqlite::RusqliteMigration; use secrecy::SecretVec; @@ -42,6 +43,8 @@ pub(super) fn all_migrations( // add_account_birthdays // | // v_sapling_shard_unscanned_ranges + // | + // wallet_summaries vec![ Box::new(initial_setup::Migration {}), Box::new(utxos_table::Migration {}), @@ -72,5 +75,6 @@ pub(super) fn all_migrations( Box::new(v_sapling_shard_unscanned_ranges::Migration { params: params.clone(), }), + Box::new(wallet_summaries::Migration), ] } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/v_sapling_shard_unscanned_ranges.rs b/zcash_client_sqlite/src/wallet/init/migrations/v_sapling_shard_unscanned_ranges.rs index ea1aa68ce2..7a72942c09 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/v_sapling_shard_unscanned_ranges.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/v_sapling_shard_unscanned_ranges.rs @@ -38,14 +38,13 @@ impl RusqliteMigration for Migration

{ fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), Self::Error> { transaction.execute_batch( &format!( - "CREATE VIEW v_sapling_shard_unscanned_ranges AS - WITH wallet_birthday AS (SELECT MIN(birthday_height) AS height FROM accounts) + "CREATE VIEW v_sapling_shard_scan_ranges AS SELECT shard.shard_index, shard.shard_index << {} AS start_position, (shard.shard_index + 1) << {} AS end_position_exclusive, IFNULL(prev_shard.subtree_end_height, {}) AS subtree_start_height, - shard.subtree_end_height AS subtree_end_height, + shard.subtree_end_height, shard.contains_marked, scan_queue.block_range_start, scan_queue.block_range_end, @@ -54,22 +53,39 @@ impl RusqliteMigration for Migration

{ LEFT OUTER JOIN sapling_tree_shards prev_shard ON shard.shard_index = prev_shard.shard_index + 1 INNER JOIN scan_queue ON + (scan_queue.block_range_start >= subtree_start_height AND shard.subtree_end_height IS NULL) OR (scan_queue.block_range_start BETWEEN subtree_start_height AND shard.subtree_end_height) OR ((scan_queue.block_range_end - 1) BETWEEN subtree_start_height AND shard.subtree_end_height) OR ( scan_queue.block_range_start <= prev_shard.subtree_end_height AND (scan_queue.block_range_end - 1) >= shard.subtree_end_height - ) - INNER JOIN wallet_birthday - WHERE scan_queue.priority > {} - AND scan_queue.block_range_end > wallet_birthday.height;", + )", SAPLING_SHARD_HEIGHT, SAPLING_SHARD_HEIGHT, u32::from(self.params.activation_height(NetworkUpgrade::Sapling).unwrap()), - priority_code(&ScanPriority::Scanned), ) )?; + transaction.execute_batch(&format!( + "CREATE VIEW v_sapling_shard_unscanned_ranges AS + WITH wallet_birthday AS (SELECT MIN(birthday_height) AS height FROM accounts) + SELECT + shard_index, + start_position, + end_position_exclusive, + subtree_start_height, + subtree_end_height, + contains_marked, + block_range_start, + block_range_end, + priority + FROM v_sapling_shard_scan_ranges + INNER JOIN wallet_birthday + WHERE priority > {} + AND block_range_end > wallet_birthday.height;", + priority_code(&ScanPriority::Scanned), + ))?; + Ok(()) } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/wallet_summaries.rs b/zcash_client_sqlite/src/wallet/init/migrations/wallet_summaries.rs new file mode 100644 index 0000000000..2be9ca15ab --- /dev/null +++ b/zcash_client_sqlite/src/wallet/init/migrations/wallet_summaries.rs @@ -0,0 +1,89 @@ +//! This migration adds views and database changes required to provide accurate wallet summaries. + +use std::collections::HashSet; + +use schemer_rusqlite::RusqliteMigration; +use uuid::Uuid; + +use crate::wallet::init::WalletMigrationError; + +use super::v_sapling_shard_unscanned_ranges; + +pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0xc5bf7f71_2297_41ff_89e1_75e07c4e8838); + +pub(super) struct Migration; + +impl schemer::Migration for Migration { + fn id(&self) -> Uuid { + MIGRATION_ID + } + + fn dependencies(&self) -> HashSet { + [v_sapling_shard_unscanned_ranges::MIGRATION_ID] + .into_iter() + .collect() + } + + fn description(&self) -> &'static str { + "Adds views and data required to produce accurate wallet summaries." + } +} + +impl RusqliteMigration for Migration { + type Error = WalletMigrationError; + + fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), Self::Error> { + // Add columns to the `blocks` table to track the number of scanned outputs in each block. + // We use the note commitment tree size information that we have in contiguous regions to + // populate this data, but we don't make any attempt to handle the boundary cases because + // we're just using this information for the progress metric, which can be a bit sloppy. + transaction.execute_batch( + "ALTER TABLE blocks ADD COLUMN sapling_output_count INTEGER; + ALTER TABLE blocks ADD COLUMN orchard_action_count INTEGER;", + )?; + + transaction.execute_batch( + // set the number of outputs everywhere that we have sequential blocks + "CREATE TEMPORARY TABLE block_deltas AS + SELECT + cur.height AS height, + (cur.sapling_commitment_tree_size - prev.sapling_commitment_tree_size) AS sapling_delta, + (cur.orchard_commitment_tree_size - prev.orchard_commitment_tree_size) AS orchard_delta + FROM blocks cur + INNER JOIN blocks prev + ON cur.height = prev.height + 1; + + UPDATE blocks + SET sapling_output_count = block_deltas.sapling_delta, + orchard_action_count = block_deltas.orchard_delta + FROM block_deltas + WHERE block_deltas.height = blocks.height;" + )?; + + transaction.execute_batch( + "CREATE VIEW v_sapling_shards_scan_state AS + SELECT + shard_index, + start_position, + end_position_exclusive, + subtree_start_height, + subtree_end_height, + contains_marked, + MAX(priority) AS max_priority + FROM v_sapling_shard_scan_ranges + GROUP BY + shard_index, + start_position, + end_position_exclusive, + subtree_start_height, + subtree_end_height, + contains_marked;", + )?; + + Ok(()) + } + + fn down(&self, _transaction: &rusqlite::Transaction) -> Result<(), Self::Error> { + panic!("This migration cannot be reverted."); + } +} diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 099e6d6847..d2a5271c11 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -437,7 +437,10 @@ pub(crate) mod tests { note_encryption::try_sapling_output_recovery, prover::TxProver, Note, PaymentAddress, }, transaction::{ - components::{amount::BalanceError, Amount}, + components::{ + amount::{BalanceError, NonNegativeAmount}, + Amount, + }, fees::{fixed::FeeRule as FixedFeeRule, zip317::FeeRule as Zip317FeeRule}, Transaction, }, @@ -450,7 +453,7 @@ pub(crate) mod tests { self, error::Error, wallet::input_selection::{GreedyInputSelector, GreedyInputSelectorError}, - AccountBirthday, ShieldedProtocol, WalletRead, + AccountBirthday, Ratio, ShieldedProtocol, WalletRead, }, decrypt_transaction, fees::{fixed, zip317, DustOutputPolicy}, @@ -462,14 +465,14 @@ pub(crate) mod tests { use crate::{ error::SqliteClientError, testing::{AddressType, BlockCache, TestBuilder, TestState}, - wallet::{commitment_tree, get_balance, get_balance_at}, + wallet::commitment_tree, AccountId, NoteId, ReceivedNoteId, }; #[cfg(feature = "transparent-inputs")] use { zcash_client_backend::{data_api::WalletWrite, wallet::WalletTransparentOutput}, - zcash_primitives::transaction::components::{amount::NonNegativeAmount, OutPoint, TxOut}, + zcash_primitives::transaction::components::{OutPoint, TxOut}, }; pub(crate) fn test_prover() -> impl TxProver { @@ -492,24 +495,12 @@ pub(crate) mod tests { let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet in a single note - let value = Amount::from_u64(60000).unwrap(); - let (h, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + let value = NonNegativeAmount::from_u64(60000).unwrap(); + let (h, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); st.scan_cached_blocks(h, 1); // Verified balance matches total balance - let (_, anchor_height) = st - .wallet() - .get_target_and_anchor_heights(NonZeroU32::new(1).unwrap()) - .unwrap() - .unwrap(); - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - value - ); - assert_eq!( - get_balance_at(&st.wallet().conn, AccountId::from(0), anchor_height).unwrap(), - value - ); + assert_eq!(st.get_total_balance(account), value); let to_extsk = ExtendedSpendingKey::master(&[]); let to: RecipientAddress = to_extsk.default_address().1.into(); @@ -666,11 +657,8 @@ pub(crate) mod tests { let dfvk = st.test_account_sapling().unwrap(); let to = dfvk.default_address().1.into(); - // Account balance should be zero - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - Amount::zero() - ); + // Wallet summary is not yet available + assert_eq!(st.get_wallet_summary(0), None); // We cannot do anything if we aren't synchronised assert_matches!( @@ -693,46 +681,42 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (_, usk, _) = st.test_account().unwrap(); + let (account, usk, _) = st.test_account().unwrap(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet in a single note - let value = Amount::from_u64(50000).unwrap(); - let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + let value = NonNegativeAmount::from_u64(50000).unwrap(); + let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); st.scan_cached_blocks(h1, 1); // Verified balance matches total balance - let (_, anchor_height) = st - .wallet() - .get_target_and_anchor_heights(NonZeroU32::new(10).unwrap()) - .unwrap() - .unwrap(); - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - value - ); + assert_eq!(st.get_total_balance(account), value); + + // Value is considered pending + assert_eq!(st.get_pending_shielded_balance(account, 10), value); + + // Wallet is fully scanned + let summary = st.get_wallet_summary(1); assert_eq!( - get_balance_at(&st.wallet().conn, AccountId::from(0), anchor_height).unwrap(), - value + summary.and_then(|s| s.scan_progress()), + Some(Ratio::new(1, 1)) ); // Add more funds to the wallet in a second note - let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); st.scan_cached_blocks(h2, 1); // Verified balance does not include the second note - let (_, anchor_height2) = st - .wallet() - .get_target_and_anchor_heights(NonZeroU32::new(10).unwrap()) - .unwrap() - .unwrap(); - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - (value + value).unwrap() - ); + let total = (value + value).unwrap(); + assert_eq!(st.get_spendable_balance(account, 2), value); + assert_eq!(st.get_pending_shielded_balance(account, 2), value); + assert_eq!(st.get_total_balance(account), total); + + // Wallet is still fully scanned + let summary = st.get_wallet_summary(1); assert_eq!( - get_balance_at(&st.wallet().conn, AccountId::from(0), anchor_height2).unwrap(), - value + summary.and_then(|s| s.scan_progress()), + Some(Ratio::new(2, 2)) ); // Spend fails because there are insufficient verified notes @@ -758,7 +742,7 @@ pub(crate) mod tests { // Mine blocks SAPLING_ACTIVATION_HEIGHT + 2 to 9 until just before the second // note is verified for _ in 2..10 { - st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); } st.scan_cached_blocks(h2 + 1, 8); @@ -781,7 +765,7 @@ pub(crate) mod tests { ); // Mine block 11 so that the second note becomes verified - let (h11, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + let (h11, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); st.scan_cached_blocks(h11, 1); // Second spend should now succeed @@ -805,17 +789,14 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (_, usk, _) = st.test_account().unwrap(); + let (account, usk, _) = st.test_account().unwrap(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet in a single note - let value = Amount::from_u64(50000).unwrap(); - let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + let value = NonNegativeAmount::from_u64(50000).unwrap(); + let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); st.scan_cached_blocks(h1, 1); - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - value - ); + assert_eq!(st.get_total_balance(account), value); // Send some of the funds to another address let extsk2 = ExtendedSpendingKey::master(&[]); @@ -855,7 +836,7 @@ pub(crate) mod tests { st.generate_next_block( &ExtendedSpendingKey::master(&[i as u8]).to_diversifiable_full_viewing_key(), AddressType::DefaultExternal, - value, + value.into(), ); } st.scan_cached_blocks(h1 + 1, 41); @@ -881,7 +862,7 @@ pub(crate) mod tests { let (h43, _, _) = st.generate_next_block( &ExtendedSpendingKey::master(&[42]).to_diversifiable_full_viewing_key(), AddressType::DefaultExternal, - value, + value.into(), ); st.scan_cached_blocks(h43, 1); @@ -904,17 +885,14 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (_, usk, _) = st.test_account().unwrap(); + let (account, usk, _) = st.test_account().unwrap(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet in a single note - let value = Amount::from_u64(50000).unwrap(); - let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + let value = NonNegativeAmount::from_u64(50000).unwrap(); + let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value.into()); st.scan_cached_blocks(h1, 1); - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - value - ); + assert_eq!(st.get_total_balance(account), value); let extsk2 = ExtendedSpendingKey::master(&[]); let addr2 = extsk2.default_address().1; @@ -985,7 +963,7 @@ pub(crate) mod tests { st.generate_next_block( &ExtendedSpendingKey::master(&[i as u8]).to_diversifiable_full_viewing_key(), AddressType::DefaultExternal, - value, + value.into(), ); } st.scan_cached_blocks(h1 + 1, 42); @@ -1005,7 +983,7 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (_, usk, _) = st.test_account().unwrap(); + let (account, usk, _) = st.test_account().unwrap(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet in a single note @@ -1014,18 +992,9 @@ pub(crate) mod tests { st.scan_cached_blocks(h, 1); // Verified balance matches total balance - let (_, anchor_height) = st - .wallet() - .get_target_and_anchor_heights(NonZeroU32::new(1).unwrap()) - .unwrap() - .unwrap(); - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - value - ); assert_eq!( - get_balance_at(&st.wallet().conn, AccountId::from(0), anchor_height).unwrap(), - value + st.get_total_balance(account), + NonNegativeAmount::try_from(value).unwrap() ); let to = TransparentAddress::PublicKey([7; 20]).into(); @@ -1049,27 +1018,24 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (_, usk, _) = st.test_account().unwrap(); + let (account, usk, _) = st.test_account().unwrap(); let dfvk = st.test_account_sapling().unwrap(); - // Add funds to the wallet in a single note + // Add funds to the wallet in a single note owned by the internal spending key let value = Amount::from_u64(60000).unwrap(); let (h, _, _) = st.generate_next_block(&dfvk, AddressType::Internal, value); st.scan_cached_blocks(h, 1); // Verified balance matches total balance - let (_, anchor_height) = st - .wallet() - .get_target_and_anchor_heights(NonZeroU32::new(10).unwrap()) - .unwrap() - .unwrap(); assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - value + st.get_total_balance(account), + NonNegativeAmount::try_from(value).unwrap() ); + + // the balance is considered pending assert_eq!( - get_balance_at(&st.wallet().conn, AccountId::from(0), anchor_height).unwrap(), - value + st.get_pending_shielded_balance(account, 10), + NonNegativeAmount::try_from(value).unwrap() ); let to = TransparentAddress::PublicKey([7; 20]).into(); @@ -1093,7 +1059,7 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (_, usk, _) = st.test_account().unwrap(); + let (account, usk, _) = st.test_account().unwrap(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet @@ -1116,18 +1082,9 @@ pub(crate) mod tests { // Verified balance matches total balance let total = Amount::from_u64(60000).unwrap(); - let (_, anchor_height) = st - .wallet() - .get_target_and_anchor_heights(NonZeroU32::new(1).unwrap()) - .unwrap() - .unwrap(); - assert_eq!( - get_balance(&st.wallet().conn, AccountId::from(0)).unwrap(), - total - ); assert_eq!( - get_balance_at(&st.wallet().conn, AccountId::from(0), anchor_height).unwrap(), - total + st.get_total_balance(account), + NonNegativeAmount::try_from(total).unwrap() ); let input_selector = GreedyInputSelector::new( diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index 640f6bf9b6..47066e1627 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -881,7 +881,8 @@ mod tests { use zcash_client_backend::data_api::{ chain::CommitmentTreeRoot, scanning::{ScanPriority, ScanRange}, - AccountBirthday, WalletCommitmentTrees, WalletRead, WalletWrite, + AccountBirthday, Ratio, WalletCommitmentTrees, WalletRead, WalletWrite, + SAPLING_SHARD_HEIGHT, }; use zcash_primitives::{ block::BlockHash, @@ -1689,6 +1690,10 @@ mod tests { ) .unwrap(); + // We have scan ranges and a subtree, but have scanned no blocks. + let summary = st.get_wallet_summary(1); + assert_eq!(summary.and_then(|s| s.scan_progress()), None); + // Set up prior chain state. This simulates us having imported a wallet // with a birthday 520 blocks below the chain tip. st.wallet_mut().update_chain_tip(prior_tip).unwrap(); @@ -1723,6 +1728,14 @@ mod tests { ); st.scan_cached_blocks(max_scanned, 1); + // We have scanned a block, so we now have a starting tree position, 500 blocks above the + // wallet birthday but before the end of the shard. + let summary = st.get_wallet_summary(1); + assert_eq!( + summary.and_then(|s| s.scan_progress()), + Some(Ratio::new(1, 0x1 << SAPLING_SHARD_HEIGHT)) + ); + // Now simulate shutting down, and then restarting 70 blocks later, after a shard // has been completed. let last_shard_start = prior_tip + 50; @@ -1758,6 +1771,14 @@ mod tests { let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap(); assert_eq!(actual, expected); + + // We've crossed a subtree boundary, and so still only have one scanned note but have two + // shards worth of notes to scan. + let summary = st.get_wallet_summary(1); + assert_eq!( + summary.and_then(|s| s.scan_progress()), + Some(Ratio::new(1, 0x1 << (SAPLING_SHARD_HEIGHT + 1))) + ); } #[test] diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index d5855b66a9..ee6d3861bc 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -16,6 +16,11 @@ and this library adheres to Rust's notion of - `impl HashSer for String` is provided under the `test-dependencies` feature flag. This is a test-only impl; the identity leaf value is `_` and the combining operation is concatenation. +- `zcash_primitives::transaction::components::amount::NonNegativeAmount::ZERO` +- Additional trait implementations for `NonNegativeAmount`: + - `TryFrom for NonNegativeAmount` + - `Add for NonNegativeAmount` + - `Add for Option` ### Changed - `zcash_primitives::transaction`: diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index b4e8c4f1e5..dbb3e85d7c 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -238,6 +238,9 @@ impl TryFrom for Amount { pub struct NonNegativeAmount(Amount); impl NonNegativeAmount { + /// Returns the identity `NonNegativeAmount` + pub const ZERO: Self = NonNegativeAmount(Amount(0)); + /// Creates a NonNegativeAmount from a u64. /// /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. @@ -259,6 +262,50 @@ impl From for Amount { } } +impl TryFrom for NonNegativeAmount { + type Error = (); + + fn try_from(value: Amount) -> Result { + if value.is_negative() { + Err(()) + } else { + Ok(NonNegativeAmount(value)) + } + } +} + +impl Add for NonNegativeAmount { + type Output = Option; + + fn add(self, rhs: NonNegativeAmount) -> Option { + (self.0 + rhs.0).map(NonNegativeAmount) + } +} + +impl Add for Option { + type Output = Self; + + fn add(self, rhs: NonNegativeAmount) -> Option { + self.and_then(|lhs| lhs + rhs) + } +} + +impl Sub for NonNegativeAmount { + type Output = Option; + + fn sub(self, rhs: NonNegativeAmount) -> Option { + (self.0 - rhs.0).and_then(|amt| NonNegativeAmount::try_from(amt).ok()) + } +} + +impl Sub for Option { + type Output = Self; + + fn sub(self, rhs: NonNegativeAmount) -> Option { + self.and_then(|lhs| lhs - rhs) + } +} + /// A type for balance violations in amount addition and subtraction /// (overflow and underflow of allowed ranges) #[derive(Copy, Clone, Debug, PartialEq, Eq)]