From 74840829c807ce3c3d0df978d76026423bac44dd Mon Sep 17 00:00:00 2001 From: sasha Date: Mon, 22 May 2023 16:08:20 -0700 Subject: [PATCH] zcash_client_backend: Make `ReceivedSaplingNote` internals private. This also adds the txid and index to `ReceivedSaplingNote` for use as a globally identifier for the note. --- zcash_client_backend/CHANGELOG.md | 8 +++ zcash_client_backend/src/data_api/wallet.rs | 15 +++-- .../src/data_api/wallet/input_selection.rs | 2 +- zcash_client_backend/src/wallet.rs | 57 +++++++++++++++++-- zcash_client_sqlite/src/wallet/sapling.rs | 26 +++++---- zcash_primitives/src/transaction/mod.rs | 6 ++ 6 files changed, 89 insertions(+), 25 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 449efeb5f..9bf7fe75e 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -7,9 +7,17 @@ and this library adheres to Rust's notion of ## [Unreleased] +### Added + +- Added methods to `zcash_client_backend::wallet::ReceivedSaplingNote`: + `{from_parts, txid, output_index, diversifier, rseed, note_commitment_tree_position}`. + ### Changed - `zcash_client_backend::data_api::chain::scan_cached_blocks` now returns a `ScanSummary` containing metadata about the scanned blocks on success. +- The fields of `zcash_client_backend::wallet::ReceivedSaplingNote` are now + private. Use `ReceivedSaplingNote::from_parts` for construction instead. + Accessor methods are provided for each previously-public field. ## [0.10.0] - 2023-09-25 diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 0d1adda6f..f4373d8bb 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -498,9 +498,8 @@ where &dfvk, checkpoint_depth, )? - .ok_or(Error::NoteMismatch(selected.note_id))?; - - builder.add_sapling_spend(key, selected.diversifier, note, merkle_path)?; + .ok_or(Error::NoteMismatch(selected.internal_note_id().clone()))?; + builder.add_sapling_spend(key, selected.diversifier(), note, merkle_path)?; } Ok(()) })?; @@ -774,15 +773,15 @@ fn select_key_for_note>( // 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.note_value.into(), selected.rseed)); + .diversified_address(selected.diversifier()) + .map(|addr| addr.create_note(selected.value().into(), selected.rseed())); let internal_note = dfvk - .diversified_change_address(selected.diversifier) - .map(|addr| addr.create_note(selected.note_value.into(), selected.rseed)); + .diversified_change_address(selected.diversifier()) + .map(|addr| addr.create_note(selected.value().into(), selected.rseed())); let expected_root = commitment_tree.root_at_checkpoint(checkpoint_depth)?; let merkle_path = commitment_tree - .witness_caching(selected.note_commitment_tree_position, checkpoint_depth)?; + .witness_caching(selected.note_commitment_tree_position(), checkpoint_depth)?; Ok(external_note .filter(|n| expected_root == merkle_path.root(Node::from_cmu(&n.cmu()))) 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 578082a33..0c575d9a0 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -419,7 +419,7 @@ where let new_available = sapling_inputs .iter() - .map(|n| n.note_value) + .map(|n| n.value()) .sum::>() .ok_or(BalanceError::Overflow)?; diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index ccfe5a75b..c7467c8a6 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -177,11 +177,58 @@ impl WalletSaplingOutput { /// with sufficient information for use in note selection. #[derive(Debug)] pub struct ReceivedSaplingNote { - pub note_id: NoteRef, - pub diversifier: sapling::Diversifier, - pub note_value: Amount, - pub rseed: sapling::Rseed, - pub note_commitment_tree_position: Position, + note_id: NoteRef, + txid: TxId, + output_index: u16, + diversifier: sapling::Diversifier, + note_value: Amount, + rseed: sapling::Rseed, + note_commitment_tree_position: Position, +} + +impl ReceivedSaplingNote { + pub fn from_parts( + note_id: NoteRef, + txid: TxId, + output_index: u16, + diversifier: sapling::Diversifier, + note_value: Amount, + rseed: sapling::Rseed, + note_commitment_tree_position: Position, + ) -> Self { + ReceivedSaplingNote { + note_id, + txid, + output_index, + diversifier, + note_value, + rseed, + note_commitment_tree_position, + } + } + + pub fn internal_note_id(&self) -> &NoteRef { + &self.note_id + } + + pub fn txid(&self) -> &TxId { + &self.txid + } + pub fn output_index(&self) -> u16 { + self.output_index + } + pub fn diversifier(&self) -> sapling::Diversifier { + self.diversifier + } + pub fn value(&self) -> Amount { + self.note_value + } + pub fn rseed(&self) -> sapling::Rseed { + self.rseed + } + pub fn note_commitment_tree_position(&self) -> Position { + self.note_commitment_tree_position + } } impl sapling_fees::InputView for ReceivedSaplingNote { diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 6bf84cf00..1fa831f41 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -9,7 +9,7 @@ use zcash_primitives::{ consensus::BlockHeight, memo::MemoBytes, sapling::{self, Diversifier, Note, Nullifier, Rseed}, - transaction::components::Amount, + transaction::{components::Amount, TxId}, zip32::AccountId, }; @@ -83,8 +83,10 @@ impl ReceivedSaplingOutput for DecryptedOutput { fn to_spendable_note(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)?; let diversifier = { - let d: Vec<_> = row.get(1)?; + let d: Vec<_> = row.get(3)?; if d.len() != 11 { return Err(SqliteClientError::CorruptedData( "Invalid diversifier length".to_string(), @@ -95,10 +97,10 @@ fn to_spendable_note(row: &Row) -> Result, S Diversifier(tmp) }; - let note_value = Amount::from_i64(row.get(2)?).unwrap(); + let note_value = Amount::from_i64(row.get(4)?).unwrap(); let rseed = { - let rcm_bytes: Vec<_> = row.get(3)?; + let rcm_bytes: Vec<_> = row.get(5)?; // We store rcm directly in the data DB, regardless of whether the note // used a v1 or v2 note plaintext, so for the purposes of spending let's @@ -113,17 +115,19 @@ fn to_spendable_note(row: &Row) -> Result, S }; let note_commitment_tree_position = - Position::from(u64::try_from(row.get::<_, i64>(4)?).map_err(|_| { + Position::from(u64::try_from(row.get::<_, i64>(6)?).map_err(|_| { SqliteClientError::CorruptedData("Note commitment tree position invalid.".to_string()) })?); - Ok(ReceivedSaplingNote { + Ok(ReceivedSaplingNote::from_parts( note_id, + txid, + output_index, diversifier, note_value, rseed, note_commitment_tree_position, - }) + )) } /// Utility method for determining whether we have any spendable notes @@ -170,7 +174,7 @@ pub(crate) fn get_spendable_sapling_notes( } let mut stmt_select_notes = conn.prepare_cached( - "SELECT id_note, diversifier, value, rcm, commitment_tree_position + "SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position FROM sapling_received_notes INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx WHERE account = :account @@ -244,7 +248,7 @@ 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, 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 FROM sapling_received_notes @@ -266,10 +270,10 @@ pub(crate) fn select_spendable_sapling_notes( AND unscanned.block_range_end > :wallet_birthday ) ) - SELECT id_note, diversifier, value, rcm, commitment_tree_position + SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position FROM eligible WHERE so_far < :target_value UNION - SELECT id_note, diversifier, value, rcm, commitment_tree_position + SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)", )?; diff --git a/zcash_primitives/src/transaction/mod.rs b/zcash_primitives/src/transaction/mod.rs index 5d3ebf365..f44a3849d 100644 --- a/zcash_primitives/src/transaction/mod.rs +++ b/zcash_primitives/src/transaction/mod.rs @@ -92,6 +92,12 @@ impl AsRef<[u8; 32]> for TxId { } } +impl From for [u8; 32] { + fn from(value: TxId) -> Self { + value.0 + } +} + impl TxId { pub fn from_bytes(bytes: [u8; 32]) -> Self { TxId(bytes)