From f2db7600aa169cdd9375ffc4419cd46aa29d2010 Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Wed, 27 Nov 2019 14:06:18 +0100 Subject: [PATCH 1/7] fix: don't unwrap parameters Return parameters as result instead of panicing in case of a failure. --- filecoin-proofs/src/api/mod.rs | 2 +- filecoin-proofs/src/api/seal.rs | 6 ++-- filecoin-proofs/src/bin/paramcache.rs | 5 +-- filecoin-proofs/src/caches.rs | 8 ++--- filecoin-proofs/src/parameters.rs | 18 ++++++----- filecoin-proofs/src/types/porep_config.rs | 28 +++++++++-------- filecoin-proofs/src/types/post_config.rs | 38 +++++++++++++---------- 7 files changed, 57 insertions(+), 48 deletions(-) diff --git a/filecoin-proofs/src/api/mod.rs b/filecoin-proofs/src/api/mod.rs index 4477d34a3..d9e43536a 100644 --- a/filecoin-proofs/src/api/mod.rs +++ b/filecoin-proofs/src/api/mod.rs @@ -73,7 +73,7 @@ pub fn get_unsealed_range + AsRef>( let pp = public_params( PaddedBytesAmount::from(porep_config), usize::from(PoRepProofPartitions::from(porep_config)), - ); + )?; let offset_padded: PaddedBytesAmount = UnpaddedBytesAmount::from(offset).into(); let num_bytes_padded: PaddedBytesAmount = num_bytes.into(); diff --git a/filecoin-proofs/src/api/seal.rs b/filecoin-proofs/src/api/seal.rs index 0e5f4530a..d697be0be 100644 --- a/filecoin-proofs/src/api/seal.rs +++ b/filecoin-proofs/src/api/seal.rs @@ -77,7 +77,7 @@ pub fn seal_pre_commit, T: AsRef, S: AsRef>( vanilla_params: setup_params( PaddedBytesAmount::from(porep_config), usize::from(PoRepProofPartitions::from(porep_config)), - ), + )?, partitions: Some(usize::from(PoRepProofPartitions::from(porep_config))), }; @@ -219,7 +219,7 @@ pub fn seal_commit>( vanilla_params: setup_params( PaddedBytesAmount::from(porep_config), usize::from(PoRepProofPartitions::from(porep_config)), - ), + )?, partitions: Some(usize::from(PoRepProofPartitions::from(porep_config))), }; @@ -290,7 +290,7 @@ pub fn verify_seal( vanilla_params: setup_params( PaddedBytesAmount::from(porep_config), usize::from(PoRepProofPartitions::from(porep_config)), - ), + )?, partitions: Some(usize::from(PoRepProofPartitions::from(porep_config))), }; diff --git a/filecoin-proofs/src/bin/paramcache.rs b/filecoin-proofs/src/bin/paramcache.rs index a1530e770..d47c6a4db 100644 --- a/filecoin-proofs/src/bin/paramcache.rs +++ b/filecoin-proofs/src/bin/paramcache.rs @@ -35,7 +35,8 @@ fn cache_porep_params(porep_config: PoRepConfig) { let public_params = public_params( PaddedBytesAmount::from(porep_config), usize::from(PoRepProofPartitions::from(porep_config)), - ); + ) + .unwrap(); { let circuit = = diff --git a/filecoin-proofs/src/caches.rs b/filecoin-proofs/src/caches.rs index 039c2fc4a..79e3a21d5 100644 --- a/filecoin-proofs/src/caches.rs +++ b/filecoin-proofs/src/caches.rs @@ -81,7 +81,7 @@ pub fn get_stacked_params(porep_config: PoRepConfig) -> Result Result Result>> { - let post_public_params = post_public_params(post_config); + let post_public_params = post_public_params(post_config)?; let parameters_generator = || { as CompoundProof< @@ -126,7 +126,7 @@ pub fn get_stacked_verifying_key(porep_config: PoRepConfig) -> Result Result Result> { - let post_public_params = post_public_params(post_config); + let post_public_params = post_public_params(post_config)?; let vk_generator = || { as CompoundProof< diff --git a/filecoin-proofs/src/parameters.rs b/filecoin-proofs/src/parameters.rs index cf59d5dfe..25cb72510 100644 --- a/filecoin-proofs/src/parameters.rs +++ b/filecoin-proofs/src/parameters.rs @@ -22,12 +22,11 @@ pub type PostPublicParams = election_post::PublicParams; pub fn public_params( sector_bytes: PaddedBytesAmount, partitions: usize, -) -> stacked::PublicParams { +) -> Result> { StackedDrg::::setup(&setup_params( sector_bytes, partitions, - )) - .unwrap() + )?) } pub fn window_size_nodes_for_sector_bytes(sector_size: PaddedBytesAmount) -> Result { @@ -42,8 +41,8 @@ pub fn window_size_nodes_for_sector_bytes(sector_size: PaddedBytesAmount) -> Res } } -pub fn post_public_params(post_config: PoStConfig) -> PostPublicParams { - ElectionPoSt::::setup(&post_setup_params(post_config)).unwrap() +pub fn post_public_params(post_config: PoStConfig) -> Result { + ElectionPoSt::::setup(&post_setup_params(post_config)) } pub fn post_setup_params(post_config: PoStConfig) -> PostSetupParams { @@ -54,7 +53,10 @@ pub fn post_setup_params(post_config: PoStConfig) -> PostSetupParams { } } -pub fn setup_params(sector_bytes: PaddedBytesAmount, partitions: usize) -> stacked::SetupParams { +pub fn setup_params( + sector_bytes: PaddedBytesAmount, + partitions: usize, +) -> Result { let window_challenges = select_challenges(partitions, POREP_WINDOW_MINIMUM_CHALLENGES, LAYERS); let wrapper_challenges = select_challenges(partitions, POREP_WRAPPER_MINIMUM_CHALLENGES, LAYERS); @@ -80,14 +82,14 @@ pub fn setup_params(sector_bytes: PaddedBytesAmount, partitions: usize) -> stack ); let nodes = sector_bytes / 32; - stacked::SetupParams { + Ok(stacked::SetupParams { nodes, degree: BASE_DEGREE, expansion_degree: EXP_DEGREE, seed: DRG_SEED, config, window_size_nodes, - } + }) } fn select_challenges( diff --git a/filecoin-proofs/src/types/porep_config.rs b/filecoin-proofs/src/types/porep_config.rs index 9c0fdcb9d..d3fd98790 100644 --- a/filecoin-proofs/src/types/porep_config.rs +++ b/filecoin-proofs/src/types/porep_config.rs @@ -1,5 +1,7 @@ use std::path::PathBuf; +use anyhow::Result; + use paired::bls12_381::Bls12; use storage_proofs::circuit::stacked::{StackedCircuit, StackedCompound}; use storage_proofs::drgraph::DefaultTreeHasher; @@ -47,29 +49,29 @@ impl From for SectorSize { impl PoRepConfig { /// Returns the cache identifier as used by `storage-proofs::paramater_cache`. - pub fn get_cache_identifier(&self) -> String { + pub fn get_cache_identifier(&self) -> Result { let params = - crate::parameters::public_params(self.sector_size.into(), self.partitions.into()); + crate::parameters::public_params(self.sector_size.into(), self.partitions.into())?; - , _, - >>::cache_identifier(¶ms) + >>::cache_identifier(¶ms)) } - pub fn get_cache_metadata_path(&self) -> PathBuf { - let id = self.get_cache_identifier(); - parameter_cache::parameter_cache_metadata_path(&id) + pub fn get_cache_metadata_path(&self) -> Result { + let id = self.get_cache_identifier()?; + Ok(parameter_cache::parameter_cache_metadata_path(&id)) } - pub fn get_cache_verifying_key_path(&self) -> PathBuf { - let id = self.get_cache_identifier(); - parameter_cache::parameter_cache_verifying_key_path(&id) + pub fn get_cache_verifying_key_path(&self) -> Result { + let id = self.get_cache_identifier()?; + Ok(parameter_cache::parameter_cache_verifying_key_path(&id)) } - pub fn get_cache_params_path(&self) -> PathBuf { - let id = self.get_cache_identifier(); - parameter_cache::parameter_cache_params_path(&id) + pub fn get_cache_params_path(&self) -> Result { + let id = self.get_cache_identifier()?; + Ok(parameter_cache::parameter_cache_params_path(&id)) } } diff --git a/filecoin-proofs/src/types/post_config.rs b/filecoin-proofs/src/types/post_config.rs index 0fd8c9e65..ba3b9bc7f 100644 --- a/filecoin-proofs/src/types/post_config.rs +++ b/filecoin-proofs/src/types/post_config.rs @@ -1,5 +1,7 @@ use std::path::PathBuf; +use anyhow::Result; + use paired::bls12_381::Bls12; use storage_proofs::circuit::election_post::{ElectionPoStCircuit, ElectionPoStCompound}; use storage_proofs::drgraph::DefaultTreeHasher; @@ -30,28 +32,30 @@ impl From for UnpaddedBytesAmount { impl PoStConfig { /// Returns the cache identifier as used by `storage-proofs::paramater_cache`. - pub fn get_cache_identifier(self) -> String { - let params = crate::parameters::post_public_params(self); - - as CacheableParameters< - Bls12, - ElectionPoStCircuit<_, DefaultTreeHasher>, - _, - >>::cache_identifier(¶ms) + pub fn get_cache_identifier(self) -> Result { + let params = crate::parameters::post_public_params(self)?; + + Ok( + as CacheableParameters< + Bls12, + ElectionPoStCircuit<_, DefaultTreeHasher>, + _, + >>::cache_identifier(¶ms), + ) } - pub fn get_cache_metadata_path(self) -> PathBuf { - let id = self.get_cache_identifier(); - parameter_cache::parameter_cache_metadata_path(&id) + pub fn get_cache_metadata_path(self) -> Result { + let id = self.get_cache_identifier()?; + Ok(parameter_cache::parameter_cache_metadata_path(&id)) } - pub fn get_cache_verifying_key_path(self) -> PathBuf { - let id = self.get_cache_identifier(); - parameter_cache::parameter_cache_verifying_key_path(&id) + pub fn get_cache_verifying_key_path(self) -> Result { + let id = self.get_cache_identifier()?; + Ok(parameter_cache::parameter_cache_verifying_key_path(&id)) } - pub fn get_cache_params_path(self) -> PathBuf { - let id = self.get_cache_identifier(); - parameter_cache::parameter_cache_params_path(&id) + pub fn get_cache_params_path(self) -> Result { + let id = self.get_cache_identifier()?; + Ok(parameter_cache::parameter_cache_params_path(&id)) } } From 5739ecbba7c173f93824f1cc138491fcc551c714 Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Wed, 27 Nov 2019 14:20:31 +0100 Subject: [PATCH 2/7] fix: return error if mmap fails Return an error instead of panicking when the mmap fails during the seal_pre_commit. --- filecoin-proofs/src/api/seal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filecoin-proofs/src/api/seal.rs b/filecoin-proofs/src/api/seal.rs index d697be0be..f466a49e5 100644 --- a/filecoin-proofs/src/api/seal.rs +++ b/filecoin-proofs/src/api/seal.rs @@ -71,7 +71,7 @@ pub fn seal_pre_commit, T: AsRef, S: AsRef>( // Zero-pad the data to the requested size by extending the underlying file if needed. f_data.set_len(sector_bytes as u64)?; - let mut data = unsafe { MmapOptions::new().map_mut(&f_data).unwrap() }; + let mut data = unsafe { MmapOptions::new().map_mut(&f_data)? }; let compound_setup_params = compound_proof::SetupParams { vanilla_params: setup_params( From bf0bb4ba4217c98f41a1e800ecfacfde2ccc8b83 Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Thu, 28 Nov 2019 04:58:58 +0100 Subject: [PATCH 3/7] fix: make stacked storage proof unwrap panic free --- storage-proofs/src/stacked/params.rs | 67 +++++++++++++++++++--------- 1 file changed, 45 insertions(+), 22 deletions(-) diff --git a/storage-proofs/src/stacked/params.rs b/storage-proofs/src/stacked/params.rs index d854e048f..d9253d5c1 100644 --- a/storage-proofs/src/stacked/params.rs +++ b/storage-proofs/src/stacked/params.rs @@ -388,6 +388,7 @@ impl WindowProof { layer, window_index ); + check!(proof.window_index.is_some()); check_eq!(window_index as u64, proof.window_index.unwrap()); let expected_labeled_node = self @@ -486,12 +487,12 @@ pub struct TemporaryAux { } impl TemporaryAux { - pub fn labels_for_layer(&self, layer: usize) -> DiskStore { + pub fn labels_for_layer(&self, layer: usize) -> Result> { self.labels.labels_for_layer(layer) } pub fn domain_node_at_layer(&self, layer: usize, node_index: u32) -> Result { - Ok(self.labels_for_layer(layer).read_at(node_index as usize)?) + Ok(self.labels_for_layer(layer)?.read_at(node_index as usize)?) } pub fn column(&self, layers: usize, column_index: u32) -> Result> { @@ -501,7 +502,10 @@ impl TemporaryAux { pub fn delete(t_aux: TemporaryAux) -> Result<()> { // TODO: once optimized, compact tree_r_last to only store the top part of the tree. - let tree_d_size = t_aux.tree_d_config.size.unwrap(); + let tree_d_size = t_aux + .tree_d_config + .size + .ok_or_else(|| anyhow!("tree_d config has no size"))?; let tree_d_store: DiskStore = DiskStore::new_from_disk(tree_d_size, &t_aux.tree_d_config).context("tree_d")?; let tree_d: Tree = @@ -509,7 +513,10 @@ impl TemporaryAux { .context("tree_d")?; tree_d.delete(t_aux.tree_d_config).context("tree_d")?; - let tree_c_size = t_aux.tree_c_config.size.unwrap(); + let tree_c_size = t_aux + .tree_c_config + .size + .ok_or_else(|| anyhow!("tree_c config has no size"))?; let tree_c_store: DiskStore = DiskStore::new_from_disk(tree_c_size, &t_aux.tree_c_config).context("tree_c")?; let tree_c: Tree = @@ -517,7 +524,10 @@ impl TemporaryAux { .context("tree_c")?; tree_c.delete(t_aux.tree_c_config).context("tree_c")?; - let tree_q_size = t_aux.tree_q_config.size.unwrap(); + let tree_q_size = t_aux + .tree_q_config + .size + .ok_or_else(|| anyhow!("tree_q config has no size"))?; let tree_q_store: DiskStore = DiskStore::new_from_disk(tree_q_size, &t_aux.tree_q_config).context("tree_q")?; let tree_q: Tree = @@ -549,37 +559,50 @@ pub struct TemporaryAuxCache { impl TemporaryAuxCache { pub fn new(t_aux: &TemporaryAux) -> Result { trace!("restoring tree_d"); - let tree_d_size = t_aux.tree_d_config.size.unwrap(); + let tree_d_size = t_aux + .tree_d_config + .size + .ok_or_else(|| anyhow!("tree_d config has no size"))?; let tree_d_store: DiskStore = - DiskStore::new_from_disk(tree_d_size, &t_aux.tree_d_config)?; + DiskStore::new_from_disk(tree_d_size, &t_aux.tree_d_config).context("tree_d")?; let tree_d: Tree = MerkleTree::from_data_store(tree_d_store, get_merkle_tree_leafs(tree_d_size))?; trace!("restoring tree_c"); - let tree_c_size = t_aux.tree_c_config.size.unwrap(); + let tree_c_size = t_aux + .tree_c_config + .size + .ok_or_else(|| anyhow!("tree_c config has no size"))?; let tree_c_store: DiskStore = - DiskStore::new_from_disk(tree_c_size, &t_aux.tree_c_config)?; + DiskStore::new_from_disk(tree_c_size, &t_aux.tree_c_config).context("tree_c")?; let tree_c: Tree = MerkleTree::from_data_store(tree_c_store, get_merkle_tree_leafs(tree_c_size))?; trace!("restoring tree_r_last"); - let tree_r_last_size = t_aux.tree_r_last_config.size.unwrap(); + let tree_r_last_size = t_aux + .tree_r_last_config + .size + .ok_or_else(|| anyhow!("tree_r config has no size"))?; let tree_r_last_store: DiskStore = - DiskStore::new_from_disk(tree_r_last_size, &t_aux.tree_r_last_config)?; + DiskStore::new_from_disk(tree_r_last_size, &t_aux.tree_r_last_config) + .context("tree_r")?; let tree_r_last: Tree = MerkleTree::from_data_store( tree_r_last_store, get_merkle_tree_leafs(tree_r_last_size), )?; trace!("restoring tree_q"); - let tree_q_size = t_aux.tree_q_config.size.unwrap(); + let tree_q_size = t_aux + .tree_q_config + .size + .ok_or_else(|| anyhow!("tree_q config has no size"))?; let tree_q_store: DiskStore = - DiskStore::new_from_disk(tree_q_size, &t_aux.tree_q_config)?; + DiskStore::new_from_disk(tree_q_size, &t_aux.tree_q_config).context("tree_q")?; let tree_q: Tree = MerkleTree::from_data_store(tree_q_store, get_merkle_tree_leafs(tree_q_size))?; Ok(TemporaryAuxCache { - labels: LabelsCache::new(&t_aux.labels), + labels: LabelsCache::new(&t_aux.labels)?, tree_d, tree_r_last, tree_c, @@ -623,7 +646,7 @@ impl Labels { self.labels.is_empty() } - pub fn labels_for_layer(&self, layer: usize) -> DiskStore { + pub fn labels_for_layer(&self, layer: usize) -> Result> { assert!(layer != 0, "Layer cannot be 0"); assert!( layer <= self.layers(), @@ -636,11 +659,11 @@ impl Labels { let config = self.labels[row_index].clone(); assert!(config.size.is_some()); - DiskStore::new_from_disk(config.size.unwrap(), &config).unwrap() + DiskStore::new_from_disk(config.size.unwrap(), &config) } /// Returns label for the last layer. - pub fn labels_for_last_layer(&self) -> DiskStore { + pub fn labels_for_last_layer(&self) -> Result> { self.labels_for_layer(self.labels.len() - 1) } @@ -656,7 +679,7 @@ impl Labels { .iter() .map(|label| { assert!(label.size.is_some()); - let store = DiskStore::new_from_disk(label.size.unwrap(), &label).unwrap(); + let store = DiskStore::new_from_disk(label.size.unwrap(), &label)?; store.read_at(node as usize) }) .collect::>()?; @@ -679,16 +702,16 @@ impl LabelsCache { } } - pub fn new(labels: &Labels) -> Self { + pub fn new(labels: &Labels) -> Result { let mut disk_store_labels: Vec> = Vec::with_capacity(labels.len()); for i in 0..labels.len() { - disk_store_labels.push(labels.labels_for_layer(i + 1)); + disk_store_labels.push(labels.labels_for_layer(i + 1)?); } - LabelsCache { + Ok(LabelsCache { labels: disk_store_labels, _h: PhantomData, - } + }) } pub fn len(&self) -> usize { From 69ce83be25fcb7188dd6e34c95940d6a40f2da1a Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Thu, 28 Nov 2019 20:10:06 +0100 Subject: [PATCH 4/7] fix: less unwraps for stacked proof --- storage-proofs/src/stacked/proof.rs | 40 +++++++++++++++++++---------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/storage-proofs/src/stacked/proof.rs b/storage-proofs/src/stacked/proof.rs index ccdf15a34..832230aa5 100644 --- a/storage-proofs/src/stacked/proof.rs +++ b/storage-proofs/src/stacked/proof.rs @@ -101,7 +101,14 @@ impl<'a, H: 'static + Hasher, G: 'static + Hasher> StackedDrg<'a, H, G> { ) -> Result> { // Sanity checks on restored trees. assert!(pub_inputs.tau.is_some()); - assert_eq!(pub_inputs.tau.as_ref().unwrap().comm_d, t_aux.tree_d.root()); + assert_eq!( + pub_inputs + .tau + .as_ref() + .ok_or_else(|| anyhow!("no tau in inputs"))? + .comm_d, + t_aux.tree_d.root() + ); // Derive the set of challenges we are proving over. let config = &pub_params.config; @@ -456,9 +463,9 @@ impl<'a, H: 'static + Hasher, G: 'static + Hasher> StackedDrg<'a, H, G> { data.par_chunks_mut(pub_params.window_size_bytes()) .enumerate() - .for_each(|(window_index, data_chunk)| { - Self::extract_single_window(pub_params, replica_id, data_chunk, window_index); - }); + .try_for_each(|(window_index, data_chunk)| { + Self::extract_single_window(pub_params, replica_id, data_chunk, window_index) + })?; Ok(()) } @@ -490,7 +497,9 @@ impl<'a, H: 'static + Hasher, G: 'static + Hasher> StackedDrg<'a, H, G> { .skip(first_window_index) .flat_map(|(window_index, chunk)| { let mut decoded_chunk = chunk.to_vec(); - Self::extract_single_window(pp, replica_id, &mut decoded_chunk, window_index); + // TODO replace unwrap with proper error handling + Self::extract_single_window(pp, replica_id, &mut decoded_chunk, window_index) + .unwrap(); decoded_chunk }) .collect(); @@ -518,7 +527,7 @@ impl<'a, H: 'static + Hasher, G: 'static + Hasher> StackedDrg<'a, H, G> { replica_id: &::Domain, data_chunk: &mut [u8], window_index: usize, - ) { + ) -> Result<()> { trace!("extract_single_window"); let window_graph = &pub_params.window_graph; let layers = pub_params.config.layers(); @@ -557,11 +566,10 @@ impl<'a, H: 'static + Hasher, G: 'static + Hasher> StackedDrg<'a, H, G> { if layer == layers { // on the last layer we encode the data - let keyd = H::Domain::try_from_bytes(&key).unwrap(); + let keyd = H::Domain::try_from_bytes(&key)?; let data_node = H::Domain::try_from_bytes( &data_chunk[node * NODE_SIZE..(node + 1) * NODE_SIZE], - ) - .unwrap(); + )?; let decoded_node = decode::(keyd, data_node); data_chunk[node * NODE_SIZE..(node + 1) * NODE_SIZE].copy_from_slice(AsRef::< [u8], @@ -579,6 +587,8 @@ impl<'a, H: 'static + Hasher, G: 'static + Hasher> StackedDrg<'a, H, G> { } } } + + Ok(()) } fn label_encode_all_windows( @@ -639,7 +649,7 @@ impl<'a, H: 'static + Hasher, G: 'static + Hasher> StackedDrg<'a, H, G> { &mut layer_labels, data_chunk, window_index, - ); + )?; if layer < layers { if let Some(ref mut exp_parents_data) = exp_parents_data { @@ -676,7 +686,7 @@ impl<'a, H: 'static + Hasher, G: 'static + Hasher> StackedDrg<'a, H, G> { layer_labels: &mut [u8], data_chunk: &mut [u8], window_index: usize, - ) { + ) -> Result<()> { for node in 0..window_graph.size() { window_graph.parents(node, parents); @@ -697,12 +707,14 @@ impl<'a, H: 'static + Hasher, G: 'static + Hasher> StackedDrg<'a, H, G> { if layer == layers { // on the last layer we encode the data - let keyd = H::Domain::try_from_bytes(&key).unwrap(); - let data_node = H::Domain::try_from_bytes(&data_chunk[start..end]).unwrap(); + let keyd = H::Domain::try_from_bytes(&key)?; + let data_node = H::Domain::try_from_bytes(&data_chunk[start..end])?; let encoded_node = encode(keyd, data_node); data_chunk[start..end].copy_from_slice(AsRef::<[u8]>::as_ref(&encoded_node)); } } + + Ok(()) } fn build_tree(tree_data: &[u8], config: Option) -> Result> { @@ -714,6 +726,7 @@ impl<'a, H: 'static + Hasher, G: 'static + Hasher> StackedDrg<'a, H, G> { MerkleTree::from_par_iter_with_config( (0..leafs) .into_par_iter() + // TODO proper error handling instead of `unwrap()` .map(|i| get_node::(tree_data, i).unwrap()), config, ) @@ -721,6 +734,7 @@ impl<'a, H: 'static + Hasher, G: 'static + Hasher> StackedDrg<'a, H, G> { MerkleTree::from_par_iter( (0..leafs) .into_par_iter() + // TODO proper error handling instead of `unwrap()` .map(|i| get_node::(tree_data, i).unwrap()), ) } From 80159c58d20edcac053912a74514be13d9e2c992 Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Thu, 28 Nov 2019 21:46:53 +0100 Subject: [PATCH 5/7] fix: make create_label return a result instead of panicing --- storage-proofs/src/circuit/create_label.rs | 4 ++-- storage-proofs/src/crypto/create_label.rs | 7 ++++--- storage-proofs/src/hasher/blake2s.rs | 4 ++-- storage-proofs/src/hasher/pedersen.rs | 4 ++-- storage-proofs/src/hasher/sha256.rs | 4 ++-- storage-proofs/src/hasher/types.rs | 2 +- storage-proofs/src/test_helper.rs | 2 +- 7 files changed, 14 insertions(+), 13 deletions(-) diff --git a/storage-proofs/src/circuit/create_label.rs b/storage-proofs/src/circuit/create_label.rs index 1c4bf36aa..fb2f92c62 100644 --- a/storage-proofs/src/circuit/create_label.rs +++ b/storage-proofs/src/circuit/create_label.rs @@ -119,7 +119,7 @@ mod tests { acc }); - let expected = crypto::create_label::create_label(input_bytes.as_slice(), m); + let expected = crypto::create_label::create_label(input_bytes.as_slice(), m).unwrap(); assert_eq!( expected, @@ -178,7 +178,7 @@ mod tests { input_bytes.extend_from_slice(parent); } - let expected = crypto::create_label::create_label(input_bytes.as_slice(), m); + let expected = crypto::create_label::create_label(input_bytes.as_slice(), m).unwrap(); assert_eq!( expected, diff --git a/storage-proofs/src/crypto/create_label.rs b/storage-proofs/src/crypto/create_label.rs index ade89a469..168859eed 100644 --- a/storage-proofs/src/crypto/create_label.rs +++ b/storage-proofs/src/crypto/create_label.rs @@ -1,3 +1,4 @@ +use anyhow::Result; use ff::PrimeField; use paired::bls12_381::Fr; use sha2::{Digest, Sha256}; @@ -5,9 +6,9 @@ use sha2::{Digest, Sha256}; use crate::fr32::bytes_into_fr_repr_safe; /// Key derivation function, based on pedersen hashing. -pub fn create_label(data: &[u8], _m: usize) -> Fr { +pub fn create_label(data: &[u8], _m: usize) -> Result { let hash = Sha256::digest(data); - Fr::from_repr(bytes_into_fr_repr_safe(hash.as_ref())).unwrap() + Ok(Fr::from_repr(bytes_into_fr_repr_safe(hash.as_ref()))?) } #[cfg(test)] @@ -30,7 +31,7 @@ mod tests { ]; let expected = Fr::from_repr(FrRepr(repr)).unwrap(); - let res = create_label(&data, m); + let res = create_label(&data, m).unwrap(); assert_eq!(res, expected); } } diff --git a/storage-proofs/src/hasher/blake2s.rs b/storage-proofs/src/hasher/blake2s.rs index 713b44b85..b663731c9 100644 --- a/storage-proofs/src/hasher/blake2s.rs +++ b/storage-proofs/src/hasher/blake2s.rs @@ -26,7 +26,7 @@ impl Hasher for Blake2sHasher { "Blake2sHasher".into() } - fn create_label(data: &[u8], m: usize) -> Self::Domain { + fn create_label(data: &[u8], m: usize) -> Result { assert_eq!( data.len(), 32 * (1 + m), @@ -35,7 +35,7 @@ impl Hasher for Blake2sHasher { m ); - >::hash(data) + Ok(>::hash(data)) } fn sloth_encode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain { diff --git a/storage-proofs/src/hasher/pedersen.rs b/storage-proofs/src/hasher/pedersen.rs index 43d724fd2..d8998b8a4 100644 --- a/storage-proofs/src/hasher/pedersen.rs +++ b/storage-proofs/src/hasher/pedersen.rs @@ -27,8 +27,8 @@ impl Hasher for PedersenHasher { "PedersenHasher".into() } - fn create_label(data: &[u8], m: usize) -> Self::Domain { - create_label::create_label(data, m).into() + fn create_label(data: &[u8], m: usize) -> Result { + Ok(create_label::create_label(data, m)?.into()) } #[inline] diff --git a/storage-proofs/src/hasher/sha256.rs b/storage-proofs/src/hasher/sha256.rs index 626a68c1f..5aae4cb8f 100644 --- a/storage-proofs/src/hasher/sha256.rs +++ b/storage-proofs/src/hasher/sha256.rs @@ -26,8 +26,8 @@ impl Hasher for Sha256Hasher { "Sha256Hasher".into() } - fn create_label(data: &[u8], _m: usize) -> Self::Domain { - >::hash(data) + fn create_label(data: &[u8], _m: usize) -> Result { + Ok(>::hash(data)) } fn sloth_encode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain { diff --git a/storage-proofs/src/hasher/types.rs b/storage-proofs/src/hasher/types.rs index ecf770cc2..66c9a4791 100644 --- a/storage-proofs/src/hasher/types.rs +++ b/storage-proofs/src/hasher/types.rs @@ -73,7 +73,7 @@ pub trait Hasher: Clone + ::std::fmt::Debug + Eq + Default + Send + Sync { type Domain: Domain + LightHashable + AsRef; type Function: HashFunction; - fn create_label(data: &[u8], m: usize) -> Self::Domain; + fn create_label(data: &[u8], m: usize) -> Result; fn sloth_encode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain; fn sloth_decode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain; diff --git a/storage-proofs/src/test_helper.rs b/storage-proofs/src/test_helper.rs index cc602a32c..563538bc7 100644 --- a/storage-proofs/src/test_helper.rs +++ b/storage-proofs/src/test_helper.rs @@ -68,7 +68,7 @@ pub fn fake_drgpoprep_proof( ) .unwrap(); - let key = crypto::create_label::create_label(ciphertexts.as_slice(), m); + let key = crypto::create_label::create_label(ciphertexts.as_slice(), m).unwrap(); // run sloth(key, node) let replica_node: Fr = crypto::sloth::encode::(&key, &data_node); From 2456cc77af5d5d10f2f462164b731187a601be2c Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Thu, 28 Nov 2019 22:46:23 +0100 Subject: [PATCH 6/7] fix: make hashers panicing less --- storage-proofs/benches/encode.rs | 2 +- storage-proofs/src/drgporep.rs | 2 +- storage-proofs/src/hasher/blake2s.rs | 12 ++++++------ storage-proofs/src/hasher/pedersen.rs | 16 ++++++++-------- storage-proofs/src/hasher/sha256.rs | 12 ++++++------ storage-proofs/src/hasher/types.rs | 4 ++-- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/storage-proofs/benches/encode.rs b/storage-proofs/benches/encode.rs index 0a8533878..406c9a2f2 100644 --- a/storage-proofs/benches/encode.rs +++ b/storage-proofs/benches/encode.rs @@ -54,7 +54,7 @@ fn encode_single_node( let node_data = H::Domain::try_from_bytes(&data[start..end]).unwrap(); let key_data = H::Domain::try_from_bytes(&key).unwrap(); - let encoded = H::sloth_encode(&key_data, &node_data); + let encoded = H::sloth_encode(&key_data, &node_data).unwrap(); encoded.write_bytes(&mut data[start..end]).unwrap(); } diff --git a/storage-proofs/src/drgporep.rs b/storage-proofs/src/drgporep.rs index 3b2743ea6..7cf9a76a2 100644 --- a/storage-proofs/src/drgporep.rs +++ b/storage-proofs/src/drgporep.rs @@ -459,7 +459,7 @@ where let end = start + NODE_SIZE; let node_data = H::Domain::try_from_bytes(&data[start..end])?; - let encoded = H::sloth_encode(key.as_ref(), &node_data); + let encoded = H::sloth_encode(key.as_ref(), &node_data)?; encoded.write_bytes(&mut data[start..end])?; } diff --git a/storage-proofs/src/hasher/blake2s.rs b/storage-proofs/src/hasher/blake2s.rs index b663731c9..f83433d98 100644 --- a/storage-proofs/src/hasher/blake2s.rs +++ b/storage-proofs/src/hasher/blake2s.rs @@ -38,17 +38,17 @@ impl Hasher for Blake2sHasher { Ok(>::hash(data)) } - fn sloth_encode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain { + fn sloth_encode(key: &Self::Domain, ciphertext: &Self::Domain) -> Result { // TODO: validate this is how sloth should work in this case let k = (*key).into(); let c = (*ciphertext).into(); - sloth::encode::(&k, &c).into() + Ok(sloth::encode::(&k, &c).into()) } - fn sloth_decode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain { + fn sloth_decode(key: &Self::Domain, ciphertext: &Self::Domain) -> Result { // TODO: validate this is how sloth should work in this case - sloth::decode::(&(*key).into(), &(*ciphertext).into()).into() + Ok(sloth::decode::(&(*key).into(), &(*ciphertext).into()).into()) } } @@ -247,8 +247,8 @@ impl HashFunction for Blake2sFunction { Some(_) => { let bits = alloc_bits .iter() - .map(|v| v.get_value().unwrap()) - .collect::>(); + .map(|v| v.get_value().ok_or(SynthesisError::AssignmentMissing)) + .collect::, SynthesisError>>()?; // TODO: figure out if we can avoid this let frs = multipack::compute_multipacking::(&bits); Ok(frs[0]) diff --git a/storage-proofs/src/hasher/pedersen.rs b/storage-proofs/src/hasher/pedersen.rs index d8998b8a4..b5fc9b164 100644 --- a/storage-proofs/src/hasher/pedersen.rs +++ b/storage-proofs/src/hasher/pedersen.rs @@ -32,20 +32,20 @@ impl Hasher for PedersenHasher { } #[inline] - fn sloth_encode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain { + fn sloth_encode(key: &Self::Domain, ciphertext: &Self::Domain) -> Result { // Unrapping here is safe; `Fr` elements and hash domain elements are the same byte length. - let key = Fr::from_repr(key.0).unwrap(); - let ciphertext = Fr::from_repr(ciphertext.0).unwrap(); - sloth::encode::(&key, &ciphertext).into() + let key = Fr::from_repr(key.0)?; + let ciphertext = Fr::from_repr(ciphertext.0)?; + Ok(sloth::encode::(&key, &ciphertext).into()) } #[inline] - fn sloth_decode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain { + fn sloth_decode(key: &Self::Domain, ciphertext: &Self::Domain) -> Result { // Unrapping here is safe; `Fr` elements and hash domain elements are the same byte length. - let key = Fr::from_repr(key.0).unwrap(); - let ciphertext = Fr::from_repr(ciphertext.0).unwrap(); + let key = Fr::from_repr(key.0)?; + let ciphertext = Fr::from_repr(ciphertext.0)?; - sloth::decode::(&key, &ciphertext).into() + Ok(sloth::decode::(&key, &ciphertext).into()) } } diff --git a/storage-proofs/src/hasher/sha256.rs b/storage-proofs/src/hasher/sha256.rs index 5aae4cb8f..27b266175 100644 --- a/storage-proofs/src/hasher/sha256.rs +++ b/storage-proofs/src/hasher/sha256.rs @@ -30,17 +30,17 @@ impl Hasher for Sha256Hasher { Ok(>::hash(data)) } - fn sloth_encode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain { + fn sloth_encode(key: &Self::Domain, ciphertext: &Self::Domain) -> Result { // TODO: validate this is how sloth should work in this case let k = (*key).into(); let c = (*ciphertext).into(); - sloth::encode::(&k, &c).into() + Ok(sloth::encode::(&k, &c).into()) } - fn sloth_decode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain { + fn sloth_decode(key: &Self::Domain, ciphertext: &Self::Domain) -> Result { // TODO: validate this is how sloth should work in this case - sloth::decode::(&(*key).into(), &(*ciphertext).into()).into() + Ok(sloth::decode::(&(*key).into(), &(*ciphertext).into()).into()) } } @@ -223,8 +223,8 @@ impl HashFunction for Sha256Function { let fr = if alloc_bits[0].get_value().is_some() { let be_bits = alloc_bits .iter() - .map(|v| v.get_value().unwrap()) - .collect::>(); + .map(|v| v.get_value().ok_or(SynthesisError::AssignmentMissing)) + .collect::, SynthesisError>>()?; let le_bits = be_bits .chunks(8) diff --git a/storage-proofs/src/hasher/types.rs b/storage-proofs/src/hasher/types.rs index 66c9a4791..1c97f207d 100644 --- a/storage-proofs/src/hasher/types.rs +++ b/storage-proofs/src/hasher/types.rs @@ -74,8 +74,8 @@ pub trait Hasher: Clone + ::std::fmt::Debug + Eq + Default + Send + Sync { type Function: HashFunction; fn create_label(data: &[u8], m: usize) -> Result; - fn sloth_encode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain; - fn sloth_decode(key: &Self::Domain, ciphertext: &Self::Domain) -> Self::Domain; + fn sloth_encode(key: &Self::Domain, ciphertext: &Self::Domain) -> Result; + fn sloth_decode(key: &Self::Domain, ciphertext: &Self::Domain) -> Result; fn name() -> String; } From 514a64bc2448551e4905999e405728fa225931fd Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Thu, 28 Nov 2019 22:54:59 +0100 Subject: [PATCH 7/7] fix: make curcuit code less panicy --- storage-proofs/src/circuit/create_label.rs | 4 ++-- storage-proofs/src/circuit/variables.rs | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/storage-proofs/src/circuit/create_label.rs b/storage-proofs/src/circuit/create_label.rs index fb2f92c62..090b8c1b3 100644 --- a/storage-proofs/src/circuit/create_label.rs +++ b/storage-proofs/src/circuit/create_label.rs @@ -44,8 +44,8 @@ where let fr = if alloc_bits[0].get_value().is_some() { let be_bits = alloc_bits .iter() - .map(|v| v.get_value().unwrap()) - .collect::>(); + .map(|v| v.get_value().ok_or(SynthesisError::AssignmentMissing)) + .collect::, SynthesisError>>()?; let le_bits = be_bits .chunks(8) diff --git a/storage-proofs/src/circuit/variables.rs b/storage-proofs/src/circuit/variables.rs index 345a30915..5b6ed606b 100644 --- a/storage-proofs/src/circuit/variables.rs +++ b/storage-proofs/src/circuit/variables.rs @@ -1,5 +1,7 @@ use std::fmt; +use anyhow::Result; + use bellperson::gadgets::num::AllocatedNum; use bellperson::{ConstraintSystem, SynthesisError}; use paired::Engine; @@ -39,8 +41,8 @@ impl Root { } } - pub fn var>(cs: CS, fr: E::Fr) -> Self { - Root::Var(AllocatedNum::alloc(cs, || Ok(fr)).unwrap()) + pub fn var>(cs: CS, fr: E::Fr) -> Result { + Ok(Root::Var(AllocatedNum::alloc(cs, || Ok(fr))?)) } pub fn is_some(&self) -> bool {