Skip to content

Commit

Permalink
zcash_client_sqlite: Change WalletDb::AccountId associated type to …
Browse files Browse the repository at this point in the history
…`AccountUuid`

This requires a few annoying changes to migrations in order to avoid
hitting cases where account UUIDs are expected before they exist in the
database schema.
  • Loading branch information
nuttycom committed Nov 27, 2024
1 parent bf42ec2 commit 4885b4d
Show file tree
Hide file tree
Showing 16 changed files with 576 additions and 409 deletions.
7 changes: 4 additions & 3 deletions zcash_client_backend/src/data_api/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,8 @@ where
<Cache::BlockSource as BlockSource>::Error: fmt::Debug,
ParamsT: consensus::Parameters + Send + 'static,
DbT: InputSource + WalletTest + WalletWrite + WalletCommitmentTrees,
<DbT as WalletRead>::AccountId: ConditionallySelectable + Default + Send + 'static,
<DbT as WalletRead>::AccountId:
std::fmt::Debug + ConditionallySelectable + Default + Send + 'static,
{
/// Invokes [`scan_cached_blocks`] with the given arguments, expecting success.
pub fn scan_cached_blocks(&mut self, from_height: BlockHeight, limit: usize) -> ScanSummary {
Expand Down Expand Up @@ -855,7 +856,7 @@ where
impl<Cache, DbT, ParamsT, AccountIdT, ErrT> TestState<Cache, DbT, ParamsT>
where
ParamsT: consensus::Parameters + Send + 'static,
AccountIdT: std::cmp::Eq + std::hash::Hash,
AccountIdT: std::fmt::Debug + std::cmp::Eq + std::hash::Hash,
ErrT: std::fmt::Debug,
DbT: InputSource<AccountId = AccountIdT, Error = ErrT>
+ WalletTest
Expand Down Expand Up @@ -1286,7 +1287,7 @@ pub struct InitialChainState {
/// Trait representing the ability to construct a new data store for use in a test.
pub trait DataStoreFactory {
type Error: core::fmt::Debug;
type AccountId: ConditionallySelectable + Default + Hash + Eq + Send + 'static;
type AccountId: std::fmt::Debug + ConditionallySelectable + Default + Hash + Eq + Send + 'static;
type Account: Account<AccountId = Self::AccountId> + Clone;
type DsError: core::fmt::Debug;
type DataStore: InputSource<AccountId = Self::AccountId, Error = Self::DsError>
Expand Down
20 changes: 10 additions & 10 deletions zcash_client_backend/src/data_api/testing/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1559,7 +1559,7 @@ pub fn external_address_change_spends_detected_in_restore_from_seed<T: ShieldedP
// Add two accounts to the wallet.
let seed = Secret::new([0u8; 32].to_vec());
let birthday = AccountBirthday::from_sapling_activation(st.network(), BlockHash([0; 32]));
let (account_id, usk) = st.wallet_mut().create_account(&seed, &birthday).unwrap();
let (account1, usk) = st.wallet_mut().create_account(&seed, &birthday).unwrap();
let dfvk = T::sk_to_fvk(T::usk_to_sk(&usk));

let (account2, usk2) = st.wallet_mut().create_account(&seed, &birthday).unwrap();
Expand All @@ -1571,8 +1571,8 @@ pub fn external_address_change_spends_detected_in_restore_from_seed<T: ShieldedP
st.scan_cached_blocks(h, 1);

// Spendable balance matches total balance
assert_eq!(st.get_total_balance(account_id), value);
assert_eq!(st.get_spendable_balance(account_id, 1), value);
assert_eq!(st.get_total_balance(account1), value);
assert_eq!(st.get_spendable_balance(account1, 1), value);
assert_eq!(st.get_total_balance(account2), NonNegativeAmount::ZERO);

let amount_sent = NonNegativeAmount::from_u64(20000).unwrap();
Expand Down Expand Up @@ -1610,35 +1610,35 @@ pub fn external_address_change_spends_detected_in_restore_from_seed<T: ShieldedP
let pending_change = (amount_left - amount_legacy_change).unwrap();

// The "legacy change" is not counted by get_pending_change().
assert_eq!(st.get_pending_change(account_id, 1), pending_change);
assert_eq!(st.get_pending_change(account1, 1), pending_change);
// We spent the only note so we only have pending change.
assert_eq!(st.get_total_balance(account_id), pending_change);
assert_eq!(st.get_total_balance(account1), pending_change);

let (h, _) = st.generate_next_block_including(txid);
st.scan_cached_blocks(h, 1);

assert_eq!(st.get_total_balance(account2), amount_sent,);
assert_eq!(st.get_total_balance(account_id), amount_left);
assert_eq!(st.get_total_balance(account1), amount_left);

st.reset();

// Account creation and DFVK derivation should be deterministic.
let (_, restored_usk) = st.wallet_mut().create_account(&seed, &birthday).unwrap();
let (account1, restored_usk) = st.wallet_mut().create_account(&seed, &birthday).unwrap();
assert!(T::fvks_equal(
&T::sk_to_fvk(T::usk_to_sk(&restored_usk)),
&dfvk,
));

let (_, restored_usk2) = st.wallet_mut().create_account(&seed, &birthday).unwrap();
let (account2, restored_usk2) = st.wallet_mut().create_account(&seed, &birthday).unwrap();
assert!(T::fvks_equal(
&T::sk_to_fvk(T::usk_to_sk(&restored_usk2)),
&dfvk2,
));

st.scan_cached_blocks(st.sapling_activation_height(), 2);

assert_eq!(st.get_total_balance(account2), amount_sent,);
assert_eq!(st.get_total_balance(account_id), amount_left);
assert_eq!(st.get_total_balance(account2), amount_sent);
assert_eq!(st.get_total_balance(account1), amount_left);
}

#[allow(dead_code)]
Expand Down
18 changes: 6 additions & 12 deletions zcash_client_sqlite/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ use zcash_keys::keys::AddressGenerationError;
use zcash_primitives::zip32;
use zcash_primitives::{consensus::BlockHeight, transaction::components::amount::BalanceError};

use crate::wallet::commitment_tree;
use crate::AccountId;
use crate::{wallet::commitment_tree, AccountUuid};

#[cfg(feature = "transparent-inputs")]
use {
Expand Down Expand Up @@ -79,7 +78,7 @@ pub enum SqliteClientError {

/// The account being added collides with an existing account in the wallet with the given ID.
/// The collision can be on the seed and ZIP-32 account index, or a shared FVK component.
AccountCollision(AccountId),
AccountCollision(AccountUuid),

/// The account was imported, and ZIP-32 derivation information is not known for it.
UnknownZip32Derivation,
Expand All @@ -90,12 +89,8 @@ pub enum SqliteClientError {
/// An error occurred while processing an account due to a failure in deriving the account's keys.
BadAccountData(String),

/// A caller attempted to initialize the accounts table with a discontinuous
/// set of account identifiers.
AccountIdDiscontinuity,

/// A caller attempted to construct a new account with an invalid account identifier.
AccountIdOutOfRange,
/// A caller attempted to construct a new account with an invalid ZIP 32 account identifier.
Zip32AccountIndexOutOfRange,

/// The address associated with a record being inserted was not recognized as
/// belonging to the wallet.
Expand Down Expand Up @@ -129,7 +124,7 @@ pub enum SqliteClientError {
/// ephemeral address outputs have been mined. The parameters are the account id and
/// the index that could not safely be reserved.
#[cfg(feature = "transparent-inputs")]
ReachedGapLimit(AccountId, u32),
ReachedGapLimit(AccountUuid, u32),

/// An ephemeral address would be reused. The parameters are the address in string
/// form, and the txid of the earliest transaction in which it is known to have been
Expand Down Expand Up @@ -181,8 +176,7 @@ impl fmt::Display for SqliteClientError {
SqliteClientError::UnknownZip32Derivation => write!(f, "ZIP-32 derivation information is not known for this account."),
SqliteClientError::KeyDerivationError(acct_id) => write!(f, "Key derivation failed for account {}", u32::from(*acct_id)),
SqliteClientError::BadAccountData(e) => write!(f, "Failed to add account: {}", e),
SqliteClientError::AccountIdDiscontinuity => write!(f, "Wallet account identifiers must be sequential."),
SqliteClientError::AccountIdOutOfRange => write!(f, "Wallet account identifiers must be less than 0x7FFFFFFF."),
SqliteClientError::Zip32AccountIndexOutOfRange => write!(f, "ZIP 32 account identifiers must be less than 0x7FFFFFFF."),
SqliteClientError::AccountCollision(id) => write!(f, "An account corresponding to the data provided already exists in the wallet with internal identifier {}.", id.0),
#[cfg(feature = "transparent-inputs")]
SqliteClientError::AddressNotRecognized(_) => write!(f, "The address associated with a received txo is not identifiable as belonging to the wallet."),
Expand Down
Loading

0 comments on commit 4885b4d

Please sign in to comment.