Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZIP-221: Validate chain history commitments in the non-finalized state #2301

Merged
merged 38 commits into from
Jul 7, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
5c7d941
sketch of implementation
conradoplg Jun 14, 2021
5226145
refined implementation; still incomplete
conradoplg Jun 16, 2021
3abbfa5
update librustzcash, change zcash_history to work with it
conradoplg Jun 17, 2021
e929f3f
simplified code per review; renamed MMR to HistoryTree
conradoplg Jun 17, 2021
c1dd319
expand HistoryTree implementation
conradoplg Jun 17, 2021
e290fbd
Merge remote-tracking branch 'origin/main' into zip221-non-finalized-…
conradoplg Jun 17, 2021
edc3fe0
Merge remote-tracking branch 'origin/main' into zip221-non-finalized-…
conradoplg Jun 21, 2021
59b46fe
handle and propagate errors
conradoplg Jun 21, 2021
d06dfcc
simplify check.rs tracing
conradoplg Jun 21, 2021
1cf9013
add suggested TODO
conradoplg Jun 21, 2021
89c12c6
add HistoryTree::prune
conradoplg Jun 21, 2021
b3d773a
fix bug in pruning
conradoplg Jun 22, 2021
e2ab46c
fix compilation of tests; still need to make them pass
conradoplg Jun 22, 2021
c52a66d
Apply suggestions from code review
conradoplg Jun 23, 2021
e977cd0
Apply suggestions from code review
conradoplg Jun 23, 2021
030240f
improvements from code review
conradoplg Jun 23, 2021
e416ef6
improve check.rs comments and variable names
conradoplg Jun 23, 2021
23e9bc2
fix HistoryTree which should use BTreeMap and not HashMap; fix non_fi…
conradoplg Jun 24, 2021
cf339a3
fix finalized_state proptest
conradoplg Jun 25, 2021
2271d73
fix non_finalized_state tests by setting the correct commitments
conradoplg Jun 25, 2021
13e50e2
renamed mmr.rs to history_tree.rs
conradoplg Jun 25, 2021
e5a62e1
Merge remote-tracking branch 'origin/main' into zip221-non-finalized-…
conradoplg Jun 25, 2021
7837704
Add HistoryTree struct
conradoplg Jun 25, 2021
985c6aa
expand non_finalized_state protest
conradoplg Jun 28, 2021
d9a36ea
Merge branch 'history-tree' into zip221-non-finalized-state
conradoplg Jun 28, 2021
5249335
fix typo
conradoplg Jun 28, 2021
787cc60
Merge remote-tracking branch 'origin/main' into zip221-non-finalized-…
conradoplg Jun 29, 2021
fca06e6
Add HistoryTree struct
conradoplg Jun 25, 2021
fa847b8
Update zebra-chain/src/primitives/zcash_history.rs
conradoplg Jun 29, 2021
8e38ed4
fix formatting
conradoplg Jun 29, 2021
f154696
Apply suggestions from code review
conradoplg Jun 29, 2021
81b0ddb
history_tree.rs: fixes from code review
conradoplg Jun 29, 2021
63d4fb0
Merge remote-tracking branch 'origin/main' into zip221-non-finalized-…
conradoplg Jul 1, 2021
590a0d7
Merge branch 'history-tree' into zip221-non-finalized-state
conradoplg Jul 1, 2021
608c2e2
fixes to work with updated HistoryTree
conradoplg Jul 1, 2021
8dae6e7
Improvements from code review
conradoplg Jul 2, 2021
2e928db
Add Debug implementations to allow comparing Chains with proptest_ass…
conradoplg Jul 5, 2021
e9add63
Merge remote-tracking branch 'origin/main' into zip221-non-finalized-…
conradoplg Jul 6, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions zebra-state/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?
},
}
4 changes: 2 additions & 2 deletions zebra-state/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
37 changes: 36 additions & 1 deletion 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},
block::{self, Block, ChainHistoryMmrRootHash},
parameters::POW_AVERAGING_WINDOW,
parameters::{Network, NetworkUpgrade},
work::difficulty::CompactDifficulty,
Expand Down Expand Up @@ -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(
Expand Down
20 changes: 17 additions & 3 deletions zebra-state/src/service/non_finalized_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);

Expand All @@ -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.
Expand Down
20 changes: 18 additions & 2 deletions zebra-state/src/service/non_finalized_state/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -27,6 +32,7 @@ pub struct Chain {
sapling_nullifiers: HashSet<sapling::Nullifier>,
orchard_nullifiers: HashSet<orchard::Nullifier>,
partial_cumulative_work: PartialCumulativeWork,
// TODO: MMR tree
}

impl Chain {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -160,6 +170,8 @@ impl UpdateWith<PreparedBlock> 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
Expand Down Expand Up @@ -241,6 +253,10 @@ impl UpdateWith<PreparedBlock> 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())
Expand Down