Make SCRIPT_VERIFY_STRICTENC compatible with BIP62 #5004

1. 
   sipa commented at 11:25 pm on September 29, 2014: member

   • Delete canonical_tests.cpp, and move the tests to script_tests.cpp.
   • Split off SCRIPT_VERIFY_DERSIG from SCRIPT_VERIFY_STRICTENC (the BIP62 part of it).
   • Change signature STRICTENC/DERSIG semantics to fail the script entirely rather than the CHECKSIG result (softfork safety, and BIP62 requirement).
   • Add autogenerated tests for several odd cases.
2. sipa force-pushed on Sep 29, 2014
3. 
   BitcoinPullTester commented at 1:18 am on September 30, 2014: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p5004_1039e0a6cac595871710055408e60453f23fe09f/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
4. 
   sipa commented at 0:37 am on October 1, 2014: member

   @schildbach @petertodd You guys may be interesting in this. Note the new DERSIG flag, and the changed semantics for encoding errors for the signatures.
5. 
   laanwj commented at 10:45 am on October 1, 2014: member

   ACK
6. 
   sipa commented at 1:59 am on October 2, 2014: member

   @gavinandresen @gmaxwell @jgarzik Comments?
7. in src/script/interpreter.cpp: in 1039e0a6ca outdated

   55@@ -56,10 +56,7 @@ static inline void popstack(vector<valtype>& stack)
   56     stack.pop_back();
   57 }
   58 
   59-bool IsCanonicalPubKey(const valtype &vchPubKey, unsigned int flags) {
   60-    if (!(flags & SCRIPT_VERIFY_STRICTENC))
   61-        return true;
   62-
   63+bool IsStandardPubKey(const valtype &vchPubKey) {
   

   ---

   ---

   

   gavinandresen commented at 3:41 pm on October 2, 2014:

   I was confused by the use of the word “Standard” here when reviewing. Why the rename from Canonical to Standard? I think of “standard” pubkeys as the standard script forms that the wallet recognizes.

   ---

   

   sipa commented at 5:37 pm on October 2, 2014:

   ‘Canonical’ seems to imply there is One Single Truth, but as several encoding rules get added through BIP62, that is no longer the case, so I want to get rid of that and have separate functions that check separate (and mostly independent) encoding rules. I agree ‘standard’ has the wrong meaning as well. I could go for ‘IsCompressedOrUncompressedPubKey()’ ?
8. 
   gavinandresen commented at 3:42 pm on October 2, 2014: contributor

   Code changes look good, except for a naming nit.
9. sipa force-pushed on Oct 2, 2014
10. 
    sipa commented at 6:16 pm on October 2, 2014: member

    Updated to use ‘IsCompressedOrUncompressedPubKey()’.
11. sipa force-pushed on Oct 3, 2014
12. sipa force-pushed on Oct 6, 2014
13. sipa force-pushed on Oct 7, 2014
14. 
    sipa commented at 7:53 am on October 7, 2014: member

    Rebased on top of #5055.
15. sipa force-pushed on Oct 8, 2014
16. sipa force-pushed on Oct 8, 2014
17. 
    gmaxwell commented at 10:41 pm on October 8, 2014: contributor

    ACK
18. Make SCRIPT_VERIFY_STRICTENC compatible with BIP62

    * Delete canonical_tests.cpp, and move the tests to script_tests.cpp.
    * Split off SCRIPT_VERIFY_DERSIG from SCRIPT_VERIFY_STRICTENC (the BIP62 part of it).
    * Change signature STRICTENC/DERSIG semantics to fail the script entirely rather than the CHECKSIG result (softfork safety, and BIP62 requirement).
    * Add many autogenerated tests for several odd cases.
    * Mention specific BIP62 rules in the script verification flags.

    9df9cf5a9f
19. sipa force-pushed on Oct 8, 2014
20. sipa merged this on Oct 8, 2014
21. sipa closed this on Oct 8, 2014

    ---

22. sipa referenced this in commit bf7b4fc6d3 on Oct 8, 2014
23. MarcoFalke locked this on Sep 8, 2021

Contributors
sipa BitcoinPullTester laanwj gavinandresen gmaxwell