diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index f04a1640f..ade2268be 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -32,8 +32,10 @@ and this library adheres to Rust's notion of - `RecipientAddress::Unified` - `zcash_client_backend::data_api`: - `WalletRead::get_unified_full_viewing_keys` + - `WalletRead::get_current_address` - `WalletRead::get_all_nullifiers` - `WalletWrite::remove_unmined_tx` (behind the `unstable` feature flag). + - `WalletWrite::get_next_available_address` - `zcash_client_backend::proto`: - `actions` field on `compact_formats::CompactTx` - `compact_formats::CompactOrchardAction` @@ -79,8 +81,6 @@ and this library adheres to Rust's notion of a `min_confirmations` argument that is used to compute an upper bound on the anchor height being returned; this had previously been hardcoded to `data_api::wallet::ANCHOR_OFFSET`. - - `WalletRead::get_address` now returns a `UnifiedAddress` instead of a - `sapling::PaymentAddress`. - `WalletRead::get_spendable_notes` has been renamed to `get_spendable_sapling_notes` - `WalletRead::select_spendable_notes` has been renamed to @@ -128,6 +128,8 @@ and this library adheres to Rust's notion of - `zcash_client_backend::data_api`: - `WalletRead::get_extended_full_viewing_keys` (use `WalletRead::get_unified_full_viewing_keys` instead). + - `WalletRead::get_address` (use `WalletRead::get_current_address` or + `WalletWrite::get_next_available_address` instead.) - The hardcoded `data_api::wallet::ANCHOR_OFFSET` constant. - `zcash_client_backend::wallet::AccountId` (moved to `zcash_primitives::zip32::AccountId`). diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 59ddb37fa..388cde921 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -114,12 +114,15 @@ pub trait WalletRead { /// or `Ok(None)` if the transaction is not mined in the main chain. fn get_tx_height(&self, txid: TxId) -> Result, Self::Error>; - /// Returns the unified address for the specified account, if the account - /// identifier specified refers to a valid account for this wallet. + /// Returns the most recently generated unified address for the specified account, if the + /// account identifier specified refers to a valid account for this wallet. /// /// This will return `Ok(None)` if the account identifier does not correspond /// to a known account. - fn get_address(&self, account: AccountId) -> Result, Self::Error>; + fn get_current_address( + &self, + account: AccountId, + ) -> Result, Self::Error>; /// Returns all unified full viewing keys known to this wallet. fn get_unified_full_viewing_keys( @@ -266,6 +269,16 @@ pub struct SentTransactionOutput<'a> { /// This trait encapsulates the write capabilities required to update stored /// wallet data. pub trait WalletWrite: WalletRead { + /// Generates and persists the next available diversified address, given the current + /// addresses known to the wallet. + /// + /// Returns `Ok(None)` if the account identifier does not correspond to a known + /// account. + fn get_next_available_address( + &mut self, + account: AccountId, + ) -> Result, Self::Error>; + /// Updates the state of the wallet database by persisting the provided /// block information, along with the updated witness data that was /// produced when scanning the block for transactions pertaining to @@ -283,6 +296,8 @@ pub trait WalletWrite: WalletRead { received_tx: &DecryptedTransaction, ) -> Result; + /// Saves information about a transaction that was constructed and sent by the wallet to the + /// persistent wallet store. fn store_sent_tx(&mut self, sent_tx: &SentTransaction) -> Result; /// Removes the specified unmined transaction from the persistent wallet store, if it @@ -409,7 +424,10 @@ pub mod testing { Ok(None) } - fn get_address(&self, _account: AccountId) -> Result, Self::Error> { + fn get_current_address( + &self, + _account: AccountId, + ) -> Result, Self::Error> { Ok(None) } @@ -503,6 +521,13 @@ pub mod testing { } impl WalletWrite for MockWalletDb { + fn get_next_available_address( + &mut self, + _account: AccountId, + ) -> Result, Self::Error> { + Ok(None) + } + #[allow(clippy::type_complexity)] fn advance_by_block( &mut self, diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 07b2d50fc..1ec3f1e4c 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -16,6 +16,8 @@ and this library adheres to Rust's notion of transparent address decoding. - `SqliteClientError::RequestedRewindInvalid`, to report when requested rewinds exceed supported bounds. + - `SqliteClientError::DiversifierIndexOutOfRange`, to report when the space + of available diversifier indices has been exhausted. - An `unstable` feature flag; this is added to parts of the API that may change in any release. It enables `zcash_client_backend`'s `unstable` feature flag. - New summary views that may be directly accessed in the sqlite database. diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index d9331deaf..536f9536b 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -65,6 +65,10 @@ pub enum SqliteClientError { /// Wrapper for errors from zcash_client_backend BackendError(data_api::error::Error), + + /// The space of allocatable diversifier indices has been exhausted for + /// the given account. + DiversifierIndexOutOfRange, } impl error::Error for SqliteClientError { @@ -103,6 +107,7 @@ impl fmt::Display for SqliteClientError { SqliteClientError::Io(e) => write!(f, "{}", e), SqliteClientError::InvalidMemo(e) => write!(f, "{}", e), SqliteClientError::BackendError(e) => write!(f, "{}", e), + SqliteClientError::DiversifierIndexOutOfRange => write!(f, "The space of available diversifier indices is exhausted"), } } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 39f231695..5e0a8de0a 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -48,7 +48,7 @@ use zcash_primitives::{ merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::{Node, Nullifier}, transaction::{components::Amount, Transaction, TxId}, - zip32::{AccountId, ExtendedFullViewingKey}, + zip32::{AccountId, DiversifierIndex, ExtendedFullViewingKey}, }; use zcash_client_backend::{ @@ -153,8 +153,11 @@ impl WalletRead for WalletDb

{ wallet::get_unified_full_viewing_keys(self) } - fn get_address(&self, account: AccountId) -> Result, Self::Error> { - wallet::get_address_ua(self, account) + fn get_current_address( + &self, + account: AccountId, + ) -> Result, Self::Error> { + wallet::get_current_address(self, account).map(|res| res.map(|(addr, _)| addr)) } fn is_valid_account_extfvk( @@ -276,8 +279,11 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> { self.wallet_db.get_unified_full_viewing_keys() } - fn get_address(&self, account: AccountId) -> Result, Self::Error> { - self.wallet_db.get_address(account) + fn get_current_address( + &self, + account: AccountId, + ) -> Result, Self::Error> { + self.wallet_db.get_current_address(account) } fn is_valid_account_extfvk( @@ -396,6 +402,34 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> { #[allow(deprecated)] impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> { + fn get_next_available_address( + &mut self, + account: AccountId, + ) -> Result, Self::Error> { + match self.get_unified_full_viewing_keys()?.get(&account) { + Some(ufvk) => { + let search_from = match wallet::get_current_address(self.wallet_db, account)? { + Some((_, mut last_diversifier_index)) => { + last_diversifier_index + .increment() + .map_err(|_| SqliteClientError::DiversifierIndexOutOfRange)?; + last_diversifier_index + } + None => DiversifierIndex::default(), + }; + + let (addr, diversifier_index) = ufvk + .find_address(search_from) + .ok_or(SqliteClientError::DiversifierIndexOutOfRange)?; + + self.stmt_insert_address(account, diversifier_index, &addr)?; + + Ok(Some(addr)) + } + None => Ok(None), + } + } + #[allow(clippy::type_complexity)] fn advance_by_block( &mut self, @@ -691,13 +725,6 @@ mod tests { use rusqlite::params; use std::collections::HashMap; - use zcash_client_backend::{ - keys::{sapling, UnifiedFullViewingKey}, - proto::compact_formats::{ - CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx, - }, - }; - #[cfg(feature = "transparent-inputs")] use zcash_primitives::{legacy, legacy::keys::IncomingViewingKey}; @@ -714,7 +741,18 @@ mod tests { zip32::ExtendedFullViewingKey, }; - use crate::{wallet::init::init_accounts_table, AccountId, WalletDb}; + use zcash_client_backend::{ + data_api::{WalletRead, WalletWrite}, + keys::{sapling, UnifiedFullViewingKey}, + proto::compact_formats::{ + CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx, + }, + }; + + use crate::{ + wallet::init::{init_accounts_table, init_wallet_db}, + AccountId, WalletDb, + }; use super::BlockDb; @@ -930,6 +968,29 @@ mod tests { .unwrap(); } + #[test] + pub(crate) fn get_next_available_address() { + use tempfile::NamedTempFile; + + let data_file = NamedTempFile::new().unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), network()).unwrap(); + + let account = AccountId::from(0); + init_wallet_db(&mut db_data, None).unwrap(); + let _ = init_test_accounts_table_ufvk(&db_data); + + let current_addr = db_data.get_current_address(account).unwrap(); + assert!(current_addr.is_some()); + + let mut update_ops = db_data.get_update_ops().unwrap(); + let addr2 = update_ops.get_next_available_address(account).unwrap(); + assert!(addr2.is_some()); + assert_ne!(current_addr, addr2); + + let addr2_cur = db_data.get_current_address(account).unwrap(); + assert_eq!(addr2, addr2_cur); + } + #[cfg(feature = "transparent-inputs")] #[test] fn transparent_receivers() { diff --git a/zcash_client_sqlite/src/prepared.rs b/zcash_client_sqlite/src/prepared.rs index f80114ae5..a61fc1e9a 100644 --- a/zcash_client_sqlite/src/prepared.rs +++ b/zcash_client_sqlite/src/prepared.rs @@ -15,9 +15,11 @@ use zcash_primitives::{ merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::{Diversifier, Node, Nullifier}, transaction::{components::Amount, TxId}, - zip32::AccountId, + zip32::{AccountId, DiversifierIndex}, }; +use zcash_client_backend::address::UnifiedAddress; + use crate::{error::SqliteClientError, wallet::PoolType, NoteId, WalletDb}; #[cfg(feature = "transparent-inputs")] @@ -64,6 +66,8 @@ pub struct DataConnStmtCache<'a, P> { stmt_insert_witness: Statement<'a>, stmt_prune_witnesses: Statement<'a>, stmt_update_expired: Statement<'a>, + + stmt_insert_address: Statement<'a>, } impl<'a, P> DataConnStmtCache<'a, P> { @@ -160,6 +164,10 @@ impl<'a, P> DataConnStmtCache<'a, P> { WHERE id_tx = received_notes.spent AND block IS NULL AND expiry_height < ? )", )?, + stmt_insert_address: wallet_db.conn.prepare( + "INSERT INTO addresses (account, diversifier_index_be, address) + VALUES (:account, :diversifier_index_be, :address)", + )?, } ) } @@ -369,6 +377,27 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> { Ok(rows) } + + /// Adds the given address and diversifier index to the addresses table. + /// + /// Returns the database row for the newly-inserted address. + pub(crate) fn stmt_insert_address( + &mut self, + account: AccountId, + mut diversifier_index: DiversifierIndex, + address: &UnifiedAddress, + ) -> Result { + diversifier_index.0.reverse(); + let sql_args: &[(&str, &dyn ToSql)] = &[ + (":account", &u32::from(account)), + (":diversifier_index_be", &&diversifier_index.0[..]), + (":address", &address.encode(&self.wallet_db.params)), + ]; + + self.stmt_insert_address.execute_named(sql_args)?; + + Ok(self.wallet_db.conn.last_insert_rowid()) + } } impl<'a, P> DataConnStmtCache<'a, P> { diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index eb009250a..4bde63fcf 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -22,7 +22,7 @@ use zcash_primitives::{ merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::{keys::DiversifiableFullViewingKey, Node, Note, Nullifier, PaymentAddress}, transaction::{components::Amount, Transaction, TxId}, - zip32::{AccountId, ExtendedFullViewingKey}, + zip32::{AccountId, DiversifierIndex, ExtendedFullViewingKey}, }; use zcash_client_backend::{ @@ -158,10 +158,12 @@ pub fn get_address( wdb: &WalletDb

, account: AccountId, ) -> Result, SqliteClientError> { - // This returns the first diversified address, which will be the default one. + // This returns the most recently generated address. let addr: String = wdb.conn.query_row( - "SELECT address FROM addresses - WHERE account = ?", + "SELECT address + FROM addresses WHERE account = ? + ORDER BY diversifier_index_be DESC + LIMIT 1", &[u32::from(account)], |row| row.get(0), )?; @@ -177,21 +179,31 @@ pub fn get_address( }) } -pub(crate) fn get_address_ua( +pub(crate) fn get_current_address( wdb: &WalletDb

, account: AccountId, -) -> Result, SqliteClientError> { - // This returns the first diversified address, which will be the default one. - let addr: Option = wdb +) -> Result, SqliteClientError> { + // This returns the most recently generated address. + let addr: Option<(String, Vec)> = wdb .conn .query_row_named( - "SELECT address FROM addresses WHERE account = :account", + "SELECT address, diversifier_index_be + FROM addresses WHERE account = :account + ORDER BY diversifier_index_be DESC + LIMIT 1", &[(":account", &u32::from(account))], - |row| row.get(0), + |row| Ok((row.get(0)?, row.get(1)?)), ) .optional()?; - addr.map(|addr_str| { + addr.map(|(addr_str, di_vec)| { + let mut di_be: [u8; 11] = di_vec.try_into().map_err(|_| { + SqliteClientError::CorruptedData( + "Diverisifier index is not an 11-byte value".to_owned(), + ) + })?; + di_be.reverse(); + RecipientAddress::decode(&wdb.params, &addr_str) .ok_or_else(|| { SqliteClientError::CorruptedData("Not a valid Zcash recipient address".to_owned()) @@ -203,6 +215,7 @@ pub(crate) fn get_address_ua( addr_str, ))), }) + .map(|addr| (addr, DiversifierIndex(di_be))) }) .transpose() } @@ -619,7 +632,7 @@ pub fn get_block_hash

( pub fn get_rewind_height

(wdb: &WalletDb

) -> Result, SqliteClientError> { wdb.conn .query_row( - "SELECT MIN(tx.block) + "SELECT MIN(tx.block) FROM received_notes n JOIN transactions tx ON tx.id_tx = n.tx WHERE n.spent IS NULL", diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 9613d12fc..75ab95782 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -608,14 +608,16 @@ fn add_account_internal>( )?; // Always derive the default Unified Address for the account. - let (address, idx) = key.default_address(); + let (address, mut idx) = key.default_address(); let address_str: String = address.encode(network); + // the diversifier index is stored in big-endian order to allow sorting + idx.0.reverse(); conn.execute_named( - "INSERT INTO addresses (account, diversifier_index, address) - VALUES (:account, :diversifier_index, :address)", + "INSERT INTO addresses (account, diversifier_index_be, address) + VALUES (:account, :diversifier_index_be, :address)", &[ (":account", &::from(account)), - (":diversifier_index", &&idx.0[..]), + (":diversifier_index_be", &&idx.0[..]), (":address", &address_str), ], )?; @@ -730,10 +732,10 @@ mod tests { )", "CREATE TABLE addresses ( account INTEGER NOT NULL, - diversifier_index BLOB NOT NULL, + diversifier_index_be BLOB NOT NULL, address TEXT NOT NULL, FOREIGN KEY (account) REFERENCES accounts(account), - CONSTRAINT diversification UNIQUE (account, diversifier_index) + CONSTRAINT diversification UNIQUE (account, diversifier_index_be) )", "CREATE TABLE blocks ( height INTEGER PRIMARY KEY, diff --git a/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs b/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs index a627b5f26..9b22f3eff 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs @@ -51,10 +51,10 @@ impl RusqliteMigration for AddressesTableMigration

transaction.execute_batch( "CREATE TABLE addresses ( account INTEGER NOT NULL, - diversifier_index BLOB NOT NULL, + diversifier_index_be BLOB NOT NULL, address TEXT NOT NULL, FOREIGN KEY (account) REFERENCES accounts(account), - CONSTRAINT diversification UNIQUE (account, diversifier_index) + CONSTRAINT diversification UNIQUE (account, diversifier_index_be) ); CREATE TABLE accounts_new ( account INTEGER PRIMARY KEY,