Test improvements and some small API fixes that they turned up. #340

1. 
   gmaxwell commented at 5:19 pm on October 21, 2015: contributor

   Included in here,

   One commit changes the 6979 nonce behavior when algo16 is provided so that non-provided data and zero data do not provide the same nonce. I think equivalence between NULL and zero-bytes there is less safe.

   Another commit makes the usage of VERIFY_CHECK() side-effect free. I think this makes the code more reviewable (I personally have a tendency to assume CHECK macros aren’t hit at runtime) but mostly it simplifies coverage analysis where dead code from VERIFY otherwise counts against coverage. To avoid that I need to make VERIFY_CHECK a no-op during coverage analysis. This PR doesn’t change VERIFY_CHECK’s behavior itself.

   This PR also makes the failure cases for the tweaks functions and create behave more consistently with respect to zeroization of the output on failure.

   Other commits move recovery into the recovery module, and the privkey parser into contrib.

   There are more details in individual commit messages.

2. 
   gmaxwell commented at 5:20 pm on October 21, 2015: contributor

   This is on top of (current) #334. I still have static test vectors outstanding, but I have a lot of changes here and wanted to get what I had done in the pipeline rather than suffer rebase hell just waiting for the static vectors to finish optimizing, since they’re fairly free-standing.
3. 
   sipa commented at 10:25 pm on October 22, 2015: contributor

   Needs rebase.
4. Minor comment improvements. 6c476a8a9b
5. Make secp256k1_ec_pubkey_create skip processing invalid secret keys.

   This makes it somewhat less constant time in error conditions, but
    avoids encountering an internal assertion failure when trying
    to write out the point at infinity.

   70d4640172
6. Avoid nonce_function_rfc6979 algo16 argument emulation.

   This avoids data=NULL and data = zeros to producing the same nonce.
   
   Previously the code tried to avoid the case where some data inputs
    aliased algo16 inputs by always padding out the data.
   
   But because algo16 and data are different lengths they cannot
    emulate each other, and the padding would match a data value of
    all zeros.

   b30fc85c9e
7. Eliminate all side-effects from VERIFY_CHECK() usage.

   The side-effects make review somewhat harder because 99.9% of the
    time the macro usage has no sideeffects, so they're easily ignored.
   
   The main motivation for avoiding the side effects is so that the
    macro can be completely stubbed out for branch coverage analysis
    otherwise all the unreachable verify code gets counted against
    coverage.

   e3cd679634
8. Move secp256k1_ecdsa_sig_recover into the recovery module. 1b3efc1147
9. Move secp256k1_ec_privkey_import/export to contrib.

   These functions are intended for compatibility with legacy software,
    and are not normally needed in new secp256k1 applications.
   
   They also do not obeying any particular standard (and likely cannot
    without without undermining their compatibility), and so are a
    better fit for contrib.

   4a243da47c
10. Make the tweak function zeroize-output-on-fail behavior consistent.

    Previously the private key tweak operations left the input unchanged
     on failure but the pubkey versions zeroized on failure.

    bb5aa4df55
11. Add additional tests for eckey and arg-checks.

    This gets branch coverage up over 90% for me.

    96be20463f
12. 
    gmaxwell commented at 11:01 pm on October 22, 2015: contributor

    rebased.
13. 
    sipa commented at 5:21 pm on October 24, 2015: contributor

    ACK
14. sipa merged this on Oct 24, 2015
15. sipa closed this on Oct 24, 2015

    ---

16. sipa referenced this in commit 1a3e03a348 on Oct 24, 2015
17. sipa cross-referenced this on Oct 24, 2015 from issue [API BREAK] Explicit flag handling (rebase + modification of #324) by sipa

Contributors
gmaxwell sipa