foster/bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-05 14:06:27 -05:00
Code Issues Activity

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

Browse source 
7720d4f650 test: fix failure in feature_nulldummy.py on single-core machines (Sebastian Falbesoner)
646b3885f7 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 30d410381..149b512fc 100644
  --- a/src/util/system.cpp
  +++ b/src/util/system.cpp
  @@ -1338,7 +1338,7 @@ bool SetupNetworking()

   int GetNumCores()
   {
  -    return std:🧵: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 7720d4f650
  Saviour1001:
    Tested ACK <code>[7720d4f](7720d4f650)</code>

Tree-SHA512: 8a31ebab3e2ab38e555d7a23139b3324a134a0dedc5b879a2419348ae858323882dbbfcbbf88b68e4f8d7eea8cfe43ee19da1d0d2a36c93ae7878c4980cac31d
This commit is contained in:
MarcoFalke 2021-08-26 11:11:36 +02:00
commit 0492b56e38
No known key found for this signature in database
GPG key ID: CE2B75697E69A548
1 changed files with 15 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
test/functional/feature_nulldummy.py
View file

@ -24,7 +24,10 @@ from test_framework.blocktools import (
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 @@ class NULLDUMMYTest(BitcoinTestFramework):
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,7 +90,7 @@ class NULLDUMMYTest(BitcoinTestFramework):
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)
@ -94,28 +98,28 @@ class NULLDUMMYTest(BitcoinTestFramework):
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 @@ class NULLDUMMYTest(BitcoinTestFramework):
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