Add migrations to support using UFVKs instead of Sapling extfvks.

Fixes #594
This commit is contained in:
Kris Nuttycombe 2022-08-02 16:10:28 -06:00
parent 7c5b320108
commit cdfaa57496
6 changed files with 292 additions and 68 deletions

View File

@ -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`:

View File

@ -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);

View File

@ -181,17 +181,15 @@ pub(crate) fn get_unified_full_viewing_keys<P: consensus::Parameters>(
.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<AccountId, UnifiedFullViewingKey> = 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);

View File

@ -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<P: consensus::Parameters> {
params: P,
seed: Vec<u8>,
}
impl<P: consensus::Parameters> Migration for WalletMigration2<P> {
fn id(&self) -> Uuid {
::uuid::Uuid::parse_str("be57ef3b-388e-42ea-97e2-678dafcf9754").unwrap()
}
fn dependencies(&self) -> HashSet<Uuid> {
["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<P: consensus::Parameters> RusqliteMigration for WalletMigration2<P> {
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<P>(wdb: &mut WalletDb<P>) -> 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<P: consensus::Parameters + 'static>(
wdb: &mut WalletDb<P>,
seed: Vec<u8>,
) -> Result<(), MigratorError<SqliteClientError>> {
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<P>(wdb: &mut WalletDb<P>) -> 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<P: consensus::Parameters>(
// 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<String> = 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<String> = None;
wdb.conn.execute(
"INSERT INTO accounts (account, ufvk, address, transparent_address)
VALUES (?, ?, ?, ?)",
params![<u32>::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<P: consensus::Parameters>(
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<String> = 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<String> = None;
conn.execute(
&format!(
"INSERT INTO {} (account, ufvk, address, transparent_address)
VALUES (?, ?, ?, ?)",
accounts_table
),
params![<u32>::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];

View File

@ -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);

View File

@ -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)
}