Use `NULL` to represent the empty memo.

We don't need to store a bunch of copies of the empty memo, and code
should not be depending upon the presence or absence of a memo to
distinguish between different states of transaction retrieval.
This commit is contained in:
Kris Nuttycombe 2022-09-10 09:09:35 -06:00
parent a93c9d334e
commit 3120b304c7
3 changed files with 71 additions and 42 deletions

View File

@ -135,12 +135,17 @@ impl<'a, P> DataConnStmtCache<'a, P> {
)?, )?,
stmt_update_sent_note: wallet_db.conn.prepare( stmt_update_sent_note: wallet_db.conn.prepare(
"UPDATE sent_notes "UPDATE sent_notes
SET from_account = ?, address = ?, value = ?, memo = ? SET from_account = :account,
WHERE tx = ? AND output_pool = ? AND output_index = ?", address = :address,
value = :value,
memo = IFNULL(:memo, memo)
WHERE tx = :tx
AND output_pool = :output_pool
AND output_index = :output_index",
)?, )?,
stmt_insert_sent_note: wallet_db.conn.prepare( stmt_insert_sent_note: wallet_db.conn.prepare(
"INSERT INTO sent_notes (tx, output_pool, output_index, from_account, address, value, memo) "INSERT INTO sent_notes (tx, output_pool, output_index, from_account, address, value, memo)
VALUES (?, ?, ?, ?, ?, ?, ?)", VALUES (:tx, :output_pool, :output_index, :from_account, :address, :value, :memo)"
)?, )?,
stmt_insert_witness: wallet_db.conn.prepare( stmt_insert_witness: wallet_db.conn.prepare(
"INSERT INTO sapling_witnesses (note, block, witness) "INSERT INTO sapling_witnesses (note, block, witness)
@ -396,7 +401,12 @@ impl<'a, P> DataConnStmtCache<'a, P> {
(":value", &(value as i64)), (":value", &(value as i64)),
(":rcm", &rcm.as_ref()), (":rcm", &rcm.as_ref()),
(":nf", &nf.as_ref().map(|nf| nf.0.as_ref())), (":nf", &nf.as_ref().map(|nf| nf.0.as_ref())),
(":memo", &memo.map(|m| m.as_slice())), (
":memo",
&memo
.filter(|m| *m != &MemoBytes::empty())
.map(|m| m.as_slice()),
),
(":is_change", &is_change), (":is_change", &is_change),
]; ];
@ -433,7 +443,12 @@ impl<'a, P> DataConnStmtCache<'a, P> {
(":value", &(value as i64)), (":value", &(value as i64)),
(":rcm", &rcm.as_ref()), (":rcm", &rcm.as_ref()),
(":nf", &nf.as_ref().map(|nf| nf.0.as_ref())), (":nf", &nf.as_ref().map(|nf| nf.0.as_ref())),
(":memo", &memo.map(|m| m.as_slice())), (
":memo",
&memo
.filter(|m| *m != &MemoBytes::empty())
.map(|m| m.as_slice()),
),
(":is_change", &is_change), (":is_change", &is_change),
(":tx", &tx_ref), (":tx", &tx_ref),
(":output_index", &(output_index as i64)), (":output_index", &(output_index as i64)),
@ -478,18 +493,21 @@ impl<'a, P> DataConnStmtCache<'a, P> {
value: Amount, value: Amount,
memo: Option<&MemoBytes>, memo: Option<&MemoBytes>,
) -> Result<(), SqliteClientError> { ) -> Result<(), SqliteClientError> {
let ivalue: i64 = value.into(); let sql_args: &[(&str, &dyn ToSql)] = &[
self.stmt_insert_sent_note.execute(params![ (":tx", &tx_ref),
tx_ref, (":output_pool", &pool_type.typecode()),
pool_type.typecode(), (":output_index", &i64::try_from(output_index).unwrap()),
(output_index as i64), (":from_account", &u32::from(account)),
u32::from(account), (":address", &to_str),
to_str, (":value", &i64::from(value)),
ivalue, (
memo.filter(|m| *m != &MemoBytes::empty()) ":memo",
.map(|m| m.as_slice()), &memo
])?; .filter(|m| *m != &MemoBytes::empty())
.map(|m| m.as_slice()),
),
];
self.stmt_insert_sent_note.execute_named(sql_args)?;
Ok(()) Ok(())
} }
@ -507,17 +525,21 @@ impl<'a, P> DataConnStmtCache<'a, P> {
pool_type: PoolType, pool_type: PoolType,
output_index: usize, output_index: usize,
) -> Result<bool, SqliteClientError> { ) -> Result<bool, SqliteClientError> {
let ivalue: i64 = value.into(); let sql_args: &[(&str, &dyn ToSql)] = &[
match self.stmt_update_sent_note.execute(params![ (":account", &u32::from(account)),
u32::from(account), (":address", &to_str),
to_str, (":value", &i64::from(value)),
ivalue, (
memo.filter(|m| *m != &MemoBytes::empty()) ":memo",
.map(|m| m.as_slice()), &memo
tx_ref, .filter(|m| *m != &MemoBytes::empty())
pool_type.typecode(), .map(|m| m.as_slice()),
output_index as i64, ),
])? { (":tx", &tx_ref),
(":output_pool", &pool_type.typecode()),
(":output_index", &i64::try_from(output_index).unwrap()),
];
match self.stmt_update_sent_note.execute_named(sql_args)? {
0 => Ok(false), 0 => Ok(false),
1 => Ok(true), 1 => Ok(true),
_ => unreachable!("tx_output constraint is marked as UNIQUE"), _ => unreachable!("tx_output constraint is marked as UNIQUE"),

View File

@ -849,7 +849,6 @@ mod tests {
received_notes.is_change AS is_change, received_notes.is_change AS is_change,
CASE CASE
WHEN received_notes.memo IS NULL THEN 0 WHEN received_notes.memo IS NULL THEN 0
WHEN received_notes.memo = X'F600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' THEN 0
ELSE 1 ELSE 1
END AS memo_present END AS memo_present
FROM transactions FROM transactions
@ -866,7 +865,6 @@ mod tests {
false AS is_change, false AS is_change,
CASE CASE
WHEN sent_notes.memo IS NULL THEN 0 WHEN sent_notes.memo IS NULL THEN 0
WHEN sent_notes.memo = X'F600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' THEN 0
ELSE 1 ELSE 1
END AS memo_present END AS memo_present
FROM transactions FROM transactions
@ -883,7 +881,6 @@ mod tests {
SUM( SUM(
CASE CASE
WHEN received_notes.memo IS NULL THEN 0 WHEN received_notes.memo IS NULL THEN 0
WHEN received_notes.memo = X'F600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' THEN 0
ELSE 1 ELSE 1
END END
) AS memo_count, ) AS memo_count,
@ -906,7 +903,6 @@ mod tests {
SUM( SUM(
CASE CASE
WHEN sent_notes.memo IS NULL THEN 0 WHEN sent_notes.memo IS NULL THEN 0
WHEN sent_notes.memo = X'F600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' THEN 0
ELSE 1 ELSE 1
END END
) AS memo_count, ) AS memo_count,

View File

@ -102,6 +102,12 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
} }
} }
transaction.execute_batch(
"UPDATE sent_notes SET memo = NULL
WHERE memo = X'F600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000';
UPDATE received_notes SET memo = NULL
WHERE memo = X'F600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000';")?;
transaction.execute_batch( transaction.execute_batch(
"CREATE VIEW v_tx_sent AS "CREATE VIEW v_tx_sent AS
SELECT transactions.id_tx AS id_tx, SELECT transactions.id_tx AS id_tx,
@ -115,7 +121,6 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
SUM( SUM(
CASE CASE
WHEN sent_notes.memo IS NULL THEN 0 WHEN sent_notes.memo IS NULL THEN 0
WHEN sent_notes.memo = X'F600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' THEN 0
ELSE 1 ELSE 1
END END
) AS memo_count, ) AS memo_count,
@ -136,7 +141,6 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
SUM( SUM(
CASE CASE
WHEN received_notes.memo IS NULL THEN 0 WHEN received_notes.memo IS NULL THEN 0
WHEN received_notes.memo = X'F600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' THEN 0
ELSE 1 ELSE 1
END END
) AS memo_count, ) AS memo_count,
@ -172,7 +176,6 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
received_notes.is_change AS is_change, received_notes.is_change AS is_change,
CASE CASE
WHEN received_notes.memo IS NULL THEN 0 WHEN received_notes.memo IS NULL THEN 0
WHEN received_notes.memo = X'F600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' THEN 0
ELSE 1 ELSE 1
END AS memo_present END AS memo_present
FROM transactions FROM transactions
@ -189,7 +192,6 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
false AS is_change, false AS is_change,
CASE CASE
WHEN sent_notes.memo IS NULL THEN 0 WHEN sent_notes.memo IS NULL THEN 0
WHEN sent_notes.memo = X'F600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' THEN 0
ELSE 1 ELSE 1
END AS memo_present END AS memo_present
FROM transactions FROM transactions
@ -214,7 +216,7 @@ mod tests {
#[cfg(feature = "transparent-inputs")] #[cfg(feature = "transparent-inputs")]
use { use {
crate::wallet::init::{init_wallet_db_internal, WalletMigration2}, crate::wallet::init::WalletMigration2,
rusqlite::params, rusqlite::params,
zcash_client_backend::{encoding::AddressCodec, keys::UnifiedSpendingKey}, zcash_client_backend::{encoding::AddressCodec, keys::UnifiedSpendingKey},
zcash_primitives::{ zcash_primitives::{
@ -231,13 +233,20 @@ mod tests {
}, },
}; };
use crate::{tests, wallet::init::init_wallet_db, WalletDb}; use crate::{
tests,
wallet::init::{
init_wallet_db, init_wallet_db_internal,
migrations::addresses_table::ADDRESSES_TABLE_MIGRATION,
},
WalletDb,
};
#[test] #[test]
fn transaction_views() { fn transaction_views() {
let data_file = NamedTempFile::new().unwrap(); let data_file = NamedTempFile::new().unwrap();
let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap();
init_wallet_db(&mut db_data, None).unwrap(); init_wallet_db_internal(&mut db_data, None, Some(ADDRESSES_TABLE_MIGRATION)).unwrap();
db_data.conn.execute_batch( db_data.conn.execute_batch(
"INSERT INTO accounts (account, ufvk) VALUES (0, ''); "INSERT INTO accounts (account, ufvk) VALUES (0, '');
@ -257,6 +266,8 @@ mod tests {
VALUES (0, 1, 0, '', 7, '', 'b', true, X'63');", VALUES (0, 1, 0, '', 7, '', 'b', true, X'63');",
).unwrap(); ).unwrap();
init_wallet_db(&mut db_data, None).unwrap();
let mut q = db_data let mut q = db_data
.conn .conn
.prepare("SELECT received_total, received_note_count, memo_count FROM v_tx_received") .prepare("SELECT received_total, received_note_count, memo_count FROM v_tx_received")