[consensus] MOVEONLY: Move single-sig checking EvalScript code to EvalChecksig #18422

1. 
   jnewbery commented at 4:58 pm on March 24, 2020: member

   This is another small refactor pulled out of the Schnorr/Taproot PR #17977.

   This is in preparation for adding different signature verification rules, specifically tapscript (BIP 342), which interprets opcode 0xac and 0xad as Schnorr signature verifications.

2. jnewbery force-pushed on Mar 24, 2020
3. fanquake added the label Consensus on Mar 24, 2020
4. 
   practicalswift commented at 7:12 pm on March 24, 2020: contributor

   Concept ACK
5. in src/script/interpreter.cpp:350 in 10144d36df outdated

   341@@ -342,6 +342,35 @@ class ConditionStack {
   342 };
   343 }
   344 
   345+/** Helper for OP_CHECKSIG and OP_CHECKSIGVERIFY
   346+ *
   347+ * A return value of false means the script fails entirely. When true is returned, the
   348+ * fSuccess variable indicates whether the signature check itself succeeded.
   349+ */
   350+static bool EvalChecksig(const valtype& vchSig, const valtype& vchPubKey, CScript::const_iterator pend, CScript::const_iterator pbegincodehash, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* serror, bool& fSuccess)
   

   ---

   ---

   

   theStack commented at 9:26 pm on March 24, 2020:

   nit: Could swap the parameters pend and pbegincodehash? It’s IMHO quite unusual to pass iterator pairs in the order of (end, start) instead of (start, end).

   ---

   

   jnewbery commented at 0:59 am on March 25, 2020:

   This seems reasonable. @sipa - any reason these are reversed?

   ---

   

   sipa commented at 1:21 am on March 25, 2020:

   No reason at all.
6. 
   theStack commented at 9:28 pm on March 24, 2020: member

   Concept ACK
7. 
   DrahtBot commented at 0:24 am on March 25, 2020: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #17977 ([WIP] Implement BIP 340-342 validation (Schnorr/taproot/tapscript) by sipa)
   • #13062 (Make script interpreter independent from storage type CScript by sipa)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

8. [consensus] MOVEONLY: Move single-sig checking EvalScript code to EvalChecksig

   This is in preparation for adding different signature verification rules,
   specifically tapscript (BIP 342), which interprets opcode 0xac and 0xad
   as Schnorr signature verifications.

   14e8cf974a
9. jnewbery force-pushed on Mar 25, 2020
10. 
    sipa commented at 1:33 am on March 28, 2020: member

    ACK 14e8cf974a7a317796ef8e97e5cf9c355ceff0ee, verified move-only.
11. MarcoFalke added the label Refactoring on Mar 28, 2020
12. 
    MarcoFalke commented at 5:19 pm on March 28, 2020: member

    ACK 14e8cf974a7a317796ef8e97e5cf9c355ceff0ee, reviewed with “git show 14e8cf974a7a317796ef8e97e5cf9c355ceff0ee –color-moved=dimmed-zebra –color-moved-ws=ignore-all-space -W” 👆

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
     1Hash: SHA512
     2
     3ACK 14e8cf974a7a317796ef8e97e5cf9c355ceff0ee, reviewed with "git show 14e8cf974a7a317796ef8e97e5cf9c355ceff0ee --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space -W" 👆
     4-----BEGIN PGP SIGNATURE-----
     5
     6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
     7pUgGQAwAsaza36JaCU5T0HE5ZuADCgASph//OfX5cS9aTJmOfhos+0NOnYPbc4Kn
     8mhfhoC9FANL6yyPX+sPqggyyjk9jYEXHbqIf4EhRhek3mSwJ5J1zLZT27IDCnuS4
     9y+Q10wXdQio+bXLn8A23bfaWpkeBOWwpeX0go27KcvfHyw558r4Kwmg//Q84hvEP
    10//ihr0aPPyl5zyhCYjoaUlgfIJ4sloMbW4sjuLxYAY8K5fMBE3RU5FaeD0HP+IRQ
    11rL54QVIpYncMoqIFjgjso+e0Bz5bgQosPHGxwxEAgfSawa2Ocq3qY46oGq1vORUS
    12wE9miKu383A8teclcE9WOREz3poSZA+gAKfgHTDGB8suylNAqvBU4fDwjaqpO4dB
    13FVmCb1kb8ezbFRCGNHI4TnNCKcewyfrQ+PTV6QxNkJzaIbMrUh5MBoZbtcAtv7U/
    14I6yDtEWhI5Y/B+pEE+MBW1C5qw0YA0XjTcUhAzCgzSL6WEdkgNyqSPvNX3zpZvXH
    15mSoLyqm5
    16=wHwa
    17-----END PGP SIGNATURE-----
    

    Timestamp of file with hash 0d1c926a0f092662e838e8ab185def056ddd82a31445842a4f8134b0f3d86ad5 -

13. theStack approved
14. 
    theStack commented at 10:35 am on March 29, 2020: member

    Code-Review ACK https://github.com/bitcoin/bitcoin/commit/14e8cf974a7a317796ef8e97e5cf9c355ceff0ee
15. 
    fjahr commented at 5:46 pm on March 31, 2020: member

    Code-review ACK 14e8cf974a7a317796ef8e97e5cf9c355ceff0ee, verified that it’s move-only.
16. fanquake added this to the milestone 0.21.0 on Apr 1, 2020
17. 
    instagibbs commented at 1:43 pm on April 1, 2020: member

    code review ACK https://github.com/bitcoin/bitcoin/pull/18422/commits/14e8cf974a7a317796ef8e97e5cf9c355ceff0ee, verified move-only
18. 
    jonatack commented at 4:58 pm on April 1, 2020: member

    ACK 14e8cf974a7a317796ef8e97e5cf9c355ceff0ee
19. MarcoFalke merged this on Apr 10, 2020
20. MarcoFalke closed this on Apr 10, 2020

    ---

21. jnewbery deleted the branch on Apr 10, 2020
22. sidhujag referenced this in commit 042234f7f3 on Apr 13, 2020
23. Fabcien referenced this in commit 2dc0044214 on Jan 15, 2021
24. PhotoshiNakamoto referenced this in commit 6a91740085 on Dec 11, 2021
25. DrahtBot locked this on Feb 15, 2022

Contributors
jnewbery practicalswift theStack sipa DrahtBot MarcoFalke fjahr instagibbs jonatack

Labels
Refactoring Consensus

Milestone
0.21.0