Merge bitcoin/bitcoin#22738: test: fix failure in feature_nulldummy.py on single-core machines

7720d4f test: fix failure in feature_nulldummy.py on single-core machines (Sebastian Falbesoner)
646b388 test: refactor: use named args for block_submit in feature_nulldummy.py (Sebastian Falbesoner)

Pull request description:

  On single-core machines, executing the test `feature_nulldummy.py` results in the following assertion error:

  ```
      ...
      2021-08-18T15:37:58.805000Z TestFramework (INFO): Test 4: Non-NULLDUMMY base multisig transaction is invalid after activation
      2021-08-18T15:37:58.814000Z TestFramework (ERROR): Assertion failed
      Traceback (most recent call last):
        File "[...]/test/functional/test_framework/test_framework.py", line 131, in main
          self.run_test()
        File "[...]/test/functional/feature_nulldummy.py", line 107, in run_test
          self.block_submit(self.nodes[0], [test4tx], accept=False)
        File "[...]/test/functional/feature_nulldummy.py", line 134, in block_submit
          assert_equal(None if accept else 'block-validation-failed', node.submitblock(block.serialize().hex()))
        File "[...]/test/functional/test_framework/util.py", line 49, in assert_equal
          raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
      AssertionError: not(block-validation-failed == non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero))
      2021-08-18T15:37:58.866000Z TestFramework (INFO): Stopping nodes
      ...
  ```

  There are hardly any single-core machines around anymore, but the behaviour can be reproduced on a multi-core machine by patching the function `GetNumCores()` to return 1 on the master branch and running `feature_nulldummy.py`:

  ```diff
  diff --git a/src/util/system.cpp b/src/util/system.cpp
  index 30d4103..149b512fc 100644
  --- a/src/util/system.cpp
  +++ b/src/util/system.cpp
  @@ -1338,7 +1338,7 @@ bool SetupNetworking()

   int GetNumCores()
   {
  -    return std::thread::hardware_concurrency();
  +    return 1;
   }
  ```
  As solution, parallel script verification is disabled (`-par=1`) and the exact reject reason is checked, which also increases the precision of the test (the possibility that the block is rejected because of another unintended reason is ruled out). See also related PR #22711 which applies the same approach for the p2p segwit test. The PR also includes a refactoring commit which changes the calls to `self.block_submit()` to use named arguments and removes the default value for parameter `accept` (i.e. explicitely passing `accept=...` is mandatory), with the aim to increase the test readability.

ACKs for top commit:
  josibake:
    ACK bitcoin/bitcoin@7720d4f
  Saviour1001:
    Tested ACK <code>[7720d4f](https://github.com/bitcoin/bitcoin/commit/7720d4f650015272dc7109238230520f71858c6c)</code>

Tree-SHA512: 8a31ebab3e2ab38e555d7a23139b3324a134a0dedc5b879a2419348ae858323882dbbfcbbf88b68e4f8d7eea8cfe43ee19da1d0d2a36c93ae7878c4980cac31d

Author: MarcoFalke

test/functional/feature_nulldummy.py

@@ -24,7 +24,10 @@
 from test_framework.messages import CTransaction
 from test_framework.script import CScript
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal, assert_raises_rpc_error
+from test_framework.util import (
+    assert_equal,
+    assert_raises_rpc_error,
+)
 
 NULLDUMMY_ERROR = "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero)"
 
@@ -51,6 +54,7 @@ def set_test_params(self):
         self.extra_args = [[
             f'-segwitheight={COINBASE_MATURITY + 5}',
             '-addresstype=legacy',
+            '-par=1',  # Use only one script thread to get the exact reject reason for testing
         ]]
 
     def skip_test_if_missing_module(self):
@@ -86,36 +90,36 @@ def run_test(self):
         txid2 = self.nodes[0].sendrawtransaction(test1txs[1].serialize_with_witness().hex(), 0)
         test1txs.append(create_transaction(self.nodes[0], coinbase_txid[1], self.wit_ms_address, amount=49))
         txid3 = self.nodes[0].sendrawtransaction(test1txs[2].serialize_with_witness().hex(), 0)
-        self.block_submit(self.nodes[0], test1txs, False, True)
+        self.block_submit(self.nodes[0], test1txs, accept=True)
 
         self.log.info("Test 2: Non-NULLDUMMY base multisig transaction should not be accepted to mempool before activation")
         test2tx = create_transaction(self.nodes[0], txid2, self.ms_address, amount=47)
         trueDummy(test2tx)
         assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, test2tx.serialize_with_witness().hex(), 0)
 
         self.log.info(f"Test 3: Non-NULLDUMMY base transactions should be accepted in a block before activation [{COINBASE_MATURITY + 4}]")
-        self.block_submit(self.nodes[0], [test2tx], False, True)
+        self.block_submit(self.nodes[0], [test2tx], accept=True)
 
         self.log.info("Test 4: Non-NULLDUMMY base multisig transaction is invalid after activation")
         test4tx = create_transaction(self.nodes[0], test2tx.hash, self.address, amount=46)
         test6txs = [CTransaction(test4tx)]
         trueDummy(test4tx)
         assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, test4tx.serialize_with_witness().hex(), 0)
-        self.block_submit(self.nodes[0], [test4tx])
+        self.block_submit(self.nodes[0], [test4tx], accept=False)
 
         self.log.info("Test 5: Non-NULLDUMMY P2WSH multisig transaction invalid after activation")
         test5tx = create_transaction(self.nodes[0], txid3, self.wit_address, amount=48)
         test6txs.append(CTransaction(test5tx))
         test5tx.wit.vtxinwit[0].scriptWitness.stack[0] = b'\x01'
         assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, test5tx.serialize_with_witness().hex(), 0)
-        self.block_submit(self.nodes[0], [test5tx], True)
+        self.block_submit(self.nodes[0], [test5tx], with_witness=True, accept=False)
 
         self.log.info(f"Test 6: NULLDUMMY compliant base/witness transactions should be accepted to mempool and in block after activation [{COINBASE_MATURITY + 5}]")
         for i in test6txs:
             self.nodes[0].sendrawtransaction(i.serialize_with_witness().hex(), 0)
-        self.block_submit(self.nodes[0], test6txs, True, True)
+        self.block_submit(self.nodes[0], test6txs, with_witness=True, accept=True)
 
-    def block_submit(self, node, txs, witness=False, accept=False):
+    def block_submit(self, node, txs, *, with_witness=False, accept):
         tmpl = node.getblocktemplate(NORMAL_GBT_REQUEST_PARAMS)
         assert_equal(tmpl['previousblockhash'], self.lastblockhash)
         assert_equal(tmpl['height'], self.lastblockheight + 1)
@@ -124,11 +128,12 @@ def block_submit(self, node, txs, witness=False, accept=False):
             tx.rehash()
             block.vtx.append(tx)
         block.hashMerkleRoot = block.calc_merkle_root()
-        witness and add_witness_commitment(block)
+        if with_witness:
+            add_witness_commitment(block)
         block.rehash()
         block.solve()
-        assert_equal(None if accept else 'block-validation-failed', node.submitblock(block.serialize().hex()))
-        if (accept):
+        assert_equal(None if accept else NULLDUMMY_ERROR, node.submitblock(block.serialize().hex()))
+        if accept:
             assert_equal(node.getbestblockhash(), block.hash)
             self.lastblockhash = block.hash
             self.lastblocktime += 1