diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 45181ef33..8b51b06c8 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -1015,9 +1015,6 @@ impl WalletWrite for WalletDb })?; } - // Update now-expired transactions that didn't get mined. - wallet::update_expired_notes(wdb.conn.0, last_scanned_height)?; - wallet::scanning::scan_complete( wdb.conn.0, &wdb.params, diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index 8a0cc6b8e..f23fecf28 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -730,7 +730,7 @@ pub(crate) fn spend_fails_on_locked_notes() { value, ); } - st.scan_cached_blocks(h1 + 1, 41); + st.scan_cached_blocks(h1 + 1, 40); // Second proposal still fails assert_matches!( @@ -1927,20 +1927,20 @@ pub(crate) fn data_db_truncation() { // Scan the cache st.scan_cached_blocks(h, 2); - // Account balance should reflect both received notes + // Spendable balance should reflect both received notes assert_eq!( - st.get_total_balance(account.account_id()), + st.get_spendable_balance(account.account_id(), 1), (value + value2).unwrap() ); - // "Rewind" to height of last scanned block + // "Rewind" to height of last scanned block (this is a no-op) st.wallet_mut() .transactionally(|wdb| truncate_to_height(wdb.conn.0, &wdb.params, h + 1)) .unwrap(); - // Account balance should be unaltered + // Spendable balance should be unaltered assert_eq!( - st.get_total_balance(account.account_id()), + st.get_spendable_balance(account.account_id(), 1), (value + value2).unwrap() ); @@ -1949,15 +1949,20 @@ pub(crate) fn data_db_truncation() { .transactionally(|wdb| truncate_to_height(wdb.conn.0, &wdb.params, h)) .unwrap(); - // Account balance should only contain the first received note - assert_eq!(st.get_total_balance(account.account_id()), value); + // Spendable balance should only contain the first received note; + // the rest should be pending. + assert_eq!(st.get_spendable_balance(account.account_id(), 1), value); + assert_eq!( + st.get_pending_shielded_balance(account.account_id(), 1), + value2 + ); // Scan the cache again st.scan_cached_blocks(h, 2); // Account balance should again reflect both received notes assert_eq!( - st.get_total_balance(account.account_id()), + st.get_spendable_balance(account.account_id(), 1), (value + value2).unwrap() ); } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 4d7e01b23..9967b1eab 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1128,12 +1128,20 @@ pub(crate) fn get_wallet_summary( LEFT OUTER JOIN v_{table_prefix}_shards_scan_state scan_state ON n.commitment_tree_position >= scan_state.start_position AND n.commitment_tree_position < scan_state.end_position_exclusive - WHERE n.spent IS NULL - AND ( - t.expiry_height IS NULL - OR t.block IS NOT NULL - OR t.expiry_height >= :summary_height - )", + WHERE ( + t.block IS NOT NULL -- the receiving tx is mined + OR t.expiry_height IS NULL -- the receiving tx will not expire + OR t.expiry_height >= :summary_height -- the receiving tx is unexpired + ) + -- and the received note is unspent + AND n.id NOT IN ( + SELECT {table_prefix}_received_note_id + FROM {table_prefix}_received_note_spends + JOIN transactions t ON t.id_tx = transaction_id + WHERE t.block IS NOT NULL -- the spending transaction is mined + OR t.expiry_height IS NULL -- the spending tx will not expire + OR t.expiry_height > :summary_height -- the spending tx is unexpired + )" ))?; let mut rows = @@ -1245,10 +1253,17 @@ pub(crate) fn get_wallet_summary( let mut stmt_transparent_balances = tx.prepare( "SELECT u.received_by_account_id, SUM(u.value_zat) FROM utxos u - LEFT OUTER JOIN transactions tx - ON tx.id_tx = u.spent_in_tx WHERE u.height <= :max_height - AND (u.spent_in_tx IS NULL OR (tx.block IS NULL AND tx.expiry_height <= :stable_height)) + -- and the received txo is unspent + AND u.id NOT IN ( + SELECT transparent_received_output_id + FROM transparent_received_output_spends txo_spends + JOIN transactions tx + ON tx.id_tx = txo_spends.transaction_id + WHERE tx.block IS NOT NULL -- the spending tx is mined + OR tx.expiry_height IS NULL -- the spending tx will not expire + OR tx.expiry_height > :stable_height -- the spending tx is unexpired + ) GROUP BY u.received_by_account_id", )?; let mut rows = stmt_transparent_balances.query(named_params![ @@ -1840,7 +1855,12 @@ pub(crate) fn get_min_unspent_height( "SELECT MIN(tx.block) FROM sapling_received_notes n JOIN transactions tx ON tx.id_tx = n.tx - WHERE n.spent IS NULL", + WHERE n.id NOT IN ( + SELECT sapling_received_note_id + FROM sapling_received_note_spends + JOIN transactions tx ON tx.id_tx = transaction_id + WHERE tx.block IS NOT NULL + )", [], |row| { row.get(0) @@ -1852,7 +1872,12 @@ pub(crate) fn get_min_unspent_height( "SELECT MIN(tx.block) FROM orchard_received_notes n JOIN transactions tx ON tx.id_tx = n.tx - WHERE n.spent IS NULL", + WHERE n.id NOT IN ( + SELECT orchard_received_note_id + FROM orchard_received_note_spends + JOIN transactions tx ON tx.id_tx = transaction_id + WHERE tx.block IS NOT NULL + )", [], |row| { row.get(0) @@ -1898,7 +1923,25 @@ pub(crate) fn truncate_to_height( } } - // nothing to do if we're deleting back down to the max height + // Delete from the scanning queue any range with a start height greater than the + // truncation height, and then truncate any remaining range by setting the end + // equal to the truncation height + 1. This sets our view of the chain tip back + // to the retained height. + conn.execute( + "DELETE FROM scan_queue + WHERE block_range_start >= :new_end_height", + named_params![":new_end_height": u32::from(block_height + 1)], + )?; + conn.execute( + "UPDATE scan_queue + SET block_range_end = :new_end_height + WHERE block_range_end > :new_end_height", + named_params![":new_end_height": u32::from(block_height + 1)], + )?; + + // If we're removing scanned blocks, we need to truncate the note commitment tree, un-mine + // transactions, and remove received transparent outputs and affected block records from the + // database. if block_height < last_scanned_height { // Truncate the note commitment trees let mut wdb = WalletDb { @@ -1913,36 +1956,15 @@ pub(crate) fn truncate_to_height( tree.truncate_removing_checkpoint(&block_height).map(|_| ()) })?; - // Rewind received notes - conn.execute( - "DELETE FROM sapling_received_notes - WHERE id IN ( - SELECT rn.id - FROM sapling_received_notes rn - LEFT OUTER JOIN transactions tx - ON tx.id_tx = rn.tx - WHERE tx.block IS NOT NULL AND tx.block > ? - );", - [u32::from(block_height)], - )?; - #[cfg(feature = "orchard")] - conn.execute( - "DELETE FROM orchard_received_notes - WHERE id IN ( - SELECT rn.id - FROM orchard_received_notes rn - LEFT OUTER JOIN transactions tx - ON tx.id_tx = rn.tx - WHERE tx.block IS NOT NULL AND tx.block > ? - );", - [u32::from(block_height)], - )?; - // Do not delete sent notes; this can contain data that is not recoverable // from the chain. Wallets must continue to operate correctly in the // presence of stale sent notes that link to unmined transactions. + // Also, do not delete received notes; they may contain memo data that is + // not recoverable; balance APIs must ensure that un-mined received notes + // do not count towards spendability or transaction balalnce. - // Rewind utxos + // Rewind utxos. It is currently necessary to delete these because we do + // not have the full transaction data for the received output. conn.execute( "DELETE FROM utxos WHERE height > ?", [u32::from(block_height)], @@ -1955,7 +1977,7 @@ pub(crate) fn truncate_to_height( [u32::from(block_height)], )?; - // Now that they aren't depended on, delete scanned blocks. + // Now that they aren't depended on, delete un-mined blocks. conn.execute( "DELETE FROM blocks WHERE height > ?", [u32::from(block_height)], @@ -1968,32 +1990,6 @@ pub(crate) fn truncate_to_height( WHERE block_height > :block_height", named_params![":block_height": u32::from(block_height)], )?; - - // Delete from the scanning queue any range with a start height greater than the - // truncation height, and then truncate any remaining range by setting the end - // equal to the truncation height + 1. - conn.execute( - "DELETE FROM scan_queue - WHERE block_range_start > :block_height", - named_params![":block_height": u32::from(block_height)], - )?; - - conn.execute( - "UPDATE scan_queue - SET block_range_end = :end_height - WHERE block_range_end > :end_height", - named_params![":end_height": u32::from(block_height + 1)], - )?; - - // Prioritize the height we just rewound to for verification. - let query_range = block_height..(block_height + 1); - let scan_range = ScanRange::from_parts(query_range.clone(), ScanPriority::Verify); - replace_queue_entries::( - conn, - &query_range, - Some(scan_range).into_iter(), - false, - )?; } Ok(()) @@ -2037,11 +2033,15 @@ pub(crate) fn get_unspent_transparent_output( let mut stmt_select_utxo = conn.prepare_cached( "SELECT u.prevout_txid, u.prevout_idx, u.script, u.value_zat, u.height FROM utxos u - LEFT OUTER JOIN transactions tx - ON tx.id_tx = u.spent_in_tx WHERE u.prevout_txid = :txid AND u.prevout_idx = :output_index - AND tx.block IS NULL", + AND u.id NOT IN ( + SELECT txo_spends.id + FROM transparent_received_output_spends txo_spends + JOIN transactions tx ON tx.id_tx = txo_spends.transaction_id + WHERE tx.block IS NOT NULL -- the spending tx is mined + OR tx.expiry_height IS NULL -- the spending tx will not expire + )", )?; let result: Result, SqliteClientError> = stmt_select_utxo @@ -2078,11 +2078,17 @@ pub(crate) fn get_unspent_transparent_outputs( "SELECT u.prevout_txid, u.prevout_idx, u.script, u.value_zat, u.height FROM utxos u - LEFT OUTER JOIN transactions tx - ON tx.id_tx = u.spent_in_tx WHERE u.address = :address AND u.height <= :max_height - AND (u.spent_in_tx IS NULL OR (tx.block IS NULL AND tx.expiry_height <= :stable_height))", + AND u.id NOT IN ( + SELECT txo_spends.transparent_received_output_id + FROM transparent_received_output_spends txo_spends + JOIN transactions tx ON tx.id_tx = txo_spends.transaction_id + WHERE + tx.block IS NOT NULL -- the spending tx is mined + OR tx.expiry_height IS NULL -- the spending tx will not expire + OR tx.expiry_height > :stable_height -- the spending tx is unexpired + )", )?; let addr_str = address.encode(params); @@ -2124,11 +2130,17 @@ pub(crate) fn get_transparent_balances( let mut stmt_blocks = conn.prepare( "SELECT u.address, SUM(u.value_zat) FROM utxos u - LEFT OUTER JOIN transactions tx - ON tx.id_tx = u.spent_in_tx WHERE u.received_by_account_id = :account_id AND u.height <= :max_height - AND (u.spent_in_tx IS NULL OR (tx.block IS NULL AND tx.expiry_height <= :stable_height)) + AND u.id NOT IN ( + SELECT txo_spends.transparent_received_output_id + FROM transparent_received_output_spends txo_spends + JOIN transactions tx ON tx.id_tx = txo_spends.transaction_id + WHERE + tx.block IS NOT NULL -- the spending tx is mined + OR tx.expiry_height IS NULL -- the spending tx will not expire + OR tx.expiry_height > :stable_height -- the spending tx is unexpired + ) GROUP BY u.address", )?; @@ -2316,9 +2328,12 @@ pub(crate) fn mark_transparent_utxo_spent( outpoint: &OutPoint, ) -> Result<(), SqliteClientError> { let mut stmt_mark_transparent_utxo_spent = conn.prepare_cached( - "UPDATE utxos SET spent_in_tx = :spent_in_tx - WHERE prevout_txid = :prevout_txid - AND prevout_idx = :prevout_idx", + "INSERT INTO transparent_received_output_spends (transparent_received_output_id, transaction_id) + SELECT txo.id, :spent_in_tx + FROM utxos txo + WHERE txo.prevout_txid = :prevout_txid + AND txo.prevout_idx = :prevout_idx + ON CONFLICT (transparent_received_output_id, transaction_id) DO NOTHING", )?; let sql_args = named_params![ @@ -2418,29 +2433,6 @@ pub(crate) fn put_legacy_transparent_utxo( stmt_upsert_legacy_transparent_utxo.query_row(sql_args, |row| row.get::<_, i64>(0).map(UtxoId)) } -/// Marks notes that have not been mined in transactions -/// as expired, up to the given block height. -pub(crate) fn update_expired_notes( - conn: &rusqlite::Connection, - expiry_height: BlockHeight, -) -> Result<(), SqliteClientError> { - let mut stmt_update_sapling_expired = conn.prepare_cached( - "UPDATE sapling_received_notes SET spent = NULL WHERE EXISTS ( - SELECT id_tx FROM transactions - WHERE id_tx = sapling_received_notes.spent AND block IS NULL AND expiry_height < ? - )", - )?; - stmt_update_sapling_expired.execute([u32::from(expiry_height)])?; - let mut stmt_update_orchard_expired = conn.prepare_cached( - "UPDATE orchard_received_notes SET spent = NULL WHERE EXISTS ( - SELECT id_tx FROM transactions - WHERE id_tx = orchard_received_notes.spent AND block IS NULL AND expiry_height < ? - )", - )?; - stmt_update_orchard_expired.execute([u32::from(expiry_height)])?; - Ok(()) -} - // A utility function for creation of parameters for use in `insert_sent_output` // and `put_sent_output` fn recipient_params( diff --git a/zcash_client_sqlite/src/wallet/common.rs b/zcash_client_sqlite/src/wallet/common.rs index 663d9b717..113aac148 100644 --- a/zcash_client_sqlite/src/wallet/common.rs +++ b/zcash_client_sqlite/src/wallet/common.rs @@ -65,19 +65,26 @@ where let (table_prefix, index_col, note_reconstruction_cols) = per_protocol_names(protocol); let result = conn.query_row_and_then( &format!( - "SELECT {table_prefix}_received_notes.id, txid, {index_col}, + "SELECT rn.id, txid, {index_col}, diversifier, value, {note_reconstruction_cols}, commitment_tree_position, accounts.ufvk, recipient_key_scope - FROM {table_prefix}_received_notes - INNER JOIN accounts ON accounts.id = {table_prefix}_received_notes.account_id - INNER JOIN transactions ON transactions.id_tx = {table_prefix}_received_notes.tx + FROM {table_prefix}_received_notes rn + INNER JOIN accounts ON accounts.id = rn.account_id + INNER JOIN transactions ON transactions.id_tx = rn.tx WHERE txid = :txid + AND transactions.block IS NOT NULL AND {index_col} = :output_index AND accounts.ufvk IS NOT NULL AND recipient_key_scope IS NOT NULL AND nf IS NOT NULL AND commitment_tree_position IS NOT NULL - AND spent IS NULL" + AND rn.id NOT IN ( + SELECT {table_prefix}_received_note_id + FROM {table_prefix}_received_note_spends + JOIN transactions stx ON stx.id_tx = transaction_id + WHERE stx.block IS NOT NULL -- the spending tx is mined + OR stx.expiry_height IS NULL -- the spending tx will not expire + )" ), named_params![ ":txid": txid.as_ref(), @@ -143,10 +150,7 @@ where SELECT {table_prefix}_received_notes.id AS id, txid, {index_col}, diversifier, value, {note_reconstruction_cols}, commitment_tree_position, - SUM(value) OVER ( - PARTITION BY {table_prefix}_received_notes.account_id, spent - ORDER BY {table_prefix}_received_notes.id - ) AS so_far, + SUM(value) OVER (ROWS UNBOUNDED PRECEDING) AS so_far, accounts.ufvk as ufvk, recipient_key_scope FROM {table_prefix}_received_notes INNER JOIN accounts @@ -158,9 +162,16 @@ where AND recipient_key_scope IS NOT NULL AND nf IS NOT NULL AND commitment_tree_position IS NOT NULL - AND spent IS NULL AND transactions.block <= :anchor_height AND {table_prefix}_received_notes.id NOT IN rarray(:exclude) + AND {table_prefix}_received_notes.id NOT IN ( + SELECT {table_prefix}_received_note_id + FROM {table_prefix}_received_note_spends + JOIN transactions stx ON stx.id_tx = transaction_id + WHERE stx.block IS NOT NULL -- the spending tx is mined + OR stx.expiry_height IS NULL -- the spending tx will not expire + OR stx.expiry_height > :anchor_height -- the spending tx is unexpired + ) AND NOT EXISTS ( SELECT 1 FROM v_{table_prefix}_shard_unscanned_ranges unscanned -- select all the unscanned ranges involving the shard containing this note diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 644f6748f..7ef261d30 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -402,6 +402,17 @@ mod tests { ON UPDATE RESTRICT, CONSTRAINT nf_uniq UNIQUE (spend_pool, nf) )", + "CREATE TABLE orchard_received_note_spends ( + orchard_received_note_id INTEGER NOT NULL, + transaction_id INTEGER NOT NULL, + FOREIGN KEY (orchard_received_note_id) + REFERENCES orchard_received_notes(id) + ON DELETE CASCADE, + FOREIGN KEY (transaction_id) + -- We do not delete transactions, so this does not cascade + REFERENCES transactions(id_tx), + UNIQUE (orchard_received_note_id, transaction_id) + )", "CREATE TABLE orchard_received_notes ( id INTEGER PRIMARY KEY, tx INTEGER NOT NULL, @@ -414,12 +425,10 @@ mod tests { nf BLOB UNIQUE, is_change INTEGER NOT NULL, memo BLOB, - spent INTEGER, commitment_tree_position INTEGER, recipient_key_scope INTEGER, FOREIGN KEY (tx) REFERENCES transactions(id_tx), FOREIGN KEY (account_id) REFERENCES accounts(id), - FOREIGN KEY (spent) REFERENCES transactions(id_tx), CONSTRAINT tx_output UNIQUE (tx, action_index) )", "CREATE TABLE orchard_tree_cap ( @@ -447,6 +456,17 @@ mod tests { contains_marked INTEGER, CONSTRAINT root_unique UNIQUE (root_hash) )", + "CREATE TABLE sapling_received_note_spends ( + sapling_received_note_id INTEGER NOT NULL, + transaction_id INTEGER NOT NULL, + FOREIGN KEY (sapling_received_note_id) + REFERENCES sapling_received_notes(id) + ON DELETE CASCADE, + FOREIGN KEY (transaction_id) + -- We do not delete transactions, so this does not cascade + REFERENCES transactions(id_tx), + UNIQUE (sapling_received_note_id, transaction_id) + )", r#"CREATE TABLE "sapling_received_notes" ( id INTEGER PRIMARY KEY, tx INTEGER NOT NULL, @@ -458,12 +478,10 @@ mod tests { nf BLOB UNIQUE, is_change INTEGER NOT NULL, memo BLOB, - spent INTEGER, commitment_tree_position INTEGER, recipient_key_scope INTEGER, FOREIGN KEY (tx) REFERENCES transactions(id_tx), FOREIGN KEY (account_id) REFERENCES accounts(id), - FOREIGN KEY (spent) REFERENCES transactions(id_tx), CONSTRAINT tx_output UNIQUE (tx, output_index) )"#, "CREATE TABLE sapling_tree_cap ( @@ -535,6 +553,17 @@ mod tests { fee INTEGER, FOREIGN KEY (block) REFERENCES blocks(height) )", + "CREATE TABLE transparent_received_output_spends ( + transparent_received_output_id INTEGER NOT NULL, + transaction_id INTEGER NOT NULL, + FOREIGN KEY (transparent_received_output_id) + REFERENCES utxos(id) + ON DELETE CASCADE, + FOREIGN KEY (transaction_id) + -- We do not delete transactions, so this does not cascade + REFERENCES transactions(id_tx), + UNIQUE (transparent_received_output_id, transaction_id) + )", "CREATE TABLE tx_locator_map ( block_height INTEGER NOT NULL, tx_index INTEGER NOT NULL, @@ -550,9 +579,7 @@ mod tests { script BLOB NOT NULL, value_zat INTEGER NOT NULL, height INTEGER NOT NULL, - spent_in_tx INTEGER, FOREIGN KEY (received_by_account_id) REFERENCES accounts(id), - FOREIGN KEY (spent_in_tx) REFERENCES transactions(id_tx), CONSTRAINT tx_outpoint UNIQUE (prevout_txid, prevout_idx) )"#, ]; @@ -584,18 +611,12 @@ mod tests { r#"CREATE INDEX orchard_received_notes_account ON orchard_received_notes ( account_id ASC )"#, - r#"CREATE INDEX orchard_received_notes_spent ON orchard_received_notes ( - spent ASC - )"#, r#"CREATE INDEX orchard_received_notes_tx ON orchard_received_notes ( tx ASC )"#, r#"CREATE INDEX "sapling_received_notes_account" ON "sapling_received_notes" ( "account_id" ASC )"#, - r#"CREATE INDEX "sapling_received_notes_spent" ON "sapling_received_notes" ( - "spent" ASC - )"#, r#"CREATE INDEX "sapling_received_notes_tx" ON "sapling_received_notes" ( "tx" ASC )"#, @@ -603,7 +624,6 @@ mod tests { r#"CREATE INDEX sent_notes_to_account ON "sent_notes" (to_account_id)"#, r#"CREATE INDEX sent_notes_tx ON "sent_notes" (tx)"#, r#"CREATE INDEX utxos_received_by_account ON "utxos" (received_by_account_id)"#, - r#"CREATE INDEX utxos_spent_in_tx ON "utxos" (spent_in_tx)"#, ]; let mut indices_query = st .wallet() @@ -686,6 +706,19 @@ mod tests { subtree_start_height, subtree_end_height, contains_marked".to_owned(), + // v_received_note_spends + "CREATE VIEW v_received_note_spends AS + SELECT + 2 AS pool, + sapling_received_note_id AS received_note_id, + transaction_id + FROM sapling_received_note_spends + UNION + SELECT + 3 AS pool, + orchard_received_note_id AS received_note_id, + transaction_id + FROM orchard_received_note_spends".to_owned(), // v_received_notes "CREATE VIEW v_received_notes AS SELECT @@ -697,7 +730,6 @@ mod tests { sapling_received_notes.value, is_change, sapling_received_notes.memo, - spent, sent_notes.id AS sent_note_id FROM sapling_received_notes LEFT JOIN sent_notes @@ -713,7 +745,6 @@ mod tests { orchard_received_notes.value, is_change, orchard_received_notes.memo, - spent, sent_notes.id AS sent_note_id FROM orchard_received_notes LEFT JOIN sent_notes @@ -834,8 +865,11 @@ mod tests { 0 AS received_count, 0 AS memo_present FROM v_received_notes + JOIN v_received_note_spends rns + ON rns.pool = v_received_notes.pool + AND rns.received_note_id = v_received_notes.id_within_pool_table JOIN transactions - ON transactions.id_tx = v_received_notes.spent + ON transactions.id_tx = rns.transaction_id UNION -- Transparent TXOs spent in this transaction SELECT utxos.received_by_account_id AS account_id, @@ -848,8 +882,10 @@ mod tests { 0 AS received_count, 0 AS memo_present FROM utxos + JOIN transparent_received_output_spends tros + ON tros.transparent_received_output_id = utxos.id JOIN transactions - ON transactions.id_tx = utxos.spent_in_tx + ON transactions.id_tx = tros.transaction_id ), -- Obtain a count of the notes that the wallet created in each transaction, -- not counting change notes. diff --git a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs index a7099a61a..59a5158fd 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs @@ -229,12 +229,10 @@ impl RusqliteMigration for Migration

