From bf42ec29a1792ce318bb38dd28c7731734de2b58 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 22 Nov 2024 02:46:05 +0000 Subject: [PATCH] zcash_client_sqlite: Add `AccountUuid` to expose the `uuid` column --- zcash_client_sqlite/CHANGELOG.md | 9 +++++ zcash_client_sqlite/src/lib.rs | 62 +++++++++++++++++++++++-------- zcash_client_sqlite/src/wallet.rs | 54 +++++++++++++++++++++++---- 3 files changed, 102 insertions(+), 23 deletions(-) diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index addf9c867..0d21f113f 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -7,6 +7,11 @@ and this library adheres to Rust's notion of ## [Unreleased] +### Added +- `zcash_client_sqlite::AccountUuid` +- `zcash_client_sqlite::Account::uuid` +- `zcash_client_sqlite::WalletDb::get_account_for_uuid` + ### Changed - The `v_transactions` view has been modified: - The `account_id` column has been replaced with `account_uuid`. @@ -14,6 +19,10 @@ and this library adheres to Rust's notion of - The `from_account_id` column has been replaced with `from_account_uuid`. - The `to_account_id` column has been replaced with `to_account_uuid`. +### Removed +- `zcash_client_sqlite::AccountId::{from_u32, as_u32}` (use `AccountUuid` and + its methods instead). + ## [0.13.0] - 2024-11-14 ### Added diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 74e2d23a4..3aeeee55d 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -43,6 +43,7 @@ use std::{ }; use subtle::ConditionallySelectable; use tracing::{debug, trace, warn}; +use uuid::Uuid; use zcash_client_backend::{ address::UnifiedAddress, @@ -161,30 +162,48 @@ pub(crate) const UA_TRANSPARENT: bool = true; pub(crate) const DEFAULT_UA_REQUEST: UnifiedAddressRequest = UnifiedAddressRequest::unsafe_new(UA_ORCHARD, true, UA_TRANSPARENT); -/// The ID type for accounts. +/// Unique identifier for a specific account tracked by a [`WalletDb`]. +/// +/// Account identifiers are "one-way stable": a given identifier always points to a +/// specific viewing key within a specific [`WalletDb`] instance, but the same viewing key +/// may have multiple account identifiers over time. In particular, this crate upholds the +/// following properties: +/// +/// - When an account starts being tracked within a [`WalletDb`] instance (via APIs like +/// [`WalletWrite::create_account`], [`WalletWrite::import_account_hd`], or +/// [`WalletWrite::import_account_ufvk`]), a new `AccountUuid` is generated. +/// - If an `AccountUuid` is present within a [`WalletDb`], it always points to the same +/// account. +/// +/// What this means is that account identifiers are not stable across "wallet recreation +/// events". Examples of these include: +/// - Restoring a wallet from a backed-up seed. +/// - Importing the same viewing key into two different wallet instances. #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Default)] -pub struct AccountId(u32); +pub struct AccountUuid(Uuid); -impl AccountId { - /// Constructs an `AccountId` from a bare `u32` value. The resulting identifier is not - /// guaranteed to correspond to any account stored in the database. - #[cfg(feature = "unstable")] - pub fn from_u32(value: u32) -> Self { - AccountId(value) +impl AccountUuid { + /// Constructs an `AccountUuid` from a bare [`Uuid`] value. + /// + /// The resulting identifier is not guaranteed to correspond to any account stored in + /// a [`WalletDb`]. Callers should use [`WalletDb::get_account_for_uuid`] + pub fn from_uuid(value: Uuid) -> Self { + AccountUuid(value) } - /// Unwraps a raw `accounts` table primary key value from its typesafe wrapper. - /// - /// Note that account identifiers are not guaranteed to be stable; if a wallet is restored from - /// seed, the account identifiers of the restored wallet are not likely to correspond to the - /// identifiers for the same accounts in another wallet created or restored from the same seed. - /// These unwrapped identifier values should therefore be treated as ephemeral. - #[cfg(feature = "unstable")] - pub fn as_u32(&self) -> u32 { + /// Exposes the opaque account identifier from its typesafe wrapper. + pub fn expose_uuid(&self) -> Uuid { self.0 } } +/// The local identifier for an account. +/// +/// This is an ephemeral value for efficiently and generically working with accounts in a +/// [`WalletDb`]. To reference accounts in external contexts, use [`AccountUuid`]. +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Default)] +pub struct AccountId(u32); + impl ConditionallySelectable for AccountId { fn conditional_select(a: &Self, b: &Self, choice: subtle::Choice) -> Self { AccountId(ConditionallySelectable::conditional_select( @@ -219,6 +238,17 @@ pub struct WalletDb { params: P, } +impl, P: consensus::Parameters> WalletDb { + /// Returns the account with the given "one-way stable" identifier, if it was created + /// by this wallet instance. + pub fn get_account_for_uuid( + &self, + account_uuid: AccountUuid, + ) -> Result, SqliteClientError> { + wallet::get_account_for_uuid(self.conn.borrow(), &self.params, account_uuid) + } +} + /// A wrapper for a SQLite transaction affecting the wallet database. pub struct SqlTransaction<'conn>(pub(crate) &'conn rusqlite::Transaction<'conn>); diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index c4a1263e1..260c51dc4 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -120,7 +120,7 @@ use crate::{ AccountId, SqlTransaction, TransferType, WalletCommitmentTrees, WalletDb, DEFAULT_UA_REQUEST, PRUNING_DEPTH, SAPLING_TABLES_PREFIX, }; -use crate::{TxRef, VERIFY_LOOKAHEAD}; +use crate::{AccountUuid, TxRef, VERIFY_LOOKAHEAD}; #[cfg(feature = "transparent-inputs")] use zcash_primitives::transaction::components::TxOut; @@ -201,11 +201,17 @@ pub(crate) enum ViewingKey { #[derive(Debug, Clone)] pub struct Account { account_id: AccountId, + uuid: AccountUuid, kind: AccountSource, viewing_key: ViewingKey, } impl Account { + /// Returns the "one-way stable" identifier for this account within its [`WalletDb`]. + pub fn uuid(&self) -> AccountUuid { + self.uuid + } + /// Returns the default Unified Address for the account, /// along with the diversifier index that generated it. /// @@ -368,6 +374,8 @@ pub(crate) fn add_account( } // TODO(#1490): check for IVK collisions. + let uuid = AccountUuid(Uuid::new_v4()); + let (hd_seed_fingerprint, hd_account_index, spending_key_available) = match kind { AccountSource::Derived { seed_fingerprint, @@ -425,7 +433,7 @@ pub(crate) fn add_account( RETURNING id; "#, named_params![ - ":uuid": Uuid::new_v4(), + ":uuid": uuid.0, ":account_kind": account_kind_code(kind), ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.to_bytes()), ":hd_account_index": hd_account_index.map(u32::from), @@ -465,6 +473,7 @@ pub(crate) fn add_account( let account = Account { account_id, + uuid, kind, viewing_key, }; @@ -709,7 +718,7 @@ pub(crate) fn get_account_for_ufvk( let transparent_item: Option> = None; let mut stmt = conn.prepare( - "SELECT id, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, has_spend_key + "SELECT id, uuid, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, has_spend_key FROM accounts WHERE orchard_fvk_item_cache = :orchard_fvk_item_cache OR sapling_fvk_item_cache = :sapling_fvk_item_cache @@ -725,6 +734,7 @@ pub(crate) fn get_account_for_ufvk( ], |row| { let account_id = row.get::<_, u32>("id").map(AccountId)?; + let uuid = AccountUuid(row.get("uuid")?); let kind = parse_account_source( row.get("account_kind")?, row.get("hd_seed_fingerprint")?, @@ -746,6 +756,7 @@ pub(crate) fn get_account_for_ufvk( Ok(Account { account_id, + uuid, kind, viewing_key, }) @@ -771,7 +782,7 @@ pub(crate) fn get_derived_account( account_index: zip32::AccountId, ) -> Result, SqliteClientError> { let mut stmt = conn.prepare( - "SELECT id, ufvk + "SELECT id, uuid, ufvk FROM accounts WHERE hd_seed_fingerprint = :hd_seed_fingerprint AND hd_account_index = :account_id", @@ -783,8 +794,9 @@ pub(crate) fn get_derived_account( ":hd_account_index": u32::from(account_index), ], |row| { - let account_id = row.get::<_, u32>(0).map(AccountId)?; - let ufvk = match row.get::<_, Option>(1)? { + let account_id = row.get::<_, u32>("id").map(AccountId)?; + let uuid = AccountUuid(row.get("uuid")?); + let ufvk = match row.get::<_, Option>("ufvk")? { None => Err(SqliteClientError::CorruptedData(format!( "Missing unified full viewing key for derived account {:?}", account_id, @@ -798,6 +810,7 @@ pub(crate) fn get_derived_account( }?; Ok(Account { account_id, + uuid, kind: AccountSource::Derived { seed_fingerprint: *seed, account_index, @@ -1833,6 +1846,30 @@ pub(crate) fn block_height_extrema( }) } +pub(crate) fn get_account_for_uuid( + conn: &rusqlite::Connection, + params: &P, + account_uuid: AccountUuid, +) -> Result, SqliteClientError> { + match conn + .query_row( + "SELECT id FROM accounts WHERE uuid = :uuid", + named_params! {":uuid": account_uuid.0}, + |row| row.get("id").map(AccountId), + ) + .optional()? + { + None => Ok(None), + Some(account_id) => Ok(Some( + // TODO: `get_account` should return a non-optional value now that `AccountId` + // is guaranteed to exist (because it can't be externally constructed), but + // the `WalletRead::AccountId` associated type is permitted to not exist, and + // I don't want to deal with the refactor right now. + get_account(conn, params, account_id)?.expect("account_id exists"), + )), + } +} + pub(crate) fn get_account( conn: &rusqlite::Connection, params: &P, @@ -1840,7 +1877,7 @@ pub(crate) fn get_account( ) -> Result, SqliteClientError> { let mut sql = conn.prepare_cached( r#" - SELECT account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, has_spend_key + SELECT uuid, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, has_spend_key FROM accounts WHERE id = :account_id "#, @@ -1850,6 +1887,8 @@ pub(crate) fn get_account( let row = result.next()?; match row { Some(row) => { + let uuid = AccountUuid(row.get("uuid")?); + let kind = parse_account_source( row.get("account_kind")?, row.get("hd_seed_fingerprint")?, @@ -1873,6 +1912,7 @@ pub(crate) fn get_account( Ok(Some(Account { account_id, + uuid, kind, viewing_key, }))