tests: improves tapscript unit tests #31640

pull EthanHeilman wants to merge 2 commits into bitcoin:master from EthanHeilman:taptest changing 2 files +84 −2
  1. EthanHeilman commented at 1:14 am on January 11, 2025: contributor

    This commit creates new test utilities for future Taproot script tests within script_tests.json. The key features of this commit are the addition of three new tags: #SCRIPT#, #CONTROLBLOCK#, and #TAPROOTOUTPUT#. These tags streamline the test creation process by eliminating the need to manually generate these components outside the test suite.

    • #SCRIPT#: Parses Tapscript and outputs a byte string of opcodes.
    • #CONTROLBLOCK#: Automatically generates the control block for a given Taproot output.
    • #TAPROOTOUTPUT#: Generates the final Taproot scriptPubKey.

    This code was originally part of the OP_CAT PR #29247 but was pulled out into a separate PR to reduce the rebase treadmill for the OP_CAT PR.

    Rationale

    While writing JSON script tests (script_tests.json) for #29247 we ran into the following problem. The JSON script tests are simple and easy to write for pre-Tapscript scripts, but adding or changing a Tapscript test requires substantial work per test. Consider the following pre-tapscript test:

    0["'aa' 'bb'", "CAT 0x4c 0x02 0xaabb EQUAL", "P2SH,STRICTENC", "DISABLED_OPCODE", "CAT disabled"]
    

    whereas a Tapscript test for the same script (annotated with comments for better readability) would look like:

     0[
     1    [
     2        "aa",
     3        "bb",
     4        "7e4c02aabb87", // output script
     5        "c0d6889cb081036e0faefa3a35157ad71086b123b2b144b649798b494c300a961d", // control block
     6        0.00000001
     7    ],
     8    "",
     9    "0x51 0x20 0x15048ed3a65748549c27b671936987093cf73a4c9cb18522a74fb9553060ca99", // Tapscript output
    10    "P2SH,WITNESS,TAPROOT",
    11    "OK",
    12    "TAPSCRIPT CATs aa and bb together and checks if EQUAL to aabb"
    13]
    

    Computing the Tapscript output, such as 0x51 0x20 0x15048ed3a65748549c27b671936987093cf73a4c9cb18522a74fb9553060ca99, requires writing custom code and running it for each test. The same is true for the Tapscript control block, such as c0d6889cb081036e0faefa3a35157ad71086b123b2b144b649798b494c300a961d. If a test is changed or updated new outputs and control blocks must be computed. The complexity of doing this is likely the reason that no one has added any Tapscript tests to JSON script tests until this PR.

    In this PR we address this issue by adding the following improvements to JSON script tests:

    Adding simple macros ("#SCRIPT# and #CONTROLBLOCK#) that allow the script test parser to automatically generate and inject a valid Tapscript output and control block to be computed automatically from the JSON script. Allowing Tapscript scripts to use the human readable strings like pre-script scripts by marking the location of the script in the witness stack using #SCRIPT#. This transforms the unreadable script 7e4c02aabb87 into #SCRIPT# CAT 0x4c 0x02 0xaabb EQUAL. This results in the following JSON script test which is far easier to write and easier to read.

     0[
     1    [
     2        "aa",
     3        "bb",
     4        "#SCRIPT# CAT",
     5        "#CONTROLBLOCK#",
     6        0.00000001
     7    ],
     8    "",
     9    "0x51 0x20 #TAPROOTOUTPUT#",
    10    "P2SH,WITNESS,TAPROOT,OP_CAT",
    11    "OK",
    12    "TAPSCRIPT Test of OP_CAT flag by calling CAT on two elements. TAPSCRIPT_OP_CAT flag is set so CAT is executed."
    13],
    
  2. DrahtBot commented at 1:14 am on January 11, 2025: contributor

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

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31640.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Stale ACK 0xBEEFCAF3

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #29247 (CAT in Tapscript (BIP-347) by 0xBEEFCAF3)

    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.

  3. DrahtBot added the label Tests on Jan 11, 2025
  4. EthanHeilman force-pushed on Jan 11, 2025
  5. DrahtBot commented at 1:17 am on January 11, 2025: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/35460490112

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  6. DrahtBot added the label CI failed on Jan 11, 2025
  7. EthanHeilman force-pushed on Jan 11, 2025
  8. EthanHeilman force-pushed on Jan 11, 2025
  9. EthanHeilman force-pushed on Jan 11, 2025
  10. EthanHeilman force-pushed on Jan 11, 2025
  11. EthanHeilman force-pushed on Jan 11, 2025
  12. EthanHeilman force-pushed on Jan 11, 2025
  13. DrahtBot removed the label CI failed on Jan 11, 2025
  14. instagibbs commented at 3:18 pm on January 11, 2025: member
    is this in draft for a reason?
  15. EthanHeilman commented at 2:31 pm on January 14, 2025: contributor

    is this in draft for a reason? @instagibbs Yes, my todos to make this not a draft are:

    [X] - Better PR description [X] - A few extra tapscript tests (it was most cat tapscript tests before) [X] - Doublechecking my code to make sure the rebase didn’t break anything

    I got most of this done over the weekend, I just haven’t pushed it yet. Will likely push it tonight, Wednesday.

    Edit just pushed. Not longer a draft.

  16. EthanHeilman force-pushed on Jan 15, 2025
  17. EthanHeilman force-pushed on Jan 15, 2025
  18. DrahtBot added the label CI failed on Jan 15, 2025
  19. DrahtBot commented at 0:40 am on January 15, 2025: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/35623022004

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  20. EthanHeilman force-pushed on Jan 15, 2025
  21. EthanHeilman marked this as ready for review on Jan 15, 2025
  22. EthanHeilman force-pushed on Jan 15, 2025
  23. EthanHeilman force-pushed on Jan 15, 2025
  24. tests: improves tapscript unit tests
    This commit creates new test utilities for future Taproot script
    tests within script_tests.json. The key features of this commit are the
    addition of three new tags: `#SCRIPT#`, `#CONTROLBLOCK#`, and
    `#TAPROOTOUTPUT#`. These tags streamline the test creation process by
    eliminating the need to manually generate these components outside the
    test suite.
    
    * `#SCRIPT#`: Parses Tapscript and outputs a byte string of opcodes.
    * `#CONTROLBLOCK#`: Automatically generates the control block for a given
    Taproot output.
    * `#TAPROOTOUTPUT#`: Generates the final Taproot scriptPubKey.
    1bd2af1de6
  25. in src/test/data/script_tests.json:2626 in e3343db8b2 outdated
    2621+    ],
    2622+    "",
    2623+    "0x51 0x20 #TAPROOTOUTPUT#",
    2624+    "P2SH,WITNESS,TAPROOT",
    2625+    "OK",
    2626+    "TAPSCRIPT Tests testing tapscript with many different op codes including ALTSTACK interactions. "
    


    0xBEEFCAF3 commented at 1:10 am on January 15, 2025:
    Super nit: empty space on this line. Not sure if the linter even picks up on this

    EthanHeilman commented at 1:12 am on January 15, 2025:
    Fixed! Thanks
  26. EthanHeilman force-pushed on Jan 15, 2025
  27. EthanHeilman requested review from 0xBEEFCAF3 on Jan 15, 2025
  28. DrahtBot removed the label CI failed on Jan 15, 2025
  29. in src/test/script_tests.cpp:964 in 1bd2af1de6 outdated
    957@@ -940,7 +958,14 @@ BOOST_AUTO_TEST_CASE(script_json_test)
    958         std::string scriptSigString = test[pos++].get_str();
    959         CScript scriptSig = ParseScript(scriptSigString);
    960         std::string scriptPubKeyString = test[pos++].get_str();
    961-        CScript scriptPubKey = ParseScript(scriptPubKeyString);
    962+        CScript scriptPubKey;
    963+        // If requested, auto-generate the taproot output
    964+        if (strcmp(scriptPubKeyString.c_str(), "0x51 0x20 #TAPROOTOUTPUT#")== 0) {
    965+            BOOST_CHECK_MESSAGE(taprootBuilder.IsComplete(), "Failed to autogenerate Tapscript Script PubKey");
    


    0xBEEFCAF3 commented at 3:14 pm on January 20, 2025:
    Nit: I would use “taproot output key” here instead of “Tapscript Script Pubkey” per BIP341

    EthanHeilman commented at 3:51 pm on January 20, 2025:
    Fixed!
  30. 0xBEEFCAF3 approved
  31. 0xBEEFCAF3 commented at 3:29 pm on January 20, 2025: none
    tACK 1bd2af1de6551d67c1c363b4cd04d4973e3650bf Tested by rebasing the new bip347 tapscript unit tests on top of these changes.
  32. Addresses comment renaming pubkey to output key 2f9040e412

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-01-21 03:12 UTC

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