Merge pull request #623 from zcash/489-addresses-table

`zcash_client_*`: Various changes to how addresses are handled

zcash_client_backend/CHANGELOG.md

@@ -32,6 +32,7 @@ and this library adheres to Rust's notion of
   - `RecipientAddress::Unified`
 - `zcash_client_backend::data_api`:
   - `WalletRead::get_unified_full_viewing_keys`
+  - `WalletRead::get_all_nullifiers`
   - `WalletWrite::remove_unmined_tx` (behind the `unstable` feature flag).
 - `zcash_client_backend::proto`:
   - `actions` field on `compact_formats::CompactTx`
@@ -78,13 +79,12 @@ and this library adheres to Rust's notion of
     a `min_confirmations` argument that is used to compute an upper bound on
     the anchor height being returned; this had previously been hardcoded to
     `data_api::wallet::ANCHOR_OFFSET`.
+  - `WalletRead::get_address` now returns a `UnifiedAddress` instead of a
+    `sapling::PaymentAddress`.
   - `WalletRead::get_spendable_notes` has been renamed to
     `get_spendable_sapling_notes`
   - `WalletRead::select_spendable_notes` has been renamed to
     `select_spendable_sapling_notes`
-  - `WalletRead::get_all_nullifiers` has been
-    added. This method provides access to all Sapling nullifiers, including
-    for notes that have been previously marked spent.
 - The `zcash_client_backend::data_api::SentTransaction` type has been
   substantially modified to accommodate handling of transparent inputs.
   Per-output data has been split out into a new struct `SentTransactionOutput`

zcash_client_backend/src/data_api.rs

@@ -4,18 +4,21 @@ use std::cmp;
 use std::collections::HashMap;
 use std::fmt::Debug;
 
