diff --git a/test/functional/abc-minimaldata-activation.py b/test/functional/abc-minimaldata-activation.py --- a/test/functional/abc-minimaldata-activation.py +++ b/test/functional/abc-minimaldata-activation.py @@ -136,8 +136,7 @@ (Can't actually get banned, since bitcoind won't ban local peers.)""" self.nodes[0].p2p.send_blocks_and_test( - [block], self.nodes[0], success=False, reject_code=reject_code, reject_reason=reject_reason) - self.nodes[0].p2p.wait_for_disconnect() + [block], self.nodes[0], success=False, reject_code=reject_code, reject_reason=reject_reason, expect_disconnect=True) self.reconnect_p2p() def run_test(self): diff --git a/test/functional/abc-schnorr.py b/test/functional/abc-schnorr.py --- a/test/functional/abc-schnorr.py +++ b/test/functional/abc-schnorr.py @@ -124,7 +124,7 @@ (Can't actually get banned, since bitcoind won't ban local peers.)""" self.nodes[0].p2p.send_txs_and_test( - [tx], self.nodes[0], success=False, expect_disconnect=True, reject_code=reject_code, reject_reason=reject_reason) + [tx], self.nodes[0], success=False, reject_code=reject_code, reject_reason=reject_reason, expect_disconnect=True) self.reconnect_p2p() def check_for_ban_on_rejected_block(self, block, reject_code=None, reject_reason=None): @@ -132,8 +132,7 @@ (Can't actually get banned, since bitcoind won't ban local peers.)""" self.nodes[0].p2p.send_blocks_and_test( - [block], self.nodes[0], success=False, reject_code=reject_code, reject_reason=reject_reason) - self.nodes[0].p2p.wait_for_disconnect() + [block], self.nodes[0], success=False, reject_code=reject_code, reject_reason=reject_reason, expect_disconnect=True) self.reconnect_p2p() def run_test(self): diff --git a/test/functional/abc-schnorrmultisig-activation.py b/test/functional/abc-schnorrmultisig-activation.py --- a/test/functional/abc-schnorrmultisig-activation.py +++ b/test/functional/abc-schnorrmultisig-activation.py @@ -160,8 +160,7 @@ (Can't actually get banned, since bitcoind won't ban local peers.)""" self.nodes[0].p2p.send_blocks_and_test( - [block], self.nodes[0], success=False, reject_code=reject_code, reject_reason=reject_reason) - self.nodes[0].p2p.wait_for_disconnect() + [block], self.nodes[0], success=False, reject_code=reject_code, reject_reason=reject_reason, expect_disconnect=True) self.reconnect_p2p() def run_test(self): diff --git a/test/functional/abc-segwit-recovery.py b/test/functional/abc-segwit-recovery.py --- a/test/functional/abc-segwit-recovery.py +++ b/test/functional/abc-segwit-recovery.py @@ -152,8 +152,7 @@ # submit current tip and check it was rejected (and we are banned) def rejected(node, reject_code, reject_reason): node.p2p.send_blocks_and_test( - [self.tip], node, success=False, reject_code=reject_code, reject_reason=reject_reason) - node.p2p.wait_for_disconnect() + [self.tip], node, success=False, reject_code=reject_code, reject_reason=reject_reason, expect_disconnect=True) self.reconnect_p2p() # move the tip back to a previous block diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -900,8 +900,8 @@ tx.vin.append(CTxIn(COutPoint(b64a.vtx[1].sha256, 0))) b64a = self.update_block("64a", [tx]) assert_equal(len(b64a.serialize()), LEGACY_MAX_BLOCK_SIZE + 8) - self.sync_blocks([b64a], success=False, reject_code=1, - reject_reason=b'error parsing message') + self.sync_blocks([b64a], success=False, + reject_reason='non-canonical ReadCompactSize(): iostream error') # bitcoind doesn't disconnect us for sending a bloated block, but if we subsequently # resend the header message, it won't send us the getdata message again. Just @@ -1421,8 +1421,8 @@ """Sends blocks to test node. Syncs and verifies that tip has advanced to most recent block. Call with success = False if the tip shouldn't advance to the most recent block.""" - self.nodes[0].p2p.send_blocks_and_test(blocks, self.nodes[0], success=success, reject_code=reject_code, - reject_reason=reject_reason, request_block=request_block, timeout=timeout) + self.nodes[0].p2p.send_blocks_and_test(blocks, self.nodes[0], success=success, + reject_reason=reject_reason, request_block=request_block, timeout=timeout, expect_disconnect=reconnect) if reconnect: self.reconnect_p2p() diff --git a/test/functional/p2p_invalid_tx.py b/test/functional/p2p_invalid_tx.py --- a/test/functional/p2p_invalid_tx.py +++ b/test/functional/p2p_invalid_tx.py @@ -142,10 +142,12 @@ # Mempool should be empty assert_equal(0, node.getmempoolinfo()['size']) - assert_equal(2, len(node.getpeerinfo())) # p2ps[1] is still connected + # p2ps[1] is still connected + assert_equal(2, len(node.getpeerinfo())) self.log.info('Send the withhold tx ... ') - node.p2p.send_txs_and_test([tx_withhold], node, success=True) + with node.assert_debug_log(expected_msgs=["bad-txns-in-belowout"]): + node.p2p.send_txs_and_test([tx_withhold], node, success=True) # Transactions that should end up in the mempool expected_mempool = { @@ -170,15 +172,8 @@ 'Test a transaction that is rejected, with BIP61 disabled') self.restart_node(0, ['-enablebip61=0', '-persistmempool=0']) self.reconnect_p2p(num_connections=1) - with node.assert_debug_log(expected_msgs=[ - "{} from peer=0 was not accepted: mandatory-script-verify-flag-failed (Invalid OP_IF construction) (code 16)".format( - tx1.hash), - "disconnecting peer=0", - ]): - node.p2p.send_txs_and_test( - [tx1], node, success=False, expect_disconnect=True) - # send_txs_and_test will have waited for disconnect, so we can safely check that no reject has been received - assert_equal(node.p2p.reject_code_received, None) + node.p2p.send_txs_and_test( + [tx1], node, success=False, reject_reason="{} from peer=0 was not accepted: mandatory-script-verify-flag-failed (Invalid OP_IF construction) (code 16)".format(tx1.hash), expect_disconnect=True) if __name__ == '__main__': diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -553,8 +553,6 @@ def __init__(self): super().__init__() - self.reject_code_received = None - self.reject_reason_received = None # store of blocks. key is block hash, value is a CBlock object self.block_store = {} self.last_block_hash = '' @@ -608,12 +606,8 @@ if response is not None: self.send_message(response) - def on_reject(self, message): - """Store reject reason and code for testing.""" - self.reject_code_received = message.code - self.reject_reason_received = message.reason - - def send_blocks_and_test(self, blocks, node, *, success=True, request_block=True, reject_code=None, reject_reason=None, timeout=60): + def send_blocks_and_test(self, blocks, node, *, success=True, request_block=True, reject_code=None, reject_reason=None, expect_disconnect=False, timeout=60): + # TODO: Migrate all tests off of using `reject_code` and then remove it. """Send blocks to test node and test whether the tip advances. - add all blocks to our block_store @@ -623,74 +617,70 @@ ensure that any getdata messages are responded to - if success is True: assert that the node's tip advances to the most recent block - if success is False: assert that the node's tip doesn't advance - - if reject_code and reject_reason are set: assert that the correct reject message is received""" + - if reject_reason is set: assert that the correct reject message is logged""" with mininode_lock: - self.reject_code_received = None - self.reject_reason_received = None - for block in blocks: self.block_store[block.sha256] = block self.last_block_hash = block.sha256 - self.send_message(msg_headers([CBlockHeader(blocks[-1])])) + # TODO: Remove decode() once all callers are migrated to use strings instead of bytes. + if isinstance(reject_reason, bytes): + reject_reason = reject_reason.decode('utf-8') + reject_reason = [reject_reason] if reject_reason else [] + with node.assert_debug_log(expected_msgs=reject_reason): + self.send_message(msg_headers([CBlockHeader(blocks[-1])])) - if request_block: - wait_until( - lambda: blocks[-1].sha256 in self.getdata_requests, timeout=timeout, lock=mininode_lock) + if request_block: + wait_until( + lambda: blocks[-1].sha256 in self.getdata_requests, timeout=timeout, lock=mininode_lock) - if success: - wait_until(lambda: node.getbestblockhash() == - blocks[-1].hash, timeout=timeout) - else: - assert node.getbestblockhash() != blocks[-1].hash + if expect_disconnect: + self.wait_for_disconnect() + else: + self.sync_with_ping() - if reject_code is not None: - wait_until(lambda: self.reject_code_received == - reject_code, lock=mininode_lock) - if reject_reason is not None: - wait_until(lambda: self.reject_reason_received == - reject_reason, lock=mininode_lock) + if success: + wait_until(lambda: node.getbestblockhash() == + blocks[-1].hash, timeout=timeout) + else: + assert node.getbestblockhash() != blocks[-1].hash def send_txs_and_test(self, txs, node, *, success=True, expect_disconnect=False, reject_code=None, reject_reason=None): + # TODO: Migrate all tests off of using `reject_code` and then remove it. """Send txs to test node and test whether they're accepted to the mempool. - add all txs to our tx_store - send tx messages for all txs - if success is True/False: assert that the txs are/are not accepted to the mempool - if expect_disconnect is True: Skip the sync with ping - - if reject_code and reject_reason are set: assert that the correct reject message is received.""" + - if reject_reason is set: assert that the correct reject message is logged.""" with mininode_lock: - self.reject_code_received = None - self.reject_reason_received = None - for tx in txs: self.tx_store[tx.sha256] = tx - for tx in txs: - self.send_message(msg_tx(tx)) - - if expect_disconnect: - self.wait_for_disconnect() - else: - self.sync_with_ping() - - raw_mempool = node.getrawmempool() - if success: - # Check that all txs are now in the mempool - for tx in txs: - assert tx.hash in raw_mempool, "{} not found in mempool".format( - tx.hash) - else: - # Check that none of the txs are now in the mempool + # TODO: Remove decode() once all callers are migrated to use strings instead of bytes. + if isinstance(reject_reason, bytes): + reject_reason = reject_reason.decode('utf-8') + reject_reason = [reject_reason] if reject_reason else [] + with node.assert_debug_log(expected_msgs=reject_reason): for tx in txs: - assert tx.hash not in raw_mempool, "{} tx found in mempool".format( - tx.hash) - - if reject_code is not None: - wait_until(lambda: self.reject_code_received == - reject_code, lock=mininode_lock) - if reject_reason is not None: - wait_until(lambda: self.reject_reason_received == - reject_reason, lock=mininode_lock) + self.send_message(msg_tx(tx)) + + if expect_disconnect: + self.wait_for_disconnect() + else: + self.sync_with_ping() + + raw_mempool = node.getrawmempool() + if success: + # Check that all txs are now in the mempool + for tx in txs: + assert tx.hash in raw_mempool, "{} not found in mempool".format( + tx.hash) + else: + # Check that none of the txs are now in the mempool + for tx in txs: + assert tx.hash not in raw_mempool, "{} tx found in mempool".format( + tx.hash)