Skip to content

Commit

Permalink
zcash_client_sqlite: Add AccountUuid to expose the uuid column
Browse files Browse the repository at this point in the history
  • Loading branch information
str4d committed Nov 23, 2024
1 parent fd5bb46 commit bf42ec2
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 23 deletions.
9 changes: 9 additions & 0 deletions zcash_client_sqlite/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +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
Expand Down
62 changes: 46 additions & 16 deletions zcash_client_sqlite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use std::{
};
use subtle::ConditionallySelectable;
use tracing::{debug, trace, warn};
use uuid::Uuid;

use zcash_client_backend::{
address::UnifiedAddress,
Expand Down Expand Up @@ -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)

Check warning on line 191 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L190-L191

Added lines #L190 - L191 were not covered by tests
}

/// 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 {

Check warning on line 195 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L195

Added line #L195 was not covered by tests
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(
Expand Down Expand Up @@ -219,6 +238,17 @@ pub struct WalletDb<C, P> {
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(

Check warning on line 244 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L244

Added line #L244 was not covered by tests
&self,
account_uuid: AccountUuid,
) -> Result<Option<wallet::Account>, SqliteClientError> {
wallet::get_account_for_uuid(self.conn.borrow(), &self.params, account_uuid)

Check warning on line 248 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L248

Added line #L248 was not covered by tests
}
}

/// A wrapper for a SQLite transaction affecting the wallet database.
pub struct SqlTransaction<'conn>(pub(crate) &'conn rusqlite::Transaction<'conn>);

Expand Down
54 changes: 47 additions & 7 deletions zcash_client_sqlite/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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

Check warning on line 212 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L211-L212

Added lines #L211 - L212 were not covered by tests
}

/// Returns the default Unified Address for the account,
/// along with the diversifier index that generated it.
///
Expand Down Expand Up @@ -368,6 +374,8 @@ pub(crate) fn add_account<P: consensus::Parameters>(
}
// 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,
Expand Down Expand Up @@ -425,7 +433,7 @@ pub(crate) fn add_account<P: consensus::Parameters>(
RETURNING id;
"#,
named_params![
":uuid": Uuid::new_v4(),
":uuid": uuid.0,

Check warning on line 436 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L436

Added line #L436 was not covered by tests
":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),
Expand Down Expand Up @@ -465,6 +473,7 @@ pub(crate) fn add_account<P: consensus::Parameters>(

let account = Account {
account_id,
uuid,
kind,
viewing_key,
};
Expand Down Expand Up @@ -709,7 +718,7 @@ pub(crate) fn get_account_for_ufvk<P: consensus::Parameters>(
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
Expand All @@ -725,6 +734,7 @@ pub(crate) fn get_account_for_ufvk<P: consensus::Parameters>(
],
|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")?,
Expand All @@ -746,6 +756,7 @@ pub(crate) fn get_account_for_ufvk<P: consensus::Parameters>(

Ok(Account {
account_id,
uuid,
kind,
viewing_key,
})
Expand All @@ -771,7 +782,7 @@ pub(crate) fn get_derived_account<P: consensus::Parameters>(
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",
Expand All @@ -783,8 +794,9 @@ pub(crate) fn get_derived_account<P: consensus::Parameters>(
":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")? {

Check warning on line 799 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L797-L799

Added lines #L797 - L799 were not covered by tests
None => Err(SqliteClientError::CorruptedData(format!(
"Missing unified full viewing key for derived account {:?}",
account_id,
Expand All @@ -798,6 +810,7 @@ pub(crate) fn get_derived_account<P: consensus::Parameters>(
}?;
Ok(Account {
account_id,
uuid,

Check warning on line 813 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L813

Added line #L813 was not covered by tests
kind: AccountSource::Derived {
seed_fingerprint: *seed,
account_index,
Expand Down Expand Up @@ -1833,14 +1846,38 @@ pub(crate) fn block_height_extrema(
})
}

pub(crate) fn get_account_for_uuid<P: Parameters>(

Check warning on line 1849 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L1849

Added line #L1849 was not covered by tests
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),

Check warning on line 1858 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L1854-L1858

Added lines #L1854 - L1858 were not covered by tests
)
.optional()?

Check warning on line 1860 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L1860

Added line #L1860 was not covered by tests
{
None => Ok(None),
Some(account_id) => Ok(Some(

Check warning on line 1863 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L1862-L1863

Added lines #L1862 - L1863 were not covered by tests
// 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"),

Check warning on line 1868 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L1868

Added line #L1868 was not covered by tests
)),
}
}

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
"#,
Expand All @@ -1850,6 +1887,8 @@ pub(crate) fn get_account<P: Parameters>(
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")?,
Expand All @@ -1873,6 +1912,7 @@ pub(crate) fn get_account<P: Parameters>(

Ok(Some(Account {
account_id,
uuid,

Check warning on line 1915 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L1915

Added line #L1915 was not covered by tests
kind,
viewing_key,
}))
Expand Down

0 comments on commit bf42ec2

Please sign in to comment.