Address comments from code review.

Co-authored-by: Daira Hopwood <daira@jacaranda.org>
This commit is contained in:
Kris Nuttycombe 2022-01-31 16:36:09 -07:00
parent 8916a16f38
commit 3d51c53d68
6 changed files with 68 additions and 77 deletions

View File

@ -27,7 +27,7 @@ use crate::{
};
#[cfg(feature = "transparent-inputs")]
use crate::data_api::{WalletWriteTransparent};
use crate::data_api::WalletWriteTransparent;
/// Scans a [`Transaction`] for any information that can be decrypted by the accounts in
/// the wallet, and saves it to the wallet.

View File

@ -47,8 +47,8 @@ pub enum SqliteClientError {
/// A requested rewind would violate invariants of the
/// storage layer. The payload returned with this error is
/// the safe rewind height.
RequestedRewindInvalid(BlockHeight),
/// (safe rewind height, requested height).
RequestedRewindInvalid(BlockHeight, BlockHeight),
/// Wrapper for errors from zcash_client_backend
BackendError(data_api::error::Error<NoteId>),
@ -76,8 +76,8 @@ impl fmt::Display for SqliteClientError {
SqliteClientError::InvalidNote => write!(f, "Invalid note"),
SqliteClientError::InvalidNoteId =>
write!(f, "The note ID associated with an inserted witness must correspond to a received note."),
SqliteClientError::RequestedRewindInvalid(h) =>
write!(f, "A rewind must be either of less than {} blocks, or at least back to block {} for your wallet.", PRUNING_HEIGHT, h),
SqliteClientError::RequestedRewindInvalid(h, r) =>
write!(f, "A rewind must be either of less than {} blocks, or at least back to block {} for your wallet; the requested height was {}.", PRUNING_HEIGHT, h, r),
SqliteClientError::Bech32(e) => write!(f, "{}", e),
SqliteClientError::Base58(e) => write!(f, "{}", e),
SqliteClientError::TransparentAddress(e) => write!(f, "{}", e),

View File

@ -551,7 +551,7 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
}
}
// Prune the stored witnesses (we only expect rollbacks of at most 100 blocks).
// Prune the stored witnesses (we only expect rollbacks of at most PRUNING_HEIGHT blocks).
wallet::prune_witnesses(up, block.block_height - PRUNING_HEIGHT)?;
// Update now-expired transactions that didn't get mined.

View File

