zecfoundation
/
zebra
mirror of https://github.com/ZcashFoundation/zebra.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix(state): Fix minute-long delays in block verification after a chain fork (#6122) 

* Split Chain fields into sections

* Replace Chain.sprout_note_commitment_tree with a lookup method

* Add TODOs

* Show full debug info when tests fail because chains aren't equal

* Print sprout and sapling tree Nodes as hex when debugging

* Correctly revert temporary finalized tip trees and anchors

* Fix tests

* Refactor removal functions

* Replace the Chain.sapling_note_commitment_tree field with a lookup method

* Replace the Chain.orchard_note_commitment_tree field with a lookup method

* Replace the Chain.history_tree field with a lookup method and remove redundant code

* Update comments

* Ignore clippy::unwrap_in_result

* Remove redundant fork() Result

* Put conditional code in blocks

* fastmod history_tree_at_tip history_block_commitment_tree zebra-state
This commit is contained in:
teor 2023-02-14 07:44:31 +10:00 committed by GitHub
parent 09faf48e1f
commit 9452487c61
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 594 additions and 402 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
zebra-chain/src/sapling/tree.rs
View File

@ -169,9 +169,15 @@ impl ZcashDeserialize for Root {
///
/// Note that it's handled as a byte buffer and not a point coordinate (jubjub::Fq)
/// because that's how the spec handles the MerkleCRH^Sapling function inputs and outputs.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
#[derive(Copy, Clone, Eq, PartialEq)]
struct Node([u8; 32]);
impl fmt::Debug for Node {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.debug_tuple("Node").field(&hex::encode(self.0)).finish()
}
}
/// Required to convert [`NoteCommitmentTree`] into [`SerializedTree`].
///
/// Zebra stores Sapling note commitment trees as [`Frontier`][1]s while the

10
zebra-chain/src/sprout/tree.rs
View File

