Skip to content

Commit

Permalink
Revert "ZIP-221: Validate chain history commitments in the non-finali…
Browse files Browse the repository at this point in the history
…zed state (#2301)"

This reverts commit 91b1fcb.
  • Loading branch information
teor2345 authored Jul 8, 2021
1 parent f817df6 commit 8321c40
Show file tree
Hide file tree
Showing 13 changed files with 114 additions and 487 deletions.
6 changes: 0 additions & 6 deletions zebra-chain/src/block/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,6 @@ impl From<[u8; 32]> for ChainHistoryMmrRootHash {
}
}

impl From<ChainHistoryMmrRootHash> 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
Expand Down
9 changes: 0 additions & 9 deletions zebra-chain/src/history_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ 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,
Expand Down Expand Up @@ -245,11 +244,3 @@ impl Clone for HistoryTree {
}
}
}

impl PartialEq for HistoryTree {
fn eq(&self, other: &Self) -> bool {
self.hash() == other.hash()
}
}

impl Eq for HistoryTree {}
11 changes: 1 addition & 10 deletions zebra-chain/src/primitives/zcash_history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, Debug)]
#[derive(Clone)]
pub struct Entry {
inner: [u8; zcash_history::MAX_ENTRY_SIZE],
}
Expand Down Expand Up @@ -231,15 +231,6 @@ 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.
Expand Down
19 changes: 1 addition & 18 deletions zebra-state/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@ use std::sync::Arc;
use chrono::{DateTime, Utc};
use thiserror::Error;

use zebra_chain::{
block::{self, ChainHistoryMmrRootHash},
history_tree::HistoryTreeError,
work::difficulty::CompactDifficulty,
};
use zebra_chain::{block, work::difficulty::CompactDifficulty};

/// A wrapper for type erased errors that is itself clonable and implements the
/// Error trait
Expand Down Expand Up @@ -78,17 +74,4 @@ pub enum ValidateContextError {
difficulty_threshold: CompactDifficulty,
expected_difficulty: CompactDifficulty,
},

#[error("block contains an invalid commitment")]
InvalidBlockCommitment(#[from] block::CommitmentError),

#[error("block history commitment {candidate_commitment:?} is different to the expected commitment {expected_commitment:?}")]
#[non_exhaustive]
InvalidHistoryCommitment {
candidate_commitment: ChainHistoryMmrRootHash,
expected_commitment: ChainHistoryMmrRootHash,
},

#[error("error building the history tree")]
HistoryTreeError(#[from] HistoryTreeError),
}
7 changes: 3 additions & 4 deletions zebra-state/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,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.disk.history_tree().clone())?;
self.mem.commit_new_chain(prepared)?;
} else {
self.mem.commit_block(prepared, self.disk.history_tree())?;
self.mem.commit_block(prepared)?;
}

