Remove OpenSSL tests #635

pull real-or-random wants to merge 1 commits into bitcoin-core:master from real-or-random:remove-openssl-tests changing 4 files +0 −243
  1. real-or-random commented at 11:04 am on June 5, 2019: contributor

    This is for discussion obviously.

    think we don’t need the tests anymore, and they do somehwat more harm than good, e.g., they make our tests more complex, and valgrind will complain about the use of uninitialized memory in the tests (which is done on purpose in OpenSSL).

  2. Remove OpenSSL tests b6621e9231
  3. gmaxwell commented at 4:53 pm on June 5, 2019: contributor

    I don’t follow your argument: The openssl tests are largely separated so although they are additional things, they don’t increase complexity of other things– their cost primarily just their maintenance, which has been fairly minimal.

    If you are getting valgrind errors from openssl it means your openssl was not complied with -DPURIFY, you should go scream at your distribution to fix that– many distributions compile their openssl with -DPURIFY. The tests with openssl are valgrind clean for me on fedora, for example.

    An argument I’d make for removing it would be that it’s possible that its presence falsely causes people to believe that openssl is a dependency of the library and then causes them to use less well reviewed alternatives. I don’t think we have any evidence that this is currently happening, but I’m not sure we would.

    An argument I’d make against removing it is that we have many useful randomized tests which lack a ground truth. These tests lose a lot of their testing power unless they operate as a comparison against and independent implementation. One possible alternative would be to include a tiny independent implementation (potentially uses GMP for its numbers), has it’s own distinct group law (perhaps an entirely affine naive implementation). This would be useful as it would be essentially nearly maintenance-free (GMP api is quite stable) and could plausibly be more independent than openssl (which could have some bugs in common with us). It would also generalize to more functions that we implement that openssl does not. The downside would be that it would take a little effort to create.

  4. gmaxwell commented at 5:01 pm on June 5, 2019: contributor

    Some history: while openssl was considered consensus critical in Bitcoin testing against it was essential. Now that it isn’t – it isn’t.

    But testing two implementations against each other on ‘random’ inputs (maybe very non-uniformly random) is a very powerful testing methodology which we use intentionally, have found real issues with, and I think if anything we should be increasing our use of… This, however, doesn’t itself require openssl– we just had that due to the history and because it was easy.

  5. real-or-random commented at 10:01 pm on June 12, 2019: contributor
    Okay I’m convinced that cross-testing implementations is a good thing. But then I think the OpenSSL tests are good for that purpose, i.e., it is not worth the time and effort to replace them by some alternative implementation. (If someone wants to give it a try, I’m not against it of course.)
  6. real-or-random closed this on Jun 12, 2019

  7. real-or-random commented at 11:34 am on June 14, 2019: contributor
  8. elichai commented at 10:59 pm on June 18, 2019: contributor

    What kind of tests would you like to see added? Random private key and data, then signing and verifying by both libraries? (and if both are deterministic we can even compare sigs)

    DER tests?

  9. elichai cross-referenced this on Jun 24, 2019 from issue Adding random tests against a naive implementation by elichai
  10. real-or-random cross-referenced this on Nov 9, 2019 from issue cross-testing by real-or-random
  11. real-or-random cross-referenced this on Apr 9, 2020 from issue Remove OpenSSL benchmarks? by real-or-random
  12. real-or-random cross-referenced this on Sep 28, 2021 from issue [RFC] Remove OpenSSL testing support by sipa

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-22 22:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me