From 9716617b558bec40f937fbe5c82b84ed3645e229 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 24 Jun 2024 11:40:34 -0600 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Jack Grigg Co-authored-by: Daira-Emma Hopwood --- zcash_client_sqlite/CHANGELOG.md | 1 + zcash_client_sqlite/src/wallet/db.rs | 35 ++- .../wallet/init/migrations/utxos_to_txos.rs | 240 +++++++++--------- zcash_client_sqlite/src/wallet/transparent.rs | 2 - 4 files changed, 147 insertions(+), 131 deletions(-) diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 738284fc4..6c3e8df80 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -11,6 +11,7 @@ and this library adheres to Rust's notion of - `zcash_client_sqlite::error::SqliteClientError` has changed variants: - Removed `HdwalletError`. - Added `TransparentDerivation`. +- The `block` column of the `v_transactions` view has been renamed to `mined_height`. ## [0.10.3] - 2024-04-08 diff --git a/zcash_client_sqlite/src/wallet/db.rs b/zcash_client_sqlite/src/wallet/db.rs index 27274feba..6e54eb3dc 100644 --- a/zcash_client_sqlite/src/wallet/db.rs +++ b/zcash_client_sqlite/src/wallet/db.rs @@ -609,7 +609,7 @@ notes AS ( CASE WHEN ro.is_change THEN 1 ELSE 0 - END AS is_change, + END AS change_note_count, CASE WHEN ro.is_change THEN 0 ELSE 1 @@ -630,7 +630,7 @@ notes AS ( ro.pool AS pool, id_within_pool_table, -ro.value AS value, - 0 AS is_change, + 0 AS change_note_count, 0 AS received_count, 0 AS memo_present FROM v_received_outputs ro @@ -672,7 +672,7 @@ SELECT notes.account_id AS account_id, transactions.raw AS raw, SUM(notes.value) AS account_balance_delta, transactions.fee AS fee_paid, - SUM(notes.is_change) > 0 AS has_change, + SUM(notes.change_note_count) > 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, @@ -691,8 +691,22 @@ LEFT JOIN sent_note_counts AND sent_note_counts.txid = notes.txid GROUP BY notes.account_id, notes.txid"; +/// Selects all outputs received by the wallet, plus any outputs sent from the wallet to +/// external recipients. +/// +/// This will contain: +/// * Outputs received from external recipients +/// * Outputs sent to external recipients +/// * Outputs received as part of a wallet-internal operation, including +/// both outputs received as a consequence of wallet-internal transfers +/// and as change. +/// +/// The `to_address` column will only contain an address when the recipient is +/// external. In all other cases, the recipient account id indicates the account +/// that controls the output. pub(super) const VIEW_TX_OUTPUTS: &str = " CREATE VIEW v_tx_outputs AS +-- select all outputs received by the wallet SELECT transactions.txid AS txid, ro.pool AS output_pool, ro.output_index AS output_index, @@ -705,24 +719,25 @@ SELECT transactions.txid AS txid, FROM v_received_outputs ro JOIN transactions ON transactions.id_tx = ro.transaction_id -LEFT JOIN sent_notes - ON sent_notes.id = ro.sent_note_id +-- join to the sent_notes table to obtain `from_account_id` +LEFT JOIN sent_notes ON sent_notes.id = ro.sent_note_id UNION +-- select all outputs sent from the wallet to external recipients SELECT transactions.txid AS txid, sent_notes.output_pool AS output_pool, sent_notes.output_index AS output_index, sent_notes.from_account_id AS from_account_id, - ro.account_id AS to_account_id, + NULL AS to_account_id, sent_notes.to_address AS to_address, sent_notes.value AS value, - 0 AS is_change, + FALSE AS is_change, sent_notes.memo AS memo FROM sent_notes JOIN transactions ON transactions.id_tx = sent_notes.tx -LEFT JOIN v_received_outputs ro - ON sent_notes.id = ro.sent_note_id -WHERE COALESCE(ro.is_change, 0) = 0"; +LEFT JOIN v_received_outputs ro ON ro.sent_note_id = sent_notes.id +-- exclude any sent notes for which a row exists in the v_received_outputs view +WHERE ro.account_id IS NULL"; pub(super) fn view_sapling_shard_scan_ranges(params: &P) -> String { format!( diff --git a/zcash_client_sqlite/src/wallet/init/migrations/utxos_to_txos.rs b/zcash_client_sqlite/src/wallet/init/migrations/utxos_to_txos.rs index 1d2680b65..b1ccbbd7f 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/utxos_to_txos.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/utxos_to_txos.rs @@ -188,131 +188,133 @@ impl RusqliteMigration for Migration { FROM transparent_received_output_spends; CREATE VIEW v_transactions AS - WITH - notes AS ( - -- Outputs received in this transaction - SELECT ro.account_id AS account_id, - transactions.mined_height AS mined_height, - transactions.txid AS txid, - ro.pool AS pool, - id_within_pool_table, - ro.value AS value, - CASE - WHEN ro.is_change THEN 1 - ELSE 0 - END AS is_change, - CASE - WHEN ro.is_change THEN 0 - ELSE 1 - END AS received_count, - CASE - WHEN (ro.memo IS NULL OR ro.memo = X'F6') - THEN 0 - ELSE 1 - END AS memo_present - FROM v_received_outputs ro - JOIN transactions - ON transactions.id_tx = ro.transaction_id - UNION - -- Outputs spent in this transaction - SELECT ro.account_id AS account_id, - transactions.mined_height AS mined_height, - transactions.txid AS txid, - ro.pool AS pool, - id_within_pool_table, - -ro.value AS value, - 0 AS is_change, - 0 AS received_count, - 0 AS memo_present - FROM v_received_outputs ro - JOIN v_received_output_spends ros - ON ros.pool = ro.pool - AND ros.received_output_id = ro.id_within_pool_table - JOIN transactions - ON transactions.id_tx = ro.transaction_id - ), - -- Obtain a count of the notes that the wallet created in each transaction, - -- not counting change notes. - sent_note_counts AS ( - SELECT sent_notes.from_account_id AS account_id, - transactions.txid AS txid, - COUNT(DISTINCT sent_notes.id) AS sent_notes, - SUM( - CASE - WHEN (sent_notes.memo IS NULL OR sent_notes.memo = X'F6' OR ro.transaction_id 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 v_received_outputs ro - ON sent_notes.id = ro.sent_note_id - WHERE COALESCE(ro.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.mined_height 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.mined_height - 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; - - CREATE VIEW v_tx_outputs AS - SELECT transactions.txid AS txid, - ro.pool AS output_pool, - ro.output_index AS output_index, - sent_notes.from_account_id AS from_account_id, - ro.account_id AS to_account_id, - NULL AS to_address, - ro.value AS value, - ro.is_change AS is_change, - ro.memo AS memo + WITH + notes AS ( + -- Outputs received in this transaction + SELECT ro.account_id AS account_id, + transactions.mined_height AS mined_height, + transactions.txid AS txid, + ro.pool AS pool, + id_within_pool_table, + ro.value AS value, + CASE + WHEN ro.is_change THEN 1 + ELSE 0 + END AS change_note_count, + CASE + WHEN ro.is_change THEN 0 + ELSE 1 + END AS received_count, + CASE + WHEN (ro.memo IS NULL OR ro.memo = X'F6') + THEN 0 + ELSE 1 + END AS memo_present FROM v_received_outputs ro JOIN transactions - ON transactions.id_tx = ro.transaction_id - LEFT JOIN sent_notes - ON sent_notes.id = ro.sent_note_id + ON transactions.id_tx = ro.transaction_id UNION - SELECT transactions.txid AS txid, - sent_notes.output_pool AS output_pool, - sent_notes.output_index AS output_index, - sent_notes.from_account_id AS from_account_id, - ro.account_id AS to_account_id, - sent_notes.to_address AS to_address, - sent_notes.value AS value, - 0 AS is_change, - sent_notes.memo AS memo + -- Outputs spent in this transaction + SELECT ro.account_id AS account_id, + transactions.mined_height AS mined_height, + transactions.txid AS txid, + ro.pool AS pool, + id_within_pool_table, + -ro.value AS value, + 0 AS change_note_count, + 0 AS received_count, + 0 AS memo_present + FROM v_received_outputs ro + JOIN v_received_output_spends ros + ON ros.pool = ro.pool + AND ros.received_output_id = ro.id_within_pool_table + JOIN transactions + ON transactions.id_tx = ro.transaction_id + ), + -- Obtain a count of the notes that the wallet created in each transaction, + -- not counting change notes. + sent_note_counts AS ( + SELECT sent_notes.from_account_id AS account_id, + transactions.txid AS txid, + COUNT(DISTINCT sent_notes.id) AS sent_notes, + SUM( + CASE + WHEN (sent_notes.memo IS NULL OR sent_notes.memo = X'F6' OR ro.transaction_id IS NOT NULL) + THEN 0 + ELSE 1 + END + ) AS memo_count FROM sent_notes JOIN transactions - ON transactions.id_tx = sent_notes.tx + ON transactions.id_tx = sent_notes.tx LEFT JOIN v_received_outputs ro - ON sent_notes.id = ro.sent_note_id - WHERE COALESCE(ro.is_change, 0) = 0; + ON sent_notes.id = ro.sent_note_id + WHERE COALESCE(ro.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.mined_height 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.change_note_count) > 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.mined_height + 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; + + CREATE VIEW v_tx_outputs AS + -- select all outputs received by the wallet + SELECT transactions.txid AS txid, + ro.pool AS output_pool, + ro.output_index AS output_index, + sent_notes.from_account_id AS from_account_id, + ro.account_id AS to_account_id, + NULL AS to_address, + ro.value AS value, + ro.is_change AS is_change, + ro.memo AS memo + FROM v_received_outputs ro + JOIN transactions + ON transactions.id_tx = ro.transaction_id + -- join to the sent_notes table to obtain `from_account_id` + LEFT JOIN sent_notes ON sent_notes.id = ro.sent_note_id + UNION + -- select all outputs sent from the wallet to external recipients + SELECT transactions.txid AS txid, + sent_notes.output_pool AS output_pool, + sent_notes.output_index AS output_index, + sent_notes.from_account_id AS from_account_id, + NULL AS to_account_id, + sent_notes.to_address AS to_address, + sent_notes.value AS value, + FALSE AS is_change, + sent_notes.memo AS memo + FROM sent_notes + JOIN transactions + ON transactions.id_tx = sent_notes.tx + LEFT JOIN v_received_outputs ro ON ro.sent_note_id = sent_notes.id + -- exclude any sent notes for which a row exists in the v_received_outputs view + WHERE ro.account_id IS NULL; DROP TABLE utxos; diff --git a/zcash_client_sqlite/src/wallet/transparent.rs b/zcash_client_sqlite/src/wallet/transparent.rs index c06e3620d..181bc30c8 100644 --- a/zcash_client_sqlite/src/wallet/transparent.rs +++ b/zcash_client_sqlite/src/wallet/transparent.rs @@ -365,7 +365,6 @@ pub(crate) fn add_transparent_account_balances( mempool_height: BlockHeight, account_balances: &mut HashMap, ) -> Result<(), SqliteClientError> { - let transparent_trace = tracing::info_span!("stmt_transparent_balances").entered(); let mut stmt_account_balances = conn.prepare( "SELECT u.account_id, SUM(u.value_zat) FROM transparent_received_outputs u @@ -404,7 +403,6 @@ pub(crate) fn add_transparent_account_balances( .or_insert(AccountBalance::ZERO) .add_unshielded_value(value)?; } - drop(transparent_trace); Ok(()) }