Skip to content

Commit

Permalink
zcash_client_sqlite: Store UFVK/UA instead of Sapling ExtFVK/address
Browse files Browse the repository at this point in the history
This is a breaking change to the database format. We don't have support
for migrations yet, so existing wallets won't work after this commit
until #489 is done.
  • Loading branch information
str4d committed Jun 14, 2022
1 parent e86ba92 commit 0d0527d
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 57 deletions.
10 changes: 10 additions & 0 deletions zcash_client_backend/src/address.rs
Expand Up @@ -108,6 +108,11 @@ impl UnifiedAddress {
self.sapling.as_ref()
}

/// Returns the transparent receiver within this Unified Address, if any.
pub fn transparent(&self) -> Option<&TransparentAddress> {
self.transparent.as_ref()
}

fn to_address(&self, net: Network) -> ZcashAddress {
let ua = unified::Address::try_from_items(
self.unknown
Expand Down Expand Up @@ -137,6 +142,11 @@ impl UnifiedAddress {
.expect("UnifiedAddress should only be constructed safely");
ZcashAddress::from_unified(net, ua)
}

/// Returns the string encoding of this `UnifiedAddress` for the given network.
pub fn encode<P: consensus::Parameters>(&self, params: &P) -> String {
self.to_address(params_to_network(params)).to_string()
}
}

/// An address that funds can be sent to.
Expand Down
1 change: 1 addition & 0 deletions zcash_client_backend/src/data_api.rs
Expand Up @@ -115,6 +115,7 @@ pub trait WalletRead {
///
/// This will return `Ok(None)` if the account identifier does not correspond
/// to a known account.
// TODO: This does not appear to be the case.
fn get_address(&self, account: AccountId) -> Result<Option<PaymentAddress>, Self::Error>;

/// Returns all extended full viewing keys known about by this wallet.
Expand Down
15 changes: 10 additions & 5 deletions zcash_client_sqlite/CHANGELOG.md
Expand Up @@ -18,6 +18,16 @@ and this library adheres to Rust's notion of
rewinds exceed supported bounds.

### Changed
- Various **BREAKING CHANGES** have been made to the database tables. These will
require migrations, which may need to be performed in multiple steps.
- 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
Viewing Key should match the old value in the `extfvk` column.
- A new non-null column, `output_pool` has been added to the `sent_notes`
table to enable distinguishing between Sapling and transparent outputs
(and in the future, outputs to other pools). Values for this column should
be assigned by inference from the address type in the stored data.
- MSRV is now 1.56.1.
- Bumped dependencies to `ff 0.12`, `group 0.12`, `jubjub 0.9`.
- Renamed the following to use lower-case abbreviations (matching Rust
Expand All @@ -38,11 +48,6 @@ 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.
- A new non-null column, `output_pool` has been added to the `sent_notes`
table to enable distinguishing between Sapling and transparent outputs
(and in the future, outputs to other pools). This will require a migration,
which may need to be performed in multiple steps. Values for this column
should be assigned by inference from the address type in the stored data.

### Deprecated
- A number of public API methods that are used internally to support the
Expand Down
9 changes: 0 additions & 9 deletions zcash_client_sqlite/src/lib.rs
Expand Up @@ -53,7 +53,6 @@ use zcash_client_backend::{
data_api::{
BlockSource, DecryptedTransaction, PrunedBlock, SentTransaction, WalletRead, WalletWrite,
},
encoding::encode_payment_address,
proto::compact_formats::CompactBlock,
wallet::SpendableNote,
};
Expand Down Expand Up @@ -721,14 +720,6 @@ impl BlockSource for BlockDb {
}
}

fn address_from_extfvk<P: consensus::Parameters>(
params: &P,
extfvk: &ExtendedFullViewingKey,
) -> String {
let addr = extfvk.default_address().1;
encode_payment_address(params.hrp_sapling_payment_address(), &addr)
}

#[cfg(test)]
mod tests {
use ff::PrimeField;
Expand Down
63 changes: 37 additions & 26 deletions zcash_client_sqlite/src/wallet.rs
Expand Up @@ -23,11 +23,10 @@ use zcash_primitives::{
};

use zcash_client_backend::{
address::RecipientAddress,
data_api::error::Error,
encoding::{
decode_extended_full_viewing_key, decode_payment_address, encode_extended_full_viewing_key,
encode_payment_address_p, encode_transparent_address_p,
},
encoding::{encode_payment_address_p, encode_transparent_address_p},
keys::UnifiedFullViewingKey,
wallet::{WalletShieldedOutput, WalletTx},
DecryptedOutput,
};
Expand Down Expand Up @@ -162,8 +161,15 @@ pub fn get_address<P: consensus::Parameters>(
|row| row.get(0),
)?;

decode_payment_address(wdb.params.hrp_sapling_payment_address(), &addr)
.map_err(SqliteClientError::Bech32)
RecipientAddress::decode(&wdb.params, &addr)
.ok_or_else(|| {
SqliteClientError::CorruptedData("Not a valid Zcash recipient address".to_owned())
})
.map(|addr| match addr {
// TODO: Return the UA, not its Sapling component.
RecipientAddress::Unified(ua) => ua.sapling().cloned(),
_ => None,
})
}

/// Returns the [`ExtendedFullViewingKey`]s for the wallet.
Expand All @@ -178,21 +184,20 @@ pub fn get_extended_full_viewing_keys<P: consensus::Parameters>(
// Fetch the ExtendedFullViewingKeys we are tracking
let mut stmt_fetch_accounts = wdb
.conn
.prepare("SELECT account, extfvk FROM accounts ORDER BY account ASC")?;
.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 extfvk = row.get(1).map(|extfvk: String| {
decode_extended_full_viewing_key(
wdb.params.hrp_sapling_extended_full_viewing_key(),
&extfvk,
)
.map_err(SqliteClientError::Bech32)
.and_then(|k| k.ok_or(SqliteClientError::IncorrectHrpExtFvk))
})?;

Ok((AccountId::from(acct), extfvk))
let account = AccountId::from(acct);
let ufvk_str: String = row.get(1)?;
let ufvk = UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str, account)
.map_err(SqliteClientError::CorruptedData);
// TODO: Return the UFVK, not its Sapling component.
let extfvk =
ufvk.map(|ufvk| ufvk.sapling().cloned().expect("TODO: Add Orchard support"));

Ok((account, extfvk))
})
.map_err(SqliteClientError::from)?;