{ nf BLOB UNIQUE, is_change INTEGER NOT NULL, memo BLOB, - spent INTEGER, commitment_tree_position INTEGER, recipient_key_scope INTEGER, FOREIGN KEY (tx) REFERENCES transactions(id_tx), FOREIGN KEY (account_id) REFERENCES accounts(id), - FOREIGN KEY (spent) REFERENCES transactions(id_tx), CONSTRAINT tx_output UNIQUE (tx, output_index) ); CREATE INDEX "sapling_received_notes_account" ON "sapling_received_notes_new" ( @@ -243,11 +241,36 @@ impl RusqliteMigration for Migration

{ CREATE INDEX "sapling_received_notes_tx" ON "sapling_received_notes_new" ( "tx" ASC ); - CREATE INDEX "sapling_received_notes_spent" ON "sapling_received_notes_new" ( - "spent" ASC + + -- Replace the `spent` column in `sapling_received_notes` with a junction table between + -- received notes and the transactions that spend them. This is necessary as otherwise + -- we cannot compute the correct value of transactions that expire unmined. + CREATE TABLE sapling_received_note_spends ( + sapling_received_note_id INTEGER NOT NULL, + transaction_id INTEGER NOT NULL, + FOREIGN KEY (sapling_received_note_id) + REFERENCES sapling_received_notes(id) + ON DELETE CASCADE, + FOREIGN KEY (transaction_id) + -- We do not delete transactions, so this does not cascade + REFERENCES transactions(id_tx), + UNIQUE (sapling_received_note_id, transaction_id) ); - INSERT INTO sapling_received_notes_new (id, tx, output_index, account_id, diversifier, value, rcm, nf, is_change, memo, spent, commitment_tree_position, recipient_key_scope) - SELECT id_note, tx, output_index, account, diversifier, value, rcm, nf, is_change, memo, spent, commitment_tree_position, recipient_key_scope + + INSERT INTO sapling_received_note_spends (sapling_received_note_id, transaction_id) + SELECT id_note, spent + FROM sapling_received_notes + WHERE spent IS NOT NULL; + + INSERT INTO sapling_received_notes_new ( + id, tx, output_index, account_id, + diversifier, value, rcm, nf, is_change, memo, commitment_tree_position, + recipient_key_scope + ) + SELECT + id_note, tx, output_index, account, + diversifier, value, rcm, nf, is_change, memo, commitment_tree_position, + recipient_key_scope FROM sapling_received_notes; DROP TABLE sapling_received_notes; @@ -295,21 +318,39 @@ impl RusqliteMigration for Migration

{ script BLOB NOT NULL, value_zat INTEGER NOT NULL, height INTEGER NOT NULL, - spent_in_tx INTEGER, FOREIGN KEY (received_by_account_id) REFERENCES accounts(id), - FOREIGN KEY (spent_in_tx) REFERENCES transactions(id_tx), CONSTRAINT tx_outpoint UNIQUE (prevout_txid, prevout_idx) ); CREATE INDEX utxos_received_by_account ON utxos_new (received_by_account_id); - CREATE INDEX utxos_spent_in_tx ON utxos_new (spent_in_tx); - INSERT INTO utxos_new (id, received_by_account_id, address, prevout_txid, prevout_idx, script, value_zat, height, spent_in_tx) - SELECT id_utxo, received_by_account, address, prevout_txid, prevout_idx, script, value_zat, height, spent_in_tx + + INSERT INTO utxos_new (id, received_by_account_id, address, prevout_txid, prevout_idx, script, value_zat, height) + SELECT id_utxo, received_by_account, address, prevout_txid, prevout_idx, script, value_zat, height FROM utxos; + -- Replace the `spent_in_tx` column in `utxos` with a junction table between received + -- outputs and the transactions that spend them. This is necessary as otherwise we + -- cannot compute the correct value of transactions that expire unmined. + CREATE TABLE transparent_received_output_spends ( + transparent_received_output_id INTEGER NOT NULL, + transaction_id INTEGER NOT NULL, + FOREIGN KEY (transparent_received_output_id) + REFERENCES utxos(id) + ON DELETE CASCADE, + FOREIGN KEY (transaction_id) + -- We do not delete transactions, so this does not cascade + REFERENCES transactions(id_tx), + UNIQUE (transparent_received_output_id, transaction_id) + ); + + INSERT INTO transparent_received_output_spends (transparent_received_output_id, transaction_id) + SELECT id_utxo, spent_in_tx + FROM utxos + WHERE spent_in_tx IS NOT NULL; + DROP TABLE utxos; ALTER TABLE utxos_new RENAME TO utxos; "#, - )?; + )?; // Rewrite v_transactions view transaction.execute_batch( @@ -361,8 +402,10 @@ impl RusqliteMigration for Migration

{ 0 AS received_count, 0 AS memo_present FROM sapling_received_notes + JOIN sapling_received_note_spends + ON sapling_received_note_id = sapling_received_notes.id JOIN transactions - ON transactions.id_tx = sapling_received_notes.spent + ON transactions.id_tx = sapling_received_note_spends.transaction_id UNION SELECT utxos.id AS id, utxos.received_by_account_id AS account_id, @@ -374,8 +417,10 @@ impl RusqliteMigration for Migration

{ 0 AS received_count, 0 AS memo_present FROM utxos - JOIN transactions - ON transactions.id_tx = utxos.spent_in_tx + JOIN transparent_received_output_spends txo_spends + ON txo_spends.transparent_received_output_id = txos.id + JOIN transactions + ON transactions.id_tx = txo_spends.transaction_id ), sent_note_counts AS ( SELECT sent_notes.from_account_id AS account_id, diff --git a/zcash_client_sqlite/src/wallet/init/migrations/orchard_received_notes.rs b/zcash_client_sqlite/src/wallet/init/migrations/orchard_received_notes.rs index 7e174164b..550dc14f4 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/orchard_received_notes.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/orchard_received_notes.rs @@ -45,12 +45,10 @@ impl RusqliteMigration for Migration { nf BLOB UNIQUE, is_change INTEGER NOT NULL, memo BLOB, - spent INTEGER, commitment_tree_position INTEGER, recipient_key_scope INTEGER, FOREIGN KEY (tx) REFERENCES transactions(id_tx), FOREIGN KEY (account_id) REFERENCES accounts(id), - FOREIGN KEY (spent) REFERENCES transactions(id_tx), CONSTRAINT tx_output UNIQUE (tx, action_index) ); CREATE INDEX orchard_received_notes_account ON orchard_received_notes ( @@ -59,8 +57,17 @@ impl RusqliteMigration for Migration { CREATE INDEX orchard_received_notes_tx ON orchard_received_notes ( tx ASC ); - CREATE INDEX orchard_received_notes_spent ON orchard_received_notes ( - spent ASC + + CREATE TABLE orchard_received_note_spends ( + orchard_received_note_id INTEGER NOT NULL, + transaction_id INTEGER NOT NULL, + FOREIGN KEY (orchard_received_note_id) + REFERENCES orchard_received_notes(id) + ON DELETE CASCADE, + FOREIGN KEY (transaction_id) + -- We do not delete transactions, so this does not cascade + REFERENCES transactions(id_tx), + UNIQUE (orchard_received_note_id, transaction_id) );", )?; @@ -78,7 +85,6 @@ impl RusqliteMigration for Migration { sapling_received_notes.value, is_change, sapling_received_notes.memo, - spent, sent_notes.id AS sent_note_id FROM sapling_received_notes LEFT JOIN sent_notes @@ -94,7 +100,6 @@ impl RusqliteMigration for Migration { orchard_received_notes.value, is_change, orchard_received_notes.memo, - spent, sent_notes.id AS sent_note_id FROM orchard_received_notes LEFT JOIN sent_notes @@ -103,6 +108,25 @@ impl RusqliteMigration for Migration { ) })?; + transaction.execute_batch({ + let sapling_pool_code = pool_code(PoolType::Shielded(ShieldedProtocol::Sapling)); + let orchard_pool_code = pool_code(PoolType::Shielded(ShieldedProtocol::Orchard)); + &format!( + "CREATE VIEW v_received_note_spends AS + SELECT + {sapling_pool_code} AS pool, + sapling_received_note_id AS received_note_id, + transaction_id + FROM sapling_received_note_spends + UNION + SELECT + {orchard_pool_code} AS pool, + orchard_received_note_id AS received_note_id, + transaction_id + FROM orchard_received_note_spends;" + ) + })?; + transaction.execute_batch({ let transparent_pool_code = pool_code(PoolType::Transparent); &format!( @@ -157,8 +181,11 @@ impl RusqliteMigration for Migration { 0 AS received_count, 0 AS memo_present FROM v_received_notes + JOIN v_received_note_spends rns + ON rns.pool = v_received_notes.pool + AND rns.received_note_id = v_received_notes.id_within_pool_table JOIN transactions - ON transactions.id_tx = v_received_notes.spent + ON transactions.id_tx = rns.transaction_id UNION -- Transparent TXOs spent in this transaction SELECT utxos.received_by_account_id AS account_id, @@ -171,8 +198,10 @@ impl RusqliteMigration for Migration { 0 AS received_count, 0 AS memo_present FROM utxos + JOIN transparent_received_output_spends tros + ON tros.transparent_received_output_id = utxos.id JOIN transactions - ON transactions.id_tx = utxos.spent_in_tx + ON transactions.id_tx = tros.transaction_id ), -- Obtain a count of the notes that the wallet created in each transaction, -- not counting change notes. @@ -223,9 +252,14 @@ impl RusqliteMigration for Migration { 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; + GROUP BY notes.account_id, notes.txid;" + ) + })?; - DROP VIEW v_tx_outputs; + transaction.execute_batch({ + let transparent_pool_code = pool_code(PoolType::Transparent); + &format!( + "DROP VIEW v_tx_outputs; CREATE VIEW v_tx_outputs AS SELECT transactions.txid AS txid, v_received_notes.pool AS output_pool, @@ -267,7 +301,8 @@ impl RusqliteMigration for Migration { ON transactions.id_tx = sent_notes.tx LEFT JOIN v_received_notes ON sent_notes.id = v_received_notes.sent_note_id - WHERE COALESCE(v_received_notes.is_change, 0) = 0;") + WHERE COALESCE(v_received_notes.is_change, 0) = 0;" + ) })?; Ok(()) diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 0a5e515d3..27c1559bc 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -3,7 +3,7 @@ use orchard::{ keys::Diversifier, note::{Note, Nullifier, RandomSeed, Rho}, }; -use rusqlite::{named_params, params, Connection, Row}; +use rusqlite::{named_params, Connection, Row, Transaction}; use zcash_client_backend::{ data_api::NullifierQuery, @@ -222,7 +222,7 @@ pub(crate) fn select_spendable_orchard_notes( /// - A transaction will not contain more than 2^63 shielded outputs. /// - A note value will never exceed 2^63 zatoshis. pub(crate) fn put_received_note( - conn: &Connection, + conn: &Transaction, output: &T, tx_ref: i64, spent_in: Option, @@ -232,13 +232,13 @@ pub(crate) fn put_received_note( ( tx, action_index, account_id, diversifier, value, rho, rseed, memo, nf, - is_change, spent, commitment_tree_position, + is_change, commitment_tree_position, recipient_key_scope ) VALUES ( :tx, :action_index, :account_id, :diversifier, :value, :rho, :rseed, :memo, :nf, - :is_change, :spent, :commitment_tree_position, + :is_change, :commitment_tree_position, :recipient_key_scope ) ON CONFLICT (tx, action_index) DO UPDATE @@ -250,9 +250,9 @@ pub(crate) fn put_received_note( nf = IFNULL(:nf, nf), 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), - recipient_key_scope = :recipient_key_scope", + recipient_key_scope = :recipient_key_scope + RETURNING orchard_received_notes.id", )?; let rseed = output.note().rseed(); @@ -270,15 +270,25 @@ pub(crate) fn put_received_note( ":nf": output.nullifier().map(|nf| nf.to_bytes()), ":memo": memo_repr(output.memo()), ":is_change": output.is_change(), - ":spent": spent_in, ":commitment_tree_position": output.note_commitment_tree_position().map(u64::from), ":recipient_key_scope": output.recipient_key_scope().map(scope_code), ]; - stmt_upsert_received_note - .execute(sql_args) + let received_note_id = stmt_upsert_received_note + .query_row(sql_args, |row| row.get::<_, i64>(0)) .map_err(SqliteClientError::from)?; + if let Some(spent_in) = spent_in { + conn.execute( + "INSERT INTO orchard_received_note_spends (orchard_received_note_id, transaction_id) + VALUES (:orchard_received_note_id, :transaction_id) + ON CONFLICT (orchard_received_note_id, transaction_id) DO NOTHING", + named_params![ + ":orchard_received_note_id": received_note_id, + ":transaction_id": spent_in + ], + )?; + } Ok(()) } @@ -297,10 +307,16 @@ pub(crate) fn get_orchard_nullifiers( NullifierQuery::Unspent => conn.prepare( "SELECT rn.account_id, rn.nf FROM orchard_received_notes rn - LEFT OUTER JOIN transactions tx - ON tx.id_tx = rn.spent - WHERE tx.block IS NULL - AND nf IS NOT NULL", + JOIN transactions tx ON tx.id_tx = rn.tx + WHERE rn.nf IS NOT NULL + AND tx.block IS NOT NULL + AND rn.id NOT IN ( + SELECT spends.orchard_received_note_id + FROM orchard_received_note_spends spends + JOIN transactions stx ON stx.id_tx = spends.transaction_id + WHERE stx.block IS NOT NULL -- the spending tx is mined + OR stx.expiry_height IS NULL -- the spending tx will not expire + )", )?, NullifierQuery::All => conn.prepare( "SELECT rn.account_id, rn.nf @@ -329,10 +345,16 @@ pub(crate) fn mark_orchard_note_spent( tx_ref: i64, nf: &Nullifier, ) -> Result { - let mut stmt_mark_orchard_note_spent = - conn.prepare_cached("UPDATE orchard_received_notes SET spent = ? WHERE nf = ?")?; + let mut stmt_mark_orchard_note_spent = conn.prepare_cached( + "INSERT INTO orchard_received_note_spends (orchard_received_note_id, transaction_id) + SELECT id, :transaction_id FROM orchard_received_notes WHERE nf = :nf + ON CONFLICT (orchard_received_note_id, transaction_id) DO NOTHING", + )?; - match stmt_mark_orchard_note_spent.execute(params![tx_ref, nf.to_bytes()])? { + match stmt_mark_orchard_note_spent.execute(named_params![ + ":nf": nf.to_bytes(), + ":transaction_id": tx_ref + ])? { 0 => Ok(false), 1 => Ok(true), _ => unreachable!("nf column is marked as UNIQUE"), diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 690384ad3..8a7c4ad8f 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -2,7 +2,7 @@ use group::ff::PrimeField; use incrementalmerkletree::Position; -use rusqlite::{named_params, params, Connection, Row}; +use rusqlite::{named_params, Connection, Row, Transaction}; use sapling::{self, Diversifier, Nullifier, Rseed}; use zcash_client_backend::{ @@ -241,10 +241,16 @@ pub(crate) fn get_sapling_nullifiers( NullifierQuery::Unspent => conn.prepare( "SELECT rn.account_id, rn.nf FROM sapling_received_notes rn - LEFT OUTER JOIN transactions tx - ON tx.id_tx = rn.spent - WHERE tx.block IS NULL - AND nf IS NOT NULL", + JOIN transactions tx ON tx.id_tx = rn.tx + WHERE rn.nf IS NOT NULL + AND tx.block IS NOT NULL + AND rn.id NOT IN ( + SELECT spends.sapling_received_note_id + FROM sapling_received_note_spends spends + JOIN transactions stx ON stx.id_tx = spends.transaction_id + WHERE stx.block IS NOT NULL -- the spending tx is mined + OR stx.expiry_height IS NULL -- the spending tx will not expire + )", ), NullifierQuery::All => conn.prepare( "SELECT rn.account_id, rn.nf @@ -273,10 +279,16 @@ pub(crate) fn mark_sapling_note_spent( tx_ref: i64, nf: &sapling::Nullifier, ) -> Result { - let mut stmt_mark_sapling_note_spent = - conn.prepare_cached("UPDATE sapling_received_notes SET spent = ? WHERE nf = ?")?; + let mut stmt_mark_sapling_note_spent = conn.prepare_cached( + "INSERT INTO sapling_received_note_spends (sapling_received_note_id, transaction_id) + SELECT id, :transaction_id FROM sapling_received_notes WHERE nf = :nf + ON CONFLICT (sapling_received_note_id, transaction_id) DO NOTHING", + )?; - match stmt_mark_sapling_note_spent.execute(params![tx_ref, &nf.0[..]])? { + match stmt_mark_sapling_note_spent.execute(named_params![ + ":nf": &nf.0[..], + ":transaction_id": tx_ref + ])? { 0 => Ok(false), 1 => Ok(true), _ => unreachable!("nf column is marked as UNIQUE"), @@ -289,7 +301,7 @@ pub(crate) fn mark_sapling_note_spent( /// - A transaction will not contain more than 2^63 shielded outputs. /// - A note value will never exceed 2^63 zatoshis. pub(crate) fn put_received_note( - conn: &Connection, + conn: &Transaction, output: &T, tx_ref: i64, spent_in: Option, @@ -297,7 +309,7 @@ pub(crate) fn put_received_note( let mut stmt_upsert_received_note = conn.prepare_cached( "INSERT INTO sapling_received_notes (tx, output_index, account_id, diversifier, value, rcm, memo, nf, - is_change, spent, commitment_tree_position, + is_change, commitment_tree_position, recipient_key_scope) VALUES ( :tx, @@ -309,7 +321,6 @@ pub(crate) fn put_received_note( :memo, :nf, :is_change, - :spent, :commitment_tree_position, :recipient_key_scope ) @@ -321,9 +332,9 @@ pub(crate) fn put_received_note( nf = IFNULL(:nf, nf), 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), - recipient_key_scope = :recipient_key_scope", + recipient_key_scope = :recipient_key_scope + RETURNING sapling_received_notes.id", )?; let rcm = output.note().rcm().to_repr(); @@ -340,15 +351,26 @@ pub(crate) fn put_received_note( ":nf": output.nullifier().map(|nf| nf.0.as_ref()), ":memo": memo_repr(output.memo()), ":is_change": output.is_change(), - ":spent": spent_in, ":commitment_tree_position": output.note_commitment_tree_position().map(u64::from), ":recipient_key_scope": output.recipient_key_scope().map(scope_code) ]; - stmt_upsert_received_note - .execute(sql_args) + let received_note_id = stmt_upsert_received_note + .query_row(sql_args, |row| row.get::<_, i64>(0)) .map_err(SqliteClientError::from)?; + if let Some(spent_in) = spent_in { + conn.execute( + "INSERT INTO sapling_received_note_spends (sapling_received_note_id, transaction_id) + VALUES (:sapling_received_note_id, :transaction_id) + ON CONFLICT (sapling_received_note_id, transaction_id) DO NOTHING", + named_params![ + ":sapling_received_note_id": received_note_id, + ":transaction_id": spent_in + ], + )?; + } + Ok(()) }