Conversation
3811162 to
1f57321
Compare
CBMC Results (ML-DSA-87)Full Results (187 proofs)
|
CBMC Results (ML-DSA-44)Full Results (187 proofs)
|
CBMC Results (ML-DSA-65)Full Results (187 proofs)
|
2458351 to
8185ec7
Compare
hanno-becker
left a comment
There was a problem hiding this comment.
Looks good, @jakemas, thank you! Can you please to the CBMC spec+proof at the same time? You can express in C that the output is a permutation of the input.
cca5edc to
99ba911
Compare
Thanks! Added CBMC contract. |
| let nttunpack_order = new_definition | ||
| `nttunpack_order i = | ||
| let block = i DIV 64 in | ||
| let pos = i MOD 64 in | ||
| let lane = pos DIV 8 in | ||
| let offset = pos MOD 8 in | ||
| 64 * block + 8 * offset + lane`;; | ||
|
|
||
| let nttunpack_unorder = new_definition | ||
| `nttunpack_unorder i = | ||
| let block = i DIV 64 in | ||
| let pos = i MOD 64 in | ||
| let lane = pos MOD 8 in | ||
| let offset = pos DIV 8 in | ||
| 64 * block + 8 * lane + offset`;; |
There was a problem hiding this comment.
This reordering is the same as used for the specification of the [inv]NTT, or not? It determines how the custom NTT domain differs from the normal bitreversed one.
If I understand that correctly, we should reuse the same definition(s) as for the [inv]NTT proofs.
There was a problem hiding this comment.
The nttunpack order is the intra-block component of the full AVX2 NTT order, specifically, mldsa_avx2_ntt_order = bitreverse8 * nttunpack_order. They're related but not identical, since the NTT order additionally applies bitreverse8 on top.
I've moved nttunpack_order/nttunpack_unorder to common/mldsa_specs.ml so the definitions are shared. A follow-up could refactor mldsa_avx2_ntt_order to be expressed in terms of nttunpack_order and prove the composition relationship as a lemma. This may require changes to ntt/intt as the pattern matching may no longer work. I'll test it out.
There was a problem hiding this comment.
Ok, I’ve refactored mldsa_avx2_ntt_order to be defined as bitreverse8(nttunpack_order i) directly. All downstream clause computations (MLDSA_AVX2_NTT_ORDER_CLAUSES, MLDSA_FORWARD_NTT_CONV, etc.) work with the new definition. The NTT/iNTT proofs re-running in CI to confirm.
There was a problem hiding this comment.
I tried redefining mldsa_avx2_ntt_order directly in terms of nttunpack_order, but the NTT proof breaks, it pattern-matches on the expanded arithmetic form during simulation. Keeping the original definition with the decomposition lemma alongside it seemed like the right tradeoff. Proposing a follow up to clean this up, but don't want to block the addition of the proof on it longer. iNTT/NTT proofs are ~3hours to run.
hanno-becker
left a comment
There was a problem hiding this comment.
As I understand, the permutation here should be the same as for the [inv]NTT, so we should have only one set of HOL definition for those in mlkem_specs.ml.
|
@jakemas, could you please rebase this and address @hanno-becker's feedback? |
|
@jakemas: Gentle ping. Could you please get this into shape for merging? |
8886dcd to
223ba07
Compare
|
@hanno-becker @mkannwischer Okay, shaped up and ready for merge. I've added a |
223ba07 to
497f6c3
Compare
Signed-off-by: Jake Massimo <jakemas@amazon.com>
497f6c3 to
2405606
Compare
nttunpack#913