From bb9b1294dc825fd9b4f3a8ee80db1da9cf37ebd3 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 19 Jul 2022 13:27:54 +1000 Subject: [PATCH 01/22] Split disk reads from CPU-heavy Sprout interstitial tree cryptography --- zebra-state/src/service/check/anchors.rs | 182 +++++++++++------- .../src/service/non_finalized_state.rs | 22 ++- 2 files changed, 129 insertions(+), 75 deletions(-) diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index ab62f03ae95..cdd99209c2d 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -10,63 +10,33 @@ use crate::{ PreparedBlock, ValidateContextError, }; -/// Checks that the Sprout, Sapling, and Orchard anchors specified by -/// transactions in this block have been computed previously within the context -/// of its parent chain. We do not check any anchors in checkpointed blocks, -/// which avoids JoinSplits +/// Checks the final Sapling and Orchard anchors specified by transactions in this +/// `prepared` block. /// -/// Sprout anchors may refer to some earlier block's final treestate (like -/// Sapling and Orchard do exclusively) _or_ to the interstitial output -/// treestate of any prior `JoinSplit` _within the same transaction_. +/// This method checks for anchors computed from the final treestate of each block in +/// the `parent_chain` or `finalized_state`. +/// +/// Sprout anchors may also refer to the interstitial output treestate of any prior +/// `JoinSplit` _within the same transaction_. +/// +/// This function fetches and returns the Sprout final treestates from the state, +/// so [`sprout_anchors_refer_to_interstitial_treestates()`] can check Sprout +/// final and interstitial treestates without accessing the disk. #[tracing::instrument(skip(finalized_state, parent_chain, prepared))] -pub(crate) fn anchors_refer_to_earlier_treestates( +pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( finalized_state: &ZebraDb, parent_chain: &Chain, prepared: &PreparedBlock, -) -> Result<(), ValidateContextError> { +) -> Result>, ValidateContextError> +{ + let mut sprout_final_treestates = HashMap::new(); + for transaction in prepared.block.transactions.iter() { // Sprout JoinSplits, with interstitial treestates to check as well. if transaction.has_sprout_joinsplit_data() { - let mut interstitial_trees: HashMap< - sprout::tree::Root, - Arc, - > = HashMap::new(); - for joinsplit in transaction.sprout_groth16_joinsplits() { - // Check all anchor sets, including the one for interstitial - // anchors. - // - // The anchor is checked and the matching tree is obtained, - // which is used to create the interstitial tree state for this - // JoinSplit: - // - // > For each JoinSplit description in a transaction, an - // > interstitial output treestate is constructed which adds the - // > note commitments and nullifiers specified in that JoinSplit - // > description to the input treestate referred to by its - // > anchor. This interstitial output treestate is available for - // > use as the anchor of subsequent JoinSplit descriptions in - // > the same transaction. - // - // - // - // # Consensus - // - // > The anchor of each JoinSplit description in a transaction - // > MUST refer to either some earlier block’s final Sprout - // > treestate, or to the interstitial output treestate of any - // > prior JoinSplit description in the same transaction. - // - // > For the first JoinSplit description of a transaction, the - // > anchor MUST be the output Sprout treestate of a previous - // > block. - // - // - // - // Note that in order to satisfy the latter consensus rule above, - // [`interstitial_trees`] is always empty in the first iteration - // of the loop. - let input_tree = interstitial_trees + // Fetch and return final anchor sets, avoiding duplicate fetches + let input_tree = sprout_final_treestates .get(&joinsplit.anchor) .cloned() .or_else(|| { @@ -80,30 +50,11 @@ pub(crate) fn anchors_refer_to_earlier_treestates( }) }); - let mut input_tree = match input_tree { - Some(tree) => tree, - None => { - tracing::warn!(?joinsplit.anchor, ?prepared.height, ?prepared.hash, "failed to find sprout anchor"); - return Err(ValidateContextError::UnknownSproutAnchor { - anchor: joinsplit.anchor, - }); - } - }; - - let input_tree_inner = Arc::make_mut(&mut input_tree); - - tracing::debug!(?joinsplit.anchor, "validated sprout anchor"); - - // Add new anchors to the interstitial note commitment tree. - for cm in joinsplit.commitments { - input_tree_inner - .append(cm) - .expect("note commitment should be appendable to the tree"); + if let Some(input_tree) = input_tree { + sprout_final_treestates.insert(input_tree.root(), input_tree); } - interstitial_trees.insert(input_tree.root(), input_tree); - - tracing::debug!(?joinsplit.anchor, "observed sprout anchor"); + tracing::debug!(?joinsplit.anchor, "observed sprout final treestate anchor"); } } @@ -166,5 +117,96 @@ pub(crate) fn anchors_refer_to_earlier_treestates( } } + Ok(sprout_final_treestates) +} + +/// Checks the Sprout anchors specified by transactions in this `prepared` block. +/// +/// Sprout anchors may refer to some earlier block's final treestate (like +/// Sapling and Orchard do exclusively) _or_ to the interstitial output +/// treestate of any prior `JoinSplit` _within the same transaction_. +/// +/// This method checks for anchors computed from the final and interstitial treestates of +/// each transaction in the `prepared` block, using the supplied `sprout_final_treestates`. +#[tracing::instrument(skip(sprout_final_treestates, prepared))] +pub(crate) fn sprout_anchors_refer_to_treestates( + sprout_final_treestates: HashMap>, + prepared: &PreparedBlock, +) -> Result<(), ValidateContextError> { + for transaction in prepared.block.transactions.iter() { + // Sprout JoinSplits, with interstitial treestates to check as well. + if transaction.has_sprout_joinsplit_data() { + let mut interstitial_trees: HashMap< + sprout::tree::Root, + Arc, + > = HashMap::new(); + + for joinsplit in transaction.sprout_groth16_joinsplits() { + // Check all anchor sets, including the one for interstitial + // anchors. + // + // The anchor is checked and the matching tree is obtained, + // which is used to create the interstitial tree state for this + // JoinSplit: + // + // > For each JoinSplit description in a transaction, an + // > interstitial output treestate is constructed which adds the + // > note commitments and nullifiers specified in that JoinSplit + // > description to the input treestate referred to by its + // > anchor. This interstitial output treestate is available for + // > use as the anchor of subsequent JoinSplit descriptions in + // > the same transaction. + // + // + // + // # Consensus + // + // > The anchor of each JoinSplit description in a transaction + // > MUST refer to either some earlier block’s final Sprout + // > treestate, or to the interstitial output treestate of any + // > prior JoinSplit description in the same transaction. + // + // > For the first JoinSplit description of a transaction, the + // > anchor MUST be the output Sprout treestate of a previous + // > block. + // + // + // + // Note that in order to satisfy the latter consensus rule above, + // [`interstitial_trees`] is always empty in the first iteration + // of the loop. + let input_tree = interstitial_trees + .get(&joinsplit.anchor) + .cloned() + .or_else(|| sprout_final_treestates.get(&joinsplit.anchor).cloned()); + + let mut input_tree = match input_tree { + Some(tree) => tree, + None => { + tracing::debug!(?joinsplit.anchor, ?prepared.height, ?prepared.hash, "failed to find sprout anchor"); + return Err(ValidateContextError::UnknownSproutAnchor { + anchor: joinsplit.anchor, + }); + } + }; + + let input_tree_inner = Arc::make_mut(&mut input_tree); + + tracing::debug!(?joinsplit.anchor, "validated sprout anchor"); + + // Add new anchors to the interstitial note commitment tree. + for cm in joinsplit.commitments { + input_tree_inner + .append(cm) + .expect("note commitment should be appendable to the tree"); + } + + interstitial_trees.insert(input_tree.root(), input_tree); + + tracing::debug!(?joinsplit.anchor, "observed sprout anchor"); + } + } + } + Ok(()) } diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 251582a829c..a2c7fc96c90 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -205,6 +205,9 @@ impl NonFinalizedState { prepared: PreparedBlock, finalized_state: &ZebraDb, ) -> Result, ValidateContextError> { + // Reads from disk + // + // TODO: if these disk reads show up in profiles, run them in parallel, using std::thread::spawn() let spent_utxos = check::utxo::transparent_spend( &prepared, &new_chain.unspent_utxos(), @@ -212,18 +215,25 @@ impl NonFinalizedState { finalized_state, )?; + // CPU-heavy cryptography check::prepared_block_commitment_is_valid_for_chain_history( &prepared, self.network, &new_chain.history_tree, )?; - check::anchors::anchors_refer_to_earlier_treestates( - finalized_state, - &new_chain, - &prepared, - )?; + // Reads from disk + let sprout_final_treestates = + check::anchors::sapling_orchard_anchors_refer_to_final_treestates( + finalized_state, + &new_chain, + &prepared, + )?; + + // CPU-heavy cryptography + check::anchors::sprout_anchors_refer_to_treestates(sprout_final_treestates, &prepared)?; + // Quick check that doesn't read from disk let contextual = ContextuallyValidBlock::with_block_and_spent_utxos( prepared.clone(), spent_utxos.clone(), @@ -240,6 +250,8 @@ impl NonFinalizedState { // We're pretty sure the new block is valid, // so clone the inner chain if needed, then add the new block. + // + // CPU-heavy cryptography, multiple batches? Arc::try_unwrap(new_chain) .unwrap_or_else(|shared_chain| (*shared_chain).clone()) .push(contextual) From cb4436e0fa530c615ff0a55e854ace69a512789d Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Jul 2022 12:23:58 +1000 Subject: [PATCH 02/22] Improve anchor validation debugging and error messages --- zebra-state/src/error.rs | 42 +++++-- zebra-state/src/service/check/anchors.rs | 136 ++++++++++++++++++----- 2 files changed, 142 insertions(+), 36 deletions(-) diff --git a/zebra-state/src/error.rs b/zebra-state/src/error.rs index 4b5169ff26c..3256f795a11 100644 --- a/zebra-state/src/error.rs +++ b/zebra-state/src/error.rs @@ -214,13 +214,13 @@ pub enum ValidateContextError { height: Option, }, - #[error("error in Sprout note commitment tree")] + #[error("error updating Sprout note commitment tree")] SproutNoteCommitmentTreeError(#[from] zebra_chain::sprout::tree::NoteCommitmentTreeError), - #[error("error in Sapling note commitment tree")] + #[error("error updating Sapling note commitment tree")] SaplingNoteCommitmentTreeError(#[from] zebra_chain::sapling::tree::NoteCommitmentTreeError), - #[error("error in Orchard note commitment tree")] + #[error("error updating Orchard note commitment tree")] OrchardNoteCommitmentTreeError(#[from] zebra_chain::orchard::tree::NoteCommitmentTreeError), #[error("error building the history tree")] @@ -229,17 +229,41 @@ pub enum ValidateContextError { #[error("block contains an invalid commitment")] InvalidBlockCommitment(#[from] block::CommitmentError), - #[error("unknown Sprout anchor: {anchor:?}")] + #[error( + "unknown Sprout anchor: {anchor:?},\n\ + {height:?}, index in block: {tx_index_in_block:?}, {transaction_hash:?}" + )] #[non_exhaustive] - UnknownSproutAnchor { anchor: sprout::tree::Root }, + UnknownSproutAnchor { + anchor: sprout::tree::Root, + height: block::Height, + tx_index_in_block: usize, + transaction_hash: transaction::Hash, + }, - #[error("unknown Sapling anchor: {anchor:?}")] + #[error( + "unknown Sapling anchor: {anchor:?},\n\ + {height:?}, index in block: {tx_index_in_block:?}, {transaction_hash:?}" + )] #[non_exhaustive] - UnknownSaplingAnchor { anchor: sapling::tree::Root }, + UnknownSaplingAnchor { + anchor: sapling::tree::Root, + height: block::Height, + tx_index_in_block: usize, + transaction_hash: transaction::Hash, + }, - #[error("unknown Orchard anchor: {anchor:?}")] + #[error( + "unknown Orchard anchor: {anchor:?},\n\ + {height:?}, index in block: {tx_index_in_block:?}, {transaction_hash:?}" + )] #[non_exhaustive] - UnknownOrchardAnchor { anchor: orchard::tree::Root }, + UnknownOrchardAnchor { + anchor: orchard::tree::Root, + height: block::Height, + tx_index_in_block: usize, + transaction_hash: transaction::Hash, + }, } /// Trait for creating the corresponding duplicate nullifier error from a nullifier. diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index cdd99209c2d..b34899cf1c2 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -31,30 +31,36 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( { let mut sprout_final_treestates = HashMap::new(); - for transaction in prepared.block.transactions.iter() { + for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() { // Sprout JoinSplits, with interstitial treestates to check as well. if transaction.has_sprout_joinsplit_data() { - for joinsplit in transaction.sprout_groth16_joinsplits() { - // Fetch and return final anchor sets, avoiding duplicate fetches - let input_tree = sprout_final_treestates + for (joinsplit_index_in_tx, joinsplit) in + transaction.sprout_groth16_joinsplits().enumerate() + { + // Avoid duplicate fetches + if sprout_final_treestates.contains_key(&joinsplit.anchor) { + continue; + } + + // Fetch and return final anchor sets + let input_tree = parent_chain + .sprout_trees_by_anchor .get(&joinsplit.anchor) .cloned() .or_else(|| { - parent_chain - .sprout_trees_by_anchor - .get(&joinsplit.anchor) - .cloned() - .or_else(|| { - finalized_state - .sprout_note_commitment_tree_by_anchor(&joinsplit.anchor) - }) + finalized_state.sprout_note_commitment_tree_by_anchor(&joinsplit.anchor) }); if let Some(input_tree) = input_tree { + tracing::debug!( + ?joinsplit.anchor, + ?joinsplit_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "observed sprout final treestate anchor", + ); sprout_final_treestates.insert(input_tree.root(), input_tree); } - - tracing::debug!(?joinsplit.anchor, "observed sprout final treestate anchor"); } } @@ -76,16 +82,33 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( // // The "earlier treestate" check is implemented here. if transaction.has_sapling_shielded_data() { - for anchor in transaction.sapling_anchors() { - tracing::debug!(?anchor, "observed sapling anchor"); + for (anchor_index_in_tx, anchor) in transaction.sapling_anchors().enumerate() { + tracing::debug!( + ?anchor, + ?anchor_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "observed sapling anchor", + ); if !parent_chain.sapling_anchors.contains(&anchor) && !finalized_state.contains_sapling_anchor(&anchor) { - return Err(ValidateContextError::UnknownSaplingAnchor { anchor }); + return Err(ValidateContextError::UnknownSaplingAnchor { + anchor, + height: prepared.height, + tx_index_in_block, + transaction_hash: prepared.transaction_hashes[tx_index_in_block], + }); } - tracing::debug!(?anchor, "validated sapling anchor"); + tracing::debug!( + ?anchor, + ?anchor_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "validated sapling anchor", + ); } } @@ -101,7 +124,12 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( // // if let Some(orchard_shielded_data) = transaction.orchard_shielded_data() { - tracing::debug!(?orchard_shielded_data.shared_anchor, "observed orchard anchor"); + tracing::debug!( + ?orchard_shielded_data.shared_anchor, + ?tx_index_in_block, + height = ?prepared.height, + "observed orchard anchor", + ); if !parent_chain .orchard_anchors @@ -110,13 +138,28 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( { return Err(ValidateContextError::UnknownOrchardAnchor { anchor: orchard_shielded_data.shared_anchor, + height: prepared.height, + tx_index_in_block, + transaction_hash: prepared.transaction_hashes[tx_index_in_block], }); } - tracing::debug!(?orchard_shielded_data.shared_anchor, "validated orchard anchor"); + tracing::debug!( + ?orchard_shielded_data.shared_anchor, + ?tx_index_in_block, + height = ?prepared.height, + "validated orchard anchor", + ); } } + tracing::trace!( + sprout_final_treestate_count = ?sprout_final_treestates.len(), + ?sprout_final_treestates, + height = ?prepared.height, + "returning sprout final treestate anchors", + ); + Ok(sprout_final_treestates) } @@ -133,7 +176,14 @@ pub(crate) fn sprout_anchors_refer_to_treestates( sprout_final_treestates: HashMap>, prepared: &PreparedBlock, ) -> Result<(), ValidateContextError> { - for transaction in prepared.block.transactions.iter() { + tracing::trace!( + sprout_final_treestate_count = ?sprout_final_treestates.len(), + ?sprout_final_treestates, + height = ?prepared.height, + "received sprout final treestate anchors", + ); + + for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() { // Sprout JoinSplits, with interstitial treestates to check as well. if transaction.has_sprout_joinsplit_data() { let mut interstitial_trees: HashMap< @@ -141,7 +191,9 @@ pub(crate) fn sprout_anchors_refer_to_treestates( Arc, > = HashMap::new(); - for joinsplit in transaction.sprout_groth16_joinsplits() { + for (joinsplit_index_in_tx, joinsplit) in + transaction.sprout_groth16_joinsplits().enumerate() + { // Check all anchor sets, including the one for interstitial // anchors. // @@ -180,19 +232,44 @@ pub(crate) fn sprout_anchors_refer_to_treestates( .cloned() .or_else(|| sprout_final_treestates.get(&joinsplit.anchor).cloned()); + tracing::trace!( + ?input_tree, + final_lookup = ?sprout_final_treestates.get(&joinsplit.anchor), + interstitial_lookup = ?interstitial_trees.get(&joinsplit.anchor), + interstitial_tree_count = ?interstitial_trees.len(), + ?interstitial_trees, + height = ?prepared.height, + "looked up sprout treestate anchor", + ); + let mut input_tree = match input_tree { Some(tree) => tree, None => { - tracing::debug!(?joinsplit.anchor, ?prepared.height, ?prepared.hash, "failed to find sprout anchor"); + tracing::debug!( + ?joinsplit.anchor, + ?joinsplit_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "failed to find sprout anchor", + ); return Err(ValidateContextError::UnknownSproutAnchor { anchor: joinsplit.anchor, + height: prepared.height, + tx_index_in_block, + transaction_hash: prepared.transaction_hashes[tx_index_in_block], }); } }; - let input_tree_inner = Arc::make_mut(&mut input_tree); + tracing::debug!( + ?joinsplit.anchor, + ?joinsplit_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "validated sprout anchor", + ); - tracing::debug!(?joinsplit.anchor, "validated sprout anchor"); + let input_tree_inner = Arc::make_mut(&mut input_tree); // Add new anchors to the interstitial note commitment tree. for cm in joinsplit.commitments { @@ -202,8 +279,13 @@ pub(crate) fn sprout_anchors_refer_to_treestates( } interstitial_trees.insert(input_tree.root(), input_tree); - - tracing::debug!(?joinsplit.anchor, "observed sprout anchor"); + tracing::debug!( + ?joinsplit.anchor, + ?joinsplit_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "observed sprout interstitial anchor", + ); } } } From 35d97e0c65ca98320a8240ec98609ba50526f999 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Jul 2022 13:46:08 +1000 Subject: [PATCH 03/22] Work around a test data bug, and save some CPU --- zebra-state/src/service/check/anchors.rs | 56 +++++++++++++++--------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index b34899cf1c2..0792aba45a3 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -38,28 +38,44 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( transaction.sprout_groth16_joinsplits().enumerate() { // Avoid duplicate fetches - if sprout_final_treestates.contains_key(&joinsplit.anchor) { - continue; - } + if !sprout_final_treestates.contains_key(&joinsplit.anchor) { + // Fetch and return final anchor sets + let input_tree = parent_chain + .sprout_trees_by_anchor + .get(&joinsplit.anchor) + .cloned() + .or_else(|| { + finalized_state.sprout_note_commitment_tree_by_anchor(&joinsplit.anchor) + }); - // Fetch and return final anchor sets - let input_tree = parent_chain - .sprout_trees_by_anchor - .get(&joinsplit.anchor) - .cloned() - .or_else(|| { - finalized_state.sprout_note_commitment_tree_by_anchor(&joinsplit.anchor) - }); + if let Some(input_tree) = input_tree { + /* TODO: + - fix tests that generate incorrect root data + - assert that roots match the fetched tree during tests + - move this CPU-intensive check to sprout_anchors_refer_to_treestates() + + assert_eq!( + input_tree.root(), + joinsplit.anchor, + "anchor and fetched input tree root did not match:\n\ + anchor: {anchor:?},\n\ + input tree root: {input_tree_root:?},\n\ + input_tree: {input_tree:?}", + anchor = joinsplit.anchor + ); + */ + + sprout_final_treestates.insert(joinsplit.anchor, input_tree); - if let Some(input_tree) = input_tree { - tracing::debug!( - ?joinsplit.anchor, - ?joinsplit_index_in_tx, - ?tx_index_in_block, - height = ?prepared.height, - "observed sprout final treestate anchor", - ); - sprout_final_treestates.insert(input_tree.root(), input_tree); + tracing::debug!( + sprout_final_treestate_count = ?sprout_final_treestates.len(), + ?joinsplit.anchor, + ?joinsplit_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "observed sprout final treestate anchor", + ); + } } } } From 4072a92ef2f8047c572c69a875473601a7aa9f33 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Jul 2022 13:55:49 +1000 Subject: [PATCH 04/22] Remove redundant checks for empty shielded data --- zebra-state/src/service/check/anchors.rs | 323 +++++++++++------------ 1 file changed, 160 insertions(+), 163 deletions(-) diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index 0792aba45a3..d36fabe85eb 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -22,6 +22,8 @@ use crate::{ /// This function fetches and returns the Sprout final treestates from the state, /// so [`sprout_anchors_refer_to_interstitial_treestates()`] can check Sprout /// final and interstitial treestates without accessing the disk. +// +// TODO: split this method into sprout, sapling, orchard #[tracing::instrument(skip(finalized_state, parent_chain, prepared))] pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( finalized_state: &ZebraDb, @@ -32,51 +34,50 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( let mut sprout_final_treestates = HashMap::new(); for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() { - // Sprout JoinSplits, with interstitial treestates to check as well. - if transaction.has_sprout_joinsplit_data() { - for (joinsplit_index_in_tx, joinsplit) in - transaction.sprout_groth16_joinsplits().enumerate() - { - // Avoid duplicate fetches - if !sprout_final_treestates.contains_key(&joinsplit.anchor) { - // Fetch and return final anchor sets - let input_tree = parent_chain - .sprout_trees_by_anchor - .get(&joinsplit.anchor) - .cloned() - .or_else(|| { - finalized_state.sprout_note_commitment_tree_by_anchor(&joinsplit.anchor) - }); + // Fetch and return Sprout JoinSplit final treestates + for (joinsplit_index_in_tx, joinsplit) in + transaction.sprout_groth16_joinsplits().enumerate() + { + // Avoid duplicate fetches + if sprout_final_treestates.contains_key(&joinsplit.anchor) { + continue; + } + + let input_tree = parent_chain + .sprout_trees_by_anchor + .get(&joinsplit.anchor) + .cloned() + .or_else(|| { + finalized_state.sprout_note_commitment_tree_by_anchor(&joinsplit.anchor) + }); - if let Some(input_tree) = input_tree { - /* TODO: - - fix tests that generate incorrect root data - - assert that roots match the fetched tree during tests - - move this CPU-intensive check to sprout_anchors_refer_to_treestates() + if let Some(input_tree) = input_tree { + /* TODO: + - fix tests that generate incorrect root data + - assert that roots match the fetched tree during tests + - move this CPU-intensive check to sprout_anchors_refer_to_treestates() - assert_eq!( - input_tree.root(), - joinsplit.anchor, - "anchor and fetched input tree root did not match:\n\ - anchor: {anchor:?},\n\ - input tree root: {input_tree_root:?},\n\ - input_tree: {input_tree:?}", - anchor = joinsplit.anchor - ); - */ + assert_eq!( + input_tree.root(), + joinsplit.anchor, + "anchor and fetched input tree root did not match:\n\ + anchor: {anchor:?},\n\ + input tree root: {input_tree_root:?},\n\ + input_tree: {input_tree:?}", + anchor = joinsplit.anchor + ); + */ - sprout_final_treestates.insert(joinsplit.anchor, input_tree); + sprout_final_treestates.insert(joinsplit.anchor, input_tree); - tracing::debug!( - sprout_final_treestate_count = ?sprout_final_treestates.len(), - ?joinsplit.anchor, - ?joinsplit_index_in_tx, - ?tx_index_in_block, - height = ?prepared.height, - "observed sprout final treestate anchor", - ); - } - } + tracing::debug!( + sprout_final_treestate_count = ?sprout_final_treestates.len(), + ?joinsplit.anchor, + ?joinsplit_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "observed sprout final treestate anchor", + ); } } @@ -97,35 +98,33 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( // [`zebra_chain::sapling::shielded_data`]. // // The "earlier treestate" check is implemented here. - if transaction.has_sapling_shielded_data() { - for (anchor_index_in_tx, anchor) in transaction.sapling_anchors().enumerate() { - tracing::debug!( - ?anchor, - ?anchor_index_in_tx, - ?tx_index_in_block, - height = ?prepared.height, - "observed sapling anchor", - ); - - if !parent_chain.sapling_anchors.contains(&anchor) - && !finalized_state.contains_sapling_anchor(&anchor) - { - return Err(ValidateContextError::UnknownSaplingAnchor { - anchor, - height: prepared.height, - tx_index_in_block, - transaction_hash: prepared.transaction_hashes[tx_index_in_block], - }); - } + for (anchor_index_in_tx, anchor) in transaction.sapling_anchors().enumerate() { + tracing::debug!( + ?anchor, + ?anchor_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "observed sapling anchor", + ); - tracing::debug!( - ?anchor, - ?anchor_index_in_tx, - ?tx_index_in_block, - height = ?prepared.height, - "validated sapling anchor", - ); + if !parent_chain.sapling_anchors.contains(&anchor) + && !finalized_state.contains_sapling_anchor(&anchor) + { + return Err(ValidateContextError::UnknownSaplingAnchor { + anchor, + height: prepared.height, + tx_index_in_block, + transaction_hash: prepared.transaction_hashes[tx_index_in_block], + }); } + + tracing::debug!( + ?anchor, + ?anchor_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "validated sapling anchor", + ); } // Orchard Actions @@ -201,108 +200,106 @@ pub(crate) fn sprout_anchors_refer_to_treestates( for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() { // Sprout JoinSplits, with interstitial treestates to check as well. - if transaction.has_sprout_joinsplit_data() { - let mut interstitial_trees: HashMap< - sprout::tree::Root, - Arc, - > = HashMap::new(); - - for (joinsplit_index_in_tx, joinsplit) in - transaction.sprout_groth16_joinsplits().enumerate() - { - // Check all anchor sets, including the one for interstitial - // anchors. - // - // The anchor is checked and the matching tree is obtained, - // which is used to create the interstitial tree state for this - // JoinSplit: - // - // > For each JoinSplit description in a transaction, an - // > interstitial output treestate is constructed which adds the - // > note commitments and nullifiers specified in that JoinSplit - // > description to the input treestate referred to by its - // > anchor. This interstitial output treestate is available for - // > use as the anchor of subsequent JoinSplit descriptions in - // > the same transaction. - // - // - // - // # Consensus - // - // > The anchor of each JoinSplit description in a transaction - // > MUST refer to either some earlier block’s final Sprout - // > treestate, or to the interstitial output treestate of any - // > prior JoinSplit description in the same transaction. - // - // > For the first JoinSplit description of a transaction, the - // > anchor MUST be the output Sprout treestate of a previous - // > block. - // - // - // - // Note that in order to satisfy the latter consensus rule above, - // [`interstitial_trees`] is always empty in the first iteration - // of the loop. - let input_tree = interstitial_trees - .get(&joinsplit.anchor) - .cloned() - .or_else(|| sprout_final_treestates.get(&joinsplit.anchor).cloned()); + let mut interstitial_trees: HashMap< + sprout::tree::Root, + Arc, + > = HashMap::new(); - tracing::trace!( - ?input_tree, - final_lookup = ?sprout_final_treestates.get(&joinsplit.anchor), - interstitial_lookup = ?interstitial_trees.get(&joinsplit.anchor), - interstitial_tree_count = ?interstitial_trees.len(), - ?interstitial_trees, - height = ?prepared.height, - "looked up sprout treestate anchor", - ); + for (joinsplit_index_in_tx, joinsplit) in + transaction.sprout_groth16_joinsplits().enumerate() + { + // Check all anchor sets, including the one for interstitial + // anchors. + // + // The anchor is checked and the matching tree is obtained, + // which is used to create the interstitial tree state for this + // JoinSplit: + // + // > For each JoinSplit description in a transaction, an + // > interstitial output treestate is constructed which adds the + // > note commitments and nullifiers specified in that JoinSplit + // > description to the input treestate referred to by its + // > anchor. This interstitial output treestate is available for + // > use as the anchor of subsequent JoinSplit descriptions in + // > the same transaction. + // + // + // + // # Consensus + // + // > The anchor of each JoinSplit description in a transaction + // > MUST refer to either some earlier block’s final Sprout + // > treestate, or to the interstitial output treestate of any + // > prior JoinSplit description in the same transaction. + // + // > For the first JoinSplit description of a transaction, the + // > anchor MUST be the output Sprout treestate of a previous + // > block. + // + // + // + // Note that in order to satisfy the latter consensus rule above, + // [`interstitial_trees`] is always empty in the first iteration + // of the loop. + let input_tree = interstitial_trees + .get(&joinsplit.anchor) + .cloned() + .or_else(|| sprout_final_treestates.get(&joinsplit.anchor).cloned()); - let mut input_tree = match input_tree { - Some(tree) => tree, - None => { - tracing::debug!( - ?joinsplit.anchor, - ?joinsplit_index_in_tx, - ?tx_index_in_block, - height = ?prepared.height, - "failed to find sprout anchor", - ); - return Err(ValidateContextError::UnknownSproutAnchor { - anchor: joinsplit.anchor, - height: prepared.height, - tx_index_in_block, - transaction_hash: prepared.transaction_hashes[tx_index_in_block], - }); - } - }; + tracing::trace!( + ?input_tree, + final_lookup = ?sprout_final_treestates.get(&joinsplit.anchor), + interstitial_lookup = ?interstitial_trees.get(&joinsplit.anchor), + interstitial_tree_count = ?interstitial_trees.len(), + ?interstitial_trees, + height = ?prepared.height, + "looked up sprout treestate anchor", + ); - tracing::debug!( - ?joinsplit.anchor, - ?joinsplit_index_in_tx, - ?tx_index_in_block, - height = ?prepared.height, - "validated sprout anchor", - ); + let mut input_tree = match input_tree { + Some(tree) => tree, + None => { + tracing::debug!( + ?joinsplit.anchor, + ?joinsplit_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "failed to find sprout anchor", + ); + return Err(ValidateContextError::UnknownSproutAnchor { + anchor: joinsplit.anchor, + height: prepared.height, + tx_index_in_block, + transaction_hash: prepared.transaction_hashes[tx_index_in_block], + }); + } + }; - let input_tree_inner = Arc::make_mut(&mut input_tree); + tracing::debug!( + ?joinsplit.anchor, + ?joinsplit_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "validated sprout anchor", + ); - // Add new anchors to the interstitial note commitment tree. - for cm in joinsplit.commitments { - input_tree_inner - .append(cm) - .expect("note commitment should be appendable to the tree"); - } + let input_tree_inner = Arc::make_mut(&mut input_tree); - interstitial_trees.insert(input_tree.root(), input_tree); - tracing::debug!( - ?joinsplit.anchor, - ?joinsplit_index_in_tx, - ?tx_index_in_block, - height = ?prepared.height, - "observed sprout interstitial anchor", - ); + // Add new anchors to the interstitial note commitment tree. + for cm in joinsplit.commitments { + input_tree_inner + .append(cm) + .expect("note commitment should be appendable to the tree"); } + + interstitial_trees.insert(input_tree.root(), input_tree); + tracing::debug!( + ?joinsplit.anchor, + ?joinsplit_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "observed sprout interstitial anchor", + ); } } From 1e9d5dbfac9be3de27592f481460bb933267aead Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Jul 2022 14:00:27 +1000 Subject: [PATCH 05/22] Skip generating unused interstitial treestates --- zebra-state/src/service/check/anchors.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index d36fabe85eb..0fa8521a91d 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -205,6 +205,8 @@ pub(crate) fn sprout_anchors_refer_to_treestates( Arc, > = HashMap::new(); + let joinsplit_count = transaction.sprout_groth16_joinsplits().count(); + for (joinsplit_index_in_tx, joinsplit) in transaction.sprout_groth16_joinsplits().enumerate() { @@ -283,6 +285,12 @@ pub(crate) fn sprout_anchors_refer_to_treestates( "validated sprout anchor", ); + // The last interstitial treestate in a transaction can never be used, + // so we avoid generating it. + if joinsplit_index_in_tx == joinsplit_count - 1 { + continue; + } + let input_tree_inner = Arc::make_mut(&mut input_tree); // Add new anchors to the interstitial note commitment tree. @@ -293,6 +301,7 @@ pub(crate) fn sprout_anchors_refer_to_treestates( } interstitial_trees.insert(input_tree.root(), input_tree); + tracing::debug!( ?joinsplit.anchor, ?joinsplit_index_in_tx, From b19cf9c803c37ade33caf9d17201128d3d897f1e Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Jul 2022 14:05:25 +1000 Subject: [PATCH 06/22] Do disk fetches and quick checks, then CPU-heavy cryptography --- zebra-state/src/service/check.rs | 25 ++----------- zebra-state/src/service/check/anchors.rs | 23 ++++++------ zebra-state/src/service/finalized_state.rs | 4 +- .../src/service/non_finalized_state.rs | 37 +++++++++++++------ 4 files changed, 43 insertions(+), 46 deletions(-) diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs index b5c9e1bc670..5a95768d6ba 100644 --- a/zebra-state/src/service/check.rs +++ b/zebra-state/src/service/check.rs @@ -12,7 +12,7 @@ use zebra_chain::{ work::difficulty::CompactDifficulty, }; -use crate::{constants, BoxError, FinalizedBlock, PreparedBlock, ValidateContextError}; +use crate::{constants, BoxError, PreparedBlock, ValidateContextError}; // use self as check use super::check; @@ -104,27 +104,8 @@ where /// Check that the `prepared` block is contextually valid for `network`, using /// the `history_tree` up to and including the previous block. -#[tracing::instrument(skip(prepared, history_tree))] -pub(crate) fn prepared_block_commitment_is_valid_for_chain_history( - prepared: &PreparedBlock, - network: Network, - history_tree: &HistoryTree, -) -> Result<(), ValidateContextError> { - block_commitment_is_valid_for_chain_history(prepared.block.clone(), network, history_tree) -} - -/// Check that the `finalized` block is contextually valid for `network`, using -/// the `history_tree` up to and including the previous block. -#[tracing::instrument(skip(finalized, history_tree))] -pub(crate) fn finalized_block_commitment_is_valid_for_chain_history( - finalized: &FinalizedBlock, - network: Network, - history_tree: &HistoryTree, -) -> Result<(), ValidateContextError> { - block_commitment_is_valid_for_chain_history(finalized.block.clone(), network, history_tree) -} - -fn block_commitment_is_valid_for_chain_history( +#[tracing::instrument(skip(block, history_tree))] +pub(crate) fn block_commitment_is_valid_for_chain_history( block: Arc, network: Network, history_tree: &HistoryTree, diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index 0fa8521a91d..6874984ef93 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -6,6 +6,7 @@ use std::{collections::HashMap, sync::Arc}; use zebra_chain::sprout; use crate::{ + request::ContextuallyValidBlock, service::{finalized_state::ZebraDb, non_finalized_state::Chain}, PreparedBlock, ValidateContextError, }; @@ -178,7 +179,7 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( Ok(sprout_final_treestates) } -/// Checks the Sprout anchors specified by transactions in this `prepared` block. +/// Checks the Sprout anchors specified by transactions in this `contextual` block. /// /// Sprout anchors may refer to some earlier block's final treestate (like /// Sapling and Orchard do exclusively) _or_ to the interstitial output @@ -186,19 +187,19 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( /// /// This method checks for anchors computed from the final and interstitial treestates of /// each transaction in the `prepared` block, using the supplied `sprout_final_treestates`. -#[tracing::instrument(skip(sprout_final_treestates, prepared))] +#[tracing::instrument(skip(sprout_final_treestates, contextual))] pub(crate) fn sprout_anchors_refer_to_treestates( sprout_final_treestates: HashMap>, - prepared: &PreparedBlock, + contextual: &ContextuallyValidBlock, ) -> Result<(), ValidateContextError> { tracing::trace!( sprout_final_treestate_count = ?sprout_final_treestates.len(), ?sprout_final_treestates, - height = ?prepared.height, + height = ?contextual.height, "received sprout final treestate anchors", ); - for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() { + for (tx_index_in_block, transaction) in contextual.block.transactions.iter().enumerate() { // Sprout JoinSplits, with interstitial treestates to check as well. let mut interstitial_trees: HashMap< sprout::tree::Root, @@ -254,7 +255,7 @@ pub(crate) fn sprout_anchors_refer_to_treestates( interstitial_lookup = ?interstitial_trees.get(&joinsplit.anchor), interstitial_tree_count = ?interstitial_trees.len(), ?interstitial_trees, - height = ?prepared.height, + height = ?contextual.height, "looked up sprout treestate anchor", ); @@ -265,14 +266,14 @@ pub(crate) fn sprout_anchors_refer_to_treestates( ?joinsplit.anchor, ?joinsplit_index_in_tx, ?tx_index_in_block, - height = ?prepared.height, + height = ?contextual.height, "failed to find sprout anchor", ); return Err(ValidateContextError::UnknownSproutAnchor { anchor: joinsplit.anchor, - height: prepared.height, + height: contextual.height, tx_index_in_block, - transaction_hash: prepared.transaction_hashes[tx_index_in_block], + transaction_hash: contextual.transaction_hashes[tx_index_in_block], }); } }; @@ -281,7 +282,7 @@ pub(crate) fn sprout_anchors_refer_to_treestates( ?joinsplit.anchor, ?joinsplit_index_in_tx, ?tx_index_in_block, - height = ?prepared.height, + height = ?contextual.height, "validated sprout anchor", ); @@ -306,7 +307,7 @@ pub(crate) fn sprout_anchors_refer_to_treestates( ?joinsplit.anchor, ?joinsplit_index_in_tx, ?tx_index_in_block, - height = ?prepared.height, + height = ?contextual.height, "observed sprout interstitial anchor", ); } diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 958910a260c..5cda3b5e8a6 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -280,8 +280,8 @@ impl FinalizedState { // that is not called by the checkpoint verifier, and keeping a history tree there // would be harder to implement. let history_tree = self.db.history_tree(); - check::finalized_block_commitment_is_valid_for_chain_history( - &finalized, + check::block_commitment_is_valid_for_chain_history( + finalized.block.clone(), self.network, &history_tree, )?; diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index a2c7fc96c90..4ab9726075a 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -2,7 +2,11 @@ //! //! [RFC0005]: https://zebra.zfnd.org/dev/rfcs/0005-state-updates.html -use std::{collections::BTreeSet, mem, sync::Arc}; +use std::{ + collections::{BTreeSet, HashMap}, + mem, + sync::Arc, +}; use zebra_chain::{ block::{self, Block}, @@ -215,13 +219,6 @@ impl NonFinalizedState { finalized_state, )?; - // CPU-heavy cryptography - check::prepared_block_commitment_is_valid_for_chain_history( - &prepared, - self.network, - &new_chain.history_tree, - )?; - // Reads from disk let sprout_final_treestates = check::anchors::sapling_orchard_anchors_refer_to_final_treestates( @@ -230,9 +227,6 @@ impl NonFinalizedState { &prepared, )?; - // CPU-heavy cryptography - check::anchors::sprout_anchors_refer_to_treestates(sprout_final_treestates, &prepared)?; - // Quick check that doesn't read from disk let contextual = ContextuallyValidBlock::with_block_and_spent_utxos( prepared.clone(), @@ -248,6 +242,27 @@ impl NonFinalizedState { } })?; + self.validate_and_update_parallel(new_chain, contextual, sprout_final_treestates) + } + + /// Validate `contextual` and update `new_chain`, doing CPU-intensive work on parallel threads. + #[tracing::instrument(skip(self, new_chain, sprout_final_treestates))] + fn validate_and_update_parallel( + &self, + new_chain: Arc, + contextual: ContextuallyValidBlock, + sprout_final_treestates: HashMap>, + ) -> Result, ValidateContextError> { + // CPU-heavy cryptography + check::block_commitment_is_valid_for_chain_history( + contextual.block.clone(), + self.network, + &new_chain.history_tree, + )?; + + // CPU-heavy cryptography + check::anchors::sprout_anchors_refer_to_treestates(sprout_final_treestates, &contextual)?; + // We're pretty sure the new block is valid, // so clone the inner chain if needed, then add the new block. // From 64e91f3b0b182f6e8b0fc45c824af3e46605605e Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Jul 2022 14:54:40 +1000 Subject: [PATCH 07/22] Wrap HistoryTree in an Arc in the state --- zebra-state/src/service/arbitrary.rs | 18 ++++++++--- .../service/finalized_state/zebra_db/block.rs | 4 +-- .../zebra_db/block/tests/snapshot.rs | 2 +- .../service/finalized_state/zebra_db/chain.rs | 32 +++++++++---------- .../finalized_state/zebra_db/shielded.rs | 2 +- .../src/service/non_finalized_state.rs | 2 +- .../src/service/non_finalized_state/chain.rs | 18 +++++------ .../service/non_finalized_state/tests/prop.rs | 15 ++++++--- 8 files changed, 55 insertions(+), 38 deletions(-) diff --git a/zebra-state/src/service/arbitrary.rs b/zebra-state/src/service/arbitrary.rs index acb08070e87..ed723321674 100644 --- a/zebra-state/src/service/arbitrary.rs +++ b/zebra-state/src/service/arbitrary.rs @@ -32,7 +32,7 @@ pub struct PreparedChainTree { chain: Arc>>, count: BinarySearch, network: Network, - history_tree: HistoryTree, + history_tree: Arc, } impl ValueTree for PreparedChainTree { @@ -40,7 +40,7 @@ impl ValueTree for PreparedChainTree { Arc>>, ::Value, Network, - HistoryTree, + Arc, ); fn current(&self) -> Self::Value { @@ -64,7 +64,13 @@ impl ValueTree for PreparedChainTree { #[derive(Debug, Default)] pub struct PreparedChain { // the proptests are threaded (not async), so we want to use a threaded mutex here - chain: std::sync::Mutex>>, HistoryTree)>>, + chain: std::sync::Mutex< + Option<( + Network, + Arc>>, + Arc, + )>, + >, // the strategy for generating LedgerStates. If None, it calls [`LedgerState::genesis_strategy`]. ledger_strategy: Option>, generate_valid_commitments: bool, @@ -155,7 +161,11 @@ impl Strategy for PreparedChain { &Default::default(), ) .expect("history tree should be created"); - *chain = Some((network, Arc::new(SummaryDebug(blocks)), history_tree)); + *chain = Some(( + network, + Arc::new(SummaryDebug(blocks)), + Arc::new(history_tree), + )); } let chain = chain.clone().expect("should be generated"); diff --git a/zebra-state/src/service/finalized_state/zebra_db/block.rs b/zebra-state/src/service/finalized_state/zebra_db/block.rs index 50f0024200a..c2d30b6b2f7 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/block.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/block.rs @@ -229,7 +229,7 @@ impl ZebraDb { pub(in super::super) fn write_block( &mut self, finalized: FinalizedBlock, - history_tree: HistoryTree, + history_tree: Arc, network: Network, source: &str, ) -> Result { @@ -371,7 +371,7 @@ impl DiskWriteBatch { spent_utxos_by_out_loc: BTreeMap, address_balances: HashMap, mut note_commitment_trees: NoteCommitmentTrees, - history_tree: HistoryTree, + history_tree: Arc, value_pool: ValueBalance, ) -> Result<(), BoxError> { let FinalizedBlock { diff --git a/zebra-state/src/service/finalized_state/zebra_db/block/tests/snapshot.rs b/zebra-state/src/service/finalized_state/zebra_db/block/tests/snapshot.rs index e5be8ec5a44..3eb2bd1a237 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/block/tests/snapshot.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/block/tests/snapshot.rs @@ -297,7 +297,7 @@ fn snapshot_block_and_transaction_data(state: &FinalizedState) { assert_eq!(orchard_tree_at_tip, orchard_tree_by_height); // Skip these checks for empty history trees. - if let Some(history_tree_at_tip) = history_tree_at_tip.as_ref() { + if let Some(history_tree_at_tip) = history_tree_at_tip.as_ref().as_ref() { assert_eq!(history_tree_at_tip.current_height(), max_height); assert_eq!(history_tree_at_tip.network(), state.network()); } diff --git a/zebra-state/src/service/finalized_state/zebra_db/chain.rs b/zebra-state/src/service/finalized_state/zebra_db/chain.rs index 6684fff13b0..4e672aadaa3 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/chain.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/chain.rs @@ -11,7 +11,7 @@ //! The [`crate::constants::DATABASE_FORMAT_VERSION`] constant must //! be incremented each time the database format (column, serialization, etc) changes. -use std::{borrow::Borrow, collections::HashMap}; +use std::{borrow::Borrow, collections::HashMap, sync::Arc}; use zebra_chain::{ amount::NonNegative, @@ -32,20 +32,19 @@ use crate::{ impl ZebraDb { /// Returns the ZIP-221 history tree of the finalized tip or `None` /// if it does not exist yet in the state (pre-Heartwood). - pub fn history_tree(&self) -> HistoryTree { - match self.finalized_tip_height() { - Some(height) => { - let history_tree_cf = self.db.cf_handle("history_tree").unwrap(); - let history_tree: Option = - self.db.zs_get(&history_tree_cf, &height); - if let Some(non_empty_tree) = history_tree { - HistoryTree::from(non_empty_tree) - } else { - Default::default() - } + pub fn history_tree(&self) -> Arc { + if let Some(height) = self.finalized_tip_height() { + let history_tree_cf = self.db.cf_handle("history_tree").unwrap(); + + let history_tree: Option = + self.db.zs_get(&history_tree_cf, &height); + + if let Some(non_empty_tree) = history_tree { + return Arc::new(HistoryTree::from(non_empty_tree)); } - None => Default::default(), } + + Default::default() } /// Returns the stored `ValueBalance` for the best chain at the finalized tip height. @@ -74,13 +73,14 @@ impl DiskWriteBatch { finalized: &FinalizedBlock, sapling_root: sapling::tree::Root, orchard_root: orchard::tree::Root, - mut history_tree: HistoryTree, + mut history_tree: Arc, ) -> Result<(), BoxError> { let history_tree_cf = db.cf_handle("history_tree").unwrap(); let FinalizedBlock { block, height, .. } = finalized; - history_tree.push(self.network(), block.clone(), sapling_root, orchard_root)?; + let history_tree_mut = Arc::make_mut(&mut history_tree); + history_tree_mut.push(self.network(), block.clone(), sapling_root, orchard_root)?; // Update the tree in state let current_tip_height = *height - 1; @@ -93,7 +93,7 @@ impl DiskWriteBatch { // Otherwise, the ReadStateService could access a height // that was just deleted by a concurrent StateService write. // This requires a database version update. - if let Some(history_tree) = history_tree.as_ref() { + if let Some(history_tree) = history_tree.as_ref().as_ref() { self.zs_insert(&history_tree_cf, height, history_tree); } diff --git a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs index 95b0b63f2cc..98c2d9099c9 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs @@ -276,7 +276,7 @@ impl DiskWriteBatch { db: &DiskDb, finalized: &FinalizedBlock, note_commitment_trees: NoteCommitmentTrees, - history_tree: HistoryTree, + history_tree: Arc, ) -> Result<(), BoxError> { let sprout_anchors = db.cf_handle("sprout_anchors").unwrap(); let sapling_anchors = db.cf_handle("sapling_anchors").unwrap(); diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 4ab9726075a..646874120bd 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -415,7 +415,7 @@ impl NonFinalizedState { sprout_note_commitment_tree: Arc, sapling_note_commitment_tree: Arc, orchard_note_commitment_tree: Arc, - history_tree: HistoryTree, + history_tree: Arc, ) -> Result, ValidateContextError> { match self.find_chain(|chain| chain.non_finalized_tip_hash() == parent_hash) { // Clone the existing Arc in the non-finalized state diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 0c9f51a392f..1569cdcd519 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -84,7 +84,7 @@ pub struct Chain { BTreeMap>, /// The ZIP-221 history tree of the tip of this [`Chain`], /// including all finalized blocks, and the non-finalized `blocks` in this chain. - pub(crate) history_tree: HistoryTree, + pub(crate) history_tree: Arc, /// The Sprout anchors created by `blocks`. pub(crate) sprout_anchors: MultiSet, @@ -132,7 +132,7 @@ impl Chain { sprout_note_commitment_tree: Arc, sapling_note_commitment_tree: Arc, orchard_note_commitment_tree: Arc, - history_tree: HistoryTree, + history_tree: Arc, finalized_tip_chain_value_pools: ValueBalance, ) -> Self { Self { @@ -176,8 +176,6 @@ impl Chain { /// even if the blocks in the two chains are equal. #[cfg(test)] pub(crate) fn eq_internal_state(&self, other: &Chain) -> bool { - use zebra_chain::history_tree::NonEmptyHistoryTree; - // blocks, heights, hashes self.blocks == other.blocks && self.height_by_hash == other.height_by_hash && @@ -196,7 +194,7 @@ impl Chain { self.orchard_trees_by_height== other.orchard_trees_by_height && // history tree - self.history_tree.as_ref().map(NonEmptyHistoryTree::hash) == other.history_tree.as_ref().map(NonEmptyHistoryTree::hash) && + self.history_tree.as_ref().as_ref().map(|tree| tree.hash()) == other.history_tree.as_ref().as_ref().map(|other_tree| other_tree.hash()) && // anchors self.sprout_anchors == other.sprout_anchors && @@ -278,7 +276,7 @@ impl Chain { sprout_note_commitment_tree: Arc, sapling_note_commitment_tree: Arc, orchard_note_commitment_tree: Arc, - history_tree: HistoryTree, + history_tree: Arc, ) -> Result, ValidateContextError> { if !self.height_by_hash.contains_key(&fork_tip) { return Ok(None); @@ -350,7 +348,8 @@ impl Chain { .get(&block.height) .expect("Orchard anchors must exist for pre-fork blocks"); - forked.history_tree.push( + let history_tree_mut = Arc::make_mut(&mut forked.history_tree); + history_tree_mut.push( self.network, block.block.clone(), *sapling_root, @@ -674,7 +673,7 @@ impl Chain { sprout_note_commitment_tree: Arc, sapling_note_commitment_tree: Arc, orchard_note_commitment_tree: Arc, - history_tree: HistoryTree, + history_tree: Arc, ) -> Self { Chain { network: self.network, @@ -862,7 +861,8 @@ impl UpdateWith for Chain { self.orchard_anchors.insert(orchard_root); self.orchard_anchors_by_height.insert(height, orchard_root); - self.history_tree.push( + let history_tree_mut = Arc::make_mut(&mut self.history_tree); + history_tree_mut.push( self.network, contextually_valid.block.clone(), sapling_root, 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 410a7004fea..d2958ae82d6 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -551,18 +551,25 @@ fn different_blocks_different_chains() -> Result<()> { )| { let prev_block1 = vec1[0].clone(); let prev_block2 = vec2[0].clone(); + let height1 = prev_block1.coinbase_height().unwrap(); let height2 = prev_block1.coinbase_height().unwrap(); - let finalized_tree1: HistoryTree = if height1 >= Heartwood.activation_height(Network::Mainnet).unwrap() { - NonEmptyHistoryTree::from_block(Network::Mainnet, prev_block1, &Default::default(), &Default::default()).unwrap().into() + + let finalized_tree1: Arc = if height1 >= Heartwood.activation_height(Network::Mainnet).unwrap() { + Arc::new( + NonEmptyHistoryTree::from_block(Network::Mainnet, prev_block1, &Default::default(), &Default::default()).unwrap().into() + ) } else { Default::default() }; - let finalized_tree2 = if height2 >= NetworkUpgrade::Heartwood.activation_height(Network::Mainnet).unwrap() { - NonEmptyHistoryTree::from_block(Network::Mainnet, prev_block2, &Default::default(), &Default::default()).unwrap().into() + let finalized_tree2: Arc = if height2 >= NetworkUpgrade::Heartwood.activation_height(Network::Mainnet).unwrap() { + Arc::new( + NonEmptyHistoryTree::from_block(Network::Mainnet, prev_block2, &Default::default(), &Default::default()).unwrap().into() + ) } else { 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()); From e8c8d5f4730397e78708f409e33e89fef2b51a50 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Jul 2022 15:13:37 +1000 Subject: [PATCH 08/22] Run CPU-intensive chain validation and updates in parallel rayon threads --- Cargo.lock | 1 + zebra-state/Cargo.toml | 6 +- zebra-state/src/service/check/anchors.rs | 31 ++++---- .../src/service/non_finalized_state.rs | 74 +++++++++++++------ .../src/service/non_finalized_state/chain.rs | 5 ++ 5 files changed, 80 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5f041cac303..12740d68071 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6497,6 +6497,7 @@ dependencies = [ "once_cell", "proptest", "proptest-derive", + "rayon", "regex", "rlimit", "rocksdb", diff --git a/zebra-state/Cargo.toml b/zebra-state/Cargo.toml index 4937fed1be2..32b4f978e57 100644 --- a/zebra-state/Cargo.toml +++ b/zebra-state/Cargo.toml @@ -19,8 +19,6 @@ itertools = "0.10.3" lazy_static = "1.4.0" metrics = "0.18.1" mset = "0.1.0" -proptest = { version = "0.10.1", optional = true } -proptest-derive = { version = "0.3.0", optional = true } regex = "1.5.6" rlimit = "0.8.3" rocksdb = { version = "0.18.0", default_features = false, features = ["lz4"] } @@ -28,6 +26,7 @@ serde = { version = "1.0.137", features = ["serde_derive"] } tempfile = "3.3.0" thiserror = "1.0.31" +rayon = "1.5.3" tokio = { version = "1.20.0", features = ["sync", "tracing"] } tower = { version = "0.4.13", features = ["buffer", "util"] } tracing = "0.1.31" @@ -35,6 +34,9 @@ tracing = "0.1.31" zebra-chain = { path = "../zebra-chain" } zebra-test = { path = "../zebra-test/", optional = true } +proptest = { version = "0.10.1", optional = true } +proptest-derive = { version = "0.3.0", optional = true } + [dev-dependencies] color-eyre = "0.6.1" once_cell = "1.12.0" diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index 6874984ef93..53e4debeaa5 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -3,10 +3,12 @@ use std::{collections::HashMap, sync::Arc}; -use zebra_chain::sprout; +use zebra_chain::{ + block::{Block, Height}, + sprout, transaction, +}; use crate::{ - request::ContextuallyValidBlock, service::{finalized_state::ZebraDb, non_finalized_state::Chain}, PreparedBlock, ValidateContextError, }; @@ -179,7 +181,7 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( Ok(sprout_final_treestates) } -/// Checks the Sprout anchors specified by transactions in this `contextual` block. +/// Checks the Sprout anchors specified by transactions in `block`. /// /// Sprout anchors may refer to some earlier block's final treestate (like /// Sapling and Orchard do exclusively) _or_ to the interstitial output @@ -187,19 +189,22 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( /// /// This method checks for anchors computed from the final and interstitial treestates of /// each transaction in the `prepared` block, using the supplied `sprout_final_treestates`. -#[tracing::instrument(skip(sprout_final_treestates, contextual))] +#[tracing::instrument(skip(sprout_final_treestates, block, transaction_hashes))] pub(crate) fn sprout_anchors_refer_to_treestates( sprout_final_treestates: HashMap>, - contextual: &ContextuallyValidBlock, + block: Arc, + // Only used for debugging + height: Height, + transaction_hashes: Arc<[transaction::Hash]>, ) -> Result<(), ValidateContextError> { tracing::trace!( sprout_final_treestate_count = ?sprout_final_treestates.len(), ?sprout_final_treestates, - height = ?contextual.height, + ?height, "received sprout final treestate anchors", ); - for (tx_index_in_block, transaction) in contextual.block.transactions.iter().enumerate() { + for (tx_index_in_block, transaction) in block.transactions.iter().enumerate() { // Sprout JoinSplits, with interstitial treestates to check as well. let mut interstitial_trees: HashMap< sprout::tree::Root, @@ -255,7 +260,7 @@ pub(crate) fn sprout_anchors_refer_to_treestates( interstitial_lookup = ?interstitial_trees.get(&joinsplit.anchor), interstitial_tree_count = ?interstitial_trees.len(), ?interstitial_trees, - height = ?contextual.height, + ?height, "looked up sprout treestate anchor", ); @@ -266,14 +271,14 @@ pub(crate) fn sprout_anchors_refer_to_treestates( ?joinsplit.anchor, ?joinsplit_index_in_tx, ?tx_index_in_block, - height = ?contextual.height, + ?height, "failed to find sprout anchor", ); return Err(ValidateContextError::UnknownSproutAnchor { anchor: joinsplit.anchor, - height: contextual.height, + height, tx_index_in_block, - transaction_hash: contextual.transaction_hashes[tx_index_in_block], + transaction_hash: transaction_hashes[tx_index_in_block], }); } }; @@ -282,7 +287,7 @@ pub(crate) fn sprout_anchors_refer_to_treestates( ?joinsplit.anchor, ?joinsplit_index_in_tx, ?tx_index_in_block, - height = ?contextual.height, + ?height, "validated sprout anchor", ); @@ -307,7 +312,7 @@ pub(crate) fn sprout_anchors_refer_to_treestates( ?joinsplit.anchor, ?joinsplit_index_in_tx, ?tx_index_in_block, - height = ?contextual.height, + ?height, "observed sprout interstitial anchor", ); } diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 646874120bd..2e4b2884304 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -242,35 +242,65 @@ impl NonFinalizedState { } })?; - self.validate_and_update_parallel(new_chain, contextual, sprout_final_treestates) + Self::validate_and_update_parallel(new_chain, contextual, sprout_final_treestates) } - /// Validate `contextual` and update `new_chain`, doing CPU-intensive work on parallel threads. - #[tracing::instrument(skip(self, new_chain, sprout_final_treestates))] + /// Validate `contextual` and update `new_chain`, doing CPU-intensive work in parallel batches. + #[allow(clippy::unwrap_in_result)] + #[tracing::instrument(skip(new_chain, sprout_final_treestates))] fn validate_and_update_parallel( - &self, new_chain: Arc, contextual: ContextuallyValidBlock, sprout_final_treestates: HashMap>, ) -> Result, ValidateContextError> { - // CPU-heavy cryptography - check::block_commitment_is_valid_for_chain_history( - contextual.block.clone(), - self.network, - &new_chain.history_tree, - )?; - - // CPU-heavy cryptography - check::anchors::sprout_anchors_refer_to_treestates(sprout_final_treestates, &contextual)?; - - // We're pretty sure the new block is valid, - // so clone the inner chain if needed, then add the new block. - // - // CPU-heavy cryptography, multiple batches? - Arc::try_unwrap(new_chain) - .unwrap_or_else(|shared_chain| (*shared_chain).clone()) - .push(contextual) - .map(Arc::new) + let mut block_commitment_result = None; + let mut sprout_anchor_result = None; + let mut chain_push_result = None; + + // 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 block2 = contextual.block.clone(); + let height = contextual.height; + let transaction_hashes = contextual.transaction_hashes.clone(); + + rayon::in_place_scope_fifo(|scope| { + scope.spawn_fifo(|_scope| { + block_commitment_result = Some(check::block_commitment_is_valid_for_chain_history( + block, + network, + &history_tree, + )); + }); + + scope.spawn_fifo(|_scope| { + sprout_anchor_result = Some(check::anchors::sprout_anchors_refer_to_treestates( + sprout_final_treestates, + block2, + height, + transaction_hashes, + )); + }); + + // We're pretty sure the new block is valid, + // so clone the inner chain if needed, then add the new block. + // + // Pushing a block onto a Chain can launch additional parallel batches. + // TODO: should we pass _scope into Chain::push()? + scope.spawn_fifo(|_scope| { + let new_chain = Arc::try_unwrap(new_chain) + .unwrap_or_else(|shared_chain| (*shared_chain).clone()); + chain_push_result = Some(new_chain.push(contextual).map(Arc::new)); + }); + }); + + // Don't return the updated Chain unless all the parallel results were Ok + block_commitment_result.expect("scope has finished")?; + sprout_anchor_result.expect("scope has finished")?; + + chain_push_result.expect("scope has finished") } /// 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 1569cdcd519..a026b749100 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -372,6 +372,11 @@ impl Chain { Ok(Some(forked)) } + /// Returns the [`Network`] for this chain. + pub fn network(&self) -> Network { + self.network + } + /// Returns the [`ContextuallyValidBlock`] with [`block::Hash`] or /// [`Height`](zebra_chain::block::Height), if it exists in this chain. pub fn block(&self, hash_or_height: HashOrHeight) -> Option<&ContextuallyValidBlock> { From 00214ec905f19341fc94c7fcad9415d6c97c2229 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Jul 2022 15:35:16 +1000 Subject: [PATCH 09/22] Refactor to prepare for parallel tree root calculations --- zebra-state/src/service/finalized_state.rs | 2 ++ .../service/finalized_state/zebra_db/chain.rs | 1 + .../service/finalized_state/zebra_db/shielded.rs | 16 +++++++++++++--- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 5cda3b5e8a6..cc2207b9e95 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -279,6 +279,8 @@ impl FinalizedState { // the history tree root. While it _is_ checked during contextual validation, // that is not called by the checkpoint verifier, and keeping a history tree there // would be harder to implement. + // + // TODO: run this CPU-intensive cryptography in a parallel rayon thread, if it shows up in profiles let history_tree = self.db.history_tree(); check::block_commitment_is_valid_for_chain_history( finalized.block.clone(), diff --git a/zebra-state/src/service/finalized_state/zebra_db/chain.rs b/zebra-state/src/service/finalized_state/zebra_db/chain.rs index 4e672aadaa3..25c316b72a2 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/chain.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/chain.rs @@ -79,6 +79,7 @@ impl DiskWriteBatch { let FinalizedBlock { block, height, .. } = finalized; + // TODO: run this CPU-intensive cryptography in a parallel rayon thread, if it shows up in profiles let history_tree_mut = Arc::make_mut(&mut history_tree); history_tree_mut.push(self.network(), block.clone(), sapling_root, orchard_root)?; diff --git a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs index 98c2d9099c9..f0a1c2ddf76 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs @@ -288,9 +288,8 @@ impl DiskWriteBatch { let FinalizedBlock { height, .. } = finalized; - let sprout_root = note_commitment_trees.sprout.root(); - let sapling_root = note_commitment_trees.sapling.root(); - let orchard_root = note_commitment_trees.orchard.root(); + let (sprout_root, sapling_root, orchard_root) = + Self::note_commitment_roots_parallel(note_commitment_trees.clone()); // Compute the new anchors and index them // Note: if the root hasn't changed, we write the same value again. @@ -329,6 +328,17 @@ impl DiskWriteBatch { self.prepare_history_batch(db, finalized, sapling_root, orchard_root, history_tree) } + /// Calculate the note commitment tree roots using parallel `rayon` threads. + fn note_commitment_roots_parallel( + note_commitment_trees: NoteCommitmentTrees, + ) -> (sprout::tree::Root, sapling::tree::Root, orchard::tree::Root) { + let sprout_root = note_commitment_trees.sprout.root(); + let sapling_root = note_commitment_trees.sapling.root(); + let orchard_root = note_commitment_trees.orchard.root(); + + (sprout_root, sapling_root, orchard_root) + } + /// Prepare a database batch containing the initial note commitment trees, /// and return it (without actually writing anything). /// From b46a8df661986b11ea6afdfe26af6eb1f7f786ed Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Jul 2022 15:38:58 +1000 Subject: [PATCH 10/22] Run finalized state note commitment tree root updates in parallel rayon threads --- .../finalized_state/zebra_db/shielded.rs | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs index f0a1c2ddf76..77f5522f009 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs @@ -332,11 +332,35 @@ impl DiskWriteBatch { fn note_commitment_roots_parallel( note_commitment_trees: NoteCommitmentTrees, ) -> (sprout::tree::Root, sapling::tree::Root, orchard::tree::Root) { - let sprout_root = note_commitment_trees.sprout.root(); - let sapling_root = note_commitment_trees.sapling.root(); - let orchard_root = note_commitment_trees.orchard.root(); - - (sprout_root, sapling_root, orchard_root) + let NoteCommitmentTrees { + sprout, + sapling, + orchard, + } = note_commitment_trees; + + let mut sprout_root = None; + let mut sapling_root = None; + let mut orchard_root = None; + + rayon::in_place_scope_fifo(|scope| { + scope.spawn_fifo(|_scope| { + sprout_root = Some(sprout.root()); + }); + + scope.spawn_fifo(|_scope| { + sapling_root = Some(sapling.root()); + }); + + scope.spawn_fifo(|_scope| { + orchard_root = Some(orchard.root()); + }); + }); + + ( + sprout_root.expect("scope has finished"), + sapling_root.expect("scope has finished"), + orchard_root.expect("scope has finished"), + ) } /// Prepare a database batch containing the initial note commitment trees, From ee74c5511f7c6e38c13b71daa82ce644be859846 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Jul 2022 16:17:21 +1000 Subject: [PATCH 11/22] Update finalized state note commitment trees using parallel rayon threads --- zebra-chain/src/orchard/tree.rs | 7 +- zebra-chain/src/sapling/tree.rs | 7 +- zebra-chain/src/sprout.rs | 1 + .../finalized_state/zebra_db/shielded.rs | 132 +++++++++++++++--- 4 files changed, 129 insertions(+), 18 deletions(-) diff --git a/zebra-chain/src/orchard/tree.rs b/zebra-chain/src/orchard/tree.rs index cb7f9e28035..36a1fad4af3 100644 --- a/zebra-chain/src/orchard/tree.rs +++ b/zebra-chain/src/orchard/tree.rs @@ -34,6 +34,11 @@ use crate::serialization::{ serde_helpers, ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize, }; +/// The type that is used to update the note commitment tree. +/// +/// Unfortunately, this is not the same as `orchard::NoteCommitment`. +pub type NoteCommitmentUpdate = pallas::Base; + pub(super) const MERKLE_DEPTH: usize = 32; /// MerkleCRH^Orchard Hash Function @@ -302,7 +307,7 @@ impl NoteCommitmentTree { /// /// Returns an error if the tree is full. #[allow(clippy::unwrap_in_result)] - pub fn append(&mut self, cm_x: pallas::Base) -> Result<(), NoteCommitmentTreeError> { + pub fn append(&mut self, cm_x: NoteCommitmentUpdate) -> Result<(), NoteCommitmentTreeError> { if self.inner.append(&cm_x.into()) { // Invalidate cached root let cached_root = self diff --git a/zebra-chain/src/sapling/tree.rs b/zebra-chain/src/sapling/tree.rs index 90d8af1b5ba..9187b86c25c 100644 --- a/zebra-chain/src/sapling/tree.rs +++ b/zebra-chain/src/sapling/tree.rs @@ -38,6 +38,11 @@ use crate::serialization::{ serde_helpers, ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize, }; +/// The type that is used to update the note commitment tree. +/// +/// Unfortunately, this is not the same as `sapling::NoteCommitment`. +pub type NoteCommitmentUpdate = jubjub::Fq; + pub(super) const MERKLE_DEPTH: usize = 32; /// MerkleCRH^Sapling Hash Function @@ -307,7 +312,7 @@ impl NoteCommitmentTree { /// /// Returns an error if the tree is full. #[allow(clippy::unwrap_in_result)] - pub fn append(&mut self, cm_u: jubjub::Fq) -> Result<(), NoteCommitmentTreeError> { + pub fn append(&mut self, cm_u: NoteCommitmentUpdate) -> Result<(), NoteCommitmentTreeError> { if self.inner.append(&cm_u.into()) { // Invalidate cached root let cached_root = self diff --git a/zebra-chain/src/sprout.rs b/zebra-chain/src/sprout.rs index 478c5d1f5cd..a99caa48ebe 100644 --- a/zebra-chain/src/sprout.rs +++ b/zebra-chain/src/sprout.rs @@ -14,6 +14,7 @@ pub mod keys; pub mod note; pub mod tree; +pub use commitment::NoteCommitment; pub use joinsplit::JoinSplit; pub use joinsplit::RandomSeed; pub use note::{EncryptedNote, Note, Nullifier}; diff --git a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs index 77f5522f009..1e4d7220925 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs @@ -15,7 +15,10 @@ use std::sync::Arc; use zebra_chain::{ - block::Height, history_tree::HistoryTree, orchard, sapling, sprout, transaction::Transaction, + block::{Block, Height}, + history_tree::HistoryTree, + orchard, sapling, sprout, + transaction::Transaction, }; use crate::{ @@ -194,10 +197,10 @@ impl DiskWriteBatch { // Index each transaction's shielded data for transaction in &block.transactions { self.prepare_nullifier_batch(db, transaction)?; - - DiskWriteBatch::update_note_commitment_trees(transaction, note_commitment_trees)?; } + DiskWriteBatch::update_note_commitment_trees_parallel(block, note_commitment_trees)?; + Ok(()) } @@ -231,7 +234,8 @@ impl DiskWriteBatch { Ok(()) } - /// Updates the supplied note commitment trees. + /// Update the supplied note commitment trees for the entire block, + /// using parallel `rayon` threads. /// /// If this method returns an error, it will be propagated, /// and the batch should not be written to the database. @@ -239,26 +243,122 @@ impl DiskWriteBatch { /// # Errors /// /// - Propagates any errors from updating note commitment trees - pub fn update_note_commitment_trees( - transaction: &Transaction, + pub fn update_note_commitment_trees_parallel( + block: &Arc, note_commitment_trees: &mut NoteCommitmentTrees, ) -> Result<(), BoxError> { - let sprout_nct = Arc::make_mut(&mut note_commitment_trees.sprout); - for sprout_note_commitment in transaction.sprout_note_commitments() { - sprout_nct.append(*sprout_note_commitment)?; + // Prepare arguments for parallel threads + let NoteCommitmentTrees { + sprout, + sapling, + orchard, + } = note_commitment_trees.clone(); + + let sprout_note_commitments: Vec<_> = block + .transactions + .iter() + .flat_map(|tx| tx.sprout_note_commitments()) + .cloned() + .collect(); + let sapling_note_commitments: Vec<_> = block + .transactions + .iter() + .flat_map(|tx| tx.sapling_note_commitments()) + .cloned() + .collect(); + let orchard_note_commitments: Vec<_> = block + .transactions + .iter() + .flat_map(|tx| tx.orchard_note_commitments()) + .cloned() + .collect(); + + let mut sprout_result = None; + let mut sapling_result = None; + let mut orchard_result = None; + + rayon::in_place_scope_fifo(|scope| { + if !sprout_note_commitments.is_empty() { + scope.spawn_fifo(|_scope| { + sprout_result = Some(Self::update_sprout_note_commitment_tree( + sprout, + sprout_note_commitments, + )); + }); + } + + if !sapling_note_commitments.is_empty() { + scope.spawn_fifo(|_scope| { + sapling_result = Some(Self::update_sapling_note_commitment_tree( + sapling, + sapling_note_commitments, + )); + }); + } + + if !orchard_note_commitments.is_empty() { + scope.spawn_fifo(|_scope| { + orchard_result = Some(Self::update_orchard_note_commitment_tree( + orchard, + orchard_note_commitments, + )); + }); + } + }); + + if let Some(sprout_result) = sprout_result { + note_commitment_trees.sprout = sprout_result?; + } + if let Some(sapling_result) = sapling_result { + note_commitment_trees.sapling = sapling_result?; + } + if let Some(orchard_result) = orchard_result { + note_commitment_trees.orchard = orchard_result?; } - let sapling_nct = Arc::make_mut(&mut note_commitment_trees.sapling); - for sapling_note_commitment in transaction.sapling_note_commitments() { - sapling_nct.append(*sapling_note_commitment)?; + Ok(()) + } + + /// Update the sprout note commitment tree. + fn update_sprout_note_commitment_tree( + mut sprout: Arc, + sprout_note_commitments: Vec, + ) -> Result, BoxError> { + let sprout_nct = Arc::make_mut(&mut sprout); + + for sprout_note_commitment in sprout_note_commitments { + sprout_nct.append(sprout_note_commitment)?; } - let orchard_nct = Arc::make_mut(&mut note_commitment_trees.orchard); - for orchard_note_commitment in transaction.orchard_note_commitments() { - orchard_nct.append(*orchard_note_commitment)?; + Ok(sprout) + } + + /// Update the sapling note commitment tree. + fn update_sapling_note_commitment_tree( + mut sapling: Arc, + sapling_note_commitments: Vec, + ) -> Result, BoxError> { + let sapling_nct = Arc::make_mut(&mut sapling); + + for sapling_note_commitment in sapling_note_commitments { + sapling_nct.append(sapling_note_commitment)?; } - Ok(()) + Ok(sapling) + } + + /// Update the orchard note commitment tree. + fn update_orchard_note_commitment_tree( + mut orchard: Arc, + orchard_note_commitments: Vec, + ) -> Result, BoxError> { + let orchard_nct = Arc::make_mut(&mut orchard); + + for orchard_note_commitment in orchard_note_commitments { + orchard_nct.append(orchard_note_commitment)?; + } + + Ok(orchard) } /// Prepare a database batch containing the note commitment and history tree updates From 34ed93a069513895d4315b5d71ec40177d8950a0 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 21 Jul 2022 11:42:56 +1000 Subject: [PATCH 12/22] Fix a comment typo and add a TODO --- zebra-state/src/service/check.rs | 2 +- zebra-state/src/service/finalized_state/zebra_db/shielded.rs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs index 5a95768d6ba..b3a43057a20 100644 --- a/zebra-state/src/service/check.rs +++ b/zebra-state/src/service/check.rs @@ -102,7 +102,7 @@ where Ok(()) } -/// Check that the `prepared` block is contextually valid for `network`, using +/// Check that `block` is contextually valid for `network`, using /// the `history_tree` up to and including the previous block. #[tracing::instrument(skip(block, history_tree))] pub(crate) fn block_commitment_is_valid_for_chain_history( diff --git a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs index 1e4d7220925..e83afe3a445 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs @@ -442,6 +442,9 @@ impl DiskWriteBatch { let mut sapling_root = None; let mut orchard_root = None; + // TODO: consider doing these calculations after the trees are updated, + // in the rayon scopes for each tree. + // To do this, we need to make sure we only calculate the roots once. rayon::in_place_scope_fifo(|scope| { scope.spawn_fifo(|_scope| { sprout_root = Some(sprout.root()); From 88dafb826a14ccb3a505bc9e7df639cccee823ef Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 21 Jul 2022 11:49:18 +1000 Subject: [PATCH 13/22] Split sprout treestate fetch into its own function --- zebra-state/src/service/check/anchors.rs | 129 +++++++++--------- .../src/service/non_finalized_state.rs | 13 +- 2 files changed, 76 insertions(+), 66 deletions(-) diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index 53e4debeaa5..3e6f205cf14 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -18,72 +18,13 @@ use crate::{ /// /// This method checks for anchors computed from the final treestate of each block in /// the `parent_chain` or `finalized_state`. -/// -/// Sprout anchors may also refer to the interstitial output treestate of any prior -/// `JoinSplit` _within the same transaction_. -/// -/// This function fetches and returns the Sprout final treestates from the state, -/// so [`sprout_anchors_refer_to_interstitial_treestates()`] can check Sprout -/// final and interstitial treestates without accessing the disk. -// -// TODO: split this method into sprout, sapling, orchard #[tracing::instrument(skip(finalized_state, parent_chain, prepared))] pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( finalized_state: &ZebraDb, parent_chain: &Chain, prepared: &PreparedBlock, -) -> Result>, ValidateContextError> -{ - let mut sprout_final_treestates = HashMap::new(); - +) -> Result<(), ValidateContextError> { for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() { - // Fetch and return Sprout JoinSplit final treestates - for (joinsplit_index_in_tx, joinsplit) in - transaction.sprout_groth16_joinsplits().enumerate() - { - // Avoid duplicate fetches - if sprout_final_treestates.contains_key(&joinsplit.anchor) { - continue; - } - - let input_tree = parent_chain - .sprout_trees_by_anchor - .get(&joinsplit.anchor) - .cloned() - .or_else(|| { - finalized_state.sprout_note_commitment_tree_by_anchor(&joinsplit.anchor) - }); - - if let Some(input_tree) = input_tree { - /* TODO: - - fix tests that generate incorrect root data - - assert that roots match the fetched tree during tests - - move this CPU-intensive check to sprout_anchors_refer_to_treestates() - - assert_eq!( - input_tree.root(), - joinsplit.anchor, - "anchor and fetched input tree root did not match:\n\ - anchor: {anchor:?},\n\ - input tree root: {input_tree_root:?},\n\ - input_tree: {input_tree:?}", - anchor = joinsplit.anchor - ); - */ - - sprout_final_treestates.insert(joinsplit.anchor, input_tree); - - tracing::debug!( - sprout_final_treestate_count = ?sprout_final_treestates.len(), - ?joinsplit.anchor, - ?joinsplit_index_in_tx, - ?tx_index_in_block, - height = ?prepared.height, - "observed sprout final treestate anchor", - ); - } - } - // Sapling Spends // // MUST refer to some earlier block’s final Sapling treestate. @@ -171,6 +112,72 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( } } + Ok(()) +} + +/// This function fetches and returns the Sprout final treestates from the state, +/// so [`sprout_anchors_refer_to_interstitial_treestates()`] can check Sprout +/// final and interstitial treestates without accessing the disk. +/// +/// Sprout anchors may also refer to the interstitial output treestate of any prior +/// `JoinSplit` _within the same transaction_. +#[tracing::instrument(skip(finalized_state, parent_chain, prepared))] +pub(crate) fn fetch_sprout_final_treestates( + finalized_state: &ZebraDb, + parent_chain: &Chain, + prepared: &PreparedBlock, +) -> HashMap> { + let mut sprout_final_treestates = HashMap::new(); + + for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() { + // Fetch and return Sprout JoinSplit final treestates + for (joinsplit_index_in_tx, joinsplit) in + transaction.sprout_groth16_joinsplits().enumerate() + { + // Avoid duplicate fetches + if sprout_final_treestates.contains_key(&joinsplit.anchor) { + continue; + } + + let input_tree = parent_chain + .sprout_trees_by_anchor + .get(&joinsplit.anchor) + .cloned() + .or_else(|| { + finalized_state.sprout_note_commitment_tree_by_anchor(&joinsplit.anchor) + }); + + if let Some(input_tree) = input_tree { + /* TODO: + - fix tests that generate incorrect root data + - assert that roots match the fetched tree during tests + - move this CPU-intensive check to sprout_anchors_refer_to_treestates() + + assert_eq!( + input_tree.root(), + joinsplit.anchor, + "anchor and fetched input tree root did not match:\n\ + anchor: {anchor:?},\n\ + input tree root: {input_tree_root:?},\n\ + input_tree: {input_tree:?}", + anchor = joinsplit.anchor + ); + */ + + sprout_final_treestates.insert(joinsplit.anchor, input_tree); + + tracing::debug!( + sprout_final_treestate_count = ?sprout_final_treestates.len(), + ?joinsplit.anchor, + ?joinsplit_index_in_tx, + ?tx_index_in_block, + height = ?prepared.height, + "observed sprout final treestate anchor", + ); + } + } + } + tracing::trace!( sprout_final_treestate_count = ?sprout_final_treestates.len(), ?sprout_final_treestates, @@ -178,7 +185,7 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( "returning sprout final treestate anchors", ); - Ok(sprout_final_treestates) + sprout_final_treestates } /// Checks the Sprout anchors specified by transactions in `block`. diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 2e4b2884304..5a23bd07889 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -219,13 +219,16 @@ impl NonFinalizedState { finalized_state, )?; + // Reads from disk + check::anchors::sapling_orchard_anchors_refer_to_final_treestates( + finalized_state, + &new_chain, + &prepared, + )?; + // Reads from disk let sprout_final_treestates = - check::anchors::sapling_orchard_anchors_refer_to_final_treestates( - finalized_state, - &new_chain, - &prepared, - )?; + check::anchors::fetch_sprout_final_treestates(finalized_state, &new_chain, &prepared); // Quick check that doesn't read from disk let contextual = ContextuallyValidBlock::with_block_and_spent_utxos( From 6deaaabd87ae9d9311fbfa335735613c2529c0b5 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 21 Jul 2022 12:23:10 +1000 Subject: [PATCH 14/22] Move parallel note commitment trees to zebra-chain --- Cargo.lock | 1 + zebra-chain/Cargo.toml | 1 + zebra-chain/src/lib.rs | 1 + zebra-chain/src/orchard/tree.rs | 5 +- zebra-chain/src/parallel.rs | 3 + zebra-chain/src/parallel/tree.rs | 161 ++++++++++++++++++ zebra-chain/src/sapling/tree.rs | 6 +- zebra-chain/src/sprout/tree.rs | 4 +- .../service/finalized_state/zebra_db/block.rs | 3 +- .../finalized_state/zebra_db/shielded.rs | 143 +--------------- 10 files changed, 178 insertions(+), 150 deletions(-) create mode 100644 zebra-chain/src/parallel.rs create mode 100644 zebra-chain/src/parallel/tree.rs diff --git a/Cargo.lock b/Cargo.lock index 12740d68071..1b9ebc42aad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6320,6 +6320,7 @@ dependencies = [ "rand 0.8.5", "rand_chacha 0.3.1", "rand_core 0.6.3", + "rayon", "redjubjub", "ripemd", "secp256k1", diff --git a/zebra-chain/Cargo.toml b/zebra-chain/Cargo.toml index 548ca83ed7a..39c3f8dd015 100644 --- a/zebra-chain/Cargo.toml +++ b/zebra-chain/Cargo.toml @@ -81,6 +81,7 @@ rand_chacha = { version = "0.3.1", optional = true } tokio = { version = "1.20.0", features = ["tracing"], optional = true } zebra-test = { path = "../zebra-test/", optional = true } +rayon = "1.5.3" [dev-dependencies] diff --git a/zebra-chain/src/lib.rs b/zebra-chain/src/lib.rs index 755cd48c9cf..a0bd3da12e5 100644 --- a/zebra-chain/src/lib.rs +++ b/zebra-chain/src/lib.rs @@ -21,6 +21,7 @@ pub mod chain_tip; pub mod fmt; pub mod history_tree; pub mod orchard; +pub mod parallel; pub mod parameters; pub mod primitives; pub mod sapling; diff --git a/zebra-chain/src/orchard/tree.rs b/zebra-chain/src/orchard/tree.rs index 36a1fad4af3..1ca12267ea4 100644 --- a/zebra-chain/src/orchard/tree.rs +++ b/zebra-chain/src/orchard/tree.rs @@ -11,7 +11,6 @@ //! A root of a note commitment tree is associated with each treestate. #![allow(clippy::derive_hash_xor_eq)] -#![allow(dead_code)] use std::{ fmt, @@ -253,8 +252,8 @@ impl<'de> serde::Deserialize<'de> for Node { } } -#[allow(dead_code, missing_docs)] -#[derive(Error, Debug, Clone, PartialEq, Eq)] +#[derive(Error, Copy, Clone, Debug, Eq, PartialEq, Hash)] +#[allow(missing_docs)] pub enum NoteCommitmentTreeError { #[error("The note commitment tree is full")] FullTree, diff --git a/zebra-chain/src/parallel.rs b/zebra-chain/src/parallel.rs new file mode 100644 index 00000000000..0a2dcffd720 --- /dev/null +++ b/zebra-chain/src/parallel.rs @@ -0,0 +1,3 @@ +//! Parallel chain update methods. + +pub mod tree; diff --git a/zebra-chain/src/parallel/tree.rs b/zebra-chain/src/parallel/tree.rs new file mode 100644 index 00000000000..32b50ba42b2 --- /dev/null +++ b/zebra-chain/src/parallel/tree.rs @@ -0,0 +1,161 @@ +//! Parallel note commitment tree update methods. + +use std::sync::Arc; + +use thiserror::Error; + +use crate::{block::Block, orchard, sapling, sprout}; + +/// An argument wrapper struct for note commitment trees. +#[derive(Clone, Debug)] +pub struct NoteCommitmentTrees { + /// The sprout note commitment tree. + pub sprout: Arc, + + /// The sapling note commitment tree. + pub sapling: Arc, + + /// The orchard note commitment tree. + pub orchard: Arc, +} + +/// Note commitment tree errors. +#[derive(Error, Copy, Clone, Debug, Eq, PartialEq, Hash)] +pub enum NoteCommitmentTreeError { + /// A sprout tree error + #[error("sprout error: {0}")] + Sprout(#[from] sprout::tree::NoteCommitmentTreeError), + + /// A sapling tree error + #[error("sapling error: {0}")] + Sapling(#[from] sapling::tree::NoteCommitmentTreeError), + + /// A orchard tree error + #[error("orchard error: {0}")] + Orchard(#[from] orchard::tree::NoteCommitmentTreeError), +} + +impl NoteCommitmentTrees { + /// Update these note commitment trees for the entire `block`, + /// using parallel `rayon` threads. + /// + /// If any of the tree updates cause an error, + /// it will be returned at the end of the parallel batches. + pub fn update_trees_parallel( + &mut self, + block: &Arc, + ) -> Result<(), NoteCommitmentTreeError> { + // Prepare arguments for parallel threads + let NoteCommitmentTrees { + sprout, + sapling, + orchard, + } = self.clone(); + + let sprout_note_commitments: Vec<_> = block + .transactions + .iter() + .flat_map(|tx| tx.sprout_note_commitments()) + .cloned() + .collect(); + let sapling_note_commitments: Vec<_> = block + .transactions + .iter() + .flat_map(|tx| tx.sapling_note_commitments()) + .cloned() + .collect(); + let orchard_note_commitments: Vec<_> = block + .transactions + .iter() + .flat_map(|tx| tx.orchard_note_commitments()) + .cloned() + .collect(); + + let mut sprout_result = None; + let mut sapling_result = None; + let mut orchard_result = None; + + rayon::in_place_scope_fifo(|scope| { + if !sprout_note_commitments.is_empty() { + scope.spawn_fifo(|_scope| { + sprout_result = Some(Self::update_sprout_note_commitment_tree( + sprout, + sprout_note_commitments, + )); + }); + } + + if !sapling_note_commitments.is_empty() { + scope.spawn_fifo(|_scope| { + sapling_result = Some(Self::update_sapling_note_commitment_tree( + sapling, + sapling_note_commitments, + )); + }); + } + + if !orchard_note_commitments.is_empty() { + scope.spawn_fifo(|_scope| { + orchard_result = Some(Self::update_orchard_note_commitment_tree( + orchard, + orchard_note_commitments, + )); + }); + } + }); + + if let Some(sprout_result) = sprout_result { + self.sprout = sprout_result?; + } + if let Some(sapling_result) = sapling_result { + self.sapling = sapling_result?; + } + if let Some(orchard_result) = orchard_result { + self.orchard = orchard_result?; + } + + Ok(()) + } + + /// Update the sprout note commitment tree. + fn update_sprout_note_commitment_tree( + mut sprout: Arc, + sprout_note_commitments: Vec, + ) -> Result, NoteCommitmentTreeError> { + let sprout_nct = Arc::make_mut(&mut sprout); + + for sprout_note_commitment in sprout_note_commitments { + sprout_nct.append(sprout_note_commitment)?; + } + + Ok(sprout) + } + + /// Update the sapling note commitment tree. + fn update_sapling_note_commitment_tree( + mut sapling: Arc, + sapling_note_commitments: Vec, + ) -> Result, NoteCommitmentTreeError> { + let sapling_nct = Arc::make_mut(&mut sapling); + + for sapling_note_commitment in sapling_note_commitments { + sapling_nct.append(sapling_note_commitment)?; + } + + Ok(sapling) + } + + /// Update the orchard note commitment tree. + fn update_orchard_note_commitment_tree( + mut orchard: Arc, + orchard_note_commitments: Vec, + ) -> Result, NoteCommitmentTreeError> { + let orchard_nct = Arc::make_mut(&mut orchard); + + for orchard_note_commitment in orchard_note_commitments { + orchard_nct.append(orchard_note_commitment)?; + } + + Ok(orchard) + } +} diff --git a/zebra-chain/src/sapling/tree.rs b/zebra-chain/src/sapling/tree.rs index 9187b86c25c..d366fc7eae9 100644 --- a/zebra-chain/src/sapling/tree.rs +++ b/zebra-chain/src/sapling/tree.rs @@ -10,8 +10,6 @@ //! //! A root of a note commitment tree is associated with each treestate. -#![allow(dead_code)] - use std::{ fmt, hash::{Hash, Hasher}, @@ -257,8 +255,8 @@ impl<'de> serde::Deserialize<'de> for Node { } } -#[allow(dead_code, missing_docs)] -#[derive(Error, Debug, Clone, PartialEq, Eq)] +#[derive(Error, Copy, Clone, Debug, Eq, PartialEq, Hash)] +#[allow(missing_docs)] pub enum NoteCommitmentTreeError { #[error("The note commitment tree is full")] FullTree, diff --git a/zebra-chain/src/sprout/tree.rs b/zebra-chain/src/sprout/tree.rs index cb99cc0dbb8..ed76b00f974 100644 --- a/zebra-chain/src/sprout/tree.rs +++ b/zebra-chain/src/sprout/tree.rs @@ -180,8 +180,8 @@ impl<'de> serde::Deserialize<'de> for Node { } } -#[allow(dead_code, missing_docs)] -#[derive(Error, Debug, Clone, PartialEq, Eq)] +#[derive(Error, Copy, Clone, Debug, Eq, PartialEq, Hash)] +#[allow(missing_docs)] pub enum NoteCommitmentTreeError { #[error("the note commitment tree is full")] FullTree, diff --git a/zebra-state/src/service/finalized_state/zebra_db/block.rs b/zebra-state/src/service/finalized_state/zebra_db/block.rs index c2d30b6b2f7..8674945b12f 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/block.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/block.rs @@ -21,6 +21,7 @@ use zebra_chain::{ block::{self, Block, Height}, history_tree::HistoryTree, orchard, + parallel::tree::NoteCommitmentTrees, parameters::{Network, GENESIS_PREVIOUS_BLOCK_HASH}, sapling, serialization::TrustedPreallocate, @@ -36,7 +37,7 @@ use crate::{ block::TransactionLocation, transparent::{AddressBalanceLocation, OutputLocation}, }, - zebra_db::{metrics::block_precommit_metrics, shielded::NoteCommitmentTrees, ZebraDb}, + zebra_db::{metrics::block_precommit_metrics, ZebraDb}, FinalizedBlock, }, BoxError, HashOrHeight, diff --git a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs index e83afe3a445..0438b4dc829 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs @@ -15,10 +15,8 @@ use std::sync::Arc; use zebra_chain::{ - block::{Block, Height}, - history_tree::HistoryTree, - orchard, sapling, sprout, - transaction::Transaction, + block::Height, history_tree::HistoryTree, orchard, parallel::tree::NoteCommitmentTrees, + sapling, sprout, transaction::Transaction, }; use crate::{ @@ -30,14 +28,6 @@ use crate::{ BoxError, }; -/// An argument wrapper struct for note commitment trees. -#[derive(Clone, Debug)] -pub struct NoteCommitmentTrees { - sprout: Arc, - sapling: Arc, - orchard: Arc, -} - impl ZebraDb { // Read shielded methods @@ -199,7 +189,7 @@ impl DiskWriteBatch { self.prepare_nullifier_batch(db, transaction)?; } - DiskWriteBatch::update_note_commitment_trees_parallel(block, note_commitment_trees)?; + note_commitment_trees.update_trees_parallel(block)?; Ok(()) } @@ -234,133 +224,6 @@ impl DiskWriteBatch { Ok(()) } - /// Update the supplied note commitment trees for the entire block, - /// using parallel `rayon` threads. - /// - /// If this method returns an error, it will be propagated, - /// and the batch should not be written to the database. - /// - /// # Errors - /// - /// - Propagates any errors from updating note commitment trees - pub fn update_note_commitment_trees_parallel( - block: &Arc, - note_commitment_trees: &mut NoteCommitmentTrees, - ) -> Result<(), BoxError> { - // Prepare arguments for parallel threads - let NoteCommitmentTrees { - sprout, - sapling, - orchard, - } = note_commitment_trees.clone(); - - let sprout_note_commitments: Vec<_> = block - .transactions - .iter() - .flat_map(|tx| tx.sprout_note_commitments()) - .cloned() - .collect(); - let sapling_note_commitments: Vec<_> = block - .transactions - .iter() - .flat_map(|tx| tx.sapling_note_commitments()) - .cloned() - .collect(); - let orchard_note_commitments: Vec<_> = block - .transactions - .iter() - .flat_map(|tx| tx.orchard_note_commitments()) - .cloned() - .collect(); - - let mut sprout_result = None; - let mut sapling_result = None; - let mut orchard_result = None; - - rayon::in_place_scope_fifo(|scope| { - if !sprout_note_commitments.is_empty() { - scope.spawn_fifo(|_scope| { - sprout_result = Some(Self::update_sprout_note_commitment_tree( - sprout, - sprout_note_commitments, - )); - }); - } - - if !sapling_note_commitments.is_empty() { - scope.spawn_fifo(|_scope| { - sapling_result = Some(Self::update_sapling_note_commitment_tree( - sapling, - sapling_note_commitments, - )); - }); - } - - if !orchard_note_commitments.is_empty() { - scope.spawn_fifo(|_scope| { - orchard_result = Some(Self::update_orchard_note_commitment_tree( - orchard, - orchard_note_commitments, - )); - }); - } - }); - - if let Some(sprout_result) = sprout_result { - note_commitment_trees.sprout = sprout_result?; - } - if let Some(sapling_result) = sapling_result { - note_commitment_trees.sapling = sapling_result?; - } - if let Some(orchard_result) = orchard_result { - note_commitment_trees.orchard = orchard_result?; - } - - Ok(()) - } - - /// Update the sprout note commitment tree. - fn update_sprout_note_commitment_tree( - mut sprout: Arc, - sprout_note_commitments: Vec, - ) -> Result, BoxError> { - let sprout_nct = Arc::make_mut(&mut sprout); - - for sprout_note_commitment in sprout_note_commitments { - sprout_nct.append(sprout_note_commitment)?; - } - - Ok(sprout) - } - - /// Update the sapling note commitment tree. - fn update_sapling_note_commitment_tree( - mut sapling: Arc, - sapling_note_commitments: Vec, - ) -> Result, BoxError> { - let sapling_nct = Arc::make_mut(&mut sapling); - - for sapling_note_commitment in sapling_note_commitments { - sapling_nct.append(sapling_note_commitment)?; - } - - Ok(sapling) - } - - /// Update the orchard note commitment tree. - fn update_orchard_note_commitment_tree( - mut orchard: Arc, - orchard_note_commitments: Vec, - ) -> Result, BoxError> { - let orchard_nct = Arc::make_mut(&mut orchard); - - for orchard_note_commitment in orchard_note_commitments { - orchard_nct.append(orchard_note_commitment)?; - } - - Ok(orchard) - } - /// Prepare a database batch containing the note commitment and history tree updates /// from `finalized.block`, and return it (without actually writing anything). /// From bba3b1718a81c77c6ae0cbaf2af0e96290fea3d1 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 21 Jul 2022 12:33:19 +1000 Subject: [PATCH 15/22] Re-calculate the tree roots in the same parallel batches --- zebra-chain/src/parallel/tree.rs | 13 +++++- .../finalized_state/zebra_db/shielded.rs | 46 ++----------------- 2 files changed, 16 insertions(+), 43 deletions(-) diff --git a/zebra-chain/src/parallel/tree.rs b/zebra-chain/src/parallel/tree.rs index 32b50ba42b2..4520fcf04b5 100644 --- a/zebra-chain/src/parallel/tree.rs +++ b/zebra-chain/src/parallel/tree.rs @@ -36,8 +36,8 @@ pub enum NoteCommitmentTreeError { } impl NoteCommitmentTrees { - /// Update these note commitment trees for the entire `block`, - /// using parallel `rayon` threads. + /// Updates the note commitment trees using the transactions in `block`, + /// then re-calculates the cached tree roots, using parallel `rayon` threads. /// /// If any of the tree updates cause an error, /// it will be returned at the end of the parallel batches. @@ -128,6 +128,9 @@ impl NoteCommitmentTrees { sprout_nct.append(sprout_note_commitment)?; } + // Re-calculate and cache the tree root. + let _ = sprout_nct.root(); + Ok(sprout) } @@ -142,6 +145,9 @@ impl NoteCommitmentTrees { sapling_nct.append(sapling_note_commitment)?; } + // Re-calculate and cache the tree root. + let _ = sapling_nct.root(); + Ok(sapling) } @@ -156,6 +162,9 @@ impl NoteCommitmentTrees { orchard_nct.append(orchard_note_commitment)?; } + // Re-calculate and cache the tree root. + let _ = orchard_nct.root(); + Ok(orchard) } } diff --git a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs index 0438b4dc829..1963d2d2f57 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs @@ -251,10 +251,12 @@ impl DiskWriteBatch { let FinalizedBlock { height, .. } = finalized; - let (sprout_root, sapling_root, orchard_root) = - Self::note_commitment_roots_parallel(note_commitment_trees.clone()); + // Use the cached values that were previously calculated in parallel. + let sprout_root = note_commitment_trees.sprout.root(); + let sapling_root = note_commitment_trees.sapling.root(); + let orchard_root = note_commitment_trees.orchard.root(); - // Compute the new anchors and index them + // Index the new anchors. // Note: if the root hasn't changed, we write the same value again. self.zs_insert(&sprout_anchors, sprout_root, ¬e_commitment_trees.sprout); self.zs_insert(&sapling_anchors, sapling_root, ()); @@ -291,44 +293,6 @@ impl DiskWriteBatch { self.prepare_history_batch(db, finalized, sapling_root, orchard_root, history_tree) } - /// Calculate the note commitment tree roots using parallel `rayon` threads. - fn note_commitment_roots_parallel( - note_commitment_trees: NoteCommitmentTrees, - ) -> (sprout::tree::Root, sapling::tree::Root, orchard::tree::Root) { - let NoteCommitmentTrees { - sprout, - sapling, - orchard, - } = note_commitment_trees; - - let mut sprout_root = None; - let mut sapling_root = None; - let mut orchard_root = None; - - // TODO: consider doing these calculations after the trees are updated, - // in the rayon scopes for each tree. - // To do this, we need to make sure we only calculate the roots once. - rayon::in_place_scope_fifo(|scope| { - scope.spawn_fifo(|_scope| { - sprout_root = Some(sprout.root()); - }); - - scope.spawn_fifo(|_scope| { - sapling_root = Some(sapling.root()); - }); - - scope.spawn_fifo(|_scope| { - orchard_root = Some(orchard.root()); - }); - }); - - ( - sprout_root.expect("scope has finished"), - sapling_root.expect("scope has finished"), - orchard_root.expect("scope has finished"), - ) - } - /// Prepare a database batch containing the initial note commitment trees, /// and return it (without actually writing anything). /// From 241c4ce89c3b5f6d0963f7334831617ce9275ce3 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 21 Jul 2022 12:58:13 +1000 Subject: [PATCH 16/22] Do non-finalized note commitment tree updates in parallel threads --- zebra-state/src/error.rs | 12 +- .../src/service/non_finalized_state/chain.rs | 206 +++++++++++------- 2 files changed, 133 insertions(+), 85 deletions(-) diff --git a/zebra-state/src/error.rs b/zebra-state/src/error.rs index 3256f795a11..4a08edb1786 100644 --- a/zebra-state/src/error.rs +++ b/zebra-state/src/error.rs @@ -1,3 +1,5 @@ +//! Error types for Zebra's state. + use std::sync::Arc; use chrono::{DateTime, Utc}; @@ -214,14 +216,8 @@ pub enum ValidateContextError { height: Option, }, - #[error("error updating Sprout note commitment tree")] - SproutNoteCommitmentTreeError(#[from] zebra_chain::sprout::tree::NoteCommitmentTreeError), - - #[error("error updating Sapling note commitment tree")] - SaplingNoteCommitmentTreeError(#[from] zebra_chain::sapling::tree::NoteCommitmentTreeError), - - #[error("error updating Orchard note commitment tree")] - OrchardNoteCommitmentTreeError(#[from] zebra_chain::orchard::tree::NoteCommitmentTreeError), + #[error("error updating a note commitment tree")] + NoteCommitmentTreeError(#[from] zebra_chain::parallel::tree::NoteCommitmentTreeError), #[error("error building the history tree")] HistoryTreeError(#[from] HistoryTreeError), diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index a026b749100..11b0cae602f 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::{ fmt::humantime_milliseconds, history_tree::HistoryTree, orchard, + parallel::tree::NoteCommitmentTrees, parameters::Network, primitives::Groth16Proof, sapling, sprout, @@ -708,43 +709,97 @@ impl Chain { chain_value_pools: self.chain_value_pools, } } -} -/// The revert position being performed on a chain. -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -enum RevertPosition { - /// The chain root is being reverted via [`Chain::pop_root`], when a block - /// is finalized. - Root, + /// Update the chain tip with the `contextually_valid` block, + /// running note commitment tree updates in parallel with other updates. + /// + /// Used to implement `update_chain_tip_with::`. + #[instrument(skip(self, contextually_valid), fields(block = %contextually_valid.block))] + #[allow(clippy::unwrap_in_result)] + fn update_chain_tip_with_block_parallel( + &mut self, + contextually_valid: &ContextuallyValidBlock, + ) -> Result<(), ValidateContextError> { + let height = contextually_valid.height; - /// The chain tip is being reverted via [`Chain::pop_tip`], - /// when a chain is forked. - Tip, -} + // Prepare data for parallel execution + // + // TODO: use NoteCommitmentTrees to store the trees as well? + let mut nct = NoteCommitmentTrees { + sprout: self.sprout_note_commitment_tree.clone(), + sapling: self.sapling_note_commitment_tree.clone(), + orchard: self.orchard_note_commitment_tree.clone(), + }; -/// Helper trait to organize inverse operations done on the [`Chain`] type. -/// -/// Used to overload update and revert methods, based on the type of the argument, -/// and the position of the removed block in the chain. -/// -/// This trait was motivated by the length of the `push`, [`Chain::pop_root`], -/// and [`Chain::pop_tip`] functions, and fear that it would be easy to -/// introduce bugs when updating them, unless the code was reorganized to keep -/// related operations adjacent to each other. -trait UpdateWith { - /// When `T` is added to the chain tip, - /// update [`Chain`] cumulative data members to add data that are derived from `T`. - fn update_chain_tip_with(&mut self, _: &T) -> Result<(), ValidateContextError>; + let mut tree_result = None; + let mut partial_result = None; - /// When `T` is removed from `position` in the chain, - /// revert [`Chain`] cumulative data members to remove data that are derived from `T`. - fn revert_chain_with(&mut self, _: &T, position: RevertPosition); -} + // Run 4 tasks in parallel: + // - sprout, sapling, and orchard tree updates and root calculations + // - the rest of the Chain updates + rayon::in_place_scope_fifo(|scope| { + // Spawns a separate rayon task for each note commitment tree + tree_result = Some(nct.update_trees_parallel(&contextually_valid.block.clone())); -impl UpdateWith for Chain { + scope.spawn_fifo(|_scope| { + partial_result = + Some(self.update_chain_tip_with_block_except_trees(contextually_valid)); + }); + }); + + tree_result.expect("scope has already finished")?; + partial_result.expect("scope has already finished")?; + + // Update the note commitment trees in the chain. + self.sprout_note_commitment_tree = nct.sprout; + self.sapling_note_commitment_tree = nct.sapling; + self.orchard_note_commitment_tree = nct.orchard; + + // Do the Chain updates with data dependencies on note commitment tree updates + + // Update the note commitment trees indexed by height. + self.sapling_trees_by_height + .insert(height, self.sapling_note_commitment_tree.clone()); + self.orchard_trees_by_height + .insert(height, self.orchard_note_commitment_tree.clone()); + + // Having updated all the note commitment trees and nullifier sets in + // this block, the roots of the note commitment trees as of the last + // transaction are the treestates of this block. + // + // Use the previously cached roots, which were calculated in parallel. + let sprout_root = self.sprout_note_commitment_tree.root(); + self.sprout_anchors.insert(sprout_root); + self.sprout_anchors_by_height.insert(height, sprout_root); + self.sprout_trees_by_anchor + .insert(sprout_root, self.sprout_note_commitment_tree.clone()); + let sapling_root = self.sapling_note_commitment_tree.root(); + self.sapling_anchors.insert(sapling_root); + self.sapling_anchors_by_height.insert(height, sapling_root); + + let orchard_root = self.orchard_note_commitment_tree.root(); + self.orchard_anchors.insert(orchard_root); + self.orchard_anchors_by_height.insert(height, orchard_root); + + // TODO: update the history trees in a rayon thread, if they show up in CPU profiles + let history_tree_mut = Arc::make_mut(&mut self.history_tree); + history_tree_mut.push( + self.network, + contextually_valid.block.clone(), + sapling_root, + orchard_root, + )?; + + Ok(()) + } + + /// Update the chain tip with the `contextually_valid` block, + /// except for the note commitment and history tree updates. + /// + /// Used to implement `update_chain_tip_with::`. #[instrument(skip(self, contextually_valid), fields(block = %contextually_valid.block))] #[allow(clippy::unwrap_in_result)] - fn update_chain_tip_with( + fn update_chain_tip_with_block_except_trees( &mut self, contextually_valid: &ContextuallyValidBlock, ) -> Result<(), ValidateContextError> { @@ -844,40 +899,52 @@ impl UpdateWith for Chain { self.update_chain_tip_with(orchard_shielded_data)?; } - // Update the note commitment trees indexed by height. - self.sapling_trees_by_height - .insert(height, self.sapling_note_commitment_tree.clone()); - self.orchard_trees_by_height - .insert(height, self.orchard_note_commitment_tree.clone()); + // update the chain value pool balances + self.update_chain_tip_with(chain_value_pool_change)?; - // Having updated all the note commitment trees and nullifier sets in - // this block, the roots of the note commitment trees as of the last - // transaction are the treestates of this block. - let sprout_root = self.sprout_note_commitment_tree.root(); - self.sprout_anchors.insert(sprout_root); - self.sprout_anchors_by_height.insert(height, sprout_root); - self.sprout_trees_by_anchor - .insert(sprout_root, self.sprout_note_commitment_tree.clone()); - let sapling_root = self.sapling_note_commitment_tree.root(); - self.sapling_anchors.insert(sapling_root); - self.sapling_anchors_by_height.insert(height, sapling_root); + Ok(()) + } +} - let orchard_root = self.orchard_note_commitment_tree.root(); - self.orchard_anchors.insert(orchard_root); - self.orchard_anchors_by_height.insert(height, orchard_root); +/// The revert position being performed on a chain. +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +enum RevertPosition { + /// The chain root is being reverted via [`Chain::pop_root`], when a block + /// is finalized. + Root, - let history_tree_mut = Arc::make_mut(&mut self.history_tree); - history_tree_mut.push( - self.network, - contextually_valid.block.clone(), - sapling_root, - orchard_root, - )?; + /// The chain tip is being reverted via [`Chain::pop_tip`], + /// when a chain is forked. + Tip, +} - // update the chain value pool balances - self.update_chain_tip_with(chain_value_pool_change)?; +/// Helper trait to organize inverse operations done on the [`Chain`] type. +/// +/// Used to overload update and revert methods, based on the type of the argument, +/// and the position of the removed block in the chain. +/// +/// This trait was motivated by the length of the `push`, [`Chain::pop_root`], +/// and [`Chain::pop_tip`] functions, and fear that it would be easy to +/// introduce bugs when updating them, unless the code was reorganized to keep +/// related operations adjacent to each other. +trait UpdateWith { + /// When `T` is added to the chain tip, + /// update [`Chain`] cumulative data members to add data that are derived from `T`. + fn update_chain_tip_with(&mut self, _: &T) -> Result<(), ValidateContextError>; - Ok(()) + /// When `T` is removed from `position` in the chain, + /// revert [`Chain`] cumulative data members to remove data that are derived from `T`. + fn revert_chain_with(&mut self, _: &T, position: RevertPosition); +} + +impl UpdateWith for Chain { + #[instrument(skip(self, contextually_valid), fields(block = %contextually_valid.block))] + #[allow(clippy::unwrap_in_result)] + fn update_chain_tip_with( + &mut self, + contextually_valid: &ContextuallyValidBlock, + ) -> Result<(), ValidateContextError> { + self.update_chain_tip_with_block_parallel(contextually_valid) } #[instrument(skip(self, contextually_valid), fields(block = %contextually_valid.block))] @@ -1240,11 +1307,7 @@ impl UpdateWith>> for Chain { joinsplit_data: &Option>, ) -> Result<(), ValidateContextError> { if let Some(joinsplit_data) = joinsplit_data { - let sprout_ncm = Arc::make_mut(&mut self.sprout_note_commitment_tree); - - for cm in joinsplit_data.note_commitments() { - sprout_ncm.append(*cm)?; - } + // We do note commitment tree updates in parallel rayon threads. check::nullifier::add_to_non_finalized_chain_unique( &mut self.sprout_nullifiers, @@ -1290,14 +1353,7 @@ where sapling_shielded_data: &Option>, ) -> Result<(), ValidateContextError> { if let Some(sapling_shielded_data) = sapling_shielded_data { - let sapling_nct = Arc::make_mut(&mut self.sapling_note_commitment_tree); - - // The `_u` here indicates that the Sapling note commitment is - // specified only by the `u`-coordinate of the Jubjub curve - // point `(u, v)`. - for cm_u in sapling_shielded_data.note_commitments() { - sapling_nct.append(*cm_u)?; - } + // We do note commitment tree updates in parallel rayon threads. check::nullifier::add_to_non_finalized_chain_unique( &mut self.sapling_nullifiers, @@ -1340,11 +1396,7 @@ impl UpdateWith> for Chain { orchard_shielded_data: &Option, ) -> Result<(), ValidateContextError> { if let Some(orchard_shielded_data) = orchard_shielded_data { - let orchard_nct = Arc::make_mut(&mut self.orchard_note_commitment_tree); - - for cm_x in orchard_shielded_data.note_commitments() { - orchard_nct.append(*cm_x)?; - } + // We do note commitment tree updates in parallel rayon threads. check::nullifier::add_to_non_finalized_chain_unique( &mut self.orchard_nullifiers, From 380f406a6de9de86dde23e81e12f99d36c844831 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Jul 2022 07:34:06 +1000 Subject: [PATCH 17/22] Update comments about note commitment tree rebuilds --- zebra-state/src/service/non_finalized_state/chain.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 11b0cae602f..8342fb7034e 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -316,6 +316,13 @@ impl Chain { let sapling_nct = Arc::make_mut(&mut forked.sapling_note_commitment_tree); let orchard_nct = Arc::make_mut(&mut forked.orchard_note_commitment_tree); + // Rebuild the note commitment and history trees, starting from the finalized tip tree. + // + // Note commitments and history trees are not removed from the Chain during a fork, + // because we don't support that operation yet. Instead, we recreate the tree + // from the finalized tip. + // + // TODO: remove trees and anchors above the fork, to save CPU time (#4794) for block in forked.blocks.values() { for transaction in block.block.transactions.iter() { for sprout_note_commitment in transaction.sprout_note_commitments() { From 5ac243623846bbaa875f79473d2b5d195be79c3b Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 21 Jul 2022 13:34:03 +1000 Subject: [PATCH 18/22] Do post-fork tree updates in parallel threads --- zebra-chain/src/parallel/tree.rs | 47 +++++-- .../src/service/non_finalized_state/chain.rs | 133 +++++++++++------- 2 files changed, 116 insertions(+), 64 deletions(-) diff --git a/zebra-chain/src/parallel/tree.rs b/zebra-chain/src/parallel/tree.rs index 4520fcf04b5..265c3db9b11 100644 --- a/zebra-chain/src/parallel/tree.rs +++ b/zebra-chain/src/parallel/tree.rs @@ -1,10 +1,13 @@ //! Parallel note commitment tree update methods. -use std::sync::Arc; +use std::{collections::BTreeMap, sync::Arc}; use thiserror::Error; -use crate::{block::Block, orchard, sapling, sprout}; +use crate::{ + block::{Block, Height}, + orchard, sapling, sprout, +}; /// An argument wrapper struct for note commitment trees. #[derive(Clone, Debug)] @@ -41,9 +44,31 @@ impl NoteCommitmentTrees { /// /// If any of the tree updates cause an error, /// it will be returned at the end of the parallel batches. + #[allow(clippy::unwrap_in_result)] pub fn update_trees_parallel( &mut self, block: &Arc, + ) -> Result<(), NoteCommitmentTreeError> { + self.update_trees_parallel_list( + [( + block + .coinbase_height() + .expect("height was already validated"), + block.clone(), + )] + .into_iter() + .collect(), + ) + } + + /// Updates the note commitment trees using the transactions in `block`, + /// then re-calculates the cached tree roots, using parallel `rayon` threads. + /// + /// If any of the tree updates cause an error, + /// it will be returned at the end of the parallel batches. + pub fn update_trees_parallel_list( + &mut self, + block_list: BTreeMap>, ) -> Result<(), NoteCommitmentTreeError> { // Prepare arguments for parallel threads let NoteCommitmentTrees { @@ -52,21 +77,21 @@ impl NoteCommitmentTrees { orchard, } = self.clone(); - let sprout_note_commitments: Vec<_> = block - .transactions - .iter() + let sprout_note_commitments: Vec<_> = block_list + .values() + .flat_map(|block| block.transactions.iter()) .flat_map(|tx| tx.sprout_note_commitments()) .cloned() .collect(); - let sapling_note_commitments: Vec<_> = block - .transactions - .iter() + let sapling_note_commitments: Vec<_> = block_list + .values() + .flat_map(|block| block.transactions.iter()) .flat_map(|tx| tx.sapling_note_commitments()) .cloned() .collect(); - let orchard_note_commitments: Vec<_> = block - .transactions - .iter() + let orchard_note_commitments: Vec<_> = block_list + .values() + .flat_map(|block| block.transactions.iter()) .flat_map(|tx| tx.orchard_note_commitments()) .cloned() .collect(); diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 8342fb7034e..a6619735c71 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -290,20 +290,31 @@ impl Chain { history_tree, ); + // Revert blocks above the fork while forked.non_finalized_tip_hash() != fork_tip { forked.pop_tip(); } - // Rebuild the note commitment and history trees, starting from the finalized tip tree. - // - // Note commitments and history trees are not removed from the Chain during a fork, - // because we don't support that operation yet. Instead, we recreate the tree - // from the finalized tip. - // - // TODO: remove trees and anchors above the fork, to save CPU time (#4794) + // Rebuild trees from the finalized tip, because we haven't implemented reverts yet. + forked.rebuild_trees_parallel()?; + + Ok(Some(forked)) + } + + /// Rebuild the note commitment and history trees after a chain fork, + /// starting from the finalized tip trees, using parallel `rayon` threads. + /// + /// Note commitments and history trees are not removed from the Chain during a fork, + /// because we don't support that operation yet. Instead, we recreate the tree + /// from the finalized tip. + /// + /// TODO: remove trees and anchors above the fork, to save CPU time (#4794) + #[allow(clippy::unwrap_in_result)] + pub fn rebuild_trees_parallel(&mut self) -> Result<(), ValidateContextError> { let start_time = Instant::now(); - let rebuilt_block_count = forked.blocks.len(); - let fork_height = forked.non_finalized_tip_height(); + let rebuilt_block_count = self.blocks.len(); + let fork_height = self.non_finalized_tip_height(); + let fork_tip = self.non_finalized_tip_hash(); info!( ?rebuilt_block_count, @@ -312,51 +323,79 @@ impl Chain { "starting to rebuild note commitment trees after a non-finalized chain fork", ); - let sprout_nct = Arc::make_mut(&mut forked.sprout_note_commitment_tree); - let sapling_nct = Arc::make_mut(&mut forked.sapling_note_commitment_tree); - let orchard_nct = Arc::make_mut(&mut forked.orchard_note_commitment_tree); + // Prepare data for parallel execution + let block_list = self + .blocks + .iter() + .map(|(height, block)| (*height, block.block.clone())) + .collect(); - // Rebuild the note commitment and history trees, starting from the finalized tip tree. - // - // Note commitments and history trees are not removed from the Chain during a fork, - // because we don't support that operation yet. Instead, we recreate the tree - // from the finalized tip. - // - // TODO: remove trees and anchors above the fork, to save CPU time (#4794) - for block in forked.blocks.values() { - for transaction in block.block.transactions.iter() { - for sprout_note_commitment in transaction.sprout_note_commitments() { - sprout_nct - .append(*sprout_note_commitment) - .expect("must work since it was already appended before the fork"); - } + // TODO: use NoteCommitmentTrees to store the trees as well? + let mut nct = NoteCommitmentTrees { + sprout: self.sprout_note_commitment_tree.clone(), + sapling: self.sapling_note_commitment_tree.clone(), + orchard: self.orchard_note_commitment_tree.clone(), + }; - for sapling_note_commitment in transaction.sapling_note_commitments() { - sapling_nct - .append(*sapling_note_commitment) - .expect("must work since it was already appended before the fork"); - } + let mut note_result = None; + let mut history_result = None; - for orchard_note_commitment in transaction.orchard_note_commitments() { - orchard_nct - .append(*orchard_note_commitment) - .expect("must work since it was already appended before the fork"); - } - } + // Run 4 tasks in parallel: + // - sprout, sapling, and orchard tree updates and (redundant) root calculations + // - history tree updates + rayon::in_place_scope_fifo(|scope| { + // Spawns a separate rayon task for each note commitment tree. + // + // TODO: skip the unused root calculations? (redundant after #4794) + note_result = Some(nct.update_trees_parallel_list(block_list)); + + scope.spawn_fifo(|_scope| { + history_result = Some(self.rebuild_history_tree()); + }); + }); + + note_result.expect("scope has already finished")?; + history_result.expect("scope has already finished")?; + // Update the note commitment trees in the chain. + self.sprout_note_commitment_tree = nct.sprout; + self.sapling_note_commitment_tree = nct.sapling; + self.orchard_note_commitment_tree = nct.orchard; + + let rebuild_time = start_time.elapsed(); + let rebuild_time_per_block = + rebuild_time / rebuilt_block_count.try_into().expect("fits in u32"); + info!( + rebuild_time = ?humantime_milliseconds(rebuild_time), + rebuild_time_per_block = ?humantime_milliseconds(rebuild_time_per_block), + ?rebuilt_block_count, + ?fork_height, + ?fork_tip, + "finished rebuilding note commitment trees after a non-finalized chain fork", + ); + + Ok(()) + } + + /// Rebuild the history tree after a chain fork. + /// + /// TODO: remove trees and anchors above the fork, to save CPU time (#4794) + #[allow(clippy::unwrap_in_result)] + pub fn rebuild_history_tree(&mut self) -> Result<(), ValidateContextError> { + for block in self.blocks.values() { // Note that anchors don't need to be recreated since they are already // handled in revert_chain_state_with. - let sapling_root = forked + let sapling_root = self .sapling_anchors_by_height .get(&block.height) .expect("Sapling anchors must exist for pre-fork blocks"); - let orchard_root = forked + let orchard_root = self .orchard_anchors_by_height .get(&block.height) .expect("Orchard anchors must exist for pre-fork blocks"); - let history_tree_mut = Arc::make_mut(&mut forked.history_tree); + let history_tree_mut = Arc::make_mut(&mut self.history_tree); history_tree_mut.push( self.network, block.block.clone(), @@ -365,19 +404,7 @@ impl Chain { )?; } - let rebuild_time = start_time.elapsed(); - let rebuild_time_per_block = - rebuild_time / rebuilt_block_count.try_into().expect("fits in u32"); - info!( - rebuild_time = ?humantime_milliseconds(rebuild_time), - rebuild_time_per_block = ?humantime_milliseconds(rebuild_time_per_block), - ?rebuilt_block_count, - ?fork_height, - ?fork_tip, - "finished rebuilding note commitment trees after a non-finalized chain fork", - ); - - Ok(Some(forked)) + Ok(()) } /// Returns the [`Network`] for this chain. From 8fb562db535be5f1327a0822bb16d4eacf449337 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 21 Jul 2022 13:34:33 +1000 Subject: [PATCH 19/22] Add a TODO for parallel tree updates in tests --- zebra-chain/src/block/arbitrary.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/zebra-chain/src/block/arbitrary.rs b/zebra-chain/src/block/arbitrary.rs index d34fdd4963d..be078394936 100644 --- a/zebra-chain/src/block/arbitrary.rs +++ b/zebra-chain/src/block/arbitrary.rs @@ -449,6 +449,9 @@ impl Block { // would be awkward since the genesis block is handled separatedly there. // This forces us to skip the genesis block here too in order to able to use // this to test the finalized state. + // + // TODO: run note commitment tree updates in parallel rayon threads, + // using `NoteCommitmentTrees::update_trees_parallel()` if generate_valid_commitments && *height != Height(0) { for sapling_note_commitment in transaction.sapling_note_commitments() { sapling_tree.append(*sapling_note_commitment).unwrap(); From 240d53ba4c71b6d7ec9879742816e96af58cf18b Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 22 Jul 2022 09:41:17 +1000 Subject: [PATCH 20/22] Fix broken intra-doc links --- tower-batch/src/worker.rs | 4 ++-- zebra-state/src/service/check/anchors.rs | 4 ++-- zebra-state/src/service/read.rs | 8 +++----- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/tower-batch/src/worker.rs b/tower-batch/src/worker.rs index 2613f2ea30c..f2266e67100 100644 --- a/tower-batch/src/worker.rs +++ b/tower-batch/src/worker.rs @@ -99,7 +99,7 @@ where { /// Creates a new batch worker. /// - /// See [`Service::new()`](crate::Service::new) for details. + /// See [`Batch::new()`](crate::Batch::new) for details. pub(crate) fn new( service: T, rx: mpsc::UnboundedReceiver>, @@ -190,7 +190,7 @@ where /// Run loop for batch requests, which implements the batch policies. /// - /// See [`Service::new()`](crate::Service::new) for details. + /// See [`Batch::new()`](crate::Batch::new) for details. pub async fn run(mut self) { loop { // Wait on either a new message or the batch timer. diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index 3e6f205cf14..64b8cbfc406 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -116,8 +116,8 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( } /// This function fetches and returns the Sprout final treestates from the state, -/// so [`sprout_anchors_refer_to_interstitial_treestates()`] can check Sprout -/// final and interstitial treestates without accessing the disk. +/// so [`sprout_anchors_refer_to_treestates()`] can check Sprout final and interstitial treestates, +/// without accessing the disk. /// /// Sprout anchors may also refer to the interstitial output treestate of any prior /// `JoinSplit` _within the same transaction_. diff --git a/zebra-state/src/service/read.rs b/zebra-state/src/service/read.rs index a77da12862a..d611a35dd99 100644 --- a/zebra-state/src/service/read.rs +++ b/zebra-state/src/service/read.rs @@ -51,8 +51,7 @@ const FINALIZED_ADDRESS_INDEX_RETRIES: usize = 3; pub const ADDRESS_HEIGHTS_FULL_RANGE: RangeInclusive = Height(1)..=Height::MAX; /// Returns the [`Block`] with [`block::Hash`](zebra_chain::block::Hash) or -/// [`Height`](zebra_chain::block::Height), if it exists in the non-finalized -/// `chain` or finalized `db`. +/// [`Height`], if it exists in the non-finalized `chain` or finalized `db`. pub(crate) fn block( chain: Option, db: &ZebraDb, @@ -77,9 +76,8 @@ where .or_else(|| db.block(hash_or_height)) } -/// Returns the [`block:Header`] with [`block::Hash`](zebra_chain::block::Hash) or -/// [`Height`](zebra_chain::block::Height), if it exists in the non-finalized -/// `chain` or finalized `db`. +/// Returns the [`block::Header`] with [`block::Hash`](zebra_chain::block::Hash) or +/// [`Height`], if it exists in the non-finalized `chain` or finalized `db`. pub(crate) fn block_header( chain: Option, db: &ZebraDb, From eebf3f581b833dde09871a56f5a2e289a8a4f807 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 22 Jul 2022 09:45:03 +1000 Subject: [PATCH 21/22] Clarify documentation for sprout treestates --- zebra-state/src/service/check/anchors.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index 64b8cbfc406..9687fca2751 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -120,7 +120,8 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates( /// without accessing the disk. /// /// Sprout anchors may also refer to the interstitial output treestate of any prior -/// `JoinSplit` _within the same transaction_. +/// `JoinSplit` _within the same transaction_; these are created on the fly +/// in [`sprout_anchors_refer_to_treestates()`]. #[tracing::instrument(skip(finalized_state, parent_chain, prepared))] pub(crate) fn fetch_sprout_final_treestates( finalized_state: &ZebraDb, @@ -194,8 +195,10 @@ pub(crate) fn fetch_sprout_final_treestates( /// Sapling and Orchard do exclusively) _or_ to the interstitial output /// treestate of any prior `JoinSplit` _within the same transaction_. /// -/// This method checks for anchors computed from the final and interstitial treestates of -/// each transaction in the `prepared` block, using the supplied `sprout_final_treestates`. +/// This method searches for anchors in the supplied `sprout_final_treestates` +/// (which must be populated with all treestates pointed to in the `prepared` block; +/// see [`fetch_sprout_final_treestates()`]); or in the interstitial +/// treestates which are computed on the fly in this function. #[tracing::instrument(skip(sprout_final_treestates, block, transaction_hashes))] pub(crate) fn sprout_anchors_refer_to_treestates( sprout_final_treestates: HashMap>, From 0baebfc2d3643cc7d69ddb5d51ef87039d0defe8 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 22 Jul 2022 14:13:37 +1000 Subject: [PATCH 22/22] Sort Cargo.toml dependencies --- zebra-chain/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-chain/Cargo.toml b/zebra-chain/Cargo.toml index 39c3f8dd015..6a31534c0a8 100644 --- a/zebra-chain/Cargo.toml +++ b/zebra-chain/Cargo.toml @@ -66,6 +66,7 @@ serde-big-array = "0.4.1" # Processing futures = "0.3.21" itertools = "0.10.3" +rayon = "1.5.3" # ZF deps ed25519-zebra = "3.0.0" @@ -81,7 +82,6 @@ rand_chacha = { version = "0.3.1", optional = true } tokio = { version = "1.20.0", features = ["tracing"], optional = true } zebra-test = { path = "../zebra-test/", optional = true } -rayon = "1.5.3" [dev-dependencies]