From 376b15a69cf73cb63de4d40efe640604d4c72f8c Mon Sep 17 00:00:00 2001 From: arya2 Date: Tue, 29 Nov 2022 15:11:07 -0500 Subject: [PATCH 1/4] parallelize anchors checks for blocks --- zebra-state/src/service/check/anchors.rs | 57 ++++++++++++++++-------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index aebc47a460f..8e802c9d331 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -3,6 +3,8 @@ use std::{collections::HashMap, sync::Arc}; +use rayon::prelude::*; + use zebra_chain::{ block::{Block, Height}, sprout, @@ -331,8 +333,12 @@ pub(crate) fn block_sapling_orchard_anchors_refer_to_final_treestates( parent_chain: &Arc, prepared: &PreparedBlock, ) -> Result<(), ValidateContextError> { - prepared.block.transactions.iter().enumerate().try_for_each( - |(tx_index_in_block, transaction)| { + prepared + .block + .transactions + .par_iter() + .enumerate() + .try_for_each(|(tx_index_in_block, transaction)| { sapling_orchard_anchors_refer_to_final_treestates( finalized_state, Some(parent_chain), @@ -341,8 +347,7 @@ pub(crate) fn block_sapling_orchard_anchors_refer_to_final_treestates( Some(tx_index_in_block), Some(prepared.height), ) - }, - ) + }) } /// Accepts a [`ZebraDb`], [`Arc`](Chain), and [`PreparedBlock`]. @@ -406,21 +411,35 @@ pub(crate) fn block_sprout_anchors_refer_to_treestates( "received sprout final treestate anchors", ); - block - .transactions - .iter() - .enumerate() - .try_for_each(|(tx_index_in_block, transaction)| { - sprout_anchors_refer_to_treestates( - &sprout_final_treestates, - transaction, - transaction_hashes[tx_index_in_block], - Some(tx_index_in_block), - Some(height), - )?; - - Ok(()) - }) + let check_tx_sprout_anchors = |(tx_index_in_block, transaction)| { + sprout_anchors_refer_to_treestates( + &sprout_final_treestates, + transaction, + transaction_hashes[tx_index_in_block], + Some(tx_index_in_block), + Some(height), + )?; + + Ok(()) + }; + + // The overhead for a parallel iterator is unwarranted if sprout_final_treestates is empty + // because it will either return an error for the first transaction or only check that `joinsplit_data` + // is `None` for each transaction. + if sprout_final_treestates.is_empty() { + // The block has no valid sprout anchors + block + .transactions + .iter() + .enumerate() + .try_for_each(check_tx_sprout_anchors) + } else { + block + .transactions + .par_iter() + .enumerate() + .try_for_each(check_tx_sprout_anchors) + } } /// Accepts a [`ZebraDb`], an optional [`Option>`](Chain), and an [`UnminedTx`]. From 379d89534bfc127975ae4e4f7ff34437fa8f10f3 Mon Sep 17 00:00:00 2001 From: arya2 Date: Tue, 29 Nov 2022 15:11:39 -0500 Subject: [PATCH 2/4] parallelize anchors checks for unmined_tx --- zebra-state/src/service/check/anchors.rs | 86 ++++++++++++++++-------- 1 file changed, 57 insertions(+), 29 deletions(-) diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index 8e802c9d331..1e63e578e98 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -454,39 +454,67 @@ pub(crate) fn tx_anchors_refer_to_final_treestates( parent_chain: Option<&Arc>, unmined_tx: &UnminedTx, ) -> Result<(), ValidateContextError> { - sapling_orchard_anchors_refer_to_final_treestates( - finalized_state, - parent_chain, - &unmined_tx.transaction, - unmined_tx.id.mined_id(), - None, - None, - )?; + if unmined_tx.transaction.has_sprout_joinsplit_data() { + let mut sprout_anchors_result = None; + let mut sapling_orchard_anchors_result = None; + rayon::in_place_scope_fifo(|s| { + // This check is expensive, because it updates a note commitment tree for each sprout JoinSplit. + // Since we could be processing attacker-controlled mempool transactions, we need to run each one + // in its own thread, separately from tokio's blocking I/O threads. And if we are under heavy load, + // we want verification to finish in order, so that later transactions can't delay earlier ones. + s.spawn_fifo(|_s| { + let mut sprout_final_treestates = HashMap::new(); + + fetch_sprout_final_treestates( + &mut sprout_final_treestates, + finalized_state, + parent_chain, + &unmined_tx.transaction, + None, + None, + ); - let mut sprout_final_treestates = HashMap::new(); + tracing::trace!( + sprout_final_treestate_count = ?sprout_final_treestates.len(), + ?sprout_final_treestates, + "received sprout final treestate anchors", + ); - fetch_sprout_final_treestates( - &mut sprout_final_treestates, - finalized_state, - parent_chain, - &unmined_tx.transaction, - None, - None, - ); + sprout_anchors_result = Some(sprout_anchors_refer_to_treestates( + &sprout_final_treestates, + &unmined_tx.transaction, + unmined_tx.id.mined_id(), + None, + None, + )); + }); - tracing::trace!( - sprout_final_treestate_count = ?sprout_final_treestates.len(), - ?sprout_final_treestates, - "received sprout final treestate anchors", - ); + s.spawn_fifo(|_s| { + sapling_orchard_anchors_result = + Some(sapling_orchard_anchors_refer_to_final_treestates( + finalized_state, + parent_chain, + &unmined_tx.transaction, + unmined_tx.id.mined_id(), + None, + None, + )); + }); + }); - sprout_anchors_refer_to_treestates( - &sprout_final_treestates, - &unmined_tx.transaction, - unmined_tx.id.mined_id(), - None, - None, - )?; + sprout_anchors_result.expect("scope has finished")?; + sapling_orchard_anchors_result.expect("scope has finished")?; + } else { + // If there are no sprout transactions in the block, avoid running a rayon scope + sapling_orchard_anchors_refer_to_final_treestates( + finalized_state, + parent_chain, + &unmined_tx.transaction, + unmined_tx.id.mined_id(), + None, + None, + )?; + } Ok(()) } From 1521a29e407a5b2f6ba8e2e68ca5039a0b420429 Mon Sep 17 00:00:00 2001 From: arya2 Date: Tue, 29 Nov 2022 20:46:51 -0500 Subject: [PATCH 3/4] reverts par_iter in block_sapling_orchard_anchors_refer_to_final_treestates --- zebra-state/src/service/check/anchors.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index 1e63e578e98..145a2435f80 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -333,12 +333,8 @@ pub(crate) fn block_sapling_orchard_anchors_refer_to_final_treestates( parent_chain: &Arc, prepared: &PreparedBlock, ) -> Result<(), ValidateContextError> { - prepared - .block - .transactions - .par_iter() - .enumerate() - .try_for_each(|(tx_index_in_block, transaction)| { + prepared.block.transactions.iter().enumerate().try_for_each( + |(tx_index_in_block, transaction)| { sapling_orchard_anchors_refer_to_final_treestates( finalized_state, Some(parent_chain), @@ -347,7 +343,8 @@ pub(crate) fn block_sapling_orchard_anchors_refer_to_final_treestates( Some(tx_index_in_block), Some(prepared.height), ) - }) + }, + ) } /// Accepts a [`ZebraDb`], [`Arc`](Chain), and [`PreparedBlock`]. From fb37958c18e4272dc7e3aabaf552cb6889c5420d Mon Sep 17 00:00:00 2001 From: arya2 Date: Tue, 29 Nov 2022 20:52:04 -0500 Subject: [PATCH 4/4] moves fetch_sprout_final_treestates out of rayon thread --- zebra-state/src/service/check/anchors.rs | 56 +++++++++--------------- 1 file changed, 21 insertions(+), 35 deletions(-) diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index 145a2435f80..a7b2de0f241 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -451,26 +451,35 @@ pub(crate) fn tx_anchors_refer_to_final_treestates( parent_chain: Option<&Arc>, unmined_tx: &UnminedTx, ) -> Result<(), ValidateContextError> { + sapling_orchard_anchors_refer_to_final_treestates( + finalized_state, + parent_chain, + &unmined_tx.transaction, + unmined_tx.id.mined_id(), + None, + None, + )?; + + // If there are no sprout transactions in the block, avoid running a rayon scope if unmined_tx.transaction.has_sprout_joinsplit_data() { + let mut sprout_final_treestates = HashMap::new(); + + fetch_sprout_final_treestates( + &mut sprout_final_treestates, + finalized_state, + parent_chain, + &unmined_tx.transaction, + None, + None, + ); + let mut sprout_anchors_result = None; - let mut sapling_orchard_anchors_result = None; rayon::in_place_scope_fifo(|s| { // This check is expensive, because it updates a note commitment tree for each sprout JoinSplit. // Since we could be processing attacker-controlled mempool transactions, we need to run each one // in its own thread, separately from tokio's blocking I/O threads. And if we are under heavy load, // we want verification to finish in order, so that later transactions can't delay earlier ones. s.spawn_fifo(|_s| { - let mut sprout_final_treestates = HashMap::new(); - - fetch_sprout_final_treestates( - &mut sprout_final_treestates, - finalized_state, - parent_chain, - &unmined_tx.transaction, - None, - None, - ); - tracing::trace!( sprout_final_treestate_count = ?sprout_final_treestates.len(), ?sprout_final_treestates, @@ -485,32 +494,9 @@ pub(crate) fn tx_anchors_refer_to_final_treestates( None, )); }); - - s.spawn_fifo(|_s| { - sapling_orchard_anchors_result = - Some(sapling_orchard_anchors_refer_to_final_treestates( - finalized_state, - parent_chain, - &unmined_tx.transaction, - unmined_tx.id.mined_id(), - None, - None, - )); - }); }); sprout_anchors_result.expect("scope has finished")?; - sapling_orchard_anchors_result.expect("scope has finished")?; - } else { - // If there are no sprout transactions in the block, avoid running a rayon scope - sapling_orchard_anchors_refer_to_final_treestates( - finalized_state, - parent_chain, - &unmined_tx.transaction, - unmined_tx.id.mined_id(), - None, - None, - )?; } Ok(())