API BREAKAGE: add a lows flag to secp256k1_ecdsa_sign #69

1. 
   theuni commented at 1:31 am on October 22, 2014: contributor

   Not sure if tests are necessary here. I’ve probably oversimplified, but this was enough to get bitcoin’s tests passing again with libsecp256k1.
2. API BREAKAGE: add a lows flag to secp256k1_ecdsa_sign

   Makes S negation (when above order / 2) optional.

   9e7a726b58
3. 
   gmaxwell commented at 2:36 am on October 22, 2014: contributor

   What? I don’t think this should be a flag. What test is failing?
4. 
   theuni commented at 2:42 am on October 22, 2014: contributor

    0<coryfields_> sipa: i've fixed up the libsecp256k1 in bitcoin (testing it in libbitcoinconsensus), and it's having trouble with a particular test. Known issue for "P2PK with high S" ?
    1<sipa> coryfields_: that sounds very expected
    2<coryfields_> ok, will just skip over it
    3<sipa> it needs a change in secp256k1's api, i'm afraid
    4<sipa> as you can't tell it to not use low-S
    5<coryfields_> figured as much, specifying high/low S i suppose
    6<coryfields_> ?
    7<coryfields_> right
    8<sipa> yup
    9<coryfields_> want me to take a crack at it? or is it waiting on something?
   10<sipa> it's trivial to do, you can if you want to
   

   By “trivial” + api change, I assumed he was just referring to a flag similar to the one our openssl wrapper uses.

5. 
   gmaxwell commented at 2:42 am on October 22, 2014: contributor

   okay if sipa says so! seems odd to introduce an call just to get that behavior.
6. 
   theuni commented at 2:50 am on October 22, 2014: contributor

   Not sure if this is actually what he was after or not, it was just the most trivial way I could come up with.

   I suppose for a bit of future-proofing we could use a bitfield param instead, where lowS is the only valid flag for now.

7. 
   sipa commented at 10:17 am on October 26, 2014: contributor

   @theuni This is indeed exactly what I meant. @gmaxwell It’s only needed for unit tests, to be able to generate cases with longer S values. Alternatively, it could be implemented by having parse/serialize/subtraction bignum functionality in the unit tests.
8. evoskuil cross-referenced this on Oct 27, 2014 from issue Breaking change introduced by secp256k1 (dev branch) by evoskuil
9. 
   sipa commented at 9:37 pm on November 5, 2014: contributor

   I’ve implemented a replacement on the bitcoin side: bitcoin/bitcoin@4a69b3b0174014e611143c31d411dde9d377aa98
10. 
    sipa commented at 7:36 pm on November 12, 2014: contributor

    Closing in favor of bitcoin/bitcoin#5256.
11. sipa closed this on Nov 12, 2014

    ---

Contributors
theuni gmaxwell sipa