Simplify sqlite backend storage for sent notes & utxos.

The currently deprecated implementations of `insert_sent_utxo`,
`insert_sent_note`, `put_sent_utxo` and `put_sent_note` all store to the
same `sent_notes` table internally. Since there's no immediate plan to
change this arrangement, it's better to have a single pair of internal
`insert_sent_output` and `put_sent_output` methods instead.
This commit is contained in:
Kris Nuttycombe 2022-10-10 12:38:43 -06:00
parent ee9869bbeb
commit 56b2edd498
9 changed files with 145 additions and 195 deletions

View File

@ -255,12 +255,24 @@ pub struct SentTransaction<'a> {
}
#[derive(Debug, Clone)]
#[allow(clippy::large_enum_variant)]
pub enum Recipient {
Address(RecipientAddress),
InternalAccount(AccountId),
}
/// 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,
/// The Sapling value pool
Sapling,
}
pub struct SentTransactionOutput {
/// The value in which the output has been created
pub output_pool: PoolType,
/// The index within the transaction that contains the recipient output.
///
/// - If `recipient_address` is a Sapling address, this is an index into the Sapling
@ -268,8 +280,12 @@ pub struct SentTransactionOutput {
/// - If `recipient_address` is a transparent address, this is an index into the
/// transparent outputs of the transaction.
pub output_index: usize,
/// The recipient address of the transaction, or the account
/// id for wallet-internal transactions.
pub recipient: Recipient,
/// The value of the newly created output
pub value: Amount,
/// The memo that was attached to the output, if any
pub memo: Option<MemoBytes>,
}

View File

@ -23,8 +23,8 @@ use {
use crate::{
address::RecipientAddress,
data_api::{
error::Error, DecryptedTransaction, Recipient, SentTransaction, SentTransactionOutput,
WalletWrite,
error::Error, DecryptedTransaction, PoolType, Recipient, SentTransaction,
SentTransactionOutput, WalletWrite,
},
decrypt_transaction,
wallet::OvkPolicy,
@ -374,14 +374,17 @@ where
let (tx, tx_metadata) = builder.build(&prover).map_err(Error::Builder)?;
let sent_outputs = request.payments().iter().enumerate().map(|(i, payment)| {
let idx = match &payment.recipient_address {
let (output_index, output_pool) = match &payment.recipient_address {
// Sapling outputs are shuffled, so we need to look up where the output ended up.
// TODO: When we add Orchard support, we will need to trial-decrypt to find them.
RecipientAddress::Shielded(_) | RecipientAddress::Unified(_) =>
tx_metadata.output_index(i).expect("An output should exist in the transaction for each shielded payment."),
// TODO: When we add Orchard support, we will need to trial-decrypt to find them,
// and return the appropriate pool type.
RecipientAddress::Shielded(_) | RecipientAddress::Unified(_) => {
let idx = tx_metadata.output_index(i).expect("An output should exist in the transaction for each shielded payment.");
(idx, PoolType::Sapling)
}
RecipientAddress::Transparent(addr) => {
let script = addr.script();
tx.transparent_bundle()
let idx = tx.transparent_bundle()
.and_then(|b| {
b.vout
.iter()
@ -389,12 +392,15 @@ where
.find(|(_, tx_out)| tx_out.script_pubkey == script)
})
.map(|(index, _)| index)
.expect("An output should exist in the transaction for each transparent payment.")
.expect("An output should exist in the transaction for each transparent payment.");
(idx, PoolType::Transparent)
}
};
SentTransactionOutput {
output_index: idx,
output_pool,
output_index,
recipient: Recipient::Address(payment.recipient_address.clone()),
value: payment.amount,
memo: payment.memo.clone()
@ -526,6 +532,7 @@ where
created: time::OffsetDateTime::now_utc(),
account,
outputs: vec![SentTransactionOutput {
output_pool: PoolType::Sapling,
output_index,
value: amount_to_shield,
recipient: Recipient::InternalAccount(account),

View File

@ -89,12 +89,15 @@ and this library adheres to Rust's notion of
migration process.
### Removed
- `zcash_client_sqlite::wallet`:
- `get_extended_full_viewing_keys` (use
`zcash_client_backend::data_api::WalletRead::get_unified_full_viewing_keys`
instead).
- `delete_utxos_above` (use
`zcash_client_backend::data_api::WalletWrite::rewind_to_height` instead)
- The following functions have been removed from the public interface of
`zcash_client_sqlite::wallet`. Prefer methods defined on
`zcash_client_backend::data_api::{WalletRead, WalletWrite}` instead.
- `get_extended_full_viewing_keys` (use `WalletRead::get_unified_full_viewing_keys` instead).
- `insert_sent_note` (use `WalletWrite::store_sent_tx` instead)
- `insert_sent_utxo` (use `WalletWrite::store_sent_tx` instead)
- `put_sent_note` (use `WalletWrite::store_decrypted_tx` instead)
- `put_sent_utxo` (use `WalletWrite::store_decrypted_tx` instead)
- `delete_utxos_above` (use `WalletWrite::rewind_to_height` instead)
- `zcash_client_sqlite::with_blocks` (use
`zcash_client_backend::data_api::BlockSource::with_blocks` instead)
@ -131,7 +134,6 @@ and this library adheres to Rust's notion of
- `insert_witness`
- `prune_witnesses`
- `update_expired_notes`
- `put_sent_note`
- `put_sent_utxo`
- `insert_sent_note`
- `insert_sent_utxo`

View File

@ -55,8 +55,8 @@ use zcash_primitives::{
use zcash_client_backend::{
address::{RecipientAddress, UnifiedAddress},
data_api::{
BlockSource, DecryptedTransaction, PrunedBlock, Recipient, SentTransaction, WalletRead,
WalletWrite,
BlockSource, DecryptedTransaction, PoolType, PrunedBlock, Recipient, SentTransaction,
WalletRead, WalletWrite,
},
keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
proto::compact_formats::CompactBlock,
@ -539,15 +539,15 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
Recipient::InternalAccount(output.account)
};
wallet::put_sent_note(
wallet::put_sent_output(
up,
tx_ref,
output.index,
output.account,
tx_ref,
PoolType::Sapling,
output.index,
&recipient,
Amount::from_u64(output.note.value)
.map_err(|_| SqliteClientError::CorruptedData("Note value invalid.".to_string()))?,
Some(&output.memo),
Amount::from_u64(output.note.value).unwrap(),
Some(&output.memo)
)?;
}
TransferType::Incoming => {
@ -577,13 +577,16 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
.any(|input| *nf == input.nullifier)
) {
for (output_index, txout) in d_tx.tx.transparent_bundle().iter().flat_map(|b| b.vout.iter()).enumerate() {
wallet::put_sent_utxo(
let recipient = Recipient::Address(RecipientAddress::Transparent(txout.script_pubkey.address().unwrap()));
wallet::put_sent_output(
up,
tx_ref,
output_index,
*account_id,
&txout.script_pubkey.address().unwrap(),
tx_ref,
PoolType::Transparent,
output_index,
&recipient,
txout.value,
None
)?;
}
}
@ -622,27 +625,7 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
}
for output in &sent_tx.outputs {
match &output.recipient {
Recipient::Address(RecipientAddress::Transparent(addr)) => {
wallet::insert_sent_utxo(
up,
tx_ref,
output.output_index,
sent_tx.account,
addr,
output.value,
)?
}
shielded_recipient => wallet::insert_sent_note(
up,
tx_ref,
output.output_index,
sent_tx.account,
shielded_recipient,
output.value,
output.memo.as_ref(),
)?,
}
wallet::insert_sent_output(up, tx_ref, sent_tx.account, output)?;
}
// Return the row number of the transaction, so the caller can fetch it for sending.

View File

@ -18,9 +18,12 @@ use zcash_primitives::{
zip32::{AccountId, DiversifierIndex},
};
use zcash_client_backend::{address::UnifiedAddress, data_api::Recipient};
use zcash_client_backend::{
address::UnifiedAddress,
data_api::{PoolType, Recipient},
};
use crate::{error::SqliteClientError, wallet::PoolType, NoteId, WalletDb};
use crate::{error::SqliteClientError, wallet::pool_code, NoteId, WalletDb};
#[cfg(feature = "transparent-inputs")]
use {
@ -60,8 +63,8 @@ pub struct DataConnStmtCache<'a, P> {
stmt_update_received_note: Statement<'a>,
stmt_select_received_note: Statement<'a>,
stmt_insert_sent_note: Statement<'a>,
stmt_update_sent_note: Statement<'a>,
stmt_insert_sent_output: Statement<'a>,
stmt_update_sent_output: Statement<'a>,
stmt_insert_witness: Statement<'a>,
stmt_prune_witnesses: Statement<'a>,
@ -152,9 +155,9 @@ impl<'a, P> DataConnStmtCache<'a, P> {
stmt_select_received_note: wallet_db.conn.prepare(
"SELECT id_note FROM received_notes WHERE tx = ? AND output_index = ?"
)?,
stmt_update_sent_note: wallet_db.conn.prepare(
stmt_update_sent_output: wallet_db.conn.prepare(
"UPDATE sent_notes
SET from_account = :account,
SET from_account = :from_account,
to_address = :to_address,
to_account = :to_account,
value = :value,
@ -163,7 +166,7 @@ impl<'a, P> DataConnStmtCache<'a, P> {
AND output_pool = :output_pool
AND output_index = :output_index",
)?,
stmt_insert_sent_note: wallet_db.conn.prepare(
stmt_insert_sent_output: wallet_db.conn.prepare(
"INSERT INTO sent_notes (
tx, output_pool, output_index, from_account,
to_address, to_account, value, memo)
@ -362,7 +365,7 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> {
/// - If `to` is an internal account, this is an index into the Sapling outputs of the
/// transaction.
#[allow(clippy::too_many_arguments)]
pub(crate) fn stmt_insert_sent_note(
pub(crate) fn stmt_insert_sent_output(
&mut self,
tx_ref: i64,
pool_type: PoolType,
@ -373,13 +376,13 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> {
memo: Option<&MemoBytes>,
) -> Result<(), SqliteClientError> {
let (to_address, to_account) = match to {
Recipient::Address(addr) => (Some(addr.encode(&self.wallet_db.params)), None),
Recipient::InternalAccount(id) => (None, Some(u32::from(*id))),
Recipient::Address(shielded) => (Some(shielded.encode(&self.wallet_db.params)), None),
};
self.stmt_insert_sent_note.execute(named_params![
self.stmt_insert_sent_output.execute(named_params![
":tx": &tx_ref,
":output_pool": &pool_type.typecode(),
":output_pool": &pool_code(pool_type),
":output_index": &i64::try_from(output_index).unwrap(),
":from_account": &u32::from(from_account),
":to_address": &to_address,
@ -395,9 +398,9 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> {
///
/// Returns `false` if the transaction doesn't exist in the wallet.
#[allow(clippy::too_many_arguments)]
pub(crate) fn stmt_update_sent_note(
pub(crate) fn stmt_update_sent_output(
&mut self,
account: AccountId,
from_account: AccountId,
to: &Recipient,
value: Amount,
memo: Option<&MemoBytes>,
@ -406,17 +409,17 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> {
output_index: usize,
) -> Result<bool, SqliteClientError> {
let (to_address, to_account) = match to {
Recipient::Address(addr) => (Some(addr.encode(&self.wallet_db.params)), None),
Recipient::InternalAccount(id) => (None, Some(u32::from(*id))),
Recipient::Address(shielded) => (Some(shielded.encode(&self.wallet_db.params)), None),
};
match self.stmt_update_sent_note.execute(named_params![
":account": &u32::from(account),
match self.stmt_update_sent_output.execute(named_params![
":from_account": &u32::from(from_account),
":to_address": &to_address,
":to_account": &to_account,
":value": &i64::from(value),
":memo": &memo.filter(|m| *m != &MemoBytes::empty()).map(|m| m.as_slice()),
":tx": &tx_ref,
":output_pool": &pool_type.typecode(),
":output_pool": &pool_code(pool_type),
":output_index": &i64::try_from(output_index).unwrap(),
])? {
0 => Ok(false),

View File

@ -27,7 +27,7 @@ use zcash_primitives::{
use zcash_client_backend::{
address::{RecipientAddress, UnifiedAddress},
data_api::{error::Error, Recipient},
data_api::{error::Error, PoolType, Recipient, SentTransactionOutput},
keys::UnifiedFullViewingKey,
wallet::{WalletShieldedOutput, WalletTx},
DecryptedOutput,
@ -35,8 +35,6 @@ use zcash_client_backend::{
use crate::{error::SqliteClientError, DataConnStmtCache, NoteId, WalletDb, PRUNING_HEIGHT};
use zcash_primitives::legacy::TransparentAddress;
#[cfg(feature = "transparent-inputs")]
use {
crate::UtxoId,
@ -44,7 +42,7 @@ use {
std::collections::HashSet,
zcash_client_backend::{encoding::AddressCodec, wallet::WalletTransparentOutput},
zcash_primitives::{
legacy::{keys::IncomingViewingKey, Script},
legacy::{keys::IncomingViewingKey, Script, TransparentAddress},
transaction::components::{OutPoint, TxOut},
},
};
@ -52,20 +50,13 @@ use {
pub mod init;
pub mod transact;
pub(crate) enum PoolType {
Transparent,
Sapling,
}
impl PoolType {
pub(crate) fn typecode(&self) -> i64 {
// These constants are *incidentally* shared with the typecodes
// for unified addresses, but this is exclusively an internal
// implementation detail.
match self {
PoolType::Transparent => 0i64,
PoolType::Sapling => 2i64,
}
pub(crate) fn pool_code(pool_type: PoolType) -> i64 {
// These constants are *incidentally* shared with the typecodes
// for unified addresses, but this is exclusively an internal
// implementation detail.
match pool_type {
PoolType::Transparent => 0i64,
PoolType::Sapling => 2i64,
}
}
@ -1152,121 +1143,61 @@ pub fn update_expired_notes<P>(
stmts.stmt_update_expired(height)
}
/// Records information about a note that your wallet created.
#[deprecated(
note = "This method will be removed in a future update. Use zcash_client_backend::data_api::WalletWrite::store_decrypted_tx instead."
)]
#[allow(deprecated)]
pub fn put_sent_note<'a, P: consensus::Parameters>(
/// Records information about a transaction output that your wallet created.
///
/// This is a crate-internal convenience method.
pub(crate) fn insert_sent_output<'a, P: consensus::Parameters>(
stmts: &mut DataConnStmtCache<'a, P>,
tx_ref: i64,
output_index: usize,
account: AccountId,
to: &Recipient,
value: Amount,
memo: Option<&MemoBytes>,
) -> Result<(), SqliteClientError> {
// Try updating an existing sent note.
if !stmts.stmt_update_sent_note(
account,
to,
value,
memo,
tx_ref,
PoolType::Sapling,
output_index,
)? {
// It isn't there, so insert.
insert_sent_note(stmts, tx_ref, output_index, account, to, value, memo)?
}
Ok(())
}
/// Adds information about a sent transparent UTXO to the database if it does not already
/// exist, or updates it if a record for the UTXO already exists.
///
/// `output_index` is the index within transparent UTXOs of the transaction that contains the recipient output.
#[deprecated(
note = "This method will be removed in a future update. Use zcash_client_backend::data_api::WalletWrite::store_decrypted_tx instead."
)]
#[allow(deprecated)]
pub fn put_sent_utxo<'a, P: consensus::Parameters>(
stmts: &mut DataConnStmtCache<'a, P>,
tx_ref: i64,
output_index: usize,
account: AccountId,
to: &TransparentAddress,
value: Amount,
) -> Result<(), SqliteClientError> {
// Try updating an existing sent UTXO.
if !stmts.stmt_update_sent_note(
account,
&Recipient::Address(RecipientAddress::Transparent(*to)),
value,
None,
tx_ref,
PoolType::Transparent,
output_index,
)? {
// It isn't there, so insert.
insert_sent_utxo(stmts, tx_ref, output_index, account, to, value)?
}
Ok(())
}
/// Inserts a sent note into the wallet database.
///
/// `output_index` is the index within the transaction that contains the recipient output:
///
/// - If `to` is a Sapling address, this is an index into the Sapling outputs of the
/// transaction.
/// - If `to` is a transparent address, this is an index into the transparent outputs of
/// the transaction.
pub(crate) fn insert_sent_note<'a, P: consensus::Parameters>(
stmts: &mut DataConnStmtCache<'a, P>,
tx_ref: i64,
output_index: usize,
from_account: AccountId,
to: &Recipient,
value: Amount,
memo: Option<&MemoBytes>,
output: &SentTransactionOutput,
) -> Result<(), SqliteClientError> {
stmts.stmt_insert_sent_note(
stmts.stmt_insert_sent_output(
tx_ref,
PoolType::Sapling,
output_index,
output.output_pool,
output.output_index,
from_account,
to,
value,
memo,
&output.recipient,
output.value,
output.memo.as_ref(),
)
}
/// Inserts information about a sent transparent UTXO into the wallet database.
/// Records information about a transaction output that your wallet created.
///
/// `output_index` is the index within transparent UTXOs of the transaction that contains the recipient output.
#[deprecated(
note = "This method will be removed in a future update. Use zcash_client_backend::data_api::WalletWrite::store_sent_tx instead."
)]
pub fn insert_sent_utxo<'a, P: consensus::Parameters>(
/// This is a crate-internal convenience method.
#[allow(clippy::too_many_arguments)]
pub(crate) fn put_sent_output<'a, P: consensus::Parameters>(
stmts: &mut DataConnStmtCache<'a, P>,
from_account: AccountId,
tx_ref: i64,
output_pool: PoolType,
output_index: usize,
account: AccountId,
to: &TransparentAddress,
recipient: &Recipient,
value: Amount,
memo: Option<&MemoBytes>,
) -> Result<(), SqliteClientError> {
stmts.stmt_insert_sent_note(
tx_ref,
PoolType::Transparent,
output_index,
account,
&Recipient::Address(RecipientAddress::Transparent(*to)),
if !stmts.stmt_update_sent_output(
from_account,
recipient,
value,
None,
)
memo,
tx_ref,
output_pool,
output_index,
)? {
stmts.stmt_insert_sent_output(
tx_ref,
output_pool,
output_index,
from_account,
recipient,
value,
memo,
)?;
}
Ok(())
}
#[cfg(test)]

View File

@ -309,7 +309,7 @@ mod tests {
use crate::{
error::SqliteClientError,
tests::{self, network},
wallet::get_address,
wallet::{get_address, pool_code},
AccountId, WalletDb,
};
@ -387,7 +387,11 @@ mod tests {
FOREIGN KEY (tx) REFERENCES transactions(id_tx),
FOREIGN KEY (from_account) REFERENCES accounts(account),
FOREIGN KEY (to_account) REFERENCES accounts(account),
CONSTRAINT tx_output UNIQUE (tx, output_pool, output_index)
CONSTRAINT tx_output UNIQUE (tx, output_pool, output_index),
CONSTRAINT note_recipient CHECK (
(to_address IS NOT NULL OR to_account IS NOT NULL)
AND NOT (to_address IS NOT NULL AND to_account IS NOT NULL)
)
)",
"CREATE TABLE transactions (
id_tx INTEGER PRIMARY KEY,
@ -950,7 +954,7 @@ mod tests {
wdb.conn.execute(
"INSERT INTO sent_notes (tx, output_pool, output_index, from_account, address, value)
VALUES (0, ?, 0, ?, ?, 0)",
[PoolType::Transparent.typecode().to_sql()?, u32::from(account).to_sql()?, taddr.to_sql()?])?;
[pool_code(PoolType::Transparent).to_sql()?, u32::from(account).to_sql()?, taddr.to_sql()?])?;
}
Ok(())

View File

@ -42,10 +42,8 @@ impl RusqliteMigration for Migration {
type Error = WalletMigrationError;
fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> {
transaction.execute_batch("ALTER TABLE sent_notes ADD COLUMN to_account INTEGER;")?;
// `to_account` should be null for all migrated rows, since internal addresses
// have not been used for change or shielding prior to this migration.
// Adds the `to_account` column to the `sent_notes` table and establishes the
// foreign key relationship with the `account` table.
transaction.execute_batch(
"CREATE TABLE sent_notes_new (
id_note INTEGER PRIMARY KEY,
@ -60,7 +58,11 @@ impl RusqliteMigration for Migration {
FOREIGN KEY (tx) REFERENCES transactions(id_tx),
FOREIGN KEY (from_account) REFERENCES accounts(account),
FOREIGN KEY (to_account) REFERENCES accounts(account),
CONSTRAINT tx_output UNIQUE (tx, output_pool, output_index)
CONSTRAINT tx_output UNIQUE (tx, output_pool, output_index),
CONSTRAINT note_recipient CHECK (
(to_address IS NOT NULL OR to_account IS NOT NULL)
AND NOT (to_address IS NOT NULL AND to_account IS NOT NULL)
)
);
INSERT INTO sent_notes_new (
id_note, tx, output_pool, output_index,

View File

@ -7,7 +7,9 @@ use schemer_rusqlite::RusqliteMigration;
use secrecy::{ExposeSecret, SecretVec};
use uuid::Uuid;
use zcash_client_backend::{address::RecipientAddress, keys::UnifiedSpendingKey};
use zcash_client_backend::{
address::RecipientAddress, data_api::PoolType, keys::UnifiedSpendingKey,
};
use zcash_primitives::{consensus, zip32::AccountId};
#[cfg(feature = "transparent-inputs")]
@ -18,7 +20,7 @@ use zcash_client_backend::encoding::AddressCodec;
use crate::wallet::{
init::{migrations::utxos_table, WalletMigrationError},
PoolType,
pool_code,
};
pub(super) const MIGRATION_ID: Uuid = Uuid::from_fields(
@ -227,8 +229,8 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
))
})?;
let output_pool = match decoded_address {
RecipientAddress::Shielded(_) => Ok(PoolType::Sapling.typecode()),
RecipientAddress::Transparent(_) => Ok(PoolType::Transparent.typecode()),
RecipientAddress::Shielded(_) => Ok(pool_code(PoolType::Sapling)),
RecipientAddress::Transparent(_) => Ok(pool_code(PoolType::Transparent)),
RecipientAddress::Unified(_) => Err(WalletMigrationError::CorruptedData(
"Unified addresses should not yet appear in the sent_notes table."
.to_string(),