zec
/
librustzcash
mirror of https://github.com/zcash/librustzcash.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Address comments from code review.

This commit is contained in:
Kris Nuttycombe 2024-02-28 18:41:30 -07:00
parent 41b050f1e9
commit 688c36166a
9 changed files with 38 additions and 124 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
zcash_client_backend/CHANGELOG.md
View File

@ -56,7 +56,6 @@ and this library adheres to Rust's notion of
}`
- `WalletSummary::next_sapling_subtree_index`
- `wallet`:
- `Account`
- `propose_standard_transfer_to_address`
- `create_proposed_transactions`
- `input_selection`:

50
zcash_client_backend/src/data_api.rs
View File

@ -3,12 +3,16 @@
use std::{
collections::HashMap,
fmt::Debug,
hash::Hash,
io,
num::{NonZeroU32, TryFromIntError},
};
use self::scanning::ScanRange;
use self::{chain::CommitmentTreeRoot, wallet::Account};
use incrementalmerkletree::{frontier::Frontier, Retention};
use secrecy::SecretVec;
use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
use self::{chain::CommitmentTreeRoot, scanning::ScanRange};
use crate::{
address::UnifiedAddress,
decrypt::DecryptedOutput,
@ -17,9 +21,6 @@ use crate::{
wallet::{Note, NoteId, ReceivedNote, Recipient, WalletTransparentOutput, WalletTx},
ShieldedProtocol,
};
use incrementalmerkletree::{frontier::Frontier, Retention};
use secrecy::SecretVec;
use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
use zcash_primitives::{
block::BlockHash,
consensus::BlockHeight,
@ -289,7 +290,7 @@ impl<T> Ratio<T> {
/// this circumstance it is possible that a newly created transaction could conflict with a
/// not-yet-mined transaction in the mempool.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct WalletSummary<AccountId: Eq + std::hash::Hash> {
pub struct WalletSummary<AccountId: Eq + Hash> {
account_balances: HashMap<AccountId, AccountBalance>,
chain_tip_height: BlockHeight,
fully_scanned_height: BlockHeight,
@ -297,7 +298,7 @@ pub struct WalletSummary<AccountId: Eq + std::hash::Hash> {
next_sapling_subtree_index: u64,
}
impl<AccountId: Eq + std::hash::Hash> WalletSummary<AccountId> {
impl<AccountId: Eq + Hash> WalletSummary<AccountId> {
/// Constructs a new [`WalletSummary`] from its constituent parts.
pub fn new(
account_balances: HashMap<AccountId, AccountBalance>,
@ -359,13 +360,17 @@ pub trait InputSource {
/// The type of errors produced by a wallet backend.
type Error;
/// The type used to track unique account identifiers.
type AccountId;
/// Backend-specific account identifier.
///
/// An account identifier corresponds to at most a single unified spending key's worth of spend
/// authority, such that both received notes and change spendable by that spending authority
/// will be interpreted as belonging to that account. This might be a database identifier type
/// or a UUID.
type AccountId: Copy + Debug + Eq + Hash;
/// Backend-specific note identifier.
///
/// For example, this might be a database identifier type
/// or a UUID.
/// For example, this might be a database identifier type or a UUID.
type NoteRef: Copy + Debug + Eq + Ord;
/// Fetches a spendable note by indexing into a transaction's shielded outputs for the
@ -426,11 +431,12 @@ pub trait WalletRead {
/// The type of errors that may be generated when querying a wallet data store.
type Error;
/// The type used to track unique account identifiers.
type AccountId: Eq + std::hash::Hash;
/// Gets the parameters that went into creating an account (e.g. seed+index or uvk).
fn get_account(&self, account_id: &Self::AccountId) -> Result<Option<Account>, Self::Error>;
/// The type of the account identifier.
///
/// An account identifier corresponds to at most a single unified spending key's worth of spend
/// authority, such that both received notes and change spendable by that spending authority
/// will be interpreted as belonging to that account.
type AccountId: Copy + Debug + Eq + Hash;
/// Returns the height of the chain as known to the wallet as of the most recent call to
/// [`WalletWrite::update_chain_tip`].
@ -1005,8 +1011,9 @@ pub trait WalletWrite: WalletRead {
/// current set of [ZIP 316] account identifiers known to the wallet database.
///
/// 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.
/// 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
@ -1259,13 +1266,6 @@ pub mod testing {
type Error = ();
type AccountId = u32;
fn get_account(
&self,
_account_id: &Self::AccountId,
) -> Result<Option<super::Account>, Self::Error> {
Ok(None)
}
fn chain_height(&self) -> Result<Option<BlockHeight>, Self::Error> {
Ok(None)
}

2
zcash_client_backend/src/data_api/chain.rs
View File

@ -280,7 +280,7 @@ where
ParamsT: consensus::Parameters + Send + 'static,
BlockSourceT: BlockSource,
DbT: WalletWrite,
<DbT as WalletRead>::AccountId: Clone + ConditionallySelectable + Default + Send + 'static,
<DbT as WalletRead>::AccountId: ConditionallySelectable + Default + Send + 'static,
{
// Fetch the UnifiedFullViewingKeys we are tracking
let ufvks = data_db

67
zcash_client_backend/src/data_api/wallet.rs
View File

@ -6,10 +6,6 @@ use sapling::{
};
use std::num::NonZeroU32;
use zcash_keys::{
address::UnifiedAddress,
keys::{UnifiedAddressRequest, UnifiedFullViewingKey},
};
use zcash_primitives::{
consensus::{self, BlockHeight, NetworkUpgrade},
memo::MemoBytes,
@ -21,7 +17,6 @@ use zcash_primitives::{
},
zip32::Scope,
};
use zip32::DiversifierIndex;
use super::InputSource;
use crate::{
@ -53,45 +48,6 @@ use input_selection::{
GreedyInputSelector, GreedyInputSelectorError, InputSelector, InputSelectorError,
};
/// The inputs for adding an account to a wallet.
#[derive(Debug, Clone)]
pub enum Account {
/// An account that was derived from a ZIP-32 HD seed and account index.
Zip32 {
account_id: zip32::AccountId,
ufvk: UnifiedFullViewingKey,
},
/// An account for which the seed and ZIP-32 account ID are not known.
ImportedUfvk(UnifiedFullViewingKey),
}
impl Account {
/// Gets the default UA for the account.
pub fn default_address(
&self,
request: UnifiedAddressRequest,
) -> (UnifiedAddress, DiversifierIndex) {
match self {
Account::Zip32 { ufvk, .. } => ufvk.default_address(request),
Account::ImportedUfvk(ufvk) => ufvk.default_address(request),
}
}
/// Gets the unified full viewing key for this account, if available.
///
/// Accounts initialized with an incoming viewing key will not have a unified full viewing key.
pub fn ufvk(&self) -> Option<&UnifiedFullViewingKey> {
match self {
Account::Zip32 { ufvk, .. } => Some(ufvk),
Account::ImportedUfvk(ufvk) => Some(ufvk),
}
}
// TODO: When a UnifiedIncomingViewingKey is available, add a function that
// will return it. Even if the Account was initialized with a UFVK, we can always
// derive a UIVK from that.
}
/// Scans a [`Transaction`] for any information that can be decrypted by the accounts in
/// the wallet, and saves it to the wallet.
pub fn decrypt_and_store_transaction<ParamsT, DbT>(
@ -102,7 +58,6 @@ pub fn decrypt_and_store_transaction<ParamsT, DbT>(
where
ParamsT: consensus::Parameters,
DbT: WalletWrite,
<DbT as WalletRead>::AccountId: Clone,
{
// Fetch the UnifiedFullViewingKeys we are tracking
let ufvks = data.get_unified_full_viewing_keys()?;
@ -272,8 +227,6 @@ where
AccountId = <DbT as InputSource>::AccountId,
>,
DbT: WalletCommitmentTrees,
<DbT as InputSource>::AccountId: Clone,
<DbT as InputSource>::NoteRef: Copy + Eq + Ord,
{
let account = wallet_db
.get_account_for_ufvk(&usk.to_unified_full_viewing_key())
@ -384,8 +337,6 @@ where
AccountId = <DbT as InputSource>::AccountId,
>,
DbT: WalletCommitmentTrees,
<DbT as InputSource>::AccountId: Clone,
<DbT as InputSource>::NoteRef: Copy + Eq + Ord,
ParamsT: consensus::Parameters + Clone,
InputsT: InputSelector<InputSource = DbT>,
{
@ -489,7 +440,7 @@ pub fn propose_standard_transfer_to_address<DbT, ParamsT, CommitmentTreeErrT>(
wallet_db: &mut DbT,
params: &ParamsT,
fee_rule: StandardFeeRule,
spend_from_account: <DbT as WalletRead>::AccountId,
spend_from_account: <DbT as InputSource>::AccountId,
min_confirmations: NonZeroU32,
to: &Address,
amount: NonNegativeAmount,
@ -512,7 +463,6 @@ where
Error = <DbT as InputSource>::Error,
AccountId = <DbT as InputSource>::AccountId,
>,
<DbT as WalletRead>::AccountId: Clone,
DbT::NoteRef: Copy + Eq + Ord,
{
let request = zip321::TransactionRequest::new(vec![Payment {
@ -620,7 +570,6 @@ pub fn create_proposed_transactions<DbT, ParamsT, InputsErrT, FeeRuleT, N>(
>
where
DbT: WalletWrite + WalletCommitmentTrees,
<DbT as WalletRead>::AccountId: Clone,
ParamsT: consensus::Parameters + Clone,
FeeRuleT: FeeRule,
{
@ -674,7 +623,6 @@ fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT, N>(
>
where
DbT: WalletWrite + WalletCommitmentTrees,
<DbT as WalletRead>::AccountId: Clone,
ParamsT: consensus::Parameters + Clone,
FeeRuleT: FeeRule,
{
@ -1069,7 +1017,7 @@ where
)?;
sapling_output_meta.push((
Recipient::InternalAccount(
account.clone(),
account,
PoolType::Shielded(ShieldedProtocol::Sapling),
),
change_value.value(),
@ -1092,7 +1040,7 @@ where
)?;
orchard_output_meta.push((
Recipient::InternalAccount(
account.clone(),
account,
PoolType::Shielded(ShieldedProtocol::Orchard),
),
change_value.value(),
@ -1120,7 +1068,7 @@ where
.expect("An action should exist in the transaction for each Orchard output.");
let recipient = recipient
.map_internal_note(|pool| {
.map_internal_account_note(|pool| {
assert!(pool == PoolType::Shielded(ShieldedProtocol::Orchard));
build_result
.transaction()
@ -1131,7 +1079,7 @@ where
.map(|(note, _, _)| Note::Orchard(note))
})
})
.internal_note_transpose_option()
.internal_account_note_transpose_option()
.expect("Wallet-internal outputs must be decryptable with the wallet's IVK");
SentTransactionOutput::from_parts(output_index, recipient, value, memo)
@ -1150,7 +1098,7 @@ where
.expect("An output should exist in the transaction for each Sapling payment.");
let recipient = recipient
.map_internal_note(|pool| {
.map_internal_account_note(|pool| {
assert!(pool == PoolType::Shielded(ShieldedProtocol::Sapling));
build_result
.transaction()
@ -1167,7 +1115,7 @@ where
.map(|(note, _, _)| Note::Sapling(note))
})
})
.internal_note_transpose_option()
.internal_account_note_transpose_option()
.expect("Wallet-internal outputs must be decryptable with the wallet's IVK");
SentTransactionOutput::from_parts(output_index, recipient, value, memo)
@ -1269,7 +1217,6 @@ pub fn shield_transparent_funds<DbT, ParamsT, InputsT>(
where
ParamsT: consensus::Parameters,
DbT: WalletWrite + WalletCommitmentTrees + InputSource<Error = <DbT as WalletRead>::Error>,
<DbT as WalletRead>::AccountId: Clone,
InputsT: ShieldingSelector<InputSource = DbT>,
{
let proposal = propose_shielding(

1
zcash_client_backend/src/data_api/wallet/input_selection.rs
View File

@ -314,7 +314,6 @@ impl<DbT, ChangeT: ChangeStrategy> GreedyInputSelector<DbT, ChangeT> {
impl<DbT, ChangeT> InputSelector for GreedyInputSelector<DbT, ChangeT>
where
DbT: InputSource,
<DbT as InputSource>::AccountId: Clone,
ChangeT: ChangeStrategy,
ChangeT::FeeRule: Clone,
{

6
zcash_client_backend/src/scanning.rs
View File

@ -252,7 +252,7 @@ impl fmt::Display for ScanError {
pub fn scan_block<
P: consensus::Parameters + Send + 'static,
K: ScanningKey,
A: Clone + Default + Eq + Hash + Send + ConditionallySelectable + 'static,
A: Default + Eq + Hash + Send + ConditionallySelectable + 'static,
>(
params: &P,
block: CompactBlock,
@ -283,7 +283,7 @@ pub(crate) fn add_block_to_runner<P, S, T, A>(
P: consensus::Parameters + Send + 'static,
S: Clone + Send + 'static,
T: Tasks<TaggedBatch<A, S>>,
A: Clone + Default + Eq + Send + 'static,
A: Copy + Default + Eq + Send + 'static,
{
let block_hash = block.hash();
let block_height = block.height();
@ -336,7 +336,7 @@ pub(crate) fn scan_block_with_runner<
P: consensus::Parameters + Send + 'static,
K: ScanningKey,
T: Tasks<TaggedBatch<A, K::Scope>> + Sync,
A: Send + Clone + Default + Eq + Hash + ConditionallySelectable + 'static,
A: Send + Default + Eq + Hash + ConditionallySelectable + 'static,
>(
params: &P,
block: CompactBlock,

4
zcash_client_backend/src/wallet.rs
View File

@ -73,7 +73,7 @@ pub enum Recipient<AccountId, N> {
}
impl<AccountId, N> Recipient<AccountId, N> {
pub fn map_internal_note<B, F: FnOnce(N) -> B>(self, f: F) -> Recipient<AccountId, B> {
pub fn map_internal_account_note<B, F: FnOnce(N) -> B>(self, f: F) -> Recipient<AccountId, B> {
match self {
Recipient::Transparent(t) => Recipient::Transparent(t),
Recipient::Sapling(s) => Recipient::Sapling(s),
@ -84,7 +84,7 @@ impl<AccountId, N> Recipient<AccountId, N> {
}
impl<AccountId, N> Recipient<AccountId, Option<N>> {
pub fn internal_note_transpose_option(self) -> Option<Recipient<AccountId, N>> {
pub fn internal_account_note_transpose_option(self) -> Option<Recipient<AccountId, N>> {
match self {
Recipient::Transparent(t) => Some(Recipient::Transparent(t)),
Recipient::Sapling(s) => Some(Recipient::Sapling(s)),

7
zcash_client_sqlite/src/lib.rs
View File

@ -386,13 +386,6 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
fn get_account_ids(&self) -> Result<Vec<AccountId>, Self::Error> {
wallet::get_account_ids(self.conn.borrow())
}
fn get_account(
&self,
account_id: &Self::AccountId,
) -> Result<Option<data_api::wallet::Account>, Self::Error> {
wallet::get_account(self.conn.borrow(), &self.params, *account_id)
}
}
impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P> {

24
zcash_client_sqlite/src/wallet.rs
View File

@ -78,7 +78,6 @@ use zcash_client_backend::{
address::{Address, UnifiedAddress},
data_api::{
scanning::{ScanPriority, ScanRange},
wallet::Account,
AccountBalance, AccountBirthday, BlockMetadata, Ratio, SentTransactionOutput,
WalletSummary, SAPLING_SHARD_HEIGHT,
},
@ -985,29 +984,6 @@ pub(crate) fn block_height_extrema(
})
}
pub(crate) fn get_account<P: Parameters>(
conn: &rusqlite::Connection,
params: &P,
account_id: AccountId,
) -> Result<Option<Account>, SqliteClientError> {
conn.query_row(
r#"
SELECT ufvk
FROM accounts
WHERE id = :account_id
"#,
named_params![":account_id": u32::from(account_id)],
|row| row.get::<_, String>(0),
)
.optional()?
.map(|ufvk_bytes| {
let ufvk = UnifiedFullViewingKey::decode(params, &ufvk_bytes)
.map_err(SqliteClientError::CorruptedData)?;
Ok(Account::Zip32 { account_id, ufvk })
})
.transpose()
}
/// Returns the minimum and maximum heights of blocks in the chain which may be scanned.
pub(crate) fn scan_queue_extrema(
conn: &rusqlite::Connection,