From 7c5b3201086d9e3146f96bc9f7d43b0770f2a9d7 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 2 Aug 2022 10:06:15 -0600 Subject: [PATCH 1/8] Introduce wallet database schema migrations. This replaces the current wallet initialization code with a migration that brings the database up to the state produced by release 0.3.0. A subsequent commit will add migrations that correctly produce the database state as of zcash/librustzcash@602270cb1ff4d1f64cb49dd0a6c23d350c950a66. Fixes #369 --- Cargo.toml | 2 + zcash_client_sqlite/Cargo.toml | 3 + zcash_client_sqlite/src/chain.rs | 28 +-- zcash_client_sqlite/src/wallet.rs | 4 +- zcash_client_sqlite/src/wallet/init.rs | 216 ++++++++++----------- zcash_client_sqlite/src/wallet/transact.rs | 28 +-- 6 files changed, 143 insertions(+), 138 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0ddd568e4..d7ee46ff6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,3 +21,5 @@ codegen-units = 1 [patch.crates-io] zcash_encoding = { path = "components/zcash_encoding" } zcash_note_encryption = { path = "components/zcash_note_encryption" } +schemer = { git = "https://github.com/aschampion/schemer.git", rev = "2370c96f43eda5b2cb267bbc9355f0fefff850bb" } +schemer-rusqlite = { git = "https://github.com/aschampion/schemer.git", rev = "2370c96f43eda5b2cb267bbc9355f0fefff850bb" } diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index 75d74e25e..0dd5b441b 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -22,7 +22,10 @@ protobuf = "~2.27.1" # MSRV 1.52.1 rand_core = "0.6" rusqlite = { version = "0.24", features = ["bundled", "time"] } secp256k1 = { version = "0.21" } +schemer = "0.1.2" +schemer-rusqlite = "0.1.1" time = "0.2" +uuid = "1.1" zcash_client_backend = { version = "0.5", path = "../zcash_client_backend" } zcash_primitives = { version = "0.7", path = "../zcash_primitives" } diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 5902d9542..cd477f728 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -97,8 +97,8 @@ mod tests { init_cache_database(&db_cache).unwrap(); let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -176,8 +176,8 @@ mod tests { init_cache_database(&db_cache).unwrap(); let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -246,8 +246,8 @@ mod tests { init_cache_database(&db_cache).unwrap(); let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -316,8 +316,8 @@ mod tests { init_cache_database(&db_cache).unwrap(); let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -385,8 +385,8 @@ mod tests { init_cache_database(&db_cache).unwrap(); let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -440,8 +440,8 @@ mod tests { init_cache_database(&db_cache).unwrap(); let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -492,8 +492,8 @@ mod tests { init_cache_database(&db_cache).unwrap(); let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 974f883fb..7e58c2c13 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1242,8 +1242,8 @@ mod tests { #[test] fn empty_database_has_no_balance() { let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet tests::init_test_accounts_table(&db_data); diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 76b26ac5c..457893145 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -1,6 +1,8 @@ //! Functions for initializing the various databases. -use rusqlite::{params, types::ToSql, NO_PARAMS}; +use rusqlite::{params, types::ToSql, Transaction, NO_PARAMS}; +use schemer::{migration, Migrator}; +use schemer_rusqlite::{RusqliteAdapter, RusqliteAdapterError, RusqliteMigration}; use std::collections::HashMap; use zcash_primitives::{ @@ -19,6 +21,90 @@ use { zcash_primitives::legacy::keys::IncomingViewingKey, }; +struct WalletMigration0; + +migration!( + WalletMigration0, + "bc4f5e57-d600-4b6c-990f-b3538f0bfce1", + [], + "Initialize the wallet database." +); + +impl RusqliteMigration for WalletMigration0 { + type Error = RusqliteAdapterError; + + fn up(&self, transaction: &Transaction) -> Result<(), RusqliteAdapterError> { + transaction.execute_batch( + "CREATE TABLE IF NOT EXISTS accounts ( + account INTEGER PRIMARY KEY, + extfvk TEXT NOT NULL, + address TEXT NOT NULL + ); + CREATE TABLE IF NOT EXISTS blocks ( + height INTEGER PRIMARY KEY, + hash BLOB NOT NULL, + time INTEGER NOT NULL, + sapling_tree BLOB NOT NULL + ); + CREATE TABLE IF NOT EXISTS transactions ( + id_tx INTEGER PRIMARY KEY, + txid BLOB NOT NULL UNIQUE, + created TEXT, + block INTEGER, + tx_index INTEGER, + expiry_height INTEGER, + raw BLOB, + FOREIGN KEY (block) REFERENCES blocks(height) + ); + CREATE TABLE IF NOT EXISTS received_notes ( + id_note INTEGER PRIMARY KEY, + tx INTEGER NOT NULL, + output_index INTEGER NOT NULL, + account INTEGER NOT NULL, + diversifier BLOB NOT NULL, + value INTEGER NOT NULL, + rcm BLOB NOT NULL, + nf BLOB NOT NULL UNIQUE, + is_change INTEGER NOT NULL, + memo BLOB, + spent INTEGER, + FOREIGN KEY (tx) REFERENCES transactions(id_tx), + FOREIGN KEY (account) REFERENCES accounts(account), + FOREIGN KEY (spent) REFERENCES transactions(id_tx), + CONSTRAINT tx_output UNIQUE (tx, output_index) + ); + CREATE TABLE IF NOT EXISTS sapling_witnesses ( + id_witness INTEGER PRIMARY KEY, + note INTEGER NOT NULL, + block INTEGER NOT NULL, + witness BLOB NOT NULL, + FOREIGN KEY (note) REFERENCES received_notes(id_note), + FOREIGN KEY (block) REFERENCES blocks(height), + CONSTRAINT witness_height UNIQUE (note, block) + ); + CREATE TABLE IF NOT EXISTS sent_notes ( + id_note INTEGER PRIMARY KEY, + tx INTEGER NOT NULL, + output_index INTEGER NOT NULL, + from_account INTEGER NOT NULL, + address TEXT NOT NULL, + value INTEGER NOT NULL, + memo BLOB, + FOREIGN KEY (tx) REFERENCES transactions(id_tx), + FOREIGN KEY (from_account) REFERENCES accounts(account), + CONSTRAINT tx_output UNIQUE (tx, output_index) + );", + )?; + Ok(()) + } + + fn down(&self, _transaction: &Transaction) -> Result<(), RusqliteAdapterError> { + // We should never down-migrate the first migration, as that can irreversibly + // destroy data. + Ok(()) + } +} + /// Sets up the internal structure of the data database. /// /// It is safe to use a wallet database created without the ability to create transparent spends @@ -40,106 +126,20 @@ use { /// }; /// /// let data_file = NamedTempFile::new().unwrap(); -/// let db = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); -/// init_wallet_db(&db).unwrap(); +/// let mut db = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); +/// init_wallet_db(&mut db, None).unwrap(); /// ``` -pub fn init_wallet_db

