-
Notifications
You must be signed in to change notification settings - Fork 28
feat: added functionality for 100 security bits #169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
Updated mathematical expressions and formatting in SECURITY_BITS.md for clarity and consistency.
| ); | ||
|
|
||
| let mut it = transcript_challenges.as_chunks::<4>().0.iter(); | ||
| let mut it = if POW_CONFIG.lookup_pow_bits > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it happens often, do you want to also move it into function (and mark "inline always")?
| [MaybeUninit::uninit().assume_init(); NUM_FRI_STEPS]; | ||
|
|
||
| for (caps, challenge) in skeleton | ||
| for (((caps, challenge), pow_challenge), pow_bits) in skeleton |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we have too many zip-s here, and we know that iterator lengths should be same size, it's better to change it into index-based iteration
verifier_common/src/lib.rs
Outdated
| #[cfg(feature = "security_100")] | ||
| pub const MEMORY_DELEGATION_POW_BITS: usize = pow_bits_for_memory_and_delegation( | ||
| SECURITY_BITS, | ||
| cs::one_row_compiler::MAX_NUMBER_OF_CYCLES.leading_zeros() as usize, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it build in no-std? If we import cs as "definitions only", then "one row compiler" module is not available? Also, leading zeroes are very misleading. If the number of cycles is not a power of two, instead it's better to take "next power of two", and then trailing zeroes
| @@ -0,0 +1,440 @@ | |||
| pub const MERSENNE31QUARTIC_SIZE_LOG2: usize = 124; // Mersenne31Quartic size in bits | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think part of it (like calculator functions and "sized" structures) should also move somewhere under "definitions", otherwise it would be unavailable to no-std verifiers. Structures that use vector should stay here
| let bits = security_bits - pow_bits; | ||
| let init_res = bits.div_ceil(lde_factor_log2); | ||
|
|
||
| // We should add extra 20% of queries |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please expand this comment (or link to some other doc) why
| domain_size_log2: usize, | ||
| field_size_log2: usize, | ||
| ) -> usize { | ||
| let no_pow_security_bits = field_size_log2 - domain_size_log2 - 5; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do 5 and 2 (below) come from?
| num_queries: usize, | ||
| lde_factor_log2: usize, | ||
| ) -> usize { | ||
| // We should add extra 20% of queries |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should check that num_queries + num_queries.div_ceil(5) always matches the line below backwards. It's now harder to count number of bits from FRI rate after tightened conjecture bounds, so you should link from here too to where we come up with 20% while still using full conjecture security bits per query
| } | ||
|
|
||
| pub use worst_case_constants::{worst_pow_config, worst_sized_pow_config}; | ||
| mod worst_case_constants { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one I would move to verifier_common
shamatar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor structural changes and few better comments
|
@olesHolem can you merge the latest commits to dev? There were some significant changes to the GPU prover, it would be better if my PoW changes were done against the actual version. |
|
|
||
| let mut seed = Transcript::commit_initial(&transcript_input); | ||
|
|
||
| let pow_bits = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move all parameters to the caller's decision (into function parameters)? It should specify number of queries and PoW parameters. How many security bits do those parameters give should be caller's responsibility
# Conflicts: # prover/Cargo.toml
# Conflicts: # gpu_prover/src/execution/gpu_worker.rs
What ❔
Added functionality for controlling security bits using PoW right before drawing new challenge. Added functions to
prover/src/prover_stages/pow_bits_calculator.rsto determine number PoW bits for each challenge in prover. Prover computes these parameters from its inputs (circuit, lde_factor, etc.) and verifier from its constant parameters. Also, we can compute "worst case scenario" parameters and reuse them for all circuits.