Remove unstable function `WalletWrite::remove_unmined_tx`

Closes #622

Used PR https://github.com/zcash/librustzcash/pull/621 as reference to
know what had been added and remove it.
This commit is contained in:
Francisco Gindre 2023-01-31 15:21:46 -03:00
parent 7599fbd3c3
commit e9eb5f8c0f
5 changed files with 6 additions and 194 deletions

View File

@ -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

View File

@ -361,14 +361,6 @@ pub trait WalletWrite: WalletRead {
/// persistent wallet store.
fn store_sent_tx(&mut self, sent_tx: &SentTransaction) -> Result<Self::TxRef, Self::Error>;
/// 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(())
}

View File

@ -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

View File

@ -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),

View File

@ -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<u32>>(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() {