cfrg / draft-irtf-cfrg-hash-to-curve

Hashing to Elliptic Curves
Other
79 stars 27 forks source link

add one test vector #276

Closed kwantam closed 4 years ago

kwantam commented 4 years ago

This PR adds one extra test vector. With this new vector, all of the gx1/gx2 branches are covered in all suites.

I'm explicitly not trying to cover branches in exceptional cases or in choosing sign of y. For exceptional cases, it's infeasible to find inputs that trigger the cases (we test our code by generating inputs for the map functions rather than the end-to-end hash function). For sign of y, the branch taken depends on the sqrt implementation, so it's not clear to me that it makes sense to try and cover these.

kwantam commented 4 years ago

By the way, I took a super simple approach to this, which I've stored in my test_vector_coverage branch.

I don't think we should pull the changes in that branch into the master branch, because I think we'd rather have the code in poc/ be as close as possible to mechanically translated from the code listings in the document (currently that is how all of the *_generic.sage files are done).

chris-wood commented 4 years ago

By the way, I took a super simple approach to this, which I've stored in my test_vector_coverage branch.

Hah! I like it.

I don't think we should pull the changes in that branch into the master branch, because I think we'd rather have the code in poc/ be as close as possible to mechanically translated from the code listings in the document (currently that is how all of the *_generic.sage files are done).

Agreed. It's cute, but a bit cumbersome.

chris-wood commented 4 years ago

@kwantam the test vector is fine. Out of curiosity, how'd you pick it? Did you just try some random things and check the resulting coverage?

kwantam commented 4 years ago

Yup, exactly. Found something that worked on the second or third guess. I suppose we could try to find a smaller one...

chris-wood commented 4 years ago

I suppose we could try to find a smaller one...

Nah, I think it's fine as is. :)