Update unit and functional tests so that they continue to work if the default for -permitbaremultisig is changed.
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-
ajtowns commented at 8:03 AM on December 15, 2023: contributor
-
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.
- DrahtBot added the label Tests on Dec 15, 2023
-
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); -- -
tests: test both settings for permitbaremultisig in p2sh tests 7dfabdcf86
-
7b45744df3
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.
- ajtowns force-pushed on Dec 15, 2023
- DrahtBot added the label CI failed on Dec 15, 2023
-
maflcko commented at 9:23 AM on December 15, 2023: member
lgtm ACK 7b45744df33c6a4759eae1a3984f389cbac837c2
- DrahtBot removed the label CI failed on Dec 15, 2023
- fanquake requested review from instagibbs on Dec 15, 2023
-
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)
- DrahtBot removed review request from instagibbs on Dec 15, 2023
-
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 */ -
glozow commented at 5:18 PM on December 15, 2023: member
ACK 7b45744df33c6a4759eae1a3984f389cbac837c2, changed default locally and all tests passed
-
achow101 commented at 9:16 PM on December 15, 2023: member
ACK 7b45744df33c6a4759eae1a3984f389cbac837c2
- achow101 merged this on Dec 15, 2023
- achow101 closed this on Dec 15, 2023
- bitcoin locked this on Dec 14, 2024