test: descriptor: cover invalid multi/multi_a cases #32504

1. 
   brunoerg commented at 8:27 pm on May 14, 2025: contributor

   This PR adds test coverage for invalid multi() and multi_a() cases, see:

   1. https://github.com/bitcoin/bitcoin/blob/53eb5593f0a1a8ae5cf0fabea58e2f22193a5c55/src/script/descriptor.cpp#L1819-L1821

   2. https://github.com/bitcoin/bitcoin/blob/53eb5593f0a1a8ae5cf0fabea58e2f22193a5c55/src/script/descriptor.cpp#L1835-L1837

   3. https://github.com/bitcoin/bitcoin/blob/53eb5593f0a1a8ae5cf0fabea58e2f22193a5c55/src/script/descriptor.cpp#L1838-L1840

   We could also exercise to exceed the number of keys - 20 for multi and 999 for multi_a.

2. 
   DrahtBot commented at 8:27 pm on May 14, 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/32504.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	maflcko, darosior, glozow
   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.

3. DrahtBot added the label Tests on May 14, 2025
4. 
   maflcko commented at 5:50 am on May 15, 2025: member

   Nice, but two commits for adding three lines of related tests seems like it could be squashed?
5. test: descriptor: cover invalid multi/multi_a cases 58e55b17e6
6. brunoerg force-pushed on May 15, 2025
7. 
   brunoerg commented at 11:18 am on May 15, 2025: contributor

   Nice, but two commits for adding three lines of related tests seems like it could be squashed?

   Done.

8. 
   maflcko commented at 11:48 am on May 15, 2025: member

   lgtm ACK 58e55b17e632dbd4425dd64825b087f242ac4b7b
9. in src/test/descriptor_tests.cpp:930 in 58e55b17e6

   926@@ -927,6 +927,9 @@ BOOST_AUTO_TEST_CASE(descriptor_test)
   927     CheckUnparsable("multi(+1,L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1,5KYZdUEo39z3FPrtuX2QbbwGnNP5zTd7yyr2SC1j299sBCnWjss)", "multi(+1,03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", "Multi threshold '+1' is not valid"); // Invalid threshold
   928     CheckUnparsable("multi(0,L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1,5KYZdUEo39z3FPrtuX2QbbwGnNP5zTd7yyr2SC1j299sBCnWjss)", "multi(0,03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", "Multisig threshold cannot be 0, must be at least 1"); // Threshold of 0
   929     CheckUnparsable("multi(3,L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1,5KYZdUEo39z3FPrtuX2QbbwGnNP5zTd7yyr2SC1j299sBCnWjss)", "multi(3,03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", "Multisig threshold cannot be larger than the number of keys; threshold is 3 but only 2 keys specified"); // Threshold larger than number of keys
   930+    CheckUnparsable("", "multi(0)()", "Multi: expected ',', got ')'");
   

   ---

   ---

   

   sipa commented at 11:51 am on May 15, 2025:

   Strange, GitHub renders this line in the diff view as indended by 4 spaces compared to the line above. I don’t see anything strange in the code itself, though.

   ---

   

   brunoerg commented at 12:22 pm on May 15, 2025:

   Yes, this is weird. I don’t know why.

   ---

   

   maflcko commented at 12:27 pm on May 15, 2025:

   The other lines in this test are excessively long. It would be better to use c-style multi-line string:

   0CheckUnparsable("multi("
   1  "bla,"
   2  "foo)"
   3);
   

   However, this seems unrelated to the changes here and would require reformatting the whole file.

10. 
    sipa commented at 11:51 am on May 15, 2025: member

    Concept ACK
11. brunoerg requested review from sipa on Jun 9, 2025
12. achow101 requested review from davidgumberg on Oct 22, 2025
13. achow101 requested review from achow101 on Oct 22, 2025
14. achow101 requested review from darosior on Oct 22, 2025
15. darosior approved
16. 
    darosior commented at 12:48 pm on October 23, 2025: member

    utACK 58e55b17e632dbd4425dd64825b087f242ac4b7b
17. 
    glozow commented at 7:37 pm on October 27, 2025: member

    ACK 58e55b17e632dbd4425dd64825b087f242ac4b7b

    Read BIP 383, 387 and the relevant code, mutation tested to see this is new coverage.

18. glozow merged this on Oct 27, 2025
19. glozow closed this on Oct 27, 2025

    ---

20. brunoerg deleted the branch on Oct 27, 2025

Contributors
brunoerg DrahtBot maflcko sipa darosior glozow

Review Requested
sipa davidgumberg achow101

Labels
Tests