zcash_client_sqlite: Use account birthday subtree sizes for progress.

This also fixes the `update_chain_tip_stable_max_scanned` tests.
This commit is contained in:
Kris Nuttycombe 2024-03-23 19:40:01 -06:00
parent 9c9bd40549
commit e67a978ff9
5 changed files with 260 additions and 123 deletions

View File

@ -374,19 +374,27 @@ pub(crate) fn add_account<P: consensus::Parameters>(
#[cfg(not(feature = "transparent-inputs"))]
let transparent_item: Option<Vec<u8>> = None;
let birthday_sapling_tree_size = Some(birthday.sapling_frontier().tree_size());
#[cfg(feature = "orchard")]
let birthday_orchard_tree_size = Some(birthday.orchard_frontier().tree_size());
#[cfg(not(feature = "orchard"))]
let birthday_orchard_tree_size: Option<u64> = None;
let account_id: AccountId = conn.query_row(
r#"
INSERT INTO accounts (
account_kind, hd_seed_fingerprint, hd_account_index,
ufvk, uivk,
orchard_fvk_item_cache, sapling_fvk_item_cache, p2pkh_fvk_item_cache,
birthday_height, recover_until_height
birthday_height, birthday_sapling_tree_size, birthday_orchard_tree_size,
recover_until_height
)
VALUES (
:account_kind, :hd_seed_fingerprint, :hd_account_index,
:ufvk, :uivk,
:orchard_fvk_item_cache, :sapling_fvk_item_cache, :p2pkh_fvk_item_cache,
:birthday_height, :recover_until_height
:birthday_height, :birthday_sapling_tree_size, :birthday_orchard_tree_size,
:recover_until_height
)
RETURNING id;
"#,
@ -400,6 +408,8 @@ pub(crate) fn add_account<P: consensus::Parameters>(
":sapling_fvk_item_cache": sapling_item,
":p2pkh_fvk_item_cache": transparent_item,
":birthday_height": u32::from(birthday.height()),
":birthday_sapling_tree_size": birthday_sapling_tree_size,
":birthday_orchard_tree_size": birthday_orchard_tree_size,
":recover_until_height": birthday.recover_until().map(u32::from)
],
|row| Ok(AccountId(row.get(0)?)),
@ -884,22 +894,39 @@ impl ScanProgress for SubtreeScanProgress {
)
.map_err(SqliteClientError::from)
} else {
let start_height = birthday_height;
// Compute the starting number of notes directly from the blocks table
let start_size = conn.query_row(
"SELECT MAX(sapling_commitment_tree_size)
FROM blocks
WHERE height <= :start_height",
named_params![":start_height": u32::from(start_height)],
|row| row.get::<_, Option<u64>>(0),
)?;
// Get the starting note commitment tree size from the wallet birthday, or failing that
// from the blocks table.
let start_size = conn
.query_row(
"SELECT birthday_sapling_tree_size
FROM accounts
WHERE birthday_height = :birthday_height",
named_params![":birthday_height": u32::from(birthday_height)],
|row| row.get::<_, Option<u64>>(0),
)
.optional()?
.flatten()
.map(Ok)
.or_else(|| {
conn.query_row(
"SELECT MAX(sapling_commitment_tree_size - sapling_output_count)
FROM blocks
WHERE height <= :start_height",
named_params![":start_height": u32::from(birthday_height)],
|row| row.get::<_, Option<u64>>(0),
)
.optional()
.map(|opt| opt.flatten())
.transpose()
})
.transpose()?;
// Compute the total blocks scanned so far above the starting height
let scanned_count = conn.query_row(
"SELECT SUM(sapling_output_count)
FROM blocks
WHERE height > :start_height",
named_params![":start_height": u32::from(start_height)],
named_params![":start_height": u32::from(birthday_height)],
|row| row.get::<_, Option<u64>>(0),
)?;
@ -915,22 +942,22 @@ impl ScanProgress for SubtreeScanProgress {
FROM sapling_tree_shards
WHERE subtree_end_height > :start_height
OR subtree_end_height IS NULL",
named_params![":start_height": u32::from(start_height)],
named_params![":start_height": u32::from(birthday_height)],
|row| {
let min_tree_size = row
.get::<_, Option<u64>>(0)?
.map(|min| min << SAPLING_SHARD_HEIGHT);
let max_idx = row.get::<_, Option<u64>>(1)?;
Ok(start_size
.or(min_tree_size)
.zip(max_idx)
.map(|(min_tree_size, max)| {
let max_tree_size = (max + 1) << SAPLING_SHARD_HEIGHT;
.map(|min_idx| min_idx << SAPLING_SHARD_HEIGHT);
let max_tree_size = row
.get::<_, Option<u64>>(1)?
.map(|max_idx| (max_idx + 1) << SAPLING_SHARD_HEIGHT);
Ok(start_size.or(min_tree_size).zip(max_tree_size).map(
|(min_tree_size, max_tree_size)| {
Ratio::new(
scanned_count.unwrap_or(0),
max_tree_size - min_tree_size,
)
}))
},
))
},
)
.optional()?
@ -961,22 +988,38 @@ impl ScanProgress for SubtreeScanProgress {
)
.map_err(SqliteClientError::from)
} else {
let start_height = birthday_height;
// Compute the starting number of notes directly from the blocks table
let start_size = conn.query_row(
"SELECT MAX(orchard_commitment_tree_size)
FROM blocks
WHERE height <= :start_height",
named_params![":start_height": u32::from(start_height)],
|row| row.get::<_, Option<u64>>(0),
)?;
let start_size = conn
.query_row(
"SELECT birthday_orchard_tree_size
FROM accounts
WHERE birthday_height = :birthday_height",
named_params![":birthday_height": u32::from(birthday_height)],
|row| row.get::<_, Option<u64>>(0),
)
.optional()?
.flatten()
.map(Ok)
.or_else(|| {
conn.query_row(
"SELECT MAX(orchard_commitment_tree_size - orchard_action_count)
FROM blocks
WHERE height <= :start_height",
named_params![":start_height": u32::from(birthday_height)],
|row| row.get::<_, Option<u64>>(0),
)
.optional()
.map(|opt| opt.flatten())
.transpose()
})
.transpose()?;
// Compute the total blocks scanned so far above the starting height
let scanned_count = conn.query_row(
"SELECT SUM(orchard_action_count)
FROM blocks
WHERE height > :start_height",
named_params![":start_height": u32::from(start_height)],
named_params![":start_height": u32::from(birthday_height)],
|row| row.get::<_, Option<u64>>(0),
)?;
@ -992,22 +1035,22 @@ impl ScanProgress for SubtreeScanProgress {
FROM orchard_tree_shards
WHERE subtree_end_height > :start_height
OR subtree_end_height IS NULL",
named_params![":start_height": u32::from(start_height)],
named_params![":start_height": u32::from(birthday_height)],
|row| {
let min_tree_size = row
.get::<_, Option<u64>>(0)?
.map(|min| min << ORCHARD_SHARD_HEIGHT);
let max_idx = row.get::<_, Option<u64>>(1)?;
Ok(start_size
.or(min_tree_size)
.zip(max_idx)
.map(|(min_tree_size, max)| {
let max_tree_size = (max + 1) << ORCHARD_SHARD_HEIGHT;
.map(|min_idx| min_idx << ORCHARD_SHARD_HEIGHT);
let max_tree_size = row
.get::<_, Option<u64>>(1)?
.map(|max_idx| (max_idx + 1) << ORCHARD_SHARD_HEIGHT);
Ok(start_size.or(min_tree_size).zip(max_tree_size).map(
|(min_tree_size, max_tree_size)| {
Ratio::new(
scanned_count.unwrap_or(0),
max_tree_size - min_tree_size,
)
}))
},
))
},
)
.optional()?

View File

@ -370,8 +370,23 @@ mod tests {
sapling_fvk_item_cache BLOB,
p2pkh_fvk_item_cache BLOB,
birthday_height INTEGER NOT NULL,
birthday_sapling_tree_size INTEGER,
birthday_orchard_tree_size INTEGER,
recover_until_height INTEGER,
CHECK ( (account_kind = 0 AND hd_seed_fingerprint IS NOT NULL AND hd_account_index IS NOT NULL AND ufvk IS NOT NULL) OR (account_kind = 1 AND hd_seed_fingerprint IS NULL AND hd_account_index IS NULL) )
CHECK (
(
account_kind = 0
AND hd_seed_fingerprint IS NOT NULL
AND hd_account_index IS NOT NULL
AND ufvk IS NOT NULL
)
OR
(
account_kind = 1
AND hd_seed_fingerprint IS NULL
AND hd_account_index IS NULL
)
)
)"#,
r#"CREATE TABLE "addresses" (
account_id INTEGER NOT NULL,

View File

@ -56,10 +56,10 @@ pub(super) fn all_migrations<P: consensus::Parameters + 'static>(
// v_sapling_shard_unscanned_ranges \ | v_tx_outputs_use_legacy_false
// | \ | |
// wallet_summaries \ | v_transactions_shielding_balance
// \ | |
// \ | v_transactions_note_uniqueness
// \ | /
// full_account_ids
// \ \ | |
// \ \ | v_transactions_note_uniqueness
// \ \ | /
// -------------------- full_account_ids
// |
// orchard_received_notes
vec![

View File

@ -1,7 +1,7 @@
use std::{collections::HashSet, rc::Rc};
use crate::wallet::{account_kind_code, init::WalletMigrationError};
use rusqlite::{named_params, Transaction};
use rusqlite::{named_params, OptionalExtension, Transaction};
use schemer_rusqlite::RusqliteMigration;
use secrecy::{ExposeSecret, SecretVec};
use uuid::Uuid;
@ -10,7 +10,7 @@ use zcash_keys::keys::UnifiedFullViewingKey;
use zcash_primitives::consensus;
use zip32::fingerprint::SeedFingerprint;
use super::{add_account_birthdays, receiving_key_scopes, v_transactions_note_uniqueness};
use super::{add_account_birthdays, receiving_key_scopes, v_transactions_note_uniqueness, wallet_summaries};
/// The migration that switched from presumed seed-derived account IDs to supporting
/// HD accounts and all sorts of imported keys.
@ -31,6 +31,7 @@ impl<P: consensus::Parameters> schemer::Migration for Migration<P> {
receiving_key_scopes::MIGRATION_ID,
add_account_birthdays::MIGRATION_ID,
v_transactions_note_uniqueness::MIGRATION_ID,
wallet_summaries::MIGRATION_ID,
]
.into_iter()
.collect()
@ -50,8 +51,8 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
account_index: zip32::AccountId::ZERO,
});
let account_kind_imported = account_kind_code(AccountSource::Imported);
transaction.execute_batch(
&format!(r#"
transaction.execute_batch(&format!(
r#"
PRAGMA foreign_keys = OFF;
PRAGMA legacy_alter_table = ON;
@ -66,18 +67,29 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
sapling_fvk_item_cache BLOB,
p2pkh_fvk_item_cache BLOB,
birthday_height INTEGER NOT NULL,
birthday_sapling_tree_size INTEGER,
birthday_orchard_tree_size INTEGER,
recover_until_height INTEGER,
CHECK (
(account_kind = {account_kind_derived} AND hd_seed_fingerprint IS NOT NULL AND hd_account_index IS NOT NULL AND ufvk IS NOT NULL)
OR
(account_kind = {account_kind_imported} AND hd_seed_fingerprint IS NULL AND hd_account_index IS NULL)
(
account_kind = {account_kind_derived}
AND hd_seed_fingerprint IS NOT NULL
AND hd_account_index IS NOT NULL
AND ufvk IS NOT NULL
)
OR
(
account_kind = {account_kind_imported}
AND hd_seed_fingerprint IS NULL
AND hd_account_index IS NULL
)
)
);
CREATE UNIQUE INDEX hd_account ON accounts_new (hd_seed_fingerprint, hd_account_index);
CREATE UNIQUE INDEX accounts_uivk ON accounts_new (uivk);
CREATE UNIQUE INDEX accounts_ufvk ON accounts_new (ufvk);
"#),
)?;
"#
))?;
// We require the seed *if* there are existing accounts in the table.
if transaction.query_row("SELECT COUNT(*) FROM accounts", [], |row| {
@ -158,19 +170,40 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
#[cfg(not(feature = "transparent-inputs"))]
let transparent_item: Option<Vec<u8>> = None;
// Get the tree sizes for the birthday height from the blocks table, if
// available.
let (birthday_sapling_tree_size, birthday_orchard_tree_size) = transaction
.query_row(
"SELECT sapling_commitment_tree_size - sapling_output_count,
orchard_commitment_tree_size - orchard_action_count
FROM blocks
WHERE height = :birthday_height",
named_params![":birthday_height": birthday_height],
|row| {
Ok(row
.get::<_, Option<u32>>(0)?
.zip(row.get::<_, Option<u32>>(1)?))
},
)
.optional()?
.flatten()
.map_or((None, None), |(s, o)| (Some(s), Some(o)));
transaction.execute(
r#"
INSERT INTO accounts_new (
id, account_kind, hd_seed_fingerprint, hd_account_index,
ufvk, uivk,
orchard_fvk_item_cache, sapling_fvk_item_cache, p2pkh_fvk_item_cache,
birthday_height, recover_until_height
birthday_height, birthday_sapling_tree_size, birthday_orchard_tree_size,
recover_until_height
)
VALUES (
:account_id, :account_kind, :seed_id, :account_index,
:ufvk, :uivk,
:orchard_fvk_item_cache, :sapling_fvk_item_cache, :p2pkh_fvk_item_cache,
:birthday_height, :recover_until_height
:birthday_height, :birthday_sapling_tree_size, :birthday_orchard_tree_size,
:recover_until_height
);
"#,
named_params![
@ -184,6 +217,8 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
":sapling_fvk_item_cache": ufvk_parsed.sapling().map(|k| k.to_bytes()),
":p2pkh_fvk_item_cache": transparent_item,
":birthday_height": birthday_height,
":birthday_sapling_tree_size": birthday_sapling_tree_size,
":birthday_orchard_tree_size": birthday_orchard_tree_size,
":recover_until_height": recover_until_height,
],
)?;

View File

@ -594,7 +594,6 @@ pub(crate) mod tests {
consensus::{BlockHeight, NetworkUpgrade, Parameters},
transaction::components::amount::NonNegativeAmount,
};
use zcash_protocol::ShieldedProtocol;
use crate::{
error::SqliteClientError,
@ -610,10 +609,7 @@ pub(crate) mod tests {
};
#[cfg(feature = "orchard")]
use {
crate::wallet::orchard::tests::OrchardPoolTester, orchard::tree::MerkleHashOrchard,
zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT,
};
use {crate::wallet::orchard::tests::OrchardPoolTester, orchard::tree::MerkleHashOrchard};
#[test]
fn sapling_scan_complete() {
@ -1188,9 +1184,7 @@ pub(crate) mod tests {
assert_eq!(actual, expected);
}
// FIXME: This requires fixes to the test framework.
#[test]
#[cfg(feature = "orchard")]
fn sapling_update_chain_tip_stable_max_scanned() {
update_chain_tip_stable_max_scanned::<SaplingPoolTester>();
}
@ -1201,36 +1195,74 @@ pub(crate) mod tests {
update_chain_tip_stable_max_scanned::<OrchardPoolTester>();
}
// FIXME: This requires fixes to the test framework.
#[allow(dead_code)]
fn update_chain_tip_stable_max_scanned<T: ShieldedPoolTester>() {
use ScanPriority::*;
// Use a non-zero birthday offset because Sapling and NU5 are activated at the same height.
let (mut st, dfvk, birthday, sap_active) =
test_with_nu5_birthday_offset::<T>(76, BlockHash([0; 32]));
// Set up the following situation:
//
// prior_tip new_tip
// |<--- 500 --->|<- 20 ->|<-- 50 -->|<- 20 ->|
// wallet_birthday max_scanned last_shard_start
//
let max_scanned = birthday.height() + 500;
let prior_tip = max_scanned + 20;
let birthday_offset = 76;
let birthday_prior_block_hash = BlockHash([0; 32]);
// We set the Sapling and Orchard frontiers at the birthday block initial state to 1234
// notes beyond the end of the first shard.
let frontier_tree_size: u32 = (0x1 << 16) + 1234;
let mut st = TestBuilder::new()
.with_block_cache()
.with_initial_chain_state(|rng, network| {
let birthday_height =
network.activation_height(NetworkUpgrade::Nu5).unwrap() + birthday_offset;
// Set up some shard root history before the wallet birthday.
let second_to_last_shard_start = birthday.height() - 1000;
T::put_subtree_roots(
&mut st,
0,
&[CommitmentTreeRoot::from_parts(
second_to_last_shard_start,
// fake a hash, the value doesn't matter
T::empty_tree_leaf(),
)],
)
.unwrap();
// Construct a fake chain state for the end of the block with the given
// birthday_offset from the Nu5 birthday.
let (prior_sapling_roots, sapling_initial_tree) =
Frontier::random_with_prior_subtree_roots(
rng,
frontier_tree_size.into(),
NonZeroU8::new(16).unwrap(),
);
// There will only be one prior root
let prior_sapling_roots = prior_sapling_roots
.into_iter()
.map(|root| CommitmentTreeRoot::from_parts(birthday_height - 10, root))
.collect::<Vec<_>>();
#[cfg(feature = "orchard")]
let (prior_orchard_roots, orchard_initial_tree) =
Frontier::random_with_prior_subtree_roots(
rng,
frontier_tree_size.into(),
NonZeroU8::new(16).unwrap(),
);
// There will only be one prior root
#[cfg(feature = "orchard")]
let prior_orchard_roots = prior_orchard_roots
.into_iter()
.map(|root| CommitmentTreeRoot::from_parts(birthday_height - 10, root))
.collect::<Vec<_>>();
InitialChainState {
chain_state: ChainState::new(
birthday_height - 1,
birthday_prior_block_hash,
sapling_initial_tree,
#[cfg(feature = "orchard")]
orchard_initial_tree,
),
prior_sapling_roots,
#[cfg(feature = "orchard")]
prior_orchard_roots,
}
})
.with_account_having_current_birthday()
.build();
let account = st.test_account().cloned().unwrap();
let dfvk = T::test_account_fvk(&st);
let birthday = account.birthday();
let sap_active = st.sapling_activation_height();
// We have scan ranges and a subtree, but have scanned no blocks.
let summary = st.get_wallet_summary(1);
@ -1238,67 +1270,55 @@ pub(crate) mod tests {
// Set up prior chain state. This simulates us having imported a wallet
// with a birthday 520 blocks below the chain tip.
let max_scanned = birthday.height() + 500;
let prior_tip = max_scanned + 20;
st.wallet_mut().update_chain_tip(prior_tip).unwrap();
// Verify that the suggested scan ranges match what is expected.
let expected = vec![
scan_range(birthday.height().into()..(prior_tip + 1).into(), ChainTip),
scan_range(sap_active..birthday.height().into(), Ignored),
scan_range(sap_active.into()..birthday.height().into(), Ignored),
];
let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();
assert_eq!(actual, expected);
// Now, scan the max scanned block.
let initial_sapling_tree_size = birthday
.sapling_frontier()
.value()
.map(|f| u64::from(f.position() + 1))
.unwrap_or(0)
.try_into()
.unwrap();
#[cfg(feature = "orchard")]
let initial_orchard_tree_size = birthday
.orchard_frontier()
.value()
.map(|f| u64::from(f.position() + 1))
.unwrap_or(0)
.try_into()
.unwrap();
#[cfg(not(feature = "orchard"))]
let initial_orchard_tree_size = 0;
// Simulate that in the blocks between the wallet birthday and the max_scanned height,
// there are 10 Sapling notes and 10 Orchard notes created on the chain.
st.generate_block_at(
max_scanned,
BlockHash([0u8; 32]),
BlockHash([1; 32]),
&dfvk,
AddressType::DefaultExternal,
NonNegativeAmount::const_from_u64(10000),
initial_sapling_tree_size,
initial_orchard_tree_size,
frontier_tree_size + 10,
frontier_tree_size + 10,
);
st.scan_cached_blocks(max_scanned, 1);
// We have scanned a block, so we now have a starting tree position, 500 blocks above the
// wallet birthday but before the end of the shard.
let summary = st.get_wallet_summary(1);
assert_eq!(summary.as_ref().map(|s| T::next_subtree_index(s)), Some(0),);
assert_eq!(summary.as_ref().map(|s| T::next_subtree_index(s)), Some(0));
// Progress denominator depends on which pools are enabled (which changes the
// initial tree states in `test_with_nu5_birthday_offset`).
let expected_denom = 1 << SAPLING_SHARD_HEIGHT;
// initial tree states). Here we compute the denominator based upon the fact that
// the trees are the same size at present.
let expected_denom = (1 << SAPLING_SHARD_HEIGHT) * 2 - frontier_tree_size;
#[cfg(feature = "orchard")]
let expected_denom = expected_denom + (1 << ORCHARD_SHARD_HEIGHT);
let expected_denom = expected_denom * 2;
assert_eq!(
summary.and_then(|s| s.scan_progress()),
Some(Ratio::new(1, expected_denom))
Some(Ratio::new(1, u64::from(expected_denom)))
);
// Now simulate shutting down, and then restarting 70 blocks later, after a shard
// has been completed.
// has been completed in one pool. This shard will have index 2, as our birthday
// was in shard 1.
let last_shard_start = prior_tip + 50;
T::put_subtree_roots(
&mut st,
0,
2,
&[CommitmentTreeRoot::from_parts(
last_shard_start,
// fake a hash, the value doesn't matter
@ -1307,6 +1327,36 @@ pub(crate) mod tests {
)
.unwrap();
{
let mut shard_stmt = st
.wallet_mut()
.conn
.prepare("SELECT shard_index, subtree_end_height FROM sapling_tree_shards")
.unwrap();
(shard_stmt
.query_and_then::<_, rusqlite::Error, _, _>([], |row| {
Ok((row.get::<_, u32>(0)?, row.get::<_, Option<u32>>(1)?))
})
.unwrap()
.collect::<Result<Vec<_>, _>>())
.unwrap();
}
{
let mut shard_stmt = st
.wallet_mut()
.conn
.prepare("SELECT shard_index, subtree_end_height FROM orchard_tree_shards")
.unwrap();
(shard_stmt
.query_and_then::<_, rusqlite::Error, _, _>([], |row| {
Ok((row.get::<_, u32>(0)?, row.get::<_, Option<u32>>(1)?))
})
.unwrap()
.collect::<Result<Vec<_>, _>>())
.unwrap();
}
let new_tip = last_shard_start + 20;
st.wallet_mut().update_chain_tip(new_tip).unwrap();
let chain_end = u32::from(new_tip + 1);
@ -1320,26 +1370,20 @@ pub(crate) mod tests {
// The max scanned block itself is left as-is.
scan_range(max_scanned.into()..(max_scanned + 1).into(), Scanned),
// The range below the second-to-last shard is ignored.
scan_range(sap_active..birthday.height().into(), Ignored),
scan_range(sap_active.into()..birthday.height().into(), Ignored),
];
let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();
assert_eq!(actual, expected);
// We've crossed a subtree boundary, and so still only have one scanned note but have two
// shards worth of notes to scan.
let expected_denom = expected_denom
+ match T::SHIELDED_PROTOCOL {
ShieldedProtocol::Sapling => 1 << SAPLING_SHARD_HEIGHT,
#[cfg(feature = "orchard")]
ShieldedProtocol::Orchard => 1 << ORCHARD_SHARD_HEIGHT,
#[cfg(not(feature = "orchard"))]
ShieldedProtocol::Orchard => unreachable!(),
};
// We've crossed a subtree boundary, but only in one pool. We still only have one scanned
// note but in the pool where we crossed the subtree boundary we have two shards worth of
// notes to scan.
let expected_denom = expected_denom + (1 << 16);
let summary = st.get_wallet_summary(1);
assert_eq!(
summary.and_then(|s| s.scan_progress()),
Some(Ratio::new(1, expected_denom))
Some(Ratio::new(1, u64::from(expected_denom)))
);
}