+#[cfg(feature = "transparent-inputs")]
+use std::collections::HashSet;
+
 use zcash_primitives::{
     block::BlockHash,
     consensus::BlockHeight,
     memo::{Memo, MemoBytes},
     merkle_tree::{CommitmentTree, IncrementalWitness},
-    sapling::{Node, Nullifier, PaymentAddress},
+    sapling::{Node, Nullifier},
     transaction::{components::Amount, Transaction, TxId},
     zip32::{AccountId, ExtendedFullViewingKey},
 };
 
 use crate::{
-    address::RecipientAddress,
+    address::{RecipientAddress, UnifiedAddress},
     decrypt::DecryptedOutput,
     keys::UnifiedFullViewingKey,
     proto::compact_formats::CompactBlock,
@@ -111,13 +114,12 @@ pub trait WalletRead {
     /// or `Ok(None)` if the transaction is not mined in the main chain.
     fn get_tx_height(&self, txid: TxId) -> Result<Option<BlockHeight>, Self::Error>;
 
-    /// Returns the payment address for the specified account, if the account
+    /// Returns the unified address for the specified account, if the account
     /// identifier specified refers to a valid account for this wallet.
     ///
     /// 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<UnifiedAddress>, Self::Error>;
-    fn get_address(&self, account: AccountId) -> Result<Option<PaymentAddress>, Self::Error>;
 
     /// Returns all unified full viewing keys known to this wallet.
     fn get_unified_full_viewing_keys(
@@ -194,6 +196,16 @@ pub trait WalletRead {
 
 #[cfg(feature = "transparent-inputs")]
 pub trait WalletReadTransparent: WalletRead {
+    /// Returns the set of all transparent receivers associated with the given account.
+    ///
+    /// The set contains all transparent receivers that are known to have been derived
+    /// under this account. Wallets should scan the chain for UTXOs sent to these
+    /// receivers.
+    fn get_transparent_receivers(
+        &self,
+        account: AccountId,
+    ) -> Result<HashSet<TransparentAddress>, Self::Error>;
+
     /// Returns a list of unspent transparent UTXOs that appear in the chain at heights up to and
     /// including `max_height`.
     fn get_unspent_transparent_outputs(
@@ -327,18 +339,22 @@ pub trait BlockSource {
 pub mod testing {
     use std::collections::HashMap;
 
+    #[cfg(feature = "transparent-inputs")]
+    use std::collections::HashSet;
+
     use zcash_primitives::{
         block::BlockHash,
         consensus::BlockHeight,
         legacy::TransparentAddress,
         memo::Memo,
         merkle_tree::{CommitmentTree, IncrementalWitness},
-        sapling::{Node, Nullifier, PaymentAddress},
+        sapling::{Node, Nullifier},
         transaction::{components::Amount, Transaction, TxId},
         zip32::{AccountId, ExtendedFullViewingKey},
     };
 
     use crate::{
+        address::UnifiedAddress,
         keys::UnifiedFullViewingKey,
         proto::compact_formats::CompactBlock,
         wallet::{SpendableNote, WalletTransparentOutput},
@@ -392,7 +408,7 @@ pub mod testing {
             Ok(None)
         }
 
-        fn get_address(&self, _account: AccountId) -> Result<Option<PaymentAddress>, Self::Error> {
+        fn get_address(&self, _account: AccountId) -> Result<Option<UnifiedAddress>, Self::Error> {
             Ok(None)
         }
 
@@ -469,6 +485,13 @@ pub mod testing {
 
     #[cfg(feature = "transparent-inputs")]
     impl WalletReadTransparent for MockWalletDb {
+        fn get_transparent_receivers(
+            &self,
+            _account: AccountId,
+        ) -> Result<HashSet<TransparentAddress>, Self::Error> {
+            Ok(HashSet::new())
+        }
+
         fn get_unspent_transparent_outputs(
             &self,
             _address: &TransparentAddress,

zcash_client_sqlite/CHANGELOG.md

@@ -29,6 +29,12 @@ and this library adheres to Rust's notion of
     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.
+  - The `address` and `transparent_address` columns of the `accounts` table have
+    been removed.
+    - A new `addresses` table stores Unified Addresses, keyed on their `account`
+      and `diversifier_index`, to enable storing diversifed Unified Addresses.
+    - Transparent addresses for an account should be obtained by extracting the
+      transparent receiver of a Unified Address for the account.
   - 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
@@ -66,6 +72,11 @@ and this library adheres to Rust's notion of
     `zcash_client_backend::data_api::WalletRead::get_unified_full_viewing_keys`
     instead).
 
+### Fixed
+- The `zcash_client_backend::data_api::WalletRead::get_address` implementation
+  for `zcash_client_sqlite::WalletDb` now correctly returns `Ok(None)` if the
+  account identifier does not correspond to a known account.
+
 ### Deprecated
 - A number of public API methods that are used internally to support the
   `zcash_client_backend::data_api::{WalletRead, WalletWrite}` interfaces have

zcash_client_sqlite/Cargo.toml

@@ -18,6 +18,7 @@ bech32 = "0.8"
 bs58 = { version = "0.4", features = ["check"] }
 ff = "0.12"
 group = "0.12"
+hdwallet = { version = "0.3.1", optional = true }
 jubjub = "0.9"
 protobuf = "~2.27.1" # MSRV 1.52.1
 rand_core = "0.6"
@@ -38,7 +39,7 @@ zcash_proofs = { version = "0.7", path = "../zcash_proofs" }
 [features]
 mainnet = []
 test-dependencies = ["zcash_client_backend/test-dependencies"]
-transparent-inputs = ["zcash_client_backend/transparent-inputs"]
+transparent-inputs = ["hdwallet", "zcash_client_backend/transparent-inputs"]
 unstable = ["zcash_client_backend/unstable"]
 
 [lib]

zcash_client_sqlite/src/error.rs

@@ -43,6 +43,9 @@ pub enum SqliteClientError {
     /// Base58 decoding error
     Base58(bs58::decode::Error),
 
+    #[cfg(feature = "transparent-inputs")]
+    HdwalletError(hdwallet::error::Error),
+
     /// Base58 decoding error
     TransparentAddress(TransparentCodecError),
 
@@ -90,6 +93,8 @@ impl fmt::Display for SqliteClientError {
                 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::Bech32DecodeError(e) => write!(f, "{}", e),
             SqliteClientError::Base58(e) => write!(f, "{}", e),
+            #[cfg(feature = "transparent-inputs")]
+            SqliteClientError::HdwalletError(e) => write!(f, "{:?}", e),
             SqliteClientError::TransparentAddress(e) => write!(f, "{}", e),
             SqliteClientError::TableNotEmpty => write!(f, "Table is not empty"),
             #[cfg(feature = "unstable")]
@@ -126,6 +131,13 @@ impl From<bs58::decode::Error> for SqliteClientError {
     }
 }
 
+#[cfg(feature = "transparent-inputs")]
+impl From<hdwallet::error::Error> for SqliteClientError {
+    fn from(e: hdwallet::error::Error) -> Self {
+        SqliteClientError::HdwalletError(e)
+    }
+}
+
 impl From<zcash_primitives::memo::Error> for SqliteClientError {
     fn from(e: zcash_primitives::memo::Error) -> Self {
         SqliteClientError::InvalidMemo(e)

zcash_client_sqlite/src/lib.rs

@@ -36,6 +36,9 @@ use std::collections::HashMap;
 use std::fmt;
 use std::path::Path;
 
+#[cfg(feature = "transparent-inputs")]
+use std::collections::HashSet;
+
 use rusqlite::{Connection, NO_PARAMS};
 
 use zcash_primitives::{
@@ -43,13 +46,13 @@ use zcash_primitives::{
     consensus::{self, BlockHeight},
     memo::Memo,
     merkle_tree::{CommitmentTree, IncrementalWitness},
-    sapling::{Node, Nullifier, PaymentAddress},
+    sapling::{Node, Nullifier},
     transaction::{components::Amount, Transaction, TxId},
     zip32::{AccountId, ExtendedFullViewingKey},
 };
 
 use zcash_client_backend::{
-    address::RecipientAddress,
+    address::{RecipientAddress, UnifiedAddress},
     data_api::{
         BlockSource, DecryptedTransaction, PrunedBlock, SentTransaction, WalletRead, WalletWrite,
     },
@@ -150,9 +153,8 @@ impl<P: consensus::Parameters> WalletRead for WalletDb<P> {
         wallet::get_unified_full_viewing_keys(self)
     }
 
-    fn get_address(&self, account: AccountId) -> Result<Option<PaymentAddress>, Self::Error> {
+    fn get_address(&self, account: AccountId) -> Result<Option<UnifiedAddress>, Self::Error> {
-        #[allow(deprecated)]
+        wallet::get_address_ua(self, account)
-        wallet::get_address(self, account)
     }
 
     fn is_valid_account_extfvk(
@@ -235,6 +237,13 @@ impl<P: consensus::Parameters> WalletRead for WalletDb<P> {
 
 #[cfg(feature = "transparent-inputs")]
 impl<P: consensus::Parameters> WalletReadTransparent for WalletDb<P> {
+    fn get_transparent_receivers(
+        &self,
+        account: AccountId,
+    ) -> Result<HashSet<TransparentAddress>, Self::Error> {
+        wallet::get_transparent_receivers(self, account)
+    }
+
     fn get_unspent_transparent_outputs(
         &self,
         address: &TransparentAddress,
@@ -267,7 +276,7 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> {
         self.wallet_db.get_unified_full_viewing_keys()
     }
 
-    fn get_address(&self, account: AccountId) -> Result<Option<PaymentAddress>, Self::Error> {
+    fn get_address(&self, account: AccountId) -> Result<Option<UnifiedAddress>, Self::Error> {
         self.wallet_db.get_address(account)
     }
 
@@ -340,6 +349,13 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> {
 
 #[cfg(feature = "transparent-inputs")]
 impl<'a, P: consensus::Parameters> WalletReadTransparent for DataConnStmtCache<'a, P> {
+    fn get_transparent_receivers(
+        &self,
+        account: AccountId,
+    ) -> Result<HashSet<TransparentAddress>, Self::Error> {
+        self.wallet_db.get_transparent_receivers(account)
+    }
+
     fn get_unspent_transparent_outputs(
         &self,
         address: &TransparentAddress,
@@ -725,6 +741,14 @@ mod tests {
     pub(crate) fn init_test_accounts_table(
         db_data: &WalletDb<Network>,
     ) -> (DiversifiableFullViewingKey, Option<TransparentAddress>) {
+        let (ufvk, taddr) = init_test_accounts_table_ufvk(db_data);
+        (ufvk.sapling().unwrap().clone(), taddr)
+    }
+
+    #[cfg(test)]
+    pub(crate) fn init_test_accounts_table_ufvk(
+        db_data: &WalletDb<Network>,
+    ) -> (UnifiedFullViewingKey, Option<TransparentAddress>) {
         let seed = [0u8; 32];
         let account = AccountId::from(0);
         let extsk = sapling::spending_key(&seed, network().coin_type(), account);
@@ -745,15 +769,15 @@ mod tests {
         let ufvk = UnifiedFullViewingKey::new(
             #[cfg(feature = "transparent-inputs")]
             tkey,
-            Some(dfvk.clone()),
+            Some(dfvk),
             None,
         )
         .unwrap();
 
-        let ufvks = HashMap::from([(account, ufvk)]);
+        let ufvks = HashMap::from([(account, ufvk.clone())]);
         init_accounts_table(db_data, &ufvks).unwrap();
 
-        (dfvk, taddr)
+        (ufvk, taddr)
     }
 
     /// Create a fake CompactBlock at the given height, containing a single output paying
@@ -901,6 +925,36 @@ mod tests {
             .unwrap();
     }
 
+    #[cfg(feature = "transparent-inputs")]
+    #[test]
+    fn transparent_receivers() {
+        use secrecy::Secret;
+        use tempfile::NamedTempFile;
+        use zcash_client_backend::data_api::WalletReadTransparent;
+
+        use crate::{chain::init::init_cache_database, wallet::init::init_wallet_db};
+
+        let cache_file = NamedTempFile::new().unwrap();
+        let db_cache = BlockDb::for_path(cache_file.path()).unwrap();
+        init_cache_database(&db_cache).unwrap();
+
+        let data_file = NamedTempFile::new().unwrap();
+        let mut db_data = WalletDb::for_path(data_file.path(), network()).unwrap();
+        init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap();
+
+        // Add an account to the wallet.
+        let (ufvk, taddr) = init_test_accounts_table_ufvk(&db_data);
+        let taddr = taddr.unwrap();
+
+        let receivers = db_data.get_transparent_receivers(0.into()).unwrap();
+
+        // The receiver for the default UA should be in the set.
+        assert!(receivers.contains(ufvk.default_address().0.transparent().unwrap()));
+
+        // The default t-addr should be in the set.
+        assert!(receivers.contains(&taddr));
+    }
+
     #[cfg(feature = "unstable")]
     #[test]
     fn remove_unmined_tx_reverts_balance() {

zcash_client_sqlite/src/wallet.rs

@@ -12,6 +12,9 @@ use rusqlite::{OptionalExtension, ToSql, NO_PARAMS};
 use std::collections::HashMap;
 use std::convert::TryFrom;
 
+#[cfg(feature = "transparent-inputs")]
+use std::collections::HashSet;
+
 use zcash_primitives::{
     block::BlockHash,
     consensus::{self, BlockHeight, BranchId, NetworkUpgrade, Parameters},
@@ -23,7 +26,7 @@ use zcash_primitives::{
 };
 
 use zcash_client_backend::{
-    address::RecipientAddress,
+    address::{RecipientAddress, UnifiedAddress},
     data_api::error::Error,
     encoding::{encode_payment_address_p, encode_transparent_address_p},
     keys::UnifiedFullViewingKey,
@@ -41,7 +44,7 @@ use {
     rusqlite::params,
     zcash_client_backend::{encoding::AddressCodec, wallet::WalletTransparentOutput},
     zcash_primitives::{
-        legacy::Script,
+        legacy::{keys::IncomingViewingKey, Script},
         transaction::components::{OutPoint, TxOut},
     },
 };
@@ -155,8 +158,9 @@ pub fn get_address<P: consensus::Parameters>(
     wdb: &WalletDb<P>,
     account: AccountId,
 ) -> Result<Option<PaymentAddress>, SqliteClientError> {
+    // This returns the first diversified address, which will be the default one.
     let addr: String = wdb.conn.query_row(
-        "SELECT address FROM accounts
+        "SELECT address FROM addresses
         WHERE account = ?",
         &[u32::from(account)],
         |row| row.get(0),
@@ -173,6 +177,86 @@ pub fn get_address<P: consensus::Parameters>(
         })
 }
 
+pub(crate) fn get_address_ua<P: consensus::Parameters>(
+    wdb: &WalletDb<P>,
+    account: AccountId,
+) -> Result<Option<UnifiedAddress>, SqliteClientError> {
+    // This returns the first diversified address, which will be the default one.
+    let addr: Option<String> = wdb
+        .conn
+        .query_row_named(
+            "SELECT address FROM addresses WHERE account = :account",
+            &[(":account", &u32::from(account))],
+            |row| row.get(0),
+        )
+        .optional()?;
+
+    addr.map(|addr_str| {
+        RecipientAddress::decode(&wdb.params, &addr_str)
+            .ok_or_else(|| {
+                SqliteClientError::CorruptedData("Not a valid Zcash recipient address".to_owned())
+            })
+            .and_then(|addr| match addr {
+                RecipientAddress::Unified(ua) => Ok(ua),
+                _ => Err(SqliteClientError::CorruptedData(format!(
+                    "Addresses table contains {} which is not a unified address",
+                    addr_str,
+                ))),
+            })
+    })
+    .transpose()
+}
+
+#[cfg(feature = "transparent-inputs")]
+pub(crate) fn get_transparent_receivers<P: consensus::Parameters>(
+    wdb: &WalletDb<P>,
+    account: AccountId,
+) -> Result<HashSet<TransparentAddress>, SqliteClientError> {
+    let mut ret = HashSet::new();
+
+    // Get all UAs derived
+    let mut ua_query = wdb
+        .conn
+        .prepare("SELECT address FROM addresses WHERE account = :account")?;
+    let mut rows = ua_query.query_named(&[(":account", &u32::from(account))])?;
+
+    while let Some(row) = rows.next()? {
+        let ua_str: String = row.get(0)?;
+        let ua = RecipientAddress::decode(&wdb.params, &ua_str)
+            .ok_or_else(|| {
+                SqliteClientError::CorruptedData("Not a valid Zcash recipient address".to_owned())
+            })
+            .and_then(|addr| match addr {
+                RecipientAddress::Unified(ua) => Ok(ua),
+                _ => Err(SqliteClientError::CorruptedData(format!(
+                    "Addresses table contains {} which is not a unified address",
+                    ua_str,
+                ))),
+            })?;
+        if let Some(taddr) = ua.transparent() {
+            ret.insert(*taddr);
+        }
+    }
+
+    // Get the UFVK for the account.
+    let ufvk_str: String = wdb.conn.query_row(
+        "SELECT ufvk FROM accounts WHERE account = :account",
+        &[u32::from(account)],
+        |row| row.get(0),
+    )?;
+    let ufvk = UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str)
+        .map_err(SqliteClientError::CorruptedData)?;
+
+    // Derive the default transparent address (if it wasn't already part of a derived UA).
+    if let Some(tfvk) = ufvk.transparent() {
+        let tivk = tfvk.derive_external_ivk()?;
+        let taddr = tivk.default_address().0;
+        ret.insert(taddr);
+    }
+
+    Ok(ret)
+}
+
 /// Returns the [`UnifiedFullViewingKey`]s for the wallet.
 pub(crate) fn get_unified_full_viewing_keys<P: consensus::Parameters>(
     wdb: &WalletDb<P>,

zcash_client_sqlite/src/wallet/init.rs

@@ -26,6 +26,8 @@ use {
     zcash_primitives::legacy::keys::IncomingViewingKey,
 };
 
+mod migrations;
+
 #[derive(Debug)]
 pub enum WalletMigrationError {
     /// The seed is required for the migration.
@@ -289,12 +291,34 @@ impl<P: consensus::Parameters> RusqliteMigration for WalletMigration2<P> {
                     }
                 }
 
-                add_account_internal::<P, WalletMigrationError>(
+                let ufvk_str: String = ufvk.encode(&self.params);
-                    &self.params,
+                let address_str: String = ufvk.default_address().0.encode(&self.params);
-                    transaction,
+
-                    "accounts_new",
+                // This migration, and the wallet behaviour before it, stored the default
-                    account,
+                // transparent address in the `accounts` table. This does not necessarily
-                    &ufvk,
+                // match the transparent receiver in the default Unified Address. Starting
+                // from `AddressesTableMigration` below, we no longer store transparent
+                // addresses directly, but instead extract them from the Unified Address
+                // (or from the UFVK if the UA was derived without a transparent receiver,
+                // which is not the case for UAs generated by this crate).
+                #[cfg(feature = "transparent-inputs")]
+                let taddress_str: Option<String> = ufvk.transparent().and_then(|k| {
+                    k.derive_external_ivk()
+                        .ok()
+                        .map(|k| k.default_address().0.encode(&self.params))
+                });
+                #[cfg(not(feature = "transparent-inputs"))]
+                let taddress_str: Option<String> = None;
+
+                transaction.execute_named(
+                    "INSERT INTO accounts_new (account, ufvk, address, transparent_address)
+                    VALUES (:account, :ufvk, :address, :transparent_address)",
+                    &[
+                        (":account", &<u32>::from(account)),
+                        (":ufvk", &ufvk_str),
+                        (":address", &address_str),
+                        (":transparent_address", &taddress_str),
+                    ],
                 )?;
             } else {
                 return Err(WalletMigrationError::SeedRequired);
@@ -463,9 +487,12 @@ pub fn init_wallet_db<P: consensus::Parameters + 'static>(
         params: wdb.params.clone(),
         seed,
     });
+    let addrs_migration = Box::new(migrations::AddressesTableMigration {
+        params: wdb.params.clone(),
+    });
 
     migrator
-        .register_multiple(vec![migration0, migration1, migration2])
+        .register_multiple(vec![migration0, migration1, migration2, addrs_migration])
         .expect("Wallet migration registration should have been successful.");
     migrator.up(None)?;
     wdb.conn
@@ -557,23 +584,25 @@ fn add_account_internal<P: consensus::Parameters, E: From<rusqlite::Error>>(
     key: &UnifiedFullViewingKey,
 ) -> Result<(), E> {
     let ufvk_str: String = key.encode(network);
-    let address_str: String = key.default_address().0.encode(network);
+    conn.execute_named(
-    #[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)
+            "INSERT INTO {} (account, ufvk) VALUES (:account, :ufvk)",
-            VALUES (?, ?, ?, ?)",
             accounts_table
         ),
-        params![<u32>::from(account), ufvk_str, address_str, taddress_str],
+        &[(":account", &<u32>::from(account)), (":ufvk", &ufvk_str)],
+    )?;
+
+    // Always derive the default Unified Address for the account.
+    let (address, idx) = key.default_address();
+    let address_str: String = address.encode(network);
+    conn.execute_named(
+        "INSERT INTO addresses (account, diversifier_index, address)
+        VALUES (:account, :diversifier_index, :address)",
+        &[
+            (":account", &<u32>::from(account)),
+            (":diversifier_index", &&idx.0[..]),
+            (":address", &address_str),
+        ],
     )?;
 
     Ok(())
@@ -683,9 +712,14 @@ mod tests {
         let expected = vec![
             "CREATE TABLE \"accounts\" (
                 account INTEGER PRIMARY KEY,
-                ufvk TEXT NOT NULL,
+                ufvk TEXT NOT NULL
-                address TEXT,
+            )",
-                transparent_address TEXT
+            "CREATE TABLE addresses (
+                account INTEGER NOT NULL,
+                diversifier_index BLOB NOT NULL,
+                address TEXT NOT NULL,
+                FOREIGN KEY (account) REFERENCES accounts(account),
+                CONSTRAINT diversification UNIQUE (account, diversifier_index)
             )",
             "CREATE TABLE blocks (
                 height INTEGER PRIMARY KEY,
@@ -1148,10 +1182,9 @@ mod tests {
             // add a transparent "sent note"
             #[cfg(feature = "transparent-inputs")]
             {
-                let taddr = RecipientAddress::Transparent(
+                let taddr =
-                    ufvk.default_address().0.transparent().unwrap().clone(),
+                    RecipientAddress::Transparent(*ufvk.default_address().0.transparent().unwrap())
-                )
+                        .encode(&tests::network());
-                .encode(&tests::network());
                 wdb.conn.execute(
                     "INSERT INTO blocks (height, hash, time, sapling_tree) VALUES (0, 0, 0, '')",
                     NO_PARAMS,

zcash_client_sqlite/src/wallet/init/migrations.rs

@@ -0,0 +1,2 @@
+mod addresses_table;
+pub(super) use addresses_table::AddressesTableMigration;

zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs

@@ -0,0 +1,176 @@
+use std::collections::HashSet;
+
+use rusqlite::{Transaction, NO_PARAMS};
+use schemer::Migration;
+use schemer_rusqlite::RusqliteMigration;
+use uuid::Uuid;
+use zcash_client_backend::{address::RecipientAddress, keys::UnifiedFullViewingKey};
+use zcash_primitives::{consensus, zip32::AccountId};
+
+use super::super::{add_account_internal, WalletMigrationError};
+
+#[cfg(feature = "transparent-inputs")]
+use zcash_primitives::legacy::keys::IncomingViewingKey;
+
+/// The migration that removed the address columns from the `accounts` table, and created
+/// the `accounts` table.
+///
+/// d956978c-9c87-4d6e-815d-fb8f088d094c
+pub(super) const ADDRESSES_TABLE_MIGRATION: Uuid = Uuid::from_fields(
+    0xd956978c,
+    0x9c87,
+    0x4d6e,
+    b"\x81\x5d\xfb\x8f\x08\x8d\x09\x4c",
+);
+
+pub(crate) struct AddressesTableMigration<P: consensus::Parameters> {
+    pub(crate) params: P,
+}
+
+impl<P: consensus::Parameters> Migration for AddressesTableMigration<P> {
+    fn id(&self) -> Uuid {
+        ADDRESSES_TABLE_MIGRATION
+    }
+
+    fn dependencies(&self) -> HashSet<Uuid> {
+        ["be57ef3b-388e-42ea-97e2-678dafcf9754"]
+            .iter()
+            .map(|uuidstr| ::uuid::Uuid::parse_str(uuidstr).unwrap())
+            .collect()
+    }
+
+    fn description(&self) -> &'static str {
+        "Adds the addresses table for tracking diversified UAs"
+    }
+}
+
+impl<P: consensus::Parameters> RusqliteMigration for AddressesTableMigration<P> {
+    type Error = WalletMigrationError;
+
+    fn up(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> {
+        transaction.execute_batch(
+            "CREATE TABLE addresses (
+                account INTEGER NOT NULL,
+                diversifier_index BLOB NOT NULL,
+                address TEXT NOT NULL,
+                FOREIGN KEY (account) REFERENCES accounts(account),
+                CONSTRAINT diversification UNIQUE (account, diversifier_index)
+            );
+            CREATE TABLE accounts_new (
+                account INTEGER PRIMARY KEY,
+                ufvk TEXT NOT NULL
+            );",
+        )?;
+
+        let mut stmt_fetch_accounts = transaction
+            .prepare("SELECT account, ufvk, address, transparent_address FROM accounts")?;
+
+        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 ufvk_str: String = row.get(1)?;
+            let ufvk = UnifiedFullViewingKey::decode(&self.params, &ufvk_str)
+                .map_err(WalletMigrationError::CorruptedData)?;
+
+            // Verify that the address column contains the expected value.
+            let address: String = row.get(2)?;
+            let decoded = RecipientAddress::decode(&self.params, &address).ok_or_else(|| {
+                WalletMigrationError::CorruptedData(format!(
+                    "Could not decode {} as a valid Zcash address.",
+                    address
+                ))
+            })?;
+            let decoded_address = if let RecipientAddress::Unified(ua) = decoded {
+                ua
+            } else {
+                return Err(WalletMigrationError::CorruptedData(
+                    "Address in accounts table was not a Unified Address.".to_string(),
+                ));
+            };
+            let (expected_address, idx) = ufvk.default_address();
+            if decoded_address != expected_address {
+                return Err(WalletMigrationError::CorruptedData(format!(
+                    "Decoded UA {} does not match the UFVK's default address {} at {:?}.",
+                    address,
+                    RecipientAddress::Unified(expected_address).encode(&self.params),
+                    idx,
+                )));
+            }
+
+            // The transparent_address column might not be filled, depending on how this
+            // crate was compiled.
+            if let Some(transparent_address) = row.get::<_, Option<String>>(3)? {
+                let decoded_transparent =
+                    RecipientAddress::decode(&self.params, &transparent_address).ok_or_else(
+                        || {
+                            WalletMigrationError::CorruptedData(format!(
+                                "Could not decode {} as a valid Zcash address.",
+                                address
+                            ))
+                        },
+                    )?;
+                let decoded_transparent_address = if let RecipientAddress::Transparent(addr) =
+                    decoded_transparent
+                {
+                    addr
+                } else {
+                    return Err(WalletMigrationError::CorruptedData(
+                        "Address in transparent_address column of accounts table was not a transparent address.".to_string(),
+                    ));
+                };
+
+                // Verify that the transparent_address column contains the expected value,
+                // so we can confidently delete the column knowing we can regenerate the
+                // values from the stored UFVKs.
+
+                // We can only check if it is the expected transparent address if the
+                // transparent-inputs feature flag is enabled.
+                #[cfg(feature = "transparent-inputs")]
+                {
+                    let expected_address = ufvk
+                        .transparent()
+                        .and_then(|k| k.derive_external_ivk().ok().map(|k| k.default_address().0));
+                    if Some(decoded_transparent_address) != expected_address {
+                        return Err(WalletMigrationError::CorruptedData(format!(
+                            "Decoded transparent address {} is not the default transparent address.",
+                            transparent_address,
+                        )));
+                    }
+                }
+
+                // If the transparent_address column is not empty, and we can't check its
+                // value, return an error.
+                #[cfg(not(feature = "transparent-inputs"))]
+                {
+                    let _ = decoded_transparent_address;
+                    return Err(WalletMigrationError::CorruptedData(
+                        "Database needs transparent-inputs feature flag enabled to migrate"
+                            .to_string(),
+                    ));
+                }
+            }
+
+            add_account_internal::<P, WalletMigrationError>(
+                &self.params,
+                transaction,
+                "accounts_new",
+                account,
+                &ufvk,
+            )?;
+        }
+
+        transaction.execute_batch(
+            "DROP TABLE accounts;
+            ALTER TABLE accounts_new RENAME TO accounts;",
+        )?;
+
+        Ok(())
+    }
+
+    fn down(&self, _transaction: &Transaction) -> Result<(), WalletMigrationError> {
+        // TODO: something better than just panic?
+        panic!("Cannot revert this migration.");
+    }
+}

zcash_primitives/CHANGELOG.md

@@ -7,7 +7,9 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 ### Added
-- `zcash_primitives::legacy::AccountPrivKey::{to_bytes, from_bytes}`
+- `zcash_primitives::legacy`:
+  - `impl {Copy, Eq, Ord} for TransparentAddress`
+  - `keys::AccountPrivKey::{to_bytes, from_bytes}`
 - `zcash_primitives::sapling::NullifierDerivingKey`
 - Added in `zcash_primitives::sapling::keys`
   - `DecodingError`

zcash_primitives/src/legacy.rs

@@ -95,7 +95,7 @@ impl Shl<&[u8]> for Script {
 }
 
 /// A transparent address corresponding to either a public key or a `Script`.
-#[derive(Debug, PartialEq, PartialOrd, Hash, Clone)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum TransparentAddress {
     PublicKey([u8; 20]), // TODO: Rename to PublicKeyHash
     Script([u8; 20]),    // TODO: Rename to ScriptHash