Fixes for nearly all PR comments

This commit is contained in:
Andrew Arnott 2024-07-23 19:47:37 -06:00
parent fa5248def0
commit 664526f74a
No known key found for this signature in database
GPG Key ID: 251505B99C25745D
6 changed files with 49 additions and 33 deletions

View File

@ -26,7 +26,6 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
### Added
- `zcash_client_backend::data_api`:
- `chain::BlockCache` trait, behind the `sync` feature flag.
- `WalletWrite` trait methods `import_account_hd` and `import_account_ufvk`.
- `WalletRead::get_spendable_transparent_outputs`.
- `zcash_client_backend::fees`:
- `EphemeralBalance`
@ -63,6 +62,7 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
methods when the "transparent-inputs" feature is enabled.
- `WalletWrite` has a new `reserve_next_n_ephemeral_addresses` method when
the "transparent-inputs" feature is enabled.
- `WalletWrite` has new methods `import_account_hd` and `import_account_ufvk`.
- `error::Error` has new `Address` and (when the "transparent-inputs" feature
is enabled) `PaysEphemeralTransparentAddress` variants.
- `wallet::input_selection::InputSelectorError` has a new `Address` variant.

View File

@ -1565,6 +1565,13 @@ pub trait WalletWrite: WalletRead {
/// Tells the wallet to track the next available account-level spend authority, given the
/// current set of [ZIP 316] account identifiers known to the wallet database.
///
/// The "next available account" is defined as the first unused ZIP-32 account index (counting from 0)
/// among all accounts that share the given seed.
/// When [`Self::import_account_hd`] is used to import an account with a specific index,
/// account indexes *may* become fragmented instead of one contiguous range.
/// Where fragmentation occurs, the implementations *may* choose to find the first unused
/// account index or add 1 to the highest existing account index.
///
/// Returns the account identifier for the newly-created wallet database entry, along with the
/// associated [`UnifiedSpendingKey`]. Note that the unique account identifier should *not* be
/// assumed equivalent to the ZIP 32 account index. It is an opaque identifier for a pool of
@ -1605,13 +1612,11 @@ pub trait WalletWrite: WalletRead {
/// Tells the wallet to track a specific account index for a given seed.
///
/// Returns the account for the newly-created wallet database entry, along with the
/// associated [`UnifiedSpendingKey`]. Note that the unique account identifier should *not* be
/// assumed equivalent to the ZIP 32 account index. It is an opaque identifier for a pool of
/// funds or set of outputs controlled by a single spending authority.
///
/// The ZIP-32 account index may be obtained by calling [`WalletRead::get_account`]
/// with the returned account identifier.
/// Returns details about the imported account, including the unique account
/// identifier for the newly-created wallet database entry, along with the associated
/// [`UnifiedSpendingKey`]. Note that the unique account identifier should *not* be
/// assumed equivalent to the ZIP 32 account index. It is an opaque identifier for a
/// pool of funds or set of outputs controlled by a single spending authority.
///
/// If `birthday.height()` is below the current chain tip, this operation will
/// trigger a re-scan of the blocks at and above the provided height. The birthday height is
@ -1631,6 +1636,14 @@ pub trait WalletWrite: WalletRead {
/// By convention, wallets should only allow a new account to be generated after confirmed
/// funds have been received by the currently-available account (in order to enable automated
/// account recovery).
/// The [`Self::create_occount`] function helps to conform with this convention by automatically
/// selecting the next unused account index, while *this* function should be used only
/// to import an existing account with a specific index.
///
/// Avoid fragmenting the account indexes by importing accounts with indexes that are one greater
/// than the highest existing account index.
///
/// # Panics
///
/// Panics if the length of the seed is not between 32 and 252 bytes inclusive.
///
@ -1644,7 +1657,11 @@ pub trait WalletWrite: WalletRead {
/// Tells the wallet to track an account using a unified full viewing key.
///
/// Returns the account for the newly-created wallet database entry.
/// Returns details about the imported account, including the unique account
/// identifier for the newly-created wallet database entry. Unlike the other account
/// creation APIs ([`Self::create_account`] and [`Self::import_account_hd`]), no
/// spending key is returned because the wallet has no information about how the UFVK
/// was derived.
///
/// If `birthday.height()` is below the current chain tip, this operation will
/// trigger a re-scan of the blocks at and above the provided height. The birthday height is
@ -1661,6 +1678,8 @@ pub trait WalletWrite: WalletRead {
/// were used for the wallet birthday, a transaction targeted at a height greater than the
/// chain tip could be mined at a height below that tip as part of a reorg.
///
/// # Panics
///
/// Panics if the length of the seed is not between 32 and 252 bytes inclusive.
fn import_account_ufvk(
&mut self,

View File

@ -171,7 +171,7 @@ impl fmt::Display for SqliteClientError {
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::AccountCollision(id) => write!(f, "An account with ID {} already exists in the wallet.", id.0),
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."),
SqliteClientError::CommitmentTree(err) => write!(f, "An error occurred accessing or updating note commitment tree data: {}.", err),

View File

@ -2078,7 +2078,7 @@ mod tests {
pub(crate) fn import_account_hd_0() {
let st = TestBuilder::new()
.with_account_from_sapling_activation(BlockHash([0; 32]))
.with_account_having_index(zip32::AccountId::ZERO)
.set_account_index(zip32::AccountId::ZERO)
.build();
assert_matches!(
st.test_account().unwrap().account().source(),
@ -2095,23 +2095,24 @@ mod tests {
);
let seed = Secret::new(vec![0u8; 32]);
let zip32_index = zip32::AccountId::ZERO.next().unwrap();
let zip32_index_1 = zip32::AccountId::ZERO.next().unwrap();
let first = st
.wallet_mut()
.import_account_hd(&seed, zip32_index, &birthday)
.import_account_hd(&seed, zip32_index_1, &birthday)
.unwrap();
assert_matches!(
first.0.source(),
AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32_index);
AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32_index_1);
let zip32_index_2 = zip32_index_1.next().unwrap();
let second = st
.wallet_mut()
.import_account_hd(&seed, zip32_index.next().unwrap(), &birthday)
.import_account_hd(&seed, zip32_index_2, &birthday)
.unwrap();
assert_matches!(
second.0.source(),
AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32_index.next().unwrap());
AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32_index_2);
}
#[test]
@ -2124,15 +2125,15 @@ mod tests {
);
let seed = Secret::new(vec![0u8; 32]);
let zip32_index = zip32::AccountId::ZERO.next().unwrap();
let zip32_index_1 = zip32::AccountId::ZERO.next().unwrap();
let first = st
.wallet_mut()
.import_account_hd(&seed, zip32_index, &birthday)
.import_account_hd(&seed, zip32_index_1, &birthday)
.unwrap();
assert_matches!(
st.wallet_mut().import_account_hd(&seed, zip32_index, &birthday),
st.wallet_mut().import_account_hd(&seed, zip32_index_1, &birthday),
Err(SqliteClientError::AccountCollision(id)) if id == first.0.id());
}
@ -2158,6 +2159,8 @@ mod tests {
ufvk.encode(&st.wallet().params),
account.ufvk().unwrap().encode(&st.wallet().params)
);
assert_matches!(account.source(), AccountSource::Imported);
}
#[test]

View File

@ -216,7 +216,10 @@ impl<Cache> TestBuilder<Cache> {
self
}
pub(crate) fn with_account_having_index(mut self, index: zip32::AccountId) -> Self {
/// Sets the [`account_index`] field for the test account
///
/// Call either [`with_account_from_sapling_activation`] or [`with_account_having_current_birthday`] before calling this method.
pub(crate) fn set_account_index(mut self, index: zip32::AccountId) -> Self {
assert!(self.account_index.is_none());
self.account_index = Some(index);
self

View File

@ -81,7 +81,7 @@ use zcash_address::ZcashAddress;
use zcash_client_backend::{
data_api::{
scanning::{ScanPriority, ScanRange},
AccountBalance, AccountBirthday, AccountSource, BlockMetadata, Ratio,
Account as _, AccountBalance, AccountBirthday, AccountSource, BlockMetadata, Ratio,
SentTransactionOutput, WalletSummary, SAPLING_SHARD_HEIGHT,
},
encoding::AddressCodec,
@ -204,13 +204,6 @@ impl Account {
) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> {
self.uivk().default_address(request)
}
pub(crate) fn uivk(&self) -> UnifiedIncomingViewingKey {
match &self.viewing_key {
ViewingKey::Full(ufvk) => ufvk.to_unified_incoming_viewing_key(),
ViewingKey::Incoming(uivk) => *uivk.clone(),
}
}
}
impl zcash_client_backend::data_api::Account<AccountId> for Account {
@ -534,7 +527,9 @@ pub(crate) fn add_account<P: consensus::Parameters>(
)?;
}
// Always derive the default Unified Address for the account.
// Always derive the default Unified Address for the account. If the account's viewing
// key has fewer components than the wallet supports (most likely due to this being an
// imported viewing key), derive an address containing the common subset of receivers.
let ua_request = account
.uivk()
.to_address_request()
@ -549,10 +544,6 @@ pub(crate) fn add_account<P: consensus::Parameters>(
#[cfg(feature = "transparent-inputs")]
transparent::ephemeral::init_account(conn, params, account_id)?;
// Initialize the `ephemeral_addresses` table.
#[cfg(feature = "transparent-inputs")]
transparent::ephemeral::init_account(conn, params, account_id)?;
Ok(account)
}