Expand All @@ -218,16 +223,22 @@ pub fn is_valid_account_extfvk<P: consensus::Parameters>(
extfvk: &ExtendedFullViewingKey,
) -> Result<bool, SqliteClientError> {
wdb.conn
.prepare("SELECT * FROM accounts WHERE account = ? AND extfvk = ?")?
.exists(&[
u32::from(account).to_sql()?,
encode_extended_full_viewing_key(
wdb.params.hrp_sapling_extended_full_viewing_key(),
extfvk,
)
.to_sql()?,
])
.prepare("SELECT ufvk FROM accounts WHERE account = ?")?
.query_row(&[u32::from(account).to_sql()?], |row| {
row.get(0).map(|ufvk_str: String| {
UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str, account)
.map_err(SqliteClientError::CorruptedData)
})
})
.optional()
.map_err(SqliteClientError::from)
.and_then(|row| {
if let Some(ufvk) = row {
ufvk.map(|ufvk| ufvk.sapling() == Some(extfvk))
} else {
Ok(false)
}
})
}

/// Returns the balance for the account, including all mined unspent notes that we know
Expand Down
26 changes: 9 additions & 17 deletions zcash_client_sqlite/src/wallet/init.rs
Expand Up @@ -7,11 +7,9 @@ use zcash_primitives::{
consensus::{self, BlockHeight},
};

use zcash_client_backend::{
encoding::encode_extended_full_viewing_key, keys::UnifiedFullViewingKey,
};
use zcash_client_backend::keys::UnifiedFullViewingKey;

use crate::{address_from_extfvk, error::SqliteClientError, WalletDb};
use crate::{error::SqliteClientError, WalletDb};

#[cfg(feature = "transparent-inputs")]
use {
Expand Down Expand Up @@ -44,10 +42,12 @@ use {
/// init_wallet_db(&db).unwrap();
/// ```
pub fn init_wallet_db<P>(wdb: &WalletDb<P>) -> 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,
extfvk TEXT,
ufvk TEXT,
address TEXT,
transparent_address TEXT
)",
Expand Down Expand Up @@ -200,16 +200,8 @@ pub fn init_accounts_table<P: consensus::Parameters>(
// Insert accounts atomically
wdb.conn.execute("BEGIN IMMEDIATE", NO_PARAMS)?;
for key in keys.iter() {
let extfvk_str: Option<String> = key.sapling().map(|extfvk| {
encode_extended_full_viewing_key(
wdb.params.hrp_sapling_extended_full_viewing_key(),
extfvk,
)
});

let address_str: Option<String> = key
.sapling()
.map(|extfvk| address_from_extfvk(&wdb.params, extfvk));
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()
Expand All @@ -220,11 +212,11 @@ pub fn init_accounts_table<P: consensus::Parameters>(
let taddress_str: Option<String> = None;

wdb.conn.execute(
"INSERT INTO accounts (account, extfvk, address, transparent_address)
"INSERT INTO accounts (account, ufvk, address, transparent_address)
VALUES (?, ?, ?, ?)",
params![
u32::from(key.account()),
extfvk_str,
ufvk_str,
address_str,
taddress_str,
],
Expand Down

0 comments on commit 0d0527d

Please sign in to comment.