tests: Don't depend on value of DEFAULT_PERMIT_BAREMULTISIG #29088

pull ajtowns wants to merge 2 commits into bitcoin:master from ajtowns:202312-testbaremulti changing 3 files +13 −4
  1. ajtowns commented at 8:03 AM on December 15, 2023: contributor

    Update unit and functional tests so that they continue to work if the default for -permitbaremultisig is changed.

  2. DrahtBot commented at 8:03 AM on December 15, 2023: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK maflcko, instagibbs, glozow, achow101

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #29001 (Ephemeral Anchors by instagibbs)

    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 Dec 15, 2023
  4. maflcko commented at 8:37 AM on December 15, 2023: member
    elay_fee, std::string& reason);
          |                                                                                                      ^
    test/script_p2sh_tests.cpp:29:43: error: argument name 'permit_baremultisig' in comment does not match parameter name 'permit_bare_multisig' [bugprone-argument-comment,-warnings-as-errors]
       29 |            IsStandardTx(tx, std::nullopt, /*permit_baremultisig=*/false, CFeeRate{DUST_RELAY_TX_FEE}, reason);
          |                                           ^~~~~~~~~~~~~~~~~~~~~~~~
          |                                           /*permit_bare_multisig=*/
    ./policy/policy.h:140:102: note: 'permit_bare_multisig' declared here
      140 | bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_datacarrier_bytes, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason);
    --
    
  5. tests: test both settings for permitbaremultisig in p2sh tests 7dfabdcf86
  6. tests: ensure functional tests set permitbaremultisig=1 when needed
    The mempool_dust and mempool_sigoplimits functional tests both use bare
    multisig txs, so ensure they're allowed by policy.
    7b45744df3
  7. ajtowns force-pushed on Dec 15, 2023
  8. DrahtBot added the label CI failed on Dec 15, 2023
  9. maflcko commented at 9:23 AM on December 15, 2023: member

    lgtm ACK 7b45744df33c6a4759eae1a3984f389cbac837c2

  10. DrahtBot removed the label CI failed on Dec 15, 2023
  11. fanquake requested review from instagibbs on Dec 15, 2023
  12. instagibbs commented at 3:09 PM on December 15, 2023: member

    crACK https://github.com/bitcoin/bitcoin/pull/29088/commits/7b45744df33c6a4759eae1a3984f389cbac837c2

    (someone should probably switch it default false and check)

  13. DrahtBot removed review request from instagibbs on Dec 15, 2023
  14. maflcko commented at 3:48 PM on December 15, 2023: member

    (someone should probably switch it default false and check)

    done with

    diff --git a/src/policy/policy.h b/src/policy/policy.h
    index 6a7980c312..42359f12a5 100644
    --- a/src/policy/policy.h
    +++ b/src/policy/policy.h
    @@ -36,7 +36,7 @@ static constexpr unsigned int DEFAULT_INCREMENTAL_RELAY_FEE{1000};
     /** Default for -bytespersigop */
     static constexpr unsigned int DEFAULT_BYTES_PER_SIGOP{20};
     /** Default for -permitbaremultisig */
    -static constexpr bool DEFAULT_PERMIT_BAREMULTISIG{true};
    +static constexpr bool DEFAULT_PERMIT_BAREMULTISIG{0};
     /** The maximum number of witness stack items in a standard P2WSH script */
     static constexpr unsigned int MAX_STANDARD_P2WSH_STACK_ITEMS{100};
     /** The maximum size in bytes of each witness stack item in a standard P2WSH script */
    
  15. ajtowns commented at 4:29 PM on December 15, 2023: contributor

    crACK 7b45744

    (someone should probably switch it default false and check)

    see also CI on #29093

  16. glozow commented at 5:18 PM on December 15, 2023: member

    ACK 7b45744df33c6a4759eae1a3984f389cbac837c2, changed default locally and all tests passed

  17. achow101 commented at 9:16 PM on December 15, 2023: member

    ACK 7b45744df33c6a4759eae1a3984f389cbac837c2

  18. achow101 merged this on Dec 15, 2023
  19. achow101 closed this on Dec 15, 2023

  20. bitcoin locked this on Dec 14, 2024

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: 2026-04-15 03:13 UTC

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