scripted-diff: adapt script error constant names in feature_taproot.py #32415

pull theStack wants to merge 1 commits into bitcoin:master from theStack:202505-test-adapt_consensus_error_constants changing 1 files +98 −98
  1. theStack commented at 6:28 pm on May 3, 2025: contributor

    While reviewing #31622 I noticed that the constant name (SCRIPT_)ERR_SIG_HASHTYPE is used for two different script verification error codes, namely one for legacy and one for Schnorr signatures:

    https://github.com/bitcoin/bitcoin/blob/eba5f9c4b63fe46261fbb3e71b9a94832d105b23/src/script/script_error.cpp#L56-L57 https://github.com/bitcoin/bitcoin/blob/eba5f9c4b63fe46261fbb3e71b9a94832d105b23/test/functional/feature_taproot.py#L600

    In order to resolve this confusion, this PR adapts all script error constant names in the functional tests (currently only in feature_taproot.py) to the ones used in our C++ codebase (see script_error.cpp) with a scripted diff. This also makes checking whether we have test coverage for a certain script error easier.

  2. DrahtBot commented at 6:28 pm on May 3, 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/32415.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK jamesob
    Concept ACK sipa

    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:

    • #32301 (test: cover invalid codesep positions for signature in taproot by instagibbs)
    • #32270 (test: fix pushdata scripts by instagibbs)
    • #32247 (BIP-348 (OP_CHECKSIGFROMSTACK) (regtest only) by jamesob)
    • #29491 ([EXPERIMENTAL] Schnorr batch verification for blocks by fjahr)
    • #29371 (test: Add leaf_version parameter to taproot_tree_helper() by Christewart)

    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.

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    Error: “neither compressed or uncompressed” → “neither compressed nor uncompressed”

  3. DrahtBot added the label Refactoring on May 3, 2025
  4. scripted-diff: adapt script error constant names in feature_taproot.py
    In order to remove potential confusion, this commit adapts all script
    error constant names in the functional tests (currently only in
    feature_taproot.py) to the ones used in our C++ codebase. This also
    makes checking whether we have test coverage for a certain script error
    easier.
    
    -BEGIN VERIFY SCRIPT-
    
    ren() { sed -i "s|$1|$2|g" $( git grep -l "$1" -- "./test" ) ; }
    
    ren ERR_SIG_SIZE          ERR_SCHNORR_SIG_SIZE
    ren ERR_SIG_HASHTYPE      ERR_SCHNORR_SIG_HASHTYPE
    ren ERR_SIG_SCHNORR       ERR_SCHNORR_SIG
    ren ERR_CONTROLBLOCK_SIZE ERR_TAPROOT_WRONG_CONTROL_SIZE
    ren ERR_PUSH_LIMIT        ERR_PUSH_SIZE
    ren ERR_MINIMALIF         ERR_TAPSCRIPT_MINIMALIF
    ren ERR_UNKNOWN_PUBKEY    ERR_PUBKEYTYPE
    ren ERR_STACK_EMPTY       ERR_INVALID_STACK_OPERATION
    ren ERR_SIGOPS_RATIO      ERR_TAPSCRIPT_VALIDATION_WEIGHT
    ren ERR_UNDECODABLE       ERR_BAD_OPCODE
    ren ERR_NO_SUCCESS        ERR_EVAL_FALSE
    ren ERR_EMPTY_WITNESS     ERR_WITNESS_PROGRAM_WITNESS_EMPTY
    
    -END VERIFY SCRIPT-
    b5f580c580
  5. theStack force-pushed on May 3, 2025
  6. DrahtBot added the label CI failed on May 3, 2025
  7. DrahtBot commented at 6:32 pm on May 3, 2025: contributor

    🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/41591166714 LLM reason (✨ experimental): The CI failure is caused by errors in the lint check related to script evaluation failures, specifically ERR_EVAL_FALSE errors.

    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.

  8. DrahtBot removed the label CI failed on May 3, 2025
  9. sipa commented at 9:06 pm on May 3, 2025: member
    Concept ACK
  10. jamesob commented at 1:45 am on May 4, 2025: contributor

    crACK https://github.com/bitcoin/bitcoin/pull/32415/commits/b5f580c580257d28d295cae3f787b55eb1863f16

    Scripted-diff/func-test-only makes this an easy review. Good cleanup for this daunting file.

  11. DrahtBot requested review from sipa on May 4, 2025

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-05-05 12:12 UTC

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