test: descriptor: fix test for `MaxSatisfactionWeight` #31570

pull brunoerg wants to merge 1 commits into bitcoin:master from brunoerg:2024-12-descriptor-fix-test changing 1 files +2 −1

brunoerg commented at 5:23 PM on December 26, 2024: contributor

To get the maximum size of a satisfaction for a descriptor with no max sig, the parameter use_max_sig should be false.
DrahtBot commented at 5:23 PM on December 26, 2024: 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/31570.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK tdb3, furszy, fjahr, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Tests on Dec 26, 2024
tdb3 approved
tdb3 commented at 12:01 AM on December 27, 2024: contributor

ACK 00ec80b24ec32ac695d947587dd0f860fc6f1efa lgtm

Type	Reviewers
ACK	tdb3, furszy, fjahr, achow101

in src/test/descriptor_tests.cpp:164 in 00ec80b24e outdated

 160 | @@ -161,7 +161,7 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int
 161 |      // We must be able to estimate the max satisfaction size for any solvable descriptor top descriptor (but combo).
 162 |      const bool is_nontop_or_nonsolvable{!parse_priv->IsSolvable() || !parse_priv->GetOutputType()};
 163 |      const auto max_sat_maxsig{parse_priv->MaxSatisfactionWeight(true)};
 164 | -    const auto max_sat_nonmaxsig{parse_priv->MaxSatisfactionWeight(true)};
 165 | +    const auto max_sat_nonmaxsig{parse_priv->MaxSatisfactionWeight(false)};

furszy commented at 2:23 PM on December 27, 2024:

Haven't verified all the scenarios but maybe you could verify that max_sat_nonmaxsig < max_sat_maxsig (or <= if that causes any complication).

brunoerg commented at 8:56 PM on December 27, 2024:

Done.

test: descriptor: fix test for `MaxSatisfactionWeight`

To get the maximum size of a satisfaction for a descriptor
without considering the max sig, the parameter `use_max_sig`
should be false.

b29d68f942

brunoerg force-pushed on Dec 27, 2024
brunoerg commented at 8:56 PM on December 27, 2024: contributor

Force-pushed addressing #31570 (review)
tdb3 approved
tdb3 commented at 5:22 PM on December 28, 2024: contributor

re ACK b29d68f942e333d2cfdd6be8d49fe484e0d15e11

in src/test/descriptor_tests.cpp:165 in b29d68f942

 160 | @@ -161,7 +161,8 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int
 161 |      // We must be able to estimate the max satisfaction size for any solvable descriptor top descriptor (but combo).
 162 |      const bool is_nontop_or_nonsolvable{!parse_priv->IsSolvable() || !parse_priv->GetOutputType()};
 163 |      const auto max_sat_maxsig{parse_priv->MaxSatisfactionWeight(true)};
 164 | -    const auto max_sat_nonmaxsig{parse_priv->MaxSatisfactionWeight(true)};
 165 | +    const auto max_sat_nonmaxsig{parse_priv->MaxSatisfactionWeight(false)};
 166 | +    BOOST_CHECK(max_sat_nonmaxsig <= max_sat_maxsig);

furszy commented at 2:38 PM on December 30, 2024:

nit: could use BOOST_CHECK_LE.

furszy commented at 2:38 PM on December 30, 2024: member

utACK b29d68f942e
fjahr commented at 4:20 PM on December 30, 2024: contributor

utACK b29d68f942e333d2cfdd6be8d49fe484e0d15e11

Nice catch!
achow101 commented at 7:27 PM on December 30, 2024: member

ACK b29d68f942e333d2cfdd6be8d49fe484e0d15e11
achow101 merged this on Dec 30, 2024
achow101 closed this on Dec 30, 2024
sedited referenced this in commit 230a439a4a on Jan 17, 2025
stickies-v referenced this in commit d760fd3dda on Mar 17, 2025
stickies-v referenced this in commit cc83553352 on Mar 17, 2025
stickies-v referenced this in commit 2614933f06 on Mar 17, 2025
stickies-v referenced this in commit b70418c5fc on Mar 17, 2025
stickies-v referenced this in commit 69f8a1fe50 on Mar 17, 2025
bug-castercv502 referenced this in commit 44b075fe71 on Sep 28, 2025
bitcoin locked this on Dec 30, 2025

Contributors

Labels

test: descriptor: fix test for `MaxSatisfactionWeight` #31570

Code Coverage & Benchmarks

Reviews