test: add unit test for non-standard bare multisig txs #17502

1. 
   theStack commented at 2:37 AM on November 18, 2019: contributor

   Approaches another missing unit test of issue #17394: Checks that the function IsStandardTx() returns rejection reason "bare-multisig" if any one of the outputs' scriptPubKey has bare multisignature format (i.e. M <PubKey1> <PubKey2> ... <PubKeyN> N OP_CHECKSIG, not P2SH!) and the policy flag fIsBareMultisigStd is set to false.

2. fanquake added the label Tests on Nov 18, 2019
3. theStack cross-referenced this on Nov 20, 2019 from issue test: add functional test for non-standard bare multisig txs by theStack
4. fanquake requested review from instagibbs on Nov 20, 2019
5. theStack cross-referenced this on Nov 21, 2019 from issue Add tests checking missing reject reasons for function IsStandardTx by theStack
6. 
   instagibbs commented at 1:51 PM on November 21, 2019: member

   huh, permitbaremultisig has no tests either, good you're working through these.

7. in src/test/transaction_tests.cpp:803 in ac9da052ab outdated

    796 | @@ -797,6 +797,17 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
    797 |      reason.clear();
    798 |      BOOST_CHECK(!IsStandardTx(CTransaction(t), reason));
    799 |      BOOST_CHECK_EQUAL(reason, "scriptsig-size");
    800 | +
    801 | +    // Check bare multisig (standard if policy flag fIsBareMultisigStd is set)
    802 | +    fIsBareMultisigStd = true;
    803 | +    t.vout[0].scriptPubKey = GetScriptForMultisig(2, std::vector<CPubKey>(3, key.GetPubKey())); // 2-of-3
   

   ---

   ---

   

   instagibbs commented at 1:52 PM on November 21, 2019:

   For this test I think you should do the simplest base case to make it trivially certain you're not hitting some size-based restriction(as I recall, naked multisigs increase virtual size of transactions during relay....)

   so just make a 1-of-1 multisig

   ---

   

   theStack commented at 3:06 PM on November 21, 2019:

   Done, thanks for reviewing.

8. test: add unit test for non-standard bare multisig txs

   The function IsStandardTx() returns rejection reason "bare-multisig" if the
   transaction has a bare multisig output and the policy flag fIsBareMultisigStd
   is false (set by the boolean command-line argument "-permitbaremultisig" -- for
   the unit test, we simply set the global flag variable directly).

   1bb5d517aa
9. theStack force-pushed on Nov 21, 2019
10. 
    instagibbs commented at 3:12 PM on November 21, 2019: member

    utACK https://github.com/bitcoin/bitcoin/pull/17502/commits/1bb5d517aa616c1d5b5801d2ea36a2de5fb61eba

11. fanquake requested review from MarcoFalke on Nov 21, 2019
12. 
    DrahtBot commented at 12:00 PM on November 23, 2019: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #17570 (test: add unit test for non-standard txs w/ too large tx size by KaanKC)

    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.

13. MarcoFalke referenced this in commit ea756bc48c on Dec 10, 2019
14. MarcoFalke merged this on Dec 10, 2019
15. MarcoFalke closed this on Dec 10, 2019

    ---

16. sidhujag referenced this in commit 67c61e5a9a on Dec 10, 2019
17. MarcoFalke referenced this in commit ec9b964cc9 on Jan 16, 2020
18. sidhujag referenced this in commit 6f3eaef555 on Jan 17, 2020
19. fanquake cross-referenced this on Jan 29, 2020 from issue tests: reset fIsBareMultisigStd after bare-multisig tests by fanquake
20. MarcoFalke referenced this in commit 0130abbdb7 on Jan 30, 2020
21. MarkLTZ cross-referenced this on Apr 4, 2020 from issue Bitcoin PR tracking by MarkLTZ
22. deadalnix referenced this in commit 0fb36f0811 on Nov 7, 2020
23. sidhujag referenced this in commit c4127b0278 on Nov 10, 2020
24. sidhujag referenced this in commit 348d49416b on Nov 10, 2020
25. theStack deleted the branch on Dec 1, 2020
26. bitcoin locked this on Feb 15, 2022

Contributors

theStackinstagibbsDrahtBot

Review Requested
MarcoFalke