The pifac module includes lots of tests, but some of them are not testing exactly what they say they are testing. We need to review these and fix documentation / test names for the ones that are misleading.
Specifically, the test test_no_small_factors_proof_negative_cases() has many test of the following form:
// for a given valid `input` and `proof`:
let invalid_proof = PiFacProof::prove(input, ... other bad fields ...)?;
let invalid_input = CommonInput::new(...bad things...);
assert!(invalid_proof.verify(invalid_input, &(), &mut transcript()).is_err());
This applies to small_proof, and the two mixed_proofs.
However, these tests are failing because the common input is different between proving (input) and verifying (invalid_input), so the challenge will be wrong. It's a bit hard to tell because this proof, unlike the others, doesn't specifically include the challenge in the proof, so it's not explicit in the logs. If you print out the challenge in proving and verifying, though, you'll see it's different.
However, the proofs are trying to demonstrate different properties:
small_proof manually selects small factors and expects to fail because the factors are not in the correct range.
mixed proofs choose a small and a normal-sized factor and expects to fail because one of the factors is too small
In fact, I think these test won't ever work because the proof actually tests that no factors are too large (e.g. the range check is an upper bound, not a lower bound). It doesn't explicitly encode that the modulus is supposed to be 2048 bits. See test_modulus_cannot_have_large_factors for the thing we can actually verify.
So, action items that remain are
[x] Test my theory: fix the common input to be the same in the three identified tests and see if they fail
[x] Look at the paper and see if we should add a "minimum size check" for N in validation, or if it's okay to assume that's checked outside the proof (see also: #466)
The
pifacmodule includes lots of tests, but some of them are not testing exactly what they say they are testing. We need to review these and fix documentation / test names for the ones that are misleading.Specifically, the test
test_no_small_factors_proof_negative_cases()has many test of the following form:This applies to
small_proof, and the twomixed_proofs.However, these tests are failing because the common input is different between proving (
input) and verifying (invalid_input), so the challenge will be wrong. It's a bit hard to tell because this proof, unlike the others, doesn't specifically include the challenge in the proof, so it's not explicit in the logs. If you print out the challenge in proving and verifying, though, you'll see it's different.However, the proofs are trying to demonstrate different properties:
small_proofmanually selects small factors and expects to fail because the factors are not in the correct range.mixed proofs choose a small and a normal-sized factor and expects to fail because one of the factors is too smallIn fact, I think these test won't ever work because the proof actually tests that no factors are too large (e.g. the range check is an upper bound, not a lower bound). It doesn't explicitly encode that the modulus is supposed to be 2048 bits. See
test_modulus_cannot_have_large_factorsfor the thing we can actually verify.So, action items that remain are
Nin validation, or if it's okay to assume that's checked outside the proof (see also: #466)