@ -15,13 +15,13 @@ use std::{fmt, ops::Deref};
use byteorder::{BigEndian, ByteOrder};
use incrementalmerkletree::{bridgetree, Frontier};
use lazy_static::lazy_static;
use sha2::digest::generic_array::GenericArray;
use thiserror::Error;
use super::commitment::NoteCommitment;
#[cfg(any(test, feature = "proptest-impl"))]
use proptest_derive::Arbitrary;
use sha2::digest::generic_array::GenericArray;
/// Sprout note commitment trees have a max depth of 29.
///
@ -127,9 +127,15 @@ impl From<&Root> for [u8; 32] {
}
/// A node of the Sprout note commitment tree.
#[derive(Clone, Debug)]
#[derive(Clone, Copy, Eq, PartialEq)]
struct Node([u8; 32]);
impl fmt::Debug for Node {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.debug_tuple("Node").field(&hex::encode(self.0)).finish()
}
}
impl incrementalmerkletree::Hashable for Node {
/// Returns an empty leaf.
fn empty_leaf() -> Self {

58
zebra-state/src/service/non_finalized_state.rs
View File

@ -10,10 +10,8 @@ use std::{
use zebra_chain::{
block::{self, Block},
history_tree::HistoryTree,
orchard,
parameters::Network,
sapling, sprout, transparent,
sprout, transparent,
};
use crate::{
@ -157,13 +155,7 @@ impl NonFinalizedState {
let parent_hash = prepared.block.header.previous_block_hash;
let (height, hash) = (prepared.height, prepared.hash);
let parent_chain = self.parent_chain(
parent_hash,
finalized_state.sprout_note_commitment_tree(),
finalized_state.sapling_note_commitment_tree(),
finalized_state.orchard_note_commitment_tree(),
finalized_state.history_tree(),
)?;
let parent_chain = self.parent_chain(parent_hash)?;
// If the block is invalid, return the error,
// and drop the cloned parent Arc, or newly created chain fork.
@ -185,13 +177,23 @@ impl NonFinalizedState {
/// Commit block to the non-finalized state as a new chain where its parent
/// is the finalized tip.
#[tracing::instrument(level = "debug", skip(self, finalized_state, prepared))]
#[allow(clippy::unwrap_in_result)]
pub fn commit_new_chain(
&mut self,
prepared: PreparedBlock,
finalized_state: &ZebraDb,
) -> Result<(), ValidateContextError> {
let finalized_tip_height = finalized_state.finalized_tip_height();
// TODO: fix tests that don't initialize the finalized state
#[cfg(not(test))]
let finalized_tip_height = finalized_tip_height.expect("finalized state contains blocks");
#[cfg(test)]
let finalized_tip_height = finalized_tip_height.unwrap_or(zebra_chain::block::Height(0));
let chain = Chain::new(
self.network,
finalized_tip_height,
finalized_state.sprout_note_commitment_tree(),
finalized_state.sapling_note_commitment_tree(),
finalized_state.orchard_note_commitment_tree(),
@ -279,7 +281,7 @@ impl NonFinalizedState {
// Clone function arguments for different threads
let block = contextual.block.clone();
let network = new_chain.network();
let history_tree = new_chain.history_tree.clone();
let history_tree = new_chain.history_block_commitment_tree();
let block2 = contextual.block.clone();
let height = contextual.height;
@ -435,7 +437,10 @@ impl NonFinalizedState {
/// Returns `true` if the best chain contains `sapling_nullifier`.
#[cfg(test)]
pub fn best_contains_sapling_nullifier(&self, sapling_nullifier: &sapling::Nullifier) -> bool {
pub fn best_contains_sapling_nullifier(
&self,
sapling_nullifier: &zebra_chain::sapling::Nullifier,
) -> bool {
self.best_chain()
.map(|best_chain| best_chain.sapling_nullifiers.contains(sapling_nullifier))
.unwrap_or(false)
@ -443,7 +448,10 @@ impl NonFinalizedState {
/// Returns `true` if the best chain contains `orchard_nullifier`.
#[cfg(test)]
pub fn best_contains_orchard_nullifier(&self, orchard_nullifier: &orchard::Nullifier) -> bool {
pub fn best_contains_orchard_nullifier(
&self,
orchard_nullifier: &zebra_chain::orchard::Nullifier,
) -> bool {
self.best_chain()
.map(|best_chain| best_chain.orchard_nullifiers.contains(orchard_nullifier))
.unwrap_or(false)
@ -456,19 +464,12 @@ impl NonFinalizedState {
/// 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.
///
/// The trees must be the trees of the finalized tip.
/// They are used to recreate the trees if a fork is needed.
/// The chain can be an existing chain in the non-finalized state, or a freshly
/// created fork.
#[allow(clippy::unwrap_in_result)]
fn parent_chain(
&mut self,
parent_hash: block::Hash,
sprout_note_commitment_tree: Arc<sprout::tree::NoteCommitmentTree>,
sapling_note_commitment_tree: Arc<sapling::tree::NoteCommitmentTree>,
orchard_note_commitment_tree: Arc<orchard::tree::NoteCommitmentTree>,
history_tree: Arc<HistoryTree>,
) -> Result<Arc<Chain>, ValidateContextError> {
match self.find_chain(|chain| chain.non_finalized_tip_hash() == parent_hash) {
// Clone the existing Arc<Chain> in the non-finalized state
@ -480,18 +481,7 @@ impl NonFinalizedState {
let fork_chain = self
.chain_set
.iter()
.find_map(|chain| {
chain
.fork(
parent_hash,
sprout_note_commitment_tree.clone(),
sapling_note_commitment_tree.clone(),
orchard_note_commitment_tree.clone(),
history_tree.clone(),
)
.transpose()
})
.transpose()?
.find_map(|chain| chain.fork(parent_hash))
.ok_or(ValidateContextError::NotReadyToBeCommitted)?;
Ok(Arc::new(fork_chain))

786
zebra-state/src/service/non_finalized_state/chain.rs
View File

File diff suppressed because it is too large Load Diff

99
zebra-state/src/service/non_finalized_state/tests/prop.rs
View File

@ -6,7 +6,7 @@ use zebra_test::prelude::*;
use zebra_chain::{
amount::NonNegative,
block::{self, arbitrary::allow_all_transparent_coinbase_spends, Block},
block::{self, arbitrary::allow_all_transparent_coinbase_spends, Block, Height},
fmt::DisplayToDebug,
history_tree::{HistoryTree, NonEmptyHistoryTree},
parameters::NetworkUpgrade::*,
@ -47,7 +47,7 @@ fn push_genesis_chain() -> Result<()> {
|((chain, count, network, empty_tree) in PreparedChain::default())| {
prop_assert!(empty_tree.is_none());
let mut only_chain = Chain::new(network, Default::default(), Default::default(), Default::default(), empty_tree, ValueBalance::zero());
let mut only_chain = Chain::new(network, Height(0), Default::default(), Default::default(), Default::default(), empty_tree, ValueBalance::zero());
// contains the block value pool changes and chain value pool balances for each height
let mut chain_values = BTreeMap::new();
@ -99,7 +99,7 @@ fn push_history_tree_chain() -> Result<()> {
let count = std::cmp::min(count, chain.len() - 1);
let chain = &chain[1..];
let mut only_chain = Chain::new(network, Default::default(), Default::default(), Default::default(), finalized_tree, ValueBalance::zero());
let mut only_chain = Chain::new(network, Height(0), Default::default(), Default::default(), Default::default(), finalized_tree, ValueBalance::zero());
for block in chain
.iter()
@ -143,6 +143,7 @@ fn forked_equals_pushed_genesis() -> Result<()> {
// correspond to the blocks in the original chain before the fork.
let mut partial_chain = Chain::new(
network,
Height(0),
Default::default(),
Default::default(),
Default::default(),
@ -162,10 +163,11 @@ fn forked_equals_pushed_genesis() -> Result<()> {
// This chain will be forked.
let mut full_chain = Chain::new(
network,
Height(0),
Default::default(),
Default::default(),
Default::default(),
empty_tree.clone(),
empty_tree,
ValueBalance::zero(),
);
for block in chain.iter().cloned() {
@ -195,18 +197,12 @@ fn forked_equals_pushed_genesis() -> Result<()> {
}
// Use [`fork_at_count`] as the fork tip.
let fork_tip_hash = chain[fork_at_count - 1].hash;
let fork_tip_height = fork_at_count - 1;
let fork_tip_hash = chain[fork_tip_height].hash;
// Fork the chain.
let mut forked = full_chain
.fork(
fork_tip_hash,
Default::default(),
Default::default(),
Default::default(),
empty_tree,
)
.expect("fork works")
.fork(fork_tip_hash)
.expect("hash is present");
// This check is redundant, but it's useful for debugging.
@ -254,8 +250,8 @@ fn forked_equals_pushed_history_tree() -> Result<()> {
// use `fork_at_count` as the fork tip
let fork_tip_hash = chain[fork_at_count - 1].hash;
let mut full_chain = Chain::new(network, Default::default(), Default::default(), Default::default(), finalized_tree.clone(), ValueBalance::zero());
let mut partial_chain = Chain::new(network, Default::default(), Default::default(), Default::default(), finalized_tree.clone(), ValueBalance::zero());
let mut full_chain = Chain::new(network, Height(0), Default::default(), Default::default(), Default::default(), finalized_tree.clone(), ValueBalance::zero());
let mut partial_chain = Chain::new(network, Height(0), Default::default(), Default::default(), Default::default(), finalized_tree, ValueBalance::zero());
for block in chain
.iter()
@ -271,14 +267,7 @@ fn forked_equals_pushed_history_tree() -> Result<()> {
}
let mut forked = full_chain
.fork(
fork_tip_hash,
Default::default(),
Default::default(),
Default::default(),
finalized_tree,
)
.expect("fork works")
.fork(fork_tip_hash)
.expect("hash is present");
// the first check is redundant, but it's useful for debugging
@ -318,14 +307,17 @@ fn finalized_equals_pushed_genesis() -> Result<()> {
prop_assert!(empty_tree.is_none());
// TODO: fix this test or the code so the full_chain temporary trees aren't overwritten
let chain = chain.iter().filter(|block| block.height != Height(0));
// use `end_count` as the number of non-finalized blocks at the end of the chain
let finalized_count = chain.len() - end_count;
let finalized_count = chain.clone().count() - end_count;
let fake_value_pool = ValueBalance::<NonNegative>::fake_populated_pool();
let mut full_chain = Chain::new(network, Default::default(), Default::default(), Default::default(), empty_tree, fake_value_pool);
let mut full_chain = Chain::new(network, Height(0), Default::default(), Default::default(), Default::default(), empty_tree, fake_value_pool);
for block in chain
.iter()
.clone()
.take(finalized_count)
.map(ContextuallyValidBlock::test_with_zero_spent_utxos) {
full_chain = full_chain.push(block)?;
@ -333,21 +325,21 @@ fn finalized_equals_pushed_genesis() -> Result<()> {
let mut partial_chain = Chain::new(
network,
full_chain.sprout_note_commitment_tree.clone(),
full_chain.sapling_note_commitment_tree.clone(),
full_chain.orchard_note_commitment_tree.clone(),
full_chain.history_tree.clone(),
full_chain.non_finalized_tip_height(),
full_chain.sprout_note_commitment_tree(),
full_chain.sapling_note_commitment_tree(),
full_chain.orchard_note_commitment_tree(),
full_chain.history_block_commitment_tree(),
full_chain.chain_value_pools,
);
for block in chain
.iter()
.clone()
.skip(finalized_count)
.map(ContextuallyValidBlock::test_with_zero_spent_utxos) {
partial_chain = partial_chain.push(block.clone())?;
}
for block in chain
.iter()
.skip(finalized_count)
.map(ContextuallyValidBlock::test_with_zero_spent_utxos) {
full_chain = full_chain.push(block.clone())?;
@ -357,8 +349,18 @@ fn finalized_equals_pushed_genesis() -> Result<()> {
full_chain.pop_root();
}
// Make sure the temporary trees from finalized tip forks are removed.
// TODO: update the test or the code so this extra step isn't needed?
full_chain.pop_root();
partial_chain.pop_root();
prop_assert_eq!(full_chain.blocks.len(), partial_chain.blocks.len());
prop_assert!(full_chain.eq_internal_state(&partial_chain));
prop_assert!(
full_chain.eq_internal_state(&partial_chain),
"\n\
full chain:\n{full_chain:#?}\n\n\
partial chain:\n{partial_chain:#?}\n",
);
});
Ok(())
@ -393,7 +395,7 @@ fn finalized_equals_pushed_history_tree() -> Result<()> {
let fake_value_pool = ValueBalance::<NonNegative>::fake_populated_pool();
let mut full_chain = Chain::new(network, Default::default(), Default::default(), Default::default(), finalized_tree, fake_value_pool);
let mut full_chain = Chain::new(network, Height(0), Default::default(), Default::default(), Default::default(), finalized_tree, fake_value_pool);
for block in chain
.iter()
.take(finalized_count)
@ -403,10 +405,11 @@ fn finalized_equals_pushed_history_tree() -> Result<()> {
let mut partial_chain = Chain::new(
network,
full_chain.sprout_note_commitment_tree.clone(),
full_chain.sapling_note_commitment_tree.clone(),
full_chain.orchard_note_commitment_tree.clone(),
full_chain.history_tree.clone(),
Height(finalized_count.try_into().unwrap()),
full_chain.sprout_note_commitment_tree(),
full_chain.sapling_note_commitment_tree(),
full_chain.orchard_note_commitment_tree(),
full_chain.history_block_commitment_tree(),
full_chain.chain_value_pools,
);
@ -428,8 +431,18 @@ fn finalized_equals_pushed_history_tree() -> Result<()> {
full_chain.pop_root();
}
// Make sure the temporary trees from finalized tip forks are removed.
// TODO: update the test or the code so this extra step isn't needed?
full_chain.pop_root();
partial_chain.pop_root();
prop_assert_eq!(full_chain.blocks.len(), partial_chain.blocks.len());
prop_assert!(full_chain.eq_internal_state(&partial_chain));
prop_assert!(
full_chain.eq_internal_state(&partial_chain),
"\n\
full chain:\n{full_chain:#?}\n\n\
partial chain:\n{partial_chain:#?}\n",
);
});
Ok(())
@ -570,8 +583,8 @@ fn different_blocks_different_chains() -> Result<()> {
Default::default()
};
let chain1 = Chain::new(Network::Mainnet, Default::default(), Default::default(), Default::default(), finalized_tree1, ValueBalance::fake_populated_pool());
let chain2 = Chain::new(Network::Mainnet, Default::default(), Default::default(), Default::default(), finalized_tree2, ValueBalance::fake_populated_pool());
let chain1 = Chain::new(Network::Mainnet, Height(0), Default::default(), Default::default(), Default::default(), finalized_tree1, ValueBalance::fake_populated_pool());
let chain2 = Chain::new(Network::Mainnet, Height(0), Default::default(), Default::default(), Default::default(), finalized_tree2, ValueBalance::fake_populated_pool());
let block1 = vec1[1].clone().prepare().test_with_zero_spent_utxos();
let block2 = vec2[1].clone().prepare().test_with_zero_spent_utxos();
@ -606,16 +619,12 @@ fn different_blocks_different_chains() -> Result<()> {
chain1.spent_utxos = chain2.spent_utxos.clone();
// note commitment trees
chain1.sprout_note_commitment_tree = chain2.sprout_note_commitment_tree.clone();
chain1.sprout_trees_by_anchor = chain2.sprout_trees_by_anchor.clone();
chain1.sprout_trees_by_height = chain2.sprout_trees_by_height.clone();
chain1.sapling_note_commitment_tree = chain2.sapling_note_commitment_tree.clone();
chain1.sapling_trees_by_height = chain2.sapling_trees_by_height.clone();
chain1.orchard_note_commitment_tree = chain2.orchard_note_commitment_tree.clone();
chain1.orchard_trees_by_height = chain2.orchard_trees_by_height.clone();
// history trees
chain1.history_tree = chain2.history_tree.clone();
chain1.history_trees_by_height = chain2.history_trees_by_height.clone();
// anchors

35
zebra-state/src/service/non_finalized_state/tests/vectors.rs
View File

@ -4,7 +4,7 @@ use std::sync::Arc;
use zebra_chain::{
amount::NonNegative,
block::Block,
block::{Block, Height},
history_tree::NonEmptyHistoryTree,
parameters::{Network, NetworkUpgrade},
serialization::ZcashDeserializeInto,
@ -27,6 +27,7 @@ fn construct_empty() {
let _init_guard = zebra_test::init();
let _chain = Chain::new(
Network::Mainnet,
Height(0),
Default::default(),
Default::default(),
Default::default(),
@ -43,6 +44,7 @@ fn construct_single() -> Result<()> {
let mut chain = Chain::new(
Network::Mainnet,
Height(0),
Default::default(),
Default::default(),
Default::default(),
@ -73,6 +75,7 @@ fn construct_many() -> Result<()> {
let mut chain = Chain::new(
Network::Mainnet,
Height(block.coinbase_height().unwrap().0 - 1),
Default::default(),
Default::default(),
Default::default(),
@ -99,6 +102,7 @@ fn ord_matches_work() -> Result<()> {
let mut lesser_chain = Chain::new(
Network::Mainnet,
Height(0),
Default::default(),
Default::default(),
Default::default(),
@ -109,6 +113,7 @@ fn ord_matches_work() -> Result<()> {
let mut bigger_chain = Chain::new(
Network::Mainnet,
Height(0),
Default::default(),
Default::default(),
Default::default(),
@ -434,12 +439,12 @@ fn history_tree_is_updated_for_network_upgrade(
let chain = state.best_chain().unwrap();
if network_upgrade == NetworkUpgrade::Heartwood {
assert!(
chain.history_tree.as_ref().is_none(),
chain.history_block_commitment_tree().as_ref().is_none(),
"history tree must not exist yet"
);
} else {
assert!(
chain.history_tree.as_ref().is_some(),
chain.history_block_commitment_tree().as_ref().is_some(),
"history tree must already exist"
);
}
@ -453,11 +458,16 @@ fn history_tree_is_updated_for_network_upgrade(
let chain = state.best_chain().unwrap();
assert!(
chain.history_tree.as_ref().is_some(),
chain.history_block_commitment_tree().as_ref().is_some(),
"history tree must have been (re)created"
);
assert_eq!(
chain.history_tree.as_ref().as_ref().unwrap().size(),
chain
.history_block_commitment_tree()
.as_ref()
.as_ref()
.unwrap()
.size(),
1,
"history tree must have a single node"
);
@ -466,8 +476,8 @@ fn history_tree_is_updated_for_network_upgrade(
let tree = NonEmptyHistoryTree::from_block(
Network::Mainnet,
activation_block.clone(),
&chain.sapling_note_commitment_tree.root(),
&chain.orchard_note_commitment_tree.root(),
&chain.sapling_note_commitment_tree().root(),
&chain.orchard_note_commitment_tree().root(),
)
.unwrap();
@ -480,7 +490,12 @@ fn history_tree_is_updated_for_network_upgrade(
.unwrap();
assert!(
state.best_chain().unwrap().history_tree.as_ref().is_some(),
state
.best_chain()
.unwrap()
.history_block_commitment_tree()
.as_ref()
.is_some(),
"history tree must still exist"
);
@ -541,8 +556,8 @@ fn commitment_is_validated_for_network_upgrade(network: Network, network_upgrade
let tree = NonEmptyHistoryTree::from_block(
Network::Mainnet,
activation_block.clone(),
&chain.sapling_note_commitment_tree.root(),
&chain.orchard_note_commitment_tree.root(),
&chain.sapling_note_commitment_tree().root(),
&chain.orchard_note_commitment_tree().root(),
)
.unwrap();