Zook : Parameter Selection #257
Conversation
|
@ocdbytes Can you please resolve the merge conflicts on this PR? I'm reviewing it and we are planning to merge it by this week. |
Resolve conflicts with the buffer abstraction while preserving parameter-selection protocol semantics.
|
Hey @shreyas-londhe, this branch did not have any conflicts with main before merging buffer abstraction. I opened a PR to this branch here ocdbytes#11, which adds buffer abstraction. Merging it will make this branch conflict free again. cc @ocdbytes |
Resolve conflicts with buffer abstraction
|
Thanks @zkfriendly I have merged your PR now there are no conflicts. I skimmed through the changes will go through them properly tomorrow to see if there is anything to be changed. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
| #[derive(Clone, Debug)] | ||
| pub struct ProtocolConfig<M: Embedding> { | ||
| security: SecuritySpec, | ||
| tuning: TuningSpec, | ||
| rounds: Vec<RoundConfig<M>>, | ||
| basecase: BasecasePlan<M::Target>, | ||
| } |
There was a problem hiding this comment.
Can ProtocolConfig derive Serialize/Deserialize? It's #[derive(Clone, Debug)] today, so nothing downstream can embed it in a serialized scheme.
Context for why I'm asking: ProveKit builds its Fiat-Shamir domain separator by serializing the whole scheme (WHIR config included) and hashing that. whir::Config already handles it, with Serialize/Deserialize and #[serde(bound = "")] for the embedding generic. Once this stack has a prover/verifier runtime and replaces whir::Config as what schemes carry, ProtocolConfig will need the same.
It's not a one-liner though. RoundConfig, RoundMode, MaskOracleConfig, BasecasePlan in this file, plus Solved and CodeSwitchConfig, all need it too, and none of them derive it yet (SecuritySpec and TuningSpec already do). The embedding-generic ones want #[serde(bound = "")] like whir::Config.
There was a problem hiding this comment.
Hey @BornPsych
I have added these derives in wiring PR (next PR). Ig missed it here in this PR
| MCA_FIELD_BITS, | ||
| ); | ||
| let bcss25_const = (2.0 * 10.5_f64.powi(5) / 3.0).log2(); | ||
| let expected = bcss25_const + (MCA_MESSAGE_LENGTH as f64).log2() + 2.5 * MCA_LOG_INV_RATE |
There was a problem hiding this comment.
issue (non-blocking): expected is built from the same expression as the implementation, so this test can't catch a transcription error in the bound itself.
A wrong constant or exponent in eps_mca_log2 would update both sides equally and the test would stay green. Since this file is where the soundness math lives, I'd anchor at least one case per regime to a hand-computed number: fix k, ρ, and |F|, work out the bits from the theorem by hand, and assert against that literal with the derivation in a comment. The analytic_error_*_formula tests in the solver modules and privacy_error_bits_matches_bound_3_sum in derive.rs have the same shape.
| let regime = DecodingRegimeParams::from_policy(decoding_regime, rate); | ||
| // Query error is (1 - δ)^q in bits = -q · log2(1 - δ). | ||
| let log_one_minus_delta = regime.one_minus_distance_log2(-rate.log2()); | ||
| let q = (security_target / -log_one_minus_delta).ceil() as usize; |
There was a problem hiding this comment.
issue (non-blocking): num_in_domain_queries divides by zero when rate == 1.
At log_inv_rate = 0 the Unique branch gives 1 − δ = 1, so log_one_minus_delta is 0 and q saturates to usize::MAX through the ceil() as usize cast. TuningSpec doesn't reject starting_log_inv_rate == 0 and -r 0 sets it directly from the CLI, so this is reachable from the binary. A starting_log_inv_rate ≥ 1 check in round_layout (or a new LayoutError variant) would turn this into a clean error.
| let error = match self { | ||
| Self::Unique => log_k + log_inv_rate, | ||
| Self::Johnson { slack } => { | ||
| debug_assert!( |
There was a problem hiding this comment.
suggestion (non-blocking): keep these as hard assert!s like the pre-PR code.
The baseline enforced the η lower bound with assert!, so release builds kept the backstop. The DecodingRegimeParams variants are pub, so a future caller can construct Johnson { slack } directly with an out-of-range η and silently get an over-optimistic error bound in release. The check is two float ops on a cold path, so keeping it hard costs nothing. Same applies to the Capacity guard on line 121.
4 participants
Changes :
Tests :
This PR was squashed from 47 commits during conflict resolution against PR #251 (merged to
main)Commit history maintained in personal fork.