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

1. 
   theStack commented at 2:37 am on November 18, 2019: member

   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. fanquake requested review from instagibbs on Nov 20, 2019
4. 
   instagibbs commented at 1:51 pm on November 21, 2019: member

   huh, permitbaremultisig has no tests either, good you’re working through these.
5. 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.
6. 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
7. theStack force-pushed on Nov 21, 2019
8. 
   instagibbs commented at 3:12 pm on November 21, 2019: member

   utACK https://github.com/bitcoin/bitcoin/pull/17502/commits/1bb5d517aa616c1d5b5801d2ea36a2de5fb61eba
9. fanquake requested review from MarcoFalke on Nov 21, 2019
10. 
    DrahtBot commented at 12:00 pm on November 23, 2019: member

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

    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.

11. MarcoFalke referenced this in commit ea756bc48c on Dec 10, 2019
12. MarcoFalke merged this on Dec 10, 2019
13. MarcoFalke closed this on Dec 10, 2019

    ---

14. sidhujag referenced this in commit 67c61e5a9a on Dec 10, 2019
15. MarcoFalke referenced this in commit ec9b964cc9 on Jan 16, 2020
16. sidhujag referenced this in commit 6f3eaef555 on Jan 17, 2020
17. MarcoFalke referenced this in commit 0130abbdb7 on Jan 30, 2020
18. deadalnix referenced this in commit 0fb36f0811 on Nov 7, 2020
19. sidhujag referenced this in commit c4127b0278 on Nov 10, 2020
20. sidhujag referenced this in commit 348d49416b on Nov 10, 2020
21. theStack deleted the branch on Dec 1, 2020
22. DrahtBot locked this on Feb 15, 2022

Contributors
theStack instagibbs DrahtBot

Review Requested
MarcoFalke

Labels
Tests