From a63d5e51d1630e14be54e738d3788bc3b6842af3 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 26 Feb 2024 16:53:15 -0700 Subject: [PATCH] zcash_client_backend: Return decoding errors from `BatchRunners::add_block` --- zcash_client_backend/src/data_api/chain.rs | 12 +-- zcash_client_backend/src/scanning.rs | 91 ++++++++++++++++------ 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index 60e494053..5f68831a9 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -320,15 +320,9 @@ where let mut runners = BatchRunners::<_, (), ()>::for_keys(100, &scanning_keys); - block_source.with_blocks::<_, DbT::Error>( - Some(from_height), - Some(limit), - |block: CompactBlock| { - runners.add_block(params, block); - - Ok(()) - }, - )?; + block_source.with_blocks::<_, DbT::Error>(Some(from_height), Some(limit), |block| { + runners.add_block(params, block).map_err(|e| e.into()) + })?; runners.flush(); diff --git a/zcash_client_backend/src/scanning.rs b/zcash_client_backend/src/scanning.rs index b6b6a465d..70cda21cb 100644 --- a/zcash_client_backend/src/scanning.rs +++ b/zcash_client_backend/src/scanning.rs @@ -336,6 +336,14 @@ impl ScanningKeys bool { use ScanError::*; match self { + EncodingInvalid { .. } => false, PrevHashMismatch { .. } => true, BlockHeightDiscontinuity { .. } => true, TreeSizeMismatch { .. } => true, @@ -390,6 +399,7 @@ impl ScanError { pub fn at_height(&self) -> BlockHeight { use ScanError::*; match self { + EncodingInvalid { at_height, .. } => *at_height, PrevHashMismatch { at_height } => *at_height, BlockHeightDiscontinuity { new_height, .. } => *new_height, TreeSizeMismatch { at_height, .. } => *at_height, @@ -403,6 +413,11 @@ impl fmt::Display for ScanError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { use ScanError::*; match &self { + EncodingInvalid { txid, pool_type, index, .. } => write!( + f, + "{:?} output {} of transaction {} was improperly encoded.", + pool_type, index, txid + ), PrevHashMismatch { at_height } => write!( f, "The parent hash of proposed block does not correspond to the block hash at height {}.", @@ -536,7 +551,7 @@ where } #[tracing::instrument(skip_all, fields(height = block.height))] - pub(crate) fn add_block

(&mut self, params: &P, block: CompactBlock) + pub(crate) fn add_block

(&mut self, params: &P, block: CompactBlock) -> Result<(), ScanError> where P: consensus::Parameters + Send + 'static, IvkTag: Copy + Send + 'static, @@ -554,11 +569,18 @@ where |_| SaplingDomain::new(zip212_enforcement), &tx.outputs .iter() - .map(|output| { - CompactOutputDescription::try_from(output) - .expect("Invalid output found in compact block decoding.") + .enumerate() + .map(|(i, output)| { + CompactOutputDescription::try_from(output).map_err(|_| { + ScanError::EncodingInvalid { + at_height: block_height, + txid, + pool_type: ShieldedProtocol::Sapling, + index: i, + } + }) }) - .collect::>(), + .collect::, _>>()?, ); #[cfg(feature = "orchard")] @@ -568,13 +590,20 @@ where |action| OrchardDomain::for_nullifier(action.nullifier()), &tx.actions .iter() - .map(|action| { - CompactAction::try_from(action) - .expect("Invalid action found in compact block decoding.") + .enumerate() + .map(|(i, action)| { + CompactAction::try_from(action).map_err(|_| ScanError::EncodingInvalid { + at_height: block_height, + txid, + pool_type: ShieldedProtocol::Sapling, + index: i, + }) }) - .collect::>(), + .collect::, _>>()?, ); } + + Ok(()) } } @@ -777,14 +806,21 @@ where &spent_from_accounts, &tx.outputs .iter() - .map(|output| { - ( + .enumerate() + .map(|(i, output)| { + Ok(( SaplingDomain::new(zip212_enforcement), - CompactOutputDescription::try_from(output) - .expect("Invalid output found in compact block decoding."), - ) + CompactOutputDescription::try_from(output).map_err(|_| { + ScanError::EncodingInvalid { + at_height: cur_height, + txid, + pool_type: ShieldedProtocol::Sapling, + index: i, + } + })?, + )) }) - .collect::>(), + .collect::, _>>()?, batch_runners .as_mut() .map(|runners| |txid| runners.sapling.collect_results(cur_hash, txid)), @@ -804,12 +840,19 @@ where &spent_from_accounts, &tx.actions .iter() - .map(|action| { - let action = CompactAction::try_from(action) - .expect("Invalid output found in compact block decoding."); - (OrchardDomain::for_nullifier(action.nullifier()), action) + .enumerate() + .map(|(i, action)| { + let action = CompactAction::try_from(action).map_err(|_| { + ScanError::EncodingInvalid { + at_height: cur_height, + txid, + pool_type: ShieldedProtocol::Orchard, + index: i, + } + })?; + Ok((OrchardDomain::for_nullifier(action.nullifier()), action)) }) - .collect::>(), + .collect::, _>>()?, batch_runners .as_mut() .map(|runners| |txid| runners.orchard.collect_results(cur_hash, txid)), @@ -1223,7 +1266,9 @@ mod tests { let mut batch_runners = if scan_multithreaded { let mut runners = BatchRunners::<_, (), ()>::for_keys(10, &scanning_keys); - runners.add_block(&Network::TestNetwork, cb.clone()); + runners + .add_block(&Network::TestNetwork, cb.clone()) + .unwrap(); runners.flush(); Some(runners) @@ -1306,7 +1351,9 @@ mod tests { let mut batch_runners = if scan_multithreaded { let mut runners = BatchRunners::<_, (), ()>::for_keys(10, &scanning_keys); - runners.add_block(&Network::TestNetwork, cb.clone()); + runners + .add_block(&Network::TestNetwork, cb.clone()) + .unwrap(); runners.flush(); Some(runners)