From 8c1480304eab4cc306ca72fd1271c992d4469b42 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 28 Nov 2023 10:58:51 -0700 Subject: [PATCH 1/5] zcash_client_backend: Wrap entire Sapling note in `SaplingReceivedNote` --- zcash_client_backend/src/wallet.rs | 30 ++++---- zcash_client_sqlite/src/lib.rs | 3 +- zcash_client_sqlite/src/wallet/sapling.rs | 71 ++++++++++++++----- zcash_primitives/CHANGELOG.md | 1 + .../src/transaction/components/amount.rs | 8 +++ 5 files changed, 81 insertions(+), 32 deletions(-) diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 9a467dbbd..3e45331e6 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -179,9 +179,10 @@ pub struct ReceivedSaplingNote { note_id: NoteRef, txid: TxId, output_index: u16, - diversifier: sapling::Diversifier, - note_value: NonNegativeAmount, - rseed: sapling::Rseed, + // FIXME: We do not yet expose the note directly, because while we know its diversifier to be + // correct, the recipient address may have been reconstructed from persisted data using the + // incorrect IVK. + note: sapling::Note, note_commitment_tree_position: Position, } @@ -190,18 +191,14 @@ impl ReceivedSaplingNote { note_id: NoteRef, txid: TxId, output_index: u16, - diversifier: sapling::Diversifier, - note_value: NonNegativeAmount, - rseed: sapling::Rseed, + note: sapling::Note, note_commitment_tree_position: Position, ) -> Self { ReceivedSaplingNote { note_id, txid, output_index, - diversifier, - note_value, - rseed, + note, note_commitment_tree_position, } } @@ -209,7 +206,6 @@ impl ReceivedSaplingNote { pub fn internal_note_id(&self) -> &NoteRef { &self.note_id } - pub fn txid(&self) -> &TxId { &self.txid } @@ -217,13 +213,16 @@ impl ReceivedSaplingNote { self.output_index } pub fn diversifier(&self) -> sapling::Diversifier { - self.diversifier + *self.note.recipient().diversifier() } pub fn value(&self) -> NonNegativeAmount { - self.note_value + self.note + .value() + .try_into() + .expect("Sapling notes must have values in the range of valid non-negative ZEC values.") } pub fn rseed(&self) -> sapling::Rseed { - self.rseed + *self.note.rseed() } pub fn note_commitment_tree_position(&self) -> Position { self.note_commitment_tree_position @@ -236,7 +235,10 @@ impl sapling_fees::InputView for ReceivedSaplingNote } fn value(&self) -> NonNegativeAmount { - self.note_value + self.note + .value() + .try_into() + .expect("Sapling notes must have values in the range of valid non-negative ZEC values.") } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 39dae79fc..2b534dfd4 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -178,7 +178,7 @@ impl, P: consensus::Parameters> SaplingInputSour txid: &TxId, index: u32, ) -> Result>, Self::Error> { - wallet::sapling::get_spendable_sapling_note(self.conn.borrow(), txid, index) + wallet::sapling::get_spendable_sapling_note(self.conn.borrow(), &self.params, txid, index) } fn select_spendable_sapling_notes( @@ -190,6 +190,7 @@ impl, P: consensus::Parameters> SaplingInputSour ) -> Result>, Self::Error> { wallet::sapling::select_spendable_sapling_notes( self.conn.borrow(), + &self.params, account, target_value, anchor_height, diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 93b890721..233f97099 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -6,7 +6,7 @@ use rusqlite::{named_params, params, types::Value, Connection, Row}; use std::rc::Rc; use zcash_primitives::{ - consensus::BlockHeight, + consensus::{self, BlockHeight}, memo::MemoBytes, sapling::{self, Diversifier, Note, Nullifier, Rseed}, transaction::{ @@ -17,6 +17,7 @@ use zcash_primitives::{ }; use zcash_client_backend::{ + keys::UnifiedFullViewingKey, wallet::{ReceivedSaplingNote, WalletSaplingOutput}, DecryptedOutput, TransferType, }; @@ -84,7 +85,10 @@ impl ReceivedSaplingOutput for DecryptedOutput { } } -fn to_spendable_note(row: &Row) -> Result, SqliteClientError> { +fn to_spendable_note( + params: &P, + row: &Row, +) -> Result, SqliteClientError> { let note_id = ReceivedNoteId(row.get(0)?); let txid = row.get::<_, [u8; 32]>(1).map(TxId::from_bytes)?; let output_index = row.get(2)?; @@ -124,13 +128,36 @@ fn to_spendable_note(row: &Row) -> Result, S SqliteClientError::CorruptedData("Note commitment tree position invalid.".to_string()) })?); + let ufvk_str: String = row.get(7)?; + let ufvk = UnifiedFullViewingKey::decode(params, &ufvk_str) + .map_err(SqliteClientError::CorruptedData)?; + + let is_change: bool = row.get(8)?; + + // FIXME: We attempt to recover the recipient address for the received note based upon the + // change flag; however, this is inaccurate for change notes received prior to the switch to + // internal receivers. However, for now it's okay, because we don't use the recipient directly + // in spends; instead, we use just the diversifier component. A future migration will update + // the persistent received note information so that the note can be definitively linked to + // either the internal or external key component. + let external_address = ufvk + .sapling() + .and_then(|dfvk| dfvk.diversified_address(diversifier)); + let internal_address = ufvk + .sapling() + .and_then(|dfvk| dfvk.diversified_change_address(diversifier)); + let recipient = if is_change { + internal_address.or(external_address) + } else { + external_address + } + .ok_or_else(|| SqliteClientError::CorruptedData("Diversifier invalid.".to_owned()))?; + Ok(ReceivedSaplingNote::from_parts( note_id, txid, output_index, - diversifier, - note_value, - rseed, + sapling::Note::from_parts(recipient, note_value.into(), rseed), note_commitment_tree_position, )) } @@ -139,14 +166,17 @@ fn to_spendable_note(row: &Row) -> Result, S // (https://github.com/rust-lang/rust-clippy/issues/11308) means it fails to identify that the `result` temporary // is required in order to resolve the borrows involved in the `query_and_then` call. #[allow(clippy::let_and_return)] -pub(crate) fn get_spendable_sapling_note( +pub(crate) fn get_spendable_sapling_note( conn: &Connection, + params: &P, txid: &TxId, index: u32, ) -> Result>, SqliteClientError> { let mut stmt_select_note = conn.prepare_cached( - "SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position + "SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position, + accounts.ufvk, is_change FROM sapling_received_notes + INNER JOIN accounts on accounts.account = sapling_received_notes.account INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx WHERE txid = :txid AND output_index = :output_index @@ -159,7 +189,7 @@ pub(crate) fn get_spendable_sapling_note( ":txid": txid.as_ref(), ":output_index": index, ], - to_spendable_note, + |r| to_spendable_note(params, r), )? .next() .transpose(); @@ -182,8 +212,8 @@ fn unscanned_tip_exists( "SELECT EXISTS ( SELECT 1 FROM v_sapling_shard_unscanned_ranges range WHERE range.block_range_start <= :anchor_height - AND :anchor_height BETWEEN - range.subtree_start_height + AND :anchor_height BETWEEN + range.subtree_start_height AND IFNULL(range.subtree_end_height, :anchor_height) )", named_params![":anchor_height": u32::from(anchor_height),], @@ -191,8 +221,9 @@ fn unscanned_tip_exists( ) } -pub(crate) fn select_spendable_sapling_notes( +pub(crate) fn select_spendable_sapling_notes( conn: &Connection, + params: &P, account: AccountId, target_value: Amount, anchor_height: BlockHeight, @@ -229,13 +260,16 @@ pub(crate) fn select_spendable_sapling_notes( // 4) Match the selected notes against the witnesses at the desired height. let mut stmt_select_notes = conn.prepare_cached( "WITH eligible AS ( - SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position, + SELECT + id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position, SUM(value) - OVER (PARTITION BY account, spent ORDER BY id_note) AS so_far + OVER (PARTITION BY sapling_received_notes.account, spent ORDER BY id_note) AS so_far, + accounts.ufvk as ufvk, is_change FROM sapling_received_notes + INNER JOIN accounts on accounts.account = sapling_received_notes.account INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx - WHERE account = :account + WHERE sapling_received_notes.account = :account AND commitment_tree_position IS NOT NULL AND spent IS NULL AND transactions.block <= :anchor_height @@ -251,10 +285,10 @@ pub(crate) fn select_spendable_sapling_notes( AND unscanned.block_range_end > :wallet_birthday ) ) - SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position + SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position, ufvk, is_change FROM eligible WHERE so_far < :target_value UNION - SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position + SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position, ufvk, is_change FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)", )?; @@ -269,7 +303,7 @@ pub(crate) fn select_spendable_sapling_notes( ":exclude": &excluded_ptr, ":wallet_birthday": u32::from(birthday_height) ], - to_spendable_note, + |r| to_spendable_note(params, r), )?; notes.collect::>() @@ -1514,6 +1548,7 @@ pub(crate) mod tests { // Verify that the received note is not considered spendable let spendable = select_spendable_sapling_notes( &st.wallet().conn, + &st.wallet().params, AccountId::from(0), Amount::const_from_i64(300000), received_tx_height + 10, @@ -1529,6 +1564,7 @@ pub(crate) mod tests { // Verify that the received note is now considered spendable let spendable = select_spendable_sapling_notes( &st.wallet().conn, + &st.wallet().params, AccountId::from(0), Amount::const_from_i64(300000), received_tx_height + 10, @@ -1582,6 +1618,7 @@ pub(crate) mod tests { // Verify that our note is considered spendable let spendable = select_spendable_sapling_notes( &st.wallet().conn, + &st.wallet().params, account, Amount::const_from_i64(300000), birthday.height() + 5, diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 97503e948..bc243b660 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -95,6 +95,7 @@ and this library adheres to Rust's notion of - `impl From for zcash_primitives::sapling::value::NoteValue` - `impl Sum for Option` - `impl<'a> Sum<&'a NonNegativeAmount> for Option` + - `impl TryFrom for NonNegativeAmount` - `impl {Clone, PartialEq, Eq} for zcash_primitives::memo::Error` - `impl {PartialEq, Eq} for zcash_primitives::sapling::note::Rseed` - `impl From for [u8; 32]` diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index 0a6a701a0..f352c463e 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -323,6 +323,14 @@ impl From for sapling::value::NoteValue { } } +impl TryFrom for NonNegativeAmount { + type Error = (); + + fn try_from(value: sapling::value::NoteValue) -> Result { + Self::from_u64(value.inner()) + } +} + impl TryFrom for NonNegativeAmount { type Error = (); From 4a7dd2bed226d150ba2a5396cadc1a6646ccb1d8 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 28 Nov 2023 17:03:13 -0700 Subject: [PATCH 2/5] zcash_client_sqlite: Add receiving key scope information to received notes. --- zcash_client_sqlite/src/wallet.rs | 8 ++ zcash_client_sqlite/src/wallet/init.rs | 1 + .../src/wallet/init/migrations.rs | 26 ++-- .../init/migrations/receiving_key_scopes.rs | 111 ++++++++++++++++++ 4 files changed, 135 insertions(+), 11 deletions(-) create mode 100644 zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 3458b5132..9200d8a27 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -75,6 +75,7 @@ use std::ops::RangeInclusive; use tracing::debug; use zcash_client_backend::data_api::{AccountBalance, Ratio, WalletSummary}; use zcash_primitives::transaction::components::amount::NonNegativeAmount; +use zcash_primitives::zip32::Scope; use zcash_client_backend::data_api::{ scanning::{ScanPriority, ScanRange}, @@ -137,6 +138,13 @@ pub(crate) fn pool_code(pool_type: PoolType) -> i64 { } } +pub(crate) fn scope_code(scope: Scope) -> i64 { + match scope { + Scope::External => 0i64, + Scope::Internal => 1i64, + } +} + pub(crate) fn memo_repr(memo: Option<&MemoBytes>) -> Option<&[u8]> { memo.map(|m| { if m == &MemoBytes::empty() { diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 1de985dbf..b7713b193 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -248,6 +248,7 @@ mod tests { memo BLOB, spent INTEGER, commitment_tree_position INTEGER, + recipient_key_scope INTEGER NOT NULL DEFAULT 0, FOREIGN KEY (tx) REFERENCES transactions(id_tx), FOREIGN KEY (account) REFERENCES accounts(account), FOREIGN KEY (spent) REFERENCES transactions(id_tx), diff --git a/zcash_client_sqlite/src/wallet/init/migrations.rs b/zcash_client_sqlite/src/wallet/init/migrations.rs index 4c1486be5..9041d6b10 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations.rs @@ -5,6 +5,7 @@ mod addresses_table; mod initial_setup; mod nullifier_map; mod received_notes_nullable_nf; +mod receiving_key_scopes; mod sapling_memo_consistency; mod sent_notes_to_internal; mod shardtree_support; @@ -40,17 +41,17 @@ pub(super) fn all_migrations( // | // v_transactions_net // | - // received_notes_nullable_nf - // / | \ - // shardtree_support nullifier_map sapling_memo_consistency - // | | - // add_account_birthdays v_transactions_transparent_history - // | | - // v_sapling_shard_unscanned_ranges v_tx_outputs_use_legacy_false - // | | - // wallet_summaries v_transactions_shielding_balance - // | - // v_transactions_note_uniqueness + // received_notes_nullable_nf + // / | \ + // shardtree_support nullifier_map sapling_memo_consistency + // / \ | + // add_account_birthdays receiving_key_scopes v_transactions_transparent_history + // | | + // v_sapling_shard_unscanned_ranges v_tx_outputs_use_legacy_false + // | | + // wallet_summaries v_transactions_shielding_balance + // | + // v_transactions_note_uniqueness vec![ Box::new(initial_setup::Migration {}), Box::new(utxos_table::Migration {}), @@ -86,5 +87,8 @@ pub(super) fn all_migrations( Box::new(v_tx_outputs_use_legacy_false::Migration), Box::new(v_transactions_shielding_balance::Migration), Box::new(v_transactions_note_uniqueness::Migration), + Box::new(receiving_key_scopes::Migration { + params: params.clone(), + }), ] } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs new file mode 100644 index 000000000..f8420b460 --- /dev/null +++ b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs @@ -0,0 +1,111 @@ +//! This migration adds support for the Orchard protocol to `zcash_client_sqlite` + +use std::collections::HashSet; + +use rusqlite::{self, named_params}; +use schemer; +use schemer_rusqlite::RusqliteMigration; + +use tracing::debug; +use uuid::Uuid; + +use zcash_client_backend::{keys::UnifiedFullViewingKey, scanning::ScanningKey}; +use zcash_primitives::{ + consensus::{self, sapling_zip212_enforcement, BlockHeight, BranchId}, + sapling::note_encryption::{try_sapling_note_decryption, PreparedIncomingViewingKey}, + transaction::Transaction, + zip32::Scope, +}; + +use crate::wallet::{ + init::{migrations::shardtree_support, WalletMigrationError}, + scope_code, +}; + +pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0xee89ed2b_c1c2_421e_9e98_c1e3e54a7fc2); + +pub(super) struct Migration

{ + pub(super) params: P, +} + +impl

schemer::Migration for Migration

{ + fn id(&self) -> Uuid { + MIGRATION_ID + } + + fn dependencies(&self) -> HashSet { + [shardtree_support::MIGRATION_ID].into_iter().collect() + } + + fn description(&self) -> &'static str { + "Add support for receiving storage of note commitment tree data using the `shardtree` crate." + } +} + +impl RusqliteMigration for Migration

{ + type Error = WalletMigrationError; + + fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { + // Add commitment tree sizes to block metadata. + debug!("Adding new columns"); + transaction.execute_batch( + &format!( + "ALTER TABLE sapling_received_notes ADD COLUMN recipient_key_scope INTEGER NOT NULL DEFAULT {};", + scope_code(Scope::External) + ) + )?; + + // For all notes marked as change, we have to determine whether they were actually sent to + // the internal key or the external key for the account, so we trial-decrypt the original + // output with both and pick the scope of whichever worked. + let mut stmt_select_notes = transaction.prepare( + "SELECT id_note, output_index, transactions.raw, transactions.expiry_height, accounts.ufvk + FROM sapling_received_notes + INNER JOIN accounts on accounts.account = sapling_received_notes.account + INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx + WHERE is_change = 1" + )?; + + let mut rows = stmt_select_notes.query([])?; + while let Some(row) = rows.next()? { + let note_id: i64 = row.get(0)?; + let output_index: usize = row.get(1)?; + let tx_data: Vec = row.get(2)?; + + let tx = Transaction::read(&tx_data[..], BranchId::Canopy) + .expect("Transaction must be valid"); + let output = tx + .sapling_bundle() + .and_then(|b| b.shielded_outputs().get(output_index)) + .unwrap_or_else(|| panic!("A Sapling output must exist at index {}", output_index)); + let tx_expiry_height = BlockHeight::from(row.get::<_, u32>(3)?); + let zip212_enforcement = sapling_zip212_enforcement(&self.params, tx_expiry_height); + + let ufvk_str: String = row.get(4)?; + let ufvk = UnifiedFullViewingKey::decode(&self.params, &ufvk_str) + .expect("Stored UFVKs must be valid"); + let dfvk = ufvk + .sapling() + .expect("UFVK must have a Sapling component to have received Sapling notes"); + let keys = dfvk.to_sapling_keys(); + + for (scope, ivk, _) in keys { + let pivk = PreparedIncomingViewingKey::new(&ivk); + if try_sapling_note_decryption(&pivk, output, zip212_enforcement).is_some() { + transaction.execute( + "UPDATE sapling_received_notes SET recipient_key_scope = :scope + WHERE id_note = :note_id", + named_params! {":scope": scope_code(scope), ":note_id": note_id}, + )?; + } + } + } + + Ok(()) + } + + fn down(&self, _transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { + // TODO: something better than just panic? + panic!("Cannot revert this migration."); + } +} From 6b10a6dc863b153c8e7b19881b6ed0925891fbaf Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 31 Oct 2023 10:03:37 -0600 Subject: [PATCH 3/5] zcash_client_backend: Move essential wallet types into the `wallet` module. --- zcash_client_backend/CHANGELOG.md | 6 ++ zcash_client_backend/src/data_api.rs | 86 ++----------------- zcash_client_backend/src/data_api/error.rs | 4 +- zcash_client_backend/src/data_api/wallet.rs | 19 ++-- zcash_client_backend/src/lib.rs | 30 +++++++ zcash_client_backend/src/proto.rs | 3 +- zcash_client_backend/src/scanning.rs | 3 +- zcash_client_backend/src/wallet.rs | 49 +++++++++++ zcash_client_sqlite/src/error.rs | 11 ++- zcash_client_sqlite/src/lib.rs | 10 +-- zcash_client_sqlite/src/wallet.rs | 16 ++-- .../wallet/init/migrations/ufvk_support.rs | 4 +- .../init/migrations/v_transactions_net.rs | 2 +- zcash_client_sqlite/src/wallet/sapling.rs | 4 +- 14 files changed, 129 insertions(+), 118 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index fd2a182e7..741d103d7 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -49,6 +49,12 @@ and this library adheres to Rust's notion of wallet::input_selection::{Proposal, SaplingInputs}, }` +### Moved +- `zcash_client_backend::data_api::{PoolType, ShieldedProtocol}` have + been moved into the `zcash_client_backend` root module. +- `zcash_client_backend::data_api::{NoteId, Recipient}` have + been moved into the `zcash_client_backend::wallet` module. + ### Changed - `zcash_client_backend::data_api`: - Arguments to `BlockMetadata::from_parts` have changed to include Orchard. diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index bce56e1b7..6bd01de0b 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -2,7 +2,7 @@ use std::{ collections::{BTreeMap, HashMap}, - fmt::{self, Debug}, + fmt::Debug, io, num::{NonZeroU32, TryFromIntError}, }; @@ -31,7 +31,7 @@ use crate::{ decrypt::DecryptedOutput, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::service::TreeState, - wallet::{ReceivedSaplingNote, WalletTransparentOutput, WalletTx}, + wallet::{NoteId, ReceivedSaplingNote, Recipient, WalletTransparentOutput, WalletTx}, }; use self::chain::CommitmentTreeRoot; @@ -756,81 +756,6 @@ pub struct SentTransaction<'a> { pub utxos_spent: Vec, } -/// A shielded transfer protocol supported by the wallet. -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] -pub enum ShieldedProtocol { - /// The Sapling protocol - Sapling, - /// The Orchard protocol - Orchard, -} - -/// A unique identifier for a shielded transaction output -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] -pub struct NoteId { - txid: TxId, - protocol: ShieldedProtocol, - output_index: u16, -} - -impl NoteId { - /// Constructs a new `NoteId` from its parts. - pub fn new(txid: TxId, protocol: ShieldedProtocol, output_index: u16) -> Self { - Self { - txid, - protocol, - output_index, - } - } - - /// Returns the ID of the transaction containing this note. - pub fn txid(&self) -> &TxId { - &self.txid - } - - /// Returns the shielded protocol used by this note. - pub fn protocol(&self) -> ShieldedProtocol { - self.protocol - } - - /// Returns the index of this note within its transaction's corresponding list of - /// shielded outputs. - pub fn output_index(&self) -> u16 { - self.output_index - } -} - -/// A value pool to which the wallet supports sending transaction outputs. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub enum PoolType { - /// The transparent value pool - Transparent, - /// A shielded value pool. - Shielded(ShieldedProtocol), -} - -impl fmt::Display for PoolType { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - PoolType::Transparent => f.write_str("Transparent"), - PoolType::Shielded(ShieldedProtocol::Sapling) => f.write_str("Sapling"), - PoolType::Shielded(ShieldedProtocol::Orchard) => f.write_str("Orchard"), - } - } -} - -/// A type that represents the recipient of a transaction output; a recipient address (and, for -/// unified addresses, the pool to which the payment is sent) in the case of outgoing output, or an -/// internal account ID and the pool to which funds were sent in the case of a wallet-internal -/// output. -#[derive(Debug, Clone)] -pub enum Recipient { - Transparent(TransparentAddress), - Sapling(sapling::PaymentAddress), - Unified(UnifiedAddress, PoolType), - InternalAccount(AccountId, PoolType), -} - /// A type that represents an output (either Sapling or transparent) that was sent by the wallet. pub struct SentTransactionOutput { output_index: usize, @@ -1149,14 +1074,13 @@ pub mod testing { use crate::{ address::{AddressMetadata, UnifiedAddress}, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, - wallet::{ReceivedSaplingNote, WalletTransparentOutput}, + wallet::{NoteId, ReceivedSaplingNote, WalletTransparentOutput}, }; use super::{ chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, BlockMetadata, - DecryptedTransaction, NoteId, NullifierQuery, SaplingInputSource, ScannedBlock, - SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, - SAPLING_SHARD_HEIGHT, + DecryptedTransaction, NullifierQuery, SaplingInputSource, ScannedBlock, SentTransaction, + WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }; pub struct MockWalletDb { diff --git a/zcash_client_backend/src/data_api/error.rs b/zcash_client_backend/src/data_api/error.rs index 1d2a17611..cf18d87c8 100644 --- a/zcash_client_backend/src/data_api/error.rs +++ b/zcash_client_backend/src/data_api/error.rs @@ -15,12 +15,12 @@ use zcash_primitives::{ }; use crate::data_api::wallet::input_selection::InputSelectorError; -use crate::data_api::PoolType; +use crate::PoolType; #[cfg(feature = "transparent-inputs")] use zcash_primitives::{legacy::TransparentAddress, zip32::DiversifierIndex}; -use super::NoteId; +use crate::wallet::NoteId; /// Errors that can occur as a consequence of wallet operations. #[derive(Debug)] diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 4beb8d9f9..1207d96ff 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -24,15 +24,16 @@ use zcash_primitives::{ use crate::{ address::RecipientAddress, data_api::{ - error::Error, wallet::input_selection::Proposal, DecryptedTransaction, PoolType, Recipient, - SentTransaction, SentTransactionOutput, WalletCommitmentTrees, WalletRead, WalletWrite, + error::Error, wallet::input_selection::Proposal, DecryptedTransaction, SentTransaction, + SentTransactionOutput, WalletCommitmentTrees, WalletRead, WalletWrite, SAPLING_SHARD_HEIGHT, }, decrypt_transaction, fees::{self, ChangeValue, DustOutputPolicy}, keys::UnifiedSpendingKey, - wallet::{OvkPolicy, ReceivedSaplingNote}, + wallet::{NoteId, OvkPolicy, ReceivedSaplingNote, Recipient}, zip321::{self, Payment}, + PoolType, ShieldedProtocol, }; pub mod input_selection; @@ -40,7 +41,7 @@ use input_selection::{ GreedyInputSelector, GreedyInputSelectorError, InputSelector, InputSelectorError, }; -use super::{NoteId, SaplingInputSource, ShieldedProtocol}; +use super::SaplingInputSource; #[cfg(feature = "transparent-inputs")] use { @@ -597,11 +598,11 @@ where sapling_inputs.anchor_height(), )? .ok_or_else(|| { - Error::NoteMismatch(NoteId { - txid: *selected.txid(), - protocol: ShieldedProtocol::Sapling, - output_index: selected.output_index(), - }) + Error::NoteMismatch(NoteId::new( + *selected.txid(), + ShieldedProtocol::Sapling, + selected.output_index(), + )) })?; let key = match scope { diff --git a/zcash_client_backend/src/lib.rs b/zcash_client_backend/src/lib.rs index c2382ba87..b8a45ba89 100644 --- a/zcash_client_backend/src/lib.rs +++ b/zcash_client_backend/src/lib.rs @@ -23,8 +23,38 @@ pub mod zip321; #[cfg(feature = "unstable-serialization")] pub mod serialization; +use std::fmt; + pub use decrypt::{decrypt_transaction, DecryptedOutput, TransferType}; #[cfg(test)] #[macro_use] extern crate assert_matches; + +/// A shielded transfer protocol supported by the wallet. +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub enum ShieldedProtocol { + /// The Sapling protocol + Sapling, + /// The Orchard protocol + Orchard, +} + +/// A value pool to which the wallet supports sending transaction outputs. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum PoolType { + /// The transparent value pool + Transparent, + /// A shielded value pool. + Shielded(ShieldedProtocol), +} + +impl fmt::Display for PoolType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + PoolType::Transparent => f.write_str("Transparent"), + PoolType::Shielded(ShieldedProtocol::Sapling) => f.write_str("Sapling"), + PoolType::Shielded(ShieldedProtocol::Orchard) => f.write_str("Orchard"), + } + } +} diff --git a/zcash_client_backend/src/proto.rs b/zcash_client_backend/src/proto.rs index 1216bfcd6..8c0050190 100644 --- a/zcash_client_backend/src/proto.rs +++ b/zcash_client_backend/src/proto.rs @@ -27,10 +27,11 @@ use zcash_note_encryption::{EphemeralKeyBytes, COMPACT_NOTE_SIZE}; use crate::{ data_api::{ wallet::input_selection::{Proposal, ProposalError, SaplingInputs}, - PoolType, SaplingInputSource, ShieldedProtocol, TransparentInputSource, + SaplingInputSource, TransparentInputSource, }, fees::{ChangeValue, TransactionBalance}, zip321::{TransactionRequest, Zip321Error}, + PoolType, ShieldedProtocol, }; #[rustfmt::skip] diff --git a/zcash_client_backend/src/scanning.rs b/zcash_client_backend/src/scanning.rs index 5f778e487..aafeac951 100644 --- a/zcash_client_backend/src/scanning.rs +++ b/zcash_client_backend/src/scanning.rs @@ -19,11 +19,12 @@ use zcash_primitives::{ zip32::{AccountId, Scope}, }; -use crate::data_api::{BlockMetadata, ScannedBlock, ShieldedProtocol}; +use crate::data_api::{BlockMetadata, ScannedBlock}; use crate::{ proto::compact_formats::CompactBlock, scan::{Batch, BatchRunner, Tasks}, wallet::{WalletSaplingOutput, WalletSaplingSpend, WalletTx}, + ShieldedProtocol, }; /// A key that can be used to perform trial decryption and nullifier diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 3e45331e6..94b145001 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -18,6 +18,55 @@ use zcash_primitives::{ zip32::AccountId, }; +use crate::{address::UnifiedAddress, PoolType, ShieldedProtocol}; + +/// A unique identifier for a shielded transaction output +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub struct NoteId { + txid: TxId, + protocol: ShieldedProtocol, + output_index: u16, +} + +impl NoteId { + /// Constructs a new `NoteId` from its parts. + pub fn new(txid: TxId, protocol: ShieldedProtocol, output_index: u16) -> Self { + Self { + txid, + protocol, + output_index, + } + } + + /// Returns the ID of the transaction containing this note. + pub fn txid(&self) -> &TxId { + &self.txid + } + + /// Returns the shielded protocol used by this note. + pub fn protocol(&self) -> ShieldedProtocol { + self.protocol + } + + /// Returns the index of this note within its transaction's corresponding list of + /// shielded outputs. + pub fn output_index(&self) -> u16 { + self.output_index + } +} + +/// A type that represents the recipient of a transaction output; a recipient address (and, for +/// unified addresses, the pool to which the payment is sent) in the case of outgoing output, or an +/// internal account ID and the pool to which funds were sent in the case of a wallet-internal +/// output. +#[derive(Debug, Clone)] +pub enum Recipient { + Transparent(TransparentAddress), + Sapling(sapling::PaymentAddress), + Unified(UnifiedAddress, PoolType), + InternalAccount(AccountId, PoolType), +} + /// A subset of a [`Transaction`] relevant to wallets and light clients. /// /// [`Transaction`]: zcash_primitives::transaction::Transaction diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 952b0f93c..c1993c39b 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -4,10 +4,13 @@ use std::error; use std::fmt; use shardtree::error::ShardTreeError; -use zcash_client_backend::data_api::PoolType; -use zcash_client_backend::encoding::{Bech32DecodeError, TransparentCodecError}; -use zcash_primitives::transaction::components::amount::BalanceError; -use zcash_primitives::{consensus::BlockHeight, zip32::AccountId}; +use zcash_client_backend::{ + encoding::{Bech32DecodeError, TransparentCodecError}, + PoolType, +}; +use zcash_primitives::{ + consensus::BlockHeight, transaction::components::amount::BalanceError, zip32::AccountId, +}; use crate::wallet::commitment_tree; use crate::PRUNING_DEPTH; diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 2b534dfd4..58fd8f7e2 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -64,14 +64,14 @@ use zcash_client_backend::{ self, chain::{BlockSource, CommitmentTreeRoot}, scanning::{ScanPriority, ScanRange}, - AccountBirthday, BlockMetadata, DecryptedTransaction, NoteId, NullifierQuery, PoolType, - Recipient, SaplingInputSource, ScannedBlock, SentTransaction, ShieldedProtocol, - WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + AccountBirthday, BlockMetadata, DecryptedTransaction, NullifierQuery, SaplingInputSource, + ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary, + WalletWrite, SAPLING_SHARD_HEIGHT, }, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, - wallet::{ReceivedSaplingNote, WalletTransparentOutput}, - DecryptedOutput, TransferType, + wallet::{NoteId, ReceivedSaplingNote, Recipient, WalletTransparentOutput}, + DecryptedOutput, PoolType, ShieldedProtocol, TransferType, }; use crate::{error::SqliteClientError, wallet::commitment_tree::SqliteShardStore}; diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 9200d8a27..b6141ea01 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -77,27 +77,25 @@ use zcash_client_backend::data_api::{AccountBalance, Ratio, WalletSummary}; use zcash_primitives::transaction::components::amount::NonNegativeAmount; use zcash_primitives::zip32::Scope; -use zcash_client_backend::data_api::{ - scanning::{ScanPriority, ScanRange}, - AccountBirthday, NoteId, ShieldedProtocol, SAPLING_SHARD_HEIGHT, -}; -use zcash_primitives::transaction::TransactionData; - use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight, BranchId, NetworkUpgrade, Parameters}, memo::{Memo, MemoBytes}, merkle_tree::read_commitment_tree, - transaction::{components::Amount, Transaction, TxId}, + transaction::{components::Amount, Transaction, TransactionData, TxId}, zip32::{AccountId, DiversifierIndex}, }; use zcash_client_backend::{ address::{RecipientAddress, UnifiedAddress}, - data_api::{BlockMetadata, PoolType, Recipient, SentTransactionOutput}, + data_api::{ + scanning::{ScanPriority, ScanRange}, + AccountBirthday, BlockMetadata, SentTransactionOutput, SAPLING_SHARD_HEIGHT, + }, encoding::AddressCodec, keys::UnifiedFullViewingKey, - wallet::WalletTx, + wallet::{NoteId, Recipient, WalletTx}, + PoolType, ShieldedProtocol, }; use crate::wallet::commitment_tree::{get_max_checkpointed_height, SqliteShardStore}; diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs b/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs index 889d8d0ae..2f2340975 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs @@ -8,9 +8,7 @@ use secrecy::{ExposeSecret, SecretVec}; use uuid::Uuid; use zcash_client_backend::{ - address::RecipientAddress, - data_api::{PoolType, ShieldedProtocol}, - keys::UnifiedSpendingKey, + address::RecipientAddress, keys::UnifiedSpendingKey, PoolType, ShieldedProtocol, }; use zcash_primitives::{consensus, zip32::AccountId}; diff --git a/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs index c6ad76403..cb2d8ebe5 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs @@ -6,7 +6,7 @@ use rusqlite::{self, named_params}; use schemer; use schemer_rusqlite::RusqliteMigration; use uuid::Uuid; -use zcash_client_backend::data_api::{PoolType, ShieldedProtocol}; +use zcash_client_backend::{PoolType, ShieldedProtocol}; use super::add_transaction_views; use crate::wallet::{init::WalletMigrationError, pool_code}; diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 233f97099..3e0b68bef 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -483,14 +483,14 @@ pub(crate) mod tests { chain::CommitmentTreeRoot, error::Error, wallet::input_selection::{GreedyInputSelector, GreedyInputSelectorError}, - AccountBirthday, Ratio, ShieldedProtocol, WalletCommitmentTrees, WalletRead, - WalletWrite, + AccountBirthday, Ratio, WalletCommitmentTrees, WalletRead, WalletWrite, }, decrypt_transaction, fees::{fixed, standard, DustOutputPolicy}, keys::UnifiedSpendingKey, wallet::OvkPolicy, zip321::{self, Payment, TransactionRequest}, + ShieldedProtocol, }; use crate::{ From cad4f25b754d2c7027b69b5da97a9569a27096fb Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 31 Oct 2023 10:03:37 -0600 Subject: [PATCH 4/5] zcash_client_backend: Replace `ReceivedSaplingNote` with `ReceivedNote` `ReceivedNote` now allows Orchard notes to be represented as received notes. As part of this change, received notes now track whether they were received using internally- or externally-scoped viewing keys. This eliminates the need to trial-regenerate notes using the wallet's IVKs to determine scope at spend time. --- zcash_client_backend/CHANGELOG.md | 13 ++- zcash_client_backend/src/data_api.rs | 30 +++--- zcash_client_backend/src/data_api/wallet.rs | 81 ++++------------ .../src/data_api/wallet/input_selection.rs | 17 ++-- zcash_client_backend/src/scanning.rs | 21 ++-- zcash_client_backend/src/wallet.rs | 72 +++++++++----- zcash_client_sqlite/src/lib.rs | 12 +-- zcash_client_sqlite/src/wallet.rs | 16 +++- zcash_client_sqlite/src/wallet/sapling.rs | 96 ++++++++++++------- 9 files changed, 187 insertions(+), 171 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 741d103d7..e6455b144 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -30,8 +30,8 @@ and this library adheres to Rust's notion of functionality and move it behind the `transparent-inputs` feature flag. - `zcash_client_backend::fees::standard` - `zcash_client_backend::wallet`: - - `ReceivedSaplingNote::from_parts` - - `ReceivedSaplingNote::{txid, output_index, diversifier, rseed, note_commitment_tree_position}` + - `WalletNote` + - `ReceivedNote` - `zcash_client_backend::zip321::TransactionRequest::total` - `zcash_client_backend::zip321::parse::Param::name` - `zcash_client_backend::proto::` @@ -60,6 +60,13 @@ and this library adheres to Rust's notion of - Arguments to `BlockMetadata::from_parts` have changed to include Orchard. - `BlockMetadata::sapling_tree_size` now returns an `Option` instead of a `u32` for consistency with Orchard. + - `WalletShieldedOutput` has an additiona type parameter which is used for + key scope. `WalletShieldedOutput::from_parts` now takes an additional + argument of this type. + - `WalletTx` has an additional type parameter as a consequence of the + `WalletShieldedOutput` change. + - `ScannedBlock` has an additional type parameter as a consequence of the + `WalletTx` change. - Arguments to `ScannedBlock::from_parts` have changed. - `ScannedBlock::metadata` has been renamed to `to_block_metadata` and now returns an owned value rather than a reference. @@ -174,6 +181,8 @@ and this library adheres to Rust's notion of - `WalletTransparentOutput::value` ### Removed +- `zcash_client_backend::wallet::ReceivedSaplingNote` has been replaced by + `zcash_client_backend::ReceivedNote. - `zcash_client_backend::data_api::WalletRead::is_valid_account_extfvk` has been removed; it was unused in the ECC mobile wallet SDKs and has been superseded by `get_account_for_ufvk`. diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 6bd01de0b..e743af76b 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -23,7 +23,7 @@ use zcash_primitives::{ }, Transaction, TxId, }, - zip32::AccountId, + zip32::{AccountId, Scope}, }; use crate::{ @@ -31,7 +31,7 @@ use crate::{ decrypt::DecryptedOutput, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::service::TreeState, - wallet::{NoteId, ReceivedSaplingNote, Recipient, WalletTransparentOutput, WalletTx}, + wallet::{NoteId, ReceivedNote, Recipient, WalletTransparentOutput, WalletTx}, }; use self::chain::CommitmentTreeRoot; @@ -358,7 +358,7 @@ pub trait SaplingInputSource { &self, txid: &TxId, index: u32, - ) -> Result>, Self::Error>; + ) -> Result>, Self::Error>; /// Returns a list of spendable Sapling notes sufficient to cover the specified target value, /// if possible. @@ -368,7 +368,7 @@ pub trait SaplingInputSource { target_value: Amount, anchor_height: BlockHeight, exclude: &[Self::NoteRef], - ) -> Result>, Self::Error>; + ) -> Result>, Self::Error>; } /// A trait representing the capability to query a data store for unspent transparent UTXOs @@ -602,20 +602,20 @@ impl BlockMetadata { /// decrypted and extracted from a [`CompactBlock`]. /// /// [`CompactBlock`]: crate::proto::compact_formats::CompactBlock -pub struct ScannedBlock { +pub struct ScannedBlock { block_height: BlockHeight, block_hash: BlockHash, block_time: u32, sapling_tree_size: u32, orchard_tree_size: u32, - transactions: Vec>, + transactions: Vec>, sapling_nullifier_map: Vec<(TxId, u16, Vec)>, sapling_commitments: Vec<(sapling::Node, Retention)>, orchard_nullifier_map: Vec<(TxId, u16, Vec)>, orchard_commitments: Vec<(orchard::note::NoteCommitment, Retention)>, } -impl ScannedBlock { +impl ScannedBlock { /// Constructs a new `ScannedBlock` #[allow(clippy::too_many_arguments)] pub fn from_parts( @@ -624,7 +624,7 @@ impl ScannedBlock { block_time: u32, sapling_tree_size: u32, orchard_tree_size: u32, - transactions: Vec>, + transactions: Vec>, sapling_nullifier_map: Vec<(TxId, u16, Vec)>, sapling_commitments: Vec<(sapling::Node, Retention)>, orchard_nullifier_map: Vec<(TxId, u16, Vec)>, @@ -670,7 +670,7 @@ impl ScannedBlock { } /// Returns the list of transactions from the block that are relevant to the wallet. - pub fn transactions(&self) -> &[WalletTx] { + pub fn transactions(&self) -> &[WalletTx] { &self.transactions } @@ -981,7 +981,7 @@ pub trait WalletWrite: WalletRead { /// `blocks` must be sequential, in order of increasing block height fn put_blocks( &mut self, - blocks: Vec>, + blocks: Vec>, ) -> Result<(), Self::Error>; /// Updates the wallet's view of the blockchain. @@ -1068,13 +1068,13 @@ pub mod testing { memo::Memo, sapling, transaction::{components::Amount, Transaction, TxId}, - zip32::AccountId, + zip32::{AccountId, Scope}, }; use crate::{ address::{AddressMetadata, UnifiedAddress}, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, - wallet::{NoteId, ReceivedSaplingNote, WalletTransparentOutput}, + wallet::{NoteId, ReceivedNote, WalletTransparentOutput}, }; use super::{ @@ -1112,7 +1112,7 @@ pub mod testing { &self, _txid: &TxId, _index: u32, - ) -> Result>, Self::Error> { + ) -> Result>, Self::Error> { Ok(None) } @@ -1122,7 +1122,7 @@ pub mod testing { _target_value: Amount, _anchor_height: BlockHeight, _exclude: &[Self::NoteRef], - ) -> Result>, Self::Error> { + ) -> Result>, Self::Error> { Ok(Vec::new()) } } @@ -1290,7 +1290,7 @@ pub mod testing { #[allow(clippy::type_complexity)] fn put_blocks( &mut self, - _blocks: Vec>, + _blocks: Vec>, ) -> Result<(), Self::Error> { Ok(()) } diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 1207d96ff..6df5547fc 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -1,16 +1,11 @@ use std::num::NonZeroU32; -use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; - use zcash_primitives::{ - consensus::{self, BlockHeight, NetworkUpgrade}, + consensus::{self, NetworkUpgrade}, memo::MemoBytes, sapling::{ - self, note_encryption::{try_sapling_note_decryption, PreparedIncomingViewingKey}, prover::{OutputProver, SpendProver}, - zip32::DiversifiableFullViewingKey, - Node, }, transaction::{ builder::Builder, @@ -26,12 +21,11 @@ use crate::{ data_api::{ error::Error, wallet::input_selection::Proposal, DecryptedTransaction, SentTransaction, SentTransactionOutput, WalletCommitmentTrees, WalletRead, WalletWrite, - SAPLING_SHARD_HEIGHT, }, decrypt_transaction, fees::{self, ChangeValue, DustOutputPolicy}, keys::UnifiedSpendingKey, - wallet::{NoteId, OvkPolicy, ReceivedSaplingNote, Recipient}, + wallet::{OvkPolicy, Recipient, WalletNote}, zip321::{self, Payment}, PoolType, ShieldedProtocol, }; @@ -591,26 +585,24 @@ where if let Some(sapling_inputs) = proposal.sapling_inputs() { wallet_db.with_sapling_tree_mut::<_, _, Error<_, _, _, _>>(|sapling_tree| { for selected in sapling_inputs.notes() { - let (note, scope, merkle_path) = select_key_for_note( - sapling_tree, - selected, - &dfvk, - sapling_inputs.anchor_height(), - )? - .ok_or_else(|| { - Error::NoteMismatch(NoteId::new( - *selected.txid(), - ShieldedProtocol::Sapling, - selected.output_index(), - )) - })?; + match selected.note() { + WalletNote::Sapling(note) => { + let key = match selected.spending_key_scope() { + Scope::External => usk.sapling().clone(), + Scope::Internal => usk.sapling().derive_internal(), + }; - let key = match scope { - Scope::External => usk.sapling().clone(), - Scope::Internal => usk.sapling().derive_internal(), - }; + let merkle_path = sapling_tree.witness_at_checkpoint_id_caching( + selected.note_commitment_tree_position(), + &sapling_inputs.anchor_height(), + )?; - builder.add_sapling_spend(key, note, merkle_path)?; + builder.add_sapling_spend(key, note.clone(), merkle_path)?; + } + WalletNote::Orchard(_) => { + panic!("Orchard spends are not yet supported"); + } + } } Ok(()) })?; @@ -881,40 +873,3 @@ where &proposal, ) } - -#[allow(clippy::type_complexity)] -fn select_key_for_note>( - commitment_tree: &mut ShardTree< - S, - { sapling::NOTE_COMMITMENT_TREE_DEPTH }, - SAPLING_SHARD_HEIGHT, - >, - selected: &ReceivedSaplingNote, - dfvk: &DiversifiableFullViewingKey, - anchor_height: BlockHeight, -) -> Result, ShardTreeError> { - // Attempt to reconstruct the note being spent using both the internal and external dfvks - // corresponding to the unified spending key, checking against the witness we are using - // to spend the note that we've used the correct key. - let external_note = dfvk - .diversified_address(selected.diversifier()) - .map(|addr| addr.create_note(selected.value().try_into().unwrap(), selected.rseed())); - let internal_note = dfvk - .diversified_change_address(selected.diversifier()) - .map(|addr| addr.create_note(selected.value().try_into().unwrap(), selected.rseed())); - - let expected_root = commitment_tree.root_at_checkpoint_id(&anchor_height)?; - let merkle_path = commitment_tree.witness_at_checkpoint_id_caching( - selected.note_commitment_tree_position(), - &anchor_height, - )?; - - Ok(external_note - .filter(|n| expected_root == merkle_path.root(Node::from_cmu(&n.cmu()))) - .map(|n| (n, Scope::External, merkle_path.clone())) - .or_else(|| { - internal_note - .filter(|n| expected_root == merkle_path.root(Node::from_cmu(&n.cmu()))) - .map(|n| (n, Scope::Internal, merkle_path)) - })) -} diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs index 01d9dd445..1936ca222 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -22,7 +22,7 @@ use crate::{ address::{RecipientAddress, UnifiedAddress}, data_api::SaplingInputSource, fees::{ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance}, - wallet::{ReceivedSaplingNote, WalletTransparentOutput}, + wallet::{ReceivedNote, WalletTransparentOutput}, zip321::TransactionRequest, }; @@ -263,15 +263,12 @@ impl Debug for Proposal { #[derive(Clone, PartialEq, Eq)] pub struct SaplingInputs { anchor_height: BlockHeight, - notes: NonEmpty>, + notes: NonEmpty>, } impl SaplingInputs { /// Constructs a [`SaplingInputs`] from its constituent parts. - pub fn from_parts( - anchor_height: BlockHeight, - notes: NonEmpty>, - ) -> Self { + pub fn from_parts(anchor_height: BlockHeight, notes: NonEmpty>) -> Self { Self { anchor_height, notes, @@ -285,7 +282,7 @@ impl SaplingInputs { } /// Returns the list of Sapling notes to be used as inputs to the proposed transaction. - pub fn notes(&self) -> &NonEmpty> { + pub fn notes(&self) -> &NonEmpty> { &self.notes } } @@ -534,7 +531,7 @@ where } } - let mut sapling_inputs: Vec> = vec![]; + let mut sapling_inputs: Vec> = vec![]; let mut prior_available = NonNegativeAmount::ZERO; let mut amount_required = NonNegativeAmount::ZERO; let mut exclude: Vec = vec![]; @@ -654,7 +651,7 @@ where target_height, &transparent_inputs, &Vec::::new(), - &Vec::>::new(), + &Vec::>::new(), &Vec::::new(), &self.dust_output_policy, ); @@ -670,7 +667,7 @@ where target_height, &transparent_inputs, &Vec::::new(), - &Vec::>::new(), + &Vec::>::new(), &Vec::::new(), &self.dust_output_policy, )? diff --git a/zcash_client_backend/src/scanning.rs b/zcash_client_backend/src/scanning.rs index aafeac951..d43b37620 100644 --- a/zcash_client_backend/src/scanning.rs +++ b/zcash_client_backend/src/scanning.rs @@ -258,7 +258,7 @@ pub fn scan_block( vks: &[(&AccountId, &K)], sapling_nullifiers: &[(AccountId, sapling::Nullifier)], prior_block_metadata: Option<&BlockMetadata>, -) -> Result, ScanError> { +) -> Result, ScanError> { scan_block_with_runner::<_, _, ()>( params, block, @@ -341,7 +341,7 @@ pub(crate) fn scan_block_with_runner< nullifiers: &[(AccountId, sapling::Nullifier)], prior_block_metadata: Option<&BlockMetadata>, mut batch_runner: Option<&mut TaggedBatchRunner>, -) -> Result, ScanError> { +) -> Result, ScanError> { if let Some(scan_error) = check_hash_continuity(&block, prior_block_metadata) { return Err(scan_error); } @@ -441,7 +441,7 @@ pub(crate) fn scan_block_with_runner< )?; let compact_block_tx_count = block.vtx.len(); - let mut wtxs: Vec> = vec![]; + let mut wtxs: Vec> = vec![]; let mut sapling_nullifier_map = Vec::with_capacity(block.vtx.len()); let mut sapling_note_commitments: Vec<(sapling::Node, Retention)> = vec![]; for (tx_idx, tx) in block.vtx.into_iter().enumerate() { @@ -495,7 +495,7 @@ pub(crate) fn scan_block_with_runner< u32::try_from(tx.actions.len()).expect("Orchard action count cannot exceed a u32"); // Check for incoming notes while incrementing tree and witnesses - let mut shielded_outputs: Vec> = vec![]; + let mut shielded_outputs: Vec> = vec![]; { let decoded = &tx .outputs @@ -528,7 +528,7 @@ pub(crate) fn scan_block_with_runner< "The batch runner and scan_block must use the same set of IVKs.", ); - (d_note.note, a, (*nk).clone()) + (d_note.note, a, d_note.ivk_tag.1, (*nk).clone()) }) }) .collect() @@ -538,13 +538,13 @@ pub(crate) fn scan_block_with_runner< .flat_map(|(a, k)| { k.to_sapling_keys() .into_iter() - .map(move |(_, ivk, nk)| (**a, ivk, nk)) + .map(move |(scope, ivk, nk)| (**a, scope, ivk, nk)) }) .collect::>(); let ivks = vks .iter() - .map(|(_, ivk, _)| ivk) + .map(|(_, _, ivk, _)| ivk) .map(PreparedIncomingViewingKey::new) .collect::>(); @@ -552,8 +552,8 @@ pub(crate) fn scan_block_with_runner< .into_iter() .map(|v| { v.map(|((note, _), ivk_idx)| { - let (account, _, nk) = &vks[ivk_idx]; - (note, *account, (*nk).clone()) + let (account, scope, _, nk) = &vks[ivk_idx]; + (note, *account, scope.clone(), (*nk).clone()) }) }) .collect() @@ -574,7 +574,7 @@ pub(crate) fn scan_block_with_runner< (false, false) => Retention::Ephemeral, }; - if let Some((note, account, nk)) = dec_output { + if let Some((note, account, scope, nk)) = dec_output { // A note is marked as "change" if the account that received it // also spent notes in the same transaction. This will catch, // for instance: @@ -596,6 +596,7 @@ pub(crate) fn scan_block_with_runner< is_change, note_commitment_tree_position, nf, + scope, )); } diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 94b145001..c0aae2252 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -15,7 +15,7 @@ use zcash_primitives::{ }, TxId, }, - zip32::AccountId, + zip32::{AccountId, Scope}, }; use crate::{address::UnifiedAddress, PoolType, ShieldedProtocol}; @@ -70,11 +70,11 @@ pub enum Recipient { /// A subset of a [`Transaction`] relevant to wallets and light clients. /// /// [`Transaction`]: zcash_primitives::transaction::Transaction -pub struct WalletTx { +pub struct WalletTx { pub txid: TxId, pub index: usize, pub sapling_spends: Vec, - pub sapling_outputs: Vec>, + pub sapling_outputs: Vec>, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -159,7 +159,7 @@ impl WalletSaplingSpend { /// A subset of an [`OutputDescription`] relevant to wallets and light clients. /// /// [`OutputDescription`]: zcash_primitives::transaction::components::OutputDescription -pub struct WalletSaplingOutput { +pub struct WalletSaplingOutput { index: usize, cmu: sapling::note::ExtractedNoteCommitment, ephemeral_key: EphemeralKeyBytes, @@ -168,9 +168,10 @@ pub struct WalletSaplingOutput { is_change: bool, note_commitment_tree_position: Position, nf: N, + recipient_key_scope: S, } -impl WalletSaplingOutput { +impl WalletSaplingOutput { /// Constructs a new `WalletSaplingOutput` value from its constituent parts. #[allow(clippy::too_many_arguments)] pub fn from_parts( @@ -182,6 +183,7 @@ impl WalletSaplingOutput { is_change: bool, note_commitment_tree_position: Position, nf: N, + recipient_key_scope: S, ) -> Self { Self { index, @@ -192,6 +194,7 @@ impl WalletSaplingOutput { is_change, note_commitment_tree_position, nf, + recipient_key_scope, } } @@ -219,35 +222,58 @@ impl WalletSaplingOutput { pub fn nf(&self) -> &N { &self.nf } + pub fn recipient_key_scope(&self) -> &S { + &self.recipient_key_scope + } +} + +/// An enumeration of supported shielded note types for use in [`ReceivedNote`] +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum WalletNote { + Sapling(sapling::Note), + Orchard(orchard::Note), +} + +impl WalletNote { + pub fn value(&self) -> NonNegativeAmount { + match self { + WalletNote::Sapling(n) => n.value().try_into().expect( + "Sapling notes must have values in the range of valid non-negative ZEC values.", + ), + WalletNote::Orchard(n) => NonNegativeAmount::from_u64(n.value().inner()).expect( + "Orchard notes must have values in the range of valid non-negative ZEC values.", + ), + } + } } /// Information about a note that is tracked by the wallet that is available for spending, /// with sufficient information for use in note selection. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct ReceivedSaplingNote { +pub struct ReceivedNote { note_id: NoteRef, txid: TxId, output_index: u16, - // FIXME: We do not yet expose the note directly, because while we know its diversifier to be - // correct, the recipient address may have been reconstructed from persisted data using the - // incorrect IVK. - note: sapling::Note, + note: WalletNote, + spending_key_scope: Scope, note_commitment_tree_position: Position, } -impl ReceivedSaplingNote { +impl ReceivedNote { pub fn from_parts( note_id: NoteRef, txid: TxId, output_index: u16, - note: sapling::Note, + note: WalletNote, + spending_key_scope: Scope, note_commitment_tree_position: Position, ) -> Self { - ReceivedSaplingNote { + ReceivedNote { note_id, txid, output_index, note, + spending_key_scope, note_commitment_tree_position, } } @@ -261,33 +287,27 @@ impl ReceivedSaplingNote { pub fn output_index(&self) -> u16 { self.output_index } - pub fn diversifier(&self) -> sapling::Diversifier { - *self.note.recipient().diversifier() + pub fn note(&self) -> &WalletNote { + &self.note } pub fn value(&self) -> NonNegativeAmount { - self.note - .value() - .try_into() - .expect("Sapling notes must have values in the range of valid non-negative ZEC values.") + self.note.value() } - pub fn rseed(&self) -> sapling::Rseed { - *self.note.rseed() + pub fn spending_key_scope(&self) -> Scope { + self.spending_key_scope } pub fn note_commitment_tree_position(&self) -> Position { self.note_commitment_tree_position } } -impl sapling_fees::InputView for ReceivedSaplingNote { +impl sapling_fees::InputView for ReceivedNote { fn note_id(&self) -> &NoteRef { &self.note_id } fn value(&self) -> NonNegativeAmount { - self.note - .value() - .try_into() - .expect("Sapling notes must have values in the range of valid non-negative ZEC values.") + self.note.value() } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 58fd8f7e2..69f79e0b4 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -55,7 +55,7 @@ use zcash_primitives::{ components::amount::{Amount, NonNegativeAmount}, Transaction, TxId, }, - zip32::{AccountId, DiversifierIndex}, + zip32::{AccountId, DiversifierIndex, Scope}, }; use zcash_client_backend::{ @@ -70,7 +70,7 @@ use zcash_client_backend::{ }, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, - wallet::{NoteId, ReceivedSaplingNote, Recipient, WalletTransparentOutput}, + wallet::{NoteId, ReceivedNote, Recipient, WalletTransparentOutput}, DecryptedOutput, PoolType, ShieldedProtocol, TransferType, }; @@ -177,7 +177,7 @@ impl, P: consensus::Parameters> SaplingInputSour &self, txid: &TxId, index: u32, - ) -> Result>, Self::Error> { + ) -> Result>, Self::Error> { wallet::sapling::get_spendable_sapling_note(self.conn.borrow(), &self.params, txid, index) } @@ -187,7 +187,7 @@ impl, P: consensus::Parameters> SaplingInputSour target_value: Amount, anchor_height: BlockHeight, exclude: &[Self::NoteRef], - ) -> Result>, Self::Error> { + ) -> Result>, Self::Error> { wallet::sapling::select_spendable_sapling_notes( self.conn.borrow(), &self.params, @@ -443,7 +443,7 @@ impl WalletWrite for WalletDb #[allow(clippy::type_complexity)] fn put_blocks( &mut self, - blocks: Vec>, + blocks: Vec>, ) -> Result<(), Self::Error> { self.transactionally(|wdb| { let start_positions = blocks.first().map(|block| { @@ -487,7 +487,7 @@ impl WalletWrite for WalletDb for output in &tx.sapling_outputs { // Check whether this note was spent in a later block range that // we previously scanned. - let spent_in = wallet::query_nullifier_map( + let spent_in = wallet::query_nullifier_map::<_, Scope>( wdb.conn.0, ShieldedProtocol::Sapling, output.nf(), diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index b6141ea01..af6f03869 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -143,6 +143,14 @@ pub(crate) fn scope_code(scope: Scope) -> i64 { } } +pub(crate) fn parse_scope(code: i64) -> Option { + match code { + 0i64 => Some(Scope::External), + 1i64 => Some(Scope::Internal), + _ => None, + } +} + pub(crate) fn memo_repr(memo: Option<&MemoBytes>) -> Option<&[u8]> { memo.map(|m| { if m == &MemoBytes::empty() { @@ -1509,9 +1517,9 @@ pub(crate) fn put_block( /// Inserts information about a mined transaction that was observed to /// contain a note related to this wallet into the database. -pub(crate) fn put_tx_meta( +pub(crate) fn put_tx_meta( conn: &rusqlite::Connection, - tx: &WalletTx, + tx: &WalletTx, height: BlockHeight, ) -> Result { // It isn't there, so insert our transaction into the database. @@ -1890,7 +1898,7 @@ pub(crate) fn insert_nullifier_map>( /// Returns the row of the `transactions` table corresponding to the transaction in which /// this nullifier is revealed, if any. -pub(crate) fn query_nullifier_map>( +pub(crate) fn query_nullifier_map, S>( conn: &rusqlite::Transaction<'_>, spend_pool: ShieldedProtocol, nf: &N, @@ -1928,7 +1936,7 @@ pub(crate) fn query_nullifier_map>( // change or explicit in-wallet recipient. put_tx_meta( conn, - &WalletTx:: { + &WalletTx:: { txid, index, sapling_spends: vec![], diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 3e0b68bef..a6f3784f6 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -13,18 +13,18 @@ use zcash_primitives::{ components::{amount::NonNegativeAmount, Amount}, TxId, }, - zip32::AccountId, + zip32::{AccountId, Scope}, }; use zcash_client_backend::{ keys::UnifiedFullViewingKey, - wallet::{ReceivedSaplingNote, WalletSaplingOutput}, + wallet::{ReceivedNote, WalletNote, WalletSaplingOutput}, DecryptedOutput, TransferType, }; use crate::{error::SqliteClientError, ReceivedNoteId}; -use super::{memo_repr, wallet_birthday}; +use super::{memo_repr, parse_scope, scope_code, wallet_birthday}; /// This trait provides a generalization over shielded output representations. pub(crate) trait ReceivedSaplingOutput { @@ -35,9 +35,10 @@ pub(crate) trait ReceivedSaplingOutput { fn is_change(&self) -> bool; fn nullifier(&self) -> Option<&sapling::Nullifier>; fn note_commitment_tree_position(&self) -> Option; + fn recipient_key_scope(&self) -> Scope; } -impl ReceivedSaplingOutput for WalletSaplingOutput { +impl ReceivedSaplingOutput for WalletSaplingOutput { fn index(&self) -> usize { self.index() } @@ -59,6 +60,10 @@ impl ReceivedSaplingOutput for WalletSaplingOutput { fn note_commitment_tree_position(&self) -> Option { Some(WalletSaplingOutput::note_commitment_tree_position(self)) } + + fn recipient_key_scope(&self) -> Scope { + *self.recipient_key_scope() + } } impl ReceivedSaplingOutput for DecryptedOutput { @@ -83,12 +88,19 @@ impl ReceivedSaplingOutput for DecryptedOutput { fn note_commitment_tree_position(&self) -> Option { None } + fn recipient_key_scope(&self) -> Scope { + if self.transfer_type == TransferType::WalletInternal { + Scope::Internal + } else { + Scope::External + } + } } fn to_spendable_note( params: &P, row: &Row, -) -> Result, SqliteClientError> { +) -> Result, SqliteClientError> { let note_id = ReceivedNoteId(row.get(0)?); let txid = row.get::<_, [u8; 32]>(1).map(TxId::from_bytes)?; let output_index = row.get(2)?; @@ -132,32 +144,31 @@ fn to_spendable_note( let ufvk = UnifiedFullViewingKey::decode(params, &ufvk_str) .map_err(SqliteClientError::CorruptedData)?; - let is_change: bool = row.get(8)?; + let scope_code: i64 = row.get(8)?; + let spending_key_scope = parse_scope(scope_code).ok_or_else(|| { + SqliteClientError::CorruptedData(format!("Invalid key scope code {}", scope_code)) + })?; - // FIXME: We attempt to recover the recipient address for the received note based upon the - // change flag; however, this is inaccurate for change notes received prior to the switch to - // internal receivers. However, for now it's okay, because we don't use the recipient directly - // in spends; instead, we use just the diversifier component. A future migration will update - // the persistent received note information so that the note can be definitively linked to - // either the internal or external key component. - let external_address = ufvk - .sapling() - .and_then(|dfvk| dfvk.diversified_address(diversifier)); - let internal_address = ufvk - .sapling() - .and_then(|dfvk| dfvk.diversified_change_address(diversifier)); - let recipient = if is_change { - internal_address.or(external_address) - } else { - external_address + let recipient = match spending_key_scope { + Scope::Internal => ufvk + .sapling() + .and_then(|dfvk| dfvk.diversified_change_address(diversifier)), + Scope::External => ufvk + .sapling() + .and_then(|dfvk| dfvk.diversified_address(diversifier)), } .ok_or_else(|| SqliteClientError::CorruptedData("Diversifier invalid.".to_owned()))?; - Ok(ReceivedSaplingNote::from_parts( + Ok(ReceivedNote::from_parts( note_id, txid, output_index, - sapling::Note::from_parts(recipient, note_value.into(), rseed), + WalletNote::Sapling(sapling::Note::from_parts( + recipient, + note_value.into(), + rseed, + )), + spending_key_scope, note_commitment_tree_position, )) } @@ -171,10 +182,10 @@ pub(crate) fn get_spendable_sapling_note( params: &P, txid: &TxId, index: u32, -) -> Result>, SqliteClientError> { +) -> Result>, SqliteClientError> { let mut stmt_select_note = conn.prepare_cached( "SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position, - accounts.ufvk, is_change + accounts.ufvk, recipient_key_scope FROM sapling_received_notes INNER JOIN accounts on accounts.account = sapling_received_notes.account INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx @@ -228,7 +239,7 @@ pub(crate) fn select_spendable_sapling_notes( target_value: Amount, anchor_height: BlockHeight, exclude: &[ReceivedNoteId], -) -> Result>, SqliteClientError> { +) -> Result>, SqliteClientError> { let birthday_height = match wallet_birthday(conn)? { Some(birthday) => birthday, None => { @@ -264,7 +275,7 @@ pub(crate) fn select_spendable_sapling_notes( id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position, SUM(value) OVER (PARTITION BY sapling_received_notes.account, spent ORDER BY id_note) AS so_far, - accounts.ufvk as ufvk, is_change + accounts.ufvk as ufvk, recipient_key_scope FROM sapling_received_notes INNER JOIN accounts on accounts.account = sapling_received_notes.account INNER JOIN transactions @@ -285,10 +296,10 @@ pub(crate) fn select_spendable_sapling_notes( AND unscanned.block_range_end > :wallet_birthday ) ) - SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position, ufvk, is_change + SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position, ufvk, recipient_key_scope FROM eligible WHERE so_far < :target_value UNION - SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position, ufvk, is_change + SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position, ufvk, recipient_key_scope FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)", )?; @@ -396,7 +407,9 @@ pub(crate) fn put_received_note( ) -> Result<(), SqliteClientError> { let mut stmt_upsert_received_note = conn.prepare_cached( "INSERT INTO sapling_received_notes - (tx, output_index, account, diversifier, value, rcm, memo, nf, is_change, spent, commitment_tree_position) + (tx, output_index, account, diversifier, value, rcm, memo, nf, + is_change, spent, commitment_tree_position, + recipient_key_scope) VALUES ( :tx, :output_index, @@ -408,7 +421,8 @@ pub(crate) fn put_received_note( :nf, :is_change, :spent, - :commitment_tree_position + :commitment_tree_position, + :recipient_key_scope ) ON CONFLICT (tx, output_index) DO UPDATE SET account = :account, @@ -419,7 +433,8 @@ pub(crate) fn put_received_note( memo = IFNULL(:memo, memo), is_change = IFNULL(:is_change, is_change), spent = IFNULL(:spent, spent), - commitment_tree_position = IFNULL(:commitment_tree_position, commitment_tree_position)", + commitment_tree_position = IFNULL(:commitment_tree_position, commitment_tree_position), + recipient_key_scope = :recipient_key_scope", )?; let rcm = output.note().rcm().to_repr(); @@ -438,6 +453,7 @@ pub(crate) fn put_received_note( ":is_change": output.is_change(), ":spent": spent_in, ":commitment_tree_position": output.note_commitment_tree_position().map(u64::from), + ":recipient_key_scope": scope_code(output.recipient_key_scope()), ]; stmt_upsert_received_note @@ -452,6 +468,7 @@ pub(crate) mod tests { use std::{convert::Infallible, num::NonZeroU32}; use incrementalmerkletree::Hashable; + use rusqlite::params; use secrecy::Secret; use zcash_proofs::prover::LocalTxProver; @@ -497,8 +514,8 @@ pub(crate) mod tests { error::SqliteClientError, testing::{input_selector, AddressType, BlockCache, TestBuilder, TestState}, wallet::{ - block_max_scanned, commitment_tree, sapling::select_spendable_sapling_notes, - scanning::tests::test_with_canopy_birthday, + block_max_scanned, commitment_tree, parse_scope, + sapling::select_spendable_sapling_notes, scanning::tests::test_with_canopy_birthday, }, AccountId, NoteId, ReceivedNoteId, }; @@ -1185,6 +1202,15 @@ pub(crate) mod tests { NonNegativeAmount::ZERO ); + let change_note_scope = st.wallet().conn.query_row( + "SELECT recipient_key_scope + FROM sapling_received_notes + WHERE value = ?", + params![u64::from(value)], + |row| Ok(parse_scope(row.get(0)?)), + ); + assert_matches!(change_note_scope, Ok(Some(Scope::Internal))); + // TODO: This test was originally written to use the pre-zip-313 fee rule // and has not yet been updated. #[allow(deprecated)] From ca54b3489d48ff6b9d93c81e39307b57ac0fece7 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 5 Dec 2023 11:08:27 -0700 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Daira Emma Hopwood Co-authored-by: Jack Grigg --- zcash_client_backend/CHANGELOG.md | 5 +- zcash_client_backend/src/data_api/wallet.rs | 2 + zcash_client_backend/src/wallet.rs | 13 +++- .../init/migrations/receiving_key_scopes.rs | 74 ++++++++++++------- 4 files changed, 63 insertions(+), 31 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index e6455b144..89e91250c 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -32,6 +32,7 @@ and this library adheres to Rust's notion of - `zcash_client_backend::wallet`: - `WalletNote` - `ReceivedNote` + - `WalletSaplingOutput::recipient_key_scope` - `zcash_client_backend::zip321::TransactionRequest::total` - `zcash_client_backend::zip321::parse::Param::name` - `zcash_client_backend::proto::` @@ -60,7 +61,7 @@ and this library adheres to Rust's notion of - Arguments to `BlockMetadata::from_parts` have changed to include Orchard. - `BlockMetadata::sapling_tree_size` now returns an `Option` instead of a `u32` for consistency with Orchard. - - `WalletShieldedOutput` has an additiona type parameter which is used for + - `WalletShieldedOutput` has an additional type parameter which is used for key scope. `WalletShieldedOutput::from_parts` now takes an additional argument of this type. - `WalletTx` has an additional type parameter as a consequence of the @@ -182,7 +183,7 @@ and this library adheres to Rust's notion of ### Removed - `zcash_client_backend::wallet::ReceivedSaplingNote` has been replaced by - `zcash_client_backend::ReceivedNote. + `zcash_client_backend::ReceivedNote`. - `zcash_client_backend::data_api::WalletRead::is_valid_account_extfvk` has been removed; it was unused in the ECC mobile wallet SDKs and has been superseded by `get_account_for_ufvk`. diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 6df5547fc..78ded00a7 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -600,6 +600,8 @@ where builder.add_sapling_spend(key, note.clone(), merkle_path)?; } WalletNote::Orchard(_) => { + // FIXME: Implement this once `Proposal` has been refactored to + // include Orchard notes. panic!("Orchard spends are not yet supported"); } } diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index c0aae2252..d1ebf1430 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -55,8 +55,8 @@ impl NoteId { } } -/// A type that represents the recipient of a transaction output; a recipient address (and, for -/// unified addresses, the pool to which the payment is sent) in the case of outgoing output, or an +/// A type that represents the recipient of a transaction output: a recipient address (and, for +/// unified addresses, the pool to which the payment is sent) in the case of an outgoing output, or an /// internal account ID and the pool to which funds were sent in the case of a wallet-internal /// output. #[derive(Debug, Clone)] @@ -158,6 +158,15 @@ impl WalletSaplingSpend { /// A subset of an [`OutputDescription`] relevant to wallets and light clients. /// +/// The type parameter `` is used to specify the nullifier type, which may vary between +/// `Sapling` and `Orchard`, and also may vary depending upon the type of key that was used to +/// decrypt this output; incoming viewing keys do not have the capability to derive the nullifier +/// for a note, and the `` will be `()` in these cases. +/// +/// The type parameter `` is used to specify the type of the scope of the key used to recover +/// this output; this will usually be [`zcash_primitives::zip32::Scope`] for received notes, and +/// `()` for sent notes. +/// /// [`OutputDescription`]: zcash_primitives::transaction::components::OutputDescription pub struct WalletSaplingOutput { index: usize, diff --git a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs index f8420b460..4b9ee7322 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs @@ -1,4 +1,4 @@ -//! This migration adds support for the Orchard protocol to `zcash_client_sqlite` +//! This migration adds decryption key scope to persisted information about received notes. use std::collections::HashSet; @@ -6,20 +6,21 @@ use rusqlite::{self, named_params}; use schemer; use schemer_rusqlite::RusqliteMigration; -use tracing::debug; use uuid::Uuid; -use zcash_client_backend::{keys::UnifiedFullViewingKey, scanning::ScanningKey}; +use zcash_client_backend::keys::UnifiedFullViewingKey; use zcash_primitives::{ consensus::{self, sapling_zip212_enforcement, BlockHeight, BranchId}, - sapling::note_encryption::{try_sapling_note_decryption, PreparedIncomingViewingKey}, + sapling::note_encryption::{ + try_sapling_note_decryption, PreparedIncomingViewingKey, Zip212Enforcement, + }, transaction::Transaction, zip32::Scope, }; use crate::wallet::{ init::{migrations::shardtree_support, WalletMigrationError}, - scope_code, + scan_queue_extrema, scope_code, }; pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0xee89ed2b_c1c2_421e_9e98_c1e3e54a7fc2); @@ -38,7 +39,7 @@ impl

schemer::Migration for Migration

{ } fn description(&self) -> &'static str { - "Add support for receiving storage of note commitment tree data using the `shardtree` crate." + "Add decryption key scope to persisted information about received notes." } } @@ -46,8 +47,6 @@ impl RusqliteMigration for Migration

{ type Error = WalletMigrationError; fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { - // Add commitment tree sizes to block metadata. - debug!("Adding new columns"); transaction.execute_batch( &format!( "ALTER TABLE sapling_received_notes ADD COLUMN recipient_key_scope INTEGER NOT NULL DEFAULT {};", @@ -55,15 +54,16 @@ impl RusqliteMigration for Migration

{ ) )?; - // For all notes marked as change, we have to determine whether they were actually sent to - // the internal key or the external key for the account, so we trial-decrypt the original - // output with both and pick the scope of whichever worked. + // For all notes we have to determine whether they were actually sent to the internal key + // or the external key for the account, so we trial-decrypt the original output with the + // internal IVK and update the persisted scope value if necessary. We check all notes, + // rather than just change notes, because shielding notes may not have been considered + // change. let mut stmt_select_notes = transaction.prepare( - "SELECT id_note, output_index, transactions.raw, transactions.expiry_height, accounts.ufvk + "SELECT id_note, output_index, transactions.raw, transactions.block, transactions.expiry_height, accounts.ufvk FROM sapling_received_notes INNER JOIN accounts on accounts.account = sapling_received_notes.account - INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx - WHERE is_change = 1" + INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx" )?; let mut rows = stmt_select_notes.query([])?; @@ -78,26 +78,46 @@ impl RusqliteMigration for Migration

{ .sapling_bundle() .and_then(|b| b.shielded_outputs().get(output_index)) .unwrap_or_else(|| panic!("A Sapling output must exist at index {}", output_index)); - let tx_expiry_height = BlockHeight::from(row.get::<_, u32>(3)?); - let zip212_enforcement = sapling_zip212_enforcement(&self.params, tx_expiry_height); - let ufvk_str: String = row.get(4)?; + let tx_height = row.get::<_, Option>(3)?.map(BlockHeight::from); + let tx_expiry = row.get::<_, u32>(4)?; + let zip212_height = tx_height.map_or_else( + || { + if tx_expiry == 0 { + scan_queue_extrema(transaction).map(|extrema| extrema.map(|r| *r.end())) + } else { + Ok(Some(BlockHeight::from(tx_expiry))) + } + }, + |h| Ok(Some(h)), + )?; + + let zip212_enforcement = zip212_height.map_or_else( + || { + // If the transaction has not been mined and the expiry height is set to 0 (no + // expiry) an no chain tip information is available, then we assume it can only + // be mined under ZIP 212 enforcement rules, so we default to `On` + Zip212Enforcement::On + }, + |h| sapling_zip212_enforcement(&self.params, h), + ); + + let ufvk_str: String = row.get(5)?; let ufvk = UnifiedFullViewingKey::decode(&self.params, &ufvk_str) .expect("Stored UFVKs must be valid"); let dfvk = ufvk .sapling() .expect("UFVK must have a Sapling component to have received Sapling notes"); - let keys = dfvk.to_sapling_keys(); - for (scope, ivk, _) in keys { - let pivk = PreparedIncomingViewingKey::new(&ivk); - if try_sapling_note_decryption(&pivk, output, zip212_enforcement).is_some() { - transaction.execute( - "UPDATE sapling_received_notes SET recipient_key_scope = :scope - WHERE id_note = :note_id", - named_params! {":scope": scope_code(scope), ":note_id": note_id}, - )?; - } + // We previously set the default to external scope, so we now verify whether the output + // is decryptable using the intenally-scoped IVK and, if so, mark it as such. + let pivk = PreparedIncomingViewingKey::new(&dfvk.to_ivk(Scope::Internal)); + if try_sapling_note_decryption(&pivk, output, zip212_enforcement).is_some() { + transaction.execute( + "UPDATE sapling_received_notes SET recipient_key_scope = :scope + WHERE id_note = :note_id", + named_params! {":scope": scope_code(Scope::Internal), ":note_id": note_id}, + )?; } }