From 1026d6e31df50571e51db6e651de4e69899f342f Mon Sep 17 00:00:00 2001 From: nemo Date: Mon, 15 Jun 2020 15:36:51 -0400 Subject: [PATCH 1/3] fix: change default rows_to_discard for oct trees feat: move default value from merkle_light to internal --- fil-proofs-tooling/src/bin/benchy/stacked.rs | 3 +- filecoin-proofs/src/api/mod.rs | 3 +- filecoin-proofs/src/api/post.rs | 5 +-- filecoin-proofs/src/api/seal.rs | 11 +++---- storage-proofs/core/src/merkle/builders.rs | 4 +-- storage-proofs/core/src/settings.rs | 4 +-- storage-proofs/core/src/util.rs | 33 +++++++++++++++++++ storage-proofs/porep/src/drg/circuit.rs | 9 ++--- storage-proofs/porep/src/drg/compound.rs | 8 ++--- storage-proofs/porep/src/drg/vanilla.rs | 13 +++----- .../porep/src/stacked/circuit/proof.rs | 5 +-- .../porep/src/stacked/vanilla/proof.rs | 29 ++++++++-------- 12 files changed, 76 insertions(+), 51 deletions(-) diff --git a/fil-proofs-tooling/src/bin/benchy/stacked.rs b/fil-proofs-tooling/src/bin/benchy/stacked.rs index 60f5d7826..9243ce38f 100644 --- a/fil-proofs-tooling/src/bin/benchy/stacked.rs +++ b/fil-proofs-tooling/src/bin/benchy/stacked.rs @@ -26,6 +26,7 @@ use storage_proofs::porep::stacked::{ use storage_proofs::porep::PoRep; use storage_proofs::proof::ProofScheme; use storage_proofs::test_helper::setup_replica; +use storage_proofs::util::default_rows_to_discard; use tempfile::TempDir; fn dump_proof_bytes( @@ -116,7 +117,7 @@ where let config = StoreConfig::new( cache_dir.path(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard(nodes, BINARY_ARITY), + default_rows_to_discard(nodes, BINARY_ARITY), ); let replica_id = H::Domain::random(rng); diff --git a/filecoin-proofs/src/api/mod.rs b/filecoin-proofs/src/api/mod.rs index a55b509cf..ae40bd0be 100644 --- a/filecoin-proofs/src/api/mod.rs +++ b/filecoin-proofs/src/api/mod.rs @@ -14,6 +14,7 @@ use storage_proofs::porep::stacked::{ }; use storage_proofs::porep::PoRep; use storage_proofs::sector::SectorId; +use storage_proofs::util::default_rows_to_discard; use typenum::Unsigned; use crate::api::util::{as_safe_commitment, get_base_tree_leafs, get_base_tree_size}; @@ -149,7 +150,7 @@ where let config = StoreConfig::new( cache_path.as_ref(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard( + default_rows_to_discard( base_tree_leafs, ::Arity::to_usize(), ), diff --git a/filecoin-proofs/src/api/post.rs b/filecoin-proofs/src/api/post.rs index f48ac2cb4..49dd1dcff 100644 --- a/filecoin-proofs/src/api/post.rs +++ b/filecoin-proofs/src/api/post.rs @@ -17,6 +17,7 @@ use storage_proofs::merkle::{ use storage_proofs::multi_proof::MultiProof; use storage_proofs::post::fallback; use storage_proofs::sector::*; +use storage_proofs::util::default_rows_to_discard; use crate::api::util::{as_safe_commitment, get_base_tree_leafs, get_base_tree_size}; use crate::caches::{get_post_params, get_post_verifying_key}; @@ -149,7 +150,7 @@ impl PrivateReplicaInfo { "post: base tree size {}, base tree leafs {}, rows_to_discard {}, arities [{}, {}, {}]", base_tree_size, base_tree_leafs, - StoreConfig::default_rows_to_discard(base_tree_leafs, Tree::Arity::to_usize()), + default_rows_to_discard(base_tree_leafs, Tree::Arity::to_usize()), Tree::Arity::to_usize(), Tree::SubTreeArity::to_usize(), Tree::TopTreeArity::to_usize(), @@ -158,7 +159,7 @@ impl PrivateReplicaInfo { let mut config = StoreConfig::new( self.cache_dir_path(), CacheKey::CommRLastTree.to_string(), - StoreConfig::default_rows_to_discard(base_tree_leafs, Tree::Arity::to_usize()), + default_rows_to_discard(base_tree_leafs, Tree::Arity::to_usize()), ); config.size = Some(base_tree_size); diff --git a/filecoin-proofs/src/api/seal.rs b/filecoin-proofs/src/api/seal.rs index 817b94cbd..ca8d3365b 100644 --- a/filecoin-proofs/src/api/seal.rs +++ b/filecoin-proofs/src/api/seal.rs @@ -21,6 +21,7 @@ use storage_proofs::porep::stacked::{ }; use storage_proofs::proof::ProofScheme; use storage_proofs::sector::SectorId; +use storage_proofs::util::default_rows_to_discard; use crate::api::util::{ as_safe_commitment, commitment_from_fr, get_base_tree_leafs, get_base_tree_size, @@ -127,11 +128,10 @@ where ); trace!( - "seal phase 1: sector_size {}, base tree size {}, base tree leafs {}, rows to discard {}", + "seal phase 1: sector_size {}, base tree size {}, base tree leafs {}", u64::from(porep_config.sector_size), base_tree_size, base_tree_leafs, - StoreConfig::default_rows_to_discard(base_tree_leafs, BINARY_ARITY) ); // MT for original data is always named tree-d, and it will be @@ -139,7 +139,7 @@ where let mut config = StoreConfig::new( cache_path.as_ref(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard(base_tree_leafs, BINARY_ARITY), + default_rows_to_discard(base_tree_leafs, BINARY_ARITY), ); let data_tree = create_base_merkle_tree::>( Some(config.clone()), @@ -247,11 +247,10 @@ where "seal phase 2: base tree size {}, base tree leafs {}, rows to discard {}", base_tree_size, base_tree_leafs, - StoreConfig::default_rows_to_discard(base_tree_leafs, BINARY_ARITY) + default_rows_to_discard(base_tree_leafs, BINARY_ARITY) ); ensure!( - config.rows_to_discard - == StoreConfig::default_rows_to_discard(base_tree_leafs, BINARY_ARITY), + config.rows_to_discard == default_rows_to_discard(base_tree_leafs, BINARY_ARITY), "Invalid cache size specified" ); diff --git a/storage-proofs/core/src/merkle/builders.rs b/storage-proofs/core/src/merkle/builders.rs index 12d26ae56..ccf41ad69 100644 --- a/storage-proofs/core/src/merkle/builders.rs +++ b/storage-proofs/core/src/merkle/builders.rs @@ -13,7 +13,7 @@ use rayon::prelude::*; use crate::error::*; use crate::hasher::{Domain, Hasher, PoseidonArity}; -use crate::util::{data_at_node, NODE_SIZE}; +use crate::util::{data_at_node, default_rows_to_discard, NODE_SIZE}; use super::*; @@ -409,7 +409,7 @@ where let config = StoreConfig::new( &temp_path, format!("test-lc-tree-{}", id), - StoreConfig::default_rows_to_discard(nodes, Tree::Arity::to_usize()), + default_rows_to_discard(nodes, Tree::Arity::to_usize()), ); let mut tree = diff --git a/storage-proofs/core/src/settings.rs b/storage-proofs/core/src/settings.rs index a591657f2..c22dae6ad 100644 --- a/storage-proofs/core/src/settings.rs +++ b/storage-proofs/core/src/settings.rs @@ -21,7 +21,7 @@ pub struct Settings { pub column_write_batch_size: u32, pub use_gpu_tree_builder: bool, pub max_gpu_tree_batch_size: u32, - pub rows_to_discard: u32, + pub oct_rows_to_discard: u32, } impl Default for Settings { @@ -34,7 +34,7 @@ impl Default for Settings { column_write_batch_size: 262_144, use_gpu_tree_builder: false, max_gpu_tree_batch_size: 700_000, - rows_to_discard: 0, + oct_rows_to_discard: 2, } } } diff --git a/storage-proofs/core/src/util.rs b/storage-proofs/core/src/util.rs index 68b69ce47..5996086c4 100644 --- a/storage-proofs/core/src/util.rs +++ b/storage-proofs/core/src/util.rs @@ -2,8 +2,11 @@ use crate::error; use anyhow::ensure; use bellperson::gadgets::boolean::{self, AllocatedBit, Boolean}; use bellperson::{ConstraintSystem, SynthesisError}; +use merkletree::merkle::get_merkle_tree_row_count; use paired::Engine; +use super::settings; + pub const NODE_SIZE: usize = 32; /// Returns the start position of the data, 0-indexed. @@ -142,6 +145,36 @@ pub fn reverse_bit_numbering(bits: Vec) -> Vec usize { + let row_count = get_merkle_tree_row_count(leafs, arity); + if row_count <= 2 { + // If a tree only has a root row and/or base, there is + // nothing to discard. + return 0; + } else if row_count == 3 { + // If a tree only has 1 row between the base and root, + // it's all that can be discarded. + return 1; + } + + // row_count - 2 discounts the base layer (1) and root (1) + let max_rows_to_discard = row_count - 2; + + // This configurable setting is for a default oct-tree + // rows_to_discard value, which defaults to 2. + let rows_to_discard = settings::SETTINGS.lock().unwrap().oct_rows_to_discard as usize; + + // Discard at most 'constant value' rows (coded below, + // differing by arity) while respecting the max number that + // the tree can support discarding. + match arity { + 2 => std::cmp::min(max_rows_to_discard, 7), + 4 => std::cmp::min(max_rows_to_discard, 5), + _ => std::cmp::min(max_rows_to_discard, rows_to_discard), + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/storage-proofs/porep/src/drg/circuit.rs b/storage-proofs/porep/src/drg/circuit.rs index b58c8a633..964e7b7de 100644 --- a/storage-proofs/porep/src/drg/circuit.rs +++ b/storage-proofs/porep/src/drg/circuit.rs @@ -321,7 +321,7 @@ mod tests { merkle::MerkleProofTrait, proof::ProofScheme, test_helper::setup_replica, - util::data_at_node, + util::{data_at_node, default_rows_to_discard}, }; use super::super::compound::DrgPoRepCompound; @@ -349,7 +349,7 @@ mod tests { let config = StoreConfig::new( cache_dir.path(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard(nodes, BINARY_ARITY), + default_rows_to_discard(nodes, BINARY_ARITY), ); // Generate a replica path. @@ -395,10 +395,7 @@ mod tests { let priv_inputs = drg::PrivateInputs:: { tree_d: &aux.tree_d, tree_r: &aux.tree_r, - tree_r_config_rows_to_discard: StoreConfig::default_rows_to_discard( - nodes, - BINARY_ARITY, - ), + tree_r_config_rows_to_discard: default_rows_to_discard(nodes, BINARY_ARITY), }; let proof_nc = drg::DrgPoRep::::prove(&pp, &pub_inputs, &priv_inputs) diff --git a/storage-proofs/porep/src/drg/compound.rs b/storage-proofs/porep/src/drg/compound.rs index fce0a0d71..95d761110 100644 --- a/storage-proofs/porep/src/drg/compound.rs +++ b/storage-proofs/porep/src/drg/compound.rs @@ -296,6 +296,7 @@ mod tests { merkle::{BinaryMerkleTree, MerkleTreeTrait}, proof::NoRequirements, test_helper::setup_replica, + util::default_rows_to_discard, }; use crate::stacked::BINARY_ARITY; @@ -335,7 +336,7 @@ mod tests { let config = StoreConfig::new( cache_dir.path(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard(nodes, BINARY_ARITY), + default_rows_to_discard(nodes, BINARY_ARITY), ); // Generate a replica path. @@ -380,10 +381,7 @@ mod tests { let private_inputs = drg::PrivateInputs { tree_d: &aux.tree_d, tree_r: &aux.tree_r, - tree_r_config_rows_to_discard: StoreConfig::default_rows_to_discard( - nodes, - BINARY_ARITY, - ), + tree_r_config_rows_to_discard: default_rows_to_discard(nodes, BINARY_ARITY), }; // This duplication is necessary so public_params don't outlive public_inputs and private_inputs. diff --git a/storage-proofs/porep/src/drg/vanilla.rs b/storage-proofs/porep/src/drg/vanilla.rs index 30dec163f..ce1692fa7 100644 --- a/storage-proofs/porep/src/drg/vanilla.rs +++ b/storage-proofs/porep/src/drg/vanilla.rs @@ -619,7 +619,7 @@ mod tests { merkle::{BinaryMerkleTree, MerkleTreeTrait}, table_tests, test_helper::setup_replica, - util::data_at_node, + util::{data_at_node, default_rows_to_discard}, }; use tempfile; @@ -639,7 +639,7 @@ mod tests { let config = StoreConfig::new( cache_dir.path(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard(nodes, BINARY_ARITY), + default_rows_to_discard(nodes, BINARY_ARITY), ); // Generate a replica path. @@ -718,7 +718,7 @@ mod tests { let config = StoreConfig::new( cache_dir.path(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard(nodes, BINARY_ARITY), + default_rows_to_discard(nodes, BINARY_ARITY), ); // Generate a replica path. @@ -809,7 +809,7 @@ mod tests { let config = StoreConfig::new( cache_dir.path(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard(nodes, BINARY_ARITY), + default_rows_to_discard(nodes, BINARY_ARITY), ); // Generate a replica path. @@ -854,10 +854,7 @@ mod tests { let priv_inputs = PrivateInputs:: { tree_d: &aux.tree_d, tree_r: &aux.tree_r, - tree_r_config_rows_to_discard: StoreConfig::default_rows_to_discard( - nodes, - BINARY_ARITY, - ), + tree_r_config_rows_to_discard: default_rows_to_discard(nodes, BINARY_ARITY), }; let real_proof = DrgPoRep::::prove(&pp, &pub_inputs, &priv_inputs) diff --git a/storage-proofs/porep/src/stacked/circuit/proof.rs b/storage-proofs/porep/src/stacked/circuit/proof.rs index 02e83699f..cc943b98a 100644 --- a/storage-proofs/porep/src/stacked/circuit/proof.rs +++ b/storage-proofs/porep/src/stacked/circuit/proof.rs @@ -356,6 +356,7 @@ mod tests { merkle::{get_base_tree_count, DiskTree, MerkleTreeTrait}, proof::ProofScheme, test_helper::setup_replica, + util::default_rows_to_discard, }; use crate::stacked::{ @@ -412,7 +413,7 @@ mod tests { let config = StoreConfig::new( cache_dir.path(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard(nodes, BINARY_ARITY), + default_rows_to_discard(nodes, BINARY_ARITY), ); // Generate a replica path. @@ -608,7 +609,7 @@ mod tests { let config = StoreConfig::new( cache_dir.path(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard(nodes, BINARY_ARITY), + default_rows_to_discard(nodes, BINARY_ARITY), ); // Generate a replica path. diff --git a/storage-proofs/porep/src/stacked/vanilla/proof.rs b/storage-proofs/porep/src/stacked/vanilla/proof.rs index a3161aa18..e0fbb06fc 100644 --- a/storage-proofs/porep/src/stacked/vanilla/proof.rs +++ b/storage-proofs/porep/src/stacked/vanilla/proof.rs @@ -25,7 +25,7 @@ use storage_proofs_core::{ }, merkle::*, settings, - util::NODE_SIZE, + util::{default_rows_to_discard, NODE_SIZE}, }; use typenum::{U11, U2, U8}; @@ -973,8 +973,7 @@ impl<'a, Tree: 'static + MerkleTreeTrait, G: 'static + Hasher> StackedDrg<'a, Tr CacheKey::CommDTree.to_string(), Some(get_merkle_tree_len(nodes_count, BINARY_ARITY)?), ); - tree_d_config.rows_to_discard = - StoreConfig::default_rows_to_discard(nodes_count, BINARY_ARITY); + tree_d_config.rows_to_discard = default_rows_to_discard(nodes_count, BINARY_ARITY); let mut tree_r_last_config = StoreConfig::from_config( &config, @@ -982,17 +981,15 @@ impl<'a, Tree: 'static + MerkleTreeTrait, G: 'static + Hasher> StackedDrg<'a, Tr Some(get_merkle_tree_len(nodes_count, Tree::Arity::to_usize())?), ); - // A default 'rows_to_discard' value will be chosen for - // tree_r_last, unless the user overrides this value via the - // environment setting (FIL_PROOFS_ROWS_TO_DISCARD). If this - // value is specified, no checking is done on it and it may + // A default 'rows_to_discard' value will be chosen for tree_r_last, unless the user overrides this value via the + // environment setting (FIL_PROOFS_ROWS_TO_DISCARD). If this value is specified, no checking is done on it and it may // result in a broken configuration. Use with caution. - let env_rows_to_discard = settings::SETTINGS.lock().unwrap().rows_to_discard as usize; - tree_r_last_config.rows_to_discard = if env_rows_to_discard != 0 { - env_rows_to_discard - } else { - StoreConfig::default_rows_to_discard(nodes_count, Tree::Arity::to_usize()) - }; + tree_r_last_config.rows_to_discard = + default_rows_to_discard(nodes_count, Tree::Arity::to_usize()); + trace!( + "tree_r_last using rows_to_discard={}", + tree_r_last_config.rows_to_discard + ); let mut tree_c_config = StoreConfig::from_config( &config, @@ -1000,7 +997,7 @@ impl<'a, Tree: 'static + MerkleTreeTrait, G: 'static + Hasher> StackedDrg<'a, Tr Some(get_merkle_tree_len(nodes_count, Tree::Arity::to_usize())?), ); tree_c_config.rows_to_discard = - StoreConfig::default_rows_to_discard(nodes_count, Tree::Arity::to_usize()); + default_rows_to_discard(nodes_count, Tree::Arity::to_usize()); let labels = LabelsCache::::new(&label_configs)?; let configs = split_config(tree_c_config.clone(), tree_count)?; @@ -1264,7 +1261,7 @@ mod tests { let config = StoreConfig::new( cache_dir.path(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard(nodes, BINARY_ARITY), + default_rows_to_discard(nodes, BINARY_ARITY), ); // Generate a replica path. @@ -1441,7 +1438,7 @@ mod tests { let config = StoreConfig::new( cache_dir.path(), CacheKey::CommDTree.to_string(), - StoreConfig::default_rows_to_discard(nodes, BINARY_ARITY), + default_rows_to_discard(nodes, BINARY_ARITY), ); // Generate a replica path. From efc5740631e8a6e76173ca32ec372f990957eabd Mon Sep 17 00:00:00 2001 From: nemo Date: Mon, 15 Jun 2020 15:43:51 -0400 Subject: [PATCH 2/3] fix: update comment --- storage-proofs/porep/src/stacked/vanilla/proof.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage-proofs/porep/src/stacked/vanilla/proof.rs b/storage-proofs/porep/src/stacked/vanilla/proof.rs index e0fbb06fc..3110ceee2 100644 --- a/storage-proofs/porep/src/stacked/vanilla/proof.rs +++ b/storage-proofs/porep/src/stacked/vanilla/proof.rs @@ -982,7 +982,7 @@ impl<'a, Tree: 'static + MerkleTreeTrait, G: 'static + Hasher> StackedDrg<'a, Tr ); // A default 'rows_to_discard' value will be chosen for tree_r_last, unless the user overrides this value via the - // environment setting (FIL_PROOFS_ROWS_TO_DISCARD). If this value is specified, no checking is done on it and it may + // environment setting (FIL_PROOFS_OCT_ROWS_TO_DISCARD). If this value is specified, no checking is done on it and it may // result in a broken configuration. Use with caution. tree_r_last_config.rows_to_discard = default_rows_to_discard(nodes_count, Tree::Arity::to_usize()); From 204600f273cc3a7b58e1ccd726a5dd82674b63b8 Mon Sep 17 00:00:00 2001 From: nemo Date: Mon, 15 Jun 2020 16:02:00 -0400 Subject: [PATCH 3/3] fix: apply some review feedback --- storage-proofs/core/src/settings.rs | 4 ++-- storage-proofs/core/src/util.rs | 2 +- storage-proofs/porep/src/stacked/vanilla/proof.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/storage-proofs/core/src/settings.rs b/storage-proofs/core/src/settings.rs index c22dae6ad..1b32d3abc 100644 --- a/storage-proofs/core/src/settings.rs +++ b/storage-proofs/core/src/settings.rs @@ -21,7 +21,7 @@ pub struct Settings { pub column_write_batch_size: u32, pub use_gpu_tree_builder: bool, pub max_gpu_tree_batch_size: u32, - pub oct_rows_to_discard: u32, + pub rows_to_discard: u32, } impl Default for Settings { @@ -34,7 +34,7 @@ impl Default for Settings { column_write_batch_size: 262_144, use_gpu_tree_builder: false, max_gpu_tree_batch_size: 700_000, - oct_rows_to_discard: 2, + rows_to_discard: 2, } } } diff --git a/storage-proofs/core/src/util.rs b/storage-proofs/core/src/util.rs index 5996086c4..b42ac7ff9 100644 --- a/storage-proofs/core/src/util.rs +++ b/storage-proofs/core/src/util.rs @@ -163,7 +163,7 @@ pub fn default_rows_to_discard(leafs: usize, arity: usize) -> usize { // This configurable setting is for a default oct-tree // rows_to_discard value, which defaults to 2. - let rows_to_discard = settings::SETTINGS.lock().unwrap().oct_rows_to_discard as usize; + let rows_to_discard = settings::SETTINGS.lock().unwrap().rows_to_discard as usize; // Discard at most 'constant value' rows (coded below, // differing by arity) while respecting the max number that diff --git a/storage-proofs/porep/src/stacked/vanilla/proof.rs b/storage-proofs/porep/src/stacked/vanilla/proof.rs index 3110ceee2..e0fbb06fc 100644 --- a/storage-proofs/porep/src/stacked/vanilla/proof.rs +++ b/storage-proofs/porep/src/stacked/vanilla/proof.rs @@ -982,7 +982,7 @@ impl<'a, Tree: 'static + MerkleTreeTrait, G: 'static + Hasher> StackedDrg<'a, Tr ); // A default 'rows_to_discard' value will be chosen for tree_r_last, unless the user overrides this value via the - // environment setting (FIL_PROOFS_OCT_ROWS_TO_DISCARD). If this value is specified, no checking is done on it and it may + // environment setting (FIL_PROOFS_ROWS_TO_DISCARD). If this value is specified, no checking is done on it and it may // result in a broken configuration. Use with caution. tree_r_last_config.rows_to_discard = default_rows_to_discard(nodes_count, Tree::Arity::to_usize());