diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 25a0f044c..0ce98cd68 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -19,6 +19,10 @@ and this library adheres to Rust's notion of - The `cmu` field of `zcash_client_backend::wallet::WalletShieldedOutput`. - `zcash_client_backend::proto::compact_formats::CompactSaplingOutput::cmu`. + +### Removed +- `zcash_client_backend::data_api`: + - `WalletWrite::remove_unmined_tx` (was behind the `unstable` feature flag). ## [0.6.1] - 2022-12-06 ### Added - `zcash_client_backend::data_api::chain::scan_cached_blocks` now generates diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 6379b186b..e7ab4cd67 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -361,14 +361,6 @@ pub trait WalletWrite: WalletRead { /// persistent wallet store. fn store_sent_tx(&mut self, sent_tx: &SentTransaction) -> Result; - /// Removes the specified unmined transaction from the persistent wallet store, if it - /// exists. - /// - /// Returns an error if the specified transaction has been mined. To remove a mined - /// transaction, first use [`WalletWrite::rewind_to_height`] to unmine it. - #[cfg(feature = "unstable")] - fn remove_unmined_tx(&mut self, txid: &TxId) -> Result<(), Self::Error>; - /// Rewinds the wallet database to the specified height. /// /// This method assumes that the state of the underlying data store is @@ -596,11 +588,6 @@ pub mod testing { Ok(TxId::from_bytes([0u8; 32])) } - #[cfg(feature = "unstable")] - fn remove_unmined_tx(&mut self, _txid: &TxId) -> Result<(), Self::Error> { - Ok(()) - } - fn rewind_to_height(&mut self, _block_height: BlockHeight) -> Result<(), Self::Error> { Ok(()) } diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 688db793f..48ae206cf 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -18,6 +18,8 @@ and this library adheres to Rust's notion of ### Changed - MSRV is now 1.60.0. +### Removed +- implementation of unstable function `WalletWrite::remove_unmined_tx`, ## [0.4.2] - 2022-12-13 ### Fixed - `zcash_client_sqlite::WalletDb::get_transparent_balances` no longer returns an diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 29c09ec75..cfd326b9c 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -11,9 +11,6 @@ use crate::PRUNING_HEIGHT; #[cfg(feature = "transparent-inputs")] use zcash_primitives::legacy::TransparentAddress; -#[cfg(feature = "unstable")] -use zcash_primitives::transaction::TxId; - /// The primary error type for the SQLite wallet backend. #[derive(Debug)] pub enum SqliteClientError { @@ -33,10 +30,6 @@ pub enum SqliteClientError { /// Illegal attempt to reinitialize an already-initialized wallet database. TableNotEmpty, - /// A transaction that has already been mined was requested for removal. - #[cfg(feature = "unstable")] - TransactionIsMined(TxId), - /// A Bech32-encoded key or address decoding error Bech32DecodeError(Bech32DecodeError), @@ -112,8 +105,6 @@ impl fmt::Display for SqliteClientError { SqliteClientError::HdwalletError(e) => write!(f, "{:?}", e), SqliteClientError::TransparentAddress(e) => write!(f, "{}", e), SqliteClientError::TableNotEmpty => write!(f, "Table is not empty"), - #[cfg(feature = "unstable")] - SqliteClientError::TransactionIsMined(txid) => write!(f, "Cannot remove transaction {} which has already been mined", txid), SqliteClientError::DbError(e) => write!(f, "{}", e), SqliteClientError::Io(e) => write!(f, "{}", e), SqliteClientError::InvalidMemo(e) => write!(f, "{}", e), diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index bc0b71781..a0c2cd00a 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -690,65 +690,6 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> { }) } - #[cfg(feature = "unstable")] - fn remove_unmined_tx(&mut self, txid: &TxId) -> Result<(), Self::Error> { - self.transactionally(|up| { - // Find the transaction to be deleted. - let tx_ref = up.stmt_select_tx_ref(txid)?; - - // Check that the transaction has not been mined (otherwise deleting it would - // require deleting child transactions that we haven't been authorized to - // remove). - if up - .wallet_db - .conn - .query_row_named( - "SELECT block FROM transactions WHERE id_tx = :tx", - &[(":tx", &tx_ref)], - |row| row.get::<_, Option>(0), - )? - .is_some() - { - return Err(SqliteClientError::TransactionIsMined(*txid)); - } - - // Delete received notes that were outputs of the given transaction. - up.wallet_db.conn.execute_named( - "DELETE FROM received_notes WHERE tx = :tx", - &[(":tx", &tx_ref)], - )?; - - // Unmine received notes that were inputs to the given transaction. - up.wallet_db.conn.execute_named( - "UPDATE received_notes SET spent = null WHERE spent = :tx", - &[(":tx", &tx_ref)], - )?; - - // We don't normally want to delete sent notes, as this can contain data that - // is not recoverable from the chain. However, given that the caller wants to - // delete the transaction in which these notes were sent, we cannot keep these - // database entries around. - up.wallet_db - .conn - .execute_named("DELETE FROM sent_notes WHERE tx = :tx", &[(":tx", &tx_ref)])?; - - // Delete the UTXOs because these are effectively cached and we don't have a - // good way of knowing whether they're spent. - up.wallet_db.conn.execute_named( - "DELETE FROM utxos WHERE spent_in_tx = :tx", - &[(":tx", &tx_ref)], - )?; - - // Now that it isn't depended on, delete the transaction. - up.wallet_db.conn.execute_named( - "DELETE FROM transactions WHERE id_tx = :tx", - &[(":tx", &tx_ref)], - )?; - - Ok(()) - }) - } - fn rewind_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error> { wallet::rewind_to_height(self.wallet_db, block_height) } @@ -1384,119 +1325,6 @@ mod tests { assert!(receivers.contains_key(&taddr)); } - #[cfg(feature = "unstable")] - #[test] - fn remove_unmined_tx_reverts_balance() { - use secrecy::Secret; - use tempfile::NamedTempFile; - use zcash_client_backend::data_api::{chain::scan_cached_blocks, WalletWrite}; - use zcash_primitives::zip32::ExtendedSpendingKey; - - use crate::{ - chain::init::init_cache_database, - error::SqliteClientError, - wallet::{get_balance, init::init_wallet_db, rewind_to_height}, - }; - - let cache_file = NamedTempFile::new().unwrap(); - let db_cache = BlockDb::for_path(cache_file.path()).unwrap(); - init_cache_database(&db_cache).unwrap(); - - let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), network()).unwrap(); - init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); - - // Add an account to the wallet. - let (dfvk, _taddr) = init_test_accounts_table(&db_data); - - // Account balance should be zero. - assert_eq!( - get_balance(&db_data, AccountId::from(0)).unwrap(), - Amount::zero() - ); - - // Create a fake CompactBlock sending value to the address. - let value = Amount::from_u64(5).unwrap(); - let (cb, nf) = fake_compact_block( - sapling_activation_height(), - BlockHash([0; 32]), - &dfvk, - AddressType::DefaultExternal, - value, - ); - insert_into_cache(&db_cache, &cb); - - // Create a second fake CompactBlock spending value from the address. - let extsk2 = ExtendedSpendingKey::master(&[0]); - let to2 = extsk2.default_address().1; - let value2 = Amount::from_u64(2).unwrap(); - let cb2 = fake_compact_block_spending( - sapling_activation_height() + 1, - cb.hash(), - (nf, value), - &dfvk, - to2, - value2, - ); - insert_into_cache(&db_cache, &cb2); - - // Scan the cache. - let mut db_write = db_data.get_update_ops().unwrap(); - scan_cached_blocks(&network(), &db_cache, &mut db_write, None).unwrap(); - - // Account balance should equal the change. - assert_eq!( - get_balance(&db_data, AccountId::from(0)).unwrap(), - (value - value2).unwrap() - ); - - // Attempting to remove the first transaction should fail because it is mined. - let mut db_ops = db_data.get_update_ops().unwrap(); - let txid = cb.vtx.get(0).unwrap().txid(); - assert!(matches!( - db_ops.remove_unmined_tx(&txid).unwrap_err(), - SqliteClientError::TransactionIsMined(x) if x == txid, - )); - - // Attempting to remove the second transaction should fail because it is mined. - let txid2 = cb2.vtx.get(0).unwrap().txid(); - assert!(matches!( - db_ops.remove_unmined_tx(&txid2).unwrap_err(), - SqliteClientError::TransactionIsMined(x) if x == txid2, - )); - - // Rewind the wallet by one block, to unmine the second transaction. - rewind_to_height(&db_data, sapling_activation_height()).unwrap(); - - // Account balance should be zero: the first transaction is spent, but the second - // transaction is unmined. - assert_eq!( - get_balance(&db_data, AccountId::from(0)).unwrap(), - Amount::zero() - ); - - // Attempting to remove the first transaction should still fail. - assert!(matches!( - db_ops.remove_unmined_tx(&txid).unwrap_err(), - SqliteClientError::TransactionIsMined(x) if x == txid, - )); - - // The second transaction can be removed. - db_ops.remove_unmined_tx(&txid2).unwrap(); - - // Account balance should now reflect the first received note. - assert_eq!(get_balance(&db_data, AccountId::from(0)).unwrap(), value); - - // Scan the cache again. - scan_cached_blocks(&network(), &db_cache, &mut db_write, None).unwrap(); - - // Account balance should once again equal the change. - assert_eq!( - get_balance(&db_data, AccountId::from(0)).unwrap(), - (value - value2).unwrap() - ); - } - #[cfg(feature = "unstable")] #[test] pub(crate) fn fsblockdb_api() {