Add built-in self-tests and test vectors.#217
Add built-in self-tests and test vectors.#217gmaxwell wants to merge 2 commits intobitcoin-core:masterfrom gmaxwell:bist
Conversation
|
I guess for discuss right now, I'm not super happy that the branch coverage is only 91%. I'm not sure what opinions are about running these things at startup, they do considerably slow down start. They could be trimmed down to a smaller set to make them faster. |
|
Here is a recent lcov run with this patch in: http://people.xiph.org/~greg/secp256k1.bist1/ There are some unhit branches which are cryptographically unreachable, e.g. ecdsa_impl.h:206, and others are just serialization handling which would be easy enough to hit if thats what I was currently attempting. I'm most irritated right now with the uncovered carries in the scalar code, but I haven't tried to specifically craft something to hit them yet. |
src/bist.h
Outdated
There was a problem hiding this comment.
You can put your name here :)
|
Doesn't compile with --enable-benchmark. |
|
Hmph. Pretty sure it did at one point, I must have hosed it on a rebase. Will fix! |
|
It only fails with non-standard -O options. I suppose it's something that's expected to be inlined but becomes an external symbol otherwise. |
|
@gmaxwell See my https://github.com/sipa/secp256k1/commits/bist branch for a fix. |
|
For me, this increases the time for a secp256k1_start(SECP256K1_START_VERIFY | SECP256K1_START_SIGN) from 15ms to 53ms. |
This has a collection of static test vectors which were both randomly and hand created and then machine reduced. All of the vectors were tested against OpenSSL for agreement. The vectors are checked at _start(), unless specifically built with the them disabled. LCOV claims 91% branch coverage with the tests now.
|
Fast review ACK. Needs rebase. |
3 participants
This has a collection of static test vectors which were both randomly
and hand created and then machine reduced. All of the vectors were
tested against OpenSSL for agreement.
The vectors are checked at _start(), unless specifically built with
the them disabled (e.g. to reduce binary size.).
LCOV claims 91% branch coverage with the tests now.