Apply suggestions from code review

Co-authored-by: str4d <thestr4d@gmail.com>
This commit is contained in:
Kris Nuttycombe 2023-08-29 16:32:18 -06:00 committed by Kris Nuttycombe
parent ff8104fa75
commit 22b6ddd754
5 changed files with 45 additions and 43 deletions

View File

@ -466,6 +466,8 @@ impl SentTransactionOutput {
}
}
/// A data structure used to set the birthday height for an account, and ensure that the initial
/// note commitment tree state is recorded at that height.
#[derive(Clone, Debug)]
pub struct AccountBirthday {
height: BlockHeight,
@ -473,6 +475,12 @@ pub struct AccountBirthday {
}
impl AccountBirthday {
/// Constructs a new [`AccountBirthday`] from its constituent parts.
///
/// * `height`: The birthday height of the account. This is defined as the height of the last
/// block that is known to contain no transactions sent to addresses belonging to the account.
/// * `sapling_frontier`: The Sapling note commitment tree frontier as of the end of the block
/// at `height`.
pub fn from_parts(
height: BlockHeight,
sapling_frontier: Frontier<Node, NOTE_COMMITMENT_TREE_DEPTH>,
@ -483,10 +491,13 @@ impl AccountBirthday {
}
}
/// Returns the Sapling note commitment tree frontier as of the end of the block at
/// [`Self::height`].
pub fn sapling_frontier(&self) -> &Frontier<Node, NOTE_COMMITMENT_TREE_DEPTH> {
&self.sapling_frontier
}
/// Returns the birthday height of the account.
pub fn height(&self) -> BlockHeight {
self.height
}

View File

@ -136,7 +136,6 @@ impl<Cache> TestBuilder<Cache> {
};
TestState {
params: self.network,
cache: self.cache,
latest_cached_block: None,
_data_file: data_file,
@ -148,7 +147,6 @@ impl<Cache> TestBuilder<Cache> {
/// The state for a `zcash_client_sqlite` test.
pub(crate) struct TestState<Cache> {
params: Network,
cache: Cache,
latest_cached_block: Option<(BlockHeight, BlockHash, u32)>,
_data_file: NamedTempFile,
@ -177,15 +175,7 @@ where
let (height, prev_hash, initial_sapling_tree_size) = self
.latest_cached_block
.map(|(prev_height, prev_hash, end_size)| (prev_height + 1, prev_hash, end_size))
.unwrap_or_else(|| {
(
self.params
.activation_height(NetworkUpgrade::Sapling)
.unwrap(),
BlockHash([0; 32]),
0,
)
});
.unwrap_or_else(|| (self.sapling_activation_height(), BlockHash([0; 32]), 0));
let (res, nf) = self.generate_block_at(
height,
@ -214,7 +204,7 @@ where
initial_sapling_tree_size: u32,
) -> (Cache::InsertResult, Nullifier) {
let (cb, nf) = fake_compact_block(
&self.params,
&self.network(),
height,
prev_hash,
dfvk,
@ -246,18 +236,10 @@ where
let (height, prev_hash, initial_sapling_tree_size) = self
.latest_cached_block
.map(|(prev_height, prev_hash, end_size)| (prev_height + 1, prev_hash, end_size))
.unwrap_or_else(|| {
(
self.params
.activation_height(NetworkUpgrade::Sapling)
.unwrap(),
BlockHash([0; 32]),
0,
)
});
.unwrap_or_else(|| (self.sapling_activation_height(), BlockHash([0; 32]), 0));
let cb = fake_compact_block_spending(
&self.params,
&self.network(),
height,
prev_hash,
note,
@ -296,7 +278,7 @@ where
>,
> {
scan_cached_blocks(
&self.params,
&self.network(),
self.cache.block_source(),
&mut self.db_data,
from_height,
@ -316,9 +298,9 @@ impl<Cache> TestState<Cache> {
&mut self.db_data
}
/// Exposes an immutable reference to the network in use.
pub(crate) fn network(&self) -> &Network {
&self.db_data.params
/// Exposes the network in use.
pub(crate) fn network(&self) -> Network {
self.db_data.params
}
/// Convenience method for obtaining the Sapling activation height for the network under test.
@ -362,9 +344,10 @@ impl<Cache> TestState<Cache> {
ReceivedNoteId,
>,
> {
let params = self.network();
create_spend_to_address(
&mut self.db_data,
&self.params,
&params,
test_prover(),
usk,
to,
@ -397,9 +380,10 @@ impl<Cache> TestState<Cache> {
where
InputsT: InputSelector<DataSource = WalletDb<Connection, Network>>,
{
let params = self.network();
spend(
&mut self.db_data,
&self.params,
&params,
test_prover(),
input_selector,
usk,
@ -430,9 +414,10 @@ impl<Cache> TestState<Cache> {
where
InputsT: InputSelector<DataSource = WalletDb<Connection, Network>>,
{
let params = self.network();
propose_transfer::<_, _, _, Infallible>(
&mut self.db_data,
&self.params,
&params,
spend_from_account,
input_selector,
request,
@ -462,9 +447,10 @@ impl<Cache> TestState<Cache> {
where
InputsT: InputSelector<DataSource = WalletDb<Connection, Network>>,
{
let params = self.network();
propose_shielding::<_, _, _, Infallible>(
&mut self.db_data,
&self.params,
&params,
input_selector,
shielding_threshold,
from_addrs,
@ -493,9 +479,10 @@ impl<Cache> TestState<Cache> {
where
FeeRuleT: FeeRule,
{
let params = self.network();
create_proposed_transaction::<_, _, Infallible, _>(
&mut self.db_data,
&self.params,
&params,
test_prover(),
usk,
ovk_policy,
@ -529,9 +516,10 @@ impl<Cache> TestState<Cache> {
where
InputsT: InputSelector<DataSource = WalletDb<Connection, Network>>,
{
let params = self.network();
shield_transparent_funds(
&mut self.db_data,
&self.params,
&params,
test_prover(),
input_selector,
shielding_threshold,

View File

@ -391,7 +391,10 @@ mod tests {
zip32::sapling::ExtendedFullViewingKey,
};
use crate::{error::SqliteClientError, wallet::scanning::priority_code, AccountId, WalletDb};
use crate::{
error::SqliteClientError, testing::TestBuilder, wallet::scanning::priority_code, AccountId,
WalletDb,
};
use super::{init_accounts_table, init_blocks_table, init_wallet_db};
@ -405,9 +408,7 @@ mod tests {
#[test]
fn verify_schema() {
let data_file = NamedTempFile::new().unwrap();
let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap();
init_wallet_db(&mut db_data, None).unwrap();
let st = TestBuilder::new().build();
use regex::Regex;
let re = Regex::new(r"\s+").unwrap();
@ -560,7 +561,8 @@ mod tests {
)",
];
let mut tables_query = db_data
let mut tables_query = st
.wallet()
.conn
.prepare("SELECT sql FROM sqlite_schema WHERE type = 'table' ORDER BY tbl_name")
.unwrap();
@ -600,7 +602,7 @@ mod tests {
AND (scan_queue.block_range_end - 1) >= shard.subtree_end_height
)
WHERE scan_queue.priority > {}",
u32::from(db_data.params.activation_height(NetworkUpgrade::Sapling).unwrap()),
u32::from(st.network().activation_height(NetworkUpgrade::Sapling).unwrap()),
priority_code(&ScanPriority::Scanned),
),
// v_transactions
@ -741,7 +743,8 @@ mod tests {
OR sapling_received_notes.is_change = 0".to_owned(),
];
let mut views_query = db_data
let mut views_query = st
.wallet()
.conn
.prepare("SELECT sql FROM sqlite_schema WHERE type = 'view' ORDER BY tbl_name")
.unwrap();

View File

@ -532,7 +532,7 @@ pub(crate) mod tests {
let ufvks = [(account, usk.to_unified_full_viewing_key())]
.into_iter()
.collect();
let decrypted_outputs = decrypt_transaction(st.network(), h + 1, &tx, &ufvks);
let decrypted_outputs = decrypt_transaction(&st.network(), h + 1, &tx, &ufvks);
assert_eq!(decrypted_outputs.len(), 2);
let mut found_tx_change_memo = false;
@ -619,7 +619,7 @@ pub(crate) mod tests {
// Create a USK that doesn't exist in the wallet
let acct1 = AccountId::from(1);
let usk1 = UnifiedSpendingKey::from_seed(st.network(), &[1u8; 32], acct1).unwrap();
let usk1 = UnifiedSpendingKey::from_seed(&st.network(), &[1u8; 32], acct1).unwrap();
// Attempting to spend with a USK that is not in the wallet results in an error
assert_matches!(
@ -934,7 +934,7 @@ pub(crate) mod tests {
for output in tx.sapling_bundle().unwrap().shielded_outputs() {
// Find the output that decrypts with the external OVK
let result = try_sapling_output_recovery(
st.network(),
&st.network(),
h1,
&dfvk.to_ovk(Scope::External),
output,

View File

@ -1200,7 +1200,7 @@ mod tests {
fn create_account_creates_ignored_range() {
use ScanPriority::*;
let mut st = TestBuilder::new().with_block_cache().build();
let mut st = TestBuilder::new().build();
let sap_active = st.sapling_activation_height();