From 5c7d941160e340a0bfa4831b59f3be5de908e467 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Mon, 14 Jun 2021 20:18:37 -0300 Subject: [PATCH 01/30] sketch of implementation --- zebra-state/src/error.rs | 6 +++ zebra-state/src/service.rs | 4 +- zebra-state/src/service/check.rs | 37 ++++++++++++++++++- .../src/service/non_finalized_state.rs | 20 ++++++++-- .../src/service/non_finalized_state/chain.rs | 20 +++++++++- 5 files changed, 79 insertions(+), 8 deletions(-) diff --git a/zebra-state/src/error.rs b/zebra-state/src/error.rs index e6838dacdf2..17fb451b84f 100644 --- a/zebra-state/src/error.rs +++ b/zebra-state/src/error.rs @@ -74,4 +74,10 @@ pub enum ValidateContextError { difficulty_threshold: CompactDifficulty, expected_difficulty: CompactDifficulty, }, + + #[error("block has the wrong history commitment")] + #[non_exhaustive] + InvalidHistoryCommitment { + // TODO: add commitments? + }, } diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index 387c8723b79..a24f9a8ae70 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -149,9 +149,9 @@ impl StateService { let parent_hash = prepared.block.header.previous_block_hash; if self.disk.finalized_tip_hash() == parent_hash { - self.mem.commit_new_chain(prepared); + self.mem.commit_new_chain(prepared)?; } else { - self.mem.commit_block(prepared); + self.mem.commit_block(prepared)?; } Ok(()) diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs index bdb8bc28cc8..dab00af98bd 100644 --- a/zebra-state/src/service/check.rs +++ b/zebra-state/src/service/check.rs @@ -4,7 +4,7 @@ use std::borrow::Borrow; use chrono::Duration; use zebra_chain::{ - block::{self, Block}, + block::{self, Block, ChainHistoryMmrRootHash}, parameters::POW_AVERAGING_WINDOW, parameters::{Network, NetworkUpgrade}, work::difficulty::CompactDifficulty, @@ -86,6 +86,41 @@ where Ok(()) } +#[tracing::instrument( + name = "contextual_validation_for_chain", + fields(?network), + skip(prepared, network) +)] +pub(crate) fn block_is_contextually_valid_for_chain( + prepared: &PreparedBlock, + network: Network, + mmr_hash: &ChainHistoryMmrRootHash, +) -> Result<(), ValidateContextError> { + match prepared + .block + .commitment(network) + .expect("should have commitment (TODO: confirm)") + { + block::Commitment::PreSaplingReserved(_) => todo!("Confirm where this is being checked"), + block::Commitment::FinalSaplingRoot(_) => todo!("Confirm where this is being checked"), + block::Commitment::ChainHistoryActivationReserved => { + todo!("Confirm where this is being checked") + } + block::Commitment::ChainHistoryRoot(block_mmr_hash) => { + if block_mmr_hash == *mmr_hash { + Ok(()) + } else { + Err(ValidateContextError::InvalidHistoryCommitment {}) + } + } + block::Commitment::ChainHistoryBlockTxAuthCommitment(_) => { + // TODO: Get auth_hash from block (ZIP-244), e.g. + // let auth_hash = prepared.block.auth_hash(); + todo!("hash mmr_hash and auth_hash per ZIP-244 and compare") + } + } +} + /// Returns `ValidateContextError::OrphanedBlock` if the height of the given /// block is less than or equal to the finalized tip height. fn block_is_not_orphaned( diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 0de26391799..8b54fe4e1f1 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -21,10 +21,12 @@ use zebra_chain::{ transparent, }; -use crate::{FinalizedBlock, HashOrHeight, PreparedBlock, Utxo}; +use crate::{FinalizedBlock, HashOrHeight, PreparedBlock, Utxo, ValidateContextError}; use self::chain::Chain; +use super::check; + /// The state of the chains in memory, incuding queued blocks. #[derive(Default)] pub struct NonFinalizedState { @@ -76,7 +78,7 @@ impl NonFinalizedState { } /// Commit block to the non-finalized state. - pub fn commit_block(&mut self, prepared: PreparedBlock) { + pub fn commit_block(&mut self, prepared: PreparedBlock) -> Result<(), ValidateContextError> { let parent_hash = prepared.block.header.previous_block_hash; let (height, hash) = (prepared.height, prepared.hash); @@ -97,19 +99,31 @@ impl NonFinalizedState { }) .expect("commit_block is only called with blocks that are ready to be commited"); + check::block_is_contextually_valid_for_chain( + &prepared, + self.network, + &parent_chain.mmr_hash(), + )?; parent_chain.push(prepared); self.chain_set.insert(parent_chain); self.update_metrics_for_committed_block(height, hash); + Ok(()) } /// Commit block to the non-finalized state as a new chain where its parent /// is the finalized tip. - pub fn commit_new_chain(&mut self, prepared: PreparedBlock) { + pub fn commit_new_chain( + &mut self, + prepared: PreparedBlock, + ) -> Result<(), ValidateContextError> { + // TODO: will need to pass the MMR tree of the finalized tip to the newly built chain. How? let mut chain = Chain::default(); let (height, hash) = (prepared.height, prepared.hash); + check::block_is_contextually_valid_for_chain(&prepared, self.network, &chain.mmr_hash())?; chain.push(prepared); self.chain_set.insert(Box::new(chain)); self.update_metrics_for_committed_block(height, hash); + Ok(()) } /// Returns the length of the non-finalized portion of the current best chain. diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index abe1f63745b..6435ba4270b 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -6,8 +6,13 @@ use std::{ use tracing::{debug_span, instrument, trace}; use zebra_chain::{ - block, orchard, primitives::Groth16Proof, sapling, sprout, transaction, - transaction::Transaction::*, transparent, work::difficulty::PartialCumulativeWork, + block::{self, ChainHistoryMmrRootHash}, + orchard, + primitives::Groth16Proof, + sapling, sprout, transaction, + transaction::Transaction::*, + transparent, + work::difficulty::PartialCumulativeWork, }; use crate::{PreparedBlock, Utxo}; @@ -27,6 +32,7 @@ pub struct Chain { sapling_nullifiers: HashSet, orchard_nullifiers: HashSet, partial_cumulative_work: PartialCumulativeWork, + // TODO: MMR tree } impl Chain { @@ -117,6 +123,10 @@ impl Chain { pub fn is_empty(&self) -> bool { self.blocks.is_empty() } + + pub fn mmr_hash(&self) -> ChainHistoryMmrRootHash { + todo!("get MMR hash from the MMR tree"); + } } /// Helper trait to organize inverse operations done on the `Chain` type. Used to @@ -160,6 +170,8 @@ impl UpdateWith for Chain { .expect("work has already been validated"); self.partial_cumulative_work += block_work; + // TODO: add block data to MMR tree + // for each transaction in block for (transaction_index, (transaction, transaction_hash)) in block .transactions @@ -241,6 +253,10 @@ impl UpdateWith for Chain { .expect("work has already been validated"); self.partial_cumulative_work -= block_work; + // TODO: remove block data from MMR tree + // In order to do that, we need to load some "extra" nodes (blocks) from the + // state (see zcash_history and librustzcash). How to do that here? + // for each transaction in block for (transaction, transaction_hash) in block.transactions.iter().zip(transaction_hashes.iter()) From 5226145119d75c28b339aa085985865833227cc8 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Wed, 16 Jun 2021 15:32:38 -0300 Subject: [PATCH 02/30] refined implementation; still incomplete --- zebra-chain/src/lib.rs | 1 + zebra-chain/src/mmr.rs | 43 ++++++++++++++ zebra-state/src/error.rs | 13 +++- zebra-state/src/service.rs | 9 ++- zebra-state/src/service/check.rs | 20 +++---- zebra-state/src/service/finalized_state.rs | 6 ++ .../src/service/non_finalized_state.rs | 26 ++++++-- .../src/service/non_finalized_state/chain.rs | 59 ++++++++++++++++--- 8 files changed, 148 insertions(+), 29 deletions(-) create mode 100644 zebra-chain/src/mmr.rs diff --git a/zebra-chain/src/lib.rs b/zebra-chain/src/lib.rs index 8e53e159e63..94f677d86bc 100644 --- a/zebra-chain/src/lib.rs +++ b/zebra-chain/src/lib.rs @@ -21,6 +21,7 @@ extern crate bitflags; pub mod amount; pub mod block; pub mod fmt; +pub mod mmr; pub mod orchard; pub mod parameters; pub mod primitives; diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/mmr.rs new file mode 100644 index 00000000000..1609ca928f9 --- /dev/null +++ b/zebra-chain/src/mmr.rs @@ -0,0 +1,43 @@ +//! Merkle mountain range structure that contains information about +//! the block history as specified in ZIP-221. + +use std::borrow::Borrow; + +use crate::block::{Block, ChainHistoryMmrRootHash}; + +/// Merkle mountain range structure. +pub struct MerkleMountainRange { + // TODO +} + +impl MerkleMountainRange { + pub fn push(&mut self, block: &Block) { + // TODO: zcash_history::Tree::append_leaf receives an Arc. Should + // change that to match, or create an Arc to pass to it? + todo!(); + } + + /// Extend the MMR with the given blocks. + // TODO: add Sapling and Orchard roots to parameters + pub fn extend(&mut self, _vals: &C) + where + C: IntoIterator, + C::Item: Borrow, + C::IntoIter: ExactSizeIterator, + { + // TODO: How to convert C to `impl Iterator, sapling::tree::Root)>` + // for zcash_history::Tree::append_leaf_iter? Should change that to match C? + todo!(); + } + + /// Return the hash of the MMR root. + pub fn hash(&self) -> ChainHistoryMmrRootHash { + todo!(); + } +} + +impl Clone for MerkleMountainRange { + fn clone(&self) -> Self { + todo!(); + } +} diff --git a/zebra-state/src/error.rs b/zebra-state/src/error.rs index 17fb451b84f..3e4e2150fe0 100644 --- a/zebra-state/src/error.rs +++ b/zebra-state/src/error.rs @@ -3,7 +3,10 @@ use std::sync::Arc; use chrono::{DateTime, Utc}; use thiserror::Error; -use zebra_chain::{block, work::difficulty::CompactDifficulty}; +use zebra_chain::{ + block::{self, ChainHistoryMmrRootHash, CommitmentError}, + work::difficulty::CompactDifficulty, +}; /// A wrapper for type erased errors that is itself clonable and implements the /// Error trait @@ -75,9 +78,13 @@ pub enum ValidateContextError { expected_difficulty: CompactDifficulty, }, - #[error("block has the wrong history commitment")] + #[error("block contains an invalid commitment")] + InvalidCommitment(#[from] CommitmentError), + + #[error("block history commitment {candidate_commitment:?} is different to the expected commitment {expected_commitment:?}")] #[non_exhaustive] InvalidHistoryCommitment { - // TODO: add commitments? + candidate_commitment: ChainHistoryMmrRootHash, + expected_commitment: ChainHistoryMmrRootHash, }, } diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index a24f9a8ae70..d55f6d5071b 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -149,9 +149,14 @@ impl StateService { let parent_hash = prepared.block.header.previous_block_hash; if self.disk.finalized_tip_hash() == parent_hash { - self.mem.commit_new_chain(prepared)?; + self.mem + .commit_new_chain(prepared, self.disk.mmr().clone())?; } else { - self.mem.commit_block(prepared)?; + let block_iter = self.any_ancestor_blocks(parent_hash); + // TODO: the above creates a immutable borrow of self, and below it uses + // a mutable borrow. which causes a compiler error. How to solve this? + self.mem + .commit_block(prepared, self.disk.mmr(), &block_iter)?; } Ok(()) diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs index dab00af98bd..89b5e554c60 100644 --- a/zebra-state/src/service/check.rs +++ b/zebra-state/src/service/check.rs @@ -96,21 +96,21 @@ pub(crate) fn block_is_contextually_valid_for_chain( network: Network, mmr_hash: &ChainHistoryMmrRootHash, ) -> Result<(), ValidateContextError> { - match prepared - .block - .commitment(network) - .expect("should have commitment (TODO: confirm)") - { - block::Commitment::PreSaplingReserved(_) => todo!("Confirm where this is being checked"), - block::Commitment::FinalSaplingRoot(_) => todo!("Confirm where this is being checked"), - block::Commitment::ChainHistoryActivationReserved => { - todo!("Confirm where this is being checked") + match prepared.block.commitment(network)? { + block::Commitment::PreSaplingReserved(_) + | block::Commitment::FinalSaplingRoot(_) + | block::Commitment::ChainHistoryActivationReserved => { + // No contextual checks needed for those. + Ok(()) } block::Commitment::ChainHistoryRoot(block_mmr_hash) => { if block_mmr_hash == *mmr_hash { Ok(()) } else { - Err(ValidateContextError::InvalidHistoryCommitment {}) + Err(ValidateContextError::InvalidHistoryCommitment { + candidate_commitment: block_mmr_hash, + expected_commitment: *mmr_hash, + }) } } block::Commitment::ChainHistoryBlockTxAuthCommitment(_) => { diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 57834622b59..c56878cd5e7 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -7,6 +7,7 @@ mod tests; use std::{collections::HashMap, convert::TryInto, sync::Arc}; +use zebra_chain::mmr::MerkleMountainRange; use zebra_chain::transparent; use zebra_chain::{ block::{self, Block}, @@ -378,6 +379,11 @@ impl FinalizedState { }) } + /// Returns the MMR for the finalized state. + pub fn mmr(&self) -> &MerkleMountainRange { + todo!("add MMR to finalized state"); + } + /// If the database is `ephemeral`, delete it. fn delete_ephemeral(&self) { if self.ephemeral { diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 8b54fe4e1f1..248aee78980 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -12,10 +12,11 @@ mod tests; pub use queued_blocks::QueuedBlocks; -use std::{collections::BTreeSet, mem, ops::Deref, sync::Arc}; +use std::{borrow::Borrow, collections::BTreeSet, mem, ops::Deref, sync::Arc}; use zebra_chain::{ block::{self, Block}, + mmr::MerkleMountainRange, parameters::{Network, NetworkUpgrade::Canopy}, transaction::{self, Transaction}, transparent, @@ -78,7 +79,17 @@ impl NonFinalizedState { } /// Commit block to the non-finalized state. - pub fn commit_block(&mut self, prepared: PreparedBlock) -> Result<(), ValidateContextError> { + pub fn commit_block( + &mut self, + prepared: PreparedBlock, + finalized_tip_mmr: &MerkleMountainRange, + block_iter: &C, + ) -> Result<(), ValidateContextError> + where + C: IntoIterator, + C::Item: Borrow, + C::IntoIter: ExactSizeIterator, + { let parent_hash = prepared.block.header.previous_block_hash; let (height, hash) = (prepared.height, prepared.hash); @@ -94,7 +105,12 @@ impl NonFinalizedState { .or_else(|| { self.chain_set .iter() - .find_map(|chain| chain.fork(parent_hash)) + .find_map(|chain| { + let mut mmr = finalized_tip_mmr.clone(); + // TODO: pass Sapling and Orchard roots for each block. How? + mmr.extend(block_iter); + chain.fork(parent_hash, mmr) + }) .map(Box::new) }) .expect("commit_block is only called with blocks that are ready to be commited"); @@ -115,9 +131,9 @@ impl NonFinalizedState { pub fn commit_new_chain( &mut self, prepared: PreparedBlock, + mmr: MerkleMountainRange, ) -> Result<(), ValidateContextError> { - // TODO: will need to pass the MMR tree of the finalized tip to the newly built chain. How? - let mut chain = Chain::default(); + let mut chain = Chain::new(mmr); let (height, hash) = (prepared.height, prepared.hash); check::block_is_contextually_valid_for_chain(&prepared, self.network, &chain.mmr_hash())?; chain.push(prepared); diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 6435ba4270b..c9f3719c8d5 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -7,6 +7,7 @@ use std::{ use tracing::{debug_span, instrument, trace}; use zebra_chain::{ block::{self, ChainHistoryMmrRootHash}, + mmr::MerkleMountainRange, orchard, primitives::Groth16Proof, sapling, sprout, transaction, @@ -17,7 +18,7 @@ use zebra_chain::{ use crate::{PreparedBlock, Utxo}; -#[derive(Default, Clone)] +// #[derive(Clone)] pub struct Chain { pub blocks: BTreeMap, pub height_by_hash: HashMap, @@ -32,10 +33,30 @@ pub struct Chain { sapling_nullifiers: HashSet, orchard_nullifiers: HashSet, partial_cumulative_work: PartialCumulativeWork, - // TODO: MMR tree + mmr: MerkleMountainRange, } impl Chain { + /// Create a new empty non-finalized chain with the given MMR. + /// + /// The MMR must contain the history of the previous (finalized) blocks. + pub fn new(mmr: MerkleMountainRange) -> Self { + Chain { + blocks: Default::default(), + height_by_hash: Default::default(), + tx_by_hash: Default::default(), + created_utxos: Default::default(), + spent_utxos: Default::default(), + sprout_anchors: Default::default(), + sapling_anchors: Default::default(), + sprout_nullifiers: Default::default(), + sapling_nullifiers: Default::default(), + orchard_nullifiers: Default::default(), + partial_cumulative_work: Default::default(), + mmr, + } + } + /// Push a contextually valid non-finalized block into a chain as the new tip. #[instrument(level = "debug", skip(self, block), fields(block = %block.block))] pub fn push(&mut self, block: PreparedBlock) { @@ -73,12 +94,14 @@ impl Chain { /// Fork a chain at the block with the given hash, if it is part of this /// chain. - pub fn fork(&self, fork_tip: block::Hash) -> Option { + /// + /// `mmr`: the MMR for the fork. + pub fn fork(&self, fork_tip: block::Hash, mmr: MerkleMountainRange) -> Option { if !self.height_by_hash.contains_key(&fork_tip) { return None; } - let mut forked = self.clone(); + let mut forked = self.clone_with_mmr(mmr); while forked.non_finalized_tip_hash() != fork_tip { forked.pop_tip(); @@ -125,7 +148,27 @@ impl Chain { } pub fn mmr_hash(&self) -> ChainHistoryMmrRootHash { - todo!("get MMR hash from the MMR tree"); + self.mmr.hash() + } + + /// Clone the Chain but not the MMR. + /// + /// Useful when forking, where the MMR is rebuilt anyway. + fn clone_with_mmr(&self, mmr: MerkleMountainRange) -> Self { + Chain { + blocks: self.blocks.clone(), + height_by_hash: self.height_by_hash.clone(), + tx_by_hash: self.tx_by_hash.clone(), + created_utxos: self.created_utxos.clone(), + spent_utxos: self.spent_utxos.clone(), + sprout_anchors: self.sprout_anchors.clone(), + sapling_anchors: self.sapling_anchors.clone(), + sprout_nullifiers: self.sprout_nullifiers.clone(), + sapling_nullifiers: self.sapling_nullifiers.clone(), + orchard_nullifiers: self.orchard_nullifiers.clone(), + partial_cumulative_work: self.partial_cumulative_work, + mmr, + } } } @@ -170,7 +213,7 @@ impl UpdateWith for Chain { .expect("work has already been validated"); self.partial_cumulative_work += block_work; - // TODO: add block data to MMR tree + self.mmr.push(block); // for each transaction in block for (transaction_index, (transaction, transaction_hash)) in block @@ -253,9 +296,7 @@ impl UpdateWith for Chain { .expect("work has already been validated"); self.partial_cumulative_work -= block_work; - // TODO: remove block data from MMR tree - // In order to do that, we need to load some "extra" nodes (blocks) from the - // state (see zcash_history and librustzcash). How to do that here? + // Note: the MMR is not reverted here; it's rebuilt and passed in fork() // for each transaction in block for (transaction, transaction_hash) in From 3abbfa569cfaa6959395f61e7912909382c38a93 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Thu, 17 Jun 2021 10:22:03 -0300 Subject: [PATCH 03/30] update librustzcash, change zcash_history to work with it --- Cargo.lock | 2 +- zebra-chain/src/primitives/zcash_history.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3ebeb826cee..c7d643155c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4414,7 +4414,7 @@ dependencies = [ [[package]] name = "zcash_history" version = "0.2.0" -source = "git+https://github.com/zcash/librustzcash.git#d50bb12a97da768dc8f3ee39b81f84262103e6eb" +source = "git+https://github.com/zcash/librustzcash.git#0c3ed159985affa774e44d10172d4471d798a85a" dependencies = [ "bigint", "blake2b_simd", diff --git a/zebra-chain/src/primitives/zcash_history.rs b/zebra-chain/src/primitives/zcash_history.rs index 5ccfdad0afe..1c96ab4772a 100644 --- a/zebra-chain/src/primitives/zcash_history.rs +++ b/zebra-chain/src/primitives/zcash_history.rs @@ -21,7 +21,7 @@ use crate::{ pub struct Tree { network: Network, network_upgrade: NetworkUpgrade, - inner: zcash_history::Tree, + inner: zcash_history::Tree, } /// An encoded tree node data. @@ -48,9 +48,9 @@ pub struct Entry { inner: [u8; zcash_history::MAX_ENTRY_SIZE], } -impl From for Entry { +impl From> for Entry { /// Convert from librustzcash. - fn from(inner_entry: zcash_history::Entry) -> Self { + fn from(inner_entry: zcash_history::Entry) -> Self { let mut entry = Entry { inner: [0; zcash_history::MAX_ENTRY_SIZE], }; @@ -66,7 +66,7 @@ impl Entry { /// Sapling note commitment tree. fn new_leaf(block: Arc, network: Network, sapling_root: &sapling::tree::Root) -> Self { let node_data = block_to_history_node(block, network, sapling_root); - let inner_entry: zcash_history::Entry = node_data.into(); + let inner_entry = zcash_history::Entry::::new_leaf(node_data); inner_entry.into() } From e929f3f9c68f496bec6e67bb474c5baa0234e0fb Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Thu, 17 Jun 2021 14:25:30 -0300 Subject: [PATCH 04/30] simplified code per review; renamed MMR to HistoryTree --- zebra-chain/src/mmr.rs | 38 ++++++-------- zebra-state/src/service.rs | 8 +-- zebra-state/src/service/finalized_state.rs | 8 +-- .../src/service/non_finalized_state.rs | 35 +++++-------- .../src/service/non_finalized_state/chain.rs | 52 +++++++++++++------ 5 files changed, 72 insertions(+), 69 deletions(-) diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/mmr.rs index 1609ca928f9..d138f5e4d00 100644 --- a/zebra-chain/src/mmr.rs +++ b/zebra-chain/src/mmr.rs @@ -1,43 +1,39 @@ -//! Merkle mountain range structure that contains information about +//! History tree (Merkle mountain range) structure that contains information about //! the block history as specified in ZIP-221. -use std::borrow::Borrow; +use std::sync::Arc; use crate::block::{Block, ChainHistoryMmrRootHash}; -/// Merkle mountain range structure. -pub struct MerkleMountainRange { +/// History tree structure. +pub struct HistoryTree { // TODO } -impl MerkleMountainRange { - pub fn push(&mut self, block: &Block) { +impl HistoryTree { + /// Add block data to the tree. + pub fn push(&mut self, _block: &Block) { // TODO: zcash_history::Tree::append_leaf receives an Arc. Should // change that to match, or create an Arc to pass to it? todo!(); } - /// Extend the MMR with the given blocks. - // TODO: add Sapling and Orchard roots to parameters - pub fn extend(&mut self, _vals: &C) - where - C: IntoIterator, - C::Item: Borrow, - C::IntoIter: ExactSizeIterator, - { - // TODO: How to convert C to `impl Iterator, sapling::tree::Root)>` - // for zcash_history::Tree::append_leaf_iter? Should change that to match C? - todo!(); - } - - /// Return the hash of the MMR root. + /// Return the hash of the tree root. pub fn hash(&self) -> ChainHistoryMmrRootHash { todo!(); } } -impl Clone for MerkleMountainRange { +impl Clone for HistoryTree { fn clone(&self) -> Self { todo!(); } } + +impl Extend> for HistoryTree { + /// Extend the history tree with the given blocks. + // TODO: add Sapling and Orchard roots to parameters + fn extend>>(&mut self, _iter: T) { + todo!() + } +} diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index d55f6d5071b..cdf7ace8994 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -150,13 +150,9 @@ impl StateService { if self.disk.finalized_tip_hash() == parent_hash { self.mem - .commit_new_chain(prepared, self.disk.mmr().clone())?; + .commit_new_chain(prepared, self.disk.history_tree().clone())?; } else { - let block_iter = self.any_ancestor_blocks(parent_hash); - // TODO: the above creates a immutable borrow of self, and below it uses - // a mutable borrow. which causes a compiler error. How to solve this? - self.mem - .commit_block(prepared, self.disk.mmr(), &block_iter)?; + self.mem.commit_block(prepared, self.disk.history_tree())?; } Ok(()) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index c56878cd5e7..f4f8650916c 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -7,7 +7,7 @@ mod tests; use std::{collections::HashMap, convert::TryInto, sync::Arc}; -use zebra_chain::mmr::MerkleMountainRange; +use zebra_chain::mmr::HistoryTree; use zebra_chain::transparent; use zebra_chain::{ block::{self, Block}, @@ -379,9 +379,9 @@ impl FinalizedState { }) } - /// Returns the MMR for the finalized state. - pub fn mmr(&self) -> &MerkleMountainRange { - todo!("add MMR to finalized state"); + /// Returns the history tree for the finalized state. + pub fn history_tree(&self) -> &HistoryTree { + todo!("add history tree to finalized state"); } /// If the database is `ephemeral`, delete it. diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 248aee78980..66b9787620a 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -12,11 +12,11 @@ mod tests; pub use queued_blocks::QueuedBlocks; -use std::{borrow::Borrow, collections::BTreeSet, mem, ops::Deref, sync::Arc}; +use std::{collections::BTreeSet, mem, ops::Deref, sync::Arc}; use zebra_chain::{ block::{self, Block}, - mmr::MerkleMountainRange, + mmr::HistoryTree, parameters::{Network, NetworkUpgrade::Canopy}, transaction::{self, Transaction}, transparent, @@ -79,17 +79,11 @@ impl NonFinalizedState { } /// Commit block to the non-finalized state. - pub fn commit_block( + pub fn commit_block( &mut self, prepared: PreparedBlock, - finalized_tip_mmr: &MerkleMountainRange, - block_iter: &C, - ) -> Result<(), ValidateContextError> - where - C: IntoIterator, - C::Item: Borrow, - C::IntoIter: ExactSizeIterator, - { + finalized_tip_history_tree: &HistoryTree, + ) -> Result<(), ValidateContextError> { let parent_hash = prepared.block.header.previous_block_hash; let (height, hash) = (prepared.height, prepared.hash); @@ -105,12 +99,7 @@ impl NonFinalizedState { .or_else(|| { self.chain_set .iter() - .find_map(|chain| { - let mut mmr = finalized_tip_mmr.clone(); - // TODO: pass Sapling and Orchard roots for each block. How? - mmr.extend(block_iter); - chain.fork(parent_hash, mmr) - }) + .find_map(|chain| chain.fork(parent_hash, finalized_tip_history_tree)) .map(Box::new) }) .expect("commit_block is only called with blocks that are ready to be commited"); @@ -118,7 +107,7 @@ impl NonFinalizedState { check::block_is_contextually_valid_for_chain( &prepared, self.network, - &parent_chain.mmr_hash(), + &parent_chain.history_root_hash(), )?; parent_chain.push(prepared); self.chain_set.insert(parent_chain); @@ -131,11 +120,15 @@ impl NonFinalizedState { pub fn commit_new_chain( &mut self, prepared: PreparedBlock, - mmr: MerkleMountainRange, + history_tree: HistoryTree, ) -> Result<(), ValidateContextError> { - let mut chain = Chain::new(mmr); + let mut chain = Chain::new(history_tree); let (height, hash) = (prepared.height, prepared.hash); - check::block_is_contextually_valid_for_chain(&prepared, self.network, &chain.mmr_hash())?; + check::block_is_contextually_valid_for_chain( + &prepared, + self.network, + &chain.history_root_hash(), + )?; chain.push(prepared); self.chain_set.insert(Box::new(chain)); self.update_metrics_for_committed_block(height, hash); diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index c9f3719c8d5..57cda49987b 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -7,7 +7,7 @@ use std::{ use tracing::{debug_span, instrument, trace}; use zebra_chain::{ block::{self, ChainHistoryMmrRootHash}, - mmr::MerkleMountainRange, + mmr::HistoryTree, orchard, primitives::Groth16Proof, sapling, sprout, transaction, @@ -33,14 +33,14 @@ pub struct Chain { sapling_nullifiers: HashSet, orchard_nullifiers: HashSet, partial_cumulative_work: PartialCumulativeWork, - mmr: MerkleMountainRange, + history_tree: HistoryTree, } impl Chain { - /// Create a new empty non-finalized chain with the given MMR. + /// Create a new empty non-finalized chain with the given history tree. /// - /// The MMR must contain the history of the previous (finalized) blocks. - pub fn new(mmr: MerkleMountainRange) -> Self { + /// The history tree must contain the history of the previous (finalized) blocks. + pub fn new(history_tree: HistoryTree) -> Self { Chain { blocks: Default::default(), height_by_hash: Default::default(), @@ -53,7 +53,7 @@ impl Chain { sapling_nullifiers: Default::default(), orchard_nullifiers: Default::default(), partial_cumulative_work: Default::default(), - mmr, + history_tree, } } @@ -95,18 +95,33 @@ impl Chain { /// Fork a chain at the block with the given hash, if it is part of this /// chain. /// - /// `mmr`: the MMR for the fork. - pub fn fork(&self, fork_tip: block::Hash, mmr: MerkleMountainRange) -> Option { + /// `finalized_tip_history_tree`: the history tree for the finalized tip + /// from which the tree of the fork will be computed. + pub fn fork( + &self, + fork_tip: block::Hash, + finalized_tip_history_tree: &HistoryTree, + ) -> Option { if !self.height_by_hash.contains_key(&fork_tip) { return None; } - let mut forked = self.clone_with_mmr(mmr); + let mut forked = self.with_history_tree(finalized_tip_history_tree.clone()); while forked.non_finalized_tip_hash() != fork_tip { forked.pop_tip(); } + // Rebuild the history tree starting from the finalized tip tree. + // TODO: change to a more efficient approach by removing nodes + // from the tree of the original chain (in `pop_tip()`). + forked.history_tree.extend( + forked + .blocks + .values() + .map(|prepared_block| prepared_block.block.clone()), + ); + Some(forked) } @@ -147,14 +162,15 @@ impl Chain { self.blocks.is_empty() } - pub fn mmr_hash(&self) -> ChainHistoryMmrRootHash { - self.mmr.hash() + pub fn history_root_hash(&self) -> ChainHistoryMmrRootHash { + self.history_tree.hash() } - /// Clone the Chain but not the MMR. + /// Clone the Chain but not the history tree, using the history tree + /// specified instead. /// - /// Useful when forking, where the MMR is rebuilt anyway. - fn clone_with_mmr(&self, mmr: MerkleMountainRange) -> Self { + /// Useful when forking, where the history tree is rebuilt anyway. + fn with_history_tree(&self, history_tree: HistoryTree) -> Self { Chain { blocks: self.blocks.clone(), height_by_hash: self.height_by_hash.clone(), @@ -167,7 +183,7 @@ impl Chain { sapling_nullifiers: self.sapling_nullifiers.clone(), orchard_nullifiers: self.orchard_nullifiers.clone(), partial_cumulative_work: self.partial_cumulative_work, - mmr, + history_tree, } } } @@ -213,7 +229,7 @@ impl UpdateWith for Chain { .expect("work has already been validated"); self.partial_cumulative_work += block_work; - self.mmr.push(block); + self.history_tree.push(block); // for each transaction in block for (transaction_index, (transaction, transaction_hash)) in block @@ -296,7 +312,9 @@ impl UpdateWith for Chain { .expect("work has already been validated"); self.partial_cumulative_work -= block_work; - // Note: the MMR is not reverted here; it's rebuilt and passed in fork() + // Note: the MMR is not reverted here. + // When popping the root: there is no need to change the MMR. + // When popping the tip: the MMR is rebuilt in fork(). // for each transaction in block for (transaction, transaction_hash) in From c1dd319ff1b0467ea6664c26d361ded3e775aa78 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Thu, 17 Jun 2021 16:43:34 -0300 Subject: [PATCH 05/30] expand HistoryTree implementation --- zebra-chain/src/mmr.rs | 113 ++++++++++++++++-- zebra-chain/src/primitives/zcash_history.rs | 42 ++++--- .../primitives/zcash_history/tests/vectors.rs | 2 +- .../src/service/non_finalized_state/chain.rs | 20 ++-- 4 files changed, 138 insertions(+), 39 deletions(-) diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/mmr.rs index d138f5e4d00..72ec811b6d6 100644 --- a/zebra-chain/src/mmr.rs +++ b/zebra-chain/src/mmr.rs @@ -1,39 +1,126 @@ //! History tree (Merkle mountain range) structure that contains information about //! the block history as specified in ZIP-221. -use std::sync::Arc; +use std::{collections::HashMap, sync::Arc}; -use crate::block::{Block, ChainHistoryMmrRootHash}; +use crate::{ + block::{Block, ChainHistoryMmrRootHash}, + orchard, + parameters::{Network, NetworkUpgrade}, + primitives::zcash_history::{Entry, Tree}, + sapling, +}; /// History tree structure. pub struct HistoryTree { - // TODO + network: Network, + network_upgrade: NetworkUpgrade, + // Merkle mountain range tree. + // This is a "runtime" structure used to add / remove nodes, and it's not + // persistent. + inner: Tree, + // The number of nodes in the tree. + size: u32, + // The peaks of the tree, indexed by their position in the array representation + // of the tree. This can be persisted to save the tree. + // TODO: use NodeData instead of Entry to save space? Requires re-deriving + // the entry metadata from its position. + peaks: HashMap, } impl HistoryTree { + /// Create a new history tree with a single block. + pub fn new_from_block( + network: Network, + network_upgrade: NetworkUpgrade, + block: Arc, + sapling_root: &sapling::tree::Root, + _orchard_root: Option<&orchard::tree::Root>, + ) -> Self { + let (tree, entry) = + Tree::new_from_block(network, block, sapling_root).expect("TODO: handle error"); + let mut peaks = HashMap::new(); + peaks.insert(0u32, entry); + HistoryTree { + network, + network_upgrade, + inner: tree, + size: 1, + peaks, + } + } + /// Add block data to the tree. - pub fn push(&mut self, _block: &Block) { - // TODO: zcash_history::Tree::append_leaf receives an Arc. Should - // change that to match, or create an Arc to pass to it? - todo!(); + pub fn push( + &mut self, + block: Arc, + sapling_root: &sapling::tree::Root, + _orchard_root: Option<&orchard::tree::Root>, + ) { + // TODO: handle orchard root + let new_entries = self + .inner + .append_leaf(block, sapling_root) + .expect("TODO: handle error"); + for entry in new_entries { + // Not every entry is a peak; those will be trimmed later + self.peaks.insert(self.size, entry); + self.size += 1; + } + // TODO: trim entries? + // TODO: rebuild Tree from the peaks? When / how often? + // (zcashd rebuilds it on every operation) } /// Return the hash of the tree root. pub fn hash(&self) -> ChainHistoryMmrRootHash { - todo!(); + self.inner.hash() } } impl Clone for HistoryTree { fn clone(&self) -> Self { - todo!(); + let tree = Tree::new_from_cache( + self.network, + self.network_upgrade, + self.size, + &self.peaks, + &HashMap::new(), + ) + .expect("rebuilding an existing tree should always work"); + HistoryTree { + network: self.network, + network_upgrade: self.network_upgrade, + inner: tree, + size: self.size, + peaks: self.peaks.clone(), + } } } -impl Extend> for HistoryTree { +impl<'a> + Extend<( + Arc, + &'a sapling::tree::Root, + Option<&'a orchard::tree::Root>, + )> for HistoryTree +{ /// Extend the history tree with the given blocks. - // TODO: add Sapling and Orchard roots to parameters - fn extend>>(&mut self, _iter: T) { - todo!() + fn extend< + T: IntoIterator< + Item = ( + Arc, + &'a sapling::tree::Root, + Option<&'a orchard::tree::Root>, + ), + >, + >( + &mut self, + iter: T, + ) { + for (block, sapling_root, orchard_root) in iter { + // TODO: handle errors; give up on using Extend trait? + self.push(block, &sapling_root, orchard_root); + } } } diff --git a/zebra-chain/src/primitives/zcash_history.rs b/zebra-chain/src/primitives/zcash_history.rs index 1c96ab4772a..4973c8b8468 100644 --- a/zebra-chain/src/primitives/zcash_history.rs +++ b/zebra-chain/src/primitives/zcash_history.rs @@ -44,6 +44,7 @@ impl From<&zcash_history::NodeData> for NodeData { /// An encoded entry in the tree. /// Contains the node data and information about its position in the tree. +#[derive(Clone)] pub struct Entry { inner: [u8; zcash_history::MAX_ENTRY_SIZE], } @@ -101,12 +102,12 @@ impl Tree { /// # Panics /// /// Will panic if `peaks` is empty. - fn new_from_cache( + pub fn new_from_cache( network: Network, network_upgrade: NetworkUpgrade, length: u32, - peaks: &HashMap, - extra: &HashMap, + peaks: &HashMap, + extra: &HashMap, ) -> Result { let branch_id = network_upgrade .branch_id() @@ -132,19 +133,24 @@ impl Tree { /// Create a single-node MMR tree for the given block. /// /// `sapling_root` is the root of the Sapling note commitment tree of the block. - fn new_from_block( + pub fn new_from_block( network: Network, block: Arc, sapling_root: &sapling::tree::Root, - ) -> Result { + ) -> Result<(Self, Entry), io::Error> { let height = block .coinbase_height() .expect("block must have coinbase height during contextual verification"); let network_upgrade = NetworkUpgrade::current(network, height); let entry0 = Entry::new_leaf(block, network, sapling_root); let mut peaks = HashMap::new(); - peaks.insert(0u32, &entry0); - Tree::new_from_cache(network, network_upgrade, 1, &peaks, &HashMap::new()) + peaks.insert(0u32, entry0); + Ok(( + Tree::new_from_cache(network, network_upgrade, 1, &peaks, &HashMap::new())?, + peaks + .remove(&0u32) + .expect("must work since it was just added"), + )) } /// Append a new block to the tree, as a new leaf. @@ -157,11 +163,11 @@ impl Tree { /// /// Panics if the network upgrade of the given block is different from /// the network upgrade of the other blocks in the tree. - fn append_leaf( + pub fn append_leaf( &mut self, block: Arc, sapling_root: &sapling::tree::Root, - ) -> Result, zcash_history::Error> { + ) -> Result, zcash_history::Error> { let height = block .coinbase_height() .expect("block must have coinbase height during contextual verification"); @@ -177,17 +183,17 @@ impl Tree { let appended = self.inner.append_leaf(node_data)?; let mut new_nodes = Vec::new(); - for entry in appended { - let mut node = NodeData { - inner: [0; zcash_history::MAX_NODE_DATA_SIZE], + for entry_link in appended { + let mut entry = Entry { + inner: [0; zcash_history::MAX_ENTRY_SIZE], }; self.inner - .resolve_link(entry) + .resolve_link(entry_link) .expect("entry was just generated so it must be valid") - .data() - .write(&mut &mut node.inner[..]) + .node() + .write(&mut &mut entry.inner[..]) .expect("buffer was created with enough capacity"); - new_nodes.push(node); + new_nodes.push(entry); } Ok(new_nodes) } @@ -196,7 +202,7 @@ impl Tree { fn append_leaf_iter( &mut self, vals: impl Iterator, sapling::tree::Root)>, - ) -> Result, zcash_history::Error> { + ) -> Result, zcash_history::Error> { let mut new_nodes = Vec::new(); for (block, root) in vals { new_nodes.append(&mut self.append_leaf(block, &root)?); @@ -212,7 +218,7 @@ impl Tree { } /// Return the root hash of the tree, i.e. `hashChainHistoryRoot`. - fn hash(&self) -> ChainHistoryMmrRootHash { + pub fn hash(&self) -> ChainHistoryMmrRootHash { // Both append_leaf() and truncate_leaf() leave a root node, so it should // always exist. self.inner diff --git a/zebra-chain/src/primitives/zcash_history/tests/vectors.rs b/zebra-chain/src/primitives/zcash_history/tests/vectors.rs index 88bf6b21492..6fd927640ee 100644 --- a/zebra-chain/src/primitives/zcash_history/tests/vectors.rs +++ b/zebra-chain/src/primitives/zcash_history/tests/vectors.rs @@ -49,7 +49,7 @@ fn tree_for_network_upgrade(network: Network, network_upgrade: NetworkUpgrade) - // Build initial MMR tree with only Block 0 let sapling_root0 = sapling::tree::Root(**sapling_roots.get(&height).expect("test vector exists")); - let mut tree = Tree::new_from_block(network, block0, &sapling_root0)?; + let (mut tree, _) = Tree::new_from_block(network, block0, &sapling_root0)?; // Compute root hash of the MMR tree, which will be included in the next block let hash0 = tree.hash(); diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 57cda49987b..57dfb8ab694 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -115,12 +115,16 @@ impl Chain { // Rebuild the history tree starting from the finalized tip tree. // TODO: change to a more efficient approach by removing nodes // from the tree of the original chain (in `pop_tip()`). - forked.history_tree.extend( - forked - .blocks - .values() - .map(|prepared_block| prepared_block.block.clone()), - ); + forked + .history_tree + .extend(forked.blocks.values().map(|prepared_block| { + ( + prepared_block.block.clone(), + // TODO: pass Sapling and Orchard roots + &sapling::tree::Root([0; 32]), + None, + ) + })); Some(forked) } @@ -229,7 +233,9 @@ impl UpdateWith for Chain { .expect("work has already been validated"); self.partial_cumulative_work += block_work; - self.history_tree.push(block); + // TODO: pass Sapling and Orchard roots + self.history_tree + .push(prepared.block.clone(), &sapling::tree::Root([0; 32]), None); // for each transaction in block for (transaction_index, (transaction, transaction_hash)) in block From 59b46fe0014ed16c2d7453ba075a4e156db32d27 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Mon, 21 Jun 2021 16:18:30 -0300 Subject: [PATCH 06/30] handle and propagate errors --- zebra-chain/src/mmr.rs | 76 ++++++++++--------- zebra-state/src/error.rs | 4 + .../src/service/non_finalized_state.rs | 29 ++++--- .../src/service/non_finalized_state/chain.rs | 62 ++++++++++----- 4 files changed, 104 insertions(+), 67 deletions(-) diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/mmr.rs index 72ec811b6d6..ad0c499e23b 100644 --- a/zebra-chain/src/mmr.rs +++ b/zebra-chain/src/mmr.rs @@ -1,7 +1,9 @@ //! History tree (Merkle mountain range) structure that contains information about //! the block history as specified in ZIP-221. -use std::{collections::HashMap, sync::Arc}; +use std::{collections::HashMap, io, sync::Arc}; + +use thiserror::Error; use crate::{ block::{Block, ChainHistoryMmrRootHash}, @@ -11,6 +13,16 @@ use crate::{ sapling, }; +/// An error describing why a history tree operation failed. +#[derive(Debug, Error)] +#[non_exhaustive] +#[allow(missing_docs)] +pub enum HistoryTreeError { + #[error("error from the underlying library: {inner:?}")] + #[non_exhaustive] + InnerError { inner: zcash_history::Error }, +} + /// History tree structure. pub struct HistoryTree { network: Network, @@ -36,18 +48,18 @@ impl HistoryTree { block: Arc, sapling_root: &sapling::tree::Root, _orchard_root: Option<&orchard::tree::Root>, - ) -> Self { - let (tree, entry) = - Tree::new_from_block(network, block, sapling_root).expect("TODO: handle error"); + ) -> Result { + // TODO: handle Orchard root + let (tree, entry) = Tree::new_from_block(network, block, sapling_root)?; let mut peaks = HashMap::new(); peaks.insert(0u32, entry); - HistoryTree { + Ok(HistoryTree { network, network_upgrade, inner: tree, size: 1, peaks, - } + }) } /// Add block data to the tree. @@ -56,12 +68,12 @@ impl HistoryTree { block: Arc, sapling_root: &sapling::tree::Root, _orchard_root: Option<&orchard::tree::Root>, - ) { + ) -> Result<(), HistoryTreeError> { // TODO: handle orchard root let new_entries = self .inner .append_leaf(block, sapling_root) - .expect("TODO: handle error"); + .map_err(|e| HistoryTreeError::InnerError { inner: e })?; for entry in new_entries { // Not every entry is a peak; those will be trimmed later self.peaks.insert(self.size, entry); @@ -70,6 +82,27 @@ impl HistoryTree { // TODO: trim entries? // TODO: rebuild Tree from the peaks? When / how often? // (zcashd rebuilds it on every operation) + Ok(()) + } + + /// Extend the history tree with the given blocks. + pub fn extend< + 'a, + T: IntoIterator< + Item = ( + Arc, + &'a sapling::tree::Root, + Option<&'a orchard::tree::Root>, + ), + >, + >( + &mut self, + iter: T, + ) -> Result<(), HistoryTreeError> { + for (block, sapling_root, orchard_root) in iter { + self.push(block, sapling_root, orchard_root)?; + } + Ok(()) } /// Return the hash of the tree root. @@ -97,30 +130,3 @@ impl Clone for HistoryTree { } } } - -impl<'a> - Extend<( - Arc, - &'a sapling::tree::Root, - Option<&'a orchard::tree::Root>, - )> for HistoryTree -{ - /// Extend the history tree with the given blocks. - fn extend< - T: IntoIterator< - Item = ( - Arc, - &'a sapling::tree::Root, - Option<&'a orchard::tree::Root>, - ), - >, - >( - &mut self, - iter: T, - ) { - for (block, sapling_root, orchard_root) in iter { - // TODO: handle errors; give up on using Extend trait? - self.push(block, &sapling_root, orchard_root); - } - } -} diff --git a/zebra-state/src/error.rs b/zebra-state/src/error.rs index 3e4e2150fe0..eea10da6c7f 100644 --- a/zebra-state/src/error.rs +++ b/zebra-state/src/error.rs @@ -5,6 +5,7 @@ use thiserror::Error; use zebra_chain::{ block::{self, ChainHistoryMmrRootHash, CommitmentError}, + mmr::HistoryTreeError, work::difficulty::CompactDifficulty, }; @@ -87,4 +88,7 @@ pub enum ValidateContextError { candidate_commitment: ChainHistoryMmrRootHash, expected_commitment: ChainHistoryMmrRootHash, }, + + #[error("error building the history tree")] + HistoryTreeError(#[from] HistoryTreeError), } diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index ac9f7af95aa..c61646be925 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -94,22 +94,29 @@ impl NonFinalizedState { ); } - let mut parent_chain = self - .take_chain_if(|chain| chain.non_finalized_tip_hash() == parent_hash) - .or_else(|| { - self.chain_set - .iter() - .find_map(|chain| chain.fork(parent_hash, finalized_tip_history_tree)) - .map(Box::new) - }) - .expect("commit_block is only called with blocks that are ready to be commited"); + let mut parent_chain = + match self.take_chain_if(|chain| chain.non_finalized_tip_hash() == parent_hash) { + Some(chain) => chain, + None => Box::new( + self.chain_set + .iter() + .find_map(|chain| { + chain + .fork(parent_hash, finalized_tip_history_tree) + .transpose() + }) + .expect( + "commit_block is only called with blocks that are ready to be commited", + )?, + ), + }; check::block_is_contextually_valid_for_chain( &prepared, self.network, &parent_chain.history_root_hash(), )?; - parent_chain.push(prepared); + parent_chain.push(prepared)?; self.chain_set.insert(parent_chain); self.update_metrics_for_committed_block(height, hash); Ok(()) @@ -129,7 +136,7 @@ impl NonFinalizedState { self.network, &chain.history_root_hash(), )?; - chain.push(prepared); + chain.push(prepared)?; self.chain_set.insert(Box::new(chain)); self.update_metrics_for_committed_block(height, hash); Ok(()) diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 57dfb8ab694..0836d9bc513 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -7,7 +7,7 @@ use std::{ use tracing::{debug_span, instrument, trace}; use zebra_chain::{ block::{self, ChainHistoryMmrRootHash}, - mmr::HistoryTree, + mmr::{HistoryTree, HistoryTreeError}, orchard, primitives::Groth16Proof, sapling, sprout, transaction, @@ -59,11 +59,12 @@ impl Chain { /// Push a contextually valid non-finalized block into a chain as the new tip. #[instrument(level = "debug", skip(self, block), fields(block = %block.block))] - pub fn push(&mut self, block: PreparedBlock) { + pub fn push(&mut self, block: PreparedBlock) -> Result<(), HistoryTreeError> { // update cumulative data members - self.update_chain_state_with(&block); + self.update_chain_state_with(&block)?; tracing::debug!(block = %block.block, "adding block to chain"); self.blocks.insert(block.height, block); + Ok(()) } /// Remove the lowest height block of the non-finalized portion of a chain. @@ -101,9 +102,9 @@ impl Chain { &self, fork_tip: block::Hash, finalized_tip_history_tree: &HistoryTree, - ) -> Option { + ) -> Result, HistoryTreeError> { if !self.height_by_hash.contains_key(&fork_tip) { - return None; + return Ok(None); } let mut forked = self.with_history_tree(finalized_tip_history_tree.clone()); @@ -124,9 +125,9 @@ impl Chain { &sapling::tree::Root([0; 32]), None, ) - })); + }))?; - Some(forked) + Ok(Some(forked)) } pub fn non_finalized_tip_hash(&self) -> block::Hash { @@ -202,7 +203,7 @@ impl Chain { trait UpdateWith { /// Update `Chain` cumulative data members to add data that are derived from /// `T` - fn update_chain_state_with(&mut self, _: &T); + fn update_chain_state_with(&mut self, _: &T) -> Result<(), HistoryTreeError>; /// Update `Chain` cumulative data members to remove data that are derived /// from `T` @@ -210,7 +211,10 @@ trait UpdateWith { } impl UpdateWith for Chain { - fn update_chain_state_with(&mut self, prepared: &PreparedBlock) { + fn update_chain_state_with( + &mut self, + prepared: &PreparedBlock, + ) -> Result<(), HistoryTreeError> { let (block, hash, height, transaction_hashes) = ( prepared.block.as_ref(), prepared.hash, @@ -235,7 +239,7 @@ impl UpdateWith for Chain { // TODO: pass Sapling and Orchard roots self.history_tree - .push(prepared.block.clone(), &sapling::tree::Root([0; 32]), None); + .push(prepared.block.clone(), &sapling::tree::Root([0; 32]), None)?; // for each transaction in block for (transaction_index, (transaction, transaction_hash)) in block @@ -284,16 +288,18 @@ impl UpdateWith for Chain { ); // add the utxos this produced - self.update_chain_state_with(&prepared.new_outputs); + self.update_chain_state_with(&prepared.new_outputs)?; // add the utxos this consumed - self.update_chain_state_with(inputs); + self.update_chain_state_with(inputs)?; // add the shielded data - self.update_chain_state_with(joinsplit_data); - self.update_chain_state_with(sapling_shielded_data_per_spend_anchor); - self.update_chain_state_with(sapling_shielded_data_shared_anchor); - self.update_chain_state_with(orchard_shielded_data); + self.update_chain_state_with(joinsplit_data)?; + self.update_chain_state_with(sapling_shielded_data_per_spend_anchor)?; + self.update_chain_state_with(sapling_shielded_data_shared_anchor)?; + self.update_chain_state_with(orchard_shielded_data)?; } + + Ok(()) } #[instrument(skip(self, prepared), fields(block = %prepared.block))] @@ -377,9 +383,13 @@ impl UpdateWith for Chain { } impl UpdateWith> for Chain { - fn update_chain_state_with(&mut self, utxos: &HashMap) { + fn update_chain_state_with( + &mut self, + utxos: &HashMap, + ) -> Result<(), HistoryTreeError> { self.created_utxos .extend(utxos.iter().map(|(k, v)| (*k, v.clone()))); + Ok(()) } fn revert_chain_state_with(&mut self, utxos: &HashMap) { @@ -389,7 +399,10 @@ impl UpdateWith> for Chain { } impl UpdateWith> for Chain { - fn update_chain_state_with(&mut self, inputs: &Vec) { + fn update_chain_state_with( + &mut self, + inputs: &Vec, + ) -> Result<(), HistoryTreeError> { for consumed_utxo in inputs { match consumed_utxo { transparent::Input::PrevOut { outpoint, .. } => { @@ -398,6 +411,7 @@ impl UpdateWith> for Chain { transparent::Input::Coinbase { .. } => {} } } + Ok(()) } fn revert_chain_state_with(&mut self, inputs: &Vec) { @@ -420,7 +434,7 @@ impl UpdateWith>> for Chain { fn update_chain_state_with( &mut self, joinsplit_data: &Option>, - ) { + ) -> Result<(), HistoryTreeError> { if let Some(joinsplit_data) = joinsplit_data { for sprout::JoinSplit { nullifiers, .. } in joinsplit_data.joinsplits() { let span = debug_span!("revert_chain_state_with", ?nullifiers); @@ -430,6 +444,7 @@ impl UpdateWith>> for Chain { self.sprout_nullifiers.insert(nullifiers[1]); } } + Ok(()) } #[instrument(skip(self, joinsplit_data))] @@ -462,12 +477,13 @@ where fn update_chain_state_with( &mut self, sapling_shielded_data: &Option>, - ) { + ) -> Result<(), HistoryTreeError> { if let Some(sapling_shielded_data) = sapling_shielded_data { for nullifier in sapling_shielded_data.nullifiers() { self.sapling_nullifiers.insert(*nullifier); } } + Ok(()) } fn revert_chain_state_with( @@ -486,12 +502,16 @@ where } impl UpdateWith> for Chain { - fn update_chain_state_with(&mut self, orchard_shielded_data: &Option) { + fn update_chain_state_with( + &mut self, + orchard_shielded_data: &Option, + ) -> Result<(), HistoryTreeError> { if let Some(orchard_shielded_data) = orchard_shielded_data { for nullifier in orchard_shielded_data.nullifiers() { self.orchard_nullifiers.insert(*nullifier); } } + Ok(()) } fn revert_chain_state_with(&mut self, orchard_shielded_data: &Option) { From d06dfcc8fe24428f44557a7a7085805905259203 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Mon, 21 Jun 2021 17:01:40 -0300 Subject: [PATCH 07/30] simplify check.rs tracing --- zebra-state/src/service/check.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs index 89b5e554c60..ccb86836af4 100644 --- a/zebra-state/src/service/check.rs +++ b/zebra-state/src/service/check.rs @@ -30,8 +30,7 @@ pub(crate) mod difficulty; /// (`POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN`) blocks. #[tracing::instrument( name = "contextual_validation", - fields(?network), - skip(prepared, network, finalized_tip_height, relevant_chain) + skip(prepared, finalized_tip_height, relevant_chain) )] pub(crate) fn block_is_contextually_valid( prepared: &PreparedBlock, @@ -86,11 +85,7 @@ where Ok(()) } -#[tracing::instrument( - name = "contextual_validation_for_chain", - fields(?network), - skip(prepared, network) -)] +#[tracing::instrument(skip(prepared))] pub(crate) fn block_is_contextually_valid_for_chain( prepared: &PreparedBlock, network: Network, From 1cf901393e0cdf307d8fefe19fbe1c954bfe4055 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Mon, 21 Jun 2021 17:07:07 -0300 Subject: [PATCH 08/30] add suggested TODO --- zebra-chain/src/mmr.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/mmr.rs index ad0c499e23b..d843ad90544 100644 --- a/zebra-chain/src/mmr.rs +++ b/zebra-chain/src/mmr.rs @@ -82,6 +82,7 @@ impl HistoryTree { // TODO: trim entries? // TODO: rebuild Tree from the peaks? When / how often? // (zcashd rebuilds it on every operation) + // TODO: implement network upgrade logic: drop previous history, start new history Ok(()) } From 89c12c6846a49537d0da16df739aac06e5c25b4f Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Mon, 21 Jun 2021 20:49:07 -0300 Subject: [PATCH 09/30] add HistoryTree::prune --- zebra-chain/src/mmr.rs | 62 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/mmr.rs index d843ad90544..4d8d6557008 100644 --- a/zebra-chain/src/mmr.rs +++ b/zebra-chain/src/mmr.rs @@ -1,7 +1,11 @@ //! History tree (Merkle mountain range) structure that contains information about //! the block history as specified in ZIP-221. -use std::{collections::HashMap, io, sync::Arc}; +use std::{ + collections::{HashMap, HashSet}, + io, + sync::Arc, +}; use thiserror::Error; @@ -21,6 +25,9 @@ pub enum HistoryTreeError { #[error("error from the underlying library: {inner:?}")] #[non_exhaustive] InnerError { inner: zcash_history::Error }, + + #[error("I/O error")] + IOError(#[from] io::Error), } /// History tree structure. @@ -79,9 +86,8 @@ impl HistoryTree { self.peaks.insert(self.size, entry); self.size += 1; } - // TODO: trim entries? - // TODO: rebuild Tree from the peaks? When / how often? - // (zcashd rebuilds it on every operation) + // TODO: should we really prune every iteration? + self.prune()?; // TODO: implement network upgrade logic: drop previous history, start new history Ok(()) } @@ -106,6 +112,54 @@ impl HistoryTree { Ok(()) } + /// Prune tree, removing all non-peak entries. + fn prune(&mut self) -> Result<(), io::Error> { + // Go through all the peaks of the tree. + // This code is from a librustzcash example. + // https://github.com/zcash/librustzcash/blob/02052526925fba9389f1428d6df254d4dec967e6/zcash_history/examples/long.rs + // See also coins.cpp in zcashd for a explanation of how it works. + // https://github.com/zcash/zcash/blob/0247c0c682d59184a717a6536edb0d18834be9a7/src/coins.cpp#L351 + + // TODO: should we just copy the explanation here? + + let mut peak_pos_set = HashSet::new(); + // integer log2 of (self.size+1), -1 + let mut h = (32 - ((self.size + 1) as u32).leading_zeros() - 1) - 1; + let mut peak_pos = (1 << (h + 1)) - 1; + + loop { + if peak_pos > self.size { + // left child, -2^h + peak_pos -= 1 << h; + h -= 1; + } + + if peak_pos <= self.size { + // There is a peak at index `peak_pos` + peak_pos_set.insert(peak_pos); + + // right sibling + peak_pos = peak_pos + (1 << (h + 1)) - 1; + } + + if h == 0 { + break; + } + } + + // Remove all non-peak entries + self.peaks.retain(|k, _| peak_pos_set.contains(k)); + // Rebuild tree + self.inner = Tree::new_from_cache( + self.network, + self.network_upgrade, + self.size, + &self.peaks, + &Default::default(), + )?; + Ok(()) + } + /// Return the hash of the tree root. pub fn hash(&self) -> ChainHistoryMmrRootHash { self.inner.hash() From b3d773a8ad18be51f9889364b9aa7b147462d9a5 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Tue, 22 Jun 2021 16:18:33 -0300 Subject: [PATCH 10/30] fix bug in pruning --- zebra-chain/src/mmr.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/mmr.rs index 4d8d6557008..f0c48e654fd 100644 --- a/zebra-chain/src/mmr.rs +++ b/zebra-chain/src/mmr.rs @@ -51,11 +51,14 @@ impl HistoryTree { /// Create a new history tree with a single block. pub fn new_from_block( network: Network, - network_upgrade: NetworkUpgrade, block: Arc, sapling_root: &sapling::tree::Root, _orchard_root: Option<&orchard::tree::Root>, ) -> Result { + let height = block + .coinbase_height() + .expect("block must have coinbase height during contextual verification"); + let network_upgrade = NetworkUpgrade::current(network, height); // TODO: handle Orchard root let (tree, entry) = Tree::new_from_block(network, block, sapling_root)?; let mut peaks = HashMap::new(); @@ -135,8 +138,8 @@ impl HistoryTree { } if peak_pos <= self.size { - // There is a peak at index `peak_pos` - peak_pos_set.insert(peak_pos); + // There is a peak at index `peak_pos - 1` + peak_pos_set.insert(peak_pos - 1); // right sibling peak_pos = peak_pos + (1 << (h + 1)) - 1; From e2ab46cc45c99d9aa49d795d1b67edcdcefde9d8 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Tue, 22 Jun 2021 16:18:49 -0300 Subject: [PATCH 11/30] fix compilation of tests; still need to make them pass --- .../src/service/non_finalized_state.rs | 2 + .../service/non_finalized_state/tests/prop.rs | 25 ++-- .../non_finalized_state/tests/vectors.rs | 136 +++++++++++++----- 3 files changed, 115 insertions(+), 48 deletions(-) diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index c61646be925..5a9211af6c6 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -124,6 +124,8 @@ impl NonFinalizedState { /// Commit block to the non-finalized state as a new chain where its parent /// is the finalized tip. + /// + /// `history_tree` must contain the history of the finalized tip. pub fn commit_new_chain( &mut self, prepared: PreparedBlock, diff --git a/zebra-state/src/service/non_finalized_state/tests/prop.rs b/zebra-state/src/service/non_finalized_state/tests/prop.rs index 4d9f43c40d9..40f312b409e 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -1,5 +1,6 @@ use std::env; +use zebra_chain::{mmr::HistoryTree, sapling}; use zebra_test::prelude::*; use crate::service::non_finalized_state::{arbitrary::PreparedChain, Chain}; @@ -14,19 +15,20 @@ fn forked_equals_pushed() -> Result<()> { .ok() .and_then(|v| v.parse().ok()) .unwrap_or(DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES)), - |((chain, count, _network) in PreparedChain::default())| { + |((chain, count, network) in PreparedChain::default())| { + let dummy_tree = HistoryTree::new_from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); let fork_tip_hash = chain[count - 1].hash; - let mut full_chain = Chain::default(); - let mut partial_chain = Chain::default(); + let mut full_chain = Chain::new(dummy_tree.clone()); + let mut partial_chain = Chain::new(dummy_tree.clone()); for block in chain.iter().take(count) { - partial_chain.push(block.clone()); + partial_chain.push(block.clone())?; } for block in chain.iter() { - full_chain.push(block.clone()); + full_chain.push(block.clone())?; } - let forked = full_chain.fork(fork_tip_hash).expect("hash is present"); + let forked = full_chain.fork(fork_tip_hash, &dummy_tree).expect("fork must work").expect("hash is present"); prop_assert_eq!(forked.blocks.len(), partial_chain.blocks.len()); }); @@ -42,16 +44,17 @@ fn finalized_equals_pushed() -> Result<()> { .ok() .and_then(|v| v.parse().ok()) .unwrap_or(DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES)), - |((chain, end_count, _network) in PreparedChain::default())| { + |((chain, end_count, network) in PreparedChain::default())| { + let dummy_tree = HistoryTree::new_from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); let finalized_count = chain.len() - end_count; - let mut full_chain = Chain::default(); - let mut partial_chain = Chain::default(); + let mut full_chain = Chain::new(dummy_tree.clone()); + let mut partial_chain = Chain::new(dummy_tree); for block in chain.iter().skip(finalized_count) { - partial_chain.push(block.clone()); + partial_chain.push(block.clone())?; } for block in chain.iter() { - full_chain.push(block.clone()); + full_chain.push(block.clone())?; } for _ in 0..finalized_count { diff --git a/zebra-state/src/service/non_finalized_state/tests/vectors.rs b/zebra-state/src/service/non_finalized_state/tests/vectors.rs index c6adb460e38..ece1833a27d 100644 --- a/zebra-state/src/service/non_finalized_state/tests/vectors.rs +++ b/zebra-state/src/service/non_finalized_state/tests/vectors.rs @@ -1,6 +1,9 @@ use std::sync::Arc; -use zebra_chain::{block::Block, parameters::Network, serialization::ZcashDeserializeInto}; +use zebra_chain::{ + block::Block, mmr::HistoryTree, parameters::Network, sapling, + serialization::ZcashDeserializeInto, +}; use zebra_test::prelude::*; use crate::{ @@ -10,20 +13,22 @@ use crate::{ use self::assert_eq; -#[test] -fn construct_empty() { - zebra_test::init(); - let _chain = Chain::default(); -} - #[test] fn construct_single() -> Result<()> { zebra_test::init(); let block: Arc = zebra_test::vectors::BLOCK_MAINNET_434873_BYTES.zcash_deserialize_into()?; - let mut chain = Chain::default(); - chain.push(block.prepare()); + let dummy_tree = HistoryTree::new_from_block( + Network::Mainnet, + block.clone(), + &sapling::tree::Root::default(), + None, + ) + .unwrap(); + + let mut chain = Chain::new(dummy_tree); + chain.push(block.prepare())?; assert_eq!(1, chain.blocks.len()); @@ -44,10 +49,17 @@ fn construct_many() -> Result<()> { block = next_block; } - let mut chain = Chain::default(); + let dummy_tree = HistoryTree::new_from_block( + Network::Mainnet, + block, + &sapling::tree::Root::default(), + None, + ) + .unwrap(); + let mut chain = Chain::new(dummy_tree); for block in blocks { - chain.push(block.prepare()); + chain.push(block.prepare())?; } assert_eq!(100, chain.blocks.len()); @@ -63,11 +75,19 @@ fn ord_matches_work() -> Result<()> { .set_work(1); let more_block = less_block.clone().set_work(10); - let mut lesser_chain = Chain::default(); - lesser_chain.push(less_block.prepare()); + let dummy_tree = HistoryTree::new_from_block( + Network::Mainnet, + less_block.clone(), + &sapling::tree::Root::default(), + None, + ) + .unwrap(); + + let mut lesser_chain = Chain::new(dummy_tree.clone()); + lesser_chain.push(less_block.prepare())?; - let mut bigger_chain = Chain::default(); - bigger_chain.push(more_block.prepare()); + let mut bigger_chain = Chain::new(dummy_tree); + bigger_chain.push(more_block.prepare())?; assert!(bigger_chain > lesser_chain); @@ -93,6 +113,13 @@ fn best_chain_wins_for_network(network: Network) -> Result<()> { zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; + let dummy_tree = HistoryTree::new_from_block( + network, + block1.clone(), + &sapling::tree::Root::default(), + None, + ) + .unwrap(); let block2 = block1.make_fake_child().set_work(10); let child = block1.make_fake_child().set_work(1); @@ -100,8 +127,8 @@ fn best_chain_wins_for_network(network: Network) -> Result<()> { let expected_hash = block2.hash(); let mut state = NonFinalizedState::default(); - state.commit_new_chain(block2.prepare()); - state.commit_new_chain(child.prepare()); + state.commit_new_chain(block2.prepare(), dummy_tree.clone())?; + state.commit_new_chain(child.prepare(), dummy_tree)?; let best_chain = state.best_chain().unwrap(); assert!(best_chain.height_by_hash.contains_key(&expected_hash)); @@ -128,14 +155,21 @@ fn finalize_pops_from_best_chain_for_network(network: Network) -> Result<()> { zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; + let dummy_tree = HistoryTree::new_from_block( + network, + block1.clone(), + &sapling::tree::Root::default(), + None, + ) + .unwrap(); let block2 = block1.make_fake_child().set_work(10); let child = block1.make_fake_child().set_work(1); let mut state = NonFinalizedState::default(); - state.commit_new_chain(block1.clone().prepare()); - state.commit_block(block2.clone().prepare()); - state.commit_block(child.prepare()); + state.commit_new_chain(block1.clone().prepare(), dummy_tree.clone())?; + state.commit_block(block2.clone().prepare(), &dummy_tree)?; + state.commit_block(child.prepare(), &dummy_tree)?; let finalized = state.finalize(); assert_eq!(block1, finalized.block); @@ -170,6 +204,13 @@ fn commit_block_extending_best_chain_doesnt_drop_worst_chains_for_network( zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; + let dummy_tree = HistoryTree::new_from_block( + network, + block1.clone(), + &sapling::tree::Root::default(), + None, + ) + .unwrap(); let block2 = block1.make_fake_child().set_work(10); let child1 = block1.make_fake_child().set_work(1); @@ -177,13 +218,13 @@ fn commit_block_extending_best_chain_doesnt_drop_worst_chains_for_network( let mut state = NonFinalizedState::default(); assert_eq!(0, state.chain_set.len()); - state.commit_new_chain(block1.prepare()); + state.commit_new_chain(block1.prepare(), dummy_tree.clone())?; assert_eq!(1, state.chain_set.len()); - state.commit_block(block2.prepare()); + state.commit_block(block2.prepare(), &dummy_tree)?; assert_eq!(1, state.chain_set.len()); - state.commit_block(child1.prepare()); + state.commit_block(child1.prepare(), &dummy_tree)?; assert_eq!(2, state.chain_set.len()); - state.commit_block(child2.prepare()); + state.commit_block(child2.prepare(), &dummy_tree)?; assert_eq!(2, state.chain_set.len()); Ok(()) @@ -208,6 +249,13 @@ fn shorter_chain_can_be_best_chain_for_network(network: Network) -> Result<()> { zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; + let dummy_tree = HistoryTree::new_from_block( + network, + block1.clone(), + &sapling::tree::Root::default(), + None, + ) + .unwrap(); let long_chain_block1 = block1.make_fake_child().set_work(1); let long_chain_block2 = long_chain_block1.make_fake_child().set_work(1); @@ -215,10 +263,10 @@ fn shorter_chain_can_be_best_chain_for_network(network: Network) -> Result<()> { let short_chain_block = block1.make_fake_child().set_work(3); let mut state = NonFinalizedState::default(); - state.commit_new_chain(block1.prepare()); - state.commit_block(long_chain_block1.prepare()); - state.commit_block(long_chain_block2.prepare()); - state.commit_block(short_chain_block.prepare()); + state.commit_new_chain(block1.prepare(), dummy_tree.clone())?; + state.commit_block(long_chain_block1.prepare(), &dummy_tree)?; + state.commit_block(long_chain_block2.prepare(), &dummy_tree)?; + state.commit_block(short_chain_block.prepare(), &dummy_tree)?; assert_eq!(2, state.chain_set.len()); assert_eq!(2, state.best_chain_len()); @@ -245,6 +293,13 @@ fn longer_chain_with_more_work_wins_for_network(network: Network) -> Result<()> zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; + let dummy_tree = HistoryTree::new_from_block( + network, + block1.clone(), + &sapling::tree::Root::default(), + None, + ) + .unwrap(); let long_chain_block1 = block1.make_fake_child().set_work(1); let long_chain_block2 = long_chain_block1.make_fake_child().set_work(1); @@ -254,12 +309,12 @@ fn longer_chain_with_more_work_wins_for_network(network: Network) -> Result<()> let short_chain_block = block1.make_fake_child().set_work(3); let mut state = NonFinalizedState::default(); - state.commit_new_chain(block1.prepare()); - state.commit_block(long_chain_block1.prepare()); - state.commit_block(long_chain_block2.prepare()); - state.commit_block(long_chain_block3.prepare()); - state.commit_block(long_chain_block4.prepare()); - state.commit_block(short_chain_block.prepare()); + state.commit_new_chain(block1.prepare(), dummy_tree.clone())?; + state.commit_block(long_chain_block1.prepare(), &dummy_tree)?; + state.commit_block(long_chain_block2.prepare(), &dummy_tree)?; + state.commit_block(long_chain_block3.prepare(), &dummy_tree)?; + state.commit_block(long_chain_block4.prepare(), &dummy_tree)?; + state.commit_block(short_chain_block.prepare(), &dummy_tree)?; assert_eq!(2, state.chain_set.len()); assert_eq!(5, state.best_chain_len()); @@ -285,15 +340,22 @@ fn equal_length_goes_to_more_work_for_network(network: Network) -> Result<()> { zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; + let dummy_tree = HistoryTree::new_from_block( + network, + block1.clone(), + &sapling::tree::Root::default(), + None, + ) + .unwrap(); let less_work_child = block1.make_fake_child().set_work(1); let more_work_child = block1.make_fake_child().set_work(3); let expected_hash = more_work_child.hash(); let mut state = NonFinalizedState::default(); - state.commit_new_chain(block1.prepare()); - state.commit_block(less_work_child.prepare()); - state.commit_block(more_work_child.prepare()); + state.commit_new_chain(block1.prepare(), dummy_tree.clone())?; + state.commit_block(less_work_child.prepare(), &dummy_tree)?; + state.commit_block(more_work_child.prepare(), &dummy_tree)?; assert_eq!(2, state.chain_set.len()); let tip_hash = state.best_tip().unwrap().1; From c52a66d3442b309585220564580ff51eee0015d5 Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Wed, 23 Jun 2021 15:08:13 -0300 Subject: [PATCH 12/30] Apply suggestions from code review Co-authored-by: teor --- zebra-chain/src/mmr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/mmr.rs index f0c48e654fd..3619acdfa3c 100644 --- a/zebra-chain/src/mmr.rs +++ b/zebra-chain/src/mmr.rs @@ -22,7 +22,7 @@ use crate::{ #[non_exhaustive] #[allow(missing_docs)] pub enum HistoryTreeError { - #[error("error from the underlying library: {inner:?}")] + #[error("zcash_history error: {inner:?}")] #[non_exhaustive] InnerError { inner: zcash_history::Error }, @@ -96,7 +96,7 @@ impl HistoryTree { } /// Extend the history tree with the given blocks. - pub fn extend< + pub fn try_extend< 'a, T: IntoIterator< Item = ( From e977cd0d03d51c47e7a1025ab969f2f41096928e Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Wed, 23 Jun 2021 16:04:02 -0300 Subject: [PATCH 13/30] Apply suggestions from code review Co-authored-by: teor --- zebra-state/src/service/non_finalized_state/chain.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 0836d9bc513..b35bb2e36a2 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -59,7 +59,7 @@ impl Chain { /// Push a contextually valid non-finalized block into a chain as the new tip. #[instrument(level = "debug", skip(self, block), fields(block = %block.block))] - pub fn push(&mut self, block: PreparedBlock) -> Result<(), HistoryTreeError> { + pub fn push(&mut self, block: PreparedBlock) -> Result<(), ValidateContextError> { // update cumulative data members self.update_chain_state_with(&block)?; tracing::debug!(block = %block.block, "adding block to chain"); @@ -102,7 +102,7 @@ impl Chain { &self, fork_tip: block::Hash, finalized_tip_history_tree: &HistoryTree, - ) -> Result, HistoryTreeError> { + ) -> Result, ValidateContextError> { if !self.height_by_hash.contains_key(&fork_tip) { return Ok(None); } @@ -203,7 +203,7 @@ impl Chain { trait UpdateWith { /// Update `Chain` cumulative data members to add data that are derived from /// `T` - fn update_chain_state_with(&mut self, _: &T) -> Result<(), HistoryTreeError>; + fn update_chain_state_with(&mut self, _: &T) -> Result<(), ValidateContextError>; /// Update `Chain` cumulative data members to remove data that are derived /// from `T` From 030240f94d954cf8ab8c4ffd226af7081db5d6a6 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Wed, 23 Jun 2021 16:38:27 -0300 Subject: [PATCH 14/30] improvements from code review --- zebra-chain/src/mmr.rs | 83 +++++++++++++------ zebra-state/src/service.rs | 2 +- zebra-state/src/service/check.rs | 9 +- .../src/service/non_finalized_state.rs | 56 ++++++++----- .../src/service/non_finalized_state/chain.rs | 26 +++--- 5 files changed, 113 insertions(+), 63 deletions(-) diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/mmr.rs index 3619acdfa3c..cfaaf0228c4 100644 --- a/zebra-chain/src/mmr.rs +++ b/zebra-chain/src/mmr.rs @@ -34,16 +34,14 @@ pub enum HistoryTreeError { pub struct HistoryTree { network: Network, network_upgrade: NetworkUpgrade, - // Merkle mountain range tree. - // This is a "runtime" structure used to add / remove nodes, and it's not - // persistent. + /// Merkle mountain range tree. + /// This is a "runtime" structure used to add / remove nodes, and it's not + /// persistent. inner: Tree, - // The number of nodes in the tree. + /// The number of nodes in the tree. size: u32, - // The peaks of the tree, indexed by their position in the array representation - // of the tree. This can be persisted to save the tree. - // TODO: use NodeData instead of Entry to save space? Requires re-deriving - // the entry metadata from its position. + /// The peaks of the tree, indexed by their position in the array representation + /// of the tree. This can be persisted to save the tree. peaks: HashMap, } @@ -89,7 +87,6 @@ impl HistoryTree { self.peaks.insert(self.size, entry); self.size += 1; } - // TODO: should we really prune every iteration? self.prune()?; // TODO: implement network upgrade logic: drop previous history, start new history Ok(()) @@ -118,34 +115,70 @@ impl HistoryTree { /// Prune tree, removing all non-peak entries. fn prune(&mut self) -> Result<(), io::Error> { // Go through all the peaks of the tree. - // This code is from a librustzcash example. + // This code is based on a librustzcash example: // https://github.com/zcash/librustzcash/blob/02052526925fba9389f1428d6df254d4dec967e6/zcash_history/examples/long.rs - // See also coins.cpp in zcashd for a explanation of how it works. + // The explanation of how it works is from zcashd: // https://github.com/zcash/zcash/blob/0247c0c682d59184a717a6536edb0d18834be9a7/src/coins.cpp#L351 - // TODO: should we just copy the explanation here? - let mut peak_pos_set = HashSet::new(); - // integer log2 of (self.size+1), -1 - let mut h = (32 - ((self.size + 1) as u32).leading_zeros() - 1) - 1; - let mut peak_pos = (1 << (h + 1)) - 1; + // Assume the following example peak layout with 14 leaves, and 25 stored nodes in + // total (the "tree length"): + // + // P + // /\ + // / \ + // / \ \ + // / \ \ Altitude + // _A_ \ \ 3 + // _/ \_ B \ 2 + // / \ / \ / \ C 1 + // /\ /\ /\ /\ /\ /\ /\ 0 + // + // We start by determining the altitude of the highest peak (A). + let mut alt = (32 - ((self.size + 1) as u32).leading_zeros() - 1) - 1; + + // We determine the position of the highest peak (A) by pretending it is the right + // sibling in a tree, and its left-most leaf has position 0. Then the left sibling + // of (A) has position -1, and so we can "jump" to the peak's position by computing + // -1 + 2^(alt + 1) - 1. + let mut peak_pos = (1 << (alt + 1)) - 2; + + // Now that we have the position and altitude of the highest peak (A), we collect + // the remaining peaks (B, C). We navigate the peaks as if they were nodes in this + // Merkle tree (with additional imaginary nodes 1 and 2, that have positions beyond + // the MMR's length): + // + // / \ + // / \ + // / \ + // / \ + // A ==========> 1 + // / \ // \ + // _/ \_ B ==> 2 + // /\ /\ /\ // + // / \ / \ / \ C + // /\ /\ /\ /\ /\ /\ /\ + // loop { - if peak_pos > self.size { - // left child, -2^h - peak_pos -= 1 << h; - h -= 1; + // If peak_pos is out of bounds of the tree, we compute the position of its left + // child, and drop down one level in the tree. + if peak_pos >= self.size { + // left child, -2^alt + peak_pos -= 1 << alt; + alt -= 1; } - if peak_pos <= self.size { - // There is a peak at index `peak_pos - 1` - peak_pos_set.insert(peak_pos - 1); + // If the peak exists, we take it and then continue with its right sibling. + if peak_pos < self.size { + // There is a peak at index `peak_pos` + peak_pos_set.insert(peak_pos); // right sibling - peak_pos = peak_pos + (1 << (h + 1)) - 1; + peak_pos = peak_pos + (1 << (alt + 1)) - 1; } - if h == 0 { + if alt == 0 { break; } } diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index cdf7ace8994..cb45ad83a66 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -194,7 +194,7 @@ impl StateService { assert!(relevant_chain.len() >= POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN, "contextual validation requires at least 28 (POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN) blocks"); - check::block_is_contextually_valid( + check::block_is_valid_for_recent_chain( prepared, self.network, self.disk.finalized_tip_height(), diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs index ccb86836af4..7e680e32b55 100644 --- a/zebra-state/src/service/check.rs +++ b/zebra-state/src/service/check.rs @@ -28,11 +28,8 @@ pub(crate) mod difficulty; /// /// If the state contains less than 28 /// (`POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN`) blocks. -#[tracing::instrument( - name = "contextual_validation", - skip(prepared, finalized_tip_height, relevant_chain) -)] -pub(crate) fn block_is_contextually_valid( +#[tracing::instrument(skip(prepared, finalized_tip_height, relevant_chain))] +pub(crate) fn block_is_valid_for_recent_chain( prepared: &PreparedBlock, network: Network, finalized_tip_height: Option, @@ -86,7 +83,7 @@ where } #[tracing::instrument(skip(prepared))] -pub(crate) fn block_is_contextually_valid_for_chain( +pub(crate) fn block_commitment_is_valid_for_chain_history( prepared: &PreparedBlock, network: Network, mmr_hash: &ChainHistoryMmrRootHash, diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 5a9211af6c6..c1d31020680 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -79,6 +79,9 @@ impl NonFinalizedState { } /// Commit block to the non-finalized state. + /// + /// `finalized_tip_history_tree`: the history tree of the finalized tip used to recompute + /// the history tree, if needed. pub fn commit_block( &mut self, prepared: PreparedBlock, @@ -94,24 +97,8 @@ impl NonFinalizedState { ); } - let mut parent_chain = - match self.take_chain_if(|chain| chain.non_finalized_tip_hash() == parent_hash) { - Some(chain) => chain, - None => Box::new( - self.chain_set - .iter() - .find_map(|chain| { - chain - .fork(parent_hash, finalized_tip_history_tree) - .transpose() - }) - .expect( - "commit_block is only called with blocks that are ready to be commited", - )?, - ), - }; - - check::block_is_contextually_valid_for_chain( + let mut parent_chain = self.parent_chain(parent_hash, finalized_tip_history_tree)?; + check::block_commitment_is_valid_for_chain_history( &prepared, self.network, &parent_chain.history_root_hash(), @@ -133,7 +120,7 @@ impl NonFinalizedState { ) -> Result<(), ValidateContextError> { let mut chain = Chain::new(history_tree); let (height, hash) = (prepared.height, prepared.hash); - check::block_is_contextually_valid_for_chain( + check::block_commitment_is_valid_for_chain_history( &prepared, self.network, &chain.history_root_hash(), @@ -279,6 +266,37 @@ impl NonFinalizedState { .map(|box_chain| box_chain.deref()) } + /// Return the chain whose tip block hash is `parent_hash`. + /// + /// The chain can be an existing chain in the non-finalized state or a freshly + /// created fork, if needed. + /// + /// `finalized_tip_history_tree`: the history tree of the finalized tip used to recompute + /// the history tree, if needed. + fn parent_chain( + &mut self, + parent_hash: block::Hash, + finalized_tip_history_tree: &HistoryTree, + ) -> Result, ValidateContextError> { + match self.take_chain_if(|chain| chain.non_finalized_tip_hash() == parent_hash) { + // An existing chain in the non-finalized state + Some(chain) => Ok(chain), + // Create a new fork + None => Ok(Box::new( + self.chain_set + .iter() + .find_map(|chain| { + chain + .fork(parent_hash, finalized_tip_history_tree) + .transpose() + }) + .expect( + "commit_block is only called with blocks that are ready to be commited", + )?, + )), + } + } + /// Update the metrics after `block` is committed fn update_metrics_for_committed_block(&self, height: block::Height, hash: block::Hash) { metrics::counter!("state.memory.committed.block.count", 1); diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index b35bb2e36a2..fda24c43dbe 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -7,7 +7,7 @@ use std::{ use tracing::{debug_span, instrument, trace}; use zebra_chain::{ block::{self, ChainHistoryMmrRootHash}, - mmr::{HistoryTree, HistoryTreeError}, + mmr::HistoryTree, orchard, primitives::Groth16Proof, sapling, sprout, transaction, @@ -16,7 +16,7 @@ use zebra_chain::{ work::difficulty::PartialCumulativeWork, }; -use crate::{PreparedBlock, Utxo}; +use crate::{PreparedBlock, Utxo, ValidateContextError}; // #[derive(Clone)] pub struct Chain { @@ -116,9 +116,10 @@ impl Chain { // Rebuild the history tree starting from the finalized tip tree. // TODO: change to a more efficient approach by removing nodes // from the tree of the original chain (in `pop_tip()`). + // See https://github.com/ZcashFoundation/zebra/issues/2378 forked .history_tree - .extend(forked.blocks.values().map(|prepared_block| { + .try_extend(forked.blocks.values().map(|prepared_block| { ( prepared_block.block.clone(), // TODO: pass Sapling and Orchard roots @@ -214,7 +215,7 @@ impl UpdateWith for Chain { fn update_chain_state_with( &mut self, prepared: &PreparedBlock, - ) -> Result<(), HistoryTreeError> { + ) -> Result<(), ValidateContextError> { let (block, hash, height, transaction_hashes) = ( prepared.block.as_ref(), prepared.hash, @@ -324,9 +325,10 @@ impl UpdateWith for Chain { .expect("work has already been validated"); self.partial_cumulative_work -= block_work; - // Note: the MMR is not reverted here. - // When popping the root: there is no need to change the MMR. - // When popping the tip: the MMR is rebuilt in fork(). + // Note: the history tree is not modified in this method. + // This method is called on two scenarios: + // - When popping the root: the history tree does not change. + // - When popping the tip: the history tree is rebuilt in fork(). // for each transaction in block for (transaction, transaction_hash) in @@ -386,7 +388,7 @@ impl UpdateWith> for Chain { fn update_chain_state_with( &mut self, utxos: &HashMap, - ) -> Result<(), HistoryTreeError> { + ) -> Result<(), ValidateContextError> { self.created_utxos .extend(utxos.iter().map(|(k, v)| (*k, v.clone()))); Ok(()) @@ -402,7 +404,7 @@ impl UpdateWith> for Chain { fn update_chain_state_with( &mut self, inputs: &Vec, - ) -> Result<(), HistoryTreeError> { + ) -> Result<(), ValidateContextError> { for consumed_utxo in inputs { match consumed_utxo { transparent::Input::PrevOut { outpoint, .. } => { @@ -434,7 +436,7 @@ impl UpdateWith>> for Chain { fn update_chain_state_with( &mut self, joinsplit_data: &Option>, - ) -> Result<(), HistoryTreeError> { + ) -> Result<(), ValidateContextError> { if let Some(joinsplit_data) = joinsplit_data { for sprout::JoinSplit { nullifiers, .. } in joinsplit_data.joinsplits() { let span = debug_span!("revert_chain_state_with", ?nullifiers); @@ -477,7 +479,7 @@ where fn update_chain_state_with( &mut self, sapling_shielded_data: &Option>, - ) -> Result<(), HistoryTreeError> { + ) -> Result<(), ValidateContextError> { if let Some(sapling_shielded_data) = sapling_shielded_data { for nullifier in sapling_shielded_data.nullifiers() { self.sapling_nullifiers.insert(*nullifier); @@ -505,7 +507,7 @@ impl UpdateWith> for Chain { fn update_chain_state_with( &mut self, orchard_shielded_data: &Option, - ) -> Result<(), HistoryTreeError> { + ) -> Result<(), ValidateContextError> { if let Some(orchard_shielded_data) = orchard_shielded_data { for nullifier in orchard_shielded_data.nullifiers() { self.orchard_nullifiers.insert(*nullifier); From e416ef687e47556540d7755b04588561b4f1178b Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Wed, 23 Jun 2021 16:50:59 -0300 Subject: [PATCH 15/30] improve check.rs comments and variable names --- zebra-state/src/service/check.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs index 7e680e32b55..6ce8955a73a 100644 --- a/zebra-state/src/service/check.rs +++ b/zebra-state/src/service/check.rs @@ -18,8 +18,11 @@ use difficulty::{AdjustedDifficulty, POW_MEDIAN_BLOCK_SPAN}; pub(crate) mod difficulty; -/// Check that `block` is contextually valid for `network`, based on the -/// `finalized_tip_height` and `relevant_chain`. +/// Check that the `prepared` block is contextually valid for `network`, based +/// on the `finalized_tip_height` and `relevant_chain`. +/// +/// This function performs checks that require a small number of recent blocks, +/// including previous hash, previous height, and block difficulty. /// /// The relevant chain is an iterator over the ancestors of `block`, starting /// with its parent block. @@ -82,11 +85,14 @@ where Ok(()) } +/// Check that the `prepared` block is contextually valid for `network`, based +/// on the `history_root_hash` of the history tree up to and including the +/// previous block. #[tracing::instrument(skip(prepared))] pub(crate) fn block_commitment_is_valid_for_chain_history( prepared: &PreparedBlock, network: Network, - mmr_hash: &ChainHistoryMmrRootHash, + history_root_hash: &ChainHistoryMmrRootHash, ) -> Result<(), ValidateContextError> { match prepared.block.commitment(network)? { block::Commitment::PreSaplingReserved(_) @@ -95,13 +101,13 @@ pub(crate) fn block_commitment_is_valid_for_chain_history( // No contextual checks needed for those. Ok(()) } - block::Commitment::ChainHistoryRoot(block_mmr_hash) => { - if block_mmr_hash == *mmr_hash { + block::Commitment::ChainHistoryRoot(block_history_root_hash) => { + if block_history_root_hash == *history_root_hash { Ok(()) } else { Err(ValidateContextError::InvalidHistoryCommitment { - candidate_commitment: block_mmr_hash, - expected_commitment: *mmr_hash, + candidate_commitment: block_history_root_hash, + expected_commitment: *history_root_hash, }) } } From 23e9bc2539ddba7fea898b514e5d802435faf2d3 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Thu, 24 Jun 2021 16:59:36 -0300 Subject: [PATCH 16/30] fix HistoryTree which should use BTreeMap and not HashMap; fix non_finalized_state prop tests --- zebra-chain/src/mmr.rs | 8 ++--- zebra-chain/src/primitives/zcash_history.rs | 10 +++---- .../service/non_finalized_state/arbitrary.rs | 25 ++++++++++++++-- .../src/service/non_finalized_state/chain.rs | 2 +- .../service/non_finalized_state/tests/prop.rs | 30 ++++++++++++------- 5 files changed, 52 insertions(+), 23 deletions(-) diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/mmr.rs index cfaaf0228c4..be9785dda47 100644 --- a/zebra-chain/src/mmr.rs +++ b/zebra-chain/src/mmr.rs @@ -2,7 +2,7 @@ //! the block history as specified in ZIP-221. use std::{ - collections::{HashMap, HashSet}, + collections::{BTreeMap, HashSet}, io, sync::Arc, }; @@ -42,7 +42,7 @@ pub struct HistoryTree { size: u32, /// The peaks of the tree, indexed by their position in the array representation /// of the tree. This can be persisted to save the tree. - peaks: HashMap, + peaks: BTreeMap, } impl HistoryTree { @@ -59,7 +59,7 @@ impl HistoryTree { let network_upgrade = NetworkUpgrade::current(network, height); // TODO: handle Orchard root let (tree, entry) = Tree::new_from_block(network, block, sapling_root)?; - let mut peaks = HashMap::new(); + let mut peaks = BTreeMap::new(); peaks.insert(0u32, entry); Ok(HistoryTree { network, @@ -209,7 +209,7 @@ impl Clone for HistoryTree { self.network_upgrade, self.size, &self.peaks, - &HashMap::new(), + &Default::default(), ) .expect("rebuilding an existing tree should always work"); HistoryTree { diff --git a/zebra-chain/src/primitives/zcash_history.rs b/zebra-chain/src/primitives/zcash_history.rs index 4973c8b8468..b1e3780f131 100644 --- a/zebra-chain/src/primitives/zcash_history.rs +++ b/zebra-chain/src/primitives/zcash_history.rs @@ -6,7 +6,7 @@ mod tests; -use std::{collections::HashMap, convert::TryInto, io, sync::Arc}; +use std::{collections::BTreeMap, convert::TryInto, io, sync::Arc}; use crate::{ block::{Block, ChainHistoryMmrRootHash}, @@ -106,8 +106,8 @@ impl Tree { network: Network, network_upgrade: NetworkUpgrade, length: u32, - peaks: &HashMap, - extra: &HashMap, + peaks: &BTreeMap, + extra: &BTreeMap, ) -> Result { let branch_id = network_upgrade .branch_id() @@ -143,10 +143,10 @@ impl Tree { .expect("block must have coinbase height during contextual verification"); let network_upgrade = NetworkUpgrade::current(network, height); let entry0 = Entry::new_leaf(block, network, sapling_root); - let mut peaks = HashMap::new(); + let mut peaks = BTreeMap::new(); peaks.insert(0u32, entry0); Ok(( - Tree::new_from_cache(network, network_upgrade, 1, &peaks, &HashMap::new())?, + Tree::new_from_cache(network, network_upgrade, 1, &peaks, &BTreeMap::new())?, peaks .remove(&0u32) .expect("must work since it was just added"), diff --git a/zebra-state/src/service/non_finalized_state/arbitrary.rs b/zebra-state/src/service/non_finalized_state/arbitrary.rs index 22f068761e9..ff25ede7af0 100644 --- a/zebra-state/src/service/non_finalized_state/arbitrary.rs +++ b/zebra-state/src/service/non_finalized_state/arbitrary.rs @@ -5,7 +5,12 @@ use proptest::{ }; use std::sync::Arc; -use zebra_chain::{block::Block, fmt::SummaryDebug, parameters::NetworkUpgrade::Nu5, LedgerState}; +use zebra_chain::{ + block::Block, + fmt::SummaryDebug, + parameters::NetworkUpgrade::{Heartwood, Nu5}, + LedgerState, +}; use zebra_test::prelude::*; use crate::tests::Prepare; @@ -55,7 +60,19 @@ impl Strategy for PreparedChain { let mut chain = self.chain.lock().unwrap(); if chain.is_none() { // TODO: use the latest network upgrade (#1974) - let ledger_strategy = LedgerState::genesis_strategy(Nu5, None, false); + + // The history tree only works with Heartword onward. + // Since the network will be chosen later, we pick the larger + // between the mainnet and testnet Heartwood activation heights. + let main_height = Heartwood + .activation_height(Network::Mainnet) + .expect("must have height"); + let test_height = Heartwood + .activation_height(Network::Testnet) + .expect("must have height"); + let height = (std::cmp::max(main_height, test_height) + 1).expect("must be valid"); + + let ledger_strategy = LedgerState::height_strategy(height, Nu5, None, false); let (network, blocks) = ledger_strategy .prop_flat_map(|ledger| { @@ -78,7 +95,9 @@ impl Strategy for PreparedChain { } let chain = chain.clone().expect("should be generated"); - let count = (1..chain.1.len()).new_tree(runner)?; + // `count` must be 1 less since the first block is used to build the + // history tree. + let count = (1..chain.1.len() - 1).new_tree(runner)?; Ok(PreparedChainTree { chain: chain.1, count, diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index fda24c43dbe..2e6413780e5 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -33,7 +33,7 @@ pub struct Chain { sapling_nullifiers: HashSet, orchard_nullifiers: HashSet, partial_cumulative_work: PartialCumulativeWork, - history_tree: HistoryTree, + pub(crate) history_tree: HistoryTree, } impl Chain { diff --git a/zebra-state/src/service/non_finalized_state/tests/prop.rs b/zebra-state/src/service/non_finalized_state/tests/prop.rs index 40f312b409e..32f8a97c531 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -16,10 +16,13 @@ fn forked_equals_pushed() -> Result<()> { .and_then(|v| v.parse().ok()) .unwrap_or(DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES)), |((chain, count, network) in PreparedChain::default())| { - let dummy_tree = HistoryTree::new_from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); + // Build a history tree with the first block to simulate the tree of + // the finalized state. + let finalized_tree = HistoryTree::new_from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); + let chain = &chain[1..]; let fork_tip_hash = chain[count - 1].hash; - let mut full_chain = Chain::new(dummy_tree.clone()); - let mut partial_chain = Chain::new(dummy_tree.clone()); + let mut full_chain = Chain::new(finalized_tree.clone()); + let mut partial_chain = Chain::new(finalized_tree.clone()); for block in chain.iter().take(count) { partial_chain.push(block.clone())?; @@ -28,7 +31,7 @@ fn forked_equals_pushed() -> Result<()> { full_chain.push(block.clone())?; } - let forked = full_chain.fork(fork_tip_hash, &dummy_tree).expect("fork must work").expect("hash is present"); + let forked = full_chain.fork(fork_tip_hash, &finalized_tree).expect("fork must work").expect("hash is present"); prop_assert_eq!(forked.blocks.len(), partial_chain.blocks.len()); }); @@ -45,18 +48,25 @@ fn finalized_equals_pushed() -> Result<()> { .and_then(|v| v.parse().ok()) .unwrap_or(DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES)), |((chain, end_count, network) in PreparedChain::default())| { - let dummy_tree = HistoryTree::new_from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); + // Build a history tree with the first block to simulate the tree of + // the finalized state. + let finalized_tree = HistoryTree::new_from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); + let chain = &chain[1..]; let finalized_count = chain.len() - end_count; - let mut full_chain = Chain::new(dummy_tree.clone()); - let mut partial_chain = Chain::new(dummy_tree); + let mut full_chain = Chain::new(finalized_tree.clone()); - for block in chain.iter().skip(finalized_count) { - partial_chain.push(block.clone())?; + for block in chain.iter().take(finalized_count) { + full_chain.push(block.clone())?; } - for block in chain.iter() { + let mut partial_chain = Chain::new(full_chain.history_tree.clone()); + for block in chain.iter().skip(finalized_count) { full_chain.push(block.clone())?; } + for block in chain.iter().skip(finalized_count) { + partial_chain.push(block.clone())?; + } + for _ in 0..finalized_count { let _finalized = full_chain.pop_root(); } From cf339a3e78d28e6c725030e27cf86aa5ecffb86f Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Fri, 25 Jun 2021 14:01:05 -0300 Subject: [PATCH 17/30] fix finalized_state proptest --- .../service/non_finalized_state/arbitrary.rs | 42 ++++++++++++------- .../service/non_finalized_state/tests/prop.rs | 6 +-- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/zebra-state/src/service/non_finalized_state/arbitrary.rs b/zebra-state/src/service/non_finalized_state/arbitrary.rs index ff25ede7af0..72daa4391df 100644 --- a/zebra-state/src/service/non_finalized_state/arbitrary.rs +++ b/zebra-state/src/service/non_finalized_state/arbitrary.rs @@ -6,7 +6,7 @@ use proptest::{ use std::sync::Arc; use zebra_chain::{ - block::Block, + block::{Block, Height}, fmt::SummaryDebug, parameters::NetworkUpgrade::{Heartwood, Nu5}, LedgerState, @@ -50,6 +50,29 @@ impl ValueTree for PreparedChainTree { pub struct PreparedChain { // the proptests are threaded (not async), so we want to use a threaded mutex here chain: std::sync::Mutex>>)>>, + // the height from which to start the chain. If None, starts at the genesis block + start_height: Option, +} + +impl PreparedChain { + /// Create a PreparedChain strategy with Heartwood-onward blocks. + pub(super) fn new_heartwood() -> Self { + // The history tree only works with Heartwood onward. + // Since the network will be chosen later, we pick the larger + // between the mainnet and testnet Heartwood activation heights. + let main_height = Heartwood + .activation_height(Network::Mainnet) + .expect("must have height"); + let test_height = Heartwood + .activation_height(Network::Testnet) + .expect("must have height"); + let height = (std::cmp::max(main_height, test_height) + 1).expect("must be valid"); + + PreparedChain { + start_height: Some(height), + ..Default::default() + } + } } impl Strategy for PreparedChain { @@ -60,19 +83,10 @@ impl Strategy for PreparedChain { let mut chain = self.chain.lock().unwrap(); if chain.is_none() { // TODO: use the latest network upgrade (#1974) - - // The history tree only works with Heartword onward. - // Since the network will be chosen later, we pick the larger - // between the mainnet and testnet Heartwood activation heights. - let main_height = Heartwood - .activation_height(Network::Mainnet) - .expect("must have height"); - let test_height = Heartwood - .activation_height(Network::Testnet) - .expect("must have height"); - let height = (std::cmp::max(main_height, test_height) + 1).expect("must be valid"); - - let ledger_strategy = LedgerState::height_strategy(height, Nu5, None, false); + let ledger_strategy = match self.start_height { + Some(start_height) => LedgerState::height_strategy(start_height, Nu5, None, false), + None => LedgerState::genesis_strategy(Nu5, None, false), + }; let (network, blocks) = ledger_strategy .prop_flat_map(|ledger| { diff --git a/zebra-state/src/service/non_finalized_state/tests/prop.rs b/zebra-state/src/service/non_finalized_state/tests/prop.rs index 32f8a97c531..f1bd82d3492 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -15,7 +15,7 @@ fn forked_equals_pushed() -> Result<()> { .ok() .and_then(|v| v.parse().ok()) .unwrap_or(DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES)), - |((chain, count, network) in PreparedChain::default())| { + |((chain, count, network) in PreparedChain::new_heartwood())| { // Build a history tree with the first block to simulate the tree of // the finalized state. let finalized_tree = HistoryTree::new_from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); @@ -47,13 +47,13 @@ fn finalized_equals_pushed() -> Result<()> { .ok() .and_then(|v| v.parse().ok()) .unwrap_or(DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES)), - |((chain, end_count, network) in PreparedChain::default())| { + |((chain, end_count, network) in PreparedChain::new_heartwood())| { // Build a history tree with the first block to simulate the tree of // the finalized state. let finalized_tree = HistoryTree::new_from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); let chain = &chain[1..]; let finalized_count = chain.len() - end_count; - let mut full_chain = Chain::new(finalized_tree.clone()); + let mut full_chain = Chain::new(finalized_tree); for block in chain.iter().take(finalized_count) { full_chain.push(block.clone())?; From 2271d73418512314fc66acc19bac69957af7062a Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Fri, 25 Jun 2021 17:06:01 -0300 Subject: [PATCH 18/30] fix non_finalized_state tests by setting the correct commitments --- zebra-chain/src/block/commitment.rs | 6 + zebra-chain/src/mmr.rs | 24 +- zebra-chain/src/primitives/zcash_history.rs | 2 +- .../src/service/non_finalized_state.rs | 4 +- .../non_finalized_state/tests/vectors.rs | 323 +++++++++++------- zebra-state/src/tests.rs | 8 + 6 files changed, 233 insertions(+), 134 deletions(-) diff --git a/zebra-chain/src/block/commitment.rs b/zebra-chain/src/block/commitment.rs index 9b5876f0c9b..69d1cfd0c9e 100644 --- a/zebra-chain/src/block/commitment.rs +++ b/zebra-chain/src/block/commitment.rs @@ -149,6 +149,12 @@ impl From<[u8; 32]> for ChainHistoryMmrRootHash { } } +impl From for [u8; 32] { + fn from(hash: ChainHistoryMmrRootHash) -> Self { + hash.0 + } +} + /// A block commitment to chain history and transaction auth. /// - the chain history tree for all ancestors in the current network upgrade, /// and diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/mmr.rs index be9785dda47..f16ba05b3c6 100644 --- a/zebra-chain/src/mmr.rs +++ b/zebra-chain/src/mmr.rs @@ -10,7 +10,7 @@ use std::{ use thiserror::Error; use crate::{ - block::{Block, ChainHistoryMmrRootHash}, + block::{Block, ChainHistoryMmrRootHash, Height}, orchard, parameters::{Network, NetworkUpgrade}, primitives::zcash_history::{Entry, Tree}, @@ -43,6 +43,8 @@ pub struct HistoryTree { /// The peaks of the tree, indexed by their position in the array representation /// of the tree. This can be persisted to save the tree. peaks: BTreeMap, + /// The height of the most recent block added to the tree. + current_height: Height, } impl HistoryTree { @@ -67,16 +69,34 @@ impl HistoryTree { inner: tree, size: 1, peaks, + current_height: height, }) } /// Add block data to the tree. + /// + /// # Panics + /// + /// If the block height is not one more than the previously pushed block. pub fn push( &mut self, block: Arc, sapling_root: &sapling::tree::Root, _orchard_root: Option<&orchard::tree::Root>, ) -> Result<(), HistoryTreeError> { + // Check if the block has the expected height. + // librustzcash assumes the heights are correct and corrupts the tree if they are wrong, + // resulting in a confusing error, which we prevent here. + let height = block + .coinbase_height() + .expect("block must have coinbase height during contextual verification"); + if height - self.current_height != 1 { + panic!( + "added block with height {:?} but it must be {:?}+1", + height, self.current_height + ); + } + // TODO: handle orchard root let new_entries = self .inner @@ -89,6 +109,7 @@ impl HistoryTree { } self.prune()?; // TODO: implement network upgrade logic: drop previous history, start new history + self.current_height = height; Ok(()) } @@ -218,6 +239,7 @@ impl Clone for HistoryTree { inner: tree, size: self.size, peaks: self.peaks.clone(), + current_height: self.current_height, } } } diff --git a/zebra-chain/src/primitives/zcash_history.rs b/zebra-chain/src/primitives/zcash_history.rs index b1e3780f131..f7835446443 100644 --- a/zebra-chain/src/primitives/zcash_history.rs +++ b/zebra-chain/src/primitives/zcash_history.rs @@ -174,7 +174,7 @@ impl Tree { let network_upgrade = NetworkUpgrade::current(self.network, height); if self.network_upgrade != network_upgrade { panic!( - "added block from network upgrade {:?} but MMR tree is restricted to {:?}", + "added block from network upgrade {:?} but history tree is restricted to {:?}", network_upgrade, self.network_upgrade ); } diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index c1d31020680..6fc08f60f9d 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -116,9 +116,9 @@ impl NonFinalizedState { pub fn commit_new_chain( &mut self, prepared: PreparedBlock, - history_tree: HistoryTree, + finalized_tip_history_tree: HistoryTree, ) -> Result<(), ValidateContextError> { - let mut chain = Chain::new(history_tree); + let mut chain = Chain::new(finalized_tip_history_tree); let (height, hash) = (prepared.height, prepared.hash); check::block_commitment_is_valid_for_chain_history( &prepared, diff --git a/zebra-state/src/service/non_finalized_state/tests/vectors.rs b/zebra-state/src/service/non_finalized_state/tests/vectors.rs index ece1833a27d..0680f9a7fb9 100644 --- a/zebra-state/src/service/non_finalized_state/tests/vectors.rs +++ b/zebra-state/src/service/non_finalized_state/tests/vectors.rs @@ -1,7 +1,9 @@ use std::sync::Arc; use zebra_chain::{ - block::Block, mmr::HistoryTree, parameters::Network, sapling, + block::Block, + mmr::{HistoryTree, HistoryTreeError}, + parameters::Network, serialization::ZcashDeserializeInto, }; use zebra_test::prelude::*; @@ -13,22 +15,32 @@ use crate::{ use self::assert_eq; +/// Make a history tree for the given block givens the history tree of its parent. +fn make_tree( + block: Arc, + parent_tree: &HistoryTree, +) -> Result { + let mut tree = parent_tree.clone(); + tree.push(block, &Default::default(), None)?; + Ok(tree) +} + #[test] fn construct_single() -> Result<()> { zebra_test::init(); - let block: Arc = + let block0: Arc = zebra_test::vectors::BLOCK_MAINNET_434873_BYTES.zcash_deserialize_into()?; - let dummy_tree = HistoryTree::new_from_block( - Network::Mainnet, - block.clone(), - &sapling::tree::Root::default(), - None, - ) - .unwrap(); + let finalized_tree = + HistoryTree::new_from_block(Network::Mainnet, block0.clone(), &Default::default(), None) + .unwrap(); - let mut chain = Chain::new(dummy_tree); - chain.push(block.prepare())?; + let block1 = block0 + .make_fake_child() + .set_commitment(finalized_tree.hash().into()); + + let mut chain = Chain::new(finalized_tree); + chain.push(block1.prepare())?; assert_eq!(1, chain.blocks.len()); @@ -41,22 +53,20 @@ fn construct_many() -> Result<()> { let mut block: Arc = zebra_test::vectors::BLOCK_MAINNET_434873_BYTES.zcash_deserialize_into()?; + let finalized_tree = + HistoryTree::new_from_block(Network::Mainnet, block.clone(), &Default::default(), None) + .unwrap(); let mut blocks = vec![]; + let mut tree = finalized_tree.clone(); while blocks.len() < 100 { - let next_block = block.make_fake_child(); - blocks.push(block); + let next_block = block.make_fake_child().set_commitment(tree.hash().into()); + blocks.push(next_block.clone()); block = next_block; + tree = make_tree(block.clone(), &tree)?; } - let dummy_tree = HistoryTree::new_from_block( - Network::Mainnet, - block, - &sapling::tree::Root::default(), - None, - ) - .unwrap(); - let mut chain = Chain::new(dummy_tree); + let mut chain = Chain::new(finalized_tree); for block in blocks { chain.push(block.prepare())?; @@ -70,23 +80,25 @@ fn construct_many() -> Result<()> { #[test] fn ord_matches_work() -> Result<()> { zebra_test::init(); - let less_block = zebra_test::vectors::BLOCK_MAINNET_434873_BYTES - .zcash_deserialize_into::>()? - .set_work(1); - let more_block = less_block.clone().set_work(10); - - let dummy_tree = HistoryTree::new_from_block( - Network::Mainnet, - less_block.clone(), - &sapling::tree::Root::default(), - None, - ) - .unwrap(); - - let mut lesser_chain = Chain::new(dummy_tree.clone()); + let block = + zebra_test::vectors::BLOCK_MAINNET_434873_BYTES.zcash_deserialize_into::>()?; + let finalized_tree = + HistoryTree::new_from_block(Network::Mainnet, block.clone(), &Default::default(), None) + .unwrap(); + + let less_block = block + .make_fake_child() + .set_work(1) + .set_commitment(finalized_tree.hash().into()); + let more_block = block + .make_fake_child() + .set_work(10) + .set_commitment(finalized_tree.hash().into()); + + let mut lesser_chain = Chain::new(finalized_tree.clone()); lesser_chain.push(less_block.prepare())?; - let mut bigger_chain = Chain::new(dummy_tree); + let mut bigger_chain = Chain::new(finalized_tree); bigger_chain.push(more_block.prepare())?; assert!(bigger_chain > lesser_chain); @@ -113,22 +125,23 @@ fn best_chain_wins_for_network(network: Network) -> Result<()> { zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; - let dummy_tree = HistoryTree::new_from_block( - network, - block1.clone(), - &sapling::tree::Root::default(), - None, - ) - .unwrap(); - - let block2 = block1.make_fake_child().set_work(10); - let child = block1.make_fake_child().set_work(1); + let finalized_tree = + HistoryTree::new_from_block(network, block1.clone(), &Default::default(), None).unwrap(); + + let block2 = block1 + .make_fake_child() + .set_work(10) + .set_commitment(finalized_tree.hash().into()); + let child = block1 + .make_fake_child() + .set_work(1) + .set_commitment(finalized_tree.hash().into()); let expected_hash = block2.hash(); let mut state = NonFinalizedState::default(); - state.commit_new_chain(block2.prepare(), dummy_tree.clone())?; - state.commit_new_chain(child.prepare(), dummy_tree)?; + state.commit_new_chain(block2.prepare(), finalized_tree.clone())?; + state.commit_new_chain(child.prepare(), finalized_tree)?; let best_chain = state.best_chain().unwrap(); assert!(best_chain.height_by_hash.contains_key(&expected_hash)); @@ -147,7 +160,7 @@ fn finalize_pops_from_best_chain() -> Result<()> { } fn finalize_pops_from_best_chain_for_network(network: Network) -> Result<()> { - let block1: Arc = match network { + let block0: Arc = match network { Network::Mainnet => { zebra_test::vectors::BLOCK_MAINNET_1180900_BYTES.zcash_deserialize_into()? } @@ -155,21 +168,27 @@ fn finalize_pops_from_best_chain_for_network(network: Network) -> Result<()> { zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; - let dummy_tree = HistoryTree::new_from_block( - network, - block1.clone(), - &sapling::tree::Root::default(), - None, - ) - .unwrap(); - - let block2 = block1.make_fake_child().set_work(10); - let child = block1.make_fake_child().set_work(1); + let finalized_tree = + HistoryTree::new_from_block(network, block0.clone(), &Default::default(), None).unwrap(); + + let block1 = block0 + .make_fake_child() + .set_commitment(finalized_tree.hash().into()); + let block1_tree = make_tree(block1.clone(), &finalized_tree)?; + + let block2 = block1 + .make_fake_child() + .set_work(10) + .set_commitment(block1_tree.hash().into()); + let child = block1 + .make_fake_child() + .set_work(1) + .set_commitment(block1_tree.hash().into()); let mut state = NonFinalizedState::default(); - state.commit_new_chain(block1.clone().prepare(), dummy_tree.clone())?; - state.commit_block(block2.clone().prepare(), &dummy_tree)?; - state.commit_block(child.prepare(), &dummy_tree)?; + state.commit_new_chain(block1.clone().prepare(), finalized_tree.clone())?; + state.commit_block(block2.clone().prepare(), &finalized_tree)?; + state.commit_block(child.prepare(), &finalized_tree)?; let finalized = state.finalize(); assert_eq!(block1, finalized.block); @@ -196,7 +215,7 @@ fn commit_block_extending_best_chain_doesnt_drop_worst_chains() -> Result<()> { fn commit_block_extending_best_chain_doesnt_drop_worst_chains_for_network( network: Network, ) -> Result<()> { - let block1: Arc = match network { + let block0: Arc = match network { Network::Mainnet => { zebra_test::vectors::BLOCK_MAINNET_1180900_BYTES.zcash_deserialize_into()? } @@ -204,27 +223,37 @@ fn commit_block_extending_best_chain_doesnt_drop_worst_chains_for_network( zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; - let dummy_tree = HistoryTree::new_from_block( - network, - block1.clone(), - &sapling::tree::Root::default(), - None, - ) - .unwrap(); - - let block2 = block1.make_fake_child().set_work(10); - let child1 = block1.make_fake_child().set_work(1); - let child2 = block2.make_fake_child().set_work(1); + let finalized_tree = + HistoryTree::new_from_block(network, block0.clone(), &Default::default(), None).unwrap(); + + let block1 = block0 + .make_fake_child() + .set_commitment(finalized_tree.hash().into()); + let block1_tree = make_tree(block1.clone(), &finalized_tree)?; + + let block2 = block1 + .make_fake_child() + .set_work(10) + .set_commitment(block1_tree.hash().into()); + let block2_tree = make_tree(block2.clone(), &block1_tree)?; + let child1 = block1 + .make_fake_child() + .set_work(1) + .set_commitment(block1_tree.hash().into()); + let child2 = block2 + .make_fake_child() + .set_work(1) + .set_commitment(block2_tree.hash().into()); let mut state = NonFinalizedState::default(); assert_eq!(0, state.chain_set.len()); - state.commit_new_chain(block1.prepare(), dummy_tree.clone())?; + state.commit_new_chain(block1.prepare(), finalized_tree.clone())?; assert_eq!(1, state.chain_set.len()); - state.commit_block(block2.prepare(), &dummy_tree)?; + state.commit_block(block2.prepare(), &finalized_tree)?; assert_eq!(1, state.chain_set.len()); - state.commit_block(child1.prepare(), &dummy_tree)?; + state.commit_block(child1.prepare(), &finalized_tree)?; assert_eq!(2, state.chain_set.len()); - state.commit_block(child2.prepare(), &dummy_tree)?; + state.commit_block(child2.prepare(), &finalized_tree)?; assert_eq!(2, state.chain_set.len()); Ok(()) @@ -241,7 +270,7 @@ fn shorter_chain_can_be_best_chain() -> Result<()> { } fn shorter_chain_can_be_best_chain_for_network(network: Network) -> Result<()> { - let block1: Arc = match network { + let block0: Arc = match network { Network::Mainnet => { zebra_test::vectors::BLOCK_MAINNET_1180900_BYTES.zcash_deserialize_into()? } @@ -249,24 +278,34 @@ fn shorter_chain_can_be_best_chain_for_network(network: Network) -> Result<()> { zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; - let dummy_tree = HistoryTree::new_from_block( - network, - block1.clone(), - &sapling::tree::Root::default(), - None, - ) - .unwrap(); - - let long_chain_block1 = block1.make_fake_child().set_work(1); - let long_chain_block2 = long_chain_block1.make_fake_child().set_work(1); - - let short_chain_block = block1.make_fake_child().set_work(3); + let finalized_tree = + HistoryTree::new_from_block(network, block0.clone(), &Default::default(), None).unwrap(); + + let block1 = block0 + .make_fake_child() + .set_commitment(finalized_tree.hash().into()); + let block1_tree = make_tree(block1.clone(), &finalized_tree)?; + + let long_chain_block1 = block1 + .make_fake_child() + .set_work(1) + .set_commitment(block1_tree.hash().into()); + let long_chain_block1_tree = make_tree(long_chain_block1.clone(), &block1_tree)?; + let long_chain_block2 = long_chain_block1 + .make_fake_child() + .set_work(1) + .set_commitment(long_chain_block1_tree.hash().into()); + + let short_chain_block = block1 + .make_fake_child() + .set_work(3) + .set_commitment(block1_tree.hash().into()); let mut state = NonFinalizedState::default(); - state.commit_new_chain(block1.prepare(), dummy_tree.clone())?; - state.commit_block(long_chain_block1.prepare(), &dummy_tree)?; - state.commit_block(long_chain_block2.prepare(), &dummy_tree)?; - state.commit_block(short_chain_block.prepare(), &dummy_tree)?; + state.commit_new_chain(block1.prepare(), finalized_tree.clone())?; + state.commit_block(long_chain_block1.prepare(), &finalized_tree)?; + state.commit_block(long_chain_block2.prepare(), &finalized_tree)?; + state.commit_block(short_chain_block.prepare(), &finalized_tree)?; assert_eq!(2, state.chain_set.len()); assert_eq!(2, state.best_chain_len()); @@ -285,7 +324,7 @@ fn longer_chain_with_more_work_wins() -> Result<()> { } fn longer_chain_with_more_work_wins_for_network(network: Network) -> Result<()> { - let block1: Arc = match network { + let block0: Arc = match network { Network::Mainnet => { zebra_test::vectors::BLOCK_MAINNET_1180900_BYTES.zcash_deserialize_into()? } @@ -293,28 +332,46 @@ fn longer_chain_with_more_work_wins_for_network(network: Network) -> Result<()> zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; - let dummy_tree = HistoryTree::new_from_block( - network, - block1.clone(), - &sapling::tree::Root::default(), - None, - ) - .unwrap(); - - let long_chain_block1 = block1.make_fake_child().set_work(1); - let long_chain_block2 = long_chain_block1.make_fake_child().set_work(1); - let long_chain_block3 = long_chain_block2.make_fake_child().set_work(1); - let long_chain_block4 = long_chain_block3.make_fake_child().set_work(1); - - let short_chain_block = block1.make_fake_child().set_work(3); + let finalized_tree = + HistoryTree::new_from_block(network, block0.clone(), &Default::default(), None).unwrap(); + + let block1 = block0 + .make_fake_child() + .set_commitment(finalized_tree.hash().into()); + let block1_tree = make_tree(block1.clone(), &finalized_tree)?; + + let long_chain_block1 = block1 + .make_fake_child() + .set_work(1) + .set_commitment(block1_tree.hash().into()); + let long_chain_block1_tree = make_tree(long_chain_block1.clone(), &block1_tree)?; + let long_chain_block2 = long_chain_block1 + .make_fake_child() + .set_work(1) + .set_commitment(long_chain_block1_tree.hash().into()); + let long_chain_block2_tree = make_tree(long_chain_block2.clone(), &long_chain_block1_tree)?; + let long_chain_block3 = long_chain_block2 + .make_fake_child() + .set_work(1) + .set_commitment(long_chain_block2_tree.hash().into()); + let long_chain_block3_tree = make_tree(long_chain_block3.clone(), &long_chain_block2_tree)?; + let long_chain_block4 = long_chain_block3 + .make_fake_child() + .set_work(1) + .set_commitment(long_chain_block3_tree.hash().into()); + + let short_chain_block = block1 + .make_fake_child() + .set_work(3) + .set_commitment(block1_tree.hash().into()); let mut state = NonFinalizedState::default(); - state.commit_new_chain(block1.prepare(), dummy_tree.clone())?; - state.commit_block(long_chain_block1.prepare(), &dummy_tree)?; - state.commit_block(long_chain_block2.prepare(), &dummy_tree)?; - state.commit_block(long_chain_block3.prepare(), &dummy_tree)?; - state.commit_block(long_chain_block4.prepare(), &dummy_tree)?; - state.commit_block(short_chain_block.prepare(), &dummy_tree)?; + state.commit_new_chain(block1.prepare(), finalized_tree.clone())?; + state.commit_block(long_chain_block1.prepare(), &finalized_tree)?; + state.commit_block(long_chain_block2.prepare(), &finalized_tree)?; + state.commit_block(long_chain_block3.prepare(), &finalized_tree)?; + state.commit_block(long_chain_block4.prepare(), &finalized_tree)?; + state.commit_block(short_chain_block.prepare(), &finalized_tree)?; assert_eq!(2, state.chain_set.len()); assert_eq!(5, state.best_chain_len()); @@ -332,7 +389,7 @@ fn equal_length_goes_to_more_work() -> Result<()> { Ok(()) } fn equal_length_goes_to_more_work_for_network(network: Network) -> Result<()> { - let block1: Arc = match network { + let block0: Arc = match network { Network::Mainnet => { zebra_test::vectors::BLOCK_MAINNET_1180900_BYTES.zcash_deserialize_into()? } @@ -340,22 +397,28 @@ fn equal_length_goes_to_more_work_for_network(network: Network) -> Result<()> { zebra_test::vectors::BLOCK_TESTNET_1326100_BYTES.zcash_deserialize_into()? } }; - let dummy_tree = HistoryTree::new_from_block( - network, - block1.clone(), - &sapling::tree::Root::default(), - None, - ) - .unwrap(); - - let less_work_child = block1.make_fake_child().set_work(1); - let more_work_child = block1.make_fake_child().set_work(3); + let finalized_tree = + HistoryTree::new_from_block(network, block0.clone(), &Default::default(), None).unwrap(); + + let block1 = block0 + .make_fake_child() + .set_commitment(finalized_tree.hash().into()); + let block1_tree = make_tree(block1.clone(), &finalized_tree)?; + + let less_work_child = block1 + .make_fake_child() + .set_work(1) + .set_commitment(block1_tree.hash().into()); + let more_work_child = block1 + .make_fake_child() + .set_work(3) + .set_commitment(block1_tree.hash().into()); let expected_hash = more_work_child.hash(); let mut state = NonFinalizedState::default(); - state.commit_new_chain(block1.prepare(), dummy_tree.clone())?; - state.commit_block(less_work_child.prepare(), &dummy_tree)?; - state.commit_block(more_work_child.prepare(), &dummy_tree)?; + state.commit_new_chain(block1.prepare(), finalized_tree.clone())?; + state.commit_block(less_work_child.prepare(), &finalized_tree)?; + state.commit_block(more_work_child.prepare(), &finalized_tree)?; assert_eq!(2, state.chain_set.len()); let tip_hash = state.best_tip().unwrap().1; diff --git a/zebra-state/src/tests.rs b/zebra-state/src/tests.rs index 2abbbe7004a..7ee7b624986 100644 --- a/zebra-state/src/tests.rs +++ b/zebra-state/src/tests.rs @@ -38,6 +38,8 @@ pub trait FakeChainHelper { fn make_fake_child(&self) -> Arc; fn set_work(self, work: u128) -> Arc; + + fn set_commitment(self, commitment: [u8; 32]) -> Arc; } impl FakeChainHelper for Arc { @@ -74,6 +76,12 @@ impl FakeChainHelper for Arc { block.header.difficulty_threshold = expanded.into(); self } + + fn set_commitment(mut self, commitment: [u8; 32]) -> Arc { + let block = Arc::make_mut(&mut self); + block.header.commitment_bytes = commitment; + self + } } fn work_to_expanded(work: U256) -> ExpandedDifficulty { From 13e50e20e6fbc806b474eb7869f902096b8511a2 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Fri, 25 Jun 2021 17:26:03 -0300 Subject: [PATCH 19/30] renamed mmr.rs to history_tree.rs --- zebra-chain/src/{mmr.rs => history_tree.rs} | 0 zebra-chain/src/lib.rs | 2 +- zebra-state/src/error.rs | 2 +- zebra-state/src/service/finalized_state.rs | 2 +- zebra-state/src/service/non_finalized_state.rs | 2 +- zebra-state/src/service/non_finalized_state/chain.rs | 2 +- zebra-state/src/service/non_finalized_state/tests/prop.rs | 2 +- zebra-state/src/service/non_finalized_state/tests/vectors.rs | 2 +- 8 files changed, 7 insertions(+), 7 deletions(-) rename zebra-chain/src/{mmr.rs => history_tree.rs} (100%) diff --git a/zebra-chain/src/mmr.rs b/zebra-chain/src/history_tree.rs similarity index 100% rename from zebra-chain/src/mmr.rs rename to zebra-chain/src/history_tree.rs diff --git a/zebra-chain/src/lib.rs b/zebra-chain/src/lib.rs index 94f677d86bc..86776f61309 100644 --- a/zebra-chain/src/lib.rs +++ b/zebra-chain/src/lib.rs @@ -21,7 +21,7 @@ extern crate bitflags; pub mod amount; pub mod block; pub mod fmt; -pub mod mmr; +pub mod history_tree; pub mod orchard; pub mod parameters; pub mod primitives; diff --git a/zebra-state/src/error.rs b/zebra-state/src/error.rs index eea10da6c7f..9e2a28fd175 100644 --- a/zebra-state/src/error.rs +++ b/zebra-state/src/error.rs @@ -5,7 +5,7 @@ use thiserror::Error; use zebra_chain::{ block::{self, ChainHistoryMmrRootHash, CommitmentError}, - mmr::HistoryTreeError, + history_tree::HistoryTreeError, work::difficulty::CompactDifficulty, }; diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index f4f8650916c..ee1cb86e8c1 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -7,7 +7,7 @@ mod tests; use std::{collections::HashMap, convert::TryInto, sync::Arc}; -use zebra_chain::mmr::HistoryTree; +use zebra_chain::history_tree::HistoryTree; use zebra_chain::transparent; use zebra_chain::{ block::{self, Block}, diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 6fc08f60f9d..e1a7b747357 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeSet, mem, ops::Deref, sync::Arc}; use zebra_chain::{ block::{self, Block}, - mmr::HistoryTree, + history_tree::HistoryTree, parameters::Network, transaction::{self, Transaction}, transparent, diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 2e6413780e5..7744f4c52a4 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -7,7 +7,7 @@ use std::{ use tracing::{debug_span, instrument, trace}; use zebra_chain::{ block::{self, ChainHistoryMmrRootHash}, - mmr::HistoryTree, + history_tree::HistoryTree, orchard, primitives::Groth16Proof, sapling, sprout, transaction, diff --git a/zebra-state/src/service/non_finalized_state/tests/prop.rs b/zebra-state/src/service/non_finalized_state/tests/prop.rs index f1bd82d3492..b9d346fccea 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -1,6 +1,6 @@ use std::env; -use zebra_chain::{mmr::HistoryTree, sapling}; +use zebra_chain::{history_tree::HistoryTree, sapling}; use zebra_test::prelude::*; use crate::service::non_finalized_state::{arbitrary::PreparedChain, Chain}; diff --git a/zebra-state/src/service/non_finalized_state/tests/vectors.rs b/zebra-state/src/service/non_finalized_state/tests/vectors.rs index 0680f9a7fb9..89571d6caf1 100644 --- a/zebra-state/src/service/non_finalized_state/tests/vectors.rs +++ b/zebra-state/src/service/non_finalized_state/tests/vectors.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use zebra_chain::{ block::Block, - mmr::{HistoryTree, HistoryTreeError}, + history_tree::{HistoryTree, HistoryTreeError}, parameters::Network, serialization::ZcashDeserializeInto, }; From 783770447ca16e1b9fa86234eaf9be821e37aa2e Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Fri, 25 Jun 2021 17:53:20 -0300 Subject: [PATCH 20/30] Add HistoryTree struct --- zebra-chain/src/history_tree.rs | 245 ++++++++++++++++++ zebra-chain/src/lib.rs | 1 + zebra-chain/src/primitives/zcash_history.rs | 48 ++-- .../primitives/zcash_history/tests/vectors.rs | 2 +- 4 files changed, 274 insertions(+), 22 deletions(-) create mode 100644 zebra-chain/src/history_tree.rs diff --git a/zebra-chain/src/history_tree.rs b/zebra-chain/src/history_tree.rs new file mode 100644 index 00000000000..f16ba05b3c6 --- /dev/null +++ b/zebra-chain/src/history_tree.rs @@ -0,0 +1,245 @@ +//! History tree (Merkle mountain range) structure that contains information about +//! the block history as specified in ZIP-221. + +use std::{ + collections::{BTreeMap, HashSet}, + io, + sync::Arc, +}; + +use thiserror::Error; + +use crate::{ + block::{Block, ChainHistoryMmrRootHash, Height}, + orchard, + parameters::{Network, NetworkUpgrade}, + primitives::zcash_history::{Entry, Tree}, + sapling, +}; + +/// An error describing why a history tree operation failed. +#[derive(Debug, Error)] +#[non_exhaustive] +#[allow(missing_docs)] +pub enum HistoryTreeError { + #[error("zcash_history error: {inner:?}")] + #[non_exhaustive] + InnerError { inner: zcash_history::Error }, + + #[error("I/O error")] + IOError(#[from] io::Error), +} + +/// History tree structure. +pub struct HistoryTree { + network: Network, + network_upgrade: NetworkUpgrade, + /// Merkle mountain range tree. + /// This is a "runtime" structure used to add / remove nodes, and it's not + /// persistent. + inner: Tree, + /// The number of nodes in the tree. + size: u32, + /// The peaks of the tree, indexed by their position in the array representation + /// of the tree. This can be persisted to save the tree. + peaks: BTreeMap, + /// The height of the most recent block added to the tree. + current_height: Height, +} + +impl HistoryTree { + /// Create a new history tree with a single block. + pub fn new_from_block( + network: Network, + block: Arc, + sapling_root: &sapling::tree::Root, + _orchard_root: Option<&orchard::tree::Root>, + ) -> Result { + let height = block + .coinbase_height() + .expect("block must have coinbase height during contextual verification"); + let network_upgrade = NetworkUpgrade::current(network, height); + // TODO: handle Orchard root + let (tree, entry) = Tree::new_from_block(network, block, sapling_root)?; + let mut peaks = BTreeMap::new(); + peaks.insert(0u32, entry); + Ok(HistoryTree { + network, + network_upgrade, + inner: tree, + size: 1, + peaks, + current_height: height, + }) + } + + /// Add block data to the tree. + /// + /// # Panics + /// + /// If the block height is not one more than the previously pushed block. + pub fn push( + &mut self, + block: Arc, + sapling_root: &sapling::tree::Root, + _orchard_root: Option<&orchard::tree::Root>, + ) -> Result<(), HistoryTreeError> { + // Check if the block has the expected height. + // librustzcash assumes the heights are correct and corrupts the tree if they are wrong, + // resulting in a confusing error, which we prevent here. + let height = block + .coinbase_height() + .expect("block must have coinbase height during contextual verification"); + if height - self.current_height != 1 { + panic!( + "added block with height {:?} but it must be {:?}+1", + height, self.current_height + ); + } + + // TODO: handle orchard root + let new_entries = self + .inner + .append_leaf(block, sapling_root) + .map_err(|e| HistoryTreeError::InnerError { inner: e })?; + for entry in new_entries { + // Not every entry is a peak; those will be trimmed later + self.peaks.insert(self.size, entry); + self.size += 1; + } + self.prune()?; + // TODO: implement network upgrade logic: drop previous history, start new history + self.current_height = height; + Ok(()) + } + + /// Extend the history tree with the given blocks. + pub fn try_extend< + 'a, + T: IntoIterator< + Item = ( + Arc, + &'a sapling::tree::Root, + Option<&'a orchard::tree::Root>, + ), + >, + >( + &mut self, + iter: T, + ) -> Result<(), HistoryTreeError> { + for (block, sapling_root, orchard_root) in iter { + self.push(block, sapling_root, orchard_root)?; + } + Ok(()) + } + + /// Prune tree, removing all non-peak entries. + fn prune(&mut self) -> Result<(), io::Error> { + // Go through all the peaks of the tree. + // This code is based on a librustzcash example: + // https://github.com/zcash/librustzcash/blob/02052526925fba9389f1428d6df254d4dec967e6/zcash_history/examples/long.rs + // The explanation of how it works is from zcashd: + // https://github.com/zcash/zcash/blob/0247c0c682d59184a717a6536edb0d18834be9a7/src/coins.cpp#L351 + + let mut peak_pos_set = HashSet::new(); + + // Assume the following example peak layout with 14 leaves, and 25 stored nodes in + // total (the "tree length"): + // + // P + // /\ + // / \ + // / \ \ + // / \ \ Altitude + // _A_ \ \ 3 + // _/ \_ B \ 2 + // / \ / \ / \ C 1 + // /\ /\ /\ /\ /\ /\ /\ 0 + // + // We start by determining the altitude of the highest peak (A). + let mut alt = (32 - ((self.size + 1) as u32).leading_zeros() - 1) - 1; + + // We determine the position of the highest peak (A) by pretending it is the right + // sibling in a tree, and its left-most leaf has position 0. Then the left sibling + // of (A) has position -1, and so we can "jump" to the peak's position by computing + // -1 + 2^(alt + 1) - 1. + let mut peak_pos = (1 << (alt + 1)) - 2; + + // Now that we have the position and altitude of the highest peak (A), we collect + // the remaining peaks (B, C). We navigate the peaks as if they were nodes in this + // Merkle tree (with additional imaginary nodes 1 and 2, that have positions beyond + // the MMR's length): + // + // / \ + // / \ + // / \ + // / \ + // A ==========> 1 + // / \ // \ + // _/ \_ B ==> 2 + // /\ /\ /\ // + // / \ / \ / \ C + // /\ /\ /\ /\ /\ /\ /\ + // + loop { + // If peak_pos is out of bounds of the tree, we compute the position of its left + // child, and drop down one level in the tree. + if peak_pos >= self.size { + // left child, -2^alt + peak_pos -= 1 << alt; + alt -= 1; + } + + // If the peak exists, we take it and then continue with its right sibling. + if peak_pos < self.size { + // There is a peak at index `peak_pos` + peak_pos_set.insert(peak_pos); + + // right sibling + peak_pos = peak_pos + (1 << (alt + 1)) - 1; + } + + if alt == 0 { + break; + } + } + + // Remove all non-peak entries + self.peaks.retain(|k, _| peak_pos_set.contains(k)); + // Rebuild tree + self.inner = Tree::new_from_cache( + self.network, + self.network_upgrade, + self.size, + &self.peaks, + &Default::default(), + )?; + Ok(()) + } + + /// Return the hash of the tree root. + pub fn hash(&self) -> ChainHistoryMmrRootHash { + self.inner.hash() + } +} + +impl Clone for HistoryTree { + fn clone(&self) -> Self { + let tree = Tree::new_from_cache( + self.network, + self.network_upgrade, + self.size, + &self.peaks, + &Default::default(), + ) + .expect("rebuilding an existing tree should always work"); + HistoryTree { + network: self.network, + network_upgrade: self.network_upgrade, + inner: tree, + size: self.size, + peaks: self.peaks.clone(), + current_height: self.current_height, + } + } +} diff --git a/zebra-chain/src/lib.rs b/zebra-chain/src/lib.rs index 4ac1e9f2890..9d7f8f9e53e 100644 --- a/zebra-chain/src/lib.rs +++ b/zebra-chain/src/lib.rs @@ -23,6 +23,7 @@ extern crate bitflags; pub mod amount; pub mod block; pub mod fmt; +pub mod history_tree; pub mod orchard; pub mod parameters; pub mod primitives; diff --git a/zebra-chain/src/primitives/zcash_history.rs b/zebra-chain/src/primitives/zcash_history.rs index 1c96ab4772a..f7835446443 100644 --- a/zebra-chain/src/primitives/zcash_history.rs +++ b/zebra-chain/src/primitives/zcash_history.rs @@ -6,7 +6,7 @@ mod tests; -use std::{collections::HashMap, convert::TryInto, io, sync::Arc}; +use std::{collections::BTreeMap, convert::TryInto, io, sync::Arc}; use crate::{ block::{Block, ChainHistoryMmrRootHash}, @@ -44,6 +44,7 @@ impl From<&zcash_history::NodeData> for NodeData { /// An encoded entry in the tree. /// Contains the node data and information about its position in the tree. +#[derive(Clone)] pub struct Entry { inner: [u8; zcash_history::MAX_ENTRY_SIZE], } @@ -101,12 +102,12 @@ impl Tree { /// # Panics /// /// Will panic if `peaks` is empty. - fn new_from_cache( + pub fn new_from_cache( network: Network, network_upgrade: NetworkUpgrade, length: u32, - peaks: &HashMap, - extra: &HashMap, + peaks: &BTreeMap, + extra: &BTreeMap, ) -> Result { let branch_id = network_upgrade .branch_id() @@ -132,19 +133,24 @@ impl Tree { /// Create a single-node MMR tree for the given block. /// /// `sapling_root` is the root of the Sapling note commitment tree of the block. - fn new_from_block( + pub fn new_from_block( network: Network, block: Arc, sapling_root: &sapling::tree::Root, - ) -> Result { + ) -> Result<(Self, Entry), io::Error> { let height = block .coinbase_height() .expect("block must have coinbase height during contextual verification"); let network_upgrade = NetworkUpgrade::current(network, height); let entry0 = Entry::new_leaf(block, network, sapling_root); - let mut peaks = HashMap::new(); - peaks.insert(0u32, &entry0); - Tree::new_from_cache(network, network_upgrade, 1, &peaks, &HashMap::new()) + let mut peaks = BTreeMap::new(); + peaks.insert(0u32, entry0); + Ok(( + Tree::new_from_cache(network, network_upgrade, 1, &peaks, &BTreeMap::new())?, + peaks + .remove(&0u32) + .expect("must work since it was just added"), + )) } /// Append a new block to the tree, as a new leaf. @@ -157,18 +163,18 @@ impl Tree { /// /// Panics if the network upgrade of the given block is different from /// the network upgrade of the other blocks in the tree. - fn append_leaf( + pub fn append_leaf( &mut self, block: Arc, sapling_root: &sapling::tree::Root, - ) -> Result, zcash_history::Error> { + ) -> Result, zcash_history::Error> { let height = block .coinbase_height() .expect("block must have coinbase height during contextual verification"); let network_upgrade = NetworkUpgrade::current(self.network, height); if self.network_upgrade != network_upgrade { panic!( - "added block from network upgrade {:?} but MMR tree is restricted to {:?}", + "added block from network upgrade {:?} but history tree is restricted to {:?}", network_upgrade, self.network_upgrade ); } @@ -177,17 +183,17 @@ impl Tree { let appended = self.inner.append_leaf(node_data)?; let mut new_nodes = Vec::new(); - for entry in appended { - let mut node = NodeData { - inner: [0; zcash_history::MAX_NODE_DATA_SIZE], + for entry_link in appended { + let mut entry = Entry { + inner: [0; zcash_history::MAX_ENTRY_SIZE], }; self.inner - .resolve_link(entry) + .resolve_link(entry_link) .expect("entry was just generated so it must be valid") - .data() - .write(&mut &mut node.inner[..]) + .node() + .write(&mut &mut entry.inner[..]) .expect("buffer was created with enough capacity"); - new_nodes.push(node); + new_nodes.push(entry); } Ok(new_nodes) } @@ -196,7 +202,7 @@ impl Tree { fn append_leaf_iter( &mut self, vals: impl Iterator, sapling::tree::Root)>, - ) -> Result, zcash_history::Error> { + ) -> Result, zcash_history::Error> { let mut new_nodes = Vec::new(); for (block, root) in vals { new_nodes.append(&mut self.append_leaf(block, &root)?); @@ -212,7 +218,7 @@ impl Tree { } /// Return the root hash of the tree, i.e. `hashChainHistoryRoot`. - fn hash(&self) -> ChainHistoryMmrRootHash { + pub fn hash(&self) -> ChainHistoryMmrRootHash { // Both append_leaf() and truncate_leaf() leave a root node, so it should // always exist. self.inner diff --git a/zebra-chain/src/primitives/zcash_history/tests/vectors.rs b/zebra-chain/src/primitives/zcash_history/tests/vectors.rs index 88bf6b21492..6fd927640ee 100644 --- a/zebra-chain/src/primitives/zcash_history/tests/vectors.rs +++ b/zebra-chain/src/primitives/zcash_history/tests/vectors.rs @@ -49,7 +49,7 @@ fn tree_for_network_upgrade(network: Network, network_upgrade: NetworkUpgrade) - // Build initial MMR tree with only Block 0 let sapling_root0 = sapling::tree::Root(**sapling_roots.get(&height).expect("test vector exists")); - let mut tree = Tree::new_from_block(network, block0, &sapling_root0)?; + let (mut tree, _) = Tree::new_from_block(network, block0, &sapling_root0)?; // Compute root hash of the MMR tree, which will be included in the next block let hash0 = tree.hash(); From 985c6aac4b4536c4c03620a1277abf4b5190e705 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Mon, 28 Jun 2021 12:06:26 -0300 Subject: [PATCH 21/30] expand non_finalized_state protest --- .../src/service/non_finalized_state/chain.rs | 18 +++++++++++++++++- .../service/non_finalized_state/tests/prop.rs | 11 ++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 7744f4c52a4..88f6765986f 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -18,7 +18,6 @@ use zebra_chain::{ use crate::{PreparedBlock, Utxo, ValidateContextError}; -// #[derive(Clone)] pub struct Chain { pub blocks: BTreeMap, pub height_by_hash: HashMap, @@ -192,6 +191,23 @@ impl Chain { history_tree, } } + + /// Return of two Chains are identical. + #[cfg(test)] + pub(crate) fn is_identical(&self, other: &Self) -> bool { + self.blocks == other.blocks + && self.height_by_hash == other.height_by_hash + && self.tx_by_hash == other.tx_by_hash + && self.created_utxos == other.created_utxos + && self.spent_utxos == other.spent_utxos + && self.sprout_anchors == other.sprout_anchors + && self.sapling_anchors == other.sapling_anchors + && self.sprout_nullifiers == other.sprout_nullifiers + && self.sapling_nullifiers == other.sapling_nullifiers + && self.orchard_nullifiers == other.orchard_nullifiers + && self.partial_cumulative_work == other.partial_cumulative_work + && self.history_root_hash() == other.history_root_hash() + } } /// Helper trait to organize inverse operations done on the `Chain` type. Used to diff --git a/zebra-state/src/service/non_finalized_state/tests/prop.rs b/zebra-state/src/service/non_finalized_state/tests/prop.rs index b9d346fccea..054713b3f0b 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -31,9 +31,17 @@ fn forked_equals_pushed() -> Result<()> { full_chain.push(block.clone())?; } - let forked = full_chain.fork(fork_tip_hash, &finalized_tree).expect("fork must work").expect("hash is present"); + let mut forked = full_chain.fork(fork_tip_hash, &finalized_tree).expect("fork must work").expect("hash is present"); prop_assert_eq!(forked.blocks.len(), partial_chain.blocks.len()); + prop_assert!(forked.is_identical(&partial_chain)); + + for block in chain.iter().skip(count) { + forked.push(block.clone())?; + } + + prop_assert_eq!(forked.blocks.len(), full_chain.blocks.len()); + prop_assert!(forked.is_identical(&full_chain)); }); Ok(()) @@ -72,6 +80,7 @@ fn finalized_equals_pushed() -> Result<()> { } prop_assert_eq!(full_chain.blocks.len(), partial_chain.blocks.len()); + prop_assert!(full_chain.is_identical(&partial_chain)); }); Ok(()) From 5249335ba09040000ddffe3fb30b15dc47eb1431 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Mon, 28 Jun 2021 14:50:30 -0300 Subject: [PATCH 22/30] fix typo --- zebra-state/src/service/non_finalized_state/chain.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 88f6765986f..6da14ae8f31 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -192,7 +192,7 @@ impl Chain { } } - /// Return of two Chains are identical. + /// Return if two Chains are identical. #[cfg(test)] pub(crate) fn is_identical(&self, other: &Self) -> bool { self.blocks == other.blocks From fca06e61f269c8c29a4ac8068c26bcaa90ca69cc Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Fri, 25 Jun 2021 17:53:20 -0300 Subject: [PATCH 23/30] Add HistoryTree struct --- zebra-chain/src/history_tree.rs | 245 ++++++++++++++++++ zebra-chain/src/lib.rs | 1 + zebra-chain/src/primitives/zcash_history.rs | 48 ++-- .../primitives/zcash_history/tests/vectors.rs | 2 +- 4 files changed, 274 insertions(+), 22 deletions(-) create mode 100644 zebra-chain/src/history_tree.rs diff --git a/zebra-chain/src/history_tree.rs b/zebra-chain/src/history_tree.rs new file mode 100644 index 00000000000..f16ba05b3c6 --- /dev/null +++ b/zebra-chain/src/history_tree.rs @@ -0,0 +1,245 @@ +//! History tree (Merkle mountain range) structure that contains information about +//! the block history as specified in ZIP-221. + +use std::{ + collections::{BTreeMap, HashSet}, + io, + sync::Arc, +}; + +use thiserror::Error; + +use crate::{ + block::{Block, ChainHistoryMmrRootHash, Height}, + orchard, + parameters::{Network, NetworkUpgrade}, + primitives::zcash_history::{Entry, Tree}, + sapling, +}; + +/// An error describing why a history tree operation failed. +#[derive(Debug, Error)] +#[non_exhaustive] +#[allow(missing_docs)] +pub enum HistoryTreeError { + #[error("zcash_history error: {inner:?}")] + #[non_exhaustive] + InnerError { inner: zcash_history::Error }, + + #[error("I/O error")] + IOError(#[from] io::Error), +} + +/// History tree structure. +pub struct HistoryTree { + network: Network, + network_upgrade: NetworkUpgrade, + /// Merkle mountain range tree. + /// This is a "runtime" structure used to add / remove nodes, and it's not + /// persistent. + inner: Tree, + /// The number of nodes in the tree. + size: u32, + /// The peaks of the tree, indexed by their position in the array representation + /// of the tree. This can be persisted to save the tree. + peaks: BTreeMap, + /// The height of the most recent block added to the tree. + current_height: Height, +} + +impl HistoryTree { + /// Create a new history tree with a single block. + pub fn new_from_block( + network: Network, + block: Arc, + sapling_root: &sapling::tree::Root, + _orchard_root: Option<&orchard::tree::Root>, + ) -> Result { + let height = block + .coinbase_height() + .expect("block must have coinbase height during contextual verification"); + let network_upgrade = NetworkUpgrade::current(network, height); + // TODO: handle Orchard root + let (tree, entry) = Tree::new_from_block(network, block, sapling_root)?; + let mut peaks = BTreeMap::new(); + peaks.insert(0u32, entry); + Ok(HistoryTree { + network, + network_upgrade, + inner: tree, + size: 1, + peaks, + current_height: height, + }) + } + + /// Add block data to the tree. + /// + /// # Panics + /// + /// If the block height is not one more than the previously pushed block. + pub fn push( + &mut self, + block: Arc, + sapling_root: &sapling::tree::Root, + _orchard_root: Option<&orchard::tree::Root>, + ) -> Result<(), HistoryTreeError> { + // Check if the block has the expected height. + // librustzcash assumes the heights are correct and corrupts the tree if they are wrong, + // resulting in a confusing error, which we prevent here. + let height = block + .coinbase_height() + .expect("block must have coinbase height during contextual verification"); + if height - self.current_height != 1 { + panic!( + "added block with height {:?} but it must be {:?}+1", + height, self.current_height + ); + } + + // TODO: handle orchard root + let new_entries = self + .inner + .append_leaf(block, sapling_root) + .map_err(|e| HistoryTreeError::InnerError { inner: e })?; + for entry in new_entries { + // Not every entry is a peak; those will be trimmed later + self.peaks.insert(self.size, entry); + self.size += 1; + } + self.prune()?; + // TODO: implement network upgrade logic: drop previous history, start new history + self.current_height = height; + Ok(()) + } + + /// Extend the history tree with the given blocks. + pub fn try_extend< + 'a, + T: IntoIterator< + Item = ( + Arc, + &'a sapling::tree::Root, + Option<&'a orchard::tree::Root>, + ), + >, + >( + &mut self, + iter: T, + ) -> Result<(), HistoryTreeError> { + for (block, sapling_root, orchard_root) in iter { + self.push(block, sapling_root, orchard_root)?; + } + Ok(()) + } + + /// Prune tree, removing all non-peak entries. + fn prune(&mut self) -> Result<(), io::Error> { + // Go through all the peaks of the tree. + // This code is based on a librustzcash example: + // https://github.com/zcash/librustzcash/blob/02052526925fba9389f1428d6df254d4dec967e6/zcash_history/examples/long.rs + // The explanation of how it works is from zcashd: + // https://github.com/zcash/zcash/blob/0247c0c682d59184a717a6536edb0d18834be9a7/src/coins.cpp#L351 + + let mut peak_pos_set = HashSet::new(); + + // Assume the following example peak layout with 14 leaves, and 25 stored nodes in + // total (the "tree length"): + // + // P + // /\ + // / \ + // / \ \ + // / \ \ Altitude + // _A_ \ \ 3 + // _/ \_ B \ 2 + // / \ / \ / \ C 1 + // /\ /\ /\ /\ /\ /\ /\ 0 + // + // We start by determining the altitude of the highest peak (A). + let mut alt = (32 - ((self.size + 1) as u32).leading_zeros() - 1) - 1; + + // We determine the position of the highest peak (A) by pretending it is the right + // sibling in a tree, and its left-most leaf has position 0. Then the left sibling + // of (A) has position -1, and so we can "jump" to the peak's position by computing + // -1 + 2^(alt + 1) - 1. + let mut peak_pos = (1 << (alt + 1)) - 2; + + // Now that we have the position and altitude of the highest peak (A), we collect + // the remaining peaks (B, C). We navigate the peaks as if they were nodes in this + // Merkle tree (with additional imaginary nodes 1 and 2, that have positions beyond + // the MMR's length): + // + // / \ + // / \ + // / \ + // / \ + // A ==========> 1 + // / \ // \ + // _/ \_ B ==> 2 + // /\ /\ /\ // + // / \ / \ / \ C + // /\ /\ /\ /\ /\ /\ /\ + // + loop { + // If peak_pos is out of bounds of the tree, we compute the position of its left + // child, and drop down one level in the tree. + if peak_pos >= self.size { + // left child, -2^alt + peak_pos -= 1 << alt; + alt -= 1; + } + + // If the peak exists, we take it and then continue with its right sibling. + if peak_pos < self.size { + // There is a peak at index `peak_pos` + peak_pos_set.insert(peak_pos); + + // right sibling + peak_pos = peak_pos + (1 << (alt + 1)) - 1; + } + + if alt == 0 { + break; + } + } + + // Remove all non-peak entries + self.peaks.retain(|k, _| peak_pos_set.contains(k)); + // Rebuild tree + self.inner = Tree::new_from_cache( + self.network, + self.network_upgrade, + self.size, + &self.peaks, + &Default::default(), + )?; + Ok(()) + } + + /// Return the hash of the tree root. + pub fn hash(&self) -> ChainHistoryMmrRootHash { + self.inner.hash() + } +} + +impl Clone for HistoryTree { + fn clone(&self) -> Self { + let tree = Tree::new_from_cache( + self.network, + self.network_upgrade, + self.size, + &self.peaks, + &Default::default(), + ) + .expect("rebuilding an existing tree should always work"); + HistoryTree { + network: self.network, + network_upgrade: self.network_upgrade, + inner: tree, + size: self.size, + peaks: self.peaks.clone(), + current_height: self.current_height, + } + } +} diff --git a/zebra-chain/src/lib.rs b/zebra-chain/src/lib.rs index 4ac1e9f2890..9d7f8f9e53e 100644 --- a/zebra-chain/src/lib.rs +++ b/zebra-chain/src/lib.rs @@ -23,6 +23,7 @@ extern crate bitflags; pub mod amount; pub mod block; pub mod fmt; +pub mod history_tree; pub mod orchard; pub mod parameters; pub mod primitives; diff --git a/zebra-chain/src/primitives/zcash_history.rs b/zebra-chain/src/primitives/zcash_history.rs index 1c96ab4772a..f7835446443 100644 --- a/zebra-chain/src/primitives/zcash_history.rs +++ b/zebra-chain/src/primitives/zcash_history.rs @@ -6,7 +6,7 @@ mod tests; -use std::{collections::HashMap, convert::TryInto, io, sync::Arc}; +use std::{collections::BTreeMap, convert::TryInto, io, sync::Arc}; use crate::{ block::{Block, ChainHistoryMmrRootHash}, @@ -44,6 +44,7 @@ impl From<&zcash_history::NodeData> for NodeData { /// An encoded entry in the tree. /// Contains the node data and information about its position in the tree. +#[derive(Clone)] pub struct Entry { inner: [u8; zcash_history::MAX_ENTRY_SIZE], } @@ -101,12 +102,12 @@ impl Tree { /// # Panics /// /// Will panic if `peaks` is empty. - fn new_from_cache( + pub fn new_from_cache( network: Network, network_upgrade: NetworkUpgrade, length: u32, - peaks: &HashMap, - extra: &HashMap, + peaks: &BTreeMap, + extra: &BTreeMap, ) -> Result { let branch_id = network_upgrade .branch_id() @@ -132,19 +133,24 @@ impl Tree { /// Create a single-node MMR tree for the given block. /// /// `sapling_root` is the root of the Sapling note commitment tree of the block. - fn new_from_block( + pub fn new_from_block( network: Network, block: Arc, sapling_root: &sapling::tree::Root, - ) -> Result { + ) -> Result<(Self, Entry), io::Error> { let height = block .coinbase_height() .expect("block must have coinbase height during contextual verification"); let network_upgrade = NetworkUpgrade::current(network, height); let entry0 = Entry::new_leaf(block, network, sapling_root); - let mut peaks = HashMap::new(); - peaks.insert(0u32, &entry0); - Tree::new_from_cache(network, network_upgrade, 1, &peaks, &HashMap::new()) + let mut peaks = BTreeMap::new(); + peaks.insert(0u32, entry0); + Ok(( + Tree::new_from_cache(network, network_upgrade, 1, &peaks, &BTreeMap::new())?, + peaks + .remove(&0u32) + .expect("must work since it was just added"), + )) } /// Append a new block to the tree, as a new leaf. @@ -157,18 +163,18 @@ impl Tree { /// /// Panics if the network upgrade of the given block is different from /// the network upgrade of the other blocks in the tree. - fn append_leaf( + pub fn append_leaf( &mut self, block: Arc, sapling_root: &sapling::tree::Root, - ) -> Result, zcash_history::Error> { + ) -> Result, zcash_history::Error> { let height = block .coinbase_height() .expect("block must have coinbase height during contextual verification"); let network_upgrade = NetworkUpgrade::current(self.network, height); if self.network_upgrade != network_upgrade { panic!( - "added block from network upgrade {:?} but MMR tree is restricted to {:?}", + "added block from network upgrade {:?} but history tree is restricted to {:?}", network_upgrade, self.network_upgrade ); } @@ -177,17 +183,17 @@ impl Tree { let appended = self.inner.append_leaf(node_data)?; let mut new_nodes = Vec::new(); - for entry in appended { - let mut node = NodeData { - inner: [0; zcash_history::MAX_NODE_DATA_SIZE], + for entry_link in appended { + let mut entry = Entry { + inner: [0; zcash_history::MAX_ENTRY_SIZE], }; self.inner - .resolve_link(entry) + .resolve_link(entry_link) .expect("entry was just generated so it must be valid") - .data() - .write(&mut &mut node.inner[..]) + .node() + .write(&mut &mut entry.inner[..]) .expect("buffer was created with enough capacity"); - new_nodes.push(node); + new_nodes.push(entry); } Ok(new_nodes) } @@ -196,7 +202,7 @@ impl Tree { fn append_leaf_iter( &mut self, vals: impl Iterator, sapling::tree::Root)>, - ) -> Result, zcash_history::Error> { + ) -> Result, zcash_history::Error> { let mut new_nodes = Vec::new(); for (block, root) in vals { new_nodes.append(&mut self.append_leaf(block, &root)?); @@ -212,7 +218,7 @@ impl Tree { } /// Return the root hash of the tree, i.e. `hashChainHistoryRoot`. - fn hash(&self) -> ChainHistoryMmrRootHash { + pub fn hash(&self) -> ChainHistoryMmrRootHash { // Both append_leaf() and truncate_leaf() leave a root node, so it should // always exist. self.inner diff --git a/zebra-chain/src/primitives/zcash_history/tests/vectors.rs b/zebra-chain/src/primitives/zcash_history/tests/vectors.rs index 88bf6b21492..6fd927640ee 100644 --- a/zebra-chain/src/primitives/zcash_history/tests/vectors.rs +++ b/zebra-chain/src/primitives/zcash_history/tests/vectors.rs @@ -49,7 +49,7 @@ fn tree_for_network_upgrade(network: Network, network_upgrade: NetworkUpgrade) - // Build initial MMR tree with only Block 0 let sapling_root0 = sapling::tree::Root(**sapling_roots.get(&height).expect("test vector exists")); - let mut tree = Tree::new_from_block(network, block0, &sapling_root0)?; + let (mut tree, _) = Tree::new_from_block(network, block0, &sapling_root0)?; // Compute root hash of the MMR tree, which will be included in the next block let hash0 = tree.hash(); From fa847b86e08900b2fec5a68777c4566f947a6a40 Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Tue, 29 Jun 2021 10:49:31 -0300 Subject: [PATCH 24/30] Update zebra-chain/src/primitives/zcash_history.rs Co-authored-by: Deirdre Connolly --- zebra-chain/src/primitives/zcash_history.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zebra-chain/src/primitives/zcash_history.rs b/zebra-chain/src/primitives/zcash_history.rs index f7835446443..e7a4d0ed280 100644 --- a/zebra-chain/src/primitives/zcash_history.rs +++ b/zebra-chain/src/primitives/zcash_history.rs @@ -43,6 +43,7 @@ impl From<&zcash_history::NodeData> for NodeData { } /// An encoded entry in the tree. +/// /// Contains the node data and information about its position in the tree. #[derive(Clone)] pub struct Entry { From 8e38ed4c8ac445cd4c1a110356a05b0eb3aa55eb Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Tue, 29 Jun 2021 11:13:28 -0300 Subject: [PATCH 25/30] fix formatting --- zebra-chain/src/primitives/zcash_history.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-chain/src/primitives/zcash_history.rs b/zebra-chain/src/primitives/zcash_history.rs index e7a4d0ed280..6956b58b3f4 100644 --- a/zebra-chain/src/primitives/zcash_history.rs +++ b/zebra-chain/src/primitives/zcash_history.rs @@ -43,7 +43,7 @@ impl From<&zcash_history::NodeData> for NodeData { } /// An encoded entry in the tree. -/// +/// /// Contains the node data and information about its position in the tree. #[derive(Clone)] pub struct Entry { From f154696bbadc73d92e5c7248018c620d22ba7665 Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Tue, 29 Jun 2021 14:24:10 -0300 Subject: [PATCH 26/30] Apply suggestions from code review Co-authored-by: Deirdre Connolly --- zebra-chain/src/history_tree.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/zebra-chain/src/history_tree.rs b/zebra-chain/src/history_tree.rs index f16ba05b3c6..df29f0479c0 100644 --- a/zebra-chain/src/history_tree.rs +++ b/zebra-chain/src/history_tree.rs @@ -30,11 +30,12 @@ pub enum HistoryTreeError { IOError(#[from] io::Error), } -/// History tree structure. +/// History tree (Merkle mountain range) structure that contains information about +// the block history, as specified in [ZIP-221][https://zips.z.cash/zip-0221]. pub struct HistoryTree { network: Network, network_upgrade: NetworkUpgrade, - /// Merkle mountain range tree. + /// Merkle mountain range tree from `zcash_history`. /// This is a "runtime" structure used to add / remove nodes, and it's not /// persistent. inner: Tree, From 81b0ddb8b512f1192f03e51597131e88d85c2df9 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Tue, 29 Jun 2021 14:31:51 -0300 Subject: [PATCH 27/30] history_tree.rs: fixes from code review --- zebra-chain/src/history_tree.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/zebra-chain/src/history_tree.rs b/zebra-chain/src/history_tree.rs index df29f0479c0..b53fc6aa25b 100644 --- a/zebra-chain/src/history_tree.rs +++ b/zebra-chain/src/history_tree.rs @@ -13,7 +13,7 @@ use crate::{ block::{Block, ChainHistoryMmrRootHash, Height}, orchard, parameters::{Network, NetworkUpgrade}, - primitives::zcash_history::{Entry, Tree}, + primitives::zcash_history::{Entry, Tree as InnerHistoryTree}, sapling, }; @@ -38,7 +38,7 @@ pub struct HistoryTree { /// Merkle mountain range tree from `zcash_history`. /// This is a "runtime" structure used to add / remove nodes, and it's not /// persistent. - inner: Tree, + inner: InnerHistoryTree, /// The number of nodes in the tree. size: u32, /// The peaks of the tree, indexed by their position in the array representation @@ -50,7 +50,7 @@ pub struct HistoryTree { impl HistoryTree { /// Create a new history tree with a single block. - pub fn new_from_block( + pub fn from_block( network: Network, block: Arc, sapling_root: &sapling::tree::Root, @@ -60,8 +60,8 @@ impl HistoryTree { .coinbase_height() .expect("block must have coinbase height during contextual verification"); let network_upgrade = NetworkUpgrade::current(network, height); - // TODO: handle Orchard root - let (tree, entry) = Tree::new_from_block(network, block, sapling_root)?; + // TODO: handle Orchard root, see https://github.com/ZcashFoundation/zebra/issues/2283 + let (tree, entry) = InnerHistoryTree::new_from_block(network, block, sapling_root)?; let mut peaks = BTreeMap::new(); peaks.insert(0u32, entry); Ok(HistoryTree { @@ -208,7 +208,7 @@ impl HistoryTree { // Remove all non-peak entries self.peaks.retain(|k, _| peak_pos_set.contains(k)); // Rebuild tree - self.inner = Tree::new_from_cache( + self.inner = InnerHistoryTree::new_from_cache( self.network, self.network_upgrade, self.size, @@ -226,7 +226,7 @@ impl HistoryTree { impl Clone for HistoryTree { fn clone(&self) -> Self { - let tree = Tree::new_from_cache( + let tree = InnerHistoryTree::new_from_cache( self.network, self.network_upgrade, self.size, From 608c2e208d0aefc34a5edfcb1661ad72fdaefbcf Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Thu, 1 Jul 2021 10:45:26 -0300 Subject: [PATCH 28/30] fixes to work with updated HistoryTree --- .../service/non_finalized_state/tests/prop.rs | 4 ++-- .../non_finalized_state/tests/vectors.rs | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/zebra-state/src/service/non_finalized_state/tests/prop.rs b/zebra-state/src/service/non_finalized_state/tests/prop.rs index d7c92a89050..eebeff7ff7c 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -18,7 +18,7 @@ fn forked_equals_pushed() -> Result<()> { |((chain, count, network) in PreparedChain::new_heartwood())| { // Build a history tree with the first block to simulate the tree of // the finalized state. - let finalized_tree = HistoryTree::new_from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); + let finalized_tree = HistoryTree::from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); let chain = &chain[1..]; let fork_tip_hash = chain[count - 1].hash; let mut full_chain = Chain::new(finalized_tree.clone()); @@ -58,7 +58,7 @@ fn finalized_equals_pushed() -> Result<()> { |((chain, end_count, network) in PreparedChain::new_heartwood())| { // Build a history tree with the first block to simulate the tree of // the finalized state. - let finalized_tree = HistoryTree::new_from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); + let finalized_tree = HistoryTree::from_block(network, chain[0].block.clone(), &sapling::tree::Root::default(), None).unwrap(); let chain = &chain[1..]; let finalized_count = chain.len() - end_count; let mut full_chain = Chain::new(finalized_tree); diff --git a/zebra-state/src/service/non_finalized_state/tests/vectors.rs b/zebra-state/src/service/non_finalized_state/tests/vectors.rs index 89571d6caf1..fa64099b8ab 100644 --- a/zebra-state/src/service/non_finalized_state/tests/vectors.rs +++ b/zebra-state/src/service/non_finalized_state/tests/vectors.rs @@ -32,7 +32,7 @@ fn construct_single() -> Result<()> { zebra_test::vectors::BLOCK_MAINNET_434873_BYTES.zcash_deserialize_into()?; let finalized_tree = - HistoryTree::new_from_block(Network::Mainnet, block0.clone(), &Default::default(), None) + HistoryTree::from_block(Network::Mainnet, block0.clone(), &Default::default(), None) .unwrap(); let block1 = block0 @@ -54,7 +54,7 @@ fn construct_many() -> Result<()> { let mut block: Arc = zebra_test::vectors::BLOCK_MAINNET_434873_BYTES.zcash_deserialize_into()?; let finalized_tree = - HistoryTree::new_from_block(Network::Mainnet, block.clone(), &Default::default(), None) + HistoryTree::from_block(Network::Mainnet, block.clone(), &Default::default(), None) .unwrap(); let mut blocks = vec![]; @@ -83,7 +83,7 @@ fn ord_matches_work() -> Result<()> { let block = zebra_test::vectors::BLOCK_MAINNET_434873_BYTES.zcash_deserialize_into::>()?; let finalized_tree = - HistoryTree::new_from_block(Network::Mainnet, block.clone(), &Default::default(), None) + HistoryTree::from_block(Network::Mainnet, block.clone(), &Default::default(), None) .unwrap(); let less_block = block @@ -126,7 +126,7 @@ fn best_chain_wins_for_network(network: Network) -> Result<()> { } }; let finalized_tree = - HistoryTree::new_from_block(network, block1.clone(), &Default::default(), None).unwrap(); + HistoryTree::from_block(network, block1.clone(), &Default::default(), None).unwrap(); let block2 = block1 .make_fake_child() @@ -169,7 +169,7 @@ fn finalize_pops_from_best_chain_for_network(network: Network) -> Result<()> { } }; let finalized_tree = - HistoryTree::new_from_block(network, block0.clone(), &Default::default(), None).unwrap(); + HistoryTree::from_block(network, block0.clone(), &Default::default(), None).unwrap(); let block1 = block0 .make_fake_child() @@ -224,7 +224,7 @@ fn commit_block_extending_best_chain_doesnt_drop_worst_chains_for_network( } }; let finalized_tree = - HistoryTree::new_from_block(network, block0.clone(), &Default::default(), None).unwrap(); + HistoryTree::from_block(network, block0.clone(), &Default::default(), None).unwrap(); let block1 = block0 .make_fake_child() @@ -279,7 +279,7 @@ fn shorter_chain_can_be_best_chain_for_network(network: Network) -> Result<()> { } }; let finalized_tree = - HistoryTree::new_from_block(network, block0.clone(), &Default::default(), None).unwrap(); + HistoryTree::from_block(network, block0.clone(), &Default::default(), None).unwrap(); let block1 = block0 .make_fake_child() @@ -333,7 +333,7 @@ fn longer_chain_with_more_work_wins_for_network(network: Network) -> Result<()> } }; let finalized_tree = - HistoryTree::new_from_block(network, block0.clone(), &Default::default(), None).unwrap(); + HistoryTree::from_block(network, block0.clone(), &Default::default(), None).unwrap(); let block1 = block0 .make_fake_child() @@ -398,7 +398,7 @@ fn equal_length_goes_to_more_work_for_network(network: Network) -> Result<()> { } }; let finalized_tree = - HistoryTree::new_from_block(network, block0.clone(), &Default::default(), None).unwrap(); + HistoryTree::from_block(network, block0.clone(), &Default::default(), None).unwrap(); let block1 = block0 .make_fake_child() From 8dae6e73173b4e0e5e6d733b3bf22fda66196eea Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Fri, 2 Jul 2021 17:37:57 -0300 Subject: [PATCH 29/30] Improvements from code review --- zebra-chain/src/history_tree.rs | 8 +++ zebra-state/src/error.rs | 4 +- .../src/service/non_finalized_state/chain.rs | 26 +-------- .../service/non_finalized_state/tests/prop.rs | 6 +-- .../non_finalized_state/tests/vectors.rs | 54 ++++++++++--------- zebra-state/src/tests.rs | 6 +-- 6 files changed, 45 insertions(+), 59 deletions(-) diff --git a/zebra-chain/src/history_tree.rs b/zebra-chain/src/history_tree.rs index b53fc6aa25b..4f76527c83e 100644 --- a/zebra-chain/src/history_tree.rs +++ b/zebra-chain/src/history_tree.rs @@ -244,3 +244,11 @@ impl Clone for HistoryTree { } } } + +impl PartialEq for HistoryTree { + fn eq(&self, other: &Self) -> bool { + self.hash() == other.hash() + } +} + +impl Eq for HistoryTree {} diff --git a/zebra-state/src/error.rs b/zebra-state/src/error.rs index 9e2a28fd175..ad1f4236886 100644 --- a/zebra-state/src/error.rs +++ b/zebra-state/src/error.rs @@ -4,7 +4,7 @@ use chrono::{DateTime, Utc}; use thiserror::Error; use zebra_chain::{ - block::{self, ChainHistoryMmrRootHash, CommitmentError}, + block::{self, ChainHistoryMmrRootHash}, history_tree::HistoryTreeError, work::difficulty::CompactDifficulty, }; @@ -80,7 +80,7 @@ pub enum ValidateContextError { }, #[error("block contains an invalid commitment")] - InvalidCommitment(#[from] CommitmentError), + InvalidBlockCommitment(#[from] block::CommitmentError), #[error("block history commitment {candidate_commitment:?} is different to the expected commitment {expected_commitment:?}")] #[non_exhaustive] diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 6da14ae8f31..78162566eae 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -18,6 +18,7 @@ use zebra_chain::{ use crate::{PreparedBlock, Utxo, ValidateContextError}; +#[derive(PartialEq, Eq)] pub struct Chain { pub blocks: BTreeMap, pub height_by_hash: HashMap, @@ -191,23 +192,6 @@ impl Chain { history_tree, } } - - /// Return if two Chains are identical. - #[cfg(test)] - pub(crate) fn is_identical(&self, other: &Self) -> bool { - self.blocks == other.blocks - && self.height_by_hash == other.height_by_hash - && self.tx_by_hash == other.tx_by_hash - && self.created_utxos == other.created_utxos - && self.spent_utxos == other.spent_utxos - && self.sprout_anchors == other.sprout_anchors - && self.sapling_anchors == other.sapling_anchors - && self.sprout_nullifiers == other.sprout_nullifiers - && self.sapling_nullifiers == other.sapling_nullifiers - && self.orchard_nullifiers == other.orchard_nullifiers - && self.partial_cumulative_work == other.partial_cumulative_work - && self.history_root_hash() == other.history_root_hash() - } } /// Helper trait to organize inverse operations done on the `Chain` type. Used to @@ -544,14 +528,6 @@ impl UpdateWith> for Chain { } } -impl PartialEq for Chain { - fn eq(&self, other: &Self) -> bool { - self.partial_cmp(other) == Some(Ordering::Equal) - } -} - -impl Eq for Chain {} - impl PartialOrd for Chain { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) diff --git a/zebra-state/src/service/non_finalized_state/tests/prop.rs b/zebra-state/src/service/non_finalized_state/tests/prop.rs index eebeff7ff7c..c686f7f6ec8 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -34,14 +34,14 @@ fn forked_equals_pushed() -> Result<()> { let mut forked = full_chain.fork(fork_tip_hash, &finalized_tree).expect("fork works").expect("hash is present"); prop_assert_eq!(forked.blocks.len(), partial_chain.blocks.len()); - prop_assert!(forked.is_identical(&partial_chain)); + prop_assert!(forked == partial_chain); for block in chain.iter().skip(count) { forked.push(block.clone())?; } prop_assert_eq!(forked.blocks.len(), full_chain.blocks.len()); - prop_assert!(forked.is_identical(&full_chain)); + prop_assert!(forked == full_chain); }); Ok(()) @@ -80,7 +80,7 @@ fn finalized_equals_pushed() -> Result<()> { } prop_assert_eq!(full_chain.blocks.len(), partial_chain.blocks.len()); - prop_assert!(full_chain.is_identical(&partial_chain)); + prop_assert!(full_chain == partial_chain); }); Ok(()) diff --git a/zebra-state/src/service/non_finalized_state/tests/vectors.rs b/zebra-state/src/service/non_finalized_state/tests/vectors.rs index fa64099b8ab..b84b1f4977b 100644 --- a/zebra-state/src/service/non_finalized_state/tests/vectors.rs +++ b/zebra-state/src/service/non_finalized_state/tests/vectors.rs @@ -37,7 +37,7 @@ fn construct_single() -> Result<()> { let block1 = block0 .make_fake_child() - .set_commitment(finalized_tree.hash().into()); + .set_block_commitment(finalized_tree.hash().into()); let mut chain = Chain::new(finalized_tree); chain.push(block1.prepare())?; @@ -60,7 +60,9 @@ fn construct_many() -> Result<()> { let mut tree = finalized_tree.clone(); while blocks.len() < 100 { - let next_block = block.make_fake_child().set_commitment(tree.hash().into()); + let next_block = block + .make_fake_child() + .set_block_commitment(tree.hash().into()); blocks.push(next_block.clone()); block = next_block; tree = make_tree(block.clone(), &tree)?; @@ -89,11 +91,11 @@ fn ord_matches_work() -> Result<()> { let less_block = block .make_fake_child() .set_work(1) - .set_commitment(finalized_tree.hash().into()); + .set_block_commitment(finalized_tree.hash().into()); let more_block = block .make_fake_child() .set_work(10) - .set_commitment(finalized_tree.hash().into()); + .set_block_commitment(finalized_tree.hash().into()); let mut lesser_chain = Chain::new(finalized_tree.clone()); lesser_chain.push(less_block.prepare())?; @@ -131,11 +133,11 @@ fn best_chain_wins_for_network(network: Network) -> Result<()> { let block2 = block1 .make_fake_child() .set_work(10) - .set_commitment(finalized_tree.hash().into()); + .set_block_commitment(finalized_tree.hash().into()); let child = block1 .make_fake_child() .set_work(1) - .set_commitment(finalized_tree.hash().into()); + .set_block_commitment(finalized_tree.hash().into()); let expected_hash = block2.hash(); @@ -173,17 +175,17 @@ fn finalize_pops_from_best_chain_for_network(network: Network) -> Result<()> { let block1 = block0 .make_fake_child() - .set_commitment(finalized_tree.hash().into()); + .set_block_commitment(finalized_tree.hash().into()); let block1_tree = make_tree(block1.clone(), &finalized_tree)?; let block2 = block1 .make_fake_child() .set_work(10) - .set_commitment(block1_tree.hash().into()); + .set_block_commitment(block1_tree.hash().into()); let child = block1 .make_fake_child() .set_work(1) - .set_commitment(block1_tree.hash().into()); + .set_block_commitment(block1_tree.hash().into()); let mut state = NonFinalizedState::default(); state.commit_new_chain(block1.clone().prepare(), finalized_tree.clone())?; @@ -228,22 +230,22 @@ fn commit_block_extending_best_chain_doesnt_drop_worst_chains_for_network( let block1 = block0 .make_fake_child() - .set_commitment(finalized_tree.hash().into()); + .set_block_commitment(finalized_tree.hash().into()); let block1_tree = make_tree(block1.clone(), &finalized_tree)?; let block2 = block1 .make_fake_child() .set_work(10) - .set_commitment(block1_tree.hash().into()); + .set_block_commitment(block1_tree.hash().into()); let block2_tree = make_tree(block2.clone(), &block1_tree)?; let child1 = block1 .make_fake_child() .set_work(1) - .set_commitment(block1_tree.hash().into()); + .set_block_commitment(block1_tree.hash().into()); let child2 = block2 .make_fake_child() .set_work(1) - .set_commitment(block2_tree.hash().into()); + .set_block_commitment(block2_tree.hash().into()); let mut state = NonFinalizedState::default(); assert_eq!(0, state.chain_set.len()); @@ -283,23 +285,23 @@ fn shorter_chain_can_be_best_chain_for_network(network: Network) -> Result<()> { let block1 = block0 .make_fake_child() - .set_commitment(finalized_tree.hash().into()); + .set_block_commitment(finalized_tree.hash().into()); let block1_tree = make_tree(block1.clone(), &finalized_tree)?; let long_chain_block1 = block1 .make_fake_child() .set_work(1) - .set_commitment(block1_tree.hash().into()); + .set_block_commitment(block1_tree.hash().into()); let long_chain_block1_tree = make_tree(long_chain_block1.clone(), &block1_tree)?; let long_chain_block2 = long_chain_block1 .make_fake_child() .set_work(1) - .set_commitment(long_chain_block1_tree.hash().into()); + .set_block_commitment(long_chain_block1_tree.hash().into()); let short_chain_block = block1 .make_fake_child() .set_work(3) - .set_commitment(block1_tree.hash().into()); + .set_block_commitment(block1_tree.hash().into()); let mut state = NonFinalizedState::default(); state.commit_new_chain(block1.prepare(), finalized_tree.clone())?; @@ -337,33 +339,33 @@ fn longer_chain_with_more_work_wins_for_network(network: Network) -> Result<()> let block1 = block0 .make_fake_child() - .set_commitment(finalized_tree.hash().into()); + .set_block_commitment(finalized_tree.hash().into()); let block1_tree = make_tree(block1.clone(), &finalized_tree)?; let long_chain_block1 = block1 .make_fake_child() .set_work(1) - .set_commitment(block1_tree.hash().into()); + .set_block_commitment(block1_tree.hash().into()); let long_chain_block1_tree = make_tree(long_chain_block1.clone(), &block1_tree)?; let long_chain_block2 = long_chain_block1 .make_fake_child() .set_work(1) - .set_commitment(long_chain_block1_tree.hash().into()); + .set_block_commitment(long_chain_block1_tree.hash().into()); let long_chain_block2_tree = make_tree(long_chain_block2.clone(), &long_chain_block1_tree)?; let long_chain_block3 = long_chain_block2 .make_fake_child() .set_work(1) - .set_commitment(long_chain_block2_tree.hash().into()); + .set_block_commitment(long_chain_block2_tree.hash().into()); let long_chain_block3_tree = make_tree(long_chain_block3.clone(), &long_chain_block2_tree)?; let long_chain_block4 = long_chain_block3 .make_fake_child() .set_work(1) - .set_commitment(long_chain_block3_tree.hash().into()); + .set_block_commitment(long_chain_block3_tree.hash().into()); let short_chain_block = block1 .make_fake_child() .set_work(3) - .set_commitment(block1_tree.hash().into()); + .set_block_commitment(block1_tree.hash().into()); let mut state = NonFinalizedState::default(); state.commit_new_chain(block1.prepare(), finalized_tree.clone())?; @@ -402,17 +404,17 @@ fn equal_length_goes_to_more_work_for_network(network: Network) -> Result<()> { let block1 = block0 .make_fake_child() - .set_commitment(finalized_tree.hash().into()); + .set_block_commitment(finalized_tree.hash().into()); let block1_tree = make_tree(block1.clone(), &finalized_tree)?; let less_work_child = block1 .make_fake_child() .set_work(1) - .set_commitment(block1_tree.hash().into()); + .set_block_commitment(block1_tree.hash().into()); let more_work_child = block1 .make_fake_child() .set_work(3) - .set_commitment(block1_tree.hash().into()); + .set_block_commitment(block1_tree.hash().into()); let expected_hash = more_work_child.hash(); let mut state = NonFinalizedState::default(); diff --git a/zebra-state/src/tests.rs b/zebra-state/src/tests.rs index 7ee7b624986..e1b56630a28 100644 --- a/zebra-state/src/tests.rs +++ b/zebra-state/src/tests.rs @@ -39,7 +39,7 @@ pub trait FakeChainHelper { fn set_work(self, work: u128) -> Arc; - fn set_commitment(self, commitment: [u8; 32]) -> Arc; + fn set_block_commitment(self, commitment: [u8; 32]) -> Arc; } impl FakeChainHelper for Arc { @@ -77,9 +77,9 @@ impl FakeChainHelper for Arc { self } - fn set_commitment(mut self, commitment: [u8; 32]) -> Arc { + fn set_block_commitment(mut self, block_commitment: [u8; 32]) -> Arc { let block = Arc::make_mut(&mut self); - block.header.commitment_bytes = commitment; + block.header.commitment_bytes = block_commitment; self } } From 2e928db9fe7936dfc1049ea389f4008d9dd98ea6 Mon Sep 17 00:00:00 2001 From: "Conrado P. L. Gouvea" Date: Mon, 5 Jul 2021 10:04:35 -0300 Subject: [PATCH 30/30] Add Debug implementations to allow comparing Chains with proptest_assert_eq --- zebra-chain/src/history_tree.rs | 1 + zebra-chain/src/primitives/zcash_history.rs | 11 ++++++++++- zebra-state/src/service/non_finalized_state/chain.rs | 2 +- .../src/service/non_finalized_state/tests/prop.rs | 6 +++--- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/zebra-chain/src/history_tree.rs b/zebra-chain/src/history_tree.rs index 4f76527c83e..aea5209eba2 100644 --- a/zebra-chain/src/history_tree.rs +++ b/zebra-chain/src/history_tree.rs @@ -32,6 +32,7 @@ pub enum HistoryTreeError { /// History tree (Merkle mountain range) structure that contains information about // the block history, as specified in [ZIP-221][https://zips.z.cash/zip-0221]. +#[derive(Debug)] pub struct HistoryTree { network: Network, network_upgrade: NetworkUpgrade, diff --git a/zebra-chain/src/primitives/zcash_history.rs b/zebra-chain/src/primitives/zcash_history.rs index 6956b58b3f4..accd9568714 100644 --- a/zebra-chain/src/primitives/zcash_history.rs +++ b/zebra-chain/src/primitives/zcash_history.rs @@ -45,7 +45,7 @@ impl From<&zcash_history::NodeData> for NodeData { /// An encoded entry in the tree. /// /// Contains the node data and information about its position in the tree. -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct Entry { inner: [u8; zcash_history::MAX_ENTRY_SIZE], } @@ -231,6 +231,15 @@ impl Tree { } } +impl std::fmt::Debug for Tree { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Tree") + .field("network", &self.network) + .field("network_upgrade", &self.network_upgrade) + .finish() + } +} + /// Convert a Block into a zcash_history::NodeData used in the MMR tree. /// /// `sapling_root` is the root of the Sapling note commitment tree of the block. diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 78162566eae..cb41c673487 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -18,7 +18,7 @@ use zebra_chain::{ use crate::{PreparedBlock, Utxo, ValidateContextError}; -#[derive(PartialEq, Eq)] +#[derive(PartialEq, Eq, Debug)] pub struct Chain { pub blocks: BTreeMap, pub height_by_hash: HashMap, diff --git a/zebra-state/src/service/non_finalized_state/tests/prop.rs b/zebra-state/src/service/non_finalized_state/tests/prop.rs index c686f7f6ec8..b70cf8c787b 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -34,14 +34,14 @@ fn forked_equals_pushed() -> Result<()> { let mut forked = full_chain.fork(fork_tip_hash, &finalized_tree).expect("fork works").expect("hash is present"); prop_assert_eq!(forked.blocks.len(), partial_chain.blocks.len()); - prop_assert!(forked == partial_chain); + prop_assert_eq!(&forked, &partial_chain); for block in chain.iter().skip(count) { forked.push(block.clone())?; } prop_assert_eq!(forked.blocks.len(), full_chain.blocks.len()); - prop_assert!(forked == full_chain); + prop_assert_eq!(&forked, &full_chain); }); Ok(()) @@ -80,7 +80,7 @@ fn finalized_equals_pushed() -> Result<()> { } prop_assert_eq!(full_chain.blocks.len(), partial_chain.blocks.len()); - prop_assert!(full_chain == partial_chain); + prop_assert_eq!(&full_chain, &partial_chain); }); Ok(())