diff --git a/CHANGELOG.md b/CHANGELOG.md index 31ccd87c..aec8bc9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ be considered breaking changes. and `addresses` for transparent outputs. - `z_viewtransaction`: The `outgoing` field is now omitted on outputs that `zcashd` didn't include in its response. +- Significant performance improvements to `zallet migrate-zcashd-wallet`. ### Fixed - No longer crashes in regtest mode when a Sapling or NU5 activation height is diff --git a/zallet/src/commands/migrate_zcashd_wallet.rs b/zallet/src/commands/migrate_zcashd_wallet.rs index e77d1c70..83dd9ece 100644 --- a/zallet/src/commands/migrate_zcashd_wallet.rs +++ b/zallet/src/commands/migrate_zcashd_wallet.rs @@ -11,20 +11,19 @@ use bip0039::{English, Mnemonic}; use secp256k1::PublicKey; use secrecy::{ExposeSecret, SecretVec}; use shardtree::error::ShardTreeError; -use transparent::address::TransparentAddress; use zaino_fetch::jsonrpsee::response::block_header::GetBlockHeader; use zaino_state::{FetchServiceError, LightWalletIndexer, ZcashIndexer}; -use zcash_client_backend::data_api::{ - Account as _, AccountBirthday, AccountPurpose, AccountSource, WalletRead, WalletWrite as _, - Zip32Derivation, wallet::decrypt_and_store_transaction, +use zcash_client_backend::{ + data_api::{ + Account as _, AccountBirthday, AccountPurpose, AccountSource, WalletRead, WalletWrite as _, + Zip32Derivation, + }, + decrypt_transaction, }; use zcash_client_sqlite::error::SqliteClientError; -use zcash_keys::{ - encoding::AddressCodec, - keys::{ - DerivationError, UnifiedFullViewingKey, - zcashd::{PathParseError, ZcashdHdDerivation}, - }, +use zcash_keys::keys::{ + DerivationError, UnifiedFullViewingKey, + zcashd::{PathParseError, ZcashdHdDerivation}, }; use zcash_primitives::{block::BlockHash, transaction::Transaction}; use zcash_protocol::consensus::{BlockHeight, BranchId, NetworkType, Parameters}; @@ -558,33 +557,39 @@ impl MigrateZcashdWalletCmd { Ok(key) } - info!("Importing legacy standalone transparent keys"); // TODO: Expose how many there are in zewif-zcashd. - for (i, key) in wallet.keys().keypairs().enumerate() { - if i % 100 == 0 && i > 0 { - info!("Processed {} standalone transparent keys", i); - } - let key = convert_key(key)?; - let pubkey = key.pubkey(); - debug!( - "[{i}] Importing key for address {}", - TransparentAddress::from_pubkey(&pubkey).encode(&network_params), - ); - - keystore - .encrypt_and_store_standalone_transparent_key(&key) - .await?; - - db_data.import_standalone_transparent_pubkey( - legacy_transparent_account_uuid.ok_or(MigrateError::SeedNotAvailable)?, - pubkey, - )?; - } + let encryptor = keystore.encryptor().await?; + let transparent_keypairs = wallet.keys().keypairs().collect::>(); + info!( + "Importing {} legacy standalone transparent keys", + transparent_keypairs.len(), + ); // TODO: Expose how many there are in zewif-zcashd. + let encrypted_transparent_keys = transparent_keypairs + .into_iter() + .map(|key| { + let key = convert_key(key)?; + encryptor.encrypt_standalone_transparent_key(&key) + }) + .collect::, _>>()?; + keystore + .store_encrypted_standalone_transparent_keys(&encrypted_transparent_keys) + .await?; + db_data.import_standalone_transparent_pubkeys( + legacy_transparent_account_uuid.ok_or(MigrateError::SeedNotAvailable)?, + encrypted_transparent_keys + .into_iter() + .map(|key| *key.pubkey()), + )?; // Since we've retrieved the raw transaction data anyway, preemptively store it for faster // access to balance & to set priorities in the scan queue. if buffer_wallet_transactions { info!("Importing transactions"); + // Fetch the UnifiedFullViewingKeys we are tracking + let ufvks = db_data.get_unified_full_viewing_keys()?; + + let chain_tip_height = db_data.chain_height()?; + // Assume that the zcashd wallet was shut down immediately after its last // transaction was mined. This will be accurate except in the following case: // - User mines a transaction in any older epoch. @@ -598,41 +603,62 @@ impl MigrateZcashdWalletCmd { .map(|h| *h + 1); let mut buf = vec![]; - for (i, (h, wallet_tx)) in tx_heights.values().enumerate() { - if i % 100 == 0 && i > 0 { - info!("Processed {} transactions", i); - } - if let Some(wallet_tx) = wallet_tx { - let consensus_height = match h { - Some(h) => *h, - None => { - let expiry_height = u32::from(wallet_tx.transaction().expiry_height()); - if expiry_height == 0 { - // Transaction is unmined and unexpired, use fallback. - assumed_mempool_height.ok_or_else(|| { - ErrorKind::Generic - .context(fl!("err-migrate-wallet-all-unmined")) - })? - } else { - // A transaction's expiry height is always in same epoch - // as its eventual mined height. - BlockHeight::from_u32(expiry_height) + let decoded_txs = tx_heights + .values() + .flat_map(|(h, wallet_tx)| { + wallet_tx.map(|wallet_tx| { + let consensus_height = match h { + Some(h) => *h, + None => { + let expiry_height = + u32::from(wallet_tx.transaction().expiry_height()); + if expiry_height == 0 { + // Transaction is unmined and unexpired, use fallback. + assumed_mempool_height.ok_or_else(|| { + ErrorKind::Generic + .context(fl!("err-migrate-wallet-all-unmined")) + })? + } else { + // A transaction's expiry height is always in same epoch + // as its eventual mined height. + BlockHeight::from_u32(expiry_height) + } } - } - }; - let consensus_branch_id = - BranchId::for_height(&network_params, consensus_height); - // TODO: Use the same zcash_primitives version in zewif-zcashd - let tx = { - buf.clear(); - wallet_tx.transaction().write(&mut buf)?; - Transaction::read(buf.as_slice(), consensus_branch_id)? - }; - db_data.with_mut(|mut db| { - decrypt_and_store_transaction(&network_params, &mut db, &tx, *h) - })?; - } - } + }; + let consensus_branch_id = + BranchId::for_height(&network_params, consensus_height); + // TODO: Use the same zcash_primitives version in zewif-zcashd + let tx = { + buf.clear(); + wallet_tx.transaction().write(&mut buf)?; + Transaction::read(buf.as_slice(), consensus_branch_id)? + }; + Ok((tx, *h)) + }) + }) + .collect::, MigrateError>>()?; + + info!("Decrypting {} transactions", decoded_txs.len()); + let decrypted_txs = decoded_txs + .iter() + .enumerate() + .map(|(i, (tx, mined_height))| { + if i % 1000 == 0 && i > 0 { + tracing::info!("Decrypted {i}/{} transactions", decoded_txs.len()); + } + decrypt_transaction( + &network_params, + *mined_height, + chain_tip_height, + tx, + &ufvks, + ) + }) + .collect::>(); + + info!("Storing {} decrypted transactions", decrypted_txs.len()); + // TODO: Use chunking here if we add support for resuming migrations. + db_data.store_decrypted_txs(decrypted_txs)?; } else { info!("Not importing transactions (--buffer-wallet-transactions not set)"); } diff --git a/zallet/src/components/database/connection.rs b/zallet/src/components/database/connection.rs index 5ef1fe64..7afc80e1 100644 --- a/zallet/src/components/database/connection.rs +++ b/zallet/src/components/database/connection.rs @@ -25,6 +25,9 @@ use zcash_primitives::{block::BlockHash, transaction::Transaction}; use zcash_protocol::{ShieldedProtocol, consensus::BlockHeight}; use zip32::DiversifierIndex; +#[cfg(feature = "zcashd-import")] +use zcash_client_sqlite::AccountUuid; + use crate::{ error::{Error, ErrorKind}, network::Network, @@ -146,6 +149,70 @@ impl DbConnection { f(self.inner.lock().unwrap().as_mut(), &self.params) }) } + + /// Imports the given pubkeys into the account without key derivation information, and + /// adds the associated transparent p2pkh addresses. + /// + /// This creates a single database transaction for all inserts, significantly reducing + /// overhead for large migrated wallets. + /// + /// The imported addresses will contribute to the balance of the account (for + /// UFVK-based accounts), but spending funds held by these addresses require the + /// associated spending keys to be provided explicitly when calling + /// [`create_proposed_transactions`]. By extension, calls to [`propose_shielding`] + /// must only include addresses for which the spending application holds or can obtain + /// the spending keys. + /// + /// [`create_proposed_transactions`]: zcash_client_sqlite::data_api::wallet::create_proposed_transactions + /// [`propose_shielding`]: zcash_client_sqlite::data_api::wallet::propose_shielding + #[cfg(feature = "zcashd-import")] + pub(crate) fn import_standalone_transparent_pubkeys( + &mut self, + account: AccountUuid, + pubkeys: impl ExactSizeIterator, + ) -> Result< + (), + as WalletRead>::Error, + > { + self.with_mut(|mut db_data| { + db_data.transactionally(|wdb| { + let total = pubkeys.len(); + for (i, pubkey) in pubkeys.into_iter().enumerate() { + if i % 10000 == 0 && i > 0 { + tracing::info!("Stored {i}/{total} pubkeys"); + } + wdb.import_standalone_transparent_pubkey(account, pubkey)?; + } + Ok(()) + }) + }) + } + + /// Caches decrypted transactions in the persistent wallet store. + /// + /// This creates a single database transaction for all inserts, significantly reducing + /// overhead for large migrated wallets. + #[cfg(feature = "zcashd-import")] + pub(crate) fn store_decrypted_txs( + &mut self, + received_txs: Vec>, + ) -> Result< + (), + as WalletRead>::Error, + > { + self.with_mut(|mut db_data| { + db_data.transactionally(|wdb| { + let total = received_txs.len(); + for (i, received_tx) in received_txs.into_iter().enumerate() { + if i % 100 == 0 && i > 0 { + tracing::info!("Stored {i}/{total} transactions"); + } + wdb.store_decrypted_tx(received_tx)?; + } + Ok(()) + }) + }) + } } impl WalletRead for DbConnection { diff --git a/zallet/src/components/keystore.rs b/zallet/src/components/keystore.rs index ecee0060..bc1be596 100644 --- a/zallet/src/components/keystore.rs +++ b/zallet/src/components/keystore.rs @@ -408,7 +408,7 @@ impl KeyStore { ) -> Result<(), Error> { // If the wallet has any existing recipients, fail (we would instead need to // re-encrypt the wallet). - if !self.maybe_recipients().await?.is_empty() { + if !self.maybe_encryptor().await?.is_empty() { return Err(ErrorKind::Generic .context(fl!("err-keystore-already-initialized")) .into()); @@ -439,24 +439,24 @@ impl KeyStore { Ok(()) } - /// Fetches the age recipients for this wallet from the database. + /// Constructs the encryptor for this wallet using the recipients from the database. /// - /// Returns an error if there are none. - async fn recipients(&self) -> Result>, Error> { - let recipients = self.maybe_recipients().await?; - if recipients.is_empty() { + /// Returns an error if there are no age recipients. + pub(crate) async fn encryptor(&self) -> Result { + let encryptor = self.maybe_encryptor().await?; + if encryptor.is_empty() { Err(ErrorKind::Generic .context(KeystoreError::MissingRecipients) .into()) } else { - Ok(recipients) + Ok(encryptor) } } - /// Fetches the age recipients for this wallet from the database. + /// Constructs the encryptor for this wallet using the recipients from the database. /// - /// Unlike [`Self::recipients`], this might return an empty vec. - async fn maybe_recipients(&self) -> Result>, Error> { + /// Unlike [`Self::encryptor`], this might return an empty encryptor. + async fn maybe_encryptor(&self) -> Result { self.with_db(|conn, _| { let mut stmt = conn .prepare( @@ -472,18 +472,7 @@ impl KeyStore { .collect::>() .map_err(|e| ErrorKind::Generic.context(e))?; - // TODO: Replace with a helper with configurable callbacks. - let mut stdin_guard = age::cli_common::StdinGuard::new(false); - let recipients = age::cli_common::read_recipients( - recipient_strings, - vec![], - vec![], - None, - &mut stdin_guard, - ) - .map_err(|e| ErrorKind::Generic.context(e))?; - - Ok(recipients) + Encryptor::from_recipient_strings(recipient_strings) }) .await } @@ -536,19 +525,15 @@ impl KeyStore { &self, mnemonic: Mnemonic, ) -> Result { - let recipients = self.recipients().await?; + let encryptor = self.encryptor().await?; let seed_bytes = SecretVec::new(mnemonic.to_seed("").to_vec()); let seed_fp = SeedFingerprint::from_seed(seed_bytes.expose_secret()).expect("valid length"); // Take ownership of the memory of the mnemonic to ensure it will be correctly zeroized on drop let mnemonic = SecretString::new(mnemonic.into_phrase()); - let encrypted_mnemonic = encrypt_string( - &recipients, - mnemonic.expose_secret(), - age::armor::Format::Binary, - ) - .map_err(|e| ErrorKind::Generic.context(e))?; + let encrypted_mnemonic = + encryptor.encrypt_string(mnemonic.expose_secret(), age::armor::Format::Binary)?; self.with_db_mut(|conn, _| { conn.execute( @@ -573,12 +558,13 @@ impl KeyStore { &self, legacy_seed: &SecretVec, ) -> Result { - let recipients = self.recipients().await?; + let encryptor = self.encryptor().await?; let legacy_seed_fp = SeedFingerprint::from_seed(legacy_seed.expose_secret()) .ok_or_else(|| ErrorKind::Generic.context(fl!("err-failed-seed-fingerprinting")))?; - let encrypted_legacy_seed = encrypt_legacy_seed_bytes(&recipients, legacy_seed) + let encrypted_legacy_seed = encryptor + .encrypt_legacy_seed_bytes(legacy_seed) .map_err(|e| ErrorKind::Generic.context(e))?; self.with_db_mut(|conn, _| { @@ -604,10 +590,11 @@ impl KeyStore { &self, sapling_key: &ExtendedSpendingKey, ) -> Result { - let recipients = self.recipients().await?; + let encryptor = self.encryptor().await?; let dfvk = sapling_key.to_diversifiable_full_viewing_key(); - let encrypted_sapling_extsk = encrypt_standalone_sapling_key(&recipients, sapling_key) + let encrypted_sapling_extsk = encryptor + .encrypt_standalone_sapling_key(sapling_key) .map_err(|e| ErrorKind::Generic.context(e))?; self.with_db_mut(|conn, _| { @@ -629,27 +616,54 @@ impl KeyStore { } #[cfg(feature = "zcashd-import")] + #[allow(dead_code)] pub(crate) async fn encrypt_and_store_standalone_transparent_key( &self, key: &zcash_keys::keys::transparent::Key, ) -> Result<(), Error> { - let recipients = self.recipients().await?; + self.store_encrypted_standalone_transparent_keys(&[self + .encryptor() + .await? + .encrypt_standalone_transparent_key(key)?]) + .await + } - let encrypted_transparent_key = - encrypt_standalone_transparent_privkey(&recipients, key.secret()) + /// Stores a batch of encrypted standalone transparent keys produced by + /// [`Encryptor::encrypt_standalone_transparent_key`]. + /// + /// This creates a single database transaction for all inserts, significantly reducing + /// overhead for large migrated wallets. + #[cfg(feature = "zcashd-import")] + pub(crate) async fn store_encrypted_standalone_transparent_keys( + &self, + keys: &[EncryptedStandaloneTransparentKey], + ) -> Result<(), Error> { + self.with_db_mut(|conn, _| { + // Use an explicit transaction to avoid autocommit mode and reduce overhead. + let tx = conn + .transaction() .map_err(|e| ErrorKind::Generic.context(e))?; - self.with_db_mut(|conn, _| { - conn.execute( - "INSERT INTO ext_zallet_keystore_standalone_transparent_keys - VALUES (:pubkey, :encrypted_key_bytes) - ON CONFLICT (pubkey) DO NOTHING ", - named_params! { - ":pubkey": &key.pubkey().serialize(), - ":encrypted_key_bytes": encrypted_transparent_key, - }, - ) - .map_err(|e| ErrorKind::Generic.context(e))?; + { + let mut stmt = tx + .prepare( + "INSERT INTO ext_zallet_keystore_standalone_transparent_keys + VALUES (:pubkey, :encrypted_key_bytes) + ON CONFLICT (pubkey) DO NOTHING ", + ) + .map_err(|e| ErrorKind::Generic.context(e))?; + + for key in keys { + stmt.execute(named_params! { + ":pubkey": &key.pubkey.serialize(), + ":encrypted_key_bytes": key.encrypted_key_bytes, + }) + .map_err(|e| ErrorKind::Generic.context(e))?; + } + } + + tx.commit().map_err(|e| ErrorKind::Generic.context(e))?; + Ok(()) }) .await?; @@ -707,20 +721,18 @@ impl KeyStore { seed_fp: &SeedFingerprint, armor: bool, ) -> Result, Error> { - let recipients = self.recipients().await?; + let encryptor = self.encryptor().await?; let mnemonic = self.decrypt_mnemonic(seed_fp).await?; - let encrypted_mnemonic = encrypt_string( - &recipients, + let encrypted_mnemonic = encryptor.encrypt_string( mnemonic.expose_secret(), if armor { age::armor::Format::AsciiArmor } else { age::armor::Format::Binary }, - ) - .map_err(|e| ErrorKind::Generic.context(e))?; + )?; Ok(encrypted_mnemonic) } @@ -762,6 +774,104 @@ impl KeyStore { } } +pub(crate) struct Encryptor { + recipient_strings: Vec, + recipients: Vec>, +} + +impl Encryptor { + fn from_recipient_strings(recipient_strings: Vec) -> Result { + let recipients = Self::parse_recipient_strings(recipient_strings.clone())?; + Ok(Self { + recipient_strings, + recipients, + }) + } + + fn parse_recipient_strings( + recipient_strings: Vec, + ) -> Result>, Error> { + // TODO: Replace with a helper with configurable callbacks. + let mut stdin_guard = age::cli_common::StdinGuard::new(false); + let recipients = age::cli_common::read_recipients( + recipient_strings, + vec![], + vec![], + None, + &mut stdin_guard, + ) + .map_err(|e| ErrorKind::Generic.context(e))?; + Ok(recipients) + } + + fn is_empty(&self) -> bool { + self.recipient_strings.is_empty() + } + + fn encrypt_string( + &self, + plaintext: &str, + format: age::armor::Format, + ) -> Result, Error> { + encrypt_string(&self.recipients, plaintext, format) + .map_err(|e| ErrorKind::Generic.context(e).into()) + } + + #[cfg(feature = "zcashd-import")] + pub(crate) fn encrypt_standalone_transparent_key( + &self, + key: &zcash_keys::keys::transparent::Key, + ) -> Result { + let encrypted_key_bytes = self + .encrypt_standalone_transparent_privkey(key.secret()) + .map_err(|e| ErrorKind::Generic.context(e))?; + + Ok(EncryptedStandaloneTransparentKey { + pubkey: key.pubkey(), + encrypted_key_bytes, + }) + } + + #[cfg(feature = "zcashd-import")] + fn encrypt_legacy_seed_bytes( + &self, + seed: &SecretVec, + ) -> Result, age::EncryptError> { + encrypt_secret(&self.recipients, seed) + } + + #[cfg(feature = "zcashd-import")] + fn encrypt_standalone_sapling_key( + &self, + key: &ExtendedSpendingKey, + ) -> Result, age::EncryptError> { + let secret = SecretVec::new(key.to_bytes().to_vec()); + encrypt_secret(&self.recipients, &secret) + } + + #[cfg(feature = "transparent-key-import")] + fn encrypt_standalone_transparent_privkey( + &self, + key: &secp256k1::SecretKey, + ) -> Result, age::EncryptError> { + let secret = SecretVec::new(key.secret_bytes().to_vec()); + encrypt_secret(&self.recipients, &secret) + } +} + +#[cfg(feature = "transparent-key-import")] +pub(crate) struct EncryptedStandaloneTransparentKey { + pubkey: secp256k1::PublicKey, + encrypted_key_bytes: Vec, +} + +#[cfg(feature = "transparent-key-import")] +impl EncryptedStandaloneTransparentKey { + pub(crate) fn pubkey(&self) -> &secp256k1::PublicKey { + &self.pubkey + } +} + fn encrypt_string( recipients: &[Box], plaintext: &str, @@ -820,32 +930,6 @@ fn encrypt_secret( Ok(ciphertext) } -#[cfg(feature = "zcashd-import")] -fn encrypt_legacy_seed_bytes( - recipients: &[Box], - seed: &SecretVec, -) -> Result, age::EncryptError> { - encrypt_secret(recipients, seed) -} - -#[cfg(feature = "zcashd-import")] -fn encrypt_standalone_sapling_key( - recipients: &[Box], - key: &ExtendedSpendingKey, -) -> Result, age::EncryptError> { - let secret = SecretVec::new(key.to_bytes().to_vec()); - encrypt_secret(recipients, &secret) -} - -#[cfg(feature = "transparent-key-import")] -fn encrypt_standalone_transparent_privkey( - recipients: &[Box], - key: &secp256k1::SecretKey, -) -> Result, age::EncryptError> { - let secret = SecretVec::new(key.secret_bytes().to_vec()); - encrypt_secret(recipients, &secret) -} - #[cfg(feature = "transparent-key-import")] fn decrypt_standalone_transparent_privkey( identities: &[Box],