@ -550,76 +550,66 @@ pub(crate) fn rewind_to_height<P: consensus::Parameters>(
.or(Ok(sapling_activation_height - 1))
})?;
let rewind_height = if block_height >= (last_scanned_height - PRUNING_HEIGHT) {
Some(block_height)
} else {
if block_height < last_scanned_height - PRUNING_HEIGHT {
#[allow(deprecated)]
match get_rewind_height(&wdb)? {
Some(h) => {
if block_height > h {
return Err(SqliteClientError::RequestedRewindInvalid(h));
} else {
Some(block_height)
}
if let Some(h) = get_rewind_height(&wdb)? {
if block_height > h {
return Err(SqliteClientError::RequestedRewindInvalid(h, block_height));
}
None => Some(block_height),
}
};
}
// nothing to do if we're deleting back down to the max height
if block_height < last_scanned_height {
// Decrement witnesses.
wdb.conn.execute(
"DELETE FROM sapling_witnesses WHERE block > ?",
&[u32::from(block_height)],
)?;
if let Some(block_height) = rewind_height {
if block_height < last_scanned_height {
// Decrement witnesses.
wdb.conn.execute(
"DELETE FROM sapling_witnesses WHERE block > ?",
&[u32::from(block_height)],
)?;
// Rewind received notes
wdb.conn.execute(
"DELETE FROM received_notes
WHERE id_note IN (
SELECT rn.id_note
FROM received_notes rn
LEFT OUTER JOIN transactions tx
ON tx.id_tx = rn.tx
WHERE tx.block > ?
);",
&[u32::from(block_height)],
)?;
// Rewind received notes
wdb.conn.execute(
"DELETE FROM received_notes
WHERE id_note IN (
SELECT rn.id_note
FROM received_notes rn
LEFT OUTER JOIN transactions tx
ON tx.id_tx = rn.tx
WHERE tx.block > ?
);",
&[u32::from(block_height)],
)?;
// Rewind sent notes
wdb.conn.execute(
"DELETE FROM sent_notes
WHERE id_note IN (
SELECT sn.id_note
FROM sent_notes sn
LEFT OUTER JOIN transactions tx
ON tx.id_tx = sn.tx
WHERE tx.block > ?
);",
&[u32::from(block_height)],
)?;
// Rewind sent notes
wdb.conn.execute(
"DELETE FROM sent_notes
WHERE id_note IN (
SELECT sn.id_note
FROM sent_notes sn
LEFT OUTER JOIN transactions tx
ON tx.id_tx = sn.tx
WHERE tx.block > ?
);",
&[u32::from(block_height)],
)?;
// Rewind utxos
wdb.conn.execute(
"DELETE FROM utxos WHERE height > ?",
&[u32::from(block_height)],
)?;
// Rewind utxos
wdb.conn.execute(
"DELETE FROM utxos WHERE height > ?",
&[u32::from(block_height)],
)?;
// Un-mine transactions.
wdb.conn.execute(
"UPDATE transactions SET block = NULL, tx_index = NULL WHERE block > ?",
&[u32::from(block_height)],
)?;
// Un-mine transactions.
wdb.conn.execute(
"UPDATE transactions SET block = NULL, tx_index = NULL WHERE block > ?",
&[u32::from(block_height)],
)?;
// Now that they aren't depended on, delete scanned blocks.
wdb.conn.execute(
"DELETE FROM blocks WHERE height > ?",
&[u32::from(block_height)],
)?;
}
// Now that they aren't depended on, delete scanned blocks.
wdb.conn.execute(
"DELETE FROM blocks WHERE height > ?",
&[u32::from(block_height)],
)?;
}
Ok(())
@ -983,7 +973,7 @@ pub fn delete_utxos_above<'a, P: consensus::Parameters>(
/// Records the specified shielded output as having been received.
///
/// This implementation assumes:
/// This implementation relies on the facts that:
/// - A transaction will not contain more than 2^63 shielded outputs.
/// - A note value will never exceed 2^63 zatoshis.
#[deprecated(
@ -1118,7 +1108,7 @@ pub fn put_sent_note<'a, P: consensus::Parameters>(
}
/// Adds information about a sent transparent UTXO to the database if it does not already
/// exist, or updates it if a record for the utxo already exists.
/// exist, or updates it if a record for the UTXO already exists.
///
/// `output_index` is the index within transparent UTXOs of the transaction that contains the recipient output.
#[deprecated(
@ -1134,7 +1124,7 @@ pub fn put_sent_utxo<'a, P: consensus::Parameters>(
value: Amount,
) -> Result<(), SqliteClientError> {
let ivalue: i64 = value.into();
// Try updating an existing sent utxo.
// Try updating an existing sent UTXO.
if stmts.stmt_update_sent_note.execute(params![
account.0 as i64,
encode_transparent_address_p(&stmts.wallet_db.params, &to),

View File

@ -21,12 +21,13 @@ use {
/// Sets up the internal structure of the data database.
///
/// The database structure is the same irrespective of whether or
/// not transparent spends are enabled, and it is safe to use a
/// wallet database created without the ability to create transparent
/// spends with a build that enables transparent spends (though not
/// the reverse, as wallet balance calculations would ignore the
/// prior transparent inputs controlled by the wallet).
/// It is safe to use a wallet database created without the ability to create transparent spends
/// with a build that enables transparent spends via use of the `transparent-inputs` feature flag.
/// The reverse is unsafe, as wallet balance calculations would ignore the transparent UTXOs
/// controlled by the wallet. Note that this currently applies only to wallet databases created
/// with the same _version_ of the wallet software; database migration operations currently must
/// be manually performed to update the structure of the database when changing versions.
/// Integrated migration utilities will be provided by a future version of this library.
///
/// # Examples
///

View File

@ -69,8 +69,8 @@ impl AccountPrivKey {
/// A type representing a BIP-44 public key at the account path level
/// `m/44'/<coin_type>'/<account>'`.
///
/// This provides the necessary derivation capability for the for
/// the transparent component of a unified full viewing key.
/// This provides the necessary derivation capability for the transparent component of a unified
/// full viewing key.
#[derive(Clone, Debug)]
pub struct AccountPubKey(ExtendedPubKey);