zcash_client_backend: Restrict use of backend-specific note identifiers.

In general, it is preferable to use globally relevant identifiers where
possible. This PR removes the `WalletRead::TxRef` associated type in
favor of using `TxId` directly for the transaction identifier, and
restricts the use of the `NoteRef` type to those scenarios where the
result of one query is intended to be used directly as the input to
another query.

Closes #834
This commit is contained in:
Kris Nuttycombe 2023-08-02 13:45:49 -06:00
parent d7bd566b21
commit d3b7dffa3c
9 changed files with 196 additions and 169 deletions

View File

@ -12,6 +12,7 @@ and this library adheres to Rust's notion of
- `impl Debug` for `zcash_client_backend::{data_api::wallet::input_selection::Proposal, wallet::ReceivedSaplingNote}`
- `zcash_client_backend::data_api`:
- `BlockMetadata`
- `NoteId`
- `NullifierQuery` for use with `WalletRead::get_sapling_nullifiers`
- `ScannedBlock`
- `ShieldedProtocol`
@ -33,9 +34,14 @@ and this library adheres to Rust's notion of
- Bumped dependencies to `hdwallet 0.4`, `zcash_primitives 0.12`, `zcash_note_encryption 0.4`,
`incrementalmerkletree 0.4`, `orchard 0.5`, `bs58 0.5`
- `zcash_client_backend::data_api`:
- `WalletRead::get_memo` now returns `Result<Option<Memo>, Self::Error>`
instead of `Result<Memo, Self::Error>` in order to make representable
wallet states where the full note plaintext is not available.
- `WalletRead::TxRef` has been removed in favor of consistently using `TxId` instead.
- `WalletRead::get_transaction` now takes a `TxId` as its argument.
- `WalletWrite::{store_decrypted_tx, store_sent_tx}` now return `Result<(), Self::Error>`
as the `WalletRead::TxRef` associated type has been removed. Use `TxId` instead.
- `WalletRead::get_memo` now takes a `NoteId` as its argument instead of `Self::NoteRef`
and returns `Result<Option<Memo>, Self::Error>` instead of `Result<Memo,
Self::Error>` in order to make representable wallet states where the full
note plaintext is not available.
- `WalletRead::get_nullifiers` has been renamed to `WalletRead::get_sapling_nullifiers`
and its signature has changed; it now subsumes the removed `WalletRead::get_all_nullifiers`.
- `WalletRead::get_target_and_anchor_heights` now takes its argument as a `NonZeroU32`
@ -45,15 +51,15 @@ and this library adheres to Rust's notion of
- `chain::BlockSource::with_blocks` now takes its limit as an `Option<usize>`
instead of `Option<u32>`.
- A new `CommitmentTree` variant has been added to `data_api::error::Error`
- `data_api::wallet::{create_spend_to_address, create_proposed_transaction,
- `wallet::{create_spend_to_address, create_proposed_transaction,
shield_transparent_funds}` all now require that `WalletCommitmentTrees` be
implemented for the type passed to them for the `wallet_db` parameter.
- `data_api::wallet::create_proposed_transaction` now takes an additional
- `wallet::create_proposed_transaction` now takes an additional
`min_confirmations` argument.
- `data_api::wallet::{spend, create_spend_to_address, shield_transparent_funds,
- `wallet::{spend, create_spend_to_address, shield_transparent_funds,
propose_transfer, propose_shielding, create_proposed_transaction}` now take their
respective `min_confirmations` arguments as `NonZeroU32`
- `data_api::wallet::input_selection::InputSelector::{propose_transaction, propose_shielding}`
- `wallet::input_selection::InputSelector::{propose_transaction, propose_shielding}`
now take their respective `min_confirmations` arguments as `NonZeroU32`
- A new `Scan` variant has been added to `data_api::chain::error::Error`.
- A new `SyncRequired` variant has been added to `data_api::wallet::input_selection::InputSelectorError`.
@ -64,6 +70,9 @@ and this library adheres to Rust's notion of
- Arguments to `WalletSaplingOutput::from_parts` have changed.
- `zcash_client_backend::data_api::wallet::input_selection::InputSelector`:
- Arguments to `{propose_transaction, propose_shielding}` have changed.
- `zcash_client_backend::data_api::wallet::{create_spend_to_address, spend,
create_proposed_transaction, shield_transparent_funds}` now return the `TxId`
for the newly created transaction instead an internal database identifier.
- `zcash_client_backend::wallet::ReceivedSaplingNote::note_commitment_tree_position`
has replaced the `witness` field in the same struct.
- `zcash_client_backend::welding_rig` has been renamed to `zcash_client_backend::scanning`

View File

@ -38,6 +38,8 @@ pub mod wallet;
pub const SAPLING_SHARD_HEIGHT: u8 = sapling::NOTE_COMMITMENT_TREE_DEPTH / 2;
/// An enumeration of constraints that can be applied when querying for nullifiers for notes
/// belonging to the wallet.
pub enum NullifierQuery {
Unspent,
All,
@ -58,13 +60,6 @@ pub trait WalletRead {
/// or a UUID.
type NoteRef: Copy + Debug + Eq + Ord;
/// Backend-specific transaction identifier.
///
/// For example, this might be a database identifier type
/// or a TxId if the backend is able to support that type
/// directly.
type TxRef: Copy + Debug + Eq + Ord;
/// Returns the minimum and maximum block heights for stored blocks.
///
/// This will return `Ok(None)` if no block data is present in the database.
@ -189,14 +184,13 @@ pub trait WalletRead {
/// Returns the memo for a note.
///
/// Implementations of this method must return an error if the note identifier
/// does not appear in the backing data store. Returns `Ok(None)` if the note
/// is known to the wallet but memo data has not yet been populated for that
/// note.
fn get_memo(&self, id_note: Self::NoteRef) -> Result<Option<Memo>, Self::Error>;
/// Returns `Ok(None)` if the note is known to the wallet but memo data has not yet been
/// populated for that note, or if the note identifier does not correspond to a note
/// that is known to the wallet.
fn get_memo(&self, note_id: NoteId) -> Result<Option<Memo>, Self::Error>;
/// Returns a transaction.
fn get_transaction(&self, id_tx: Self::TxRef) -> Result<Transaction, Self::Error>;
fn get_transaction(&self, txid: TxId) -> Result<Transaction, Self::Error>;
/// Returns the nullifiers for notes that the wallet is tracking, along with their associated
/// account IDs, that are either unspent or have not yet been confirmed as spent (in that a
@ -206,7 +200,7 @@ pub trait WalletRead {
query: NullifierQuery,
) -> Result<Vec<(AccountId, sapling::Nullifier)>, Self::Error>;
/// Return all unspent Sapling notes.
/// Return all unspent Sapling notes, excluding the specified note IDs.
fn get_spendable_sapling_notes(
&self,
account: AccountId,
@ -380,13 +374,43 @@ pub struct SentTransaction<'a> {
}
/// A shielded transfer protocol supported by the wallet.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub enum ShieldedProtocol {
/// The Sapling protocol
Sapling,
// TODO: 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: u32,
}
impl NoteId {
pub fn new(txid: TxId, protocol: ShieldedProtocol, output_index: u32) -> Self {
Self {
txid,
protocol,
output_index,
}
}
pub fn txid(&self) -> &TxId {
&self.txid
}
pub fn protocol(&self) -> ShieldedProtocol {
self.protocol
}
pub fn output_index(&self) -> u32 {
self.output_index
}
}
/// A value pool to which the wallet supports sending transaction outputs.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum PoolType {
@ -508,7 +532,7 @@ pub trait WalletWrite: WalletRead {
fn put_blocks(
&mut self,
blocks: Vec<ScannedBlock<sapling::Nullifier>>,
) -> Result<Vec<Self::NoteRef>, Self::Error>;
) -> Result<(), Self::Error>;
/// Updates the wallet's view of the blockchain.
///
@ -520,14 +544,11 @@ pub trait WalletWrite: WalletRead {
fn update_chain_tip(&mut self, tip_height: BlockHeight) -> Result<(), Self::Error>;
/// Caches a decrypted transaction in the persistent wallet store.
fn store_decrypted_tx(
&mut self,
received_tx: DecryptedTransaction,
) -> Result<Self::TxRef, Self::Error>;
fn store_decrypted_tx(&mut self, received_tx: DecryptedTransaction) -> Result<(), Self::Error>;
/// Saves information about a transaction that was constructed and sent by the wallet to the
/// persistent wallet store.
fn store_sent_tx(&mut self, sent_tx: &SentTransaction) -> Result<Self::TxRef, Self::Error>;
fn store_sent_tx(&mut self, sent_tx: &SentTransaction) -> Result<(), Self::Error>;
/// Truncates the wallet database to the specified height.
///
@ -610,7 +631,7 @@ pub mod testing {
use super::{
chain::CommitmentTreeRoot, scanning::ScanRange, BlockMetadata, DecryptedTransaction,
NullifierQuery, ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead,
NoteId, NullifierQuery, ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead,
WalletWrite, SAPLING_SHARD_HEIGHT,
};
@ -635,7 +656,6 @@ pub mod testing {
impl WalletRead for MockWalletDb {
type Error = ();
type NoteRef = u32;
type TxRef = TxId;
fn block_height_extrema(&self) -> Result<Option<(BlockHeight, BlockHeight)>, Self::Error> {
Ok(None)
@ -707,11 +727,11 @@ pub mod testing {
Ok(Amount::zero())
}
fn get_memo(&self, _id_note: Self::NoteRef) -> Result<Option<Memo>, Self::Error> {
fn get_memo(&self, _id_note: NoteId) -> Result<Option<Memo>, Self::Error> {
Ok(None)
}
fn get_transaction(&self, _id_tx: Self::TxRef) -> Result<Transaction, Self::Error> {
fn get_transaction(&self, _txid: TxId) -> Result<Transaction, Self::Error> {
Err(())
}
@ -790,8 +810,8 @@ pub mod testing {
fn put_blocks(
&mut self,
_blocks: Vec<ScannedBlock<sapling::Nullifier>>,
) -> Result<Vec<Self::NoteRef>, Self::Error> {
Ok(vec![])
) -> Result<(), Self::Error> {
Ok(())
}
fn update_chain_tip(&mut self, _tip_height: BlockHeight) -> Result<(), Self::Error> {
@ -801,15 +821,12 @@ pub mod testing {
fn store_decrypted_tx(
&mut self,
_received_tx: DecryptedTransaction,
) -> Result<Self::TxRef, Self::Error> {
Ok(TxId::from_bytes([0u8; 32]))
) -> Result<(), Self::Error> {
Ok(())
}
fn store_sent_tx(
&mut self,
_sent_tx: &SentTransaction,
) -> Result<Self::TxRef, Self::Error> {
Ok(TxId::from_bytes([0u8; 32]))
fn store_sent_tx(&mut self, _sent_tx: &SentTransaction) -> Result<(), Self::Error> {
Ok(())
}
fn truncate_to_height(&mut self, _block_height: BlockHeight) -> Result<(), Self::Error> {

View File

@ -1,7 +1,7 @@
use std::fmt::Debug;
use std::{convert::Infallible, num::NonZeroU32};
use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
use zcash_primitives::transaction::TxId;
use zcash_primitives::{
consensus::{self, BlockHeight, NetworkUpgrade},
memo::MemoBytes,
@ -203,7 +203,7 @@ pub fn create_spend_to_address<DbT, ParamsT>(
ovk_policy: OvkPolicy,
min_confirmations: NonZeroU32,
) -> Result<
DbT::TxRef,
TxId,
Error<
<DbT as WalletRead>::Error,
<DbT as WalletCommitmentTrees>::Error,
@ -306,7 +306,7 @@ pub fn spend<DbT, ParamsT, InputsT>(
ovk_policy: OvkPolicy,
min_confirmations: NonZeroU32,
) -> Result<
DbT::TxRef,
TxId,
Error<
<DbT as WalletRead>::Error,
<DbT as WalletCommitmentTrees>::Error,
@ -317,7 +317,6 @@ pub fn spend<DbT, ParamsT, InputsT>(
>
where
DbT: WalletWrite + WalletCommitmentTrees,
DbT::TxRef: Copy + Debug,
DbT::NoteRef: Copy + Eq + Ord,
ParamsT: consensus::Parameters + Clone,
InputsT: InputSelector<DataSource = DbT>,
@ -441,7 +440,7 @@ pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT>(
min_confirmations: NonZeroU32,
change_memo: Option<MemoBytes>,
) -> Result<
DbT::TxRef,
TxId,
Error<
<DbT as WalletRead>::Error,
<DbT as WalletCommitmentTrees>::Error,
@ -452,7 +451,6 @@ pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT>(
>
where
DbT: WalletWrite + WalletCommitmentTrees,
DbT::TxRef: Copy + Debug,
DbT::NoteRef: Copy + Eq + Ord,
ParamsT: consensus::Parameters + Clone,
FeeRuleT: FeeRule,
@ -670,7 +668,9 @@ where
#[cfg(feature = "transparent-inputs")]
utxos_spent: utxos.iter().map(|utxo| utxo.outpoint().clone()).collect(),
})
.map_err(Error::DataSource)
.map_err(Error::DataSource)?;
Ok(tx.txid())
}
/// Constructs a transaction that consumes available transparent UTXOs belonging to
@ -720,7 +720,7 @@ pub fn shield_transparent_funds<DbT, ParamsT, InputsT>(
memo: &MemoBytes,
min_confirmations: NonZeroU32,
) -> Result<
DbT::TxRef,
TxId,
Error<
<DbT as WalletRead>::Error,
<DbT as WalletCommitmentTrees>::Error,

View File

@ -12,6 +12,7 @@ and this library adheres to Rust's notion of
- A new default-enabled feature flag `multicore`. This allows users to disable
multicore support by setting `default_features = false` on their
`zcash_primitives`, `zcash_proofs`, and `zcash_client_sqlite` dependencies.
- `zcash_client_sqlite::ReceivedNoteId`
- `zcash_client_sqlite::wallet::commitment_tree` A new module containing a
sqlite-backed implementation of `shardtree::store::ShardStore`.
@ -30,6 +31,9 @@ and this library adheres to Rust's notion of
### Removed
- The empty `wallet::transact` module has been removed.
- `zcash_client_sqlite::NoteId` has been replaced with `zcash_client_sqlite::ReceivedNoteId`
as the `SentNoteId` variant of is now unused following changes to
`zcash_client_backend::data_api::WalletRead`.
### Fixed
- Fixed an off-by-one error in the `BlockSource` implementation for the SQLite-backed

View File

@ -38,7 +38,7 @@ use maybe_rayon::{
};
use rusqlite::{self, Connection};
use secrecy::{ExposeSecret, SecretVec};
use std::{borrow::Borrow, collections::HashMap, convert::AsRef, fmt, ops::Range, path::Path};
use std::{borrow::Borrow, collections::HashMap, convert::AsRef, ops::Range, path::Path};
use incrementalmerkletree::Position;
use shardtree::{error::ShardTreeError, ShardTree};
@ -61,9 +61,9 @@ use zcash_client_backend::{
self,
chain::{BlockSource, CommitmentTreeRoot},
scanning::{ScanPriority, ScanRange},
BlockMetadata, DecryptedTransaction, NullifierQuery, PoolType, Recipient, ScannedBlock,
SentTransaction, ShieldedProtocol, WalletCommitmentTrees, WalletRead, WalletWrite,
SAPLING_SHARD_HEIGHT,
BlockMetadata, DecryptedTransaction, NoteId, NullifierQuery, PoolType, Recipient,
ScannedBlock, SentTransaction, ShieldedProtocol, WalletCommitmentTrees, WalletRead,
WalletWrite, SAPLING_SHARD_HEIGHT,
},
keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
proto::compact_formats::CompactBlock,
@ -97,22 +97,9 @@ pub(crate) const VERIFY_LOOKAHEAD: u32 = 10;
pub(crate) const SAPLING_TABLES_PREFIX: &str = "sapling";
/// A newtype wrapper for sqlite primary key values for the notes
/// table.
/// A newtype wrapper for received note identifiers.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub enum NoteId {
SentNoteId(i64),
ReceivedNoteId(i64),
}
impl fmt::Display for NoteId {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
NoteId::SentNoteId(id) => write!(f, "Sent Note {}", id),
NoteId::ReceivedNoteId(id) => write!(f, "Received Note {}", id),
}
}
}
pub struct ReceivedNoteId(pub(crate) i64);
/// A newtype wrapper for sqlite primary key values for the utxos
/// table.
@ -160,8 +147,7 @@ impl<P: consensus::Parameters + Clone> WalletDb<Connection, P> {
impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for WalletDb<C, P> {
type Error = SqliteClientError;
type NoteRef = NoteId;
type TxRef = i64;
type NoteRef = ReceivedNoteId;
fn block_height_extrema(&self) -> Result<Option<(BlockHeight, BlockHeight)>, Self::Error> {
wallet::block_height_extrema(self.conn.borrow()).map_err(SqliteClientError::from)
@ -229,17 +215,17 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
wallet::get_balance_at(self.conn.borrow(), account, anchor_height)
}
fn get_memo(&self, id_note: Self::NoteRef) -> Result<Option<Memo>, Self::Error> {
match id_note {
NoteId::SentNoteId(id_note) => wallet::get_sent_memo(self.conn.borrow(), id_note),
NoteId::ReceivedNoteId(id_note) => {
wallet::get_received_memo(self.conn.borrow(), id_note)
}
fn get_memo(&self, note_id: NoteId) -> Result<Option<Memo>, Self::Error> {
let sent_memo = wallet::get_sent_memo(self.conn.borrow(), note_id)?;
if sent_memo.is_some() {
Ok(sent_memo)
} else {
wallet::get_received_memo(self.conn.borrow(), note_id)
}
}
fn get_transaction(&self, id_tx: i64) -> Result<Transaction, Self::Error> {
wallet::get_transaction(self.conn.borrow(), &self.params, id_tx).map(|(_, tx)| tx)
fn get_transaction(&self, txid: TxId) -> Result<Transaction, Self::Error> {
wallet::get_transaction(self.conn.borrow(), &self.params, txid).map(|(_, tx)| tx)
}
fn get_sapling_nullifiers(
@ -404,7 +390,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
fn put_blocks(
&mut self,
blocks: Vec<ScannedBlock<sapling::Nullifier>>,
) -> Result<Vec<Self::NoteRef>, Self::Error> {
) -> Result<(), Self::Error> {
self.transactionally(|wdb| {
let start_positions = blocks.first().map(|block| {
(
@ -415,7 +401,6 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
),
)
});
let mut wallet_note_ids = vec![];
let mut sapling_commitments = vec![];
let mut last_scanned_height = None;
let mut note_positions = vec![];
@ -453,12 +438,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
output.nf(),
)?;
let received_note_id = wallet::sapling::put_received_note(
wdb.conn.0, output, tx_row, spent_in,
)?;
// Save witness for note.
wallet_note_ids.push(received_note_id);
wallet::sapling::put_received_note(wdb.conn.0, output, tx_row, spent_in)?;
}
}
@ -535,7 +515,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
)?;
}
Ok(wallet_note_ids)
Ok(())
})
}
@ -546,10 +526,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
Ok(())
}
fn store_decrypted_tx(
&mut self,
d_tx: DecryptedTransaction,
) -> Result<Self::TxRef, Self::Error> {
fn store_decrypted_tx(&mut self, d_tx: DecryptedTransaction) -> Result<(), Self::Error> {
self.transactionally(|wdb| {
let tx_ref = wallet::put_tx_data(wdb.conn.0, d_tx.tx, None, None)?;
@ -636,11 +613,11 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
}
}
Ok(tx_ref)
Ok(())
})
}
fn store_sent_tx(&mut self, sent_tx: &SentTransaction) -> Result<Self::TxRef, Self::Error> {
fn store_sent_tx(&mut self, sent_tx: &SentTransaction) -> Result<(), Self::Error> {
self.transactionally(|wdb| {
let tx_ref = wallet::put_tx_data(
wdb.conn.0,
@ -699,8 +676,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
}
}
// Return the row number of the transaction, so the caller can fetch it for sending.
Ok(tx_ref)
Ok(())
})
}

View File

@ -70,7 +70,7 @@ use std::convert::TryFrom;
use std::io::{self, Cursor};
use zcash_client_backend::data_api::scanning::{ScanPriority, ScanRange};
use zcash_client_backend::data_api::ShieldedProtocol;
use zcash_client_backend::data_api::{NoteId, ShieldedProtocol};
use zcash_primitives::transaction::TransactionData;
use zcash_primitives::{
@ -474,20 +474,27 @@ pub(crate) fn get_balance_at(
}
}
/// Returns the memo for a received note.
///
/// The note is identified by its row index in the `sapling_received_notes` table within the wdb
/// database.
/// Returns the memo for a received note, if the note is known to the wallet.
pub(crate) fn get_received_memo(
conn: &rusqlite::Connection,
id_note: i64,
note_id: NoteId,
) -> Result<Option<Memo>, SqliteClientError> {
let memo_bytes: Option<Vec<_>> = conn.query_row(
"SELECT memo FROM sapling_received_notes
WHERE id_note = ?",
[id_note],
|row| row.get(0),
)?;
let memo_bytes: Option<Vec<_>> = match note_id.protocol() {
ShieldedProtocol::Sapling => conn
.query_row(
"SELECT memo FROM sapling_received_notes
JOIN transactions ON sapling_received_notes.tx = transactions.id_tx
WHERE transactions.txid = :txid
AND sapling_received_notes.output_index = :output_index",
named_params![
":txid": note_id.txid().as_ref(),
":output_index": note_id.output_index()
],
|row| row.get(0),
)
.optional()?
.flatten(),
};
memo_bytes
.map(|b| {
@ -507,7 +514,7 @@ pub(crate) fn get_received_memo(
pub(crate) fn get_transaction<P: Parameters>(
conn: &rusqlite::Connection,
params: &P,
id_tx: i64,
txid: TxId,
) -> Result<(BlockHeight, Transaction), SqliteClientError> {
let (tx_bytes, block_height, expiry_height): (
Vec<_>,
@ -515,8 +522,8 @@ pub(crate) fn get_transaction<P: Parameters>(
Option<BlockHeight>,
) = conn.query_row(
"SELECT raw, block, expiry_height FROM transactions
WHERE id_tx = ?",
[id_tx],
WHERE txid = ?",
[txid.as_ref()],
|row| {
let h: Option<u32> = row.get(1)?;
let expiry: Option<u32> = row.get(2)?;
@ -572,20 +579,27 @@ pub(crate) fn get_transaction<P: Parameters>(
}
}
/// Returns the memo for a sent note.
///
/// The note is identified by its row index in the `sent_notes` table within the wdb
/// database.
/// Returns the memo for a sent note, if the sent note is known to the wallet.
pub(crate) fn get_sent_memo(
conn: &rusqlite::Connection,
id_note: i64,
note_id: NoteId,
) -> Result<Option<Memo>, SqliteClientError> {
let memo_bytes: Option<Vec<_>> = conn.query_row(
"SELECT memo FROM sent_notes
WHERE id_note = ?",
[id_note],
|row| row.get(0),
)?;
let memo_bytes: Option<Vec<_>> = conn
.query_row(
"SELECT memo FROM sent_notes
JOIN transactions ON sent_notes.tx = transactions.id_tx
WHERE transactions.txid = :txid
AND sent_notes.output_pool = :pool_code
AND sent_notes.output_index = :output_index",
named_params![
":txid": note_id.txid().as_ref(),
":pool_code": pool_code(PoolType::Shielded(note_id.protocol())),
":output_index": note_id.output_index()
],
|row| row.get(0),
)
.optional()?
.flatten();
memo_bytes
.map(|b| {

View File

@ -346,7 +346,7 @@ pub fn init_blocks_table<P: consensus::Parameters>(
#[cfg(test)]
#[allow(deprecated)]
mod tests {
use rusqlite::{self, ToSql};
use rusqlite::{self, named_params, ToSql};
use secrecy::Secret;
use std::collections::HashMap;
use tempfile::NamedTempFile;
@ -969,8 +969,11 @@ mod tests {
let mut tx_bytes = vec![];
tx.write(&mut tx_bytes).unwrap();
wdb.conn.execute(
"INSERT INTO transactions (block, id_tx, txid, raw) VALUES (0, 0, '', ?)",
[&tx_bytes[..]],
"INSERT INTO transactions (block, id_tx, txid, raw) VALUES (0, 0, :txid, :tx_bytes)",
named_params![
":txid": tx.txid().as_ref(),
":tx_bytes": &tx_bytes[..]
],
)?;
wdb.conn.execute(
"INSERT INTO sent_notes (tx, output_index, from_account, address, value)

View File

@ -8,7 +8,7 @@ use rusqlite::named_params;
use schemer_rusqlite::RusqliteMigration;
use uuid::Uuid;
use zcash_client_backend::{decrypt_transaction, keys::UnifiedFullViewingKey};
use zcash_primitives::{consensus, zip32::AccountId};
use zcash_primitives::{consensus, transaction::TxId, zip32::AccountId};
use crate::{
error::SqliteClientError,
@ -46,7 +46,9 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), Self::Error> {
let mut stmt_raw_tx = transaction.prepare(
"SELECT DISTINCT sent_notes.tx, accounts.account, accounts.ufvk
"SELECT DISTINCT
transactions.id_tx, transactions.txid,
accounts.account, accounts.ufvk
FROM sent_notes
JOIN accounts ON sent_notes.from_account = accounts.account
JOIN transactions ON transactions.id_tx = sent_notes.tx
@ -55,12 +57,13 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
let mut rows = stmt_raw_tx.query([])?;
let mut tx_sent_notes: BTreeMap<i64, HashMap<AccountId, UnifiedFullViewingKey>> =
let mut tx_sent_notes: BTreeMap<(i64, TxId), HashMap<AccountId, UnifiedFullViewingKey>> =
BTreeMap::new();
while let Some(row) = rows.next()? {
let id_tx: i64 = row.get(0)?;
let account: u32 = row.get(1)?;
let ufvk_str: String = row.get(2)?;
let txid = row.get(1).map(TxId::from_bytes)?;
let account: u32 = row.get(2)?;
let ufvk_str: String = row.get(3)?;
let ufvk = UnifiedFullViewingKey::decode(&self.params, &ufvk_str).map_err(|e| {
WalletMigrationError::CorruptedData(format!(
"Could not decode unified full viewing key for account {}: {:?}",
@ -69,7 +72,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
})?;
tx_sent_notes
.entry(id_tx)
.entry((id_tx, txid))
.or_default()
.insert(AccountId::from(account), ufvk);
}
@ -81,9 +84,9 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
AND output_index = :output_index",
)?;
for (id_tx, ufvks) in tx_sent_notes {
for ((id_tx, txid), ufvks) in tx_sent_notes {
let (block_height, tx) =
get_transaction(transaction, &self.params, id_tx).map_err(|err| match err {
get_transaction(transaction, &self.params, txid).map_err(|err| match err {
SqliteClientError::CorruptedData(msg) => {
WalletMigrationError::CorruptedData(msg)
}

View File

@ -18,7 +18,7 @@ use zcash_client_backend::{
DecryptedOutput, TransferType,
};
use crate::{error::SqliteClientError, NoteId};
use crate::{error::SqliteClientError, ReceivedNoteId};
use super::memo_repr;
@ -81,8 +81,8 @@ impl ReceivedSaplingOutput for DecryptedOutput<Note> {
}
}
fn to_spendable_note(row: &Row) -> Result<ReceivedSaplingNote<NoteId>, SqliteClientError> {
let note_id = NoteId::ReceivedNoteId(row.get(0)?);
fn to_spendable_note(row: &Row) -> Result<ReceivedSaplingNote<ReceivedNoteId>, SqliteClientError> {
let note_id = ReceivedNoteId(row.get(0)?);
let diversifier = {
let d: Vec<_> = row.get(1)?;
if d.len() != 11 {
@ -130,8 +130,8 @@ pub(crate) fn get_spendable_sapling_notes(
conn: &Connection,
account: AccountId,
anchor_height: BlockHeight,
exclude: &[NoteId],
) -> Result<Vec<ReceivedSaplingNote<NoteId>>, SqliteClientError> {
exclude: &[ReceivedNoteId],
) -> Result<Vec<ReceivedSaplingNote<ReceivedNoteId>>, SqliteClientError> {
let mut stmt_select_notes = conn.prepare_cached(
"SELECT id_note, diversifier, value, rcm, commitment_tree_position
FROM sapling_received_notes
@ -142,13 +142,7 @@ pub(crate) fn get_spendable_sapling_notes(
AND id_note NOT IN rarray(:exclude)",
)?;
let excluded: Vec<Value> = exclude
.iter()
.filter_map(|n| match n {
NoteId::ReceivedNoteId(i) => Some(Value::from(*i)),
NoteId::SentNoteId(_) => None,
})
.collect();
let excluded: Vec<Value> = exclude.iter().map(|n| Value::from(n.0)).collect();
let excluded_ptr = Rc::new(excluded);
let notes = stmt_select_notes.query_and_then(
@ -168,8 +162,8 @@ pub(crate) fn select_spendable_sapling_notes(
account: AccountId,
target_value: Amount,
anchor_height: BlockHeight,
exclude: &[NoteId],
) -> Result<Vec<ReceivedSaplingNote<NoteId>>, SqliteClientError> {
exclude: &[ReceivedNoteId],
) -> Result<Vec<ReceivedSaplingNote<ReceivedNoteId>>, SqliteClientError> {
// The goal of this SQL statement is to select the oldest notes until the required
// value has been reached, and then fetch the witnesses at the desired height for the
// selected notes. This is achieved in several steps:
@ -207,13 +201,7 @@ pub(crate) fn select_spendable_sapling_notes(
FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)",
)?;
let excluded: Vec<Value> = exclude
.iter()
.filter_map(|n| match n {
NoteId::ReceivedNoteId(i) => Some(Value::from(*i)),
NoteId::SentNoteId(_) => None,
})
.collect();
let excluded: Vec<Value> = exclude.iter().map(|n| Value::from(n.0)).collect();
let excluded_ptr = Rc::new(excluded);
let notes = stmt_select_notes.query_and_then(
@ -313,7 +301,7 @@ pub(crate) fn put_received_note<T: ReceivedSaplingOutput>(
output: &T,
tx_ref: i64,
spent_in: Option<i64>,
) -> Result<NoteId, SqliteClientError> {
) -> 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)
@ -339,8 +327,7 @@ pub(crate) fn put_received_note<T: ReceivedSaplingOutput>(
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)
RETURNING id_note",
commitment_tree_position = IFNULL(:commitment_tree_position, commitment_tree_position)",
)?;
let rcm = output.note().rcm().to_repr();
@ -362,10 +349,10 @@ pub(crate) fn put_received_note<T: ReceivedSaplingOutput>(
];
stmt_upsert_received_note
.query_row(sql_args, |row| {
row.get::<_, i64>(0).map(NoteId::ReceivedNoteId)
})
.map_err(SqliteClientError::from)
.execute(sql_args)
.map_err(SqliteClientError::from)?;
Ok(())
}
#[cfg(test)]
@ -403,7 +390,7 @@ pub(crate) mod tests {
create_proposed_transaction, create_spend_to_address,
input_selection::GreedyInputSelector, propose_transfer, spend,
},
WalletRead, WalletWrite,
ShieldedProtocol, WalletRead, WalletWrite,
},
decrypt_transaction,
fees::{fixed, zip317, DustOutputPolicy},
@ -566,13 +553,24 @@ pub(crate) mod tests {
// Verify that the stored sent notes match what we're expecting
let mut stmt_sent_notes = db_data
.conn
.prepare("SELECT id_note FROM sent_notes WHERE tx = ?")
.prepare(
"SELECT output_index
FROM sent_notes
JOIN transactions ON transactions.id_tx = sent_notes.tx
WHERE transactions.txid = ?",
)
.unwrap();
let sent_note_ids = stmt_sent_notes
.query(rusqlite::params![sent_tx_id])
.query(rusqlite::params![sent_tx_id.as_ref()])
.unwrap()
.mapped(|row| row.get::<_, i64>(0).map(NoteId::SentNoteId))
.mapped(|row| {
Ok(NoteId::new(
sent_tx_id,
ShieldedProtocol::Sapling,
row.get(0)?,
))
})
.collect::<Result<Vec<_>, _>>()
.unwrap();
@ -601,8 +599,11 @@ pub(crate) mod tests {
assert!(found_sent_change_memo);
assert!(found_sent_empty_memo);
// Check that querying for a nonexistent sent note returns an error
assert_matches!(db_data.get_memo(NoteId::SentNoteId(12345)), Err(_));
// Check that querying for a nonexistent sent note returns None
assert_matches!(
db_data.get_memo(NoteId::new(sent_tx_id, ShieldedProtocol::Sapling, 12345)),
Ok(None)
);
}
#[test]
@ -1101,7 +1102,7 @@ pub(crate) mod tests {
let to = addr2.into();
let send_and_recover_with_policy = |db_data: &mut WalletDb<Connection, _>, ovk_policy| {
let tx_row = create_spend_to_address(
let txid = create_spend_to_address(
db_data,
&tests::network(),
test_prover(),
@ -1119,8 +1120,8 @@ pub(crate) mod tests {
.conn
.query_row(
"SELECT raw FROM transactions
WHERE id_tx = ?",
[tx_row],
WHERE txid = ?",
[txid.as_ref()],
|row| row.get(0),
)
.unwrap();