From 54f59a8778b849605a36995ce6be4bc6b559c78c Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 15 Aug 2024 16:37:37 -0600 Subject: [PATCH] zcash_client_sqlite: Track all transparent spends. Prior to this change, the `mark_transparent_utxo_spent` method assumed that the UTXO information for outputs belonging to the wallet would be known to exist before their spends could be detected. However, this is not true in transparent history recovery: the spends are detected first. We now cache the information about those spends so that we can then correctly record the spend when the output being spent is eventually detected. At present, data from this spend cache is never deleted. This is because such deletions could undermine history recovery in some narrow cases related to chain reorgs. --- zcash_client_sqlite/src/wallet/db.rs | 22 +++++++ zcash_client_sqlite/src/wallet/init.rs | 1 + .../init/migrations/tx_retrieval_queue.rs | 16 ++++- zcash_client_sqlite/src/wallet/transparent.rs | 64 ++++++++++++++++--- 4 files changed, 93 insertions(+), 10 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/db.rs b/zcash_client_sqlite/src/wallet/db.rs index 44d44b927..22d08b3f3 100644 --- a/zcash_client_sqlite/src/wallet/db.rs +++ b/zcash_client_sqlite/src/wallet/db.rs @@ -365,6 +365,28 @@ CREATE TABLE "transparent_received_output_spends" ( UNIQUE (transparent_received_output_id, transaction_id) )"#; +/// A cache of the relationship between a transaction and the prevout data of its +/// transparent inputs. +/// +/// This table is used in out-of-order wallet recovery to cache the information about +/// what transaction(s) spend each transparent outpoint, so that if an output belonging +/// to the wallet is detected after the transaction that spends it has been processed, +/// the spend can also be recorded as part of the process of adding the output to +/// [`TABLE_TRANSPARENT_RECEIVED_OUTPUTS`]. +pub(super) const TABLE_TRANSPARENT_SPEND_MAP: &str = r#" +CREATE TABLE transparent_spend_map ( + spending_transaction_id INTEGER NOT NULL, + prevout_txid BLOB NOT NULL, + prevout_output_index INTEGER NOT NULL, + FOREIGN KEY (spending_transaction_id) REFERENCES transactions(id_tx) + -- NOTE: We can't create a unique constraint on just (prevout_txid, prevout_output_index) + -- because the same output may be attempted to be spent in multiple transactions, even + -- though only one will ever be mined. + CONSTRAINT transparent_spend_map_unique UNIQUE ( + spending_transaction_id, prevout_txid, prevout_output_index + ) +)"#; + /// Stores the outputs of transactions created by the wallet. /// /// Unlike with outputs received by the wallet, we store sent outputs for all pools in diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 9832a04ef..a6011832d 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -406,6 +406,7 @@ mod tests { db::TABLE_TRANSACTIONS, db::TABLE_TRANSPARENT_RECEIVED_OUTPUT_SPENDS, db::TABLE_TRANSPARENT_RECEIVED_OUTPUTS, + db::TABLE_TRANSPARENT_SPEND_MAP, db::TABLE_TRANSPARENT_SPEND_SEARCH_QUEUE, db::TABLE_TX_LOCATOR_MAP, db::TABLE_TX_RETRIEVAL_QUEUE, diff --git a/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs b/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs index 3962dbeca..d011243ad 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/tx_retrieval_queue.rs @@ -48,6 +48,19 @@ impl RusqliteMigration for Migration { output_index INTEGER NOT NULL, FOREIGN KEY (transaction_id) REFERENCES transactions(id_tx), CONSTRAINT value_received_height UNIQUE (transaction_id, output_index) + ); + + CREATE TABLE transparent_spend_map ( + spending_transaction_id INTEGER NOT NULL, + prevout_txid BLOB NOT NULL, + prevout_output_index INTEGER NOT NULL, + FOREIGN KEY (spending_transaction_id) REFERENCES transactions(id_tx) + -- NOTE: We can't create a unique constraint on just (prevout_txid, prevout_output_index) + -- because the same output may be attempted to be spent in multiple transactions, even + -- though only one will ever be mined. + CONSTRAINT transparent_spend_map_unique UNIQUE ( + spending_transaction_id, prevout_txid, prevout_output_index + ) );", )?; @@ -67,7 +80,8 @@ impl RusqliteMigration for Migration { fn down(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { transaction.execute_batch( - "DROP TABLE transparent_spend_search_queue; + "DROP TABLE transparent_spend_map; + DROP TABLE transparent_spend_search_queue; ALTER TABLE transactions DROP COLUMN target_height; DROP TABLE tx_retrieval_queue;", )?; diff --git a/zcash_client_sqlite/src/wallet/transparent.rs b/zcash_client_sqlite/src/wallet/transparent.rs index 96d4d6be4..4d82fb0dd 100644 --- a/zcash_client_sqlite/src/wallet/transparent.rs +++ b/zcash_client_sqlite/src/wallet/transparent.rs @@ -427,11 +427,18 @@ pub(crate) fn add_transparent_account_balances( } /// Marks the given UTXO as having been spent. +/// +/// Returns `true` if the UTXO was known to the wallet. pub(crate) fn mark_transparent_utxo_spent( conn: &rusqlite::Connection, - tx_ref: TxRef, + spent_in_tx: TxRef, outpoint: &OutPoint, -) -> Result<(), SqliteClientError> { +) -> Result { + let spend_params = named_params![ + ":spent_in_tx": spent_in_tx.0, + ":prevout_txid": outpoint.hash().as_ref(), + ":prevout_idx": outpoint.n(), + ]; let mut stmt_mark_transparent_utxo_spent = conn.prepare_cached( "INSERT INTO transparent_received_output_spends (transparent_received_output_id, transaction_id) SELECT txo.id, :spent_in_tx @@ -439,13 +446,12 @@ pub(crate) fn mark_transparent_utxo_spent( JOIN transactions t ON t.id_tx = txo.transaction_id WHERE t.txid = :prevout_txid AND txo.output_index = :prevout_idx - ON CONFLICT (transparent_received_output_id, transaction_id) DO NOTHING", + ON CONFLICT (transparent_received_output_id, transaction_id) + -- The following UPDATE is effectively a no-op, but we perform it anyway so that the + -- number of affected rows can be used to determine whether a record existed. + DO UPDATE SET transaction_id = :spent_in_tx", )?; - stmt_mark_transparent_utxo_spent.execute(named_params![ - ":spent_in_tx": tx_ref.0, - ":prevout_txid": outpoint.hash().as_ref(), - ":prevout_idx": outpoint.n(), - ])?; + let affected_rows = stmt_mark_transparent_utxo_spent.execute(spend_params)?; // Since we know that the output is spent, we no longer need to search for // it to find out if it has been spent. @@ -461,7 +467,24 @@ pub(crate) fn mark_transparent_utxo_spent( ":prevout_idx": outpoint.n(), ])?; - Ok(()) + // If no rows were affected, we know that we don't actually have the output in + // `transparent_received_outputs` yet, so we have to record the output as spent + // so that when we eventually detect the output, we can create the spend record. + if affected_rows == 0 { + conn.execute( + "INSERT INTO transparent_spend_map ( + spending_transaction_id, + prevout_txid, + prevout_output_index + ) + VALUES (:spent_in_tx, :prevout_txid, :prevout_idx) + ON CONFLICT (spending_transaction_id, prevout_txid, prevout_output_index) + DO NOTHING", + spend_params, + )?; + } + + Ok(affected_rows > 0) } /// Adds the given received UTXO to the datastore. @@ -728,6 +751,29 @@ pub(crate) fn put_transparent_output( let utxo_id = stmt_upsert_transparent_output .query_row(sql_args, |row| row.get::<_, i64>(0).map(UtxoId))?; + + // If we have a record of the output already having been spent, then mark it as spent using the + // stored reference to the spending transaction. + let spending_tx_ref = conn + .query_row( + "SELECT ts.spending_transaction_id + FROM transparent_spend_map ts + JOIN transactions t ON t.id_tx = ts.spending_transaction_id + WHERE ts.prevout_txid = :prevout_txid + AND ts.prevout_output_index = :prevout_idx + ORDER BY t.block NULLS LAST LIMIT 1", + named_params![ + ":prevout_txid": outpoint.txid().as_ref(), + ":prevout_idx": outpoint.n() + ], + |row| row.get::<_, i64>(0).map(TxRef), + ) + .optional()?; + + if let Some(spending_transaction_id) = spending_tx_ref { + mark_transparent_utxo_spent(conn, spending_transaction_id, outpoint)?; + } + Ok(utxo_id) }