test: cross-check RPO test vectors with Python reference implementation#822
Conversation
Al-Kindi-0
left a comment
Al-Kindi-0
left a comment
There was a problem hiding this comment.
Just to double, check the .py implementation is exactly the reference one, except for the state re-mapping, right?
Otherwise looks great to me, thank you@
Not exactly
Both of these match our Rust implementation which follows the padding rule from the 2023/1045 paper rather than the original reference. The 19 test vectors confirm it's consistent with our Rust code |
Thank you! Indeed, the padding rule has changed from the one that was used in the RPO paper. |
Farukest
force-pushed
the
feat/verify-rpo-test-vectors-768
branch
from
b92422c to
a4b28a6
Compare
Add a Python script that implements the RPO hash function with the current state layout [RATE0, RATE1, CAPACITY] and verifies all 19 test vectors match the Rust implementation. This addresses the concern raised in issue 0xMiden#768 that test vectors were not cross-checked with the reference implementation after the state layout change in PR 0xMiden#755. Closes 0xMiden#768
Farukest
force-pushed
the
feat/verify-rpo-test-vectors-768
branch
from
a4b28a6 to
0bf6ccb
Compare
| @@ -0,0 +1,288 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
This is a nice verification script. Since the Python script isn't part of CI, a future change to RPO could break consistency without detection. Would be worth considering either adding this to CI or generating test vectors from Python as the source of truth.
There was a problem hiding this comment.
I wouldn't spend effort on this right now as with v0.21 VM, we no longer use RPO/RPX and we may be dropping RPO/RPX from this repo in the near future.
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you.
This was a fairly low priority issue as RPO/RPX may be dropped from this repo soon. But since the work has already been done, I'll merge it.
| @@ -0,0 +1,288 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
I wouldn't spend effort on this right now as with v0.21 VM, we no longer use RPO/RPX and we may be dropping RPO/RPX from this repo in the near future.
4 participants
Summary
After the state layout change in #755 (
[CAPACITY, RATE0, RATE1]→[RATE0, RATE1, CAPACITY]), the RPO test vectors were updated but not verified against the reference implementation.This PR adds a Python verification script adapted from the original RPO reference that uses the current state layout and confirms all 19 test vectors match the Rust implementation.
generate_test_vectors.pyimplementing RPO hash with the current layoutpython3 generate_test_vectors.py→ all OKEXPECTEDnoting the cross-checkCloses #768
Test plan
python3 generate_test_vectors.py— 19/19 vectors matchcargo test -p miden-crypto hash_test_vectors— passescargo +nightly fmt --checkcargo clippy -p miden-cryptocargo doc -p miden-crypto --no-deps