zcash_client_sqlite: Assign UUIDs to each account

zcash_client_sqlite/CHANGELOG.md

@@ -7,6 +7,22 @@ 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`.
+- The `v_tx_outputs` view has been modified:
+  - 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

zcash_client_sqlite/Cargo.toml

@@ -66,11 +66,11 @@ incrementalmerkletree.workspace = true
 shardtree = { workspace = true, features = ["legacy-api"] }
 
 # - SQLite databases
-rusqlite = { workspace = true, features = ["time", "array"] }
+rusqlite = { workspace = true, features = ["time", "array", "uuid"] }
 schemerz.workspace = true
 schemerz-rusqlite.workspace = true
 time.workspace = true
-uuid.workspace = true
+uuid = { workspace = true, features = ["v4"] }
 regex = "1.4"
 
 # Dependencies used internally:

zcash_client_sqlite/src/lib.rs

@@ -43,6 +43,7 @@
 };
 use subtle::ConditionallySelectable;
 use tracing::{debug, trace, warn};
+use uuid::Uuid;
 
 use zcash_client_backend::{
     address::UnifiedAddress,
@@ -161,30 +162,48 @@
 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 @@
     params: P,
 }
 
+impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletDb<C, P> {
+    /// 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<Option<wallet::Account>, 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>);
 

zcash_client_sqlite/src/wallet.rs

@@ -69,6 +69,7 @@
 use rusqlite::{self, named_params, params, OptionalExtension};
 use secrecy::{ExposeSecret, SecretVec};
 use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
+use uuid::Uuid;
 use zcash_client_backend::data_api::{
     AccountPurpose, DecryptedTransaction, Progress, TransactionDataRequest, TransactionStatus,
 };
@@ -119,7 +120,7 @@
     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;
@@ -200,11 +201,17 @@
 #[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.
     ///
@@ -367,6 +374,8 @@
     }
     // 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,
@@ -404,6 +413,7 @@
         .query_row(
             r#"
             INSERT INTO accounts (
+                uuid,
                 account_kind, hd_seed_fingerprint, hd_account_index,
                 ufvk, uivk,
                 orchard_fvk_item_cache, sapling_fvk_item_cache, p2pkh_fvk_item_cache,
@@ -412,6 +422,7 @@
                 has_spend_key
             )
             VALUES (
+                :uuid,
                 :account_kind, :hd_seed_fingerprint, :hd_account_index,
                 :ufvk, :uivk,
                 :orchard_fvk_item_cache, :sapling_fvk_item_cache, :p2pkh_fvk_item_cache,
@@ -422,6 +433,7 @@
             RETURNING id;
             "#,
             named_params![
+                ":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),
@@ -461,6 +473,7 @@
 
     let account = Account {
         account_id,
+        uuid,
         kind,
         viewing_key,
     };
@@ -705,7 +718,7 @@
     let transparent_item: Option<Vec<u8>> = 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
@@ -721,6 +734,7 @@
             ],
             |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")?,
@@ -742,6 +756,7 @@
 
                 Ok(Account {
                     account_id,
+                    uuid,
                     kind,
                     viewing_key,
                 })
@@ -767,7 +782,7 @@
     account_index: zip32::AccountId,
 ) -> Result<Option<Account>, 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",
@@ -779,8 +794,9 @@
             ":hd_account_index": u32::from(account_index),
         ],
         |row| {
-            let account_id = row.get::<_, u32>(0).map(AccountId)?;
-            let ufvk = match row.get::<_, Option<String>>(1)? {
+            let account_id = row.get::<_, u32>("id").map(AccountId)?;
+            let uuid = AccountUuid(row.get("uuid")?);
+            let ufvk = match row.get::<_, Option<String>>("ufvk")? {
                 None => Err(SqliteClientError::CorruptedData(format!(
                     "Missing unified full viewing key for derived account {:?}",
                     account_id,
@@ -794,6 +810,7 @@
             }?;
             Ok(Account {
                 account_id,
+                uuid,
                 kind: AccountSource::Derived {
                     seed_fingerprint: *seed,
                     account_index,
@@ -1829,14 +1846,38 @@
     })
 }
 
+pub(crate) fn get_account_for_uuid<P: Parameters>(
+    conn: &rusqlite::Connection,
+    params: &P,
+    account_uuid: AccountUuid,
+) -> Result<Option<Account>, 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<P: Parameters>(
     conn: &rusqlite::Connection,
     params: &P,
     account_id: AccountId,
 ) -> Result<Option<Account>, 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
         "#,
@@ -1846,6 +1887,8 @@
     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")?,
@@ -1869,6 +1912,7 @@
 
             Ok(Some(Account {
                 account_id,
+                uuid,
                 kind,
                 viewing_key,
             }))
@@ -3526,8 +3570,9 @@
         conn: &rusqlite::Connection,
     ) -> Result<Vec<TransactionSummary<AccountId>>, SqliteClientError> {
         let mut stmt = conn.prepare_cached(
-            "SELECT *
+            "SELECT accounts.id as account_id, v_transactions.*
              FROM v_transactions
+             JOIN accounts ON accounts.uuid = v_transactions.account_uuid
              ORDER BY mined_height DESC, tx_index DESC",
         )?;