Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zcash_client_backend: Post-merge cleanups to test framework extraction #1539

Merged
merged 3 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions zcash_client_backend/src/data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1166,13 +1166,20 @@ pub trait WalletRead {
/// transaction data requests, such as when it is necessary to fill in purely-transparent
/// transaction history by walking the chain backwards via transparent inputs.
fn transaction_data_requests(&self) -> Result<Vec<TransactionDataRequest>, Self::Error>;
}

/// Read-only operations required for testing light wallet functions.
///
/// These methods expose internal details or unstable interfaces, primarily to enable use
/// of the [`testing`] framework. They should not be used in production software.
#[cfg(any(test, feature = "test-dependencies"))]
#[delegatable_trait]
pub trait WalletTest: WalletRead {
/// Returns a vector of transaction summaries.
///
/// Currently test-only, as production use could return a very large number of results; either
/// pagination or a streaming design will be necessary to stabilize this feature for production
/// use.
#[cfg(any(test, feature = "test-dependencies"))]
fn get_tx_history(
&self,
) -> Result<Vec<testing::TransactionSummary<Self::AccountId>>, Self::Error> {
Expand All @@ -1181,7 +1188,6 @@ pub trait WalletRead {

/// Returns the note IDs for shielded notes sent by the wallet in a particular
/// transaction.
#[cfg(any(test, feature = "test-dependencies"))]
fn get_sent_note_ids(
&self,
_txid: &TxId,
Expand Down
27 changes: 21 additions & 6 deletions zcash_client_backend/src/data_api/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ use crate::{
ShieldedProtocol,
};

use super::WalletTest;
#[allow(deprecated)]
use super::{
chain::{scan_cached_blocks, BlockSource, ChainState, CommitmentTreeRoot, ScanSummary},
Expand Down Expand Up @@ -319,14 +320,14 @@ impl<A: Account> Account for TestAccount<A> {
}
}

pub trait Reset: WalletRead + Sized {
pub trait Reset: WalletTest + Sized {
type Handle;

fn reset<C>(st: &mut TestState<C, Self, LocalNetwork>) -> Self::Handle;
}

/// The state for a `zcash_client_sqlite` test.
pub struct TestState<Cache, DataStore: WalletRead, Network> {
pub struct TestState<Cache, DataStore: WalletTest, Network> {
cache: Cache,
cached_blocks: BTreeMap<BlockHeight, CachedBlock>,
latest_block_height: Option<BlockHeight>,
Expand All @@ -336,7 +337,7 @@ pub struct TestState<Cache, DataStore: WalletRead, Network> {
rng: ChaChaRng,
}

impl<Cache, DataStore: WalletRead, Network> TestState<Cache, DataStore, Network> {
impl<Cache, DataStore: WalletTest, Network> TestState<Cache, DataStore, Network> {
/// Exposes an immutable reference to the test's `DataStore`.
pub fn wallet(&self) -> &DataStore {
&self.wallet_data
Expand All @@ -358,7 +359,7 @@ impl<Cache, DataStore: WalletRead, Network> TestState<Cache, DataStore, Network>
}
}

impl<Cache, DataStore: WalletRead, Network: consensus::Parameters>
impl<Cache, DataStore: WalletTest, Network: consensus::Parameters>
TestState<Cache, DataStore, Network>
{
/// Convenience method for obtaining the Sapling activation height for the network under test.
Expand Down Expand Up @@ -405,7 +406,7 @@ impl<Cache, DataStore: WalletRead, Network: consensus::Parameters>
impl<Cache: TestCache, DataStore, Network> TestState<Cache, DataStore, Network>
where
Network: consensus::Parameters,
DataStore: WalletWrite,
DataStore: WalletTest + WalletWrite,
<Cache::BlockSource as BlockSource>::Error: fmt::Debug,
{
/// Exposes an immutable reference to the test's [`BlockSource`].
Expand Down Expand Up @@ -697,7 +698,7 @@ where
Cache: TestCache,
<Cache::BlockSource as BlockSource>::Error: fmt::Debug,
ParamsT: consensus::Parameters + Send + 'static,
DbT: InputSource + WalletWrite + WalletCommitmentTrees,
DbT: InputSource + WalletTest + WalletWrite + WalletCommitmentTrees,
<DbT as WalletRead>::AccountId: ConditionallySelectable + Default + Send + 'static,
{
/// Invokes [`scan_cached_blocks`] with the given arguments, expecting success.
Expand Down Expand Up @@ -760,6 +761,7 @@ where
AccountIdT: std::cmp::Eq + std::hash::Hash,
ErrT: std::fmt::Debug,
DbT: InputSource<AccountId = AccountIdT, Error = ErrT>
+ WalletTest
+ WalletWrite<AccountId = AccountIdT, Error = ErrT>
+ WalletCommitmentTrees,
<DbT as WalletRead>::AccountId: ConditionallySelectable + Default + Send + 'static,
Expand Down Expand Up @@ -1069,7 +1071,19 @@ where
pub fn get_wallet_summary(&self, min_confirmations: u32) -> Option<WalletSummary<AccountIdT>> {
self.wallet().get_wallet_summary(min_confirmations).unwrap()
}
}

impl<Cache, DbT, ParamsT, AccountIdT, ErrT> TestState<Cache, DbT, ParamsT>
where
ParamsT: consensus::Parameters + Send + 'static,
AccountIdT: std::cmp::Eq + std::hash::Hash,
ErrT: std::fmt::Debug,
DbT: InputSource<AccountId = AccountIdT, Error = ErrT>
+ WalletTest
+ WalletWrite<AccountId = AccountIdT, Error = ErrT>
+ WalletCommitmentTrees,
<DbT as WalletRead>::AccountId: ConditionallySelectable + Default + Send + 'static,
{
/// Returns a transaction from the history.
#[allow(dead_code)]
pub fn get_tx_from_history(
Expand Down Expand Up @@ -1149,6 +1163,7 @@ pub trait DataStoreFactory {
type DsError: core::fmt::Debug;
type DataStore: InputSource<AccountId = Self::AccountId, Error = Self::DsError>
+ WalletRead<AccountId = Self::AccountId, Account = Self::Account, Error = Self::DsError>
+ WalletTest
+ WalletWrite
+ WalletCommitmentTrees;

Expand Down
8 changes: 4 additions & 4 deletions zcash_client_backend/src/data_api/testing/orchard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::{
data_api::{
chain::{CommitmentTreeRoot, ScanSummary},
testing::{pool::ShieldedPoolTester, TestState},
DecryptedTransaction, InputSource, WalletCommitmentTrees, WalletRead, WalletSummary,
DecryptedTransaction, InputSource, WalletCommitmentTrees, WalletSummary, WalletTest,
},
wallet::{Note, ReceivedNote},
};
Expand All @@ -40,7 +40,7 @@ impl ShieldedPoolTester for OrchardPoolTester {
type MerkleTreeHash = MerkleHashOrchard;
type Note = orchard::note::Note;

fn test_account_fvk<Cache, DbT: WalletRead, P: consensus::Parameters>(
fn test_account_fvk<Cache, DbT: WalletTest, P: consensus::Parameters>(
st: &TestState<Cache, DbT, P>,
) -> Self::Fvk {
st.test_account_orchard().unwrap().clone()
Expand Down Expand Up @@ -90,7 +90,7 @@ impl ShieldedPoolTester for OrchardPoolTester {
MerkleHashOrchard::empty_root(level)
}

fn put_subtree_roots<Cache, DbT: WalletRead + WalletCommitmentTrees, P>(
fn put_subtree_roots<Cache, DbT: WalletTest + WalletCommitmentTrees, P>(
st: &mut TestState<Cache, DbT, P>,
start_index: u64,
roots: &[CommitmentTreeRoot<Self::MerkleTreeHash>],
Expand All @@ -103,7 +103,7 @@ impl ShieldedPoolTester for OrchardPoolTester {
s.next_orchard_subtree_index()
}

fn select_spendable_notes<Cache, DbT: InputSource + WalletRead, P>(
fn select_spendable_notes<Cache, DbT: InputSource + WalletTest, P>(
st: &TestState<Cache, DbT, P>,
account: <DbT as InputSource>::AccountId,
target_value: Zatoshis,
Expand Down
8 changes: 4 additions & 4 deletions zcash_client_backend/src/data_api/testing/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::{
testing::{AddressType, TestBuilder},
wallet::{decrypt_and_store_transaction, input_selection::GreedyInputSelector},
Account as _, DecryptedTransaction, InputSource, WalletCommitmentTrees, WalletRead,
WalletSummary,
WalletSummary, WalletTest,
},
decrypt_transaction,
fees::{standard, DustOutputPolicy},
Expand All @@ -42,7 +42,7 @@ pub trait ShieldedPoolTester {
type MerkleTreeHash;
type Note;

fn test_account_fvk<Cache, DbT: WalletRead, P: consensus::Parameters>(
fn test_account_fvk<Cache, DbT: WalletTest, P: consensus::Parameters>(
st: &TestState<Cache, DbT, P>,
) -> Self::Fvk;
fn usk_to_sk(usk: &UnifiedSpendingKey) -> &Self::Sk;
Expand All @@ -68,7 +68,7 @@ pub trait ShieldedPoolTester {
fn empty_tree_leaf() -> Self::MerkleTreeHash;
fn empty_tree_root(level: Level) -> Self::MerkleTreeHash;

fn put_subtree_roots<Cache, DbT: WalletRead + WalletCommitmentTrees, P>(
fn put_subtree_roots<Cache, DbT: WalletTest + WalletCommitmentTrees, P>(
st: &mut TestState<Cache, DbT, P>,
start_index: u64,
roots: &[CommitmentTreeRoot<Self::MerkleTreeHash>],
Expand All @@ -77,7 +77,7 @@ pub trait ShieldedPoolTester {
fn next_subtree_index<A: Hash + Eq>(s: &WalletSummary<A>) -> u64;

#[allow(clippy::type_complexity)]
fn select_spendable_notes<Cache, DbT: InputSource + WalletRead, P>(
fn select_spendable_notes<Cache, DbT: InputSource + WalletTest, P>(
st: &TestState<Cache, DbT, P>,
account: <DbT as InputSource>::AccountId,
target_value: Zatoshis,
Expand Down
8 changes: 4 additions & 4 deletions zcash_client_backend/src/data_api/testing/sapling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use zip32::Scope;
use crate::{
data_api::{
chain::{CommitmentTreeRoot, ScanSummary},
DecryptedTransaction, InputSource, WalletCommitmentTrees, WalletRead, WalletSummary,
DecryptedTransaction, InputSource, WalletCommitmentTrees, WalletSummary, WalletTest,
},
wallet::{Note, ReceivedNote},
};
Expand All @@ -36,7 +36,7 @@ impl ShieldedPoolTester for SaplingPoolTester {
type MerkleTreeHash = sapling::Node;
type Note = sapling::Note;

fn test_account_fvk<Cache, DbT: WalletRead, P: consensus::Parameters>(
fn test_account_fvk<Cache, DbT: WalletTest, P: consensus::Parameters>(
st: &TestState<Cache, DbT, P>,
) -> Self::Fvk {
st.test_account_sapling().unwrap().clone()
Expand Down Expand Up @@ -74,7 +74,7 @@ impl ShieldedPoolTester for SaplingPoolTester {
::sapling::Node::empty_root(level)
}

fn put_subtree_roots<Cache, DbT: WalletRead + WalletCommitmentTrees, P>(
fn put_subtree_roots<Cache, DbT: WalletTest + WalletCommitmentTrees, P>(
st: &mut TestState<Cache, DbT, P>,
start_index: u64,
roots: &[CommitmentTreeRoot<Self::MerkleTreeHash>],
Expand All @@ -87,7 +87,7 @@ impl ShieldedPoolTester for SaplingPoolTester {
s.next_sapling_subtree_index()
}

fn select_spendable_notes<Cache, DbT: InputSource + WalletRead, P>(
fn select_spendable_notes<Cache, DbT: InputSource + WalletTest, P>(
st: &TestState<Cache, DbT, P>,
account: <DbT as InputSource>::AccountId,
target_value: Zatoshis,
Expand Down
12 changes: 7 additions & 5 deletions zcash_client_sqlite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ use zcash_client_backend::{
Account, AccountBirthday, AccountPurpose, AccountSource, BlockMetadata,
DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SeedRelevance,
SentTransaction, SpendableNotes, TransactionDataRequest, WalletCommitmentTrees, WalletRead,
WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT,
WalletSummary, WalletTest, WalletWrite, SAPLING_SHARD_HEIGHT,
},
keys::{
AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey,
Expand Down Expand Up @@ -614,13 +614,14 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W

Ok(iter.collect())
}
}

#[cfg(any(test, feature = "test-dependencies"))]
#[cfg(any(test, feature = "test-dependencies"))]
impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletTest for WalletDb<C, P> {
fn get_tx_history(&self) -> Result<Vec<TransactionSummary<Self::AccountId>>, Self::Error> {
wallet::testing::get_tx_history(self.conn.borrow())
}

#[cfg(any(test, feature = "test-dependencies"))]
fn get_sent_note_ids(
&self,
txid: &TxId,
Expand Down Expand Up @@ -1725,7 +1726,8 @@ mod tests {
use zcash_client_backend::data_api::{
chain::ChainState,
testing::{TestBuilder, TestState},
Account, AccountBirthday, AccountPurpose, AccountSource, WalletRead, WalletWrite,
Account, AccountBirthday, AccountPurpose, AccountSource, WalletRead, WalletTest,
WalletWrite,
};
use zcash_keys::keys::{UnifiedFullViewingKey, UnifiedSpendingKey};
use zcash_primitives::block::BlockHash;
Expand Down Expand Up @@ -1837,7 +1839,7 @@ mod tests {
AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32_index_2);
}

fn check_collisions<C, DbT: WalletWrite, P: consensus::Parameters>(
fn check_collisions<C, DbT: WalletTest + WalletWrite, P: consensus::Parameters>(
st: &mut TestState<C, DbT, P>,
ufvk: &UnifiedFullViewingKey,
birthday: &AccountBirthday,
Expand Down
1 change: 1 addition & 0 deletions zcash_client_sqlite/src/testing/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use {
#[derive(Delegate)]
#[delegate(InputSource, target = "wallet_db")]
#[delegate(WalletRead, target = "wallet_db")]
#[delegate(WalletTest, target = "wallet_db")]
#[delegate(WalletWrite, target = "wallet_db")]
#[delegate(WalletCommitmentTrees, target = "wallet_db")]
pub(crate) struct TestDb {
Expand Down
2 changes: 1 addition & 1 deletion zcash_client_sqlite/src/testing/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {

use rand_core::OsRng;
use zcash_client_backend::{
data_api::{TransactionDataRequest, TransactionStatus},
data_api::{TransactionDataRequest, TransactionStatus, WalletTest},
fees::ChangeValue,
wallet::TransparentAddressMetadata,
};
Expand Down
Loading