(wdb: &WalletDb

) -> Result<(), rusqlite::Error> { - // TODO: Add migrations (https://github.com/zcash/librustzcash/issues/489) - // - extfvk column -> ufvk column - wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS accounts ( - account INTEGER PRIMARY KEY, - ufvk TEXT, - address TEXT, - transparent_address TEXT - )", - NO_PARAMS, - )?; - wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS blocks ( - height INTEGER PRIMARY KEY, - hash BLOB NOT NULL, - time INTEGER NOT NULL, - sapling_tree BLOB NOT NULL - )", - NO_PARAMS, - )?; - wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS transactions ( - id_tx INTEGER PRIMARY KEY, - txid BLOB NOT NULL UNIQUE, - created TEXT, - block INTEGER, - tx_index INTEGER, - expiry_height INTEGER, - raw BLOB, - FOREIGN KEY (block) REFERENCES blocks(height) - )", - NO_PARAMS, - )?; - wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS received_notes ( - id_note INTEGER PRIMARY KEY, - tx INTEGER NOT NULL, - output_index INTEGER NOT NULL, - account INTEGER NOT NULL, - diversifier BLOB NOT NULL, - value INTEGER NOT NULL, - rcm BLOB NOT NULL, - nf BLOB NOT NULL UNIQUE, - is_change INTEGER NOT NULL, - memo BLOB, - spent INTEGER, - FOREIGN KEY (tx) REFERENCES transactions(id_tx), - FOREIGN KEY (account) REFERENCES accounts(account), - FOREIGN KEY (spent) REFERENCES transactions(id_tx), - CONSTRAINT tx_output UNIQUE (tx, output_index) - )", - NO_PARAMS, - )?; - wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS sapling_witnesses ( - id_witness INTEGER PRIMARY KEY, - note INTEGER NOT NULL, - block INTEGER NOT NULL, - witness BLOB NOT NULL, - FOREIGN KEY (note) REFERENCES received_notes(id_note), - FOREIGN KEY (block) REFERENCES blocks(height), - CONSTRAINT witness_height UNIQUE (note, block) - )", - NO_PARAMS, - )?; - wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS sent_notes ( - id_note INTEGER PRIMARY KEY, - tx INTEGER NOT NULL, - output_pool INTEGER NOT NULL, - output_index INTEGER NOT NULL, - from_account INTEGER NOT NULL, - address TEXT NOT NULL, - value INTEGER NOT NULL, - memo BLOB, - FOREIGN KEY (tx) REFERENCES transactions(id_tx), - FOREIGN KEY (from_account) REFERENCES accounts(account), - CONSTRAINT tx_output UNIQUE (tx, output_pool, output_index) - )", - NO_PARAMS, - )?; - wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS utxos ( - id_utxo INTEGER PRIMARY KEY, - address TEXT NOT NULL, - prevout_txid BLOB NOT NULL, - prevout_idx INTEGER NOT NULL, - script BLOB NOT NULL, - value_zat INTEGER NOT NULL, - height INTEGER NOT NULL, - spent_in_tx INTEGER, - FOREIGN KEY (spent_in_tx) REFERENCES transactions(id_tx), - CONSTRAINT tx_outpoint UNIQUE (prevout_txid, prevout_idx) - )", - NO_PARAMS, - )?; +pub fn init_wallet_db

(wdb: &mut WalletDb

) -> Result<(), rusqlite::Error> { + let adapter = RusqliteAdapter::new(&mut wdb.conn, Some("schemer_migrations".to_string())); + adapter.init().expect("Migrations table setup succeeds."); + + let mut migrator = Migrator::new(adapter); + let migration0 = Box::new(WalletMigration0 {}); + + migrator + .register(migration0) + .expect("Wallet migration failed"); + migrator.up(None).expect("Migrations succeed."); Ok(()) } @@ -176,8 +176,8 @@ pub fn init_wallet_db

(wdb: &WalletDb

) -> Result<(), rusqlite::Error> { /// }; /// /// let data_file = NamedTempFile::new().unwrap(); -/// let db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); -/// init_wallet_db(&db_data).unwrap(); +/// let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); +/// init_wallet_db(&mut db_data).unwrap(); /// /// let seed = [0u8; 32]; // insecure; replace with a strong random seed /// let account = AccountId::from(0); @@ -313,8 +313,8 @@ mod tests { #[test] fn init_accounts_table_only_works_once() { let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // We can call the function as many times as we want with no data init_accounts_table(&db_data, &HashMap::new()).unwrap(); @@ -353,8 +353,8 @@ mod tests { #[test] fn init_blocks_table_only_works_once() { let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // First call with data should initialise the blocks table init_blocks_table( @@ -380,8 +380,8 @@ mod tests { #[test] fn init_accounts_table_stores_correct_address() { let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); let seed = [0u8; 32]; diff --git a/zcash_client_sqlite/src/wallet/transact.rs b/zcash_client_sqlite/src/wallet/transact.rs index d20b74edb..a51a69aba 100644 --- a/zcash_client_sqlite/src/wallet/transact.rs +++ b/zcash_client_sqlite/src/wallet/transact.rs @@ -205,8 +205,8 @@ mod tests { #[test] fn create_to_address_fails_on_incorrect_extsk() { let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); let acct0 = AccountId::from(0); let acct1 = AccountId::from(1); @@ -289,8 +289,8 @@ mod tests { #[test] fn create_to_address_fails_with_no_blocks() { let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -327,8 +327,8 @@ mod tests { #[test] fn create_to_address_fails_on_insufficient_balance() { let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); init_blocks_table( &db_data, BlockHeight::from(1u32), @@ -385,8 +385,8 @@ mod tests { init_cache_database(&db_cache).unwrap(); let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -520,8 +520,8 @@ mod tests { init_cache_database(&db_cache).unwrap(); let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -651,8 +651,8 @@ mod tests { init_cache_database(&db_cache).unwrap(); let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), network).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), network).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -763,8 +763,8 @@ mod tests { init_cache_database(&db_cache).unwrap(); let data_file = NamedTempFile::new().unwrap(); - let db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&db_data).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); From cdfaa574969f30cc3ce37300709e1c5c1de4345b Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 2 Aug 2022 16:10:28 -0600 Subject: [PATCH 2/8] Add migrations to support using UFVKs instead of Sapling extfvks. Fixes #594 --- zcash_client_sqlite/CHANGELOG.md | 10 +- zcash_client_sqlite/src/chain.rs | 14 +- zcash_client_sqlite/src/wallet.rs | 20 +- zcash_client_sqlite/src/wallet/init.rs | 301 ++++++++++++++++++--- zcash_client_sqlite/src/wallet/transact.rs | 14 +- zcash_primitives/src/sapling/keys.rs | 1 - 6 files changed, 292 insertions(+), 68 deletions(-) diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 02168d2a7..d8aedacb9 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -19,7 +19,10 @@ and this library adheres to Rust's notion of ### Changed - Various **BREAKING CHANGES** have been made to the database tables. These will - require migrations, which may need to be performed in multiple steps. + require migrations, which may need to be performed in multiple steps. Migrations + will now be automatically performed for any user using + `zcash_client_sqlite::wallet::init_wallet_db`and it is recommended to use this + method to maintain the state of the database going forward. - The `extfvk` column in the `accounts` table has been replaced by a `ufvk` column. Values for this column should be derived from the wallet's seed and the account number; the Sapling component of the resulting Unified Full @@ -48,6 +51,11 @@ and this library adheres to Rust's notion of method to be used in contexts where a transaction has just been constructed, rather than only in the case that a transaction has been decrypted after being retrieved from the network. +- `zcash_client_sqlite::wallet::init_wallet_db` has been modified to + take the wallet seed as an argument so that it can correctly perform + migrations that require re-deriving key material. In particular for + this upgrade, the seed is used to derive UFVKs to replace the currently + stored Sapling extfvks as part of the migration process. ### Removed - `zcash_client_sqlite::wallet`: diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index cd477f728..fdaf80598 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -98,7 +98,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -177,7 +177,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -247,7 +247,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -317,7 +317,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -386,7 +386,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -441,7 +441,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -493,7 +493,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 7e58c2c13..750a039c9 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -181,17 +181,15 @@ pub(crate) fn get_unified_full_viewing_keys( .conn .prepare("SELECT account, ufvk FROM accounts ORDER BY account ASC")?; - let rows = stmt_fetch_accounts - .query_map(NO_PARAMS, |row| { - let acct: u32 = row.get(0)?; - let account = AccountId::from(acct); - let ufvk_str: String = row.get(1)?; - let ufvk = UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str) - .map_err(SqliteClientError::CorruptedData); + let rows = stmt_fetch_accounts.query_map(NO_PARAMS, |row| { + let acct: u32 = row.get(0)?; + let account = AccountId::from(acct); + let ufvk_str: String = row.get(1)?; + let ufvk = UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str) + .map_err(SqliteClientError::CorruptedData); - Ok((account, ufvk)) - }) - .map_err(SqliteClientError::from)?; + Ok((account, ufvk)) + })?; let mut res: HashMap = HashMap::new(); for row in rows { @@ -1243,7 +1241,7 @@ mod tests { fn empty_database_has_no_balance() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet tests::init_test_accounts_table(&db_data); diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 457893145..2e8cd0c82 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -1,9 +1,9 @@ //! Functions for initializing the various databases. - -use rusqlite::{params, types::ToSql, Transaction, NO_PARAMS}; -use schemer::{migration, Migrator}; -use schemer_rusqlite::{RusqliteAdapter, RusqliteAdapterError, RusqliteMigration}; -use std::collections::HashMap; +use rusqlite::{params, types::ToSql, Connection, Transaction, NO_PARAMS}; +use schemer::{migration, Migration, Migrator, MigratorError}; +use schemer_rusqlite::{RusqliteAdapter, RusqliteMigration}; +use std::collections::{HashMap, HashSet}; +use uuid::Uuid; use zcash_primitives::{ block::BlockHash, @@ -11,7 +11,10 @@ use zcash_primitives::{ zip32::AccountId, }; -use zcash_client_backend::keys::UnifiedFullViewingKey; +use zcash_client_backend::{ + encoding::{decode_payment_address, encode_payment_address}, + keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, +}; use crate::{error::SqliteClientError, WalletDb}; @@ -31,9 +34,9 @@ migration!( ); impl RusqliteMigration for WalletMigration0 { - type Error = RusqliteAdapterError; + type Error = SqliteClientError; - fn up(&self, transaction: &Transaction) -> Result<(), RusqliteAdapterError> { + fn up(&self, transaction: &Transaction) -> Result<(), SqliteClientError> { transaction.execute_batch( "CREATE TABLE IF NOT EXISTS accounts ( account INTEGER PRIMARY KEY, @@ -98,22 +101,209 @@ impl RusqliteMigration for WalletMigration0 { Ok(()) } - fn down(&self, _transaction: &Transaction) -> Result<(), RusqliteAdapterError> { + fn down(&self, _transaction: &Transaction) -> Result<(), SqliteClientError> { // We should never down-migrate the first migration, as that can irreversibly // destroy data. + panic!("Cannot revert the initial migration."); + } +} + +struct WalletMigration1; + +migration!( + WalletMigration1, + "a2e0ed2e-8852-475e-b0a4-f154b15b9dbe", + ["bc4f5e57-d600-4b6c-990f-b3538f0bfce1"], + "Add support for receiving transparent UTXOs." +); + +impl RusqliteMigration for WalletMigration1 { + type Error = SqliteClientError; + + fn up(&self, transaction: &Transaction) -> Result<(), SqliteClientError> { + transaction.execute_batch( + "CREATE TABLE IF NOT EXISTS utxos ( + id_utxo INTEGER PRIMARY KEY, + address TEXT NOT NULL, + prevout_txid BLOB NOT NULL, + prevout_idx INTEGER NOT NULL, + script BLOB NOT NULL, + value_zat INTEGER NOT NULL, + height INTEGER NOT NULL, + spent_in_tx INTEGER, + FOREIGN KEY (spent_in_tx) REFERENCES transactions(id_tx), + CONSTRAINT tx_outpoint UNIQUE (prevout_txid, prevout_idx) + );", + )?; Ok(()) } + + fn down(&self, transaction: &Transaction) -> Result<(), SqliteClientError> { + transaction.execute_batch("DROP TABLE utxos;")?; + Ok(()) + } +} + +struct WalletMigration2 { + params: P, + seed: Vec, +} + +impl Migration for WalletMigration2

{ + fn id(&self) -> Uuid { + ::uuid::Uuid::parse_str("be57ef3b-388e-42ea-97e2-678dafcf9754").unwrap() + } + + fn dependencies(&self) -> HashSet { + ["a2e0ed2e-8852-475e-b0a4-f154b15b9dbe"] + .iter() + .map(|uuidstr| ::uuid::Uuid::parse_str(uuidstr).unwrap()) + .collect() + } + + fn description(&self) -> &'static str { + "Add support for unified full viewing keys" + } +} + +impl RusqliteMigration for WalletMigration2

{ + type Error = SqliteClientError; + + fn up(&self, transaction: &Transaction) -> Result<(), SqliteClientError> { + // + // Update the accounts table to store ufvks rather than extfvks + // + + transaction.execute_batch( + "PRAGMA foreign_keys = OFF; + CREATE TABLE accounts_new ( + account INTEGER PRIMARY KEY, + ufvk TEXT NOT NULL, + address TEXT, + transparent_address TEXT + );", + )?; + + let mut stmt_fetch_accounts = + transaction.prepare("SELECT account, address FROM accounts")?; + + let rows = stmt_fetch_accounts.query_map(NO_PARAMS, |row| { + let account: u32 = row.get(0)?; + let address: String = row.get(1)?; + Ok((AccountId::from(account), address)) + })?; + + for row in rows { + let (account, address) = row?; + let decoded_address = + decode_payment_address(self.params.hrp_sapling_payment_address(), &address)? + .ok_or_else(|| { + SqliteClientError::CorruptedData(format!( + "Not a valid Sapling payment address: {}", + address + )) + })?; + + let usk = UnifiedSpendingKey::from_seed(&self.params, &self.seed, account).unwrap(); + let ufvk = usk.to_unified_full_viewing_key(); + + let dfvk = ufvk + .sapling() + .expect("Derivation should have produced a UFVK containing a Sapling component."); + let (idx, expected_address) = dfvk.default_address(); + if expected_address != decoded_address { + return Err(SqliteClientError::CorruptedData( + format!("Decoded Sapling address {} does not match the ufvk's Sapling address {} at {:?}.", + address, + encode_payment_address(self.params.hrp_sapling_payment_address(), &expected_address), + idx + ))); + } + + add_account_internal(&self.params, transaction, "accounts_new", account, &ufvk)?; + } + transaction.execute_batch( + "DROP TABLE accounts; + ALTER TABLE accounts_new RENAME TO accounts;", + )?; + + // + // Update the sent_notes table to inclue an output_pool column that + // is respected by the uniqueness constraint + // + + transaction.execute_batch( + "CREATE TABLE sent_notes_new ( + id_note INTEGER PRIMARY KEY, + tx INTEGER NOT NULL, + output_pool INTEGER NOT NULL , + output_index INTEGER NOT NULL, + from_account INTEGER NOT NULL, + address TEXT NOT NULL, + value INTEGER NOT NULL, + memo BLOB, + FOREIGN KEY (tx) REFERENCES transactions(id_tx), + FOREIGN KEY (from_account) REFERENCES accounts(account), + CONSTRAINT tx_output UNIQUE (tx, output_pool, output_index) + );", + )?; + + // we query in a nested scope so that the col_names iterator is correctly + // dropped and doesn't maintain a lock on the table. + let has_output_pool = { + let mut stmt_fetch_columns = transaction.prepare("PRAGMA TABLE_INFO('sent_notes')")?; + let mut col_names = stmt_fetch_columns.query_map(NO_PARAMS, |row| { + let col_name: String = row.get(1)?; + Ok(col_name) + })?; + + col_names.any(|cname| cname == Ok("output_pool".to_string())) + }; + + if has_output_pool { + transaction.execute_batch( + "INSERT INTO sent_notes_new + (id_note, tx, output_pool, output_index, from_account, address, value, memo) + SELECT id_note, tx, output_pool, output_index, from_account, address, value, memo + FROM sent_notes;" + )?; + } else { + transaction.execute_batch( + "INSERT INTO sent_notes_new + (id_note, tx, output_pool, output_index, from_account, address, value, memo) + SELECT id_note, tx, 2, output_index, from_account, address, value, memo + FROM sent_notes;", + )?; + } + + transaction.execute_batch( + "DROP TABLE sent_notes; + ALTER TABLE sent_notes_new RENAME TO sent_notes; + PRAGMA foreign_keys = ON;", + )?; + + Ok(()) + } + + fn down(&self, _transaction: &Transaction) -> Result<(), SqliteClientError> { + // TODO: something better than just panic? + panic!("Cannot revert this migration."); + } } /// Sets up the internal structure of the data database. /// -/// It is safe to use a wallet database created without the ability to create transparent spends -/// with a build that enables transparent spends via use of the `transparent-inputs` feature flag. -/// The reverse is unsafe, as wallet balance calculations would ignore the transparent UTXOs -/// controlled by the wallet. Note that this currently applies only to wallet databases created -/// with the same _version_ of the wallet software; database migration operations currently must -/// be manually performed to update the structure of the database when changing versions. -/// Integrated migration utilities will be provided by a future version of this library. +/// This procedure will automatically perform migration operations to update the wallet database to +/// the database structure required by the current version of this library, and should be invoked +/// at least once any time a client program upgrades to a new version of this library. The +/// operation of this procedure is idempotent, so it is safe (though not required) to invoke this +/// operation every time the wallet is opened. +/// +/// It is safe to use a wallet database previously created without the ability to create +/// transparent spends with a build that enables transparent spends (via use of the +/// `transparent-inputs` feature flag.) The reverse is unsafe, as wallet balance calculations would +/// ignore the transparent UTXOs already controlled by the wallet. +/// /// /// # Examples /// @@ -127,19 +317,33 @@ impl RusqliteMigration for WalletMigration0 { /// /// let data_file = NamedTempFile::new().unwrap(); /// let mut db = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); -/// init_wallet_db(&mut db, None).unwrap(); +/// init_wallet_db(&mut db, vec![]).unwrap(); /// ``` -pub fn init_wallet_db

(wdb: &mut WalletDb

) -> Result<(), rusqlite::Error> { +// TODO: It would be possible to make the transition from providing transparent support to no +// longer providing transparent support safe, by including a migration that verifies that no +// unspent transparent outputs exist in the wallet at the time of upgrading to a version of +// the library that does not support transparent use. It might be a good idea to add an explicit +// check for unspent transparent outputs whenever running initialization with a version of the +// library *not* compiled with the `transparent-inputs` feature flag, and fail if any are present. +pub fn init_wallet_db( + wdb: &mut WalletDb

, + seed: Vec, +) -> Result<(), MigratorError> { let adapter = RusqliteAdapter::new(&mut wdb.conn, Some("schemer_migrations".to_string())); adapter.init().expect("Migrations table setup succeeds."); let mut migrator = Migrator::new(adapter); let migration0 = Box::new(WalletMigration0 {}); + let migration1 = Box::new(WalletMigration1 {}); + let migration2 = Box::new(WalletMigration2 { + params: wdb.params.clone(), + seed, + }); migrator - .register(migration0) - .expect("Wallet migration failed"); - migrator.up(None).expect("Migrations succeed."); + .register_multiple(vec![migration0, migration1, migration2]) + .expect("Wallet migration registration should have been successful."); + migrator.up(None)?; Ok(()) } @@ -177,7 +381,7 @@ pub fn init_wallet_db

(wdb: &mut WalletDb

) -> Result<(), rusqlite::Error> { /// /// let data_file = NamedTempFile::new().unwrap(); /// let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); -/// init_wallet_db(&mut db_data).unwrap(); +/// init_wallet_db(&mut db_data, vec![]).unwrap(); /// /// let seed = [0u8; 32]; // insecure; replace with a strong random seed /// let account = AccountId::from(0); @@ -204,28 +408,43 @@ pub fn init_accounts_table( // Insert accounts atomically wdb.conn.execute("BEGIN IMMEDIATE", NO_PARAMS)?; for (account, key) in keys.iter() { - let ufvk_str: String = key.encode(&wdb.params); - let address_str: String = key.default_address().0.encode(&wdb.params); - #[cfg(feature = "transparent-inputs")] - let taddress_str: Option = key.transparent().and_then(|k| { - k.derive_external_ivk() - .ok() - .map(|k| k.default_address().0.encode(&wdb.params)) - }); - #[cfg(not(feature = "transparent-inputs"))] - let taddress_str: Option = None; - - wdb.conn.execute( - "INSERT INTO accounts (account, ufvk, address, transparent_address) - VALUES (?, ?, ?, ?)", - params![::from(*account), ufvk_str, address_str, taddress_str], - )?; + add_account_internal(&wdb.params, &wdb.conn, "accounts", *account, key)?; } wdb.conn.execute("COMMIT", NO_PARAMS)?; Ok(()) } +fn add_account_internal( + network: &P, + conn: &Connection, + accounts_table: &'static str, + account: AccountId, + key: &UnifiedFullViewingKey, +) -> Result<(), SqliteClientError> { + let ufvk_str: String = key.encode(network); + let address_str: String = key.default_address().0.encode(network); + #[cfg(feature = "transparent-inputs")] + let taddress_str: Option = key.transparent().and_then(|k| { + k.derive_external_ivk() + .ok() + .map(|k| k.default_address().0.encode(network)) + }); + #[cfg(not(feature = "transparent-inputs"))] + let taddress_str: Option = None; + + conn.execute( + &format!( + "INSERT INTO {} (account, ufvk, address, transparent_address) + VALUES (?, ?, ?, ?)", + accounts_table + ), + params![::from(account), ufvk_str, address_str, taddress_str], + )?; + + Ok(()) +} + /// Initialises the data database with the given block. /// /// This enables a newly-created database to be immediately-usable, without needing to @@ -314,7 +533,7 @@ mod tests { fn init_accounts_table_only_works_once() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // We can call the function as many times as we want with no data init_accounts_table(&db_data, &HashMap::new()).unwrap(); @@ -354,7 +573,7 @@ mod tests { fn init_blocks_table_only_works_once() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // First call with data should initialise the blocks table init_blocks_table( @@ -381,7 +600,7 @@ mod tests { fn init_accounts_table_stores_correct_address() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); let seed = [0u8; 32]; diff --git a/zcash_client_sqlite/src/wallet/transact.rs b/zcash_client_sqlite/src/wallet/transact.rs index a51a69aba..f01a9f73c 100644 --- a/zcash_client_sqlite/src/wallet/transact.rs +++ b/zcash_client_sqlite/src/wallet/transact.rs @@ -206,7 +206,7 @@ mod tests { fn create_to_address_fails_on_incorrect_extsk() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); let acct0 = AccountId::from(0); let acct1 = AccountId::from(1); @@ -290,7 +290,7 @@ mod tests { fn create_to_address_fails_with_no_blocks() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -328,7 +328,7 @@ mod tests { fn create_to_address_fails_on_insufficient_balance() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); init_blocks_table( &db_data, BlockHeight::from(1u32), @@ -386,7 +386,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -521,7 +521,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -652,7 +652,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), network).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -764,7 +764,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data).unwrap(); + init_wallet_db(&mut db_data, vec![]).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); diff --git a/zcash_primitives/src/sapling/keys.rs b/zcash_primitives/src/sapling/keys.rs index dae6df4d8..cd2fc592f 100644 --- a/zcash_primitives/src/sapling/keys.rs +++ b/zcash_primitives/src/sapling/keys.rs @@ -288,7 +288,6 @@ impl DiversifiableFullViewingKey { /// Returns the payment address corresponding to the smallest valid diversifier index, /// along with that index. - // TODO: See if this is only used in tests. pub fn default_address(&self) -> (zip32::DiversifierIndex, PaymentAddress) { zip32::sapling_default_address(&self.fvk, &self.dk) } From e9db8d5b017aae6cbeb208fae7dda67f2d8bfac9 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 11 Aug 2022 15:42:25 -0600 Subject: [PATCH 3/8] Use `secrecy` when handling seed phrases in migrations code. --- zcash_client_sqlite/Cargo.toml | 1 + zcash_client_sqlite/src/chain.rs | 15 +++++++------- zcash_client_sqlite/src/wallet.rs | 3 ++- zcash_client_sqlite/src/wallet/init.rs | 23 ++++++++++++++-------- zcash_client_sqlite/src/wallet/transact.rs | 15 +++++++------- 5 files changed, 34 insertions(+), 23 deletions(-) diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index 0dd5b441b..0043589cc 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -24,6 +24,7 @@ rusqlite = { version = "0.24", features = ["bundled", "time"] } secp256k1 = { version = "0.21" } schemer = "0.1.2" schemer-rusqlite = "0.1.1" +secrecy = "0.8" time = "0.2" uuid = "1.1" zcash_client_backend = { version = "0.5", path = "../zcash_client_backend" } diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index fdaf80598..1443ff8c9 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -67,6 +67,7 @@ where #[cfg(test)] #[allow(deprecated)] mod tests { + use secrecy::Secret; use tempfile::NamedTempFile; use zcash_primitives::{ @@ -98,7 +99,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -177,7 +178,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -247,7 +248,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -317,7 +318,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -386,7 +387,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -441,7 +442,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -493,7 +494,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 750a039c9..ba98af2be 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1227,6 +1227,7 @@ pub fn insert_sent_utxo<'a, P: consensus::Parameters>( #[cfg(test)] #[allow(deprecated)] mod tests { + use secrecy::Secret; use tempfile::NamedTempFile; use zcash_primitives::transaction::components::Amount; @@ -1241,7 +1242,7 @@ mod tests { fn empty_database_has_no_balance() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet tests::init_test_accounts_table(&db_data); diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 2e8cd0c82..869e6955d 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -2,6 +2,7 @@ use rusqlite::{params, types::ToSql, Connection, Transaction, NO_PARAMS}; use schemer::{migration, Migration, Migrator, MigratorError}; use schemer_rusqlite::{RusqliteAdapter, RusqliteMigration}; +use secrecy::{ExposeSecret, SecretVec}; use std::collections::{HashMap, HashSet}; use uuid::Uuid; @@ -146,7 +147,7 @@ impl RusqliteMigration for WalletMigration1 { struct WalletMigration2 { params: P, - seed: Vec, + seed: SecretVec, } impl Migration for WalletMigration2

{ @@ -204,7 +205,9 @@ impl RusqliteMigration for WalletMigration2

{ )) })?; - let usk = UnifiedSpendingKey::from_seed(&self.params, &self.seed, account).unwrap(); + let usk = + UnifiedSpendingKey::from_seed(&self.params, &self.seed.expose_secret(), account) + .unwrap(); let ufvk = usk.to_unified_full_viewing_key(); let dfvk = ufvk @@ -308,6 +311,7 @@ impl RusqliteMigration for WalletMigration2

{ /// # Examples /// /// ``` +/// use secrecy::Secret; /// use tempfile::NamedTempFile; /// use zcash_primitives::consensus::Network; /// use zcash_client_sqlite::{ @@ -317,7 +321,7 @@ impl RusqliteMigration for WalletMigration2

{ /// /// let data_file = NamedTempFile::new().unwrap(); /// let mut db = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); -/// init_wallet_db(&mut db, vec![]).unwrap(); +/// init_wallet_db(&mut db, Secret::new(vec![])).unwrap(); /// ``` // TODO: It would be possible to make the transition from providing transparent support to no // longer providing transparent support safe, by including a migration that verifies that no @@ -327,7 +331,7 @@ impl RusqliteMigration for WalletMigration2

{ // library *not* compiled with the `transparent-inputs` feature flag, and fail if any are present. pub fn init_wallet_db( wdb: &mut WalletDb

, - seed: Vec, + seed: SecretVec, ) -> Result<(), MigratorError> { let adapter = RusqliteAdapter::new(&mut wdb.conn, Some("schemer_migrations".to_string())); adapter.init().expect("Migrations table setup succeeds."); @@ -360,6 +364,7 @@ pub fn init_wallet_db( /// # #[cfg(feature = "transparent-inputs")] /// # { /// use tempfile::NamedTempFile; +/// use secrecy::Secret; /// use std::collections::HashMap; /// /// use zcash_primitives::{ @@ -381,7 +386,7 @@ pub fn init_wallet_db( /// /// let data_file = NamedTempFile::new().unwrap(); /// let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); -/// init_wallet_db(&mut db_data, vec![]).unwrap(); +/// init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); /// /// let seed = [0u8; 32]; // insecure; replace with a strong random seed /// let account = AccountId::from(0); @@ -506,6 +511,8 @@ pub fn init_blocks_table

( #[cfg(test)] #[allow(deprecated)] mod tests { + use rusqlite::{self, NO_PARAMS}; + use secrecy::Secret; use std::collections::HashMap; use tempfile::NamedTempFile; @@ -533,7 +540,7 @@ mod tests { fn init_accounts_table_only_works_once() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // We can call the function as many times as we want with no data init_accounts_table(&db_data, &HashMap::new()).unwrap(); @@ -573,7 +580,7 @@ mod tests { fn init_blocks_table_only_works_once() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // First call with data should initialise the blocks table init_blocks_table( @@ -600,7 +607,7 @@ mod tests { fn init_accounts_table_stores_correct_address() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); let seed = [0u8; 32]; diff --git a/zcash_client_sqlite/src/wallet/transact.rs b/zcash_client_sqlite/src/wallet/transact.rs index f01a9f73c..3ea27876e 100644 --- a/zcash_client_sqlite/src/wallet/transact.rs +++ b/zcash_client_sqlite/src/wallet/transact.rs @@ -157,6 +157,7 @@ pub fn select_spendable_sapling_notes

( #[allow(deprecated)] mod tests { use rusqlite::Connection; + use secrecy::Secret; use std::collections::HashMap; use tempfile::NamedTempFile; @@ -206,7 +207,7 @@ mod tests { fn create_to_address_fails_on_incorrect_extsk() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); let acct0 = AccountId::from(0); let acct1 = AccountId::from(1); @@ -290,7 +291,7 @@ mod tests { fn create_to_address_fails_with_no_blocks() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -328,7 +329,7 @@ mod tests { fn create_to_address_fails_on_insufficient_balance() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); init_blocks_table( &db_data, BlockHeight::from(1u32), @@ -386,7 +387,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -521,7 +522,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -652,7 +653,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), network).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -764,7 +765,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, vec![]).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); From 4930982d7e573bf2c1076b57c4d1f6964aad5ee4 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 11 Aug 2022 15:44:42 -0600 Subject: [PATCH 4/8] Test migrations against possibly-previously-existing database states. This adds tests that verifies that migrations can run successfully against databases in the following states: * created by release version 0.3.0 * created by the `autoshielding_poc` branch * created by current `main` prior to addition of migrations --- zcash_client_sqlite/src/wallet/init.rs | 366 ++++++++++++++++++++++++- 1 file changed, 361 insertions(+), 5 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 869e6955d..a4235e1f7 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -206,7 +206,7 @@ impl RusqliteMigration for WalletMigration2

{ })?; let usk = - UnifiedSpendingKey::from_seed(&self.params, &self.seed.expose_secret(), account) + UnifiedSpendingKey::from_seed(&self.params, self.seed.expose_secret(), account) .unwrap(); let ufvk = usk.to_unified_full_viewing_key(); @@ -219,8 +219,7 @@ impl RusqliteMigration for WalletMigration2

{ format!("Decoded Sapling address {} does not match the ufvk's Sapling address {} at {:?}.", address, encode_payment_address(self.params.hrp_sapling_payment_address(), &expected_address), - idx - ))); + idx))); } add_account_internal(&self.params, transaction, "accounts_new", account, &ufvk)?; @@ -511,12 +510,15 @@ pub fn init_blocks_table

( #[cfg(test)] #[allow(deprecated)] mod tests { - use rusqlite::{self, NO_PARAMS}; + use rusqlite::{self, ToSql, NO_PARAMS}; use secrecy::Secret; use std::collections::HashMap; use tempfile::NamedTempFile; - use zcash_client_backend::keys::{sapling, UnifiedFullViewingKey, UnifiedSpendingKey}; + use zcash_client_backend::{ + encoding::{encode_extended_full_viewing_key, encode_payment_address}, + keys::{sapling, UnifiedFullViewingKey, UnifiedSpendingKey}, + }; #[cfg(feature = "transparent-inputs")] use zcash_primitives::legacy::keys as transparent; @@ -536,6 +538,360 @@ mod tests { use super::{init_accounts_table, init_blocks_table, init_wallet_db}; + #[test] + fn init_migrate_from_0_3_0() { + fn init_0_3_0

( + wdb: &mut WalletDb

, + extfvk: &ExtendedFullViewingKey, + account: AccountId, + ) -> Result<(), rusqlite::Error> { + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS accounts ( + account INTEGER PRIMARY KEY, + extfvk TEXT NOT NULL, + address TEXT NOT NULL + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS blocks ( + height INTEGER PRIMARY KEY, + hash BLOB NOT NULL, + time INTEGER NOT NULL, + sapling_tree BLOB NOT NULL + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS transactions ( + id_tx INTEGER PRIMARY KEY, + txid BLOB NOT NULL UNIQUE, + created TEXT, + block INTEGER, + tx_index INTEGER, + expiry_height INTEGER, + raw BLOB, + FOREIGN KEY (block) REFERENCES blocks(height) + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS received_notes ( + id_note INTEGER PRIMARY KEY, + tx INTEGER NOT NULL, + output_index INTEGER NOT NULL, + account INTEGER NOT NULL, + diversifier BLOB NOT NULL, + value INTEGER NOT NULL, + rcm BLOB NOT NULL, + nf BLOB NOT NULL UNIQUE, + is_change INTEGER NOT NULL, + memo BLOB, + spent INTEGER, + FOREIGN KEY (tx) REFERENCES transactions(id_tx), + FOREIGN KEY (account) REFERENCES accounts(account), + FOREIGN KEY (spent) REFERENCES transactions(id_tx), + CONSTRAINT tx_output UNIQUE (tx, output_index) + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS sapling_witnesses ( + id_witness INTEGER PRIMARY KEY, + note INTEGER NOT NULL, + block INTEGER NOT NULL, + witness BLOB NOT NULL, + FOREIGN KEY (note) REFERENCES received_notes(id_note), + FOREIGN KEY (block) REFERENCES blocks(height), + CONSTRAINT witness_height UNIQUE (note, block) + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS sent_notes ( + id_note INTEGER PRIMARY KEY, + tx INTEGER NOT NULL, + output_index INTEGER NOT NULL, + from_account INTEGER NOT NULL, + address TEXT NOT NULL, + value INTEGER NOT NULL, + memo BLOB, + FOREIGN KEY (tx) REFERENCES transactions(id_tx), + FOREIGN KEY (from_account) REFERENCES accounts(account), + CONSTRAINT tx_output UNIQUE (tx, output_index) + )", + NO_PARAMS, + )?; + + let address = encode_payment_address( + tests::network().hrp_sapling_payment_address(), + &extfvk.default_address().1, + ); + let extfvk = encode_extended_full_viewing_key( + tests::network().hrp_sapling_extended_full_viewing_key(), + extfvk, + ); + wdb.conn.execute( + "INSERT INTO accounts (account, extfvk, address) + VALUES (?, ?, ?)", + &[ + u32::from(account).to_sql()?, + extfvk.to_sql()?, + address.to_sql()?, + ], + )?; + + Ok(()) + } + + let seed = [0xab; 32]; + let account = AccountId::from(0); + let secret_key = sapling::spending_key(&seed, tests::network().coin_type(), account); + let extfvk = ExtendedFullViewingKey::from(&secret_key); + let data_file = NamedTempFile::new().unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_0_3_0(&mut db_data, &extfvk, account).unwrap(); + init_wallet_db(&mut db_data, Secret::new(seed.to_vec())).unwrap(); + } + + #[test] + fn init_migrate_from_autoshielding_poc() { + fn init_autoshielding

( + wdb: &WalletDb

, + extfvk: &ExtendedFullViewingKey, + account: AccountId, + ) -> Result<(), rusqlite::Error> { + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS accounts ( + account INTEGER PRIMARY KEY, + extfvk TEXT NOT NULL, + address TEXT NOT NULL, + transparent_address TEXT NOT NULL + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS blocks ( + height INTEGER PRIMARY KEY, + hash BLOB NOT NULL, + time INTEGER NOT NULL, + sapling_tree BLOB NOT NULL + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS transactions ( + id_tx INTEGER PRIMARY KEY, + txid BLOB NOT NULL UNIQUE, + created TEXT, + block INTEGER, + tx_index INTEGER, + expiry_height INTEGER, + raw BLOB, + FOREIGN KEY (block) REFERENCES blocks(height) + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS received_notes ( + id_note INTEGER PRIMARY KEY, + tx INTEGER NOT NULL, + output_index INTEGER NOT NULL, + account INTEGER NOT NULL, + diversifier BLOB NOT NULL, + value INTEGER NOT NULL, + rcm BLOB NOT NULL, + nf BLOB NOT NULL UNIQUE, + is_change INTEGER NOT NULL, + memo BLOB, + spent INTEGER, + FOREIGN KEY (tx) REFERENCES transactions(id_tx), + FOREIGN KEY (account) REFERENCES accounts(account), + FOREIGN KEY (spent) REFERENCES transactions(id_tx), + CONSTRAINT tx_output UNIQUE (tx, output_index) + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS sapling_witnesses ( + id_witness INTEGER PRIMARY KEY, + note INTEGER NOT NULL, + block INTEGER NOT NULL, + witness BLOB NOT NULL, + FOREIGN KEY (note) REFERENCES received_notes(id_note), + FOREIGN KEY (block) REFERENCES blocks(height), + CONSTRAINT witness_height UNIQUE (note, block) + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS sent_notes ( + id_note INTEGER PRIMARY KEY, + tx INTEGER NOT NULL, + output_index INTEGER NOT NULL, + from_account INTEGER NOT NULL, + address TEXT NOT NULL, + value INTEGER NOT NULL, + memo BLOB, + FOREIGN KEY (tx) REFERENCES transactions(id_tx), + FOREIGN KEY (from_account) REFERENCES accounts(account), + CONSTRAINT tx_output UNIQUE (tx, output_index) + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS utxos ( + id_utxo INTEGER PRIMARY KEY, + address TEXT NOT NULL, + prevout_txid BLOB NOT NULL, + prevout_idx INTEGER NOT NULL, + script BLOB NOT NULL, + value_zat INTEGER NOT NULL, + height INTEGER NOT NULL, + spent_in_tx INTEGER, + FOREIGN KEY (spent_in_tx) REFERENCES transactions(id_tx), + CONSTRAINT tx_outpoint UNIQUE (prevout_txid, prevout_idx) + )", + NO_PARAMS, + )?; + + let address = encode_payment_address( + tests::network().hrp_sapling_payment_address(), + &extfvk.default_address().1, + ); + let extfvk = encode_extended_full_viewing_key( + tests::network().hrp_sapling_extended_full_viewing_key(), + extfvk, + ); + wdb.conn.execute( + "INSERT INTO accounts (account, extfvk, address, transparent_address) + VALUES (?, ?, ?, '')", + &[ + u32::from(account).to_sql()?, + extfvk.to_sql()?, + address.to_sql()?, + ], + )?; + + Ok(()) + } + + let seed = [0xab; 32]; + let account = AccountId::from(0); + let secret_key = sapling::spending_key(&seed, tests::network().coin_type(), account); + let extfvk = ExtendedFullViewingKey::from(&secret_key); + let data_file = NamedTempFile::new().unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_autoshielding(&db_data, &extfvk, account).unwrap(); + init_wallet_db(&mut db_data, Secret::new(seed.to_vec())).unwrap(); + } + + #[test] + fn init_migrate_from_main_pre_migrations() { + fn init_main

(wdb: &WalletDb

) -> Result<(), rusqlite::Error> { + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS accounts ( + account INTEGER PRIMARY KEY, + ufvk TEXT, + address TEXT, + transparent_address TEXT + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS blocks ( + height INTEGER PRIMARY KEY, + hash BLOB NOT NULL, + time INTEGER NOT NULL, + sapling_tree BLOB NOT NULL + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS transactions ( + id_tx INTEGER PRIMARY KEY, + txid BLOB NOT NULL UNIQUE, + created TEXT, + block INTEGER, + tx_index INTEGER, + expiry_height INTEGER, + raw BLOB, + FOREIGN KEY (block) REFERENCES blocks(height) + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS received_notes ( + id_note INTEGER PRIMARY KEY, + tx INTEGER NOT NULL, + output_index INTEGER NOT NULL, + account INTEGER NOT NULL, + diversifier BLOB NOT NULL, + value INTEGER NOT NULL, + rcm BLOB NOT NULL, + nf BLOB NOT NULL UNIQUE, + is_change INTEGER NOT NULL, + memo BLOB, + spent INTEGER, + FOREIGN KEY (tx) REFERENCES transactions(id_tx), + FOREIGN KEY (account) REFERENCES accounts(account), + FOREIGN KEY (spent) REFERENCES transactions(id_tx), + CONSTRAINT tx_output UNIQUE (tx, output_index) + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS sapling_witnesses ( + id_witness INTEGER PRIMARY KEY, + note INTEGER NOT NULL, + block INTEGER NOT NULL, + witness BLOB NOT NULL, + FOREIGN KEY (note) REFERENCES received_notes(id_note), + FOREIGN KEY (block) REFERENCES blocks(height), + CONSTRAINT witness_height UNIQUE (note, block) + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS sent_notes ( + id_note INTEGER PRIMARY KEY, + tx INTEGER NOT NULL, + output_pool INTEGER NOT NULL, + output_index INTEGER NOT NULL, + from_account INTEGER NOT NULL, + address TEXT NOT NULL, + value INTEGER NOT NULL, + memo BLOB, + FOREIGN KEY (tx) REFERENCES transactions(id_tx), + FOREIGN KEY (from_account) REFERENCES accounts(account), + CONSTRAINT tx_output UNIQUE (tx, output_pool, output_index) + )", + NO_PARAMS, + )?; + wdb.conn.execute( + "CREATE TABLE IF NOT EXISTS utxos ( + id_utxo INTEGER PRIMARY KEY, + address TEXT NOT NULL, + prevout_txid BLOB NOT NULL, + prevout_idx INTEGER NOT NULL, + script BLOB NOT NULL, + value_zat INTEGER NOT NULL, + height INTEGER NOT NULL, + spent_in_tx INTEGER, + FOREIGN KEY (spent_in_tx) REFERENCES transactions(id_tx), + CONSTRAINT tx_outpoint UNIQUE (prevout_txid, prevout_idx) + )", + NO_PARAMS, + )?; + Ok(()) + } + + let data_file = NamedTempFile::new().unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_main(&db_data).unwrap(); + init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + } + #[test] fn init_accounts_table_only_works_once() { let data_file = NamedTempFile::new().unwrap(); From 880076b38f0e62a51dbee554258ddb106ef117b2 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 15 Aug 2022 13:14:15 -0600 Subject: [PATCH 5/8] Improve error reporting for address & viewing key decoding. --- zcash_client_backend/CHANGELOG.md | 7 +- .../examples/diversify-address.rs | 8 +- zcash_client_backend/src/encoding.rs | 78 ++++++++++++++----- zcash_client_backend/src/zip321.rs | 4 +- zcash_client_sqlite/src/error.rs | 19 +++-- zcash_client_sqlite/src/wallet/init.rs | 8 +- 6 files changed, 81 insertions(+), 43 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 7f7fd13e2..f8dedbe63 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -37,6 +37,7 @@ and this library adheres to Rust's notion of - `TransactionRequest::new` for constructing a request from `Vec`. - `TransactionRequest::payments` for accessing the `Payments` that make up a request. +- `zcash_client_backend::encoding::KeyError` - New experimental APIs that should be considered unstable, and are likely to be modified and/or moved to a different module in a future release: @@ -113,7 +114,11 @@ and this library adheres to Rust's notion of derived from ZIP 316 UFVKs and UIVKs. - `welding_rig::scan_block` now uses batching for trial-decryption of transaction outputs. - +- The return type of the following methods in `zcash_client_backend::encoding` + have been changed to improve error reporting: + - `decode_extended_spending_key` + - `decode_extended_full_viewing_key` + - `decode_payment_address` ### Removed - `zcash_client_backend::data_api`: diff --git a/zcash_client_backend/examples/diversify-address.rs b/zcash_client_backend/examples/diversify-address.rs index de20a456e..1071566c9 100644 --- a/zcash_client_backend/examples/diversify-address.rs +++ b/zcash_client_backend/examples/diversify-address.rs @@ -9,16 +9,12 @@ use zcash_primitives::{ fn parse_viewing_key(s: &str) -> Result<(ExtendedFullViewingKey, bool), &'static str> { decode_extended_full_viewing_key(mainnet::HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY, s) - .ok() - .flatten() .map(|vk| (vk, true)) - .or_else(|| { + .or_else(|_| { decode_extended_full_viewing_key(testnet::HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY, s) - .ok() - .flatten() .map(|vk| (vk, false)) }) - .ok_or("Invalid Sapling viewing key") + .map_err(|_| "Invalid Sapling viewing key") } fn parse_diversifier_index(s: &str) -> Result { diff --git a/zcash_client_backend/src/encoding.rs b/zcash_client_backend/src/encoding.rs index 1bebbf5cf..c36e8c778 100644 --- a/zcash_client_backend/src/encoding.rs +++ b/zcash_client_backend/src/encoding.rs @@ -26,15 +26,55 @@ where bech32::encode(hrp, data.to_base32(), Variant::Bech32).expect("hrp is invalid") } -fn bech32_decode(hrp: &str, s: &str, read: F) -> Result, Error> +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Bech32DecodeError { + Bech32Error(Error), + IncorrectVariant(Variant), + ReadError, + HrpMismatch { expected: String, actual: String }, +} + +impl From for Bech32DecodeError { + fn from(err: Error) -> Self { + Bech32DecodeError::Bech32Error(err) + } +} + +impl fmt::Display for Bech32DecodeError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match &self { + Bech32DecodeError::Bech32Error(e) => write!(f, "{}", e), + Bech32DecodeError::IncorrectVariant(variant) => write!( + f, + "Incorrect bech32 encoding (wrong variant: {:?})", + variant + ), + Bech32DecodeError::ReadError => { + write!(f, "Failed to decode key from its binary representation.") + } + Bech32DecodeError::HrpMismatch { expected, actual } => write!( + f, + "Key was encoded for a different network: expected {}, got {}.", + expected, actual + ), + } + } +} + +fn bech32_decode(hrp: &str, s: &str, read: F) -> Result where F: Fn(Vec) -> Option, { - match bech32::decode(s)? { - (decoded_hrp, data, Variant::Bech32) if decoded_hrp == hrp => { - Vec::::from_base32(&data).map(read) - } - _ => Ok(None), + let (decoded_hrp, data, variant) = bech32::decode(s)?; + if variant != Variant::Bech32 { + Err(Bech32DecodeError::IncorrectVariant(variant)) + } else if decoded_hrp != hrp { + Err(Bech32DecodeError::HrpMismatch { + expected: hrp.to_string(), + actual: decoded_hrp, + }) + } else { + read(Vec::::from_base32(&data)?).ok_or(Bech32DecodeError::ReadError) } } @@ -121,7 +161,7 @@ pub fn encode_extended_spending_key(hrp: &str, extsk: &ExtendedSpendingKey) -> S pub fn decode_extended_spending_key( hrp: &str, s: &str, -) -> Result, Error> { +) -> Result { bech32_decode(hrp, s, |data| ExtendedSpendingKey::read(&data[..]).ok()) } @@ -155,7 +195,7 @@ pub fn encode_extended_full_viewing_key(hrp: &str, extfvk: &ExtendedFullViewingK pub fn decode_extended_full_viewing_key( hrp: &str, s: &str, -) -> Result, Error> { +) -> Result { bech32_decode(hrp, s, |data| ExtendedFullViewingKey::read(&data[..]).ok()) } @@ -240,11 +280,11 @@ pub fn encode_payment_address_p( /// HRP_SAPLING_PAYMENT_ADDRESS, /// "ztestsapling1qqqqqqqqqqqqqqqqqqcguyvaw2vjk4sdyeg0lc970u659lvhqq7t0np6hlup5lusxle75ss7jnk", /// ), -/// Ok(Some(pa)), +/// Ok(pa), /// ); /// ``` /// [`PaymentAddress`]: zcash_primitives::sapling::PaymentAddress -pub fn decode_payment_address(hrp: &str, s: &str) -> Result, Error> { +pub fn decode_payment_address(hrp: &str, s: &str) -> Result { bech32_decode(hrp, s, |data| { if data.len() != 43 { return None; @@ -392,6 +432,7 @@ mod tests { use super::{ decode_extended_full_viewing_key, decode_extended_spending_key, decode_payment_address, encode_extended_full_viewing_key, encode_extended_spending_key, encode_payment_address, + Bech32DecodeError, }; #[test] @@ -414,7 +455,7 @@ mod tests { encoded_main ) .unwrap(), - Some(extsk.clone()) + extsk ); assert_eq!( @@ -430,7 +471,7 @@ mod tests { encoded_test ) .unwrap(), - Some(extsk) + extsk ); } @@ -454,7 +495,7 @@ mod tests { encoded_main ) .unwrap(), - Some(extfvk.clone()) + extfvk ); assert_eq!( @@ -470,7 +511,7 @@ mod tests { encoded_test ) .unwrap(), - Some(extfvk) + extfvk ); } @@ -500,7 +541,7 @@ mod tests { encoded_main ) .unwrap(), - Some(addr.clone()) + addr ); assert_eq!( @@ -513,7 +554,7 @@ mod tests { encoded_test ) .unwrap(), - Some(addr) + addr ); } @@ -535,9 +576,8 @@ mod tests { decode_payment_address( constants::mainnet::HRP_SAPLING_PAYMENT_ADDRESS, &encoded_main - ) - .unwrap(), - None + ), + Err(Bech32DecodeError::ReadError) ); } } diff --git a/zcash_client_backend/src/zip321.rs b/zcash_client_backend/src/zip321.rs index 7d78b17f0..75f5a0374 100644 --- a/zcash_client_backend/src/zip321.rs +++ b/zcash_client_backend/src/zip321.rs @@ -793,7 +793,7 @@ mod tests { let expected = TransactionRequest { payments: vec![ Payment { - recipient_address: RecipientAddress::Shielded(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap().unwrap()), + recipient_address: RecipientAddress::Shielded(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap()), amount: Amount::from_u64(376876902796286).unwrap(), memo: None, label: None, @@ -811,7 +811,7 @@ mod tests { let req = TransactionRequest { payments: vec![ Payment { - recipient_address: RecipientAddress::Shielded(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap().unwrap()), + recipient_address: RecipientAddress::Shielded(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap()), amount: Amount::from_u64(0).unwrap(), memo: None, label: None, diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 33e21c2bf..a45a66a08 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -3,7 +3,10 @@ use std::error; use std::fmt; -use zcash_client_backend::{data_api, encoding::TransparentCodecError}; +use zcash_client_backend::{ + data_api, + encoding::{Bech32DecodeError, TransparentCodecError}, +}; use zcash_primitives::consensus::BlockHeight; use crate::{NoteId, PRUNING_HEIGHT}; @@ -27,8 +30,8 @@ pub enum SqliteClientError { /// Illegal attempt to reinitialize an already-initialized wallet database. TableNotEmpty, - /// Bech32 decoding error - Bech32(bech32::Error), + /// A Bech32-encoded key or address decoding error + Bech32DecodeError(Bech32DecodeError), /// Base58 decoding error Base58(bs58::decode::Error), @@ -58,7 +61,7 @@ impl error::Error for SqliteClientError { fn source(&self) -> Option<&(dyn error::Error + 'static)> { match &self { SqliteClientError::InvalidMemo(e) => Some(e), - SqliteClientError::Bech32(e) => Some(e), + SqliteClientError::Bech32DecodeError(Bech32DecodeError::Bech32Error(e)) => Some(e), SqliteClientError::DbError(e) => Some(e), SqliteClientError::Io(e) => Some(e), _ => None, @@ -78,7 +81,7 @@ impl fmt::Display for SqliteClientError { write!(f, "The note ID associated with an inserted witness must correspond to a received note."), SqliteClientError::RequestedRewindInvalid(h, r) => write!(f, "A rewind must be either of less than {} blocks, or at least back to block {} for your wallet; the requested height was {}.", PRUNING_HEIGHT, h, r), - SqliteClientError::Bech32(e) => write!(f, "{}", e), + SqliteClientError::Bech32DecodeError(e) => write!(f, "{}", e), SqliteClientError::Base58(e) => write!(f, "{}", e), SqliteClientError::TransparentAddress(e) => write!(f, "{}", e), SqliteClientError::TableNotEmpty => write!(f, "Table is not empty"), @@ -102,9 +105,9 @@ impl From for SqliteClientError { } } -impl From for SqliteClientError { - fn from(e: bech32::Error) -> Self { - SqliteClientError::Bech32(e) +impl From for SqliteClientError { + fn from(e: Bech32DecodeError) -> Self { + SqliteClientError::Bech32DecodeError(e) } } diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index a4235e1f7..18772a0dd 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -197,13 +197,7 @@ impl RusqliteMigration for WalletMigration2

{ for row in rows { let (account, address) = row?; let decoded_address = - decode_payment_address(self.params.hrp_sapling_payment_address(), &address)? - .ok_or_else(|| { - SqliteClientError::CorruptedData(format!( - "Not a valid Sapling payment address: {}", - address - )) - })?; + decode_payment_address(self.params.hrp_sapling_payment_address(), &address)?; let usk = UnifiedSpendingKey::from_seed(&self.params, self.seed.expose_secret(), account) From e0c919cc683584995bf36de72c7f7afd497b9195 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 15 Aug 2022 20:31:47 -0600 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Daira Hopwood --- zcash_client_sqlite/CHANGELOG.md | 5 +-- zcash_client_sqlite/src/wallet/init.rs | 46 ++++++++++++++------------ 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index d8aedacb9..b7f3acf95 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -21,7 +21,7 @@ and this library adheres to Rust's notion of - Various **BREAKING CHANGES** have been made to the database tables. These will require migrations, which may need to be performed in multiple steps. Migrations will now be automatically performed for any user using - `zcash_client_sqlite::wallet::init_wallet_db`and it is recommended to use this + `zcash_client_sqlite::wallet::init_wallet_db` and it is recommended to use this method to maintain the state of the database going forward. - The `extfvk` column in the `accounts` table has been replaced by a `ufvk` column. Values for this column should be derived from the wallet's seed and @@ -55,7 +55,8 @@ and this library adheres to Rust's notion of take the wallet seed as an argument so that it can correctly perform migrations that require re-deriving key material. In particular for this upgrade, the seed is used to derive UFVKs to replace the currently - stored Sapling extfvks as part of the migration process. + stored Sapling extfvks (without loss of information) as part of the + migration process. ### Removed - `zcash_client_sqlite::wallet`: diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 18772a0dd..4dfd27e35 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -229,7 +229,8 @@ impl RusqliteMigration for WalletMigration2

{ // transaction.execute_batch( - "CREATE TABLE sent_notes_new ( + "PRAGMA foreign_keys = ON; + CREATE TABLE sent_notes_new ( id_note INTEGER PRIMARY KEY, tx INTEGER NOT NULL, output_pool INTEGER NOT NULL , @@ -273,7 +274,8 @@ impl RusqliteMigration for WalletMigration2

{ } transaction.execute_batch( - "DROP TABLE sent_notes; + "PRAGMA foreign_keys = OFF; + DROP TABLE sent_notes; ALTER TABLE sent_notes_new RENAME TO sent_notes; PRAGMA foreign_keys = ON;", )?; @@ -540,7 +542,7 @@ mod tests { account: AccountId, ) -> Result<(), rusqlite::Error> { wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS accounts ( + "CREATE TABLE accounts ( account INTEGER PRIMARY KEY, extfvk TEXT NOT NULL, address TEXT NOT NULL @@ -548,7 +550,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS blocks ( + "CREATE TABLE blocks ( height INTEGER PRIMARY KEY, hash BLOB NOT NULL, time INTEGER NOT NULL, @@ -557,7 +559,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS transactions ( + "CREATE TABLE transactions ( id_tx INTEGER PRIMARY KEY, txid BLOB NOT NULL UNIQUE, created TEXT, @@ -570,7 +572,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS received_notes ( + "CREATE TABLE received_notes ( id_note INTEGER PRIMARY KEY, tx INTEGER NOT NULL, output_index INTEGER NOT NULL, @@ -590,7 +592,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS sapling_witnesses ( + "CREATE TABLE sapling_witnesses ( id_witness INTEGER PRIMARY KEY, note INTEGER NOT NULL, block INTEGER NOT NULL, @@ -602,7 +604,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS sent_notes ( + "CREATE TABLE sent_notes ( id_note INTEGER PRIMARY KEY, tx INTEGER NOT NULL, output_index INTEGER NOT NULL, @@ -656,7 +658,7 @@ mod tests { account: AccountId, ) -> Result<(), rusqlite::Error> { wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS accounts ( + "CREATE TABLE accounts ( account INTEGER PRIMARY KEY, extfvk TEXT NOT NULL, address TEXT NOT NULL, @@ -665,7 +667,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS blocks ( + "CREATE TABLE blocks ( height INTEGER PRIMARY KEY, hash BLOB NOT NULL, time INTEGER NOT NULL, @@ -674,7 +676,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS transactions ( + "CREATE TABLE transactions ( id_tx INTEGER PRIMARY KEY, txid BLOB NOT NULL UNIQUE, created TEXT, @@ -687,7 +689,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS received_notes ( + "CREATE TABLE received_notes ( id_note INTEGER PRIMARY KEY, tx INTEGER NOT NULL, output_index INTEGER NOT NULL, @@ -707,7 +709,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS sapling_witnesses ( + "CREATE TABLE sapling_witnesses ( id_witness INTEGER PRIMARY KEY, note INTEGER NOT NULL, block INTEGER NOT NULL, @@ -719,7 +721,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS sent_notes ( + "CREATE TABLE sent_notes ( id_note INTEGER PRIMARY KEY, tx INTEGER NOT NULL, output_index INTEGER NOT NULL, @@ -734,7 +736,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS utxos ( + "CREATE TABLE utxos ( id_utxo INTEGER PRIMARY KEY, address TEXT NOT NULL, prevout_txid BLOB NOT NULL, @@ -784,7 +786,7 @@ mod tests { fn init_migrate_from_main_pre_migrations() { fn init_main

(wdb: &WalletDb

) -> Result<(), rusqlite::Error> { wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS accounts ( + "CREATE TABLE accounts ( account INTEGER PRIMARY KEY, ufvk TEXT, address TEXT, @@ -793,7 +795,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS blocks ( + "CREATE TABLE blocks ( height INTEGER PRIMARY KEY, hash BLOB NOT NULL, time INTEGER NOT NULL, @@ -802,7 +804,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS transactions ( + "CREATE TABLE transactions ( id_tx INTEGER PRIMARY KEY, txid BLOB NOT NULL UNIQUE, created TEXT, @@ -815,7 +817,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS received_notes ( + "CREATE TABLE received_notes ( id_note INTEGER PRIMARY KEY, tx INTEGER NOT NULL, output_index INTEGER NOT NULL, @@ -835,7 +837,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS sapling_witnesses ( + "CREATE TABLE sapling_witnesses ( id_witness INTEGER PRIMARY KEY, note INTEGER NOT NULL, block INTEGER NOT NULL, @@ -847,7 +849,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS sent_notes ( + "CREATE TABLE sent_notes ( id_note INTEGER PRIMARY KEY, tx INTEGER NOT NULL, output_pool INTEGER NOT NULL, @@ -863,7 +865,7 @@ mod tests { NO_PARAMS, )?; wdb.conn.execute( - "CREATE TABLE IF NOT EXISTS utxos ( + "CREATE TABLE utxos ( id_utxo INTEGER PRIMARY KEY, address TEXT NOT NULL, prevout_txid BLOB NOT NULL, From 61fb732e7bac3d7be4ba4acdb7cfd0cc06879dc9 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 17 Aug 2022 14:31:28 -0600 Subject: [PATCH 7/8] Ensure that we detect the correct pool type for sent notes. --- zcash_client_sqlite/src/wallet/init.rs | 196 ++++++++++++++++++++----- 1 file changed, 158 insertions(+), 38 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 4dfd27e35..5c9fe34ef 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -13,11 +13,11 @@ use zcash_primitives::{ }; use zcash_client_backend::{ - encoding::{decode_payment_address, encode_payment_address}, + address::RecipientAddress, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, }; -use crate::{error::SqliteClientError, WalletDb}; +use crate::{error::SqliteClientError, wallet::PoolType, WalletDb}; #[cfg(feature = "transparent-inputs")] use { @@ -176,8 +176,7 @@ impl RusqliteMigration for WalletMigration2

{ // transaction.execute_batch( - "PRAGMA foreign_keys = OFF; - CREATE TABLE accounts_new ( + "CREATE TABLE accounts_new ( account INTEGER PRIMARY KEY, ufvk TEXT NOT NULL, address TEXT, @@ -188,32 +187,50 @@ impl RusqliteMigration for WalletMigration2

{ let mut stmt_fetch_accounts = transaction.prepare("SELECT account, address FROM accounts")?; - let rows = stmt_fetch_accounts.query_map(NO_PARAMS, |row| { + let mut rows = stmt_fetch_accounts.query(NO_PARAMS)?; + while let Some(row) = rows.next()? { let account: u32 = row.get(0)?; - let address: String = row.get(1)?; - Ok((AccountId::from(account), address)) - })?; - - for row in rows { - let (account, address) = row?; - let decoded_address = - decode_payment_address(self.params.hrp_sapling_payment_address(), &address)?; - + let account = AccountId::from(account); let usk = UnifiedSpendingKey::from_seed(&self.params, self.seed.expose_secret(), account) .unwrap(); let ufvk = usk.to_unified_full_viewing_key(); - let dfvk = ufvk - .sapling() - .expect("Derivation should have produced a UFVK containing a Sapling component."); - let (idx, expected_address) = dfvk.default_address(); - if expected_address != decoded_address { - return Err(SqliteClientError::CorruptedData( - format!("Decoded Sapling address {} does not match the ufvk's Sapling address {} at {:?}.", - address, - encode_payment_address(self.params.hrp_sapling_payment_address(), &expected_address), - idx))); + let address: String = row.get(1)?; + let decoded = RecipientAddress::decode(&self.params, &address).ok_or_else(|| { + SqliteClientError::CorruptedData(format!( + "Could not decode {} as a valid Zcash address.", + address + )) + })?; + match decoded { + RecipientAddress::Shielded(decoded_address) => { + let dfvk = ufvk.sapling().expect( + "Derivation should have produced a UFVK containing a Sapling component.", + ); + let (idx, expected_address) = dfvk.default_address(); + if decoded_address != expected_address { + return Err(SqliteClientError::CorruptedData( + format!("Decoded Sapling address {} does not match the ufvk's Sapling address {} at {:?}.", + address, + RecipientAddress::Shielded(expected_address).encode(&self.params), + idx))); + } + } + RecipientAddress::Transparent(_) => { + return Err(SqliteClientError::CorruptedData( + "Address field value decoded to a transparent address; should have been Sapling or unified.".to_string())); + } + RecipientAddress::Unified(decoded_address) => { + let (expected_address, idx) = ufvk.default_address(); + if decoded_address != expected_address { + return Err(SqliteClientError::CorruptedData( + format!("Decoded unified address {} does not match the ufvk's default address {} at {:?}.", + address, + RecipientAddress::Unified(expected_address).encode(&self.params), + idx))); + } + } } add_account_internal(&self.params, transaction, "accounts_new", account, &ufvk)?; @@ -229,8 +246,7 @@ impl RusqliteMigration for WalletMigration2

{ // transaction.execute_batch( - "PRAGMA foreign_keys = ON; - CREATE TABLE sent_notes_new ( + "CREATE TABLE sent_notes_new ( id_note INTEGER PRIMARY KEY, tx INTEGER NOT NULL, output_pool INTEGER NOT NULL , @@ -265,19 +281,59 @@ impl RusqliteMigration for WalletMigration2

{ FROM sent_notes;" )?; } else { - transaction.execute_batch( - "INSERT INTO sent_notes_new - (id_note, tx, output_pool, output_index, from_account, address, value, memo) - SELECT id_note, tx, 2, output_index, from_account, address, value, memo - FROM sent_notes;", + let mut stmt_fetch_sent_notes = transaction.prepare( + "SELECT id_note, tx, output_index, from_account, address, value, memo + FROM sent_notes", )?; + + let mut stmt_insert_sent_note = transaction.prepare( + "INSERT INTO sent_notes_new + (id_note, tx, output_pool, output_index, from_account, address, value, memo) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)", + )?; + + let mut rows = stmt_fetch_sent_notes.query(NO_PARAMS)?; + while let Some(row) = rows.next()? { + let id_note: i64 = row.get(0)?; + let tx_ref: i64 = row.get(1)?; + let output_index: i64 = row.get(2)?; + let account_id: u32 = row.get(3)?; + let address: String = row.get(4)?; + let value: i64 = row.get(5)?; + let memo: Option> = row.get(6)?; + + let decoded_address = + RecipientAddress::decode(&self.params, &address).ok_or_else(|| { + SqliteClientError::CorruptedData(format!( + "Could not decode {} as a valid Zcash address.", + address + )) + })?; + let output_pool = match decoded_address { + RecipientAddress::Shielded(_) => Ok(PoolType::Sapling.typecode()), + RecipientAddress::Transparent(_) => Ok(PoolType::Transparent.typecode()), + RecipientAddress::Unified(_) => Err(SqliteClientError::CorruptedData( + "Unified addresses should not yet appear in the sent_notes table." + .to_string(), + )), + }?; + + stmt_insert_sent_note.execute(params![ + id_note, + tx_ref, + output_pool, + output_index, + account_id, + address, + value, + memo + ])?; + } } transaction.execute_batch( - "PRAGMA foreign_keys = OFF; - DROP TABLE sent_notes; - ALTER TABLE sent_notes_new RENAME TO sent_notes; - PRAGMA foreign_keys = ON;", + "DROP TABLE sent_notes; + ALTER TABLE sent_notes_new RENAME TO sent_notes;", )?; Ok(()) @@ -328,6 +384,9 @@ pub fn init_wallet_db( wdb: &mut WalletDb

, seed: SecretVec, ) -> Result<(), MigratorError> { + wdb.conn + .execute("PRAGMA foreign_keys = OFF", NO_PARAMS) + .map_err(|e| MigratorError::Adapter(SqliteClientError::from(e)))?; let adapter = RusqliteAdapter::new(&mut wdb.conn, Some("schemer_migrations".to_string())); adapter.init().expect("Migrations table setup succeeds."); @@ -343,6 +402,9 @@ pub fn init_wallet_db( .register_multiple(vec![migration0, migration1, migration2]) .expect("Wallet migration registration should have been successful."); migrator.up(None)?; + wdb.conn + .execute("PRAGMA foreign_keys = ON", NO_PARAMS) + .map_err(|e| MigratorError::Adapter(SqliteClientError::from(e)))?; Ok(()) } @@ -512,6 +574,7 @@ mod tests { use tempfile::NamedTempFile; use zcash_client_backend::{ + address::RecipientAddress, encoding::{encode_extended_full_viewing_key, encode_payment_address}, keys::{sapling, UnifiedFullViewingKey, UnifiedSpendingKey}, }; @@ -769,6 +832,21 @@ mod tests { ], )?; + // add a sapling sent note + wdb.conn.execute( + "INSERT INTO blocks (height, hash, time, sapling_tree) VALUES (0, 0, 0, '')", + NO_PARAMS, + )?; + wdb.conn.execute( + "INSERT INTO transactions (block, id_tx, txid) VALUES (0, 0, '')", + NO_PARAMS, + )?; + wdb.conn.execute( + "INSERT INTO sent_notes (tx, output_index, from_account, address, value) + VALUES (0, 0, ?, ?, 0)", + &[u32::from(account).to_sql()?, address.to_sql()?], + )?; + Ok(()) } @@ -784,7 +862,11 @@ mod tests { #[test] fn init_migrate_from_main_pre_migrations() { - fn init_main

(wdb: &WalletDb

) -> Result<(), rusqlite::Error> { + fn init_main

( + wdb: &WalletDb

, + ufvk: &UnifiedFullViewingKey, + account: AccountId, + ) -> Result<(), rusqlite::Error> { wdb.conn.execute( "CREATE TABLE accounts ( account INTEGER PRIMARY KEY, @@ -879,13 +961,51 @@ mod tests { )", NO_PARAMS, )?; + + let ufvk_str = ufvk.encode(&tests::network()); + let address_str = + RecipientAddress::Unified(ufvk.default_address().0).encode(&tests::network()); + wdb.conn.execute( + "INSERT INTO accounts (account, ufvk, address, transparent_address) + VALUES (?, ?, ?, '')", + &[ + u32::from(account).to_sql()?, + ufvk_str.to_sql()?, + address_str.to_sql()?, + ], + )?; + + // add a transparent "sent note" + #[cfg(feature = "transparent-inputs")] + { + let taddr = RecipientAddress::Transparent( + ufvk.default_address().0.transparent().unwrap().clone(), + ) + .encode(&tests::network()); + wdb.conn.execute( + "INSERT INTO blocks (height, hash, time, sapling_tree) VALUES (0, 0, 0, '')", + NO_PARAMS, + )?; + wdb.conn.execute( + "INSERT INTO transactions (block, id_tx, txid) VALUES (0, 0, '')", + NO_PARAMS, + )?; + wdb.conn.execute( + "INSERT INTO sent_notes (tx, output_pool, output_index, from_account, address, value) + VALUES (0, ?, 0, ?, ?, 0)", + &[PoolType::Transparent.typecode(), u32::from(account).to_sql()?, taddr.to_sql()?])?; + } + Ok(()) } + let seed = [0xab; 32]; + let account = AccountId::from(0); + let secret_key = UnifiedSpendingKey::from_seed(&tests::network(), &seed, account).unwrap(); let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_main(&db_data).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_main(&db_data, &secret_key.to_unified_full_viewing_key(), account).unwrap(); + init_wallet_db(&mut db_data, Secret::new(seed.to_vec())).unwrap(); } #[test] From 913d5720876794fe26440976024596fd172aadf6 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 18 Aug 2022 10:03:22 -0600 Subject: [PATCH 8/8] Allow the seed to be an optional argument to database init. This adds a new `WalletMigrationError` type so that we have a good place to report whether or not the seed is required. --- zcash_client_sqlite/src/chain.rs | 14 +- zcash_client_sqlite/src/wallet.rs | 2 +- zcash_client_sqlite/src/wallet/init.rs | 212 ++++++++++++++------- zcash_client_sqlite/src/wallet/transact.rs | 14 +- 4 files changed, 154 insertions(+), 88 deletions(-) diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 1443ff8c9..4fceac181 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -99,7 +99,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -178,7 +178,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -248,7 +248,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -318,7 +318,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -387,7 +387,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -442,7 +442,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); @@ -494,7 +494,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index ba98af2be..0934e82ea 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1242,7 +1242,7 @@ mod tests { fn empty_database_has_no_balance() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet tests::init_test_accounts_table(&db_data); diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 5c9fe34ef..a367423c1 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -1,9 +1,10 @@ //! Functions for initializing the various databases. -use rusqlite::{params, types::ToSql, Connection, Transaction, NO_PARAMS}; +use rusqlite::{self, params, types::ToSql, Connection, Transaction, NO_PARAMS}; use schemer::{migration, Migration, Migrator, MigratorError}; use schemer_rusqlite::{RusqliteAdapter, RusqliteMigration}; use secrecy::{ExposeSecret, SecretVec}; use std::collections::{HashMap, HashSet}; +use std::fmt; use uuid::Uuid; use zcash_primitives::{ @@ -25,6 +26,50 @@ use { zcash_primitives::legacy::keys::IncomingViewingKey, }; +#[derive(Debug)] +pub enum WalletMigrationError { + /// The seed is required for the migration. + SeedRequired, + + /// Decoding of an existing value from its serialized form has failed. + CorruptedData(String), + + /// Wrapper for rusqlite errors. + DbError(rusqlite::Error), +} + +impl From for WalletMigrationError { + fn from(e: rusqlite::Error) -> Self { + WalletMigrationError::DbError(e) + } +} + +impl fmt::Display for WalletMigrationError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match &self { + WalletMigrationError::SeedRequired => { + write!( + f, + "The wallet seed is required in order to update the database." + ) + } + WalletMigrationError::CorruptedData(reason) => { + write!(f, "Wallet database is corrupted: {}", reason) + } + WalletMigrationError::DbError(e) => write!(f, "{}", e), + } + } +} + +impl std::error::Error for WalletMigrationError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match &self { + WalletMigrationError::DbError(e) => Some(e), + _ => None, + } + } +} + struct WalletMigration0; migration!( @@ -35,9 +80,9 @@ migration!( ); impl RusqliteMigration for WalletMigration0 { - type Error = SqliteClientError; + type Error = WalletMigrationError; - fn up(&self, transaction: &Transaction) -> Result<(), SqliteClientError> { + fn up(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { transaction.execute_batch( "CREATE TABLE IF NOT EXISTS accounts ( account INTEGER PRIMARY KEY, @@ -102,7 +147,7 @@ impl RusqliteMigration for WalletMigration0 { Ok(()) } - fn down(&self, _transaction: &Transaction) -> Result<(), SqliteClientError> { + fn down(&self, _transaction: &Transaction) -> Result<(), WalletMigrationError> { // We should never down-migrate the first migration, as that can irreversibly // destroy data. panic!("Cannot revert the initial migration."); @@ -119,9 +164,9 @@ migration!( ); impl RusqliteMigration for WalletMigration1 { - type Error = SqliteClientError; + type Error = WalletMigrationError; - fn up(&self, transaction: &Transaction) -> Result<(), SqliteClientError> { + fn up(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { transaction.execute_batch( "CREATE TABLE IF NOT EXISTS utxos ( id_utxo INTEGER PRIMARY KEY, @@ -139,7 +184,7 @@ impl RusqliteMigration for WalletMigration1 { Ok(()) } - fn down(&self, transaction: &Transaction) -> Result<(), SqliteClientError> { + fn down(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { transaction.execute_batch("DROP TABLE utxos;")?; Ok(()) } @@ -147,7 +192,7 @@ impl RusqliteMigration for WalletMigration1 { struct WalletMigration2 { params: P, - seed: SecretVec, + seed: Option>, } impl Migration for WalletMigration2

{ @@ -168,9 +213,9 @@ impl Migration for WalletMigration2

{ } impl RusqliteMigration for WalletMigration2

{ - type Error = SqliteClientError; + type Error = WalletMigrationError; - fn up(&self, transaction: &Transaction) -> Result<(), SqliteClientError> { + fn up(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { // // Update the accounts table to store ufvks rather than extfvks // @@ -189,52 +234,67 @@ impl RusqliteMigration for WalletMigration2

{ let mut rows = stmt_fetch_accounts.query(NO_PARAMS)?; while let Some(row) = rows.next()? { - let account: u32 = row.get(0)?; - let account = AccountId::from(account); - let usk = - UnifiedSpendingKey::from_seed(&self.params, self.seed.expose_secret(), account) - .unwrap(); - let ufvk = usk.to_unified_full_viewing_key(); + // We only need to check for the presence of the seed if we have keys that + // need to be migrated; otherwise, it's fine to not supply the seed if this + // migration is being used to initialize an empty database. + if let Some(seed) = &self.seed { + let account: u32 = row.get(0)?; + let account = AccountId::from(account); + let usk = + UnifiedSpendingKey::from_seed(&self.params, seed.expose_secret(), account) + .unwrap(); + let ufvk = usk.to_unified_full_viewing_key(); - let address: String = row.get(1)?; - let decoded = RecipientAddress::decode(&self.params, &address).ok_or_else(|| { - SqliteClientError::CorruptedData(format!( - "Could not decode {} as a valid Zcash address.", - address - )) - })?; - match decoded { - RecipientAddress::Shielded(decoded_address) => { - let dfvk = ufvk.sapling().expect( - "Derivation should have produced a UFVK containing a Sapling component.", - ); - let (idx, expected_address) = dfvk.default_address(); - if decoded_address != expected_address { - return Err(SqliteClientError::CorruptedData( - format!("Decoded Sapling address {} does not match the ufvk's Sapling address {} at {:?}.", - address, - RecipientAddress::Shielded(expected_address).encode(&self.params), - idx))); - } - } - RecipientAddress::Transparent(_) => { - return Err(SqliteClientError::CorruptedData( - "Address field value decoded to a transparent address; should have been Sapling or unified.".to_string())); - } - RecipientAddress::Unified(decoded_address) => { - let (expected_address, idx) = ufvk.default_address(); - if decoded_address != expected_address { - return Err(SqliteClientError::CorruptedData( - format!("Decoded unified address {} does not match the ufvk's default address {} at {:?}.", - address, - RecipientAddress::Unified(expected_address).encode(&self.params), - idx))); + let address: String = row.get(1)?; + let decoded = + RecipientAddress::decode(&self.params, &address).ok_or_else(|| { + WalletMigrationError::CorruptedData(format!( + "Could not decode {} as a valid Zcash address.", + address + )) + })?; + match decoded { + RecipientAddress::Shielded(decoded_address) => { + let dfvk = ufvk.sapling().expect( + "Derivation should have produced a UFVK containing a Sapling component.", + ); + let (idx, expected_address) = dfvk.default_address(); + if decoded_address != expected_address { + return Err(WalletMigrationError::CorruptedData( + format!("Decoded Sapling address {} does not match the ufvk's Sapling address {} at {:?}.", + address, + RecipientAddress::Shielded(expected_address).encode(&self.params), + idx))); + } + } + RecipientAddress::Transparent(_) => { + return Err(WalletMigrationError::CorruptedData( + "Address field value decoded to a transparent address; should have been Sapling or unified.".to_string())); + } + RecipientAddress::Unified(decoded_address) => { + let (expected_address, idx) = ufvk.default_address(); + if decoded_address != expected_address { + return Err(WalletMigrationError::CorruptedData( + format!("Decoded unified address {} does not match the ufvk's default address {} at {:?}.", + address, + RecipientAddress::Unified(expected_address).encode(&self.params), + idx))); + } } } + + add_account_internal::( + &self.params, + transaction, + "accounts_new", + account, + &ufvk, + )?; + } else { + return Err(WalletMigrationError::SeedRequired); } - - add_account_internal(&self.params, transaction, "accounts_new", account, &ufvk)?; } + transaction.execute_batch( "DROP TABLE accounts; ALTER TABLE accounts_new RENAME TO accounts;", @@ -304,7 +364,7 @@ impl RusqliteMigration for WalletMigration2

{ let decoded_address = RecipientAddress::decode(&self.params, &address).ok_or_else(|| { - SqliteClientError::CorruptedData(format!( + WalletMigrationError::CorruptedData(format!( "Could not decode {} as a valid Zcash address.", address )) @@ -312,7 +372,7 @@ impl RusqliteMigration for WalletMigration2

{ let output_pool = match decoded_address { RecipientAddress::Shielded(_) => Ok(PoolType::Sapling.typecode()), RecipientAddress::Transparent(_) => Ok(PoolType::Transparent.typecode()), - RecipientAddress::Unified(_) => Err(SqliteClientError::CorruptedData( + RecipientAddress::Unified(_) => Err(WalletMigrationError::CorruptedData( "Unified addresses should not yet appear in the sent_notes table." .to_string(), )), @@ -339,7 +399,7 @@ impl RusqliteMigration for WalletMigration2

{ Ok(()) } - fn down(&self, _transaction: &Transaction) -> Result<(), SqliteClientError> { + fn down(&self, _transaction: &Transaction) -> Result<(), WalletMigrationError> { // TODO: something better than just panic? panic!("Cannot revert this migration."); } @@ -372,7 +432,7 @@ impl RusqliteMigration for WalletMigration2

{ /// /// let data_file = NamedTempFile::new().unwrap(); /// let mut db = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); -/// init_wallet_db(&mut db, Secret::new(vec![])).unwrap(); +/// init_wallet_db(&mut db, Some(Secret::new(vec![]))).unwrap(); /// ``` // TODO: It would be possible to make the transition from providing transparent support to no // longer providing transparent support safe, by including a migration that verifies that no @@ -382,11 +442,11 @@ impl RusqliteMigration for WalletMigration2

{ // library *not* compiled with the `transparent-inputs` feature flag, and fail if any are present. pub fn init_wallet_db( wdb: &mut WalletDb

, - seed: SecretVec, -) -> Result<(), MigratorError> { + seed: Option>, +) -> Result<(), MigratorError> { wdb.conn .execute("PRAGMA foreign_keys = OFF", NO_PARAMS) - .map_err(|e| MigratorError::Adapter(SqliteClientError::from(e)))?; + .map_err(|e| MigratorError::Adapter(WalletMigrationError::from(e)))?; let adapter = RusqliteAdapter::new(&mut wdb.conn, Some("schemer_migrations".to_string())); adapter.init().expect("Migrations table setup succeeds."); @@ -404,7 +464,7 @@ pub fn init_wallet_db( migrator.up(None)?; wdb.conn .execute("PRAGMA foreign_keys = ON", NO_PARAMS) - .map_err(|e| MigratorError::Adapter(SqliteClientError::from(e)))?; + .map_err(|e| MigratorError::Adapter(WalletMigrationError::from(e)))?; Ok(()) } @@ -443,7 +503,7 @@ pub fn init_wallet_db( /// /// let data_file = NamedTempFile::new().unwrap(); /// let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); -/// init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); +/// init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); /// /// let seed = [0u8; 32]; // insecure; replace with a strong random seed /// let account = AccountId::from(0); @@ -470,20 +530,26 @@ pub fn init_accounts_table( // Insert accounts atomically wdb.conn.execute("BEGIN IMMEDIATE", NO_PARAMS)?; for (account, key) in keys.iter() { - add_account_internal(&wdb.params, &wdb.conn, "accounts", *account, key)?; + add_account_internal::( + &wdb.params, + &wdb.conn, + "accounts", + *account, + key, + )?; } wdb.conn.execute("COMMIT", NO_PARAMS)?; Ok(()) } -fn add_account_internal( +fn add_account_internal>( network: &P, conn: &Connection, accounts_table: &'static str, account: AccountId, key: &UnifiedFullViewingKey, -) -> Result<(), SqliteClientError> { +) -> Result<(), E> { let ufvk_str: String = key.encode(network); let address_str: String = key.default_address().0.encode(network); #[cfg(feature = "transparent-inputs")] @@ -579,9 +645,6 @@ mod tests { keys::{sapling, UnifiedFullViewingKey, UnifiedSpendingKey}, }; - #[cfg(feature = "transparent-inputs")] - use zcash_primitives::legacy::keys as transparent; - use zcash_primitives::{ block::BlockHash, consensus::{BlockHeight, Parameters}, @@ -597,6 +660,9 @@ mod tests { use super::{init_accounts_table, init_blocks_table, init_wallet_db}; + #[cfg(feature = "transparent-inputs")] + use {crate::wallet::PoolType, zcash_primitives::legacy::keys as transparent}; + #[test] fn init_migrate_from_0_3_0() { fn init_0_3_0

( @@ -710,7 +776,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); init_0_3_0(&mut db_data, &extfvk, account).unwrap(); - init_wallet_db(&mut db_data, Secret::new(seed.to_vec())).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(seed.to_vec()))).unwrap(); } #[test] @@ -857,7 +923,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); init_autoshielding(&db_data, &extfvk, account).unwrap(); - init_wallet_db(&mut db_data, Secret::new(seed.to_vec())).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(seed.to_vec()))).unwrap(); } #[test] @@ -993,7 +1059,7 @@ mod tests { wdb.conn.execute( "INSERT INTO sent_notes (tx, output_pool, output_index, from_account, address, value) VALUES (0, ?, 0, ?, ?, 0)", - &[PoolType::Transparent.typecode(), u32::from(account).to_sql()?, taddr.to_sql()?])?; + &[PoolType::Transparent.typecode().to_sql()?, u32::from(account).to_sql()?, taddr.to_sql()?])?; } Ok(()) @@ -1005,14 +1071,14 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); init_main(&db_data, &secret_key.to_unified_full_viewing_key(), account).unwrap(); - init_wallet_db(&mut db_data, Secret::new(seed.to_vec())).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(seed.to_vec()))).unwrap(); } #[test] fn init_accounts_table_only_works_once() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // We can call the function as many times as we want with no data init_accounts_table(&db_data, &HashMap::new()).unwrap(); @@ -1052,7 +1118,7 @@ mod tests { fn init_blocks_table_only_works_once() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // First call with data should initialise the blocks table init_blocks_table( @@ -1079,7 +1145,7 @@ mod tests { fn init_accounts_table_stores_correct_address() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); let seed = [0u8; 32]; diff --git a/zcash_client_sqlite/src/wallet/transact.rs b/zcash_client_sqlite/src/wallet/transact.rs index 3ea27876e..112dc9260 100644 --- a/zcash_client_sqlite/src/wallet/transact.rs +++ b/zcash_client_sqlite/src/wallet/transact.rs @@ -207,7 +207,7 @@ mod tests { fn create_to_address_fails_on_incorrect_extsk() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); let acct0 = AccountId::from(0); let acct1 = AccountId::from(1); @@ -291,7 +291,7 @@ mod tests { fn create_to_address_fails_with_no_blocks() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -329,7 +329,7 @@ mod tests { fn create_to_address_fails_on_insufficient_balance() { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); init_blocks_table( &db_data, BlockHeight::from(1u32), @@ -387,7 +387,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -522,7 +522,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -653,7 +653,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), network).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0); @@ -765,7 +765,7 @@ mod tests { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Secret::new(vec![])).unwrap(); + init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // Add an account to the wallet let account_id = AccountId::from(0);