Improve Falcon DSA verification procedure #1623
Conversation
| /// | ||
| /// Outputs: | ||
| /// Operand stack: [a1, a0, ...] | ||
| /// Advice stack: [q0, q1, r, ...] |
There was a problem hiding this comment.
| /// Advice stack: [q0, q1, r, ...] | |
| /// Advice stack: [q1, q0, r, ...] |
Based on the implementation
There was a problem hiding this comment.
I think we want that adv_push.1 pushes q0 first and when we call again adv_push.1 it pushes q1.
| padw | ||
| dup.12 | ||
| dup.12 add.1281 |
There was a problem hiding this comment.
Can we make 1281 a constant to ease readability (even if it's documented in the docstring)?
| padw | ||
| dup.4 add.1 swap.5 | ||
| dup.4 | ||
| mem_loadw | ||
|
|
||
| repeat.4 | ||
| exec.norm_sq | ||
| movdn.4 | ||
| end | ||
|
|
||
| exec.norm_sq | ||
| swap | ||
| exec.norm_sq | ||
| add | ||
| swap | ||
| exec.norm_sq | ||
| add | ||
| swap | ||
| exec.norm_sq | ||
| add | ||
| swap | ||
| add.1 |
There was a problem hiding this comment.
Could you add comments that tracks what happens on the stack in this block?
There was a problem hiding this comment.
Sure, added comments to help track what's happening
stdlib/tests/crypto/falcon.rs
Outdated
| let v = rand::thread_rng().gen_range(0..Felt::MODULUS); | ||
| let (v_lo, v_hi) = (v as u32, v >> 32); | ||
| let w = rand::thread_rng().gen_range(0..J); | ||
| let u = rand::thread_rng().gen_range(0..J); |
There was a problem hiding this comment.
We should move away from using randomly-generated values in unit tests; if we randomly fall on a failing value, we won't be able to reproduce. Broadly speaking, unit tests should test specific hardcoded values, while prop tests should do the "sweeping" (as you're doing below)
There was a problem hiding this comment.
Good point, with proptest, the above is redundant as is. Changed it now to be deterministic
Reduces the cycle count of the Falcon DSA signature verification procedure. This brings the total cycle count to less than 71000 cycles.
Once/if
horner_eval_baseis implemented, we can bring down this to hopefully 61000 cycles by removing the need for thepowers_of_tauandset_to_zeroprocedures.