From 45392cfeef1591bfc1c8fa6a3ba39659a0bb398c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 21 Nov 2024 06:25:17 +0000 Subject: [PATCH 1/5] zcash_client_sqlite: Add missing leaf migration states for tests --- zcash_client_sqlite/src/wallet/init/migrations.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/zcash_client_sqlite/src/wallet/init/migrations.rs b/zcash_client_sqlite/src/wallet/init/migrations.rs index b66ef4cc3..fdff45a35 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations.rs @@ -157,7 +157,8 @@ pub(super) fn all_migrations( /// included. #[allow(dead_code)] const PUBLIC_MIGRATION_STATES: &[&[Uuid]] = &[ - V_0_4_0, V_0_6_0, V_0_8_0, V_0_9_0, V_0_10_0, V_0_10_3, V_0_11_0, V_0_11_1, + V_0_4_0, V_0_6_0, V_0_8_0, V_0_9_0, V_0_10_0, V_0_10_3, V_0_11_0, V_0_11_1, V_0_11_2, V_0_12_0, + V_0_13_0, ]; /// Leaf migrations in the 0.4.0 release. @@ -208,6 +209,15 @@ const V_0_11_0: &[Uuid] = &[ /// Leaf migrations in the 0.11.1 release. const V_0_11_1: &[Uuid] = &[tx_retrieval_queue::MIGRATION_ID]; +/// Leaf migrations in the 0.11.2 release. +const V_0_11_2: &[Uuid] = &[support_legacy_sqlite::MIGRATION_ID]; + +/// Leaf migrations in the 0.12.0 release. +const V_0_12_0: &[Uuid] = &[fix_broken_commitment_trees::MIGRATION_ID]; + +/// Leaf migrations in the 0.13.0 release. +const V_0_13_0: &[Uuid] = &[fix_bad_change_flagging::MIGRATION_ID]; + pub(super) fn verify_network_compatibility( conn: &rusqlite::Connection, params: &P, From fd5bb46985ace0a5fdc71f5e7ccaf0f6e7a5ba09 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 21 Nov 2024 11:11:50 +0000 Subject: [PATCH 2/5] zcash_client_sqlite: Add a `uuid` column to the `accounts` table --- Cargo.lock | 4 + zcash_client_sqlite/CHANGELOG.md | 7 + zcash_client_sqlite/Cargo.toml | 4 +- zcash_client_sqlite/src/wallet.rs | 7 +- zcash_client_sqlite/src/wallet/db.rs | 25 +- zcash_client_sqlite/src/wallet/init.rs | 1 + .../src/wallet/init/migrations.rs | 10 +- .../init/migrations/add_account_uuids.rs | 325 ++++++++++++++++++ .../migrations/fix_bad_change_flagging.rs | 9 +- 9 files changed, 376 insertions(+), 16 deletions(-) create mode 100644 zcash_client_sqlite/src/wallet/init/migrations/add_account_uuids.rs diff --git a/Cargo.lock b/Cargo.lock index d8e6db001..a0390d68d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3506,6 +3506,7 @@ dependencies = [ "libsqlite3-sys", "smallvec", "time", + "uuid", ] [[package]] @@ -5504,6 +5505,9 @@ name = "uuid" version = "1.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a183cf7feeba97b4dd1c0d46788634f6221d87fa961b305bed08c851829efcc0" +dependencies = [ + "getrandom", +] [[package]] name = "valuable" diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 0f6440e54..addf9c867 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -7,6 +7,13 @@ and this library adheres to Rust's notion of ## [Unreleased] +### Changed +- The `v_transactions` view has been modified: + - The `account_id` column has been replaced with `account_uuid`. +- The `v_tx_outputs` view has been modified: + - The `from_account_id` column has been replaced with `from_account_uuid`. + - The `to_account_id` column has been replaced with `to_account_uuid`. + ## [0.13.0] - 2024-11-14 ### Added diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index 9501ba5a8..9606b9ac3 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -66,11 +66,11 @@ incrementalmerkletree.workspace = true shardtree = { workspace = true, features = ["legacy-api"] } # - SQLite databases -rusqlite = { workspace = true, features = ["time", "array"] } +rusqlite = { workspace = true, features = ["time", "array", "uuid"] } schemerz.workspace = true schemerz-rusqlite.workspace = true time.workspace = true -uuid.workspace = true +uuid = { workspace = true, features = ["v4"] } regex = "1.4" # Dependencies used internally: diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 43d8d7777..c4a1263e1 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -69,6 +69,7 @@ use incrementalmerkletree::{Marking, Retention}; use rusqlite::{self, named_params, params, OptionalExtension}; use secrecy::{ExposeSecret, SecretVec}; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; +use uuid::Uuid; use zcash_client_backend::data_api::{ AccountPurpose, DecryptedTransaction, Progress, TransactionDataRequest, TransactionStatus, }; @@ -404,6 +405,7 @@ pub(crate) fn add_account( .query_row( r#" INSERT INTO accounts ( + uuid, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, orchard_fvk_item_cache, sapling_fvk_item_cache, p2pkh_fvk_item_cache, @@ -412,6 +414,7 @@ pub(crate) fn add_account( has_spend_key ) VALUES ( + :uuid, :account_kind, :hd_seed_fingerprint, :hd_account_index, :ufvk, :uivk, :orchard_fvk_item_cache, :sapling_fvk_item_cache, :p2pkh_fvk_item_cache, @@ -422,6 +425,7 @@ pub(crate) fn add_account( RETURNING id; "#, named_params![ + ":uuid": Uuid::new_v4(), ":account_kind": account_kind_code(kind), ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.to_bytes()), ":hd_account_index": hd_account_index.map(u32::from), @@ -3526,8 +3530,9 @@ pub mod testing { conn: &rusqlite::Connection, ) -> Result>, SqliteClientError> { let mut stmt = conn.prepare_cached( - "SELECT * + "SELECT accounts.id as account_id, v_transactions.* FROM v_transactions + JOIN accounts ON accounts.uuid = v_transactions.account_uuid ORDER BY mined_height DESC, tx_index DESC", )?; diff --git a/zcash_client_sqlite/src/wallet/db.rs b/zcash_client_sqlite/src/wallet/db.rs index 69ef0224e..6f2c414b1 100644 --- a/zcash_client_sqlite/src/wallet/db.rs +++ b/zcash_client_sqlite/src/wallet/db.rs @@ -24,6 +24,7 @@ use crate::wallet::scanning::priority_code; pub(super) const TABLE_ACCOUNTS: &str = r#" CREATE TABLE "accounts" ( id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + uuid BLOB NOT NULL, account_kind INTEGER NOT NULL DEFAULT 0, hd_seed_fingerprint BLOB, hd_account_index INTEGER, @@ -52,12 +53,14 @@ CREATE TABLE "accounts" ( ) ) )"#; +pub(super) const INDEX_ACCOUNTS_UUID: &str = + r#"CREATE UNIQUE INDEX accounts_uuid ON accounts (uuid)"#; pub(super) const INDEX_ACCOUNTS_UFVK: &str = - r#"CREATE UNIQUE INDEX accounts_ufvk ON "accounts" (ufvk)"#; + r#"CREATE UNIQUE INDEX accounts_ufvk ON accounts (ufvk)"#; pub(super) const INDEX_ACCOUNTS_UIVK: &str = - r#"CREATE UNIQUE INDEX accounts_uivk ON "accounts" (uivk)"#; + r#"CREATE UNIQUE INDEX accounts_uivk ON accounts (uivk)"#; pub(super) const INDEX_HD_ACCOUNT: &str = - r#"CREATE UNIQUE INDEX hd_account ON "accounts" (hd_seed_fingerprint, hd_account_index)"#; + r#"CREATE UNIQUE INDEX hd_account ON accounts (hd_seed_fingerprint, hd_account_index)"#; /// Stores diversified Unified Addresses that have been generated from accounts in the /// wallet. @@ -825,7 +828,7 @@ sent_note_counts AS ( blocks_max_height AS ( SELECT MAX(blocks.height) AS max_height FROM blocks ) -SELECT notes.account_id AS account_id, +SELECT accounts.uuid AS account_uuid, notes.mined_height AS mined_height, notes.txid AS txid, transactions.tx_index AS tx_index, @@ -855,6 +858,7 @@ SELECT notes.account_id AS account_id, AND MAX(COALESCE(sent_note_counts.sent_notes, 0)) = 0 ) AS is_shielding FROM notes +JOIN accounts ON accounts.id = notes.account_id LEFT JOIN transactions ON notes.txid = transactions.txid JOIN blocks_max_height @@ -883,8 +887,8 @@ CREATE VIEW v_tx_outputs AS SELECT transactions.txid AS txid, ro.pool AS output_pool, ro.output_index AS output_index, - sent_notes.from_account_id AS from_account_id, - ro.account_id AS to_account_id, + from_account.uuid AS from_account_uuid, + to_account.uuid AS to_account_uuid, NULL AS to_address, ro.value AS value, ro.is_change AS is_change, @@ -894,13 +898,16 @@ JOIN transactions ON transactions.id_tx = ro.transaction_id -- join to the sent_notes table to obtain `from_account_id` LEFT JOIN sent_notes ON sent_notes.id = ro.sent_note_id +-- join on the accounts table to obtain account UUIDs +JOIN accounts from_account ON accounts.id = sent_notes.from_account_id +JOIN accounts to_account ON accounts.id = ro.account_id UNION -- select all outputs sent from the wallet to external recipients SELECT transactions.txid AS txid, sent_notes.output_pool AS output_pool, sent_notes.output_index AS output_index, - sent_notes.from_account_id AS from_account_id, - NULL AS to_account_id, + from_account.uuid AS from_account_uuid, + NULL AS to_account_uuid, sent_notes.to_address AS to_address, sent_notes.value AS value, 0 AS is_change, @@ -909,6 +916,8 @@ FROM sent_notes JOIN transactions ON transactions.id_tx = sent_notes.tx LEFT JOIN v_received_outputs ro ON ro.sent_note_id = sent_notes.id +-- join on the accounts table to obtain account UUIDs +JOIN accounts from_account ON accounts.id = sent_notes.from_account_id -- exclude any sent notes for which a row exists in the v_received_outputs view WHERE ro.account_id IS NULL"; diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index b5fbc9539..f815a1c4b 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -529,6 +529,7 @@ mod tests { let expected_indices = vec![ db::INDEX_ACCOUNTS_UFVK, db::INDEX_ACCOUNTS_UIVK, + db::INDEX_ACCOUNTS_UUID, db::INDEX_HD_ACCOUNT, db::INDEX_ADDRESSES_ACCOUNTS, db::INDEX_NF_MAP_LOCATOR_IDX, diff --git a/zcash_client_sqlite/src/wallet/init/migrations.rs b/zcash_client_sqlite/src/wallet/init/migrations.rs index fdff45a35..cb6ca97b3 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations.rs @@ -1,4 +1,5 @@ mod add_account_birthdays; +mod add_account_uuids; mod add_transaction_views; mod add_utxo_account; mod addresses_table; @@ -81,10 +82,10 @@ pub(super) fn all_migrations( // ------------------------------ tx_retrieval_queue ---------------------------- // | // support_legacy_sqlite - // | - // fix_broken_commitment_trees - // | - // fix_bad_change_flagging + // / \ + // fix_broken_commitment_trees add_account_uuids + // | + // fix_bad_change_flagging vec![ Box::new(initial_setup::Migration {}), Box::new(utxos_table::Migration {}), @@ -147,6 +148,7 @@ pub(super) fn all_migrations( params: params.clone(), }), Box::new(fix_bad_change_flagging::Migration), + Box::new(add_account_uuids::Migration), ] } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/add_account_uuids.rs b/zcash_client_sqlite/src/wallet/init/migrations/add_account_uuids.rs new file mode 100644 index 000000000..73dfb17d3 --- /dev/null +++ b/zcash_client_sqlite/src/wallet/init/migrations/add_account_uuids.rs @@ -0,0 +1,325 @@ +//! This migration adds a UUID to each account record. + +use std::collections::HashSet; + +use rusqlite::named_params; +use schemerz_rusqlite::RusqliteMigration; +use uuid::Uuid; +use zcash_client_backend::data_api::{AccountPurpose, AccountSource}; +use zip32::fingerprint::SeedFingerprint; + +use crate::wallet::{account_kind_code, init::WalletMigrationError}; + +use super::support_legacy_sqlite; + +pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0xcccc623f_3243_43c7_b884_ceef25149e04); + +const DEPENDENCIES: &[Uuid] = &[support_legacy_sqlite::MIGRATION_ID]; + +pub(super) struct Migration; + +impl schemerz::Migration for Migration { + fn id(&self) -> Uuid { + MIGRATION_ID + } + + fn dependencies(&self) -> HashSet { + DEPENDENCIES.iter().copied().collect() + } + + fn description(&self) -> &'static str { + "Adds a UUID for each account." + } +} + +impl RusqliteMigration for Migration { + type Error = WalletMigrationError; + + fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), Self::Error> { + let account_kind_derived = account_kind_code(AccountSource::Derived { + seed_fingerprint: SeedFingerprint::from_bytes([0; 32]), + account_index: zip32::AccountId::ZERO, + }); + let account_kind_imported = account_kind_code(AccountSource::Imported { + // the purpose here is irrelevant; we just use it to get the correct code + // for the account kind + purpose: AccountPurpose::ViewOnly, + }); + transaction.execute_batch(&format!( + r#" + CREATE TABLE accounts_new ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + uuid BLOB NOT NULL, + account_kind INTEGER NOT NULL DEFAULT {account_kind_derived}, + hd_seed_fingerprint BLOB, + hd_account_index INTEGER, + ufvk TEXT, + uivk TEXT NOT NULL, + orchard_fvk_item_cache BLOB, + 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, + has_spend_key INTEGER NOT NULL DEFAULT 1, + 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 + ) + ) + ); + "# + ))?; + + let mut q = transaction.prepare("SELECT * FROM accounts")?; + let mut rows = q.query([])?; + while let Some(row) = rows.next()? { + let preserve = |idx: &str| row.get::<_, rusqlite::types::Value>(idx); + transaction.execute( + r#" + INSERT INTO accounts_new ( + id, uuid, + account_kind, hd_seed_fingerprint, hd_account_index, + ufvk, uivk, + orchard_fvk_item_cache, sapling_fvk_item_cache, p2pkh_fvk_item_cache, + birthday_height, birthday_sapling_tree_size, birthday_orchard_tree_size, + recover_until_height, + has_spend_key + ) + VALUES ( + :account_id, :uuid, + :account_kind, :hd_seed_fingerprint, :hd_account_index, + :ufvk, :uivk, + :orchard_fvk_item_cache, :sapling_fvk_item_cache, :p2pkh_fvk_item_cache, + :birthday_height, :birthday_sapling_tree_size, :birthday_orchard_tree_size, + :recover_until_height, + :has_spend_key + ); + "#, + named_params! { + ":account_id": preserve("id")?, + ":uuid": Uuid::new_v4(), + ":account_kind": preserve("account_kind")?, + ":hd_seed_fingerprint": preserve("hd_seed_fingerprint")?, + ":hd_account_index": preserve("hd_account_index")?, + ":ufvk": preserve("ufvk")?, + ":uivk": preserve("uivk")?, + ":orchard_fvk_item_cache": preserve("orchard_fvk_item_cache")?, + ":sapling_fvk_item_cache": preserve("sapling_fvk_item_cache")?, + ":p2pkh_fvk_item_cache": preserve("p2pkh_fvk_item_cache")?, + ":birthday_height": preserve("birthday_height")?, + ":birthday_sapling_tree_size": preserve("birthday_sapling_tree_size")?, + ":birthday_orchard_tree_size": preserve("birthday_orchard_tree_size")?, + ":recover_until_height": preserve("recover_until_height")?, + ":has_spend_key": preserve("has_spend_key")?, + }, + )?; + } + + transaction.execute_batch( + "PRAGMA legacy_alter_table = ON; + DROP TABLE accounts; + ALTER TABLE accounts_new RENAME TO accounts; + PRAGMA legacy_alter_table = OFF; + + -- Add the new index. + CREATE UNIQUE INDEX accounts_uuid ON accounts (uuid); + + -- Recreate the existing indices now that the original ones have been deleted. + CREATE UNIQUE INDEX hd_account ON accounts (hd_seed_fingerprint, hd_account_index); + CREATE UNIQUE INDEX accounts_uivk ON accounts (uivk); + CREATE UNIQUE INDEX accounts_ufvk ON accounts (ufvk); + + -- Replace accounts.id with accounts.uuid in v_transactions. + DROP VIEW v_transactions; + CREATE VIEW v_transactions AS + WITH + notes AS ( + -- Outputs received in this transaction + SELECT ro.account_id AS account_id, + transactions.mined_height AS mined_height, + transactions.txid AS txid, + ro.pool AS pool, + id_within_pool_table, + ro.value AS value, + 0 AS spent_note_count, + CASE + WHEN ro.is_change THEN 1 + ELSE 0 + END AS change_note_count, + CASE + WHEN ro.is_change THEN 0 + ELSE 1 + END AS received_count, + CASE + WHEN (ro.memo IS NULL OR ro.memo = X'F6') + THEN 0 + ELSE 1 + END AS memo_present, + -- The wallet cannot receive transparent outputs in shielding transactions. + CASE + WHEN ro.pool = 0 + THEN 1 + ELSE 0 + END AS does_not_match_shielding + FROM v_received_outputs ro + JOIN transactions + ON transactions.id_tx = ro.transaction_id + UNION + -- Outputs spent in this transaction + SELECT ro.account_id AS account_id, + transactions.mined_height AS mined_height, + transactions.txid AS txid, + ro.pool AS pool, + id_within_pool_table, + -ro.value AS value, + 1 AS spent_note_count, + 0 AS change_note_count, + 0 AS received_count, + 0 AS memo_present, + -- The wallet cannot spend shielded outputs in shielding transactions. + CASE + WHEN ro.pool != 0 + THEN 1 + ELSE 0 + END AS does_not_match_shielding + FROM v_received_outputs ro + JOIN v_received_output_spends ros + ON ros.pool = ro.pool + AND ros.received_output_id = ro.id_within_pool_table + JOIN transactions + ON transactions.id_tx = ros.transaction_id + ), + -- Obtain a count of the notes that the wallet created in each transaction, + -- not counting change notes. + sent_note_counts AS ( + SELECT sent_notes.from_account_id AS account_id, + transactions.txid AS txid, + COUNT(DISTINCT sent_notes.id) AS sent_notes, + SUM( + CASE + WHEN (sent_notes.memo IS NULL OR sent_notes.memo = X'F6' OR ro.transaction_id IS NOT NULL) + THEN 0 + ELSE 1 + END + ) AS memo_count + FROM sent_notes + JOIN transactions + ON transactions.id_tx = sent_notes.tx + LEFT JOIN v_received_outputs ro + ON sent_notes.id = ro.sent_note_id + WHERE COALESCE(ro.is_change, 0) = 0 + GROUP BY account_id, txid + ), + blocks_max_height AS ( + SELECT MAX(blocks.height) AS max_height FROM blocks + ) + SELECT accounts.uuid AS account_uuid, + notes.mined_height AS mined_height, + notes.txid AS txid, + transactions.tx_index AS tx_index, + transactions.expiry_height AS expiry_height, + transactions.raw AS raw, + SUM(notes.value) AS account_balance_delta, + transactions.fee AS fee_paid, + SUM(notes.change_note_count) > 0 AS has_change, + MAX(COALESCE(sent_note_counts.sent_notes, 0)) AS sent_note_count, + SUM(notes.received_count) AS received_note_count, + SUM(notes.memo_present) + MAX(COALESCE(sent_note_counts.memo_count, 0)) AS memo_count, + blocks.time AS block_time, + ( + blocks.height IS NULL + AND transactions.expiry_height BETWEEN 1 AND blocks_max_height.max_height + ) AS expired_unmined, + SUM(notes.spent_note_count) AS spent_note_count, + ( + -- All of the wallet-spent and wallet-received notes are consistent with a + -- shielding transaction. + SUM(notes.does_not_match_shielding) = 0 + -- The transaction contains at least one wallet-spent output. + AND SUM(notes.spent_note_count) > 0 + -- The transaction contains at least one wallet-received note. + AND (SUM(notes.received_count) + SUM(notes.change_note_count)) > 0 + -- We do not know about any external outputs of the transaction. + AND MAX(COALESCE(sent_note_counts.sent_notes, 0)) = 0 + ) AS is_shielding + FROM notes + JOIN accounts ON accounts.id = notes.account_id + LEFT JOIN transactions + ON notes.txid = transactions.txid + JOIN blocks_max_height + LEFT JOIN blocks ON blocks.height = notes.mined_height + LEFT JOIN sent_note_counts + ON sent_note_counts.account_id = notes.account_id + AND sent_note_counts.txid = notes.txid + GROUP BY notes.account_id, notes.txid; + + -- Replace accounts.id with accounts.uuid in v_tx_outputs. + DROP VIEW v_tx_outputs; + CREATE VIEW v_tx_outputs AS + -- select all outputs received by the wallet + SELECT transactions.txid AS txid, + ro.pool AS output_pool, + ro.output_index AS output_index, + from_account.uuid AS from_account_uuid, + to_account.uuid AS to_account_uuid, + NULL AS to_address, + ro.value AS value, + ro.is_change AS is_change, + ro.memo AS memo + FROM v_received_outputs ro + JOIN transactions + ON transactions.id_tx = ro.transaction_id + -- join to the sent_notes table to obtain `from_account_id` + LEFT JOIN sent_notes ON sent_notes.id = ro.sent_note_id + -- join on the accounts table to obtain account UUIDs + JOIN accounts from_account ON accounts.id = sent_notes.from_account_id + JOIN accounts to_account ON accounts.id = ro.account_id + UNION + -- select all outputs sent from the wallet to external recipients + SELECT transactions.txid AS txid, + sent_notes.output_pool AS output_pool, + sent_notes.output_index AS output_index, + from_account.uuid AS from_account_uuid, + NULL AS to_account_uuid, + sent_notes.to_address AS to_address, + sent_notes.value AS value, + 0 AS is_change, + sent_notes.memo AS memo + FROM sent_notes + JOIN transactions + ON transactions.id_tx = sent_notes.tx + LEFT JOIN v_received_outputs ro ON ro.sent_note_id = sent_notes.id + -- join on the accounts table to obtain account UUIDs + JOIN accounts from_account ON accounts.id = sent_notes.from_account_id + -- exclude any sent notes for which a row exists in the v_received_outputs view + WHERE ro.account_id IS NULL", + )?; + + Ok(()) + } + + fn down(&self, _transaction: &rusqlite::Transaction) -> Result<(), Self::Error> { + Err(WalletMigrationError::CannotRevert(MIGRATION_ID)) + } +} + +#[cfg(test)] +mod tests { + use crate::wallet::init::migrations::tests::test_migrate; + + #[test] + fn migrate() { + test_migrate(&[super::MIGRATION_ID]); + } +} diff --git a/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs b/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs index efab795a6..0923b1630 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs @@ -103,7 +103,14 @@ mod tests { #[cfg(feature = "transparent-inputs")] fn shield_transparent() { - let ds_factory = TestDbFactory::new(super::DEPENDENCIES.to_vec()); + let ds_factory = TestDbFactory::new( + super::DEPENDENCIES + .iter() + .copied() + // Pull in the account UUID migration so `TestBuilder::build` works. + .chain(Some(super::super::add_account_uuids::MIGRATION_ID)) + .collect(), + ); let cache = BlockCache::new(); let mut st = TestBuilder::new() .with_data_store_factory(ds_factory) From bf42ec29a1792ce318bb38dd28c7731734de2b58 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 22 Nov 2024 02:46:05 +0000 Subject: [PATCH 3/5] zcash_client_sqlite: Add `AccountUuid` to expose the `uuid` column --- zcash_client_sqlite/CHANGELOG.md | 9 +++++ zcash_client_sqlite/src/lib.rs | 62 +++++++++++++++++++++++-------- zcash_client_sqlite/src/wallet.rs | 54 +++++++++++++++++++++++---- 3 files changed, 102 insertions(+), 23 deletions(-) diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index addf9c867..0d21f113f 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -7,6 +7,11 @@ and this library adheres to Rust's notion of ## [Unreleased] +### Added +- `zcash_client_sqlite::AccountUuid` +- `zcash_client_sqlite::Account::uuid` +- `zcash_client_sqlite::WalletDb::get_account_for_uuid` + ### Changed - The `v_transactions` view has been modified: - The `account_id` column has been replaced with `account_uuid`. @@ -14,6 +19,10 @@ and this library adheres to Rust's notion of - The `from_account_id` column has been replaced with `from_account_uuid`. - The `to_account_id` column has been replaced with `to_account_uuid`. +### Removed +- `zcash_client_sqlite::AccountId::{from_u32, as_u32}` (use `AccountUuid` and + its methods instead). + ## [0.13.0] - 2024-11-14 ### Added diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 74e2d23a4..3aeeee55d 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -43,6 +43,7 @@ use std::{ }; use subtle::ConditionallySelectable; use tracing::{debug, trace, warn}; +use uuid::Uuid; use zcash_client_backend::{ address::UnifiedAddress, @@ -161,30 +162,48 @@ pub(crate) const UA_TRANSPARENT: bool = true; pub(crate) const DEFAULT_UA_REQUEST: UnifiedAddressRequest = UnifiedAddressRequest::unsafe_new(UA_ORCHARD, true, UA_TRANSPARENT); -/// The ID type for accounts. +/// Unique identifier for a specific account tracked by a [`WalletDb`]. +/// +/// Account identifiers are "one-way stable": a given identifier always points to a +/// specific viewing key within a specific [`WalletDb`] instance, but the same viewing key +/// may have multiple account identifiers over time. In particular, this crate upholds the +/// following properties: +/// +/// - When an account starts being tracked within a [`WalletDb`] instance (via APIs like +/// [`WalletWrite::create_account`], [`WalletWrite::import_account_hd`], or +/// [`WalletWrite::import_account_ufvk`]), a new `AccountUuid` is generated. +/// - If an `AccountUuid` is present within a [`WalletDb`], it always points to the same +/// account. +/// +/// What this means is that account identifiers are not stable across "wallet recreation +/// events". Examples of these include: +/// - Restoring a wallet from a backed-up seed. +/// - Importing the same viewing key into two different wallet instances. #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Default)] -pub struct AccountId(u32); +pub struct AccountUuid(Uuid); -impl AccountId { - /// Constructs an `AccountId` from a bare `u32` value. The resulting identifier is not - /// guaranteed to correspond to any account stored in the database. - #[cfg(feature = "unstable")] - pub fn from_u32(value: u32) -> Self { - AccountId(value) +impl AccountUuid { + /// Constructs an `AccountUuid` from a bare [`Uuid`] value. + /// + /// The resulting identifier is not guaranteed to correspond to any account stored in + /// a [`WalletDb`]. Callers should use [`WalletDb::get_account_for_uuid`] + pub fn from_uuid(value: Uuid) -> Self { + AccountUuid(value) } - /// Unwraps a raw `accounts` table primary key value from its typesafe wrapper. - /// - /// Note that account identifiers are not guaranteed to be stable; if a wallet is restored from - /// seed, the account identifiers of the restored wallet are not likely to correspond to the - /// identifiers for the same accounts in another wallet created or restored from the same seed. - /// These unwrapped identifier values should therefore be treated as ephemeral. - #[cfg(feature = "unstable")] - pub fn as_u32(&self) -> u32 { + /// Exposes the opaque account identifier from its typesafe wrapper. + pub fn expose_uuid(&self) -> Uuid { self.0 } } +/// The local identifier for an account. +/// +/// This is an ephemeral value for efficiently and generically working with accounts in a +/// [`WalletDb`]. To reference accounts in external contexts, use [`AccountUuid`]. +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Default)] +pub struct AccountId(u32); + impl ConditionallySelectable for AccountId { fn conditional_select(a: &Self, b: &Self, choice: subtle::Choice) -> Self { AccountId(ConditionallySelectable::conditional_select( @@ -219,6 +238,17 @@ pub struct WalletDb { params: P, } +impl, P: consensus::Parameters> WalletDb { + /// Returns the account with the given "one-way stable" identifier, if it was created + /// by this wallet instance. + pub fn get_account_for_uuid( + &self, + account_uuid: AccountUuid, + ) -> Result, SqliteClientError> { + wallet::get_account_for_uuid(self.conn.borrow(), &self.params, account_uuid) + } +} + /// A wrapper for a SQLite transaction affecting the wallet database. pub struct SqlTransaction<'conn>(pub(crate) &'conn rusqlite::Transaction<'conn>); diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index c4a1263e1..260c51dc4 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -120,7 +120,7 @@ use crate::{ AccountId, SqlTransaction, TransferType, WalletCommitmentTrees, WalletDb, DEFAULT_UA_REQUEST, PRUNING_DEPTH, SAPLING_TABLES_PREFIX, }; -use crate::{TxRef, VERIFY_LOOKAHEAD}; +use crate::{AccountUuid, TxRef, VERIFY_LOOKAHEAD}; #[cfg(feature = "transparent-inputs")] use zcash_primitives::transaction::components::TxOut; @@ -201,11 +201,17 @@ pub(crate) enum ViewingKey { #[derive(Debug, Clone)] pub struct Account { account_id: AccountId, + uuid: AccountUuid, kind: AccountSource, viewing_key: ViewingKey, } impl Account { + /// Returns the "one-way stable" identifier for this account within its [`WalletDb`]. + pub fn uuid(&self) -> AccountUuid { + self.uuid + } + /// Returns the default Unified Address for the account, /// along with the diversifier index that generated it. /// @@ -368,6 +374,8 @@ pub(crate) fn add_account( } // TODO(#1490): check for IVK collisions. + let uuid = AccountUuid(Uuid::new_v4()); + let (hd_seed_fingerprint, hd_account_index, spending_key_available) = match kind { AccountSource::Derived { seed_fingerprint, @@ -425,7 +433,7 @@ pub(crate) fn add_account( RETURNING id; "#, named_params![ - ":uuid": Uuid::new_v4(), + ":uuid": uuid.0, ":account_kind": account_kind_code(kind), ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.to_bytes()), ":hd_account_index": hd_account_index.map(u32::from), @@ -465,6 +473,7 @@ pub(crate) fn add_account( let account = Account { account_id, + uuid, kind, viewing_key, }; @@ -709,7 +718,7 @@ pub(crate) fn get_account_for_ufvk( let transparent_item: Option> = None; let mut stmt = conn.prepare( - "SELECT id, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, has_spend_key + "SELECT id, uuid, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, has_spend_key FROM accounts WHERE orchard_fvk_item_cache = :orchard_fvk_item_cache OR sapling_fvk_item_cache = :sapling_fvk_item_cache @@ -725,6 +734,7 @@ pub(crate) fn get_account_for_ufvk( ], |row| { let account_id = row.get::<_, u32>("id").map(AccountId)?; + let uuid = AccountUuid(row.get("uuid")?); let kind = parse_account_source( row.get("account_kind")?, row.get("hd_seed_fingerprint")?, @@ -746,6 +756,7 @@ pub(crate) fn get_account_for_ufvk( Ok(Account { account_id, + uuid, kind, viewing_key, }) @@ -771,7 +782,7 @@ pub(crate) fn get_derived_account( account_index: zip32::AccountId, ) -> Result, SqliteClientError> { let mut stmt = conn.prepare( - "SELECT id, ufvk + "SELECT id, uuid, ufvk FROM accounts WHERE hd_seed_fingerprint = :hd_seed_fingerprint AND hd_account_index = :account_id", @@ -783,8 +794,9 @@ pub(crate) fn get_derived_account( ":hd_account_index": u32::from(account_index), ], |row| { - let account_id = row.get::<_, u32>(0).map(AccountId)?; - let ufvk = match row.get::<_, Option>(1)? { + let account_id = row.get::<_, u32>("id").map(AccountId)?; + let uuid = AccountUuid(row.get("uuid")?); + let ufvk = match row.get::<_, Option>("ufvk")? { None => Err(SqliteClientError::CorruptedData(format!( "Missing unified full viewing key for derived account {:?}", account_id, @@ -798,6 +810,7 @@ pub(crate) fn get_derived_account( }?; Ok(Account { account_id, + uuid, kind: AccountSource::Derived { seed_fingerprint: *seed, account_index, @@ -1833,6 +1846,30 @@ pub(crate) fn block_height_extrema( }) } +pub(crate) fn get_account_for_uuid( + conn: &rusqlite::Connection, + params: &P, + account_uuid: AccountUuid, +) -> Result, SqliteClientError> { + match conn + .query_row( + "SELECT id FROM accounts WHERE uuid = :uuid", + named_params! {":uuid": account_uuid.0}, + |row| row.get("id").map(AccountId), + ) + .optional()? + { + None => Ok(None), + Some(account_id) => Ok(Some( + // TODO: `get_account` should return a non-optional value now that `AccountId` + // is guaranteed to exist (because it can't be externally constructed), but + // the `WalletRead::AccountId` associated type is permitted to not exist, and + // I don't want to deal with the refactor right now. + get_account(conn, params, account_id)?.expect("account_id exists"), + )), + } +} + pub(crate) fn get_account( conn: &rusqlite::Connection, params: &P, @@ -1840,7 +1877,7 @@ pub(crate) fn get_account( ) -> Result, SqliteClientError> { let mut sql = conn.prepare_cached( r#" - SELECT account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, has_spend_key + SELECT uuid, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, has_spend_key FROM accounts WHERE id = :account_id "#, @@ -1850,6 +1887,8 @@ pub(crate) fn get_account( let row = result.next()?; match row { Some(row) => { + let uuid = AccountUuid(row.get("uuid")?); + let kind = parse_account_source( row.get("account_kind")?, row.get("hd_seed_fingerprint")?, @@ -1873,6 +1912,7 @@ pub(crate) fn get_account( Ok(Some(Account { account_id, + uuid, kind, viewing_key, })) From 546481e8aa9ace23209e19ece66b158a1a8db3e1 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 26 Nov 2024 09:36:48 -0700 Subject: [PATCH 4/5] zcash_client_sqlite: Change `WalletDb::AccountId` associated type to `AccountUuid` This requires a few annoying changes to migrations in order to avoid hitting cases where account UUIDs are expected before they exist in the database schema. --- .github/workflows/ci.yml | 2 +- zcash_client_backend/src/data_api/testing.rs | 7 +- .../src/data_api/testing/pool.rs | 20 +- zcash_client_sqlite/CHANGELOG.md | 16 +- zcash_client_sqlite/src/error.rs | 18 +- zcash_client_sqlite/src/lib.rs | 78 ++-- zcash_client_sqlite/src/testing/db.rs | 4 +- zcash_client_sqlite/src/wallet.rs | 404 ++++++++++-------- zcash_client_sqlite/src/wallet/common.rs | 26 +- zcash_client_sqlite/src/wallet/init.rs | 3 +- .../src/wallet/init/migrations.rs | 2 +- .../init/migrations/ephemeral_addresses.rs | 17 +- .../init/migrations/receiving_key_scopes.rs | 2 +- .../init/migrations/tx_retrieval_queue.rs | 145 +++++-- zcash_client_sqlite/src/wallet/orchard.rs | 48 ++- zcash_client_sqlite/src/wallet/sapling.rs | 44 +- zcash_client_sqlite/src/wallet/transparent.rs | 147 ++++--- .../src/wallet/transparent/ephemeral.rs | 26 +- 18 files changed, 592 insertions(+), 417 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e551e7461..4aab93a21 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -343,7 +343,7 @@ jobs: run: | { echo 'UUIDS<> "$GITHUB_OUTPUT" diff --git a/zcash_client_backend/src/data_api/testing.rs b/zcash_client_backend/src/data_api/testing.rs index 0041c24bc..9504d3288 100644 --- a/zcash_client_backend/src/data_api/testing.rs +++ b/zcash_client_backend/src/data_api/testing.rs @@ -796,7 +796,8 @@ where ::Error: fmt::Debug, ParamsT: consensus::Parameters + Send + 'static, DbT: InputSource + WalletTest + WalletWrite + WalletCommitmentTrees, - ::AccountId: ConditionallySelectable + Default + Send + 'static, + ::AccountId: + std::fmt::Debug + ConditionallySelectable + Default + Send + 'static, { /// Invokes [`scan_cached_blocks`] with the given arguments, expecting success. pub fn scan_cached_blocks(&mut self, from_height: BlockHeight, limit: usize) -> ScanSummary { @@ -855,7 +856,7 @@ where impl TestState where ParamsT: consensus::Parameters + Send + 'static, - AccountIdT: std::cmp::Eq + std::hash::Hash, + AccountIdT: std::fmt::Debug + std::cmp::Eq + std::hash::Hash, ErrT: std::fmt::Debug, DbT: InputSource + WalletTest @@ -1286,7 +1287,7 @@ pub struct InitialChainState { /// Trait representing the ability to construct a new data store for use in a test. pub trait DataStoreFactory { type Error: core::fmt::Debug; - type AccountId: ConditionallySelectable + Default + Hash + Eq + Send + 'static; + type AccountId: std::fmt::Debug + ConditionallySelectable + Default + Hash + Eq + Send + 'static; type Account: Account + Clone; type DsError: core::fmt::Debug; type DataStore: InputSource diff --git a/zcash_client_backend/src/data_api/testing/pool.rs b/zcash_client_backend/src/data_api/testing/pool.rs index 756be223a..c6d5d7bdb 100644 --- a/zcash_client_backend/src/data_api/testing/pool.rs +++ b/zcash_client_backend/src/data_api/testing/pool.rs @@ -1559,7 +1559,7 @@ pub fn external_address_change_spends_detected_in_restore_from_seed write!(f, "ZIP-32 derivation information is not known for this account."), SqliteClientError::KeyDerivationError(acct_id) => write!(f, "Key derivation failed for account {}", u32::from(*acct_id)), 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::Zip32AccountIndexOutOfRange => write!(f, "ZIP 32 account identifiers must be less than 0x7FFFFFFF."), 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."), diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 3aeeee55d..0bc77b210 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -182,11 +182,19 @@ pub(crate) const DEFAULT_UA_REQUEST: UnifiedAddressRequest = #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Default)] pub struct AccountUuid(Uuid); +impl ConditionallySelectable for AccountUuid { + fn conditional_select(a: &Self, b: &Self, choice: subtle::Choice) -> Self { + AccountUuid(Uuid::from_u128( + ConditionallySelectable::conditional_select(&a.0.as_u128(), &b.0.as_u128(), choice), + )) + } +} + impl AccountUuid { /// Constructs an `AccountUuid` from a bare [`Uuid`] value. /// /// The resulting identifier is not guaranteed to correspond to any account stored in - /// a [`WalletDb`]. Callers should use [`WalletDb::get_account_for_uuid`] + /// a [`WalletDb`]. pub fn from_uuid(value: Uuid) -> Self { AccountUuid(value) } @@ -202,8 +210,10 @@ impl AccountUuid { /// This is an ephemeral value for efficiently and generically working with accounts in a /// [`WalletDb`]. To reference accounts in external contexts, use [`AccountUuid`]. #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Default)] -pub struct AccountId(u32); +pub(crate) struct AccountId(u32); +/// This implementation is retained under `#[cfg(test)]` for pre-AccountUuid testing. +#[cfg(test)] impl ConditionallySelectable for AccountId { fn conditional_select(a: &Self, b: &Self, choice: subtle::Choice) -> Self { AccountId(ConditionallySelectable::conditional_select( @@ -238,17 +248,6 @@ pub struct WalletDb { params: P, } -impl, P: consensus::Parameters> WalletDb { - /// Returns the account with the given "one-way stable" identifier, if it was created - /// by this wallet instance. - pub fn get_account_for_uuid( - &self, - account_uuid: AccountUuid, - ) -> Result, SqliteClientError> { - wallet::get_account_for_uuid(self.conn.borrow(), &self.params, account_uuid) - } -} - /// A wrapper for a SQLite transaction affecting the wallet database. pub struct SqlTransaction<'conn>(pub(crate) &'conn rusqlite::Transaction<'conn>); @@ -285,7 +284,7 @@ impl WalletDb { impl, P: consensus::Parameters> InputSource for WalletDb { type Error = SqliteClientError; type NoteRef = ReceivedNoteId; - type AccountId = AccountId; + type AccountId = AccountUuid; fn get_spendable_note( &self, @@ -319,7 +318,7 @@ impl, P: consensus::Parameters> InputSource for fn select_spendable_notes( &self, - account: AccountId, + account: Self::AccountId, target_value: NonNegativeAmount, sources: &[ShieldedProtocol], anchor_height: BlockHeight, @@ -415,10 +414,10 @@ impl, P: consensus::Parameters> InputSource for impl, P: consensus::Parameters> WalletRead for WalletDb { type Error = SqliteClientError; - type AccountId = AccountId; + type AccountId = AccountUuid; type Account = wallet::Account; - fn get_account_ids(&self) -> Result, Self::Error> { + fn get_account_ids(&self) -> Result, Self::Error> { Ok(wallet::get_account_ids(self.conn.borrow())?) } @@ -522,13 +521,13 @@ impl, P: consensus::Parameters> WalletRead for W fn get_current_address( &self, - account: AccountId, + account: Self::AccountId, ) -> Result, Self::Error> { wallet::get_current_address(self.conn.borrow(), &self.params, account) .map(|res| res.map(|(addr, _)| addr)) } - fn get_account_birthday(&self, account: AccountId) -> Result { + fn get_account_birthday(&self, account: Self::AccountId) -> Result { wallet::account_birthday(self.conn.borrow(), account).map_err(SqliteClientError::from) } @@ -593,7 +592,7 @@ impl, P: consensus::Parameters> WalletRead for W fn get_unified_full_viewing_keys( &self, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { wallet::get_unified_full_viewing_keys(self.conn.borrow(), &self.params) } @@ -614,7 +613,7 @@ impl, P: consensus::Parameters> WalletRead for W fn get_sapling_nullifiers( &self, query: NullifierQuery, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { wallet::sapling::get_sapling_nullifiers(self.conn.borrow(), query) } @@ -622,14 +621,14 @@ impl, P: consensus::Parameters> WalletRead for W fn get_orchard_nullifiers( &self, query: NullifierQuery, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { wallet::orchard::get_orchard_nullifiers(self.conn.borrow(), query) } #[cfg(feature = "transparent-inputs")] fn get_transparent_receivers( &self, - account: AccountId, + account: Self::AccountId, ) -> Result>, Self::Error> { wallet::transparent::get_transparent_receivers(self.conn.borrow(), &self.params, account) } @@ -637,7 +636,7 @@ impl, P: consensus::Parameters> WalletRead for W #[cfg(feature = "transparent-inputs")] fn get_transparent_balances( &self, - account: AccountId, + account: Self::AccountId, max_height: BlockHeight, ) -> Result, Self::Error> { wallet::transparent::get_transparent_balances( @@ -668,10 +667,11 @@ impl, P: consensus::Parameters> WalletRead for W account: Self::AccountId, index_range: Option>, ) -> Result, Self::Error> { + let account_id = wallet::get_account_id(self.conn.borrow(), account)?; wallet::transparent::ephemeral::get_known_ephemeral_addresses( self.conn.borrow(), &self.params, - account, + account_id, index_range, ) } @@ -839,7 +839,7 @@ impl WalletWrite for WalletDb &mut self, seed: &SecretVec, birthday: &AccountBirthday, - ) -> Result<(AccountId, UnifiedSpendingKey), Self::Error> { + ) -> Result<(Self::AccountId, UnifiedSpendingKey), Self::Error> { self.transactionally(|wdb| { let seed_fingerprint = SeedFingerprint::from_seed(seed.expose_secret()).ok_or_else(|| { @@ -848,7 +848,10 @@ impl WalletWrite for WalletDb ) })?; let account_index = wallet::max_zip32_account_index(wdb.conn.0, &seed_fingerprint)? - .map(|a| a.next().ok_or(SqliteClientError::AccountIdOutOfRange)) + .map(|a| { + a.next() + .ok_or(SqliteClientError::Zip32AccountIndexOutOfRange) + }) .transpose()? .unwrap_or(zip32::AccountId::ZERO); @@ -925,14 +928,14 @@ impl WalletWrite for WalletDb fn get_next_available_address( &mut self, - account: AccountId, + account_uuid: Self::AccountId, request: UnifiedAddressRequest, ) -> Result, Self::Error> { self.transactionally( - |wdb| match wdb.get_unified_full_viewing_keys()?.get(&account) { + |wdb| match wdb.get_unified_full_viewing_keys()?.get(&account_uuid) { Some(ufvk) => { let search_from = - match wallet::get_current_address(wdb.conn.0, &wdb.params, account)? { + match wallet::get_current_address(wdb.conn.0, &wdb.params, account_uuid)? { Some((_, mut last_diversifier_index)) => { last_diversifier_index.increment().map_err(|_| { AddressGenerationError::DiversifierSpaceExhausted @@ -944,10 +947,11 @@ impl WalletWrite for WalletDb let (addr, diversifier_index) = ufvk.find_address(search_from, request)?; + let account_id = wallet::get_account_id(wdb.conn.0, account_uuid)?; wallet::insert_address( wdb.conn.0, &wdb.params, - account, + account_id, diversifier_index, &addr, )?; @@ -1413,14 +1417,14 @@ impl WalletWrite for WalletDb fn store_decrypted_tx( &mut self, - d_tx: DecryptedTransaction, + d_tx: DecryptedTransaction, ) -> Result<(), Self::Error> { self.transactionally(|wdb| wallet::store_decrypted_tx(wdb.conn.0, &wdb.params, d_tx)) } fn store_transactions_to_be_sent( &mut self, - transactions: &[SentTransaction], + transactions: &[SentTransaction], ) -> Result<(), Self::Error> { self.transactionally(|wdb| { for sent_tx in transactions { @@ -1441,6 +1445,7 @@ impl WalletWrite for WalletDb n: usize, ) -> Result, Self::Error> { self.transactionally(|wdb| { + let account_id = wallet::get_account_id(wdb.conn.0, account_id)?; wallet::transparent::ephemeral::reserve_next_n_ephemeral_addresses( wdb.conn.0, &wdb.params, @@ -1916,6 +1921,7 @@ extern crate assert_matches; #[cfg(test)] mod tests { use secrecy::{ExposeSecret, Secret, SecretVec}; + use uuid::Uuid; use zcash_client_backend::data_api::{ chain::ChainState, testing::{TestBuilder, TestState}, @@ -1927,7 +1933,7 @@ mod tests { use zcash_protocol::consensus; use crate::{ - error::SqliteClientError, testing::db::TestDbFactory, AccountId, DEFAULT_UA_REQUEST, + error::SqliteClientError, testing::db::TestDbFactory, AccountUuid, DEFAULT_UA_REQUEST, }; #[cfg(feature = "unstable")] @@ -1952,9 +1958,9 @@ mod tests { // check that passing an invalid account results in a failure assert!({ - let wrong_account_index = AccountId(3); + let wrong_account_uuid = AccountUuid(Uuid::nil()); !st.wallet() - .validate_seed(wrong_account_index, st.test_seed().unwrap()) + .validate_seed(wrong_account_uuid, st.test_seed().unwrap()) .unwrap() }); diff --git a/zcash_client_sqlite/src/testing/db.rs b/zcash_client_sqlite/src/testing/db.rs index 84ea3b4fd..e11cc9f97 100644 --- a/zcash_client_sqlite/src/testing/db.rs +++ b/zcash_client_sqlite/src/testing/db.rs @@ -35,7 +35,7 @@ use zcash_protocol::{consensus::BlockHeight, local_consensus::LocalNetwork, memo use crate::{ error::SqliteClientError, wallet::init::{init_wallet_db, init_wallet_db_internal}, - AccountId, WalletDb, + AccountUuid, WalletDb, }; #[cfg(feature = "transparent-inputs")] @@ -163,7 +163,7 @@ impl TestDbFactory { impl DataStoreFactory for TestDbFactory { type Error = (); - type AccountId = AccountId; + type AccountId = AccountUuid; type Account = crate::wallet::Account; type DsError = SqliteClientError; type DataStore = TestDb; diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 260c51dc4..04d9496f7 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -66,7 +66,7 @@ use incrementalmerkletree::{Marking, Retention}; -use rusqlite::{self, named_params, params, OptionalExtension}; +use rusqlite::{self, named_params, params, Connection, OptionalExtension}; use secrecy::{ExposeSecret, SecretVec}; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; use uuid::Uuid; @@ -200,18 +200,12 @@ pub(crate) enum ViewingKey { /// An account stored in a `zcash_client_sqlite` database. #[derive(Debug, Clone)] pub struct Account { - account_id: AccountId, uuid: AccountUuid, kind: AccountSource, viewing_key: ViewingKey, } impl Account { - /// Returns the "one-way stable" identifier for this account within its [`WalletDb`]. - pub fn uuid(&self) -> AccountUuid { - self.uuid - } - /// Returns the default Unified Address for the account, /// along with the diversifier index that generated it. /// @@ -226,10 +220,10 @@ impl Account { } impl zcash_client_backend::data_api::Account for Account { - type AccountId = AccountId; + type AccountId = AccountUuid; - fn id(&self) -> AccountId { - self.account_id + fn id(&self) -> AccountUuid { + self.uuid } fn source(&self) -> AccountSource { @@ -350,11 +344,10 @@ pub(crate) fn max_zip32_account_index( "SELECT MAX(hd_account_index) FROM accounts WHERE hd_seed_fingerprint = :hd_seed", [seed_id.to_bytes()], |row| { - let account_id: Option = row.get(0)?; - account_id + row.get::<_, Option>(0)? .map(zip32::AccountId::try_from) .transpose() - .map_err(|_| SqliteClientError::AccountIdOutOfRange) + .map_err(|_| SqliteClientError::Zip32AccountIndexOutOfRange) }, ) } @@ -374,7 +367,7 @@ pub(crate) fn add_account( } // TODO(#1490): check for IVK collisions. - let uuid = AccountUuid(Uuid::new_v4()); + let account_uuid = AccountUuid(Uuid::new_v4()); let (hd_seed_fingerprint, hd_account_index, spending_key_available) = match kind { AccountSource::Derived { @@ -409,7 +402,7 @@ pub(crate) fn add_account( let birthday_orchard_tree_size: Option = None; let ufvk_encoded = viewing_key.ufvk().map(|ufvk| ufvk.encode(params)); - let account_id: AccountId = conn + let account_id = conn .query_row( r#" INSERT INTO accounts ( @@ -430,10 +423,10 @@ pub(crate) fn add_account( :recover_until_height, :has_spend_key ) - RETURNING id; + RETURNING id "#, named_params![ - ":uuid": uuid.0, + ":uuid": account_uuid.0, ":account_kind": account_kind_code(kind), ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.to_bytes()), ":hd_account_index": hd_account_index.map(u32::from), @@ -448,7 +441,7 @@ pub(crate) fn add_account( ":recover_until_height": birthday.recover_until().map(u32::from), ":has_spend_key": spending_key_available as i64, ], - |row| Ok(AccountId(row.get(0)?)), + |row| row.get(0).map(AccountId), ) .map_err(|e| match e { rusqlite::Error::SqliteFailure(f, s) @@ -458,12 +451,12 @@ pub(crate) fn add_account( // the check using `get_account_for_ufvk` above, but in case it wasn't, // make a best effort to determine the AccountId of the pre-existing row // and provide that to our caller. - if let Ok(id) = conn.query_row( - "SELECT id FROM accounts WHERE ufvk = ?", + if let Ok(colliding_uuid) = conn.query_row( + "SELECT uuid FROM accounts WHERE ufvk = ?", params![ufvk_encoded], - |row| Ok(AccountId(row.get(0)?)), + |row| Ok(AccountUuid(row.get(0)?)), ) { - return SqliteClientError::AccountCollision(id); + return SqliteClientError::AccountCollision(colliding_uuid); } SqliteClientError::from(rusqlite::Error::SqliteFailure(f, s)) @@ -472,8 +465,7 @@ pub(crate) fn add_account( })?; let account = Account { - account_id, - uuid, + uuid: account_uuid, kind, viewing_key, }; @@ -594,16 +586,18 @@ pub(crate) fn add_account( pub(crate) fn get_current_address( conn: &rusqlite::Connection, params: &P, - account_id: AccountId, + account_uuid: AccountUuid, ) -> Result, SqliteClientError> { // This returns the most recently generated address. let addr: Option<(String, Vec)> = conn .query_row( "SELECT address, diversifier_index_be - FROM addresses WHERE account_id = :account_id - ORDER BY diversifier_index_be DESC - LIMIT 1", - named_params![":account_id": account_id.0], + FROM addresses + JOIN accounts ON addresses.account_id = accounts.id + WHERE accounts.uuid = :account_uuid + ORDER BY diversifier_index_be DESC + LIMIT 1", + named_params![":account_uuid": account_uuid.0], |row| Ok((row.get(0)?, row.get(1)?)), ) .optional()?; @@ -636,10 +630,10 @@ pub(crate) fn get_current_address( pub(crate) fn insert_address( conn: &rusqlite::Connection, params: &P, - account: AccountId, + account_id: AccountId, diversifier_index: DiversifierIndex, address: &UnifiedAddress, -) -> Result<(), rusqlite::Error> { +) -> Result<(), SqliteClientError> { let mut stmt = conn.prepare_cached( "INSERT INTO addresses ( account_id, @@ -648,7 +642,7 @@ pub(crate) fn insert_address( cached_transparent_receiver_address ) VALUES ( - :account, + :account_id, :diversifier_index_be, :address, :cached_transparent_receiver_address @@ -659,7 +653,7 @@ pub(crate) fn insert_address( let mut di_be = *diversifier_index.as_bytes(); di_be.reverse(); stmt.execute(named_params![ - ":account": account.0, + ":account_id": account_id.0, ":diversifier_index_be": &di_be[..], ":address": &address.encode(params), ":cached_transparent_receiver_address": &address.transparent().map(|r| r.encode(params)), @@ -672,23 +666,22 @@ pub(crate) fn insert_address( pub(crate) fn get_unified_full_viewing_keys( conn: &rusqlite::Connection, params: &P, -) -> Result, SqliteClientError> { +) -> Result, SqliteClientError> { // Fetch the UnifiedFullViewingKeys we are tracking - let mut stmt_fetch_accounts = conn.prepare("SELECT id, ufvk FROM accounts")?; + let mut stmt_fetch_accounts = conn.prepare("SELECT uuid, ufvk FROM accounts")?; let rows = stmt_fetch_accounts.query_map([], |row| { - let acct: u32 = row.get(0)?; let ufvk_str: Option = row.get(1)?; if let Some(ufvk_str) = ufvk_str { let ufvk = UnifiedFullViewingKey::decode(params, &ufvk_str) .map_err(SqliteClientError::CorruptedData); - Ok(Some((AccountId(acct), ufvk))) + Ok(Some((AccountUuid(row.get(0)?), ufvk))) } else { Ok(None) } })?; - let mut res: HashMap = HashMap::new(); + let mut res: HashMap = HashMap::new(); for row in rows { if let Some((account_id, ufvkr)) = row? { res.insert(account_id, ufvkr?); @@ -718,11 +711,11 @@ pub(crate) fn get_account_for_ufvk( let transparent_item: Option> = None; let mut stmt = conn.prepare( - "SELECT id, uuid, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, has_spend_key - FROM accounts - WHERE orchard_fvk_item_cache = :orchard_fvk_item_cache - OR sapling_fvk_item_cache = :sapling_fvk_item_cache - OR p2pkh_fvk_item_cache = :p2pkh_fvk_item_cache", + "SELECT uuid, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, has_spend_key + FROM accounts + WHERE orchard_fvk_item_cache = :orchard_fvk_item_cache + OR sapling_fvk_item_cache = :sapling_fvk_item_cache + OR p2pkh_fvk_item_cache = :p2pkh_fvk_item_cache", )?; let accounts = stmt @@ -733,8 +726,7 @@ pub(crate) fn get_account_for_ufvk( ":p2pkh_fvk_item_cache": transparent_item, ], |row| { - let account_id = row.get::<_, u32>("id").map(AccountId)?; - let uuid = AccountUuid(row.get("uuid")?); + let account_uuid = AccountUuid(row.get("uuid")?); let kind = parse_account_source( row.get("account_kind")?, row.get("hd_seed_fingerprint")?, @@ -748,15 +740,14 @@ pub(crate) fn get_account_for_ufvk( let viewing_key = ViewingKey::Full(Box::new( UnifiedFullViewingKey::decode(params, &ufvk_str).map_err(|e| { SqliteClientError::CorruptedData(format!( - "Could not decode unified full viewing key for account {:?}: {}", - account_id, e + "Could not decode unified full viewing key for account {}: {}", + account_uuid.0, e )) })?, )); Ok(Account { - account_id, - uuid, + uuid: account_uuid, kind, viewing_key, }) @@ -778,41 +769,39 @@ pub(crate) fn get_account_for_ufvk( pub(crate) fn get_derived_account( conn: &rusqlite::Connection, params: &P, - seed: &SeedFingerprint, + seed_fp: &SeedFingerprint, account_index: zip32::AccountId, ) -> Result, SqliteClientError> { let mut stmt = conn.prepare( - "SELECT id, uuid, ufvk - FROM accounts - WHERE hd_seed_fingerprint = :hd_seed_fingerprint - AND hd_account_index = :account_id", + "SELECT uuid, ufvk + FROM accounts + WHERE hd_seed_fingerprint = :hd_seed_fingerprint + AND hd_account_index = :hd_account_index", )?; let mut accounts = stmt.query_and_then::<_, SqliteClientError, _, _>( named_params![ - ":hd_seed_fingerprint": seed.to_bytes(), + ":hd_seed_fingerprint": seed_fp.to_bytes(), ":hd_account_index": u32::from(account_index), ], |row| { - let account_id = row.get::<_, u32>("id").map(AccountId)?; - let uuid = AccountUuid(row.get("uuid")?); + let account_uuid = AccountUuid(row.get("uuid")?); let ufvk = match row.get::<_, Option>("ufvk")? { None => Err(SqliteClientError::CorruptedData(format!( - "Missing unified full viewing key for derived account {:?}", - account_id, + "Missing unified full viewing key for derived account {}", + account_uuid.0, ))), Some(ufvk_str) => UnifiedFullViewingKey::decode(params, &ufvk_str).map_err(|e| { SqliteClientError::CorruptedData(format!( - "Could not decode unified full viewing key for account {:?}: {}", - account_id, e + "Could not decode unified full viewing key for account {}: {}", + account_uuid.0, e )) }), }?; Ok(Account { - account_id, - uuid, + uuid: account_uuid, kind: AccountSource::Derived { - seed_fingerprint: *seed, + seed_fingerprint: *seed_fp, account_index, }, viewing_key: ViewingKey::Full(Box::new(ufvk)), @@ -1306,7 +1295,7 @@ pub(crate) fn get_wallet_summary( params: &P, min_confirmations: u32, progress: &impl ProgressEstimator, -) -> Result>, SqliteClientError> { +) -> Result>, SqliteClientError> { let chain_tip_height = match chain_tip_height(tx)? { Some(h) => h, None => { @@ -1377,18 +1366,18 @@ pub(crate) fn get_wallet_summary( None => return Ok(None), }; - let mut stmt_accounts = tx.prepare_cached("SELECT id FROM accounts")?; + let mut stmt_accounts = tx.prepare_cached("SELECT uuid FROM accounts")?; let mut account_balances = stmt_accounts .query([])? .and_then(|row| { - Ok::<_, SqliteClientError>((AccountId(row.get::<_, u32>(0)?), AccountBalance::ZERO)) + Ok::<_, SqliteClientError>((AccountUuid(row.get::<_, Uuid>(0)?), AccountBalance::ZERO)) }) - .collect::, _>>()?; + .collect::, _>>()?; fn count_notes( tx: &rusqlite::Transaction, summary_height: BlockHeight, - account_balances: &mut HashMap, + account_balances: &mut HashMap, table_prefix: &'static str, with_pool_balance: F, ) -> Result<(), SqliteClientError> @@ -1426,8 +1415,9 @@ pub(crate) fn get_wallet_summary( let any_spendable = is_any_spendable(tx, summary_height, table_prefix)?; let mut stmt_select_notes = tx.prepare_cached(&format!( - "SELECT n.account_id, n.value, n.is_change, scan_state.max_priority, t.block + "SELECT a.uuid, n.value, n.is_change, scan_state.max_priority, t.block FROM {table_prefix}_received_notes n + JOIN accounts a ON a.id = n.account_id JOIN transactions t ON t.id_tx = n.tx LEFT OUTER JOIN v_{table_prefix}_shards_scan_state scan_state ON n.commitment_tree_position >= scan_state.start_position @@ -1451,7 +1441,7 @@ pub(crate) fn get_wallet_summary( let mut rows = stmt_select_notes.query(named_params![":summary_height": u32::from(summary_height)])?; while let Some(row) = rows.next()? { - let account = AccountId(row.get::<_, u32>(0)?); + let account = AccountUuid(row.get::<_, Uuid>(0)?); let value_raw = row.get::<_, i64>(1)?; let value = NonNegativeAmount::from_nonnegative_i64(value_raw).map_err(|_| { @@ -1723,7 +1713,7 @@ pub(crate) fn get_transaction( pub(crate) fn get_funding_accounts( conn: &rusqlite::Connection, tx: &Transaction, -) -> Result, rusqlite::Error> { +) -> Result, rusqlite::Error> { let mut funding_accounts = HashSet::new(); #[cfg(feature = "transparent-inputs")] funding_accounts.extend(transparent::detect_spending_accounts( @@ -1805,13 +1795,13 @@ pub(crate) fn wallet_birthday( pub(crate) fn account_birthday( conn: &rusqlite::Connection, - account: AccountId, + account_uuid: AccountUuid, ) -> Result { conn.query_row( "SELECT birthday_height FROM accounts - WHERE id = :account_id", - named_params![":account_id": account.0], + WHERE uuid = :account_uuid", + named_params![":account_uuid": account_uuid.0], |row| row.get::<_, u32>(0).map(BlockHeight::from), ) .optional() @@ -1846,49 +1836,50 @@ pub(crate) fn block_height_extrema( }) } -pub(crate) fn get_account_for_uuid( +pub(crate) fn get_account_id( conn: &rusqlite::Connection, - params: &P, account_uuid: AccountUuid, -) -> Result, SqliteClientError> { - match conn - .query_row( - "SELECT id FROM accounts WHERE uuid = :uuid", - named_params! {":uuid": account_uuid.0}, - |row| row.get("id").map(AccountId), - ) - .optional()? - { - None => Ok(None), - Some(account_id) => Ok(Some( - // TODO: `get_account` should return a non-optional value now that `AccountId` - // is guaranteed to exist (because it can't be externally constructed), but - // the `WalletRead::AccountId` associated type is permitted to not exist, and - // I don't want to deal with the refactor right now. - get_account(conn, params, account_id)?.expect("account_id exists"), - )), - } +) -> Result { + conn.query_row( + "SELECT id FROM accounts WHERE uuid = :account_uuid", + named_params! {":account_uuid": account_uuid.0}, + |row| row.get("id").map(AccountId), + ) + .optional()? + .ok_or(SqliteClientError::AccountUnknown) +} + +#[cfg(feature = "transparent-inputs")] +pub(crate) fn get_account_uuid( + conn: &rusqlite::Connection, + account_id: AccountId, +) -> Result { + conn.query_row( + "SELECT uuid FROM accounts WHERE id = :account_id", + named_params! {":account_id": account_id.0}, + |row| row.get("uuid").map(AccountUuid), + ) + .optional()? + .ok_or(SqliteClientError::AccountUnknown) } pub(crate) fn get_account( conn: &rusqlite::Connection, params: &P, - account_id: AccountId, + account_uuid: AccountUuid, ) -> Result, SqliteClientError> { let mut sql = conn.prepare_cached( r#" - SELECT uuid, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, has_spend_key + SELECT account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, has_spend_key FROM accounts - WHERE id = :account_id + WHERE uuid = :account_uuid "#, )?; - let mut result = sql.query(named_params![":account_id": account_id.0])?; + let mut result = sql.query(named_params![":account_uuid": account_uuid.0])?; let row = result.next()?; match row { Some(row) => { - let uuid = AccountUuid(row.get("uuid")?); - let kind = parse_account_source( row.get("account_kind")?, row.get("hd_seed_fingerprint")?, @@ -1911,8 +1902,7 @@ pub(crate) fn get_account( }; Ok(Some(Account { - account_id, - uuid, + uuid: account_uuid, kind, viewing_key, })) @@ -2182,7 +2172,7 @@ pub(crate) fn get_max_height_hash( pub(crate) fn store_transaction_to_be_sent( wdb: &mut WalletDb, P>, - sent_tx: &SentTransaction, + sent_tx: &SentTransaction, ) -> Result<(), SqliteClientError> { let tx_ref = put_tx_data( wdb.conn.0, @@ -2553,12 +2543,12 @@ pub(crate) fn truncate_to_height( /// Note that this is called from db migration code. pub(crate) fn get_account_ids( conn: &rusqlite::Connection, -) -> Result, rusqlite::Error> { - let mut stmt = conn.prepare("SELECT id FROM accounts")?; +) -> Result, rusqlite::Error> { + let mut stmt = conn.prepare("SELECT uuid FROM accounts")?; let mut rows = stmt.query([])?; let mut result = Vec::new(); while let Some(row) = rows.next()? { - let id = AccountId(row.get(0)?); + let id = AccountUuid(row.get(0)?); result.push(id); } Ok(result) @@ -2668,7 +2658,7 @@ pub(crate) fn put_block( pub(crate) fn store_decrypted_tx( conn: &rusqlite::Transaction, params: &P, - d_tx: DecryptedTransaction, + d_tx: DecryptedTransaction, ) -> Result<(), SqliteClientError> { let tx_ref = put_tx_data(conn, d_tx.tx(), None, None, None)?; if let Some(height) = d_tx.mined_height() { @@ -2904,14 +2894,14 @@ pub(crate) fn store_decrypted_tx( // If the output belongs to the wallet, add it to `transparent_received_outputs`. #[cfg(feature = "transparent-inputs")] - if let Some(account_id) = - transparent::find_account_for_transparent_address(conn, params, &address)? + if let Some(account_uuid) = + transparent::find_account_uuid_for_transparent_address(conn, params, &address)? { debug!( "{:?} output {} belongs to account {:?}", d_tx.tx().txid(), output_index, - account_id + account_uuid ); transparent::put_transparent_output( conn, @@ -2923,7 +2913,7 @@ pub(crate) fn store_decrypted_tx( txout, d_tx.mined_height(), &address, - account_id, + account_uuid, false, )?; @@ -2954,12 +2944,12 @@ pub(crate) fn store_decrypted_tx( // If a transaction we observe contains spends from our wallet, we will // store its transparent outputs in the same way they would be stored by // create_spend_to_address. - if let Some(account_id) = funding_account { + if let Some(account_uuid) = funding_account { let receiver = Receiver::Transparent(address); #[cfg(feature = "transparent-inputs")] let recipient_addr = - select_receiving_address(params, conn, account_id, &receiver)? + select_receiving_address(params, conn, account_uuid, &receiver)? .unwrap_or_else(|| receiver.to_zcash_address(params.network_type())); #[cfg(not(feature = "transparent-inputs"))] @@ -2970,7 +2960,7 @@ pub(crate) fn store_decrypted_tx( put_sent_output( conn, params, - account_id, + account_uuid, tx_ref, output_index, &recipient, @@ -3001,14 +2991,7 @@ pub(crate) fn store_decrypted_tx( // part) by this wallet. #[cfg(feature = "transparent-inputs")] if tx_has_wallet_outputs { - if let Some(b) = d_tx.tx().transparent_bundle() { - // queue the transparent inputs for enhancement - queue_tx_retrieval( - conn, - b.vin.iter().map(|txin| *txin.prevout.txid()), - Some(tx_ref), - )?; - } + queue_transparent_input_retrieval(conn, tx_ref, &d_tx)? } notify_tx_retrieved(conn, d_tx.tx().txid())?; @@ -3016,16 +2999,7 @@ pub(crate) fn store_decrypted_tx( // If the decrypted transaction is unmined and has no shielded components, add it to // the queue for status retrieval. #[cfg(feature = "transparent-inputs")] - { - let detectable_via_scanning = d_tx.tx().sapling_bundle().is_some(); - #[cfg(feature = "orchard")] - let detectable_via_scanning = - detectable_via_scanning | d_tx.tx().orchard_bundle().is_some(); - - if d_tx.mined_height().is_none() && !detectable_via_scanning { - queue_tx_retrieval(conn, std::iter::once(d_tx.tx().txid()), None)?; - } - } + queue_unmined_tx_retrieval(conn, &d_tx)?; Ok(()) } @@ -3034,7 +3008,7 @@ pub(crate) fn store_decrypted_tx( /// contain a note related to this wallet into the database. pub(crate) fn put_tx_meta( conn: &rusqlite::Connection, - tx: &WalletTx, + tx: &WalletTx, height: BlockHeight, ) -> Result { // It isn't there, so insert our transaction into the database. @@ -3065,7 +3039,7 @@ pub(crate) fn put_tx_meta( pub(crate) fn select_receiving_address( _params: &P, conn: &rusqlite::Connection, - account: AccountId, + account: AccountUuid, receiver: &Receiver, ) -> Result, SqliteClientError> { match receiver { @@ -3085,10 +3059,14 @@ pub(crate) fn select_receiving_address( .transpose() .map_err(SqliteClientError::from), receiver => { - let mut stmt = - conn.prepare_cached("SELECT address FROM addresses WHERE account_id = :account")?; + let mut stmt = conn.prepare_cached( + "SELECT address + FROM addresses + JOIN accounts ON accounts.id = addresses.account_id + WHERE accounts.uuid = :account_uuid", + )?; - let mut result = stmt.query(named_params! { ":account": account.0 })?; + let mut result = stmt.query(named_params! { ":account_uuid": account.0 })?; while let Some(row) = result.next()? { let addr_str = row.get::<_, String>(0)?; let decoded = addr_str.parse::()?; @@ -3161,6 +3139,40 @@ impl TxQueryType { } } +#[cfg(feature = "transparent-inputs")] +pub(crate) fn queue_transparent_input_retrieval( + conn: &rusqlite::Transaction<'_>, + tx_ref: TxRef, + d_tx: &DecryptedTransaction<'_, AccountId>, +) -> Result<(), SqliteClientError> { + if let Some(b) = d_tx.tx().transparent_bundle() { + // queue the transparent inputs for enhancement + queue_tx_retrieval( + conn, + b.vin.iter().map(|txin| *txin.prevout.txid()), + Some(tx_ref), + )?; + } + + Ok(()) +} + +#[cfg(feature = "transparent-inputs")] +pub(crate) fn queue_unmined_tx_retrieval( + conn: &rusqlite::Transaction<'_>, + d_tx: &DecryptedTransaction<'_, AccountId>, +) -> Result<(), SqliteClientError> { + let detectable_via_scanning = d_tx.tx().sapling_bundle().is_some(); + #[cfg(feature = "orchard")] + let detectable_via_scanning = detectable_via_scanning | d_tx.tx().orchard_bundle().is_some(); + + if d_tx.mined_height().is_none() && !detectable_via_scanning { + queue_tx_retrieval(conn, std::iter::once(d_tx.tx().txid()), None)? + } + + Ok(()) +} + pub(crate) fn queue_tx_retrieval( conn: &rusqlite::Transaction<'_>, txids: impl Iterator, @@ -3241,29 +3253,40 @@ pub(crate) fn notify_tx_retrieved( // A utility function for creation of parameters for use in `insert_sent_output` // and `put_sent_output` fn recipient_params( + conn: &Connection, params: &P, - to: &Recipient, -) -> (Option, Option, PoolType) { + from: AccountUuid, + to: &Recipient, +) -> Result<(AccountId, Option, Option, PoolType), SqliteClientError> { + let from_account_id = get_account_id(conn, from)?; match to { - Recipient::External(addr, pool) => (Some(addr.encode()), None, *pool), + Recipient::External(addr, pool) => Ok((from_account_id, Some(addr.encode()), None, *pool)), Recipient::EphemeralTransparent { receiving_account, ephemeral_address, .. - } => ( - Some(ephemeral_address.encode(params)), - Some(*receiving_account), - PoolType::TRANSPARENT, - ), + } => { + let to_account = get_account_id(conn, *receiving_account)?; + Ok(( + from_account_id, + Some(ephemeral_address.encode(params)), + Some(to_account), + PoolType::TRANSPARENT, + )) + } Recipient::InternalAccount { receiving_account, external_address, note, - } => ( - external_address.as_ref().map(|a| a.encode()), - Some(*receiving_account), - PoolType::Shielded(note.protocol()), - ), + } => { + let to_account = get_account_id(conn, *receiving_account)?; + Ok(( + from_account_id, + external_address.as_ref().map(|a| a.encode()), + Some(to_account), + PoolType::Shielded(note.protocol()), + )) + } } } @@ -3274,7 +3297,7 @@ fn flag_previously_received_change( let flag_received_change = |table_prefix| { conn.execute( &format!( - "UPDATE {table_prefix}_received_notes + "UPDATE {table_prefix}_received_notes SET is_change = 1 FROM sent_notes sn WHERE sn.tx = {table_prefix}_received_notes.tx @@ -3301,24 +3324,25 @@ pub(crate) fn insert_sent_output( conn: &rusqlite::Transaction, params: &P, tx_ref: TxRef, - from_account: AccountId, - output: &SentTransactionOutput, + from_account_uuid: AccountUuid, + output: &SentTransactionOutput, ) -> Result<(), SqliteClientError> { let mut stmt_insert_sent_output = conn.prepare_cached( "INSERT INTO sent_notes ( tx, output_pool, output_index, from_account_id, to_address, to_account_id, value, memo) - VALUES ( + VALUES ( :tx, :output_pool, :output_index, :from_account_id, :to_address, :to_account_id, :value, :memo)", )?; - let (to_address, to_account_id, pool_type) = recipient_params(params, output.recipient()); + let (from_account_id, to_address, to_account_id, pool_type) = + recipient_params(conn, params, from_account_uuid, output.recipient())?; let sql_args = named_params![ ":tx": tx_ref.0, ":output_pool": &pool_code(pool_type), ":output_index": &i64::try_from(output.output_index()).unwrap(), - ":from_account_id": from_account.0, + ":from_account_id": from_account_id.0, ":to_address": &to_address, ":to_account_id": to_account_id.map(|a| a.0), ":value": &i64::from(Amount::from(output.value())), @@ -3346,10 +3370,10 @@ pub(crate) fn insert_sent_output( pub(crate) fn put_sent_output( conn: &rusqlite::Transaction, params: &P, - from_account: AccountId, + from_account_uuid: AccountUuid, tx_ref: TxRef, output_index: usize, - recipient: &Recipient, + recipient: &Recipient, value: NonNegativeAmount, memo: Option<&MemoBytes>, ) -> Result<(), SqliteClientError> { @@ -3368,12 +3392,13 @@ pub(crate) fn put_sent_output( memo = IFNULL(:memo, memo)", )?; - let (to_address, to_account_id, pool_type) = recipient_params(params, recipient); + let (from_account_id, to_address, to_account_id, pool_type) = + recipient_params(conn, params, from_account_uuid, recipient)?; let sql_args = named_params![ ":tx": tx_ref.0, ":output_pool": &pool_code(pool_type), ":output_index": &i64::try_from(output_index).unwrap(), - ":from_account_id": from_account.0, + ":from_account_id": from_account_id.0, ":to_address": &to_address, ":to_account_id": &to_account_id.map(|a| a.0), ":value": &i64::from(Amount::from(value)), @@ -3561,43 +3586,46 @@ pub mod testing { ShieldedProtocol, }; - use crate::{error::SqliteClientError, AccountId, SAPLING_TABLES_PREFIX}; + use crate::{error::SqliteClientError, AccountUuid, SAPLING_TABLES_PREFIX}; #[cfg(feature = "orchard")] use crate::ORCHARD_TABLES_PREFIX; pub(crate) fn get_tx_history( conn: &rusqlite::Connection, - ) -> Result>, SqliteClientError> { + ) -> Result>, SqliteClientError> { let mut stmt = conn.prepare_cached( - "SELECT accounts.id as account_id, v_transactions.* + "SELECT accounts.uuid as account_uuid, v_transactions.* FROM v_transactions JOIN accounts ON accounts.uuid = v_transactions.account_uuid ORDER BY mined_height DESC, tx_index DESC", )?; let results = stmt - .query_and_then::, SqliteClientError, _, _>([], |row| { - Ok(TransactionSummary::from_parts( - AccountId(row.get("account_id")?), - TxId::from_bytes(row.get("txid")?), - row.get::<_, Option>("expiry_height")? - .map(BlockHeight::from), - row.get::<_, Option>("mined_height")? - .map(BlockHeight::from), - ZatBalance::from_i64(row.get("account_balance_delta")?)?, - row.get::<_, Option>("fee_paid")? - .map(Zatoshis::from_nonnegative_i64) - .transpose()?, - row.get("spent_note_count")?, - row.get("has_change")?, - row.get("sent_note_count")?, - row.get("received_note_count")?, - row.get("memo_count")?, - row.get("expired_unmined")?, - row.get("is_shielding")?, - )) - })? + .query_and_then::, SqliteClientError, _, _>( + [], + |row| { + Ok(TransactionSummary::from_parts( + AccountUuid(row.get("account_uuid")?), + TxId::from_bytes(row.get("txid")?), + row.get::<_, Option>("expiry_height")? + .map(BlockHeight::from), + row.get::<_, Option>("mined_height")? + .map(BlockHeight::from), + ZatBalance::from_i64(row.get("account_balance_delta")?)?, + row.get::<_, Option>("fee_paid")? + .map(Zatoshis::from_nonnegative_i64) + .transpose()?, + row.get("spent_note_count")?, + row.get("has_change")?, + row.get("sent_note_count")?, + row.get("received_note_count")?, + row.get("memo_count")?, + row.get("expired_unmined")?, + row.get("is_shielding")?, + )) + }, + )? .collect::, _>>()?; Ok(results) @@ -3646,6 +3674,7 @@ mod tests { use sapling::zip32::ExtendedSpendingKey; use secrecy::{ExposeSecret, SecretVec}; + use uuid::Uuid; use zcash_client_backend::data_api::{ testing::{AddressType, DataStoreFactory, FakeCompactOutput, TestBuilder, TestState}, Account as _, AccountSource, WalletRead, WalletWrite, @@ -3654,7 +3683,7 @@ mod tests { use crate::{ testing::{db::TestDbFactory, BlockCache}, - AccountId, + AccountUuid, }; use super::account_birthday; @@ -3683,8 +3712,7 @@ mod tests { // No default address is set for an un-initialized account assert_matches!( - st.wallet() - .get_current_address(AccountId(account.id().0 + 1)), + st.wallet().get_current_address(AccountUuid(Uuid::nil())), Ok(None) ); } diff --git a/zcash_client_sqlite/src/wallet/common.rs b/zcash_client_sqlite/src/wallet/common.rs index 9e94ea63a..ac9e3834e 100644 --- a/zcash_client_sqlite/src/wallet/common.rs +++ b/zcash_client_sqlite/src/wallet/common.rs @@ -17,7 +17,7 @@ use zcash_protocol::{ use super::wallet_birthday; use crate::{ - error::SqliteClientError, wallet::pool_code, AccountId, ReceivedNoteId, SAPLING_TABLES_PREFIX, + error::SqliteClientError, wallet::pool_code, AccountUuid, ReceivedNoteId, SAPLING_TABLES_PREFIX, }; #[cfg(feature = "orchard")] @@ -118,7 +118,7 @@ where pub(crate) fn select_spendable_notes( conn: &Connection, params: &P, - account: AccountId, + account: AccountUuid, target_value: NonNegativeAmount, anchor_height: BlockHeight, exclude: &[ReceivedNoteId], @@ -169,7 +169,8 @@ where ON accounts.id = {table_prefix}_received_notes.account_id INNER JOIN transactions ON transactions.id_tx = {table_prefix}_received_notes.tx - WHERE {table_prefix}_received_notes.account_id = :account + WHERE accounts.uuid = :account_uuid + AND {table_prefix}_received_notes.account_id = accounts.id AND value > 5000 -- FIXME #1316, allow selection of dust inputs AND accounts.ufvk IS NOT NULL AND recipient_key_scope IS NOT NULL @@ -222,7 +223,7 @@ where let notes = stmt_select_notes.query_and_then( named_params![ - ":account": account.0, + ":account_uuid": account.0, ":anchor_height": &u32::from(anchor_height), ":target_value": &u64::from(target_value), ":exclude": &excluded_ptr, @@ -240,7 +241,7 @@ pub(crate) fn spendable_notes_meta( conn: &rusqlite::Connection, protocol: ShieldedProtocol, chain_tip_height: BlockHeight, - account: AccountId, + account: AccountUuid, filter: &NoteFilter, exclude: &[ReceivedNoteId], ) -> Result, SqliteClientError> { @@ -271,7 +272,7 @@ pub(crate) fn spendable_notes_meta( FROM {table_prefix}_received_notes rn INNER JOIN accounts a ON a.id = rn.account_id INNER JOIN transactions ON transactions.id_tx = rn.tx - WHERE rn.account_id = :account_id + WHERE a.uuid = :account_uuid AND a.ufvk IS NOT NULL AND rn.value >= :min_value AND transactions.mined_height IS NOT NULL @@ -286,7 +287,7 @@ pub(crate) fn spendable_notes_meta( )" ), named_params![ - ":account_id": account.0, + ":account_uuid": account.0, ":min_value": u64::from(min_value), ":exclude": &excluded_ptr, ":chain_tip_height": u32::from(chain_tip_height) @@ -304,7 +305,7 @@ pub(crate) fn spendable_notes_meta( // determine the minimum value of notes to be produced by note splitting. fn min_note_value( conn: &rusqlite::Connection, - account: AccountId, + account: AccountUuid, filter: &NoteFilter, chain_tip_height: BlockHeight, ) -> Result, SqliteClientError> { @@ -316,7 +317,8 @@ pub(crate) fn spendable_notes_meta( SELECT s.value, NTILE(10) OVER (ORDER BY s.value) AS bucket_index FROM sent_notes s JOIN transactions t ON s.tx = t.id_tx - WHERE s.from_account_id = :account_id + JOIN accounts a on a.id = s.from_account_id + WHERE a.uuid = :account_uuid -- only count mined transactions AND t.mined_height IS NOT NULL -- exclude change and account-internal sends @@ -330,7 +332,7 @@ pub(crate) fn spendable_notes_meta( let bucket_maxima = bucket_query .query_and_then::<_, SqliteClientError, _, _>( - named_params![":account_id": account.0], + named_params![":account_uuid": account.0], |row| { NonNegativeAmount::from_nonnegative_i64(row.get::<_, i64>(0)?).map_err( |_| { @@ -354,7 +356,7 @@ pub(crate) fn spendable_notes_meta( FROM v_received_outputs rn INNER JOIN accounts a ON a.id = rn.account_id INNER JOIN transactions ON transactions.id_tx = rn.transaction_id - WHERE rn.account_id = :account_id + WHERE a.uuid = :account_uuid AND a.ufvk IS NOT NULL AND transactions.mined_height IS NOT NULL AND rn.pool != :transparent_pool @@ -369,7 +371,7 @@ pub(crate) fn spendable_notes_meta( ) )", named_params![ - ":account_id": account.0, + ":account_uuid": account.0, ":chain_tip_height": u32::from(chain_tip_height), ":transparent_pool": pool_code(PoolType::Transparent) ], diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index f815a1c4b..b9b8964aa 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -203,8 +203,7 @@ fn sqlite_client_error_to_wallet_migration_error(e: SqliteClientError) -> Wallet | SqliteClientError::NonSequentialBlocks | SqliteClientError::RequestedRewindInvalid { .. } | SqliteClientError::KeyDerivationError(_) - | SqliteClientError::AccountIdDiscontinuity - | SqliteClientError::AccountIdOutOfRange + | SqliteClientError::Zip32AccountIndexOutOfRange | SqliteClientError::AccountCollision(_) | SqliteClientError::CacheMiss(_) => { unreachable!("we only call WalletRead methods; mutations can't occur") diff --git a/zcash_client_sqlite/src/wallet/init/migrations.rs b/zcash_client_sqlite/src/wallet/init/migrations.rs index cb6ca97b3..1e076736e 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations.rs @@ -141,7 +141,7 @@ pub(super) fn all_migrations( }), Box::new(spend_key_available::Migration), Box::new(tx_retrieval_queue::Migration { - params: params.clone(), + _params: params.clone(), }), Box::new(support_legacy_sqlite::Migration), Box::new(fix_broken_commitment_trees::Migration { diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs index 69895565f..b3fcf282b 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs @@ -8,7 +8,7 @@ use zcash_protocol::consensus; use crate::wallet::init::WalletMigrationError; #[cfg(feature = "transparent-inputs")] -use crate::wallet::{self, init, transparent::ephemeral}; +use crate::{wallet::transparent::ephemeral, AccountId}; use super::utxos_to_txos; @@ -65,9 +65,13 @@ impl RusqliteMigration for Migration

{ // Make sure that at least `GAP_LIMIT` ephemeral transparent addresses are // stored in each account. #[cfg(feature = "transparent-inputs")] - for account_id in wallet::get_account_ids(transaction)? { - ephemeral::init_account(transaction, &self.params, account_id) - .map_err(init::sqlite_client_error_to_wallet_migration_error)?; + { + let mut stmt = transaction.prepare("SELECT id FROM accounts")?; + let mut rows = stmt.query([])?; + while let Some(row) = rows.next()? { + let account_id = AccountId(row.get(0)?); + ephemeral::init_account(transaction, &self.params, account_id)?; + } } Ok(()) } @@ -124,7 +128,10 @@ mod tests { ) })?; let account_index = wallet::max_zip32_account_index(wdb.conn.0, &seed_fingerprint)? - .map(|a| a.next().ok_or(SqliteClientError::AccountIdOutOfRange)) + .map(|a| { + a.next() + .ok_or(SqliteClientError::Zip32AccountIndexOutOfRange) + }) .transpose()? .unwrap_or(zip32::AccountId::ZERO); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs index cfae9d438..f6fc1cd3a 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs @@ -413,7 +413,7 @@ mod tests { (ufvk0, height, res) } - fn put_received_note_before_migration( + fn put_received_note_before_migration>( conn: &Connection, output: &T, tx_ref: i64, diff --git a/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs b/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs index e66317d78..edffe9d95 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs @@ -4,11 +4,10 @@ use rusqlite::{named_params, Transaction}; use schemerz_rusqlite::RusqliteMigration; use std::collections::HashSet; use uuid::Uuid; -use zcash_client_backend::data_api::DecryptedTransaction; use zcash_primitives::transaction::builder::DEFAULT_TX_EXPIRY_DELTA; -use zcash_protocol::consensus::{self, BlockHeight, BranchId}; +use zcash_protocol::consensus; -use crate::wallet::{self, init::WalletMigrationError}; +use crate::wallet::init::WalletMigrationError; use super::{ ensure_orchard_ua_receiver, ephemeral_addresses, nullifier_map, orchard_shardtree, @@ -17,6 +16,23 @@ use super::{ pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0xfec02b61_3988_4b4f_9699_98977fac9e7f); +#[cfg(feature = "transparent-inputs")] +use { + crate::{ + error::SqliteClientError, + wallet::{ + queue_transparent_input_retrieval, queue_unmined_tx_retrieval, + transparent::{queue_transparent_spend_detection, uivk_legacy_transparent_address}, + }, + AccountId, TxRef, + }, + rusqlite::OptionalExtension as _, + std::convert::Infallible, + zcash_client_backend::data_api::DecryptedTransaction, + zcash_keys::encoding::AddressCodec, + zcash_protocol::consensus::{BlockHeight, BranchId}, +}; + const DEPENDENCIES: &[Uuid] = &[ orchard_shardtree::MIGRATION_ID, ensure_orchard_ua_receiver::MIGRATION_ID, @@ -26,7 +42,7 @@ const DEPENDENCIES: &[Uuid] = &[ ]; pub(super) struct Migration

{ - pub(super) params: P, + pub(super) _params: P, } impl

schemerz::Migration for Migration

{ @@ -46,8 +62,8 @@ impl

schemerz::Migration for Migration

{ impl RusqliteMigration for Migration

{ type Error = WalletMigrationError; - fn up(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { - transaction.execute_batch( + fn up(&self, conn: &Transaction) -> Result<(), WalletMigrationError> { + conn.execute_batch( "CREATE TABLE tx_retrieval_queue ( txid BLOB NOT NULL UNIQUE, query_type INTEGER NOT NULL, @@ -82,7 +98,7 @@ impl RusqliteMigration for Migration

{ // Add estimated target height information for each transaction we know to // have been created by the wallet; transactions that were discovered via // chain scanning will have their `created` field set to `NULL`. - transaction.execute( + conn.execute( "UPDATE transactions SET target_height = expiry_height - :default_expiry_delta WHERE expiry_height > :default_expiry_delta @@ -90,48 +106,107 @@ impl RusqliteMigration for Migration

{ named_params![":default_expiry_delta": DEFAULT_TX_EXPIRY_DELTA], )?; - // Call `decrypt_and_store_transaction` for each transaction known to the wallet to - // populate the enhancement queues with any transparent history information that we don't + // Populate the enhancement queues with any transparent history information that we don't // already have. - let mut stmt_transactions = - transaction.prepare("SELECT raw, mined_height FROM transactions")?; - let mut rows = stmt_transactions.query([])?; - while let Some(row) = rows.next()? { - let tx_data = row.get::<_, Option>>(0)?; - let mined_height = row.get::<_, Option>(1)?.map(BlockHeight::from); - - if let Some(tx_data) = tx_data { - let tx = zcash_primitives::transaction::Transaction::read( - &tx_data[..], - // We assume unmined transactions are created with the current consensus branch ID. - mined_height - .map_or(BranchId::Sapling, |h| BranchId::for_height(&self.params, h)), - ) - .map_err(|_| { - WalletMigrationError::CorruptedData( - "Could not read serialized transaction data.".to_owned(), + #[cfg(feature = "transparent-inputs")] + { + let mut stmt_transactions = + conn.prepare("SELECT id_tx, raw, mined_height FROM transactions")?; + let mut rows = stmt_transactions.query([])?; + while let Some(row) = rows.next()? { + let tx_ref = row.get(0).map(TxRef)?; + let tx_data = row.get::<_, Option>>(1)?; + let mined_height = row.get::<_, Option>(2)?.map(BlockHeight::from); + + if let Some(tx_data) = tx_data { + let tx = zcash_primitives::transaction::Transaction::read( + &tx_data[..], + // We assume unmined transactions are created with the current consensus branch ID. + mined_height.map_or(BranchId::Sapling, |h| { + BranchId::for_height(&self._params, h) + }), ) - })?; + .map_err(|_| { + WalletMigrationError::CorruptedData( + "Could not read serialized transaction data.".to_owned(), + ) + })?; + + for (txout, output_index) in tx + .transparent_bundle() + .iter() + .flat_map(|b| b.vout.iter()) + .zip(0u32..) + { + if let Some(address) = txout.recipient_address() { + let find_address_account = || { + conn.query_row( + "SELECT account_id FROM addresses + WHERE cached_transparent_receiver_address = :address + UNION + SELECT account_id from ephemeral_addresses + WHERE address = :address", + named_params![":address": address.encode(&self._params)], + |row| row.get(0).map(AccountId), + ) + .optional() + }; + let find_legacy_address_account = + || -> Result, SqliteClientError> { + let mut stmt = conn.prepare("SELECT id, uivk FROM accounts")?; + let mut rows = stmt.query([])?; + while let Some(row) = rows.next()? { + let account_id = row.get(0).map(AccountId)?; + let uivk_str = row.get::<_, String>(1)?; - wallet::store_decrypted_tx( - transaction, - &self.params, - DecryptedTransaction::new( + if let Some((legacy_taddr, _)) = + uivk_legacy_transparent_address( + &self._params, + &uivk_str, + )? + { + if legacy_taddr == address { + return Ok(Some(account_id)); + } + } + } + + Ok(None) + }; + + if find_address_account()?.is_some() + || find_legacy_address_account()?.is_some() + { + queue_transparent_spend_detection( + conn, + &self._params, + address, + tx_ref, + output_index, + )? + } + } + } + + let d_tx = DecryptedTransaction::<'_, Infallible>::new( mined_height, &tx, vec![], #[cfg(feature = "orchard")] vec![], - ), - )?; + ); + + queue_transparent_input_retrieval(conn, tx_ref, &d_tx)?; + queue_unmined_tx_retrieval(conn, &d_tx)?; + } } } Ok(()) } - fn down(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { - transaction.execute_batch( + fn down(&self, conn: &Transaction) -> Result<(), WalletMigrationError> { + conn.execute_batch( "DROP TABLE transparent_spend_map; DROP TABLE transparent_spend_search_queue; ALTER TABLE transactions DROP COLUMN target_height; diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 1b0e0cacb..c837e53f3 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -21,14 +21,16 @@ use zcash_protocol::{ }; use zip32::Scope; -use crate::{error::SqliteClientError, AccountId, ReceivedNoteId, TxRef}; +use crate::{error::SqliteClientError, AccountUuid, ReceivedNoteId, TxRef}; -use super::{memo_repr, parse_scope, scope_code}; +use super::{get_account_id, memo_repr, parse_scope, scope_code}; /// This trait provides a generalization over shielded output representations. pub(crate) trait ReceivedOrchardOutput { + type AccountId; + fn index(&self) -> usize; - fn account_id(&self) -> AccountId; + fn account_id(&self) -> Self::AccountId; fn note(&self) -> &Note; fn memo(&self) -> Option<&MemoBytes>; fn is_change(&self) -> bool; @@ -37,11 +39,13 @@ pub(crate) trait ReceivedOrchardOutput { fn recipient_key_scope(&self) -> Option; } -impl ReceivedOrchardOutput for WalletOrchardOutput { +impl ReceivedOrchardOutput for WalletOrchardOutput { + type AccountId = AccountId; + fn index(&self) -> usize { self.index() } - fn account_id(&self) -> AccountId { + fn account_id(&self) -> Self::AccountId { *WalletOrchardOutput::account_id(self) } fn note(&self) -> &Note { @@ -64,11 +68,13 @@ impl ReceivedOrchardOutput for WalletOrchardOutput { } } -impl ReceivedOrchardOutput for DecryptedOutput { +impl ReceivedOrchardOutput for DecryptedOutput { + type AccountId = AccountId; + fn index(&self) -> usize { self.index() } - fn account_id(&self) -> AccountId { + fn account_id(&self) -> Self::AccountId { *self.account() } fn note(&self) -> &orchard::note::Note { @@ -202,7 +208,7 @@ pub(crate) fn get_spendable_orchard_note( pub(crate) fn select_spendable_orchard_notes( conn: &Connection, params: &P, - account: AccountId, + account: AccountUuid, target_value: Zatoshis, anchor_height: BlockHeight, exclude: &[ReceivedNoteId], @@ -224,12 +230,13 @@ pub(crate) fn select_spendable_orchard_notes( /// This implementation relies on the facts that: /// - A transaction will not contain more than 2^63 shielded outputs. /// - A note value will never exceed 2^63 zatoshis. -pub(crate) fn put_received_note( +pub(crate) fn put_received_note>( conn: &Transaction, output: &T, tx_ref: TxRef, spent_in: Option, ) -> Result<(), SqliteClientError> { + let account_id = get_account_id(conn, output.account_id())?; let mut stmt_upsert_received_note = conn.prepare_cached( "INSERT INTO orchard_received_notes ( @@ -265,7 +272,7 @@ pub(crate) fn put_received_note( let sql_args = named_params![ ":tx": tx_ref.0, ":action_index": i64::try_from(output.index()).expect("output indices are representable as i64"), - ":account_id": output.account_id().0, + ":account_id": account_id.0, ":diversifier": diversifier.as_array(), ":value": output.note().value().inner(), ":rho": output.note().rho().to_bytes(), @@ -304,12 +311,13 @@ pub(crate) fn put_received_note( pub(crate) fn get_orchard_nullifiers( conn: &Connection, query: NullifierQuery, -) -> Result, SqliteClientError> { +) -> Result, SqliteClientError> { // Get the nullifiers for the notes we are tracking let mut stmt_fetch_nullifiers = match query { NullifierQuery::Unspent => conn.prepare( - "SELECT rn.account_id, rn.nf + "SELECT a.uuid, rn.nf FROM orchard_received_notes rn + JOIN accounts a ON a.id = rn.account_id JOIN transactions tx ON tx.id_tx = rn.tx WHERE rn.nf IS NOT NULL AND tx.block IS NOT NULL @@ -322,14 +330,15 @@ pub(crate) fn get_orchard_nullifiers( )", )?, NullifierQuery::All => conn.prepare( - "SELECT rn.account_id, rn.nf + "SELECT a.uuid, rn.nf FROM orchard_received_notes rn + JOIN accounts a ON a.id = rn.account_id WHERE nf IS NOT NULL", )?, }; let nullifiers = stmt_fetch_nullifiers.query_and_then([], |row| { - let account = AccountId(row.get(0)?); + let account = AccountUuid(row.get(0)?); let nf_bytes: [u8; 32] = row.get(1)?; Ok::<_, rusqlite::Error>((account, Nullifier::from_bytes(&nf_bytes).unwrap())) })?; @@ -341,18 +350,19 @@ pub(crate) fn get_orchard_nullifiers( pub(crate) fn detect_spending_accounts<'a>( conn: &Connection, nfs: impl Iterator, -) -> Result, rusqlite::Error> { +) -> Result, rusqlite::Error> { let mut account_q = conn.prepare_cached( - "SELECT rn.account_id - FROM orchard_received_notes rn - WHERE rn.nf IN rarray(:nf_ptr)", + "SELECT a.uuid + FROM orchard_received_notes rn + JOIN accounts a ON a.id = rn.account_id + WHERE rn.nf IN rarray(:nf_ptr)", )?; let nf_values: Vec = nfs.map(|nf| Value::Blob(nf.to_bytes().to_vec())).collect(); let nf_ptr = Rc::new(nf_values); let res = account_q .query_and_then(named_params![":nf_ptr": &nf_ptr], |row| { - row.get::<_, u32>(0).map(AccountId) + row.get(0).map(AccountUuid) })? .collect::, _>>()?; diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 9652761b1..4d498727a 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -20,14 +20,16 @@ use zcash_protocol::{ }; use zip32::Scope; -use crate::{error::SqliteClientError, AccountId, ReceivedNoteId, TxRef}; +use crate::{error::SqliteClientError, AccountUuid, ReceivedNoteId, TxRef}; -use super::{memo_repr, parse_scope, scope_code}; +use super::{get_account_id, memo_repr, parse_scope, scope_code}; /// This trait provides a generalization over shielded output representations. pub(crate) trait ReceivedSaplingOutput { + type AccountId; + fn index(&self) -> usize; - fn account_id(&self) -> AccountId; + fn account_id(&self) -> Self::AccountId; fn note(&self) -> &sapling::Note; fn memo(&self) -> Option<&MemoBytes>; fn is_change(&self) -> bool; @@ -36,11 +38,13 @@ pub(crate) trait ReceivedSaplingOutput { fn recipient_key_scope(&self) -> Option; } -impl ReceivedSaplingOutput for WalletSaplingOutput { +impl ReceivedSaplingOutput for WalletSaplingOutput { + type AccountId = AccountId; + fn index(&self) -> usize { self.index() } - fn account_id(&self) -> AccountId { + fn account_id(&self) -> Self::AccountId { *WalletSaplingOutput::account_id(self) } fn note(&self) -> &sapling::Note { @@ -63,11 +67,13 @@ impl ReceivedSaplingOutput for WalletSaplingOutput { } } -impl ReceivedSaplingOutput for DecryptedOutput { +impl ReceivedSaplingOutput for DecryptedOutput { + type AccountId = AccountId; + fn index(&self) -> usize { self.index() } - fn account_id(&self) -> AccountId { + fn account_id(&self) -> Self::AccountId { *self.account() } fn note(&self) -> &sapling::Note { @@ -212,7 +218,7 @@ pub(crate) fn get_spendable_sapling_note( pub(crate) fn select_spendable_sapling_notes( conn: &Connection, params: &P, - account: AccountId, + account: AccountUuid, target_value: NonNegativeAmount, anchor_height: BlockHeight, exclude: &[ReceivedNoteId], @@ -238,12 +244,13 @@ pub(crate) fn select_spendable_sapling_notes( pub(crate) fn get_sapling_nullifiers( conn: &Connection, query: NullifierQuery, -) -> Result, SqliteClientError> { +) -> Result, SqliteClientError> { // Get the nullifiers for the notes we are tracking let mut stmt_fetch_nullifiers = match query { NullifierQuery::Unspent => conn.prepare( - "SELECT rn.account_id, rn.nf + "SELECT a.uuid, rn.nf FROM sapling_received_notes rn + JOIN accounts a ON a.id = rn.account_id JOIN transactions tx ON tx.id_tx = rn.tx WHERE rn.nf IS NOT NULL AND tx.block IS NOT NULL @@ -256,14 +263,15 @@ pub(crate) fn get_sapling_nullifiers( )", ), NullifierQuery::All => conn.prepare( - "SELECT rn.account_id, rn.nf + "SELECT a.uuid, rn.nf FROM sapling_received_notes rn + JOIN accounts a ON a.id = rn.account_id WHERE nf IS NOT NULL", ), }?; let nullifiers = stmt_fetch_nullifiers.query_and_then([], |row| { - let account = AccountId(row.get(0)?); + let account = AccountUuid(row.get(0)?); let nf_bytes: Vec = row.get(1)?; Ok::<_, rusqlite::Error>((account, sapling::Nullifier::from_slice(&nf_bytes).unwrap())) })?; @@ -275,10 +283,11 @@ pub(crate) fn get_sapling_nullifiers( pub(crate) fn detect_spending_accounts<'a>( conn: &Connection, nfs: impl Iterator, -) -> Result, rusqlite::Error> { +) -> Result, rusqlite::Error> { let mut account_q = conn.prepare_cached( - "SELECT rn.account_id + "SELECT accounts.uuid FROM sapling_received_notes rn + JOIN accounts ON accounts.id = rn.account_id WHERE rn.nf IN rarray(:nf_ptr)", )?; @@ -286,7 +295,7 @@ pub(crate) fn detect_spending_accounts<'a>( let nf_ptr = Rc::new(nf_values); let res = account_q .query_and_then(named_params![":nf_ptr": &nf_ptr], |row| { - row.get::<_, u32>(0).map(AccountId) + row.get(0).map(AccountUuid) })? .collect::, _>>()?; @@ -324,12 +333,13 @@ pub(crate) fn mark_sapling_note_spent( /// This implementation relies on the facts that: /// - A transaction will not contain more than 2^63 shielded outputs. /// - A note value will never exceed 2^63 zatoshis. -pub(crate) fn put_received_note( +pub(crate) fn put_received_note>( conn: &Transaction, output: &T, tx_ref: TxRef, spent_in: Option, ) -> Result<(), SqliteClientError> { + let account_id = get_account_id(conn, output.account_id())?; let mut stmt_upsert_received_note = conn.prepare_cached( "INSERT INTO sapling_received_notes (tx, output_index, account_id, diversifier, value, rcm, memo, nf, @@ -368,7 +378,7 @@ pub(crate) fn put_received_note( let sql_args = named_params![ ":tx": tx_ref.0, ":output_index": i64::try_from(output.index()).expect("output indices are representable as i64"), - ":account_id": output.account_id().0, + ":account_id": account_id.0, ":diversifier": &diversifier.0.as_ref(), ":value": output.note().value().inner(), ":rcm": &rcm.as_ref(), diff --git a/zcash_client_sqlite/src/wallet/transparent.rs b/zcash_client_sqlite/src/wallet/transparent.rs index 11bf60f18..71a1fd96d 100644 --- a/zcash_client_sqlite/src/wallet/transparent.rs +++ b/zcash_client_sqlite/src/wallet/transparent.rs @@ -7,7 +7,7 @@ use zcash_client_backend::data_api::TransactionDataRequest; use zcash_primitives::transaction::builder::DEFAULT_TX_EXPIRY_DELTA; use zip32::{DiversifierIndex, Scope}; -use zcash_address::unified::{Encoding, Ivk, Uivk}; +use zcash_address::unified::{Ivk, Uivk}; use zcash_client_backend::{ data_api::AccountBalance, wallet::{TransparentAddressMetadata, WalletTransparentOutput}, @@ -22,18 +22,20 @@ use zcash_primitives::{ }; use zcash_protocol::consensus::{self, BlockHeight}; -use super::{chain_tip_height, get_account_ids}; -use crate::{error::SqliteClientError, AccountId, TxRef, UtxoId}; +use super::{chain_tip_height, get_account_id, get_account_ids}; +use crate::AccountUuid; +use crate::{error::SqliteClientError, TxRef, UtxoId}; pub(crate) mod ephemeral; pub(crate) fn detect_spending_accounts<'a>( conn: &Connection, spent: impl Iterator, -) -> Result, rusqlite::Error> { +) -> Result, rusqlite::Error> { let mut account_q = conn.prepare_cached( - "SELECT account_id + "SELECT accounts.uuid FROM transparent_received_outputs o + JOIN accounts ON accounts.id = o.account_id JOIN transactions t ON t.id_tx = o.transaction_id WHERE t.txid = :prevout_txid AND o.output_index = :prevout_idx", @@ -46,7 +48,7 @@ pub(crate) fn detect_spending_accounts<'a>( ":prevout_txid": prevout.hash(), ":prevout_idx": prevout.n() ], - |row| row.get::<_, u32>(0).map(AccountId), + |row| row.get(0).map(AccountUuid), )? { acc.insert(account?); } @@ -80,15 +82,18 @@ fn address_index_from_diversifier_index_be( pub(crate) fn get_transparent_receivers( conn: &rusqlite::Connection, params: &P, - account: AccountId, + account_uuid: AccountUuid, ) -> Result>, SqliteClientError> { let mut ret: HashMap> = HashMap::new(); // Get all UAs derived let mut ua_query = conn.prepare( - "SELECT address, diversifier_index_be FROM addresses WHERE account_id = :account", + "SELECT address, diversifier_index_be + FROM addresses + JOIN accounts ON accounts.id = addresses.account_id + WHERE accounts.uuid = :account_uuid", )?; - let mut rows = ua_query.query(named_params![":account": account.0])?; + let mut rows = ua_query.query(named_params![":account_uuid": account_uuid.0])?; while let Some(row) = rows.next()? { let ua_str: String = row.get(0)?; @@ -113,7 +118,9 @@ pub(crate) fn get_transparent_receivers( } } - if let Some((taddr, address_index)) = get_legacy_transparent_address(params, conn, account)? { + if let Some((taddr, address_index)) = + get_legacy_transparent_address(params, conn, account_uuid)? + { let metadata = TransparentAddressMetadata::new(Scope::External.into(), address_index); ret.insert(taddr, Some(metadata)); } @@ -121,46 +128,56 @@ pub(crate) fn get_transparent_receivers( Ok(ret) } -pub(crate) fn get_legacy_transparent_address( +pub(crate) fn uivk_legacy_transparent_address( params: &P, - conn: &rusqlite::Connection, - account_id: AccountId, + uivk_str: &str, ) -> Result, SqliteClientError> { - use zcash_address::unified::Container; + use zcash_address::unified::{Container as _, Encoding as _}; use zcash_primitives::legacy::keys::ExternalIvk; + let (network, uivk) = Uivk::decode(uivk_str) + .map_err(|e| SqliteClientError::CorruptedData(format!("Unable to parse UIVK: {e}")))?; + + if params.network_type() != network { + let network_name = |n| match n { + consensus::NetworkType::Main => "mainnet", + consensus::NetworkType::Test => "testnet", + consensus::NetworkType::Regtest => "regtest", + }; + return Err(SqliteClientError::CorruptedData(format!( + "Network type mismatch: account UIVK is for {} but a {} address was requested.", + network_name(network), + network_name(params.network_type()) + ))); + } + + // Derive the default transparent address (if it wasn't already part of a derived UA). + for item in uivk.items() { + if let Ivk::P2pkh(tivk_bytes) = item { + let tivk = ExternalIvk::deserialize(&tivk_bytes)?; + return Ok(Some(tivk.default_address())); + } + } + + Ok(None) +} + +pub(crate) fn get_legacy_transparent_address( + params: &P, + conn: &rusqlite::Connection, + account_uuid: AccountUuid, +) -> Result, SqliteClientError> { // Get the UIVK for the account. let uivk_str: Option = conn .query_row( - "SELECT uivk FROM accounts WHERE id = :account", - [account_id.0], + "SELECT uivk FROM accounts WHERE uuid = :account_uuid", + named_params![":account_uuid": account_uuid.0], |row| row.get(0), ) .optional()?; if let Some(uivk_str) = uivk_str { - let (network, uivk) = Uivk::decode(&uivk_str) - .map_err(|e| SqliteClientError::CorruptedData(format!("Unable to parse UIVK: {e}")))?; - if params.network_type() != network { - let network_name = |n| match n { - consensus::NetworkType::Main => "mainnet", - consensus::NetworkType::Test => "testnet", - consensus::NetworkType::Regtest => "regtest", - }; - return Err(SqliteClientError::CorruptedData(format!( - "Network type mismatch: account UIVK is for {} but a {} address was requested.", - network_name(network), - network_name(params.network_type()) - ))); - } - - // Derive the default transparent address (if it wasn't already part of a derived UA). - for item in uivk.items() { - if let Ivk::P2pkh(tivk_bytes) = item { - let tivk = ExternalIvk::deserialize(&tivk_bytes)?; - return Ok(Some(tivk.default_address())); - } - } + return uivk_legacy_transparent_address(params, &uivk_str); } Ok(None) @@ -334,7 +351,7 @@ pub(crate) fn get_spendable_transparent_outputs( pub(crate) fn get_transparent_balances( conn: &rusqlite::Connection, params: &P, - account: AccountId, + account_uuid: AccountUuid, summary_height: BlockHeight, ) -> Result, SqliteClientError> { let chain_tip_height = chain_tip_height(conn)?.ok_or(SqliteClientError::ChainHeightUnknown)?; @@ -342,9 +359,9 @@ pub(crate) fn get_transparent_balances( let mut stmt_address_balances = conn.prepare( "SELECT u.address, SUM(u.value_zat) FROM transparent_received_outputs u - JOIN transactions t - ON t.id_tx = u.transaction_id - WHERE u.account_id = :account_id + JOIN accounts ON accounts.id = u.account_id + JOIN transactions t ON t.id_tx = u.transaction_id + WHERE accounts.uuid = :account_uuid -- the transaction that created the output is mined or is definitely unexpired AND ( t.mined_height <= :summary_height -- tx is mined @@ -370,7 +387,7 @@ pub(crate) fn get_transparent_balances( let mut res = HashMap::new(); let mut rows = stmt_address_balances.query(named_params![ - ":account_id": account.0, + ":account_uuid": account_uuid.0, ":summary_height": u32::from(summary_height), ":chain_tip_height": u32::from(chain_tip_height), ":spend_expiry_height": u32::from(std::cmp::min(summary_height, chain_tip_height + 1)), @@ -390,13 +407,13 @@ pub(crate) fn get_transparent_balances( pub(crate) fn add_transparent_account_balances( conn: &rusqlite::Connection, mempool_height: BlockHeight, - account_balances: &mut HashMap, + account_balances: &mut HashMap, ) -> Result<(), SqliteClientError> { let mut stmt_account_balances = conn.prepare( - "SELECT u.account_id, SUM(u.value_zat) + "SELECT a.uuid, SUM(u.value_zat) FROM transparent_received_outputs u - JOIN transactions t - ON t.id_tx = u.transaction_id + JOIN accounts a ON a.id = u.account_id + JOIN transactions t ON t.id_tx = u.transaction_id -- the transaction that created the output is mined or is definitely unexpired WHERE ( t.mined_height < :mempool_height -- tx is mined @@ -413,13 +430,13 @@ pub(crate) fn add_transparent_account_balances( OR tx.expiry_height = 0 -- the spending tx will not expire OR tx.expiry_height >= :mempool_height -- the spending tx is unexpired ) - GROUP BY u.account_id", + GROUP BY a.uuid", )?; let mut rows = stmt_account_balances .query(named_params![":mempool_height": u32::from(mempool_height),])?; while let Some(row) = rows.next()? { - let account = AccountId(row.get(0)?); + let account = AccountUuid(row.get(0)?); let raw_value = row.get(1)?; let value = NonNegativeAmount::from_nonnegative_i64(raw_value).map_err(|_| { SqliteClientError::CorruptedData(format!("Negative UTXO value {:?}", raw_value)) @@ -501,7 +518,9 @@ pub(crate) fn put_received_transparent_utxo( output: &WalletTransparentOutput, ) -> Result { let address = output.recipient_address(); - if let Some(receiving_account) = find_account_for_transparent_address(conn, params, address)? { + if let Some(receiving_account) = + find_account_uuid_for_transparent_address(conn, params, address)? + { put_transparent_output( conn, params, @@ -565,7 +584,7 @@ pub(crate) fn transaction_data_requests( pub(crate) fn get_transparent_address_metadata( conn: &rusqlite::Connection, params: &P, - account_id: AccountId, + account_uuid: AccountUuid, address: &TransparentAddress, ) -> Result, SqliteClientError> { let address_str = address.encode(params); @@ -573,8 +592,10 @@ pub(crate) fn get_transparent_address_metadata( if let Some(di_vec) = conn .query_row( "SELECT diversifier_index_be FROM addresses - WHERE account_id = :account_id AND cached_transparent_receiver_address = :address", - named_params![":account_id": account_id.0, ":address": &address_str], + JOIN accounts ON addresses.account_id = accounts.id + WHERE accounts.uuid = :account_uuid + AND cached_transparent_receiver_address = :address", + named_params![":account_uuid": account_uuid.0, ":address": &address_str], |row| row.get::<_, Vec>(0), ) .optional()? @@ -585,7 +606,7 @@ pub(crate) fn get_transparent_address_metadata( } if let Some((legacy_taddr, address_index)) = - get_legacy_transparent_address(params, conn, account_id)? + get_legacy_transparent_address(params, conn, account_uuid)? { if &legacy_taddr == address { let metadata = TransparentAddressMetadata::new(Scope::External.into(), address_index); @@ -595,7 +616,7 @@ pub(crate) fn get_transparent_address_metadata( // Search known ephemeral addresses. if let Some(address_index) = - ephemeral::find_index_for_ephemeral_address_str(conn, account_id, &address_str)? + ephemeral::find_index_for_ephemeral_address_str(conn, account_uuid, &address_str)? { return Ok(Some(ephemeral::metadata(address_index))); } @@ -613,18 +634,21 @@ pub(crate) fn get_transparent_address_metadata( /// /// Returns `Ok(None)` if the transparent output's recipient address is not in any of the /// above locations. This means the wallet considers the output "not interesting". -pub(crate) fn find_account_for_transparent_address( +pub(crate) fn find_account_uuid_for_transparent_address( conn: &rusqlite::Connection, params: &P, address: &TransparentAddress, -) -> Result, SqliteClientError> { +) -> Result, SqliteClientError> { let address_str = address.encode(params); if let Some(account_id) = conn .query_row( - "SELECT account_id FROM addresses WHERE cached_transparent_receiver_address = :address", + "SELECT accounts.uuid + FROM addresses + JOIN accounts ON accounts.id = addresses.account_id + WHERE cached_transparent_receiver_address = :address", named_params![":address": &address_str], - |row| Ok(AccountId(row.get(0)?)), + |row| Ok(AccountUuid(row.get(0)?)), ) .optional()? { @@ -668,10 +692,11 @@ pub(crate) fn put_transparent_output( txout: &TxOut, output_height: Option, address: &TransparentAddress, - receiving_account: AccountId, + receiving_account_uuid: AccountUuid, known_unspent: bool, ) -> Result { let output_height = output_height.map(u32::from); + let receiving_account_id = get_account_id(conn, receiving_account_uuid)?; // Check whether we have an entry in the blocks table for the output height; // if not, the transaction will be updated with its mined height when the @@ -759,7 +784,7 @@ pub(crate) fn put_transparent_output( let sql_args = named_params![ ":transaction_id": id_tx, ":output_index": &outpoint.n(), - ":account_id": receiving_account.0, + ":account_id": receiving_account_id.0, ":address": &address.encode(params), ":script": &txout.script_pubkey.0, ":value_zat": &i64::from(Amount::from(txout.value)), diff --git a/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs index 37d16ecb5..44eb19c12 100644 --- a/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs +++ b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs @@ -16,6 +16,8 @@ use zcash_primitives::{ }; use zcash_protocol::consensus; +use crate::wallet::{self, get_account_id}; +use crate::AccountUuid; use crate::{error::SqliteClientError, AccountId, TxRef}; // Returns `TransparentAddressMetadata` in the ephemeral scope for the @@ -155,12 +157,15 @@ pub(crate) fn get_known_ephemeral_addresses( let index_range = index_range.unwrap_or(0..(1 << 31)); let mut stmt = conn.prepare( - "SELECT address, address_index FROM ephemeral_addresses - WHERE account_id = :account AND address_index >= :start AND address_index < :end + "SELECT address, address_index + FROM ephemeral_addresses ea + WHERE ea.account_id = :account_id + AND address_index >= :start + AND address_index < :end ORDER BY address_index", )?; let mut rows = stmt.query(named_params![ - ":account": account_id.0, + ":account_id": account_id.0, ":start": index_range.start, ":end": min(1 << 31, index_range.end), ])?; @@ -182,12 +187,15 @@ pub(crate) fn get_known_ephemeral_addresses( pub(crate) fn find_account_for_ephemeral_address_str( conn: &rusqlite::Connection, address_str: &str, -) -> Result, SqliteClientError> { +) -> Result, SqliteClientError> { Ok(conn .query_row( - "SELECT account_id FROM ephemeral_addresses WHERE address = :address", + "SELECT accounts.uuid + FROM ephemeral_addresses ea + JOIN accounts ON accounts.id = ea.account_id + WHERE address = :address", named_params![":address": &address_str], - |row| Ok(AccountId(row.get(0)?)), + |row| Ok(AccountUuid(row.get(0)?)), ) .optional()?) } @@ -195,9 +203,10 @@ pub(crate) fn find_account_for_ephemeral_address_str( /// If this is a known ephemeral address in the given account, return its index. pub(crate) fn find_index_for_ephemeral_address_str( conn: &rusqlite::Connection, - account_id: AccountId, + account_uuid: AccountUuid, address_str: &str, ) -> Result, SqliteClientError> { + let account_id = get_account_id(conn, account_uuid)?; Ok(conn .query_row( "SELECT address_index FROM ephemeral_addresses @@ -246,8 +255,9 @@ pub(crate) fn reserve_next_n_ephemeral_addresses( return Err(AddressGenerationError::DiversifierSpaceExhausted.into()); } if allocation.end > first_unsafe { + let account_uuid = wallet::get_account_uuid(conn, account_id)?; return Err(SqliteClientError::ReachedGapLimit( - account_id, + account_uuid, max(first_unreserved, first_unsafe), )); } From 6e1933b5aa7b4d31c837e1d3a3bcd91c50feb8da Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 26 Nov 2024 22:47:32 -0700 Subject: [PATCH 5/5] zcash_client_sqlite: Rename `AccountId` internal type to `AccountRef` This is now consistent with how we name other internal primary key type wrappers. --- zcash_client_sqlite/src/lib.rs | 8 +++---- zcash_client_sqlite/src/wallet.rs | 16 +++++++------- .../init/migrations/ephemeral_addresses.rs | 14 ++++++------ .../init/migrations/receiving_key_scopes.rs | 12 +++++----- .../init/migrations/tx_retrieval_queue.rs | 8 +++---- .../src/wallet/transparent/ephemeral.rs | 22 +++++++++---------- 6 files changed, 40 insertions(+), 40 deletions(-) diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 0bc77b210..16742cf22 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -205,18 +205,18 @@ impl AccountUuid { } } -/// The local identifier for an account. +/// A typesafe wrapper for the primary key identifier for a row in the `accounts` table. /// /// This is an ephemeral value for efficiently and generically working with accounts in a /// [`WalletDb`]. To reference accounts in external contexts, use [`AccountUuid`]. #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Default)] -pub(crate) struct AccountId(u32); +pub(crate) struct AccountRef(u32); /// This implementation is retained under `#[cfg(test)]` for pre-AccountUuid testing. #[cfg(test)] -impl ConditionallySelectable for AccountId { +impl ConditionallySelectable for AccountRef { fn conditional_select(a: &Self, b: &Self, choice: subtle::Choice) -> Self { - AccountId(ConditionallySelectable::conditional_select( + AccountRef(ConditionallySelectable::conditional_select( &a.0, &b.0, choice, )) } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 04d9496f7..23efc47c7 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -117,7 +117,7 @@ use zip32::{self, DiversifierIndex, Scope}; use crate::{ error::SqliteClientError, wallet::commitment_tree::{get_max_checkpointed_height, SqliteShardStore}, - AccountId, SqlTransaction, TransferType, WalletCommitmentTrees, WalletDb, DEFAULT_UA_REQUEST, + AccountRef, SqlTransaction, TransferType, WalletCommitmentTrees, WalletDb, DEFAULT_UA_REQUEST, PRUNING_DEPTH, SAPLING_TABLES_PREFIX, }; use crate::{AccountUuid, TxRef, VERIFY_LOOKAHEAD}; @@ -441,7 +441,7 @@ pub(crate) fn add_account( ":recover_until_height": birthday.recover_until().map(u32::from), ":has_spend_key": spending_key_available as i64, ], - |row| row.get(0).map(AccountId), + |row| row.get(0).map(AccountRef), ) .map_err(|e| match e { rusqlite::Error::SqliteFailure(f, s) @@ -449,7 +449,7 @@ pub(crate) fn add_account( { // An account conflict occurred. This should already have been caught by // the check using `get_account_for_ufvk` above, but in case it wasn't, - // make a best effort to determine the AccountId of the pre-existing row + // make a best effort to determine the AccountRef of the pre-existing row // and provide that to our caller. if let Ok(colliding_uuid) = conn.query_row( "SELECT uuid FROM accounts WHERE ufvk = ?", @@ -630,7 +630,7 @@ pub(crate) fn get_current_address( pub(crate) fn insert_address( conn: &rusqlite::Connection, params: &P, - account_id: AccountId, + account_id: AccountRef, diversifier_index: DiversifierIndex, address: &UnifiedAddress, ) -> Result<(), SqliteClientError> { @@ -1839,11 +1839,11 @@ pub(crate) fn block_height_extrema( pub(crate) fn get_account_id( conn: &rusqlite::Connection, account_uuid: AccountUuid, -) -> Result { +) -> Result { conn.query_row( "SELECT id FROM accounts WHERE uuid = :account_uuid", named_params! {":account_uuid": account_uuid.0}, - |row| row.get("id").map(AccountId), + |row| row.get("id").map(AccountRef), ) .optional()? .ok_or(SqliteClientError::AccountUnknown) @@ -1852,7 +1852,7 @@ pub(crate) fn get_account_id( #[cfg(feature = "transparent-inputs")] pub(crate) fn get_account_uuid( conn: &rusqlite::Connection, - account_id: AccountId, + account_id: AccountRef, ) -> Result { conn.query_row( "SELECT uuid FROM accounts WHERE id = :account_id", @@ -3257,7 +3257,7 @@ fn recipient_params( params: &P, from: AccountUuid, to: &Recipient, -) -> Result<(AccountId, Option, Option, PoolType), SqliteClientError> { +) -> Result<(AccountRef, Option, Option, PoolType), SqliteClientError> { let from_account_id = get_account_id(conn, from)?; match to { Recipient::External(addr, pool) => Ok((from_account_id, Some(addr.encode()), None, *pool)), diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs index b3fcf282b..a982a4905 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs @@ -8,7 +8,7 @@ use zcash_protocol::consensus; use crate::wallet::init::WalletMigrationError; #[cfg(feature = "transparent-inputs")] -use crate::{wallet::transparent::ephemeral, AccountId}; +use crate::{wallet::transparent::ephemeral, AccountRef}; use super::utxos_to_txos; @@ -69,7 +69,7 @@ impl RusqliteMigration for Migration

{ let mut stmt = transaction.prepare("SELECT id FROM accounts")?; let mut rows = stmt.query([])?; while let Some(row) = rows.next()? { - let account_id = AccountId(row.get(0)?); + let account_id = AccountRef(row.get(0)?); ephemeral::init_account(transaction, &self.params, account_id)?; } } @@ -107,7 +107,7 @@ mod tests { wallet::{ self, account_kind_code, init::init_wallet_db_internal, transparent::ephemeral, }, - AccountId, WalletDb, + AccountRef, WalletDb, }, zcash_client_backend::data_api::GAP_LIMIT, }; @@ -119,7 +119,7 @@ mod tests { wdb: &mut WalletDb, seed: &SecretVec, birthday: &AccountBirthday, - ) -> Result<(AccountId, UnifiedSpendingKey), SqliteClientError> { + ) -> Result<(AccountRef, UnifiedSpendingKey), SqliteClientError> { wdb.transactionally(|wdb| { let seed_fingerprint = SeedFingerprint::from_seed(seed.expose_secret()).ok_or_else(|| { @@ -158,7 +158,7 @@ mod tests { #[cfg(not(feature = "orchard"))] let birthday_orchard_tree_size: Option = None; - let account_id: AccountId = wdb.conn.0.query_row( + let account_id: AccountRef = wdb.conn.0.query_row( r#" INSERT INTO accounts ( account_kind, hd_seed_fingerprint, hd_account_index, @@ -190,7 +190,7 @@ mod tests { ":birthday_orchard_tree_size": birthday_orchard_tree_size, ":recover_until_height": birthday.recover_until().map(u32::from) ], - |row| Ok(AccountId(row.get(0)?)), + |row| Ok(AccountRef(row.get(0)?)), )?; // Initialize the `ephemeral_addresses` table. @@ -232,7 +232,7 @@ mod tests { account_index: account0_index, }); assert_eq!(u32::from(account0_index), 0); - let account0_id = crate::AccountId(0); + let account0_id = AccountRef(0); let usk0 = UnifiedSpendingKey::from_seed(&network, &seed0, account0_index).unwrap(); let ufvk0 = usk0.to_unified_full_viewing_key(); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs index f6fc1cd3a..778ea22c2 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs @@ -324,7 +324,7 @@ mod tests { memo_repr, parse_scope, sapling::ReceivedSaplingOutput, }, - AccountId, TxRef, WalletDb, + AccountRef, TxRef, WalletDb, }; // These must be different. @@ -413,7 +413,7 @@ mod tests { (ufvk0, height, res) } - fn put_received_note_before_migration>( + fn put_received_note_before_migration>( conn: &Connection, output: &T, tx_ref: i64, @@ -529,7 +529,7 @@ mod tests { let (ufvk0, height, res) = prepare_wallet_state(&mut db_data); let tx = res.transaction(); - let account_id = AccountId(0); + let account_id = AccountRef(0); // We can't use `decrypt_and_store_transaction` because we haven't migrated yet. // Replicate its relevant innards here. @@ -544,7 +544,7 @@ mod tests { .transactionally::<_, _, rusqlite::Error>(|wdb| { let tx_ref = put_tx_data(wdb.conn.0, d_tx.tx(), None, None).unwrap(); - let mut spending_account_id: Option = None; + let mut spending_account_id: Option = None; // Orchard outputs were not supported as of the wallet states that could require this // migration. @@ -644,7 +644,7 @@ mod tests { ..Default::default() }; block.vtx.push(compact_tx); - let scanning_keys = ScanningKeys::from_account_ufvks([(AccountId(0), ufvk0)]); + let scanning_keys = ScanningKeys::from_account_ufvks([(AccountRef(0), ufvk0)]); let scanned_block = scan_block( ¶ms, @@ -875,7 +875,7 @@ mod tests { /// updates to the database schema require incompatible changes to `put_tx_meta`. pub(crate) fn put_tx_meta( conn: &rusqlite::Connection, - tx: &WalletTx, + tx: &WalletTx, height: BlockHeight, ) -> Result { // It isn't there, so insert our transaction into the database. diff --git a/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs b/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs index edffe9d95..9b6eb5ce5 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs @@ -24,7 +24,7 @@ use { queue_transparent_input_retrieval, queue_unmined_tx_retrieval, transparent::{queue_transparent_spend_detection, uivk_legacy_transparent_address}, }, - AccountId, TxRef, + AccountRef, TxRef, }, rusqlite::OptionalExtension as _, std::convert::Infallible, @@ -147,16 +147,16 @@ impl RusqliteMigration for Migration

{ SELECT account_id from ephemeral_addresses WHERE address = :address", named_params![":address": address.encode(&self._params)], - |row| row.get(0).map(AccountId), + |row| row.get(0).map(AccountRef), ) .optional() }; let find_legacy_address_account = - || -> Result, SqliteClientError> { + || -> Result, SqliteClientError> { let mut stmt = conn.prepare("SELECT id, uivk FROM accounts")?; let mut rows = stmt.query([])?; while let Some(row) = rows.next()? { - let account_id = row.get(0).map(AccountId)?; + let account_id = row.get(0).map(AccountRef)?; let uivk_str = row.get::<_, String>(1)?; if let Some((legacy_taddr, _)) = diff --git a/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs index 44eb19c12..256f8bd3b 100644 --- a/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs +++ b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs @@ -18,7 +18,7 @@ use zcash_protocol::consensus; use crate::wallet::{self, get_account_id}; use crate::AccountUuid; -use crate::{error::SqliteClientError, AccountId, TxRef}; +use crate::{error::SqliteClientError, AccountRef, TxRef}; // Returns `TransparentAddressMetadata` in the ephemeral scope for the // given address index. @@ -29,7 +29,7 @@ pub(crate) fn metadata(address_index: NonHardenedChildIndex) -> TransparentAddre /// Returns the first unstored ephemeral address index in the given account. pub(crate) fn first_unstored_index( conn: &rusqlite::Connection, - account_id: AccountId, + account_id: AccountRef, ) -> Result { match conn .query_row( @@ -53,7 +53,7 @@ pub(crate) fn first_unstored_index( /// Returns the first unreserved ephemeral address index in the given account. pub(crate) fn first_unreserved_index( conn: &rusqlite::Connection, - account_id: AccountId, + account_id: AccountRef, ) -> Result { first_unstored_index(conn, account_id)? .checked_sub(GAP_LIMIT) @@ -66,7 +66,7 @@ pub(crate) fn first_unreserved_index( /// would violate the gap invariant if used. pub(crate) fn first_unsafe_index( conn: &rusqlite::Connection, - account_id: AccountId, + account_id: AccountRef, ) -> Result { // The inner join with `transactions` excludes addresses for which // `seen_in_tx` is NULL. The query also excludes addresses observed @@ -114,7 +114,7 @@ pub(crate) fn range_from(i: u32, n: u32) -> Range { pub(crate) fn get_ephemeral_ivk( conn: &rusqlite::Connection, params: &P, - account_id: AccountId, + account_id: AccountRef, ) -> Result, SqliteClientError> { let ufvk = conn .query_row( @@ -151,7 +151,7 @@ pub(crate) fn get_ephemeral_ivk( pub(crate) fn get_known_ephemeral_addresses( conn: &rusqlite::Connection, params: &P, - account_id: AccountId, + account_id: AccountRef, index_range: Option>, ) -> Result, SqliteClientError> { let index_range = index_range.unwrap_or(0..(1 << 31)); @@ -237,7 +237,7 @@ pub(crate) fn find_index_for_ephemeral_address_str( pub(crate) fn reserve_next_n_ephemeral_addresses( conn: &rusqlite::Transaction, params: &P, - account_id: AccountId, + account_id: AccountRef, n: usize, ) -> Result, SqliteClientError> { if n == 0 { @@ -270,7 +270,7 @@ pub(crate) fn reserve_next_n_ephemeral_addresses( pub(crate) fn init_account( conn: &rusqlite::Transaction, params: &P, - account_id: AccountId, + account_id: AccountRef, ) -> Result<(), SqliteClientError> { reserve_until(conn, params, account_id, 0) } @@ -287,7 +287,7 @@ pub(crate) fn init_account( fn reserve_until( conn: &rusqlite::Transaction, params: &P, - account_id: AccountId, + account_id: AccountRef, next_to_reserve: u32, ) -> Result<(), SqliteClientError> { assert!(next_to_reserve <= 1 << 31); @@ -401,7 +401,7 @@ pub(crate) fn mark_ephemeral_address_as_used( WHERE address = :address RETURNING account_id, address_index", named_params![":tx_ref": tx_ref.0, ":address": address_str], - |row| Ok((AccountId(row.get::<_, u32>(0)?), row.get::<_, u32>(1)?)), + |row| Ok((AccountRef(row.get::<_, u32>(0)?), row.get::<_, u32>(1)?)), ) .optional()?; @@ -454,7 +454,7 @@ pub(crate) fn mark_ephemeral_address_as_seen( WHERE address = :address RETURNING account_id, address_index", named_params![":seen_in_tx": &earlier_ref, ":address": address_str], - |row| Ok((AccountId(row.get::<_, u32>(0)?), row.get::<_, u32>(1)?)), + |row| Ok((AccountRef(row.get::<_, u32>(0)?), row.get::<_, u32>(1)?)), ) .optional()?;