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)]