Merge pull request #770 from zcash/remove_remove_tx
Remove unstable function `WalletWrite::remove_unmined_tx`
This commit is contained in:
commit
8ca83c14be
|
@ -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
|
||||
|
|
|
@ -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(())
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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),
|
||||
|
|
|
@ -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() {
|
||||
|
|
Loading…
Reference in New Issue