Ok(())
Expand Down Expand Up @@ -223,7 +222,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_valid_for_recent_chain(
check::block_is_contextually_valid(
prepared,
self.network,
self.disk.finalized_tip_height(),
Expand Down
34 changes: 2 additions & 32 deletions zebra-state/src/service/arbitrary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,29 +50,6 @@ 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<Option<(Network, Arc<SummaryDebug<Vec<PreparedBlock>>>)>>,
// the height from which to start the chain. If None, starts at the genesis block
start_height: Option<Height>,
}

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 = NetworkUpgrade::Heartwood
.activation_height(Network::Mainnet)
.expect("must have height");
let test_height = NetworkUpgrade::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 {
Expand All @@ -83,12 +60,7 @@ 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 = match self.start_height {
Some(start_height) => {
LedgerState::height_strategy(start_height, NetworkUpgrade::Nu5, None, false)
}
None => LedgerState::genesis_strategy(NetworkUpgrade::Nu5, None, false),
};
let ledger_strategy = LedgerState::genesis_strategy(NetworkUpgrade::Nu5, None, false);

let (network, blocks) = ledger_strategy
.prop_flat_map(|ledger| {
Expand All @@ -111,9 +83,7 @@ impl Strategy for PreparedChain {
}

let chain = chain.clone().expect("should be generated");
// `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)?;
let count = (1..chain.1.len()).new_tree(runner)?;
Ok(PreparedChainTree {
chain: chain.1,
count,
Expand Down
51 changes: 9 additions & 42 deletions zebra-state/src/service/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::borrow::Borrow;

use chrono::Duration;
use zebra_chain::{
block::{self, Block, ChainHistoryMmrRootHash},
block::{self, Block},
parameters::POW_AVERAGING_WINDOW,
parameters::{Network, NetworkUpgrade},
work::difficulty::CompactDifficulty,
Expand All @@ -18,11 +18,8 @@ use difficulty::{AdjustedDifficulty, POW_MEDIAN_BLOCK_SPAN};

pub(crate) mod difficulty;

/// 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.
/// Check that `block` is contextually valid for `network`, based on the
/// `finalized_tip_height` and `relevant_chain`.
///
/// The relevant chain is an iterator over the ancestors of `block`, starting
/// with its parent block.
Expand All @@ -31,8 +28,12 @@ pub(crate) mod difficulty;
///
/// If the state contains less than 28
/// (`POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN`) blocks.
#[tracing::instrument(skip(prepared, finalized_tip_height, relevant_chain))]
pub(crate) fn block_is_valid_for_recent_chain<C>(
#[tracing::instrument(
name = "contextual_validation",
fields(?network),
skip(prepared, network, finalized_tip_height, relevant_chain)
)]
pub(crate) fn block_is_contextually_valid<C>(
prepared: &PreparedBlock,
network: Network,
finalized_tip_height: Option<block::Height>,
Expand Down Expand Up @@ -85,40 +86,6 @@ 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,
history_root_hash: &ChainHistoryMmrRootHash,
) -> Result<(), ValidateContextError> {
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_history_root_hash) => {
if block_history_root_hash == *history_root_hash {
Ok(())
} else {
Err(ValidateContextError::InvalidHistoryCommitment {
candidate_commitment: block_history_root_hash,
expected_commitment: *history_root_hash,
})
}
}
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(
Expand Down
6 changes: 0 additions & 6 deletions zebra-state/src/service/finalized_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ mod tests;

use std::{collections::HashMap, convert::TryInto, path::Path, sync::Arc};

use zebra_chain::history_tree::HistoryTree;
use zebra_chain::transparent;
use zebra_chain::{
block::{self, Block},
Expand Down Expand Up @@ -379,11 +378,6 @@ impl FinalizedState {
})
}

/// 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.
fn delete_ephemeral(&self) {
if self.ephemeral {
Expand Down
40 changes: 5 additions & 35 deletions zebra-state/src/service/non_finalized_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use std::{collections::BTreeSet, mem, ops::Deref, sync::Arc};

use zebra_chain::{
block::{self, Block},
history_tree::HistoryTree,
parameters::Network,
transaction::{self, Transaction},
transparent,
Expand All @@ -24,8 +23,6 @@ use crate::{FinalizedBlock, HashOrHeight, PreparedBlock, Utxo, ValidateContextEr

use self::chain::Chain;

use super::check;

/// The state of the chains in memory, incuding queued blocks.
#[derive(Default)]
pub struct NonFinalizedState {
Expand Down Expand Up @@ -77,14 +74,7 @@ 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,
finalized_tip_history_tree: &HistoryTree,
) -> Result<(), ValidateContextError> {
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);

Expand All @@ -95,12 +85,8 @@ impl NonFinalizedState {
);
}

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(),
)?;
let mut parent_chain = self.parent_chain(parent_hash)?;

parent_chain.push(prepared)?;
self.chain_set.insert(parent_chain);
self.update_metrics_for_committed_block(height, hash);
Expand All @@ -109,20 +95,12 @@ 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,
finalized_tip_history_tree: HistoryTree,
) -> Result<(), ValidateContextError> {
let mut chain = Chain::new(finalized_tip_history_tree);
let mut chain = Chain::default();
let (height, hash) = (prepared.height, prepared.hash);
check::block_commitment_is_valid_for_chain_history(
&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);
Expand Down Expand Up @@ -268,13 +246,9 @@ impl NonFinalizedState {
///
/// 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<Box<Chain>, ValidateContextError> {
match self.take_chain_if(|chain| chain.non_finalized_tip_hash() == parent_hash) {
// An existing chain in the non-finalized state
Expand All @@ -283,11 +257,7 @@ impl NonFinalizedState {
None => Ok(Box::new(
self.chain_set
.iter()
.find_map(|chain| {
chain
.fork(parent_hash, finalized_tip_history_tree)
.transpose()
})
.find_map(|chain| chain.fork(parent_hash).transpose())
.expect(
"commit_block is only called with blocks that are ready to be commited",
)?,
Expand Down
Loading

0 comments on commit 8321c40

Please sign in to comment.