diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 38ab33404..49e2b79ee 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -435,7 +435,8 @@ mod tests { "CREATE VIEW v_transactions AS WITH notes AS ( - SELECT sapling_received_notes.account AS account_id, + SELECT sapling_received_notes.id_note AS id, + sapling_received_notes.account AS account_id, transactions.block AS block, transactions.txid AS txid, 2 AS pool, @@ -457,7 +458,8 @@ mod tests { JOIN transactions ON transactions.id_tx = sapling_received_notes.tx UNION - SELECT utxos.received_by_account AS account_id, + SELECT utxos.id_utxo AS id, + utxos.received_by_account AS account_id, utxos.height AS block, utxos.prevout_txid AS txid, 0 AS pool, @@ -467,7 +469,8 @@ mod tests { 0 AS memo_present FROM utxos UNION - SELECT sapling_received_notes.account AS account_id, + SELECT sapling_received_notes.id_note AS id, + sapling_received_notes.account AS account_id, transactions.block AS block, transactions.txid AS txid, 2 AS pool, @@ -479,7 +482,8 @@ mod tests { JOIN transactions ON transactions.id_tx = sapling_received_notes.spent UNION - SELECT utxos.received_by_account AS account_id, + SELECT utxos.id_utxo AS id, + utxos.received_by_account AS account_id, transactions.block AS block, transactions.txid AS txid, 0 AS pool, diff --git a/zcash_client_sqlite/src/wallet/init/migrations.rs b/zcash_client_sqlite/src/wallet/init/migrations.rs index db8ee2194..4c1486be5 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations.rs @@ -12,6 +12,7 @@ mod ufvk_support; mod utxos_table; mod v_sapling_shard_unscanned_ranges; mod v_transactions_net; +mod v_transactions_note_uniqueness; mod v_transactions_shielding_balance; mod v_transactions_transparent_history; mod v_tx_outputs_use_legacy_false; @@ -48,6 +49,8 @@ pub(super) fn all_migrations( // v_sapling_shard_unscanned_ranges v_tx_outputs_use_legacy_false // | | // wallet_summaries v_transactions_shielding_balance + // | + // v_transactions_note_uniqueness vec![ Box::new(initial_setup::Migration {}), Box::new(utxos_table::Migration {}), @@ -82,5 +85,6 @@ pub(super) fn all_migrations( Box::new(v_transactions_transparent_history::Migration), Box::new(v_tx_outputs_use_legacy_false::Migration), Box::new(v_transactions_shielding_balance::Migration), + Box::new(v_transactions_note_uniqueness::Migration), ] } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_note_uniqueness.rs b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_note_uniqueness.rs new file mode 100644 index 000000000..3636ac93f --- /dev/null +++ b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_note_uniqueness.rs @@ -0,0 +1,250 @@ +//! This migration fixes a bug in `v_transactions` where distinct but otherwise identical notes +//! were being incorrectly deduplicated. + +use std::collections::HashSet; + +use schemer_rusqlite::RusqliteMigration; +use uuid::Uuid; + +use crate::wallet::init::WalletMigrationError; + +use super::v_transactions_shielding_balance; + +pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0xdba47c86_13b5_4601_94b2_0cde0abe1e45); + +pub(super) struct Migration; + +impl schemer::Migration for Migration { + fn id(&self) -> Uuid { + MIGRATION_ID + } + + fn dependencies(&self) -> HashSet { + [v_transactions_shielding_balance::MIGRATION_ID] + .into_iter() + .collect() + } + + fn description(&self) -> &'static str { + "Fixes a bug in v_transactions that was omitting value from identically-valued notes." + } +} + +impl RusqliteMigration for Migration { + type Error = WalletMigrationError; + + fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), Self::Error> { + transaction.execute_batch( + "DROP VIEW v_transactions; + CREATE VIEW v_transactions AS + WITH + notes AS ( + SELECT sapling_received_notes.id_note AS id, + sapling_received_notes.account AS account_id, + transactions.block AS block, + transactions.txid AS txid, + 2 AS pool, + sapling_received_notes.value AS value, + CASE + WHEN sapling_received_notes.is_change THEN 1 + ELSE 0 + END AS is_change, + CASE + WHEN sapling_received_notes.is_change THEN 0 + ELSE 1 + END AS received_count, + CASE + WHEN (sapling_received_notes.memo IS NULL OR sapling_received_notes.memo = X'F6') + THEN 0 + ELSE 1 + END AS memo_present + FROM sapling_received_notes + JOIN transactions + ON transactions.id_tx = sapling_received_notes.tx + UNION + SELECT utxos.id_utxo AS id, + utxos.received_by_account AS account_id, + utxos.height AS block, + utxos.prevout_txid AS txid, + 0 AS pool, + utxos.value_zat AS value, + 0 AS is_change, + 1 AS received_count, + 0 AS memo_present + FROM utxos + UNION + SELECT sapling_received_notes.id_note AS id, + sapling_received_notes.account AS account_id, + transactions.block AS block, + transactions.txid AS txid, + 2 AS pool, + -sapling_received_notes.value AS value, + 0 AS is_change, + 0 AS received_count, + 0 AS memo_present + FROM sapling_received_notes + JOIN transactions + ON transactions.id_tx = sapling_received_notes.spent + UNION + SELECT utxos.id_utxo AS id, + utxos.received_by_account AS account_id, + transactions.block AS block, + transactions.txid AS txid, + 0 AS pool, + -utxos.value_zat AS value, + 0 AS is_change, + 0 AS received_count, + 0 AS memo_present + FROM utxos + JOIN transactions + ON transactions.id_tx = utxos.spent_in_tx + ), + sent_note_counts AS ( + SELECT sent_notes.from_account AS account_id, + transactions.txid AS txid, + COUNT(DISTINCT sent_notes.id_note) as sent_notes, + SUM( + CASE + WHEN (sent_notes.memo IS NULL OR sent_notes.memo = X'F6' OR sapling_received_notes.tx IS NOT NULL) + THEN 0 + ELSE 1 + END + ) AS memo_count + FROM sent_notes + JOIN transactions + ON transactions.id_tx = sent_notes.tx + LEFT JOIN sapling_received_notes + ON (sent_notes.tx, sent_notes.output_pool, sent_notes.output_index) = + (sapling_received_notes.tx, 2, sapling_received_notes.output_index) + WHERE COALESCE(sapling_received_notes.is_change, 0) = 0 + GROUP BY account_id, txid + ), + blocks_max_height AS ( + SELECT MAX(blocks.height) as max_height FROM blocks + ) + SELECT notes.account_id AS account_id, + notes.block AS mined_height, + notes.txid AS txid, + transactions.tx_index AS tx_index, + transactions.expiry_height AS expiry_height, + transactions.raw AS raw, + SUM(notes.value) AS account_balance_delta, + transactions.fee AS fee_paid, + SUM(notes.is_change) > 0 AS has_change, + MAX(COALESCE(sent_note_counts.sent_notes, 0)) AS sent_note_count, + SUM(notes.received_count) AS received_note_count, + SUM(notes.memo_present) + MAX(COALESCE(sent_note_counts.memo_count, 0)) AS memo_count, + blocks.time AS block_time, + ( + blocks.height IS NULL + AND transactions.expiry_height BETWEEN 1 AND blocks_max_height.max_height + ) AS expired_unmined + FROM notes + LEFT JOIN transactions + ON notes.txid = transactions.txid + JOIN blocks_max_height + LEFT JOIN blocks ON blocks.height = notes.block + LEFT JOIN sent_note_counts + ON sent_note_counts.account_id = notes.account_id + AND sent_note_counts.txid = notes.txid + GROUP BY notes.account_id, notes.txid;" + )?; + + Ok(()) + } + + fn down(&self, _transaction: &rusqlite::Transaction) -> Result<(), Self::Error> { + panic!("This migration cannot be reverted."); + } +} + +#[cfg(test)] +mod tests { + use rusqlite::{self, params}; + use tempfile::NamedTempFile; + + use zcash_client_backend::keys::UnifiedSpendingKey; + use zcash_primitives::{consensus::Network, zip32::AccountId}; + + use crate::{ + wallet::init::{init_wallet_db_internal, migrations::v_transactions_net}, + WalletDb, + }; + + #[test] + fn v_transactions_note_uniqueness_migration() { + let data_file = NamedTempFile::new().unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); + init_wallet_db_internal(&mut db_data, None, &[v_transactions_net::MIGRATION_ID]).unwrap(); + + // Create an account in the wallet + let usk0 = + UnifiedSpendingKey::from_seed(&db_data.params, &[0u8; 32][..], AccountId::from(0)) + .unwrap(); + let ufvk0 = usk0.to_unified_full_viewing_key(); + db_data + .conn + .execute( + "INSERT INTO accounts (account, ufvk) VALUES (0, ?)", + params![ufvk0.encode(&db_data.params)], + ) + .unwrap(); + + // Tx 0 contains two received notes, both of 2 zatoshis, that are controlled by account 0. + db_data.conn.execute_batch( + "INSERT INTO blocks (height, hash, time, sapling_tree) VALUES (0, 0, 0, x'00'); + INSERT INTO transactions (block, id_tx, txid) VALUES (0, 0, 'tx0'); + + INSERT INTO received_notes (tx, output_index, account, diversifier, value, rcm, nf, is_change) + VALUES (0, 0, 0, '', 2, '', 'nf_a', false); + INSERT INTO received_notes (tx, output_index, account, diversifier, value, rcm, nf, is_change) + VALUES (0, 3, 0, '', 2, '', 'nf_b', false);").unwrap(); + + let check_balance_delta = |db_data: &mut WalletDb, + expected_notes: i64| { + let mut q = db_data + .conn + .prepare( + "SELECT account_id, account_balance_delta, has_change, memo_count, sent_note_count, received_note_count + FROM v_transactions", + ) + .unwrap(); + let mut rows = q.query([]).unwrap(); + let mut row_count = 0; + while let Some(row) = rows.next().unwrap() { + row_count += 1; + let account: i64 = row.get(0).unwrap(); + let account_balance_delta: i64 = row.get(1).unwrap(); + let has_change: bool = row.get(2).unwrap(); + let memo_count: i64 = row.get(3).unwrap(); + let sent_note_count: i64 = row.get(4).unwrap(); + let received_note_count: i64 = row.get(5).unwrap(); + match account { + 0 => { + assert_eq!(account_balance_delta, 2 * expected_notes); + assert!(!has_change); + assert_eq!(memo_count, 0); + assert_eq!(sent_note_count, 0); + assert_eq!(received_note_count, expected_notes); + } + other => { + panic!( + "Account {:?} is not expected to exist in the wallet.", + other + ); + } + } + } + assert_eq!(row_count, 1); + }; + + // Check for the bug (#1020). + check_balance_delta(&mut db_data, 1); + + // Apply the current migration. + init_wallet_db_internal(&mut db_data, None, &[super::MIGRATION_ID]).unwrap(); + + // Now it should be correct. + check_balance_delta(&mut db_data, 2); + } +}