Move chain continuity checks into `scan_block_with_runner`

In preparation for out-of-order range-based scanning, it is necessary
to ensure that the size of the Sapling note commitment tree is carried
along through the scan process and that stored blocks are always
persisted with the updated note commitment tree size.

zcash_client_backend/CHANGELOG.md

@@ -11,14 +11,16 @@ and this library adheres to Rust's notion of
 - `impl Eq for zcash_client_backend::zip321::{Payment, TransactionRequest}`
 - `impl Debug` for `zcash_client_backend::{data_api::wallet::input_selection::Proposal, wallet::ReceivedSaplingNote}`
 - `zcash_client_backend::data_api`:
-  - `WalletRead::{fully_scanned_height, suggest_scan_ranges}`
+  - `WalletRead::{block_metadata, block_fully_scanned, suggest_scan_ranges}`
   - `WalletWrite::put_block`
   - `WalletCommitmentTrees`
   - `testing::MockWalletDb::new`
   - `NullifierQuery` for use with `WalletRead::get_sapling_nullifiers`
+  - `BlockMetadata`
+  - `ScannedBlock`
   - `wallet::input_sellection::Proposal::{min_target_height, min_anchor_height}`:
 - `zcash_client_backend::wallet::WalletSaplingOutput::note_commitment_tree_position`
-- `zcash_client_backend::welding_rig::SyncError`
+- `zcash_client_backend::welding_rig::ScanError`
 
 ### Changed
 - MSRV is now 1.65.0.
@@ -56,27 +58,28 @@ and this library adheres to Rust's notion of
 - `zcash_client_backend::welding_rig::ScanningKey::sapling_nf` has been changed to
   take a note position instead of an incremental witness for the note.
 - Arguments to `zcash_client_backend::welding_rig::scan_block` have changed. This
-  method now takes an optional `CommitmentTreeMeta` argument instead of a base commitment
+  method now takes an optional `BlockMetadata` argument instead of a base commitment
-  tree and incremental witnesses for each previously-known note.
+  tree and incremental witnesses for each previously-known note. In addition, the
+  return type has now been updated to return a `Result<ScannedBlock, ScanError>` 
+  in the case of scan failure.
 
 
 ### Removed
 - `zcash_client_backend::data_api`:
   - `WalletRead::get_all_nullifiers`
-  - `WalletRead::{get_commitment_tree, get_witnesses}` (use
+  - `WalletRead::{get_commitment_tree, get_witnesses}` have been removed 
-    `WalletRead::fully_scanned_height` instead).
+    without replacement. The utility of these methods is now subsumed
+    by those available from the `WalletCommitmentTrees` trait.
   - `WalletWrite::advance_by_block` (use `WalletWrite::put_block` instead).
-  - The `commitment_tree` and `transactions` properties of the `PrunedBlock`
+  - `PrunedBlock` has been replaced by `ScannedBlock`
-    struct are now owned values instead of references, making this a fully owned type.
-    It is now parameterized by the nullifier type instead of by a lifetime for the 
-    fields that were previously references. In addition, two new properties,
-    `sapling_commitment_tree_size` and `sapling_commitments` have been added.
   - `testing::MockWalletDb`, which is available under the `test-dependencies`
     feature flag, has been modified by the addition of a `sapling_tree` property. 
   - `wallet::input_selection`:
     - `Proposal::target_height` (use `Proposal::min_target_height` instead).
 - `zcash_client_backend::data_api::chain::validate_chain` TODO: document how
   to handle validation given out-of-order blocks.
+- `zcash_client_backend::data_api::chain::error::{ChainError, Cause}` have been 
+  replaced by `zcash_client_backend::welding_rig::ScanError`
 - `zcash_client_backend::wallet::WalletSaplingOutput::{witness, witness_mut}`
   have been removed as individual incremental witnesses are no longer tracked on a
   per-note basis. The global note commitment tree for the wallet should be used

zcash_client_backend/src/data_api.rs

@@ -66,15 +66,17 @@ pub trait WalletRead {
     /// This will return `Ok(None)` if no block data is present in the database.
     fn block_height_extrema(&self) -> Result<Option<(BlockHeight, BlockHeight)>, Self::Error>;
 
-    /// Returns the height to which the wallet has been fully scanned.
+    /// Returns the available block metadata for the block at the specified height, if any.
+    fn block_metadata(&self, height: BlockHeight) -> Result<Option<BlockMetadata>, Self::Error>;
+
+    /// Returns the metadata for the block at the height to which the wallet has been fully
+    /// scanned.
     ///
     /// This is the height for which the wallet has fully trial-decrypted this and all preceding
     /// blocks above the wallet's birthday height. Along with this height, this method returns
     /// metadata describing the state of the wallet's note commitment trees as of the end of that
     /// block.
-    fn fully_scanned_height(
+    fn block_fully_scanned(&self) -> Result<Option<BlockMetadata>, Self::Error>;
-        &self,
-    ) -> Result<Option<(BlockHeight, chain::CommitmentTreeMeta)>, Self::Error>;
 
     /// Returns a vector of suggested scan ranges based upon the current wallet state.
     ///
@@ -248,17 +250,99 @@ pub trait WalletRead {
     ) -> Result<HashMap<TransparentAddress, Amount>, Self::Error>;
 }
 
+/// Metadata describing the sizes of the zcash note commitment trees as of a particular block.
+#[derive(Debug, Clone, Copy)]
+pub struct BlockMetadata {
+    block_height: BlockHeight,
+    block_hash: BlockHash,
+    sapling_tree_size: u32,
+    //TODO: orchard_tree_size: u32
+}
+
+impl BlockMetadata {
+    /// Constructs a new [`BlockMetadata`] value from its constituent parts.
+    pub fn from_parts(
+        block_height: BlockHeight,
+        block_hash: BlockHash,
+        sapling_tree_size: u32,
+    ) -> Self {
+        Self {
+            block_height,
+            block_hash,
+            sapling_tree_size,
+        }
+    }
+
+    /// Returns the block height.
+    pub fn block_height(&self) -> BlockHeight {
+        self.block_height
+    }
+
+    /// Returns the hash of the block
+    pub fn block_hash(&self) -> BlockHash {
+        self.block_hash
+    }
+
+    /// Returns the size of the Sapling note commitment tree as of the block that this
+    /// [`BlockMetadata`] describes.
+    pub fn sapling_tree_size(&self) -> u32 {
+        self.sapling_tree_size
+    }
+}
+
 /// The subset of information that is relevant to this wallet that has been
 /// decrypted and extracted from a [`CompactBlock`].
 ///
 /// [`CompactBlock`]: crate::proto::compact_formats::CompactBlock
-pub struct PrunedBlock<Nf> {
+pub struct ScannedBlock<Nf> {
-    pub block_height: BlockHeight,
+    metadata: BlockMetadata,
-    pub block_hash: BlockHash,
+    block_time: u32,
-    pub block_time: u32,
+    transactions: Vec<WalletTx<Nf>>,
-    pub transactions: Vec<WalletTx<Nf>>,
+    sapling_commitments: Vec<(sapling::Node, Retention<BlockHeight>)>,
-    pub sapling_commitment_tree_size: Option<u32>,
+}
-    pub sapling_commitments: Vec<(sapling::Node, Retention<BlockHeight>)>,
+
+impl<Nf> ScannedBlock<Nf> {
+    pub fn from_parts(
+        metadata: BlockMetadata,
+        block_time: u32,
+        transactions: Vec<WalletTx<Nf>>,
+        sapling_commitments: Vec<(sapling::Node, Retention<BlockHeight>)>,
+    ) -> Self {
+        Self {
+            metadata,
+            block_time,
+            transactions,
+            sapling_commitments,
+        }
+    }
+
+    pub fn height(&self) -> BlockHeight {
+        self.metadata.block_height
+    }
+
+    pub fn block_hash(&self) -> BlockHash {
+        self.metadata.block_hash
+    }
+
+    pub fn block_time(&self) -> u32 {
+        self.block_time
+    }
+
+    pub fn metadata(&self) -> &BlockMetadata {
+        &self.metadata
+    }
+
+    pub fn transactions(&self) -> &[WalletTx<Nf>] {
+        &self.transactions
+    }
+
+    pub fn sapling_commitments(&self) -> &[(sapling::Node, Retention<BlockHeight>)] {
+        &self.sapling_commitments
+    }
+
+    pub fn take_sapling_commitments(self) -> Vec<(sapling::Node, Retention<BlockHeight>)> {
+        self.sapling_commitments
+    }
 }
 
 /// A transaction that was detected during scanning of the blockchain,
@@ -404,7 +488,7 @@ pub trait WalletWrite: WalletRead {
     #[allow(clippy::type_complexity)]
     fn put_block(
         &mut self,
-        block: PrunedBlock<sapling::Nullifier>,
+        block: ScannedBlock<sapling::Nullifier>,
     ) -> Result<Vec<Self::NoteRef>, Self::Error>;
 
     /// Caches a decrypted transaction in the persistent wallet store.
@@ -489,7 +573,7 @@ pub mod testing {
     };
 
     use super::{
-        chain, DecryptedTransaction, NullifierQuery, PrunedBlock, SentTransaction,
+        BlockMetadata, DecryptedTransaction, NullifierQuery, ScannedBlock, SentTransaction,
         WalletCommitmentTrees, WalletRead, WalletWrite, SAPLING_SHARD_HEIGHT,
     };
 
@@ -520,9 +604,14 @@ pub mod testing {
             Ok(None)
         }
 
-        fn fully_scanned_height(
+        fn block_metadata(
             &self,
-        ) -> Result<Option<(BlockHeight, chain::CommitmentTreeMeta)>, Self::Error> {
+            _height: BlockHeight,
+        ) -> Result<Option<BlockMetadata>, Self::Error> {
+            Ok(None)
+        }
+
+        fn block_fully_scanned(&self) -> Result<Option<BlockMetadata>, Self::Error> {
             Ok(None)
         }
 
@@ -667,7 +756,7 @@ pub mod testing {
         #[allow(clippy::type_complexity)]
         fn put_block(
             &mut self,
-            _block: PrunedBlock<sapling::Nullifier>,
+            _block: ScannedBlock<sapling::Nullifier>,
         ) -> Result<Vec<Self::NoteRef>, Self::Error> {
             Ok(vec![])
         }

zcash_client_backend/src/data_api/chain.rs

@@ -17,7 +17,6 @@
 //!             BlockSource,
 //!             error::Error,
 //!             scan_cached_blocks,
-//!             validate_chain,
 //!             testing as chain_testing,
 //!         },
 //!         testing,
@@ -30,7 +29,7 @@
 //! #   test();
 //! # }
 //! #
-//! # fn test() -> Result<(), Error<(), Infallible, u32>> {
+//! # fn test() -> Result<(), Error<(), Infallible>> {
 //! let network = Network::TestNetwork;
 //! let block_source = chain_testing::MockBlockSource;
 //! let mut db_data = testing::MockWalletDb::new(Network::TestNetwork);
@@ -65,26 +64,7 @@ use crate::{
 };
 
 pub mod error;
-use error::{ChainError, Error};
+use error::Error;
-
-/// Metadata describing the sizes of the zcash note commitment trees as of a particular block.
-pub struct CommitmentTreeMeta {
-    sapling_tree_size: u32,
-    //TODO: orchard_tree_size: u32
-}
-
-impl CommitmentTreeMeta {
-    /// Constructs a new [`CommitmentTreeMeta`] value from its constituent parts.
-    pub fn from_parts(sapling_tree_size: u32) -> Self {
-        Self { sapling_tree_size }
-    }
-
-    /// Returns the size of the Sapling note commitment tree as of the block that this
-    /// [`CommitmentTreeMeta`] describes.
-    pub fn sapling_tree_size(&self) -> u32 {
-        self.sapling_tree_size
-    }
-}
 
 /// This trait provides sequential access to raw blockchain data via a callback-oriented
 /// API.
@@ -99,14 +79,14 @@ pub trait BlockSource {
     ///   as part of processing each row.
     /// * `NoteRefT`: the type of note identifiers in the wallet data store, for use in
     ///   reporting errors related to specific notes.
-    fn with_blocks<F, WalletErrT, NoteRefT>(
+    fn with_blocks<F, WalletErrT>(
         &self,
         from_height: Option<BlockHeight>,
         limit: Option<u32>,
         with_row: F,
-    ) -> Result<(), error::Error<WalletErrT, Self::Error, NoteRefT>>
+    ) -> Result<(), error::Error<WalletErrT, Self::Error>>
     where
-        F: FnMut(CompactBlock) -> Result<(), error::Error<WalletErrT, Self::Error, NoteRefT>>;
+        F: FnMut(CompactBlock) -> Result<(), error::Error<WalletErrT, Self::Error>>;
 }
 
 /// Scans at most `limit` new blocks added to the block source for any transactions received by the
@@ -133,7 +113,7 @@ pub fn scan_cached_blocks<ParamsT, DbT, BlockSourceT>(
     data_db: &mut DbT,
     from_height: Option<BlockHeight>,
     limit: Option<u32>,
-) -> Result<(), Error<DbT::Error, BlockSourceT::Error, DbT::NoteRef>>
+) -> Result<(), Error<DbT::Error, BlockSourceT::Error>>
 where
     ParamsT: consensus::Parameters + Send + 'static,
     BlockSourceT: BlockSource,
@@ -169,74 +149,60 @@ where
     );
 
     // Start at either the provided height, or where we synced up to previously.
-    let (scan_from, commitment_tree_meta) = from_height.map_or_else(
+    let (scan_from, mut prior_block_metadata) = match from_height {
-        || {
+        Some(h) => {
-            data_db.fully_scanned_height().map_or_else(
+            // if we are provided with a starting height, obtain the metadata for the previous
-                |e| Err(Error::Wallet(e)),
+            // block (if any is available)
-                |last_scanned| {
+            (
-                    Ok(last_scanned.map_or_else(|| (None, None), |(h, m)| (Some(h + 1), Some(m))))
+                Some(h),
+                if h > BlockHeight::from(0) {
+                    data_db.block_metadata(h - 1).map_err(Error::Wallet)?
+                } else {
+                    None
                 },
             )
-        },
+        }
-        |h| Ok((Some(h), None)),
+        None => {
-    )?;
+            let last_scanned = data_db.block_fully_scanned().map_err(Error::Wallet)?;
+            last_scanned.map_or_else(|| (None, None), |m| (Some(m.block_height + 1), Some(m)))
+        }
+    };
 
-    block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>(
+    block_source.with_blocks::<_, DbT::Error>(scan_from, limit, |block: CompactBlock| {
-        scan_from,
+        add_block_to_runner(params, block, &mut batch_runner);
-        limit,
+        Ok(())
-        |block: CompactBlock| {
+    })?;
-            add_block_to_runner(params, block, &mut batch_runner);
-            Ok(())
-        },
-    )?;
 
     batch_runner.flush();
 
-    let mut last_scanned = None;
+    block_source.with_blocks::<_, DbT::Error>(scan_from, limit, |block: CompactBlock| {
-    block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>(
+        let scanned_block = scan_block_with_runner(
-        scan_from,
+            params,
-        limit,
+            block,
-        |block: CompactBlock| {
+            &dfvks,
-            // block heights should be sequential within a single scan range
+            &sapling_nullifiers,
-            let block_height = block.height();
+            prior_block_metadata.as_ref(),
-            if let Some(h) = last_scanned {
+            Some(&mut batch_runner),
-                if block_height != h + 1 {
+        )
-                    return Err(Error::Chain(ChainError::block_height_discontinuity(
+        .map_err(Error::Scan)?;
-                        block.height(),
-                        h,
-                    )));
-                }
-            }
 
-            let pruned_block = scan_block_with_runner(
+        let spent_nf: Vec<&sapling::Nullifier> = scanned_block
-                params,
+            .transactions
-                block,
+            .iter()
-                &dfvks,
+            .flat_map(|tx| tx.sapling_spends.iter().map(|spend| spend.nf()))
-                &sapling_nullifiers,
+            .collect();
-                commitment_tree_meta.as_ref(),
-                Some(&mut batch_runner),
-            )
-            .map_err(Error::Sync)?;
 
-            let spent_nf: Vec<&sapling::Nullifier> = pruned_block
+        sapling_nullifiers.retain(|(_, nf)| !spent_nf.contains(&nf));
-                .transactions
+        sapling_nullifiers.extend(scanned_block.transactions.iter().flat_map(|tx| {
+            tx.sapling_outputs
                 .iter()
-                .flat_map(|tx| tx.sapling_spends.iter().map(|spend| spend.nf()))
+                .map(|out| (out.account(), *out.nf()))
-                .collect();
+        }));
 
-            sapling_nullifiers.retain(|(_, nf)| !spent_nf.contains(&nf));
+        prior_block_metadata = Some(*scanned_block.metadata());
-            sapling_nullifiers.extend(pruned_block.transactions.iter().flat_map(|tx| {
+        data_db.put_block(scanned_block).map_err(Error::Wallet)?;
-                tx.sapling_outputs
+        Ok(())
-                    .iter()
+    })?;
-                    .map(|out| (out.account(), *out.nf()))
-            }));
-
-            data_db.put_block(pruned_block).map_err(Error::Wallet)?;
-
-            last_scanned = Some(block_height);
-            Ok(())
-        },
-    )?;
 
     Ok(())
 }
@@ -255,14 +221,14 @@ pub mod testing {
     impl BlockSource for MockBlockSource {
         type Error = Infallible;
 
-        fn with_blocks<F, DbErrT, NoteRef>(
+        fn with_blocks<F, DbErrT>(
             &self,
             _from_height: Option<BlockHeight>,
             _limit: Option<u32>,
             _with_row: F,
-        ) -> Result<(), Error<DbErrT, Infallible, NoteRef>>
+        ) -> Result<(), Error<DbErrT, Infallible>>
         where
-            F: FnMut(CompactBlock) -> Result<(), Error<DbErrT, Infallible, NoteRef>>,
+            F: FnMut(CompactBlock) -> Result<(), Error<DbErrT, Infallible>>,
         {
             Ok(())
         }

zcash_client_backend/src/data_api/chain/error.rs

@@ -3,136 +3,11 @@
 use std::error;
 use std::fmt::{self, Debug, Display};
 
-use zcash_primitives::{consensus::BlockHeight, sapling, transaction::TxId};
+use crate::welding_rig::ScanError;
-
-use crate::welding_rig::SyncError;
-
-/// The underlying cause of a [`ChainError`].
-#[derive(Copy, Clone, Debug)]
-pub enum Cause<NoteRef> {
-    /// The hash of the parent block given by a proposed new chain tip does not match the hash of
-    /// the current chain tip.
-    PrevHashMismatch,
-
-    /// The block height field of the proposed new chain tip is not equal to the height of the
-    /// previous chain tip + 1. This variant stores a copy of the incorrect height value for
-    /// reporting purposes.
-    BlockHeightDiscontinuity(BlockHeight),
-
-    /// The root of an output's witness tree in a newly arrived transaction does not correspond to
-    /// root of the stored commitment tree at the recorded height.
-    ///
-    /// This error is currently only produced when performing the slow checks that are enabled by
-    /// compiling with `-C debug-assertions`.
-    InvalidNewWitnessAnchor {
-        /// The id of the transaction containing the mismatched witness.
-        txid: TxId,
-        /// The index of the shielded output within the transaction where the witness root does not
-        /// match.
-        index: usize,
-        /// The root of the witness that failed to match the root of the current note commitment
-        /// tree.
-        node: sapling::Node,
-    },
-
-    /// The root of an output's witness tree in a previously stored transaction does not correspond
-    /// to root of the current commitment tree.
-    ///
-    /// This error is currently only produced when performing the slow checks that are enabled by
-    /// compiling with `-C debug-assertions`.
-    InvalidWitnessAnchor(NoteRef),
-}
-
-/// Errors that may occur in chain scanning or validation.
-#[derive(Copy, Clone, Debug)]
-pub struct ChainError<NoteRef> {
-    at_height: BlockHeight,
-    cause: Cause<NoteRef>,
-}
-
-impl<N: Display> fmt::Display for ChainError<N> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match &self.cause {
-            Cause::PrevHashMismatch => write!(
-                f,
-                "The parent hash of proposed block does not correspond to the block hash at height {}.",
-                self.at_height
-            ),
-            Cause::BlockHeightDiscontinuity(h) => {
-                write!(f, "Block height discontinuity at height {}; next height is : {}", self.at_height, h)
-            }
-            Cause::InvalidNewWitnessAnchor { txid, index, node } => write!(
-                f,
-                "New witness for output {} in tx {} at height {} has incorrect anchor: {:?}",
-                index, txid, self.at_height, node,
-            ),
-            Cause::InvalidWitnessAnchor(id_note) => {
-                write!(f, "Witness for note {} has incorrect anchor for height {}", id_note, self.at_height)
-            }
-        }
-    }
-}
-
-impl<NoteRef> ChainError<NoteRef> {
-    /// Constructs an error that indicates block hashes failed to chain.
-    ///
-    /// * `at_height` the height of the block whose parent hash does not match the hash of the
-    ///   previous block
-    pub fn prev_hash_mismatch(at_height: BlockHeight) -> Self {
-        ChainError {
-            at_height,
-            cause: Cause::PrevHashMismatch,
-        }
-    }
-
-    /// Constructs an error that indicates a gap in block heights.
-    ///
-    /// * `at_height` the height of the block being added to the chain.
-    /// * `prev_chain_tip` the height of the previous chain tip.
-    pub fn block_height_discontinuity(at_height: BlockHeight, prev_chain_tip: BlockHeight) -> Self {
-        ChainError {
-            at_height,
-            cause: Cause::BlockHeightDiscontinuity(prev_chain_tip),
-        }
-    }
-
-    /// Constructs an error that indicates a mismatch between an updated note's witness and the
-    /// root of the current note commitment tree.
-    pub fn invalid_witness_anchor(at_height: BlockHeight, note_ref: NoteRef) -> Self {
-        ChainError {
-            at_height,
-            cause: Cause::InvalidWitnessAnchor(note_ref),
-        }
-    }
-
-    /// Constructs an error that indicates a mismatch between a new note's witness and the root of
-    /// the current note commitment tree.
-    pub fn invalid_new_witness_anchor(
-        at_height: BlockHeight,
-        txid: TxId,
-        index: usize,
-        node: sapling::Node,
-    ) -> Self {
-        ChainError {
-            at_height,
-            cause: Cause::InvalidNewWitnessAnchor { txid, index, node },
-        }
-    }
-
-    /// Returns the block height at which this error was discovered.
-    pub fn at_height(&self) -> BlockHeight {
-        self.at_height
-    }
-
-    /// Returns the cause of this error.
-    pub fn cause(&self) -> &Cause<NoteRef> {
-        &self.cause
-    }
-}
 
 /// Errors related to chain validation and scanning.
 #[derive(Debug)]
-pub enum Error<WalletError, BlockSourceError, NoteRef> {
+pub enum Error<WalletError, BlockSourceError> {
     /// An error that was produced by wallet operations in the course of scanning the chain.
     Wallet(WalletError),
 
@@ -143,13 +18,10 @@ pub enum Error<WalletError, BlockSourceError, NoteRef> {
     /// A block that was received violated rules related to chain continuity or contained note
     /// commitments that could not be reconciled with the note commitment tree(s) maintained by the
     /// wallet.
-    Chain(ChainError<NoteRef>),
+    Scan(ScanError),
-
-    /// An error occorred in block scanning.
-    Sync(SyncError),
 }
 
-impl<WE: fmt::Display, BE: fmt::Display, N: Display> fmt::Display for Error<WE, BE, N> {
+impl<WE: fmt::Display, BE: fmt::Display> fmt::Display for Error<WE, BE> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match &self {
             Error::Wallet(e) => {
@@ -166,25 +38,17 @@ impl<WE: fmt::Display, BE: fmt::Display, N: Display> fmt::Display for Error<WE,
                     e
                 )
             }
-            Error::Chain(err) => {
+            Error::Scan(e) => {
-                write!(f, "{}", err)
+                write!(f, "Scanning produced the following error: {}", e)
-            }
-            Error::Sync(SyncError::SaplingTreeSizeUnknown(h)) => {
-                write!(
-                    f,
-                    "Sync failed due to missing Sapling note commitment tree size at height {}",
-                    h
-                )
             }
         }
     }
 }
 
-impl<WE, BE, N> error::Error for Error<WE, BE, N>
+impl<WE, BE> error::Error for Error<WE, BE>
 where
     WE: Debug + Display + error::Error + 'static,
     BE: Debug + Display + error::Error + 'static,
-    N: Debug + Display,
 {
     fn source(&self) -> Option<&(dyn error::Error + 'static)> {
         match &self {
@@ -195,8 +59,8 @@ where
     }
 }
 
-impl<WE, BSE, N> From<ChainError<N>> for Error<WE, BSE, N> {
+impl<WE, BSE> From<ScanError> for Error<WE, BSE> {
-    fn from(e: ChainError<N>) -> Self {
+    fn from(e: ScanError) -> Self {
-        Error::Chain(e)
+        Error::Scan(e)
     }
 }

zcash_client_backend/src/data_api/wallet.rs

@@ -1,5 +1,5 @@
-use std::{convert::Infallible, num::NonZeroU32};
 use std::fmt::Debug;
+use std::{convert::Infallible, num::NonZeroU32};
 
 use shardtree::{ShardStore, ShardTree, ShardTreeError};
 use zcash_primitives::{
@@ -394,7 +394,7 @@ pub fn propose_shielding<DbT, ParamsT, InputsT, CommitmentTreeErrT>(
     input_selector: &InputsT,
     shielding_threshold: NonNegativeAmount,
     from_addrs: &[TransparentAddress],
-    min_confirmations: NonZeroU32
+    min_confirmations: NonZeroU32,
 ) -> Result<
     Proposal<InputsT::FeeRule, DbT::NoteRef>,
     Error<
@@ -497,7 +497,7 @@ where
                 selected,
                 usk.sapling(),
                 &dfvk,
-                usize::try_from(u32::from(min_confirmations) - 1).unwrap()
+                usize::try_from(u32::from(min_confirmations) - 1).unwrap(),
             )?
             .ok_or(Error::NoteMismatch(selected.note_id))?;
 

zcash_client_backend/src/lib.rs

@@ -16,8 +16,8 @@ pub mod fees;
 pub mod keys;
 pub mod proto;
 pub mod scan;
+pub mod scanning;
 pub mod wallet;
-pub mod welding_rig;
 pub mod zip321;
 
 pub use decrypt::{decrypt_transaction, DecryptedOutput, TransferType};

zcash_client_backend/src/welding_rig.rs

@@ -4,6 +4,7 @@
 
 use std::collections::{HashMap, HashSet};
 use std::convert::TryFrom;
+use std::fmt::{self, Debug};
 
 use incrementalmerkletree::{Position, Retention};
 use subtle::{ConditionallySelectable, ConstantTimeEq, CtOption};
@@ -20,8 +21,7 @@ use zcash_primitives::{
     zip32::{sapling::DiversifiableFullViewingKey, AccountId, Scope},
 };
 
-use crate::data_api::chain::CommitmentTreeMeta;
+use crate::data_api::{BlockMetadata, ScannedBlock};
-use crate::data_api::PrunedBlock;
 use crate::{
     proto::compact_formats::CompactBlock,
     scan::{Batch, BatchRunner, Tasks},
@@ -109,12 +109,42 @@ impl ScanningKey for SaplingIvk {
     fn sapling_nf(_key: &Self::SaplingNk, _note: &sapling::Note, _position: Position) {}
 }
 
-/// Errors that can occur in block scanning.
+/// Errors that may occur in chain scanning
-#[derive(Debug)]
+#[derive(Copy, Clone, Debug)]
-pub enum SyncError {
+pub enum ScanError {
+    /// The hash of the parent block given by a proposed new chain tip does not match the hash of
+    /// the current chain tip.
+    PrevHashMismatch { at_height: BlockHeight },
+
+    /// The block height field of the proposed new chain tip is not equal to the height of the
+    /// previous chain tip + 1. This variant stores a copy of the incorrect height value for
+    /// reporting purposes.
+    BlockHeightDiscontinuity {
+        previous_tip: BlockHeight,
+        new_height: BlockHeight,
+    },
+
     /// The size of the Sapling note commitment tree was not provided as part of a [`CompactBlock`]
     /// being scanned, making it impossible to construct the nullifier for a detected note.
-    SaplingTreeSizeUnknown(BlockHeight),
+    SaplingTreeSizeUnknown { at_height: BlockHeight },
+}
+
+impl fmt::Display for ScanError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match &self {
+            ScanError::PrevHashMismatch { at_height } => write!(
+                f,
+                "The parent hash of proposed block does not correspond to the block hash at height {}.",
+                at_height
+            ),
+            ScanError::BlockHeightDiscontinuity { previous_tip, new_height } => {
+                write!(f, "Block height discontinuity at height {}; next height is : {}", previous_tip, new_height)
+            }
+            ScanError::SaplingTreeSizeUnknown { at_height } => {
+                write!(f, "Unable to determine Sapling note commitment tree size at height {}", at_height)
+            }
+        }
+    }
 }
 
 /// Scans a [`CompactBlock`] with a set of [`ScanningKey`]s.
@@ -145,14 +175,14 @@ pub fn scan_block<P: consensus::Parameters + Send + 'static, K: ScanningKey>(
     block: CompactBlock,
     vks: &[(&AccountId, &K)],
     sapling_nullifiers: &[(AccountId, sapling::Nullifier)],
-    initial_commitment_tree_meta: Option<&CommitmentTreeMeta>,
+    prior_block_metadata: Option<&BlockMetadata>,
-) -> Result<PrunedBlock<K::Nf>, SyncError> {
+) -> Result<ScannedBlock<K::Nf>, ScanError> {
     scan_block_with_runner::<_, _, ()>(
         params,
         block,
         vks,
         sapling_nullifiers,
-        initial_commitment_tree_meta,
+        prior_block_metadata,
         None,
     )
 }
@@ -204,13 +234,28 @@ pub(crate) fn scan_block_with_runner<
     block: CompactBlock,
     vks: &[(&AccountId, &K)],
     nullifiers: &[(AccountId, sapling::Nullifier)],
-    initial_commitment_tree_meta: Option<&CommitmentTreeMeta>,
+    prior_block_metadata: Option<&BlockMetadata>,
     mut batch_runner: Option<&mut TaggedBatchRunner<P, K::Scope, T>>,
-) -> Result<PrunedBlock<K::Nf>, SyncError> {
+) -> Result<ScannedBlock<K::Nf>, ScanError> {
     let mut wtxs: Vec<WalletTx<K::Nf>> = vec![];
     let mut sapling_note_commitments: Vec<(sapling::Node, Retention<BlockHeight>)> = vec![];
-    let block_height = block.height();
+    let cur_height = block.height();
-    let block_hash = block.hash();
+    let cur_hash = block.hash();
+
+    if let Some(prev) = prior_block_metadata {
+        if cur_height != prev.block_height() + 1 {
+            return Err(ScanError::BlockHeightDiscontinuity {
+                previous_tip: prev.block_height(),
+                new_height: cur_height,
+            });
+        }
+
+        if block.prev_hash() != prev.block_hash() {
+            return Err(ScanError::PrevHashMismatch {
+                at_height: cur_height,
+            });
+        }
+    }
 
     // It's possible to make progress without a Sapling tree position if we don't have any Sapling
     // notes in the block, since we only use the position for constructing nullifiers for our own
@@ -235,7 +280,10 @@ pub(crate) fn scan_block_with_runner<
                 Some(m.sapling_commitment_tree_size - block_note_count)
             }
         })
-        .or_else(|| initial_commitment_tree_meta.map(|m| m.sapling_tree_size()));
+        .or_else(|| prior_block_metadata.map(|m| m.sapling_tree_size()))
+        .ok_or(ScanError::SaplingTreeSizeUnknown {
+            at_height: cur_height,
+        })?;
 
     let block_tx_count = block.vtx.len();
     for (tx_idx, tx) in block.vtx.into_iter().enumerate() {
@@ -283,7 +331,7 @@ pub(crate) fn scan_block_with_runner<
                 .into_iter()
                 .map(|output| {
                     (
-                        SaplingDomain::for_height(params.clone(), block_height),
+                        SaplingDomain::for_height(params.clone(), cur_height),
                         CompactOutputDescription::try_from(output)
                             .expect("Invalid output found in compact block decoding."),
                     )
@@ -300,7 +348,7 @@ pub(crate) fn scan_block_with_runner<
                     })
                     .collect::<HashMap<_, _>>();
 
-                let mut decrypted = runner.collect_results(block_hash, txid);
+                let mut decrypted = runner.collect_results(cur_hash, txid);
                 (0..decoded.len())
                     .map(|i| {
                         decrypted.remove(&(txid, i)).map(|d_note| {
@@ -347,7 +395,7 @@ pub(crate) fn scan_block_with_runner<
                 let is_checkpoint = output_idx + 1 == decoded.len() && tx_idx + 1 == block_tx_count;
                 let retention = match (dec_output.is_some(), is_checkpoint) {
                     (is_marked, true) => Retention::Checkpoint {
-                        id: block_height,
+                        id: cur_height,
                         is_marked,
                     },
                     (true, false) => Retention::Marked,
@@ -362,9 +410,9 @@ pub(crate) fn scan_block_with_runner<
                     // - Notes created by consolidation transactions.
                     // - Notes sent from one account to itself.
                     let is_change = spent_from_accounts.contains(&account);
-                    let note_commitment_tree_position = sapling_commitment_tree_size
+                    let note_commitment_tree_position = Position::from(u64::from(
-                        .map(|s| Position::from(u64::from(s + u32::try_from(output_idx).unwrap())))
+                        sapling_commitment_tree_size + u32::try_from(output_idx).unwrap(),
-                        .ok_or(SyncError::SaplingTreeSizeUnknown(block_height))?;
+                    ));
                     let nf = K::sapling_nf(&nk, &note, note_commitment_tree_position);
 
                     shielded_outputs.push(WalletSaplingOutput::from_parts(
@@ -392,17 +440,15 @@ pub(crate) fn scan_block_with_runner<
             });
         }
 
-        sapling_commitment_tree_size = sapling_commitment_tree_size.map(|s| s + tx_outputs_len);
+        sapling_commitment_tree_size += tx_outputs_len;
     }
 
-    Ok(PrunedBlock {
+    Ok(ScannedBlock::from_parts(
-        block_height,
+        BlockMetadata::from_parts(cur_height, cur_hash, sapling_commitment_tree_size),
-        block_hash,
+        block.time,
-        block_time: block.time,
+        wtxs,
-        transactions: wtxs,
+        sapling_note_commitments,
-        sapling_commitment_tree_size,
+    ))
-        sapling_commitments: sapling_note_commitments,
-    })
 }
 
 #[cfg(test)]
@@ -415,6 +461,7 @@ mod tests {
     use rand_core::{OsRng, RngCore};
     use zcash_note_encryption::Domain;
     use zcash_primitives::{
+        block::BlockHash,
         consensus::{BlockHeight, Network},
         constants::SPENDING_KEY_GENERATOR,
         memo::MemoBytes,
@@ -430,9 +477,9 @@ mod tests {
     };
 
     use crate::{
-        data_api::chain::CommitmentTreeMeta,
+        data_api::BlockMetadata,
         proto::compact_formats::{
-            BlockMetadata, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx,
+            self as compact, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx,
         },
         scan::BatchRunner,
     };
@@ -479,6 +526,7 @@ mod tests {
     /// from a `lightwalletd` that is not currently tracking note commitment tree sizes.
     fn fake_compact_block(
         height: BlockHeight,
+        prev_hash: BlockHash,
         nf: Nullifier,
         dfvk: &DiversifiableFullViewingKey,
         value: Amount,
@@ -510,6 +558,7 @@ mod tests {
                 rng.fill_bytes(&mut hash);
                 hash
             },
+            prev_hash: prev_hash.0.to_vec(),
             height: height.into(),
             ..Default::default()
         };
@@ -543,7 +592,7 @@ mod tests {
             cb.vtx.push(tx);
         }
 
-        cb.block_metadata = initial_sapling_tree_size.map(|s| BlockMetadata {
+        cb.block_metadata = initial_sapling_tree_size.map(|s| compact::BlockMetadata {
             sapling_commitment_tree_size: s + cb
                 .vtx
                 .iter()
@@ -564,6 +613,7 @@ mod tests {
 
             let cb = fake_compact_block(
                 1u32.into(),
+                BlockHash([0; 32]),
                 Nullifier([0; 32]),
                 &dfvk,
                 Amount::from_u64(5).unwrap(),
@@ -589,16 +639,20 @@ mod tests {
                 None
             };
 
-            let pruned_block = scan_block_with_runner(
+            let scanned_block = scan_block_with_runner(
                 &Network::TestNetwork,
                 cb,
                 &[(&account, &dfvk)],
                 &[],
-                Some(&CommitmentTreeMeta::from_parts(0)),
+                Some(&BlockMetadata::from_parts(
+                    BlockHeight::from(0),
+                    BlockHash([0u8; 32]),
+                    0,
+                )),
                 batch_runner.as_mut(),
             )
             .unwrap();
-            let txs = pruned_block.transactions;
+            let txs = scanned_block.transactions();
             assert_eq!(txs.len(), 1);
 
             let tx = &txs[0];
@@ -613,17 +667,17 @@ mod tests {
                 Position::from(1)
             );
 
-            assert_eq!(pruned_block.sapling_commitment_tree_size, Some(2));
+            assert_eq!(scanned_block.metadata().sapling_tree_size(), 2);
             assert_eq!(
-                pruned_block
+                scanned_block
-                    .sapling_commitments
+                    .sapling_commitments()
                     .iter()
                     .map(|(_, retention)| *retention)
                     .collect::<Vec<_>>(),
                 vec![
                     Retention::Ephemeral,
                     Retention::Checkpoint {
-                        id: pruned_block.block_height,
+                        id: scanned_block.height(),
                         is_marked: true
                     }
                 ]
@@ -643,6 +697,7 @@ mod tests {
 
             let cb = fake_compact_block(
                 1u32.into(),
+                BlockHash([0; 32]),
                 Nullifier([0; 32]),
                 &dfvk,
                 Amount::from_u64(5).unwrap(),
@@ -668,7 +723,7 @@ mod tests {
                 None
             };
 
-            let pruned_block = scan_block_with_runner(
+            let scanned_block = scan_block_with_runner(
                 &Network::TestNetwork,
                 cb,
                 &[(&AccountId::from(0), &dfvk)],
@@ -677,7 +732,7 @@ mod tests {
                 batch_runner.as_mut(),
             )
             .unwrap();
-            let txs = pruned_block.transactions;
+            let txs = scanned_block.transactions();
             assert_eq!(txs.len(), 1);
 
             let tx = &txs[0];
@@ -689,8 +744,8 @@ mod tests {
             assert_eq!(tx.sapling_outputs[0].note().value().inner(), 5);
 
             assert_eq!(
-                pruned_block
+                scanned_block
-                    .sapling_commitments
+                    .sapling_commitments()
                     .iter()
                     .map(|(_, retention)| *retention)
                     .collect::<Vec<_>>(),
@@ -698,7 +753,7 @@ mod tests {
                     Retention::Ephemeral,
                     Retention::Marked,
                     Retention::Checkpoint {
-                        id: pruned_block.block_height,
+                        id: scanned_block.height(),
                         is_marked: false
                     }
                 ]
@@ -718,6 +773,7 @@ mod tests {
 
         let cb = fake_compact_block(
             1u32.into(),
+            BlockHash([0; 32]),
             nf,
             &dfvk,
             Amount::from_u64(5).unwrap(),
@@ -727,15 +783,9 @@ mod tests {
         assert_eq!(cb.vtx.len(), 2);
         let vks: Vec<(&AccountId, &SaplingIvk)> = vec![];
 
-        let pruned_block = scan_block(
+        let scanned_block =
-            &Network::TestNetwork,
+            scan_block(&Network::TestNetwork, cb, &vks[..], &[(account, nf)], None).unwrap();
-            cb,
+        let txs = scanned_block.transactions();
-            &vks[..],
-            &[(account, nf)],
-            Some(&CommitmentTreeMeta::from_parts(0)),
-        )
-        .unwrap();
-        let txs = pruned_block.transactions;
         assert_eq!(txs.len(), 1);
 
         let tx = &txs[0];
@@ -747,15 +797,15 @@ mod tests {
         assert_eq!(tx.sapling_spends[0].account(), account);
 
         assert_eq!(
-            pruned_block
+            scanned_block
-                .sapling_commitments
+                .sapling_commitments()
                 .iter()
                 .map(|(_, retention)| *retention)
                 .collect::<Vec<_>>(),
             vec![
                 Retention::Ephemeral,
                 Retention::Checkpoint {
-                    id: pruned_block.block_height,
+                    id: scanned_block.height(),
                     is_marked: false
                 }
             ]

zcash_client_sqlite/CHANGELOG.md

@@ -19,10 +19,16 @@ and this library adheres to Rust's notion of
   a note could be spent with fewer than `min_confirmations` confirmations if the
   wallet did not contain enough observed blocks to satisfy the `min_confirmations`
   value specified; this situation is now treated as an error.
+- A `BlockConflict` variant has been added to `zcash_client_sqlite::error::SqliteClientError`
 
 ### Removed
 - The empty `wallet::transact` module has been removed.
 
+### Fixed
+- Fixed an off-by-one error in the `BlockSource` implementation for the SQLite-backed
+ `BlockDb` block database which could result in blocks being skipped at the start of
+ scan ranges.
+
 ## [0.7.1] - 2023-05-17
 
 ### Fixed

zcash_client_sqlite/src/chain.rs

@@ -26,16 +26,16 @@ pub mod migrations;
 /// Starting at `from_height`, the `with_row` callback is invoked with each block retrieved from
 /// the backing store. If the `limit` value provided is `None`, all blocks are traversed up to the
 /// maximum height.
-pub(crate) fn blockdb_with_blocks<F, DbErrT, NoteRef>(
+pub(crate) fn blockdb_with_blocks<F, DbErrT>(
     block_source: &BlockDb,
     from_height: Option<BlockHeight>,
     limit: Option<u32>,
     mut with_row: F,
-) -> Result<(), Error<DbErrT, SqliteClientError, NoteRef>>
+) -> Result<(), Error<DbErrT, SqliteClientError>>
 where
-    F: FnMut(CompactBlock) -> Result<(), Error<DbErrT, SqliteClientError, NoteRef>>,
+    F: FnMut(CompactBlock) -> Result<(), Error<DbErrT, SqliteClientError>>,
 {
-    fn to_chain_error<D, E: Into<SqliteClientError>, N>(err: E) -> Error<D, SqliteClientError, N> {
+    fn to_chain_error<D, E: Into<SqliteClientError>>(err: E) -> Error<D, SqliteClientError> {
         Error::BlockSource(err.into())
     }
 
@@ -195,16 +195,16 @@ pub(crate) fn blockmetadb_find_block(
 /// the backing store. If the `limit` value provided is `None`, all blocks are traversed up to the
 /// maximum height for which metadata is available.
 #[cfg(feature = "unstable")]
-pub(crate) fn fsblockdb_with_blocks<F, DbErrT, NoteRef>(
+pub(crate) fn fsblockdb_with_blocks<F, DbErrT>(
     cache: &FsBlockDb,
     from_height: Option<BlockHeight>,
     limit: Option<u32>,
     mut with_block: F,
-) -> Result<(), Error<DbErrT, FsBlockDbError, NoteRef>>
+) -> Result<(), Error<DbErrT, FsBlockDbError>>
 where
-    F: FnMut(CompactBlock) -> Result<(), Error<DbErrT, FsBlockDbError, NoteRef>>,
+    F: FnMut(CompactBlock) -> Result<(), Error<DbErrT, FsBlockDbError>>,
 {
-    fn to_chain_error<D, E: Into<FsBlockDbError>, N>(err: E) -> Error<D, FsBlockDbError, N> {
+    fn to_chain_error<D, E: Into<FsBlockDbError>>(err: E) -> Error<D, FsBlockDbError> {
         Error::BlockSource(err.into())
     }
 

zcash_client_sqlite/src/error.rs

@@ -53,13 +53,15 @@ pub enum SqliteClientError {
     /// A received memo cannot be interpreted as a UTF-8 string.
     InvalidMemo(zcash_primitives::memo::Error),
 
-    /// A requested rewind would violate invariants of the
+    /// An attempt to update block data would overwrite the current hash for a block with a
-    /// storage layer. The payload returned with this error is
+    /// different hash. This indicates that a required rewind was not performed.
-    /// (safe rewind height, requested height).
+    BlockConflict(BlockHeight),
+
+    /// A requested rewind would violate invariants of the storage layer. The payload returned with
+    /// this error is (safe rewind height, requested height).
     RequestedRewindInvalid(BlockHeight, BlockHeight),
 
-    /// The space of allocatable diversifier indices has been exhausted for
+    /// The space of allocatable diversifier indices has been exhausted for the given account.
-    /// the given account.
     DiversifierIndexOutOfRange,
 
     /// An error occurred deriving a spending key from a seed and an account
@@ -115,6 +117,7 @@ impl fmt::Display for SqliteClientError {
             SqliteClientError::DbError(e) => write!(f, "{}", e),
             SqliteClientError::Io(e) => write!(f, "{}", e),
             SqliteClientError::InvalidMemo(e) => write!(f, "{}", e),
+            SqliteClientError::BlockConflict(h) => write!(f, "A block hash conflict occurred at height {}; rewind required.", u32::from(*h)),
             SqliteClientError::DiversifierIndexOutOfRange => write!(f, "The space of available diversifier indices is exhausted"),
             SqliteClientError::KeyDerivationError(acct_id) => write!(f, "Key derivation failed for account {:?}", acct_id),
             SqliteClientError::AccountIdDiscontinuity => write!(f, "Wallet account identifiers must be sequential."),

zcash_client_sqlite/src/lib.rs

@@ -55,10 +55,9 @@ use zcash_primitives::{
 use zcash_client_backend::{
     address::{AddressMetadata, UnifiedAddress},
     data_api::{
-        self,
+        self, chain::BlockSource, BlockMetadata, DecryptedTransaction, NullifierQuery, PoolType,
-        chain::{BlockSource, CommitmentTreeMeta},
+        Recipient, ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, WalletWrite,
-        DecryptedTransaction, NullifierQuery, PoolType, PrunedBlock, Recipient, SentTransaction,
+        SAPLING_SHARD_HEIGHT,
-        WalletCommitmentTrees, WalletRead, WalletWrite, SAPLING_SHARD_HEIGHT,
     },
     keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
     proto::compact_formats::CompactBlock,
@@ -85,7 +84,7 @@ pub mod wallet;
 /// this delta from the chain tip to be pruned.
 pub(crate) const PRUNING_HEIGHT: u32 = 100;
 
-pub(crate) const SAPLING_TABLES_PREFIX: &'static str = "sapling";
+pub(crate) const SAPLING_TABLES_PREFIX: &str = "sapling";
 
 /// A newtype wrapper for sqlite primary key values for the notes
 /// table.
@@ -157,10 +156,12 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
         wallet::block_height_extrema(self.conn.borrow()).map_err(SqliteClientError::from)
     }
 
-    fn fully_scanned_height(
+    fn block_metadata(&self, height: BlockHeight) -> Result<Option<BlockMetadata>, Self::Error> {
-        &self,
+        wallet::block_metadata(self.conn.borrow(), height)
-    ) -> Result<Option<(BlockHeight, CommitmentTreeMeta)>, Self::Error> {
+    }
-        wallet::fully_scanned_height(self.conn.borrow())
+
+    fn block_fully_scanned(&self) -> Result<Option<BlockMetadata>, Self::Error> {
+        wallet::block_fully_scanned(self.conn.borrow())
     }
 
     fn suggest_scan_ranges(
@@ -183,6 +184,14 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
         wallet::get_tx_height(self.conn.borrow(), txid).map_err(SqliteClientError::from)
     }
 
+    fn get_current_address(
+        &self,
+        account: AccountId,
+    ) -> Result<Option<UnifiedAddress>, Self::Error> {
+        wallet::get_current_address(self.conn.borrow(), &self.params, account)
+            .map(|res| res.map(|(addr, _)| addr))
+    }
+
     fn get_unified_full_viewing_keys(
         &self,
     ) -> Result<HashMap<AccountId, UnifiedFullViewingKey>, Self::Error> {
@@ -196,14 +205,6 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
         wallet::get_account_for_ufvk(self.conn.borrow(), &self.params, ufvk)
     }
 
-    fn get_current_address(
-        &self,
-        account: AccountId,
-    ) -> Result<Option<UnifiedAddress>, Self::Error> {
-        wallet::get_current_address(self.conn.borrow(), &self.params, account)
-            .map(|res| res.map(|(addr, _)| addr))
-    }
-
     fn is_valid_account_extfvk(
         &self,
         account: AccountId,
@@ -220,10 +221,6 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
         wallet::get_balance_at(self.conn.borrow(), account, anchor_height)
     }
 
-    fn get_transaction(&self, id_tx: i64) -> Result<Transaction, Self::Error> {
-        wallet::get_transaction(self.conn.borrow(), &self.params, id_tx)
-    }
-
     fn get_memo(&self, id_note: Self::NoteRef) -> Result<Option<Memo>, Self::Error> {
         match id_note {
             NoteId::SentNoteId(id_note) => wallet::get_sent_memo(self.conn.borrow(), id_note),
@@ -233,6 +230,10 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
         }
     }
 
+    fn get_transaction(&self, id_tx: i64) -> Result<Transaction, Self::Error> {
+        wallet::get_transaction(self.conn.borrow(), &self.params, id_tx)
+    }
+
     fn get_sapling_nullifiers(
         &self,
         query: NullifierQuery,
@@ -390,25 +391,25 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
         )
     }
 
-    #[tracing::instrument(skip_all, fields(height = u32::from(block.block_height)))]
+    #[tracing::instrument(skip_all, fields(height = u32::from(block.height())))]
     #[allow(clippy::type_complexity)]
     fn put_block(
         &mut self,
-        block: PrunedBlock<sapling::Nullifier>,
+        block: ScannedBlock<sapling::Nullifier>,
     ) -> Result<Vec<Self::NoteRef>, Self::Error> {
         self.transactionally(|wdb| {
             // Insert the block into the database.
             wallet::put_block(
                 wdb.conn.0,
-                block.block_height,
+                block.height(),
-                block.block_hash,
+                block.block_hash(),
-                block.block_time,
+                block.block_time(),
-                block.sapling_commitment_tree_size.map(|s| s.into()),
+                block.metadata().sapling_tree_size(),
             )?;
 
             let mut wallet_note_ids = vec![];
-            for tx in &block.transactions {
+            for tx in block.transactions() {
-                let tx_row = wallet::put_tx_meta(wdb.conn.0, tx, block.block_height)?;
+                let tx_row = wallet::put_tx_meta(wdb.conn.0, tx, block.height())?;
 
                 // Mark notes as spent and remove them from the scanning cache
                 for spend in &tx.sapling_spends {
@@ -424,19 +425,19 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                 }
             }
 
-            let sapling_commitments_len = block.sapling_commitments.len();
+            let block_height = block.height();
-            let mut sapling_commitments = block.sapling_commitments.into_iter();
+            let sapling_tree_size = block.metadata().sapling_tree_size();
+            let sapling_commitments_len = block.sapling_commitments().len();
+            let mut sapling_commitments = block.take_sapling_commitments().into_iter();
             wdb.with_sapling_tree_mut::<_, _, SqliteClientError>(move |sapling_tree| {
-                if let Some(sapling_tree_size) = block.sapling_commitment_tree_size {
+                let start_position = Position::from(u64::from(sapling_tree_size))
-                    let start_position = Position::from(u64::from(sapling_tree_size))
+                    - u64::try_from(sapling_commitments_len).unwrap();
-                        - u64::try_from(sapling_commitments_len).unwrap();
+                sapling_tree.batch_insert(start_position, &mut sapling_commitments)?;
-                    sapling_tree.batch_insert(start_position, &mut sapling_commitments)?;
-                }
                 Ok(())
             })?;
 
             // Update now-expired transactions that didn't get mined.
-            wallet::update_expired_notes(wdb.conn.0, block.block_height)?;
+            wallet::update_expired_notes(wdb.conn.0, block_height)?;
 
             Ok(wallet_note_ids)
         })
@@ -688,17 +689,14 @@ impl BlockDb {
 impl BlockSource for BlockDb {
     type Error = SqliteClientError;
 
-    fn with_blocks<F, DbErrT, NoteRef>(
+    fn with_blocks<F, DbErrT>(
         &self,
         from_height: Option<BlockHeight>,
         limit: Option<u32>,
         with_row: F,
-    ) -> Result<(), data_api::chain::error::Error<DbErrT, Self::Error, NoteRef>>
+    ) -> Result<(), data_api::chain::error::Error<DbErrT, Self::Error>>
     where
-        F: FnMut(
+        F: FnMut(CompactBlock) -> Result<(), data_api::chain::error::Error<DbErrT, Self::Error>>,
-            CompactBlock,
-        )
-            -> Result<(), data_api::chain::error::Error<DbErrT, Self::Error, NoteRef>>,
     {
         chain::blockdb_with_blocks(self, from_height, limit, with_row)
     }
@@ -869,17 +867,14 @@ impl FsBlockDb {
 impl BlockSource for FsBlockDb {
     type Error = FsBlockDbError;
 
-    fn with_blocks<F, DbErrT, NoteRef>(
+    fn with_blocks<F, DbErrT>(
         &self,
         from_height: Option<BlockHeight>,
         limit: Option<u32>,
         with_row: F,
-    ) -> Result<(), data_api::chain::error::Error<DbErrT, Self::Error, NoteRef>>
+    ) -> Result<(), data_api::chain::error::Error<DbErrT, Self::Error>>
     where
-        F: FnMut(
+        F: FnMut(CompactBlock) -> Result<(), data_api::chain::error::Error<DbErrT, Self::Error>>,
-            CompactBlock,
-        )
-            -> Result<(), data_api::chain::error::Error<DbErrT, Self::Error, NoteRef>>,
     {
         fsblockdb_with_blocks(self, from_height, limit, with_row)
     }
@@ -967,7 +962,7 @@ mod tests {
         data_api::{WalletRead, WalletWrite},
         keys::{sapling, UnifiedFullViewingKey},
         proto::compact_formats::{
-            BlockMetadata, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx,
+            self as compact, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx,
         },
     };
 
@@ -1112,7 +1107,7 @@ mod tests {
         };
         cb.prev_hash.extend_from_slice(&prev_hash.0);
         cb.vtx.push(ctx);
-        cb.block_metadata = Some(BlockMetadata {
+        cb.block_metadata = Some(compact::BlockMetadata {
             sapling_commitment_tree_size: initial_sapling_tree_size
                 + cb.vtx.iter().map(|tx| tx.outputs.len() as u32).sum::<u32>(),
             ..Default::default()
@@ -1203,7 +1198,7 @@ mod tests {
         };
         cb.prev_hash.extend_from_slice(&prev_hash.0);
         cb.vtx.push(ctx);
-        cb.block_metadata = Some(BlockMetadata {
+        cb.block_metadata = Some(compact::BlockMetadata {
             sapling_commitment_tree_size: initial_sapling_tree_size
                 + cb.vtx.iter().map(|tx| tx.outputs.len() as u32).sum::<u32>(),
             ..Default::default()

zcash_client_sqlite/src/wallet.rs

@@ -65,9 +65,9 @@
 //! - `memo` the shielded memo associated with the output, if any.
 
 use rusqlite::{self, named_params, OptionalExtension, ToSql};
-use std::collections::HashMap;
 use std::convert::TryFrom;
 use std::io::Cursor;
+use std::{collections::HashMap, io};
 
 use zcash_primitives::{
     block::BlockHash,
@@ -83,7 +83,7 @@ use zcash_primitives::{
 
 use zcash_client_backend::{
     address::{RecipientAddress, UnifiedAddress},
-    data_api::{chain::CommitmentTreeMeta, PoolType, Recipient, SentTransactionOutput},
+    data_api::{BlockMetadata, PoolType, Recipient, SentTransactionOutput},
     encoding::AddressCodec,
     keys::UnifiedFullViewingKey,
     wallet::WalletTx,
@@ -541,24 +541,58 @@ pub(crate) fn block_height_extrema(
     })
 }
 
-pub(crate) fn fully_scanned_height(
+fn parse_block_metadata(
+    row: (BlockHeight, Vec<u8>, Option<u32>, Vec<u8>),
+) -> Option<Result<BlockMetadata, SqliteClientError>> {
+    let (block_height, hash_data, sapling_tree_size_opt, sapling_tree) = row;
+    let sapling_tree_size = sapling_tree_size_opt.map(Ok).or_else(|| {
+        if sapling_tree == BLOCK_SAPLING_FRONTIER_ABSENT {
+            None
+        } else {
+            // parse the legacy commitment tree data
+            read_commitment_tree::<
+                zcash_primitives::sapling::Node,
+                _,
+                { zcash_primitives::sapling::NOTE_COMMITMENT_TREE_DEPTH },
+            >(Cursor::new(sapling_tree))
+            .map(|tree| Some(tree.size().try_into().unwrap()))
+            .map_err(SqliteClientError::from)
+            .transpose()
+        }
+    })?;
+
+    let block_hash = BlockHash::try_from_slice(&hash_data).ok_or_else(|| {
+        SqliteClientError::from(io::Error::new(
+            io::ErrorKind::InvalidData,
+            format!("Invalid block hash length: {}", hash_data.len()),
+        ))
+    });
+
+    Some(sapling_tree_size.and_then(|sapling_tree_size| {
+        block_hash.map(|block_hash| {
+            BlockMetadata::from_parts(block_height, block_hash, sapling_tree_size)
+        })
+    }))
+}
+
+pub(crate) fn block_metadata(
     conn: &rusqlite::Connection,
-) -> Result<Option<(BlockHeight, CommitmentTreeMeta)>, SqliteClientError> {
+    block_height: BlockHeight,
-    // FIXME: this will need to be rewritten once out-of-order scan range suggestion
+) -> Result<Option<BlockMetadata>, SqliteClientError> {
-    // is implemented.
     let res_opt = conn
         .query_row(
-            "SELECT height, sapling_commitment_tree_size, sapling_tree
+            "SELECT height, hash, sapling_commitment_tree_size, sapling_tree
             FROM blocks
-            ORDER BY height DESC
+            WHERE height = :block_height",
-            LIMIT 1",
+            named_params![":block_height": u32::from(block_height)],
-            [],
             |row| {
-                let max_height: u32 = row.get(0)?;
+                let height: u32 = row.get(0)?;
-                let sapling_tree_size: Option<u32> = row.get(1)?;
+                let block_hash: Vec<u8> = row.get(1)?;
-                let sapling_tree: Vec<u8> = row.get(2)?;
+                let sapling_tree_size: Option<u32> = row.get(2)?;
+                let sapling_tree: Vec<u8> = row.get(3)?;
                 Ok((
-                    BlockHeight::from(max_height),
+                    BlockHeight::from(height),
+                    block_hash,
                     sapling_tree_size,
                     sapling_tree,
                 ))
@@ -566,32 +600,37 @@ pub(crate) fn fully_scanned_height(
         )
         .optional()?;
 
-    res_opt
+    res_opt.and_then(parse_block_metadata).transpose()
-        .and_then(|(max_height, sapling_tree_size, sapling_tree)| {
+}
-            sapling_tree_size
+
-                .map(|s| Ok(CommitmentTreeMeta::from_parts(s)))
+pub(crate) fn block_fully_scanned(
-                .or_else(|| {
+    conn: &rusqlite::Connection,
-                    if &sapling_tree == BLOCK_SAPLING_FRONTIER_ABSENT {
+) -> Result<Option<BlockMetadata>, SqliteClientError> {
-                        None
+    // FIXME: this will need to be rewritten once out-of-order scan range suggestion
-                    } else {
+    // is implemented.
-                        // parse the legacy commitment tree data
+    let res_opt = conn
-                        read_commitment_tree::<
+        .query_row(
-                            zcash_primitives::sapling::Node,
+            "SELECT height, hash, sapling_commitment_tree_size, sapling_tree
-                            _,
+            FROM blocks
-                            { zcash_primitives::sapling::NOTE_COMMITMENT_TREE_DEPTH },
+            ORDER BY height DESC
-                        >(Cursor::new(sapling_tree))
+            LIMIT 1",
-                        .map(|tree| {
+            [],
-                            Some(CommitmentTreeMeta::from_parts(
+            |row| {
-                                tree.size().try_into().unwrap(),
+                let height: u32 = row.get(0)?;
-                            ))
+                let block_hash: Vec<u8> = row.get(1)?;
-                        })
+                let sapling_tree_size: Option<u32> = row.get(2)?;
-                        .map_err(SqliteClientError::from)
+                let sapling_tree: Vec<u8> = row.get(3)?;
-                        .transpose()
+                Ok((
-                    }
+                    BlockHeight::from(height),
-                })
+                    block_hash,
-                .map(|meta_res| meta_res.map(|meta| (max_height, meta)))
+                    sapling_tree_size,
-        })
+                    sapling_tree,
-        .transpose()
+                ))
+            },
+        )
+        .optional()?;
+
+    res_opt.and_then(parse_block_metadata).transpose()
 }
 
 /// Returns the block height at which the specified transaction was mined,
@@ -834,12 +873,34 @@ pub(crate) fn get_transparent_balances<P: consensus::Parameters>(
 
 /// Inserts information about a scanned block into the database.
 pub(crate) fn put_block(
-    conn: &rusqlite::Connection,
+    conn: &rusqlite::Transaction<'_>,
     block_height: BlockHeight,
     block_hash: BlockHash,
     block_time: u32,
-    sapling_commitment_tree_size: Option<u64>,
+    sapling_commitment_tree_size: u32,
 ) -> Result<(), SqliteClientError> {
+    let block_hash_data = conn
+        .query_row(
+            "SELECT hash FROM blocks WHERE height = ?",
+            [u32::from(block_height)],
+            |row| row.get::<_, Vec<u8>>(0),
+        )
+        .optional()?;
+
+    // Ensure that in the case of an upsert, we don't overwrite block data
+    // with information for a block with a different hash.
+    if let Some(bytes) = block_hash_data {
+        let expected_hash = BlockHash::try_from_slice(&bytes).ok_or_else(|| {
+            SqliteClientError::CorruptedData(format!(
+                "Invalid block hash at height {}",
+                u32::from(block_height)
+            ))
+        })?;
+        if expected_hash != block_hash {
+            return Err(SqliteClientError::BlockConflict(block_height));
+        }
+    }
+
     let mut stmt_upsert_block = conn.prepare_cached(
         "INSERT INTO blocks (
             height,

zcash_client_sqlite/src/wallet/init.rs

@@ -1,24 +1,29 @@
 //! Functions for initializing the various databases.
 use either::Either;
+use incrementalmerkletree::Retention;
 use std::{collections::HashMap, fmt, io};
 
 use rusqlite::{self, types::ToSql};
 use schemer::{Migrator, MigratorError};
 use schemer_rusqlite::RusqliteAdapter;
 use secrecy::SecretVec;
-use shardtree::ShardTreeError;
+use shardtree::{ShardTree, ShardTreeError};
 use uuid::Uuid;
 
 use zcash_primitives::{
     block::BlockHash,
     consensus::{self, BlockHeight},
+    merkle_tree::read_commitment_tree,
+    sapling,
     transaction::components::amount::BalanceError,
     zip32::AccountId,
 };
 
-use zcash_client_backend::keys::UnifiedFullViewingKey;
+use zcash_client_backend::{data_api::SAPLING_SHARD_HEIGHT, keys::UnifiedFullViewingKey};
 
-use crate::{error::SqliteClientError, wallet, WalletDb};
+use crate::{error::SqliteClientError, wallet, WalletDb, SAPLING_TABLES_PREFIX};
+
+use super::commitment_tree::SqliteShardStore;
 
 mod migrations;
 
@@ -289,9 +294,21 @@ pub fn init_blocks_table<P: consensus::Parameters>(
             return Err(SqliteClientError::TableNotEmpty);
         }
 
+        let block_end_tree =
+            read_commitment_tree::<sapling::Node, _, { sapling::NOTE_COMMITMENT_TREE_DEPTH }>(
+                sapling_tree,
+            )
+            .map_err(|e| {
+                rusqlite::Error::FromSqlConversionFailure(
+                    sapling_tree.len(),
+                    rusqlite::types::Type::Blob,
+                    Box::new(e),
+                )
+            })?;
+
         wdb.conn.0.execute(
             "INSERT INTO blocks (height, hash, time, sapling_tree)
-        VALUES (?, ?, ?, ?)",
+            VALUES (?, ?, ?, ?)",
             [
                 u32::from(height).to_sql()?,
                 hash.0.to_sql()?,
@@ -300,6 +317,26 @@ pub fn init_blocks_table<P: consensus::Parameters>(
             ],
         )?;
 
+        if let Some(nonempty_frontier) = block_end_tree.to_frontier().value() {
+            let shard_store =
+                SqliteShardStore::<_, sapling::Node, SAPLING_SHARD_HEIGHT>::from_connection(
+                    wdb.conn.0,
+                    SAPLING_TABLES_PREFIX,
+                )?;
+            let mut shard_tree: ShardTree<
+                _,
+                { sapling::NOTE_COMMITMENT_TREE_DEPTH },
+                SAPLING_SHARD_HEIGHT,
+            > = ShardTree::new(shard_store, 100);
+            shard_tree.insert_frontier_nodes(
+                nonempty_frontier.clone(),
+                Retention::Checkpoint {
+                    id: height,
+                    is_marked: false,
+                },
+            )?;
+        }
+
         Ok(())
     })
 }
@@ -1154,7 +1191,7 @@ mod tests {
             BlockHeight::from(1u32),
             BlockHash([1; 32]),
             1,
-            &[],
+            &[0x0, 0x0, 0x0],
         )
         .unwrap();
 
@@ -1164,7 +1201,7 @@ mod tests {
             BlockHeight::from(2u32),
             BlockHash([2; 32]),
             2,
-            &[],
+            &[0x0, 0x0, 0x0],
         )
         .unwrap_err();
     }

zcash_client_sqlite/src/wallet/sapling.rs

@@ -508,7 +508,7 @@ pub(crate) mod tests {
             BlockHeight::from(1u32),
             BlockHash([1; 32]),
             1,
-            &[],
+            &[0x0, 0x0, 0x0],
         )
         .unwrap();
 
@@ -562,7 +562,7 @@ pub(crate) mod tests {
 
         // Add funds to the wallet in a single note
         let value = Amount::from_u64(50000).unwrap();
-        let (cb, _) = fake_compact_block(
+        let (mut cb, _) = fake_compact_block(
             sapling_activation_height(),
             BlockHash([0; 32]),
             &dfvk,
@@ -588,14 +588,15 @@ pub(crate) mod tests {
         );
 
         // Add more funds to the wallet in a second note
-        let (cb, _) = fake_compact_block(
+        cb = fake_compact_block(
             sapling_activation_height() + 1,
             cb.hash(),
             &dfvk,
             AddressType::DefaultExternal,
             value,
             1,
-        );
+        )
+        .0;
         insert_into_cache(&db_cache, &cb);
         scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
 
@@ -639,14 +640,15 @@ pub(crate) mod tests {
         // Mine blocks SAPLING_ACTIVATION_HEIGHT + 2 to 9 until just before the second
         // note is verified
         for i in 2..10 {
-            let (cb, _) = fake_compact_block(
+            cb = fake_compact_block(
                 sapling_activation_height() + i,
                 cb.hash(),
                 &dfvk,
                 AddressType::DefaultExternal,
                 value,
                 i,
-            );
+            )
+            .0;
             insert_into_cache(&db_cache, &cb);
         }
         scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
@@ -673,14 +675,15 @@ pub(crate) mod tests {
         );
 
         // Mine block 11 so that the second note becomes verified
-        let (cb, _) = fake_compact_block(
+        cb = fake_compact_block(
             sapling_activation_height() + 10,
             cb.hash(),
             &dfvk,
             AddressType::DefaultExternal,
             value,
             11,
-        );
+        )
+        .0;
         insert_into_cache(&db_cache, &cb);
         scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
 
@@ -718,7 +721,7 @@ pub(crate) mod tests {
 
         // Add funds to the wallet in a single note
         let value = Amount::from_u64(50000).unwrap();
-        let (cb, _) = fake_compact_block(
+        let (mut cb, _) = fake_compact_block(
             sapling_activation_height(),
             BlockHash([0; 32]),
             &dfvk,
@@ -774,14 +777,15 @@ pub(crate) mod tests {
         // Mine blocks SAPLING_ACTIVATION_HEIGHT + 1 to 41 (that don't send us funds)
         // until just before the first transaction expires
         for i in 1..42 {
-            let (cb, _) = fake_compact_block(
+            cb = fake_compact_block(
                 sapling_activation_height() + i,
                 cb.hash(),
                 &ExtendedSpendingKey::master(&[i as u8]).to_diversifiable_full_viewing_key(),
                 AddressType::DefaultExternal,
                 value,
                 i,
-            );
+            )
+            .0;
             insert_into_cache(&db_cache, &cb);
         }
         scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
@@ -807,14 +811,15 @@ pub(crate) mod tests {
         );
 
         // Mine block SAPLING_ACTIVATION_HEIGHT + 42 so that the first transaction expires
-        let (cb, _) = fake_compact_block(
+        cb = fake_compact_block(
             sapling_activation_height() + 42,
             cb.hash(),
             &ExtendedSpendingKey::master(&[42]).to_diversifiable_full_viewing_key(),
             AddressType::DefaultExternal,
             value,
             42,
-        );
+        )
+        .0;
         insert_into_cache(&db_cache, &cb);
         scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap();
 
@@ -851,7 +856,7 @@ pub(crate) mod tests {
 
         // Add funds to the wallet in a single note
         let value = Amount::from_u64(50000).unwrap();
-        let (cb, _) = fake_compact_block(
+        let (mut cb, _) = fake_compact_block(
             sapling_activation_height(),
             BlockHash([0; 32]),
             &dfvk,
@@ -922,14 +927,15 @@ pub(crate) mod tests {
         // Mine blocks SAPLING_ACTIVATION_HEIGHT + 1 to 42 (that don't send us funds)
         // so that the first transaction expires
         for i in 1..=42 {
-            let (cb, _) = fake_compact_block(
+            cb = fake_compact_block(
                 sapling_activation_height() + i,
                 cb.hash(),
                 &ExtendedSpendingKey::master(&[i as u8]).to_diversifiable_full_viewing_key(),
                 AddressType::DefaultExternal,
                 value,
                 i,
-            );
+            )
+            .0;
             insert_into_cache(&db_cache, &cb);
         }
         scan_cached_blocks(&network, &db_cache, &mut db_data, None, None).unwrap();
@@ -1073,7 +1079,7 @@ pub(crate) mod tests {
         let dfvk = usk.sapling().to_diversifiable_full_viewing_key();
 
         // Add funds to the wallet
-        let (cb, _) = fake_compact_block(
+        let (mut cb, _) = fake_compact_block(
             sapling_activation_height(),
             BlockHash([0; 32]),
             &dfvk,
@@ -1085,14 +1091,15 @@ pub(crate) mod tests {
 
         // Add 10 dust notes to the wallet
         for i in 1..=10 {
-            let (cb, _) = fake_compact_block(
+            cb = fake_compact_block(
                 sapling_activation_height() + i,
                 cb.hash(),
                 &dfvk,
                 AddressType::DefaultExternal,
                 Amount::from_u64(1000).unwrap(),
                 i,
-            );
+            )
+            .0;
             insert_into_cache(&db_cache, &cb);
         }
 

zcash_primitives/src/block.rs

@@ -39,10 +39,20 @@ impl BlockHash {
     ///
     /// This function will panic if the slice is not exactly 32 bytes.
     pub fn from_slice(bytes: &[u8]) -> Self {
-        assert_eq!(bytes.len(), 32);
+        Self::try_from_slice(bytes).unwrap()
-        let mut hash = [0; 32];
+    }
-        hash.copy_from_slice(bytes);
+
-        BlockHash(hash)
+    /// Constructs a [`BlockHash`] from the given slice.
+    ///
+    /// Returns `None` if `bytes` has any length other than 32
+    pub fn try_from_slice(bytes: &[u8]) -> Option<Self> {
+        if bytes.len() == 32 {
+            let mut hash = [0; 32];
+            hash.copy_from_slice(bytes);
+            Some(BlockHash(hash))
+        } else {
+            None
+        }
     }
 }