Merge #11771: [tests] Change invalidtxrequest to use BitcoinTestFramework

95e2e9a [tests] Change invalidtxrequest to use BitcoinTestFramework (John Newbery)
359d067 [tests] Fix flake8 warnings in invalidtxrequest (John Newbery)
c32cf9f [tests] Add P2PDataStore class (John Newbery)
cc046f6 [tests] Reduce NodeConn connection logging from info to debug (John Newbery)

Pull request description:

  Next step in #10603

  - first commit changes log level for an internal log from INFO to DEBUG. (Not really related, but I started finding the INFO level logging annoying when debuging test failures)
  - second commit introduces a `P2PStub` class - a subclass of `NodeConnCB` which has its own block and tx store and responds appropriately to getdata requests. Not all the functionality is used in `invalidtxrequest.py`, but will be used in `invalidblockrequest.py` and `p2p-fullblocktest` when those are changed to use `BitcoinTestFramework`
  - third commit tidies up `invalidtxrequest.py`
  - fourth commit removes usage of `ComparisonTestFramework`

Tree-SHA512: f3085c73c15d6ce894e401490bce8a7fa7cf52b0c9d135ff7e351f1f6f517c99accab8588fcdc443f39ea8315329aaabd66b2baa32499df5a774737882030373

3 files changed, 182 insertions, 48 deletions

diff --git a/test/functional/p2p_invalid_tx.py b/test/functional/p2p_invalid_tx.py
index 9c1100e070..64fada38e2 100755
--- a/test/functional/p2p_invalid_tx.py
+++ b/test/functional/p2p_invalid_tx.py
@@ -4,68 +4,50 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test node responses to invalid transactions.
 
-In this test we connect to one node over p2p, and test tx requests.
-"""
+In this test we connect to one node over p2p, and test tx requests."""
+from test_framework.blocktools import create_block, create_coinbase, create_transaction
+from test_framework.messages import COIN
+from test_framework.mininode import network_thread_start, P2PDataStore
+from test_framework.test_framework import BitcoinTestFramework
 
-from test_framework.test_framework import ComparisonTestFramework
-from test_framework.comptool import TestManager, TestInstance, RejectResult
-from test_framework.blocktools import *
-import time
+class InvalidTxRequestTest(BitcoinTestFramework):
 
-
-
-# Use the ComparisonTestFramework with 1 node: only use --testbinary.
-class InvalidTxRequestTest(ComparisonTestFramework):
-
-    ''' Can either run this test as 1 node with expected answers, or two and compare them. 
-        Change the "outcome" variable from each TestInstance object to only do the comparison. '''
     def set_test_params(self):
         self.num_nodes = 1
         self.setup_clean_chain = True
+        self.extra_args = [["-whitelist=127.0.0.1"]]
 
     def run_test(self):
-        test = TestManager(self, self.options.tmpdir)
-        test.add_all_connections(self.nodes)
-        self.tip = None
-        self.block_time = None
+        # Add p2p connection to node0
+        node = self.nodes[0]  # convenience reference to the node
+        node.add_p2p_connection(P2PDataStore())
+
         network_thread_start()
-        test.run()
+        node.p2p.wait_for_verack()
 
-    def get_tests(self):
-        if self.tip is None:
-            self.tip = int("0x" + self.nodes[0].getbestblockhash(), 0)
-        self.block_time = int(time.time())+1
+        best_block = self.nodes[0].getbestblockhash()
+        tip = int(best_block, 16)
+        best_block_time = self.nodes[0].getblock(best_block)['time']
+        block_time = best_block_time + 1
 
-        '''
-        Create a new block with an anyone-can-spend coinbase
-        '''
+        self.log.info("Create a new block with an anyone-can-spend coinbase.")
         height = 1
-        block = create_block(self.tip, create_coinbase(height), self.block_time)
-        self.block_time += 1
+        block = create_block(tip, create_coinbase(height), block_time)
+        block_time += 1
         block.solve()
         # Save the coinbase for later
-        self.block1 = block
-        self.tip = block.sha256
+        block1 = block
+        tip = block.sha256
         height += 1
-        yield TestInstance([[block, True]])
+        node.p2p.send_blocks_and_test([block], node, success=True)
 
-        '''
-        Now we need that block to mature so we can spend the coinbase.
-        '''
-        test = TestInstance(sync_every_block=False)
-        for i in range(100):
-            block = create_block(self.tip, create_coinbase(height), self.block_time)
-            block.solve()
-            self.tip = block.sha256
-            self.block_time += 1
-            test.blocks_and_transactions.append([block, True])
-            height += 1
-        yield test
+        self.log.info("Mature the block.")
+        self.nodes[0].generate(100)
 
         # b'\x64' is OP_NOTIF
         # Transaction will be rejected with code 16 (REJECT_INVALID)
-        tx1 = create_transaction(self.block1.vtx[0], 0, b'\x64', 50 * COIN - 12000)
-        yield TestInstance([[tx1, RejectResult(16, b'mandatory-script-verify-flag-failed')]])
+        tx1 = create_transaction(block1.vtx[0], 0, b'\x64', 50 * COIN - 12000)
+        node.p2p.send_txs_and_test([tx1], node, success=False, reject_code=16, reject_reason=b'mandatory-script-verify-flag-failed (Invalid OP_IF construction)')
 
         # TODO: test further transactions...
 
diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py
index a54a0299c7..46ef7521e0 100644
--- a/test/functional/test_framework/messages.py
+++ b/test/functional/test_framework/messages.py
@@ -44,6 +44,11 @@ NODE_UNSUPPORTED_SERVICE_BIT_5 = (1 << 5)
 NODE_UNSUPPORTED_SERVICE_BIT_7 = (1 << 7)
 NODE_NETWORK_LIMITED = (1 << 10)
 
+MSG_TX = 1
+MSG_BLOCK = 2
+MSG_WITNESS_FLAG = 1 << 30
+MSG_TYPE_MASK = 0xffffffff >> 2
+
 # Serialization/deserialization tools
 def sha256(s):
     return hashlib.new('sha256', s).digest()
@@ -204,8 +209,6 @@ class CAddress():
         return "CAddress(nServices=%i ip=%s port=%i)" % (self.nServices,
                                                          self.ip, self.port)
 
-MSG_WITNESS_FLAG = 1<<30
-
 class CInv():
     typemap = {
         0: "Error",
diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py
index 4d9661de37..99d0abc3f9 100755
--- a/test/functional/test_framework/mininode.py
+++ b/test/functional/test_framework/mininode.py
@@ -10,7 +10,9 @@ This python code was modified from ArtForz' public domain half-a-node, as
 found in the mini-node branch of http://github.com/jgarzik/pynode.
 
 P2PConnection: A low-level connection object to a node's P2P interface
-P2PInterface: A high-level interface object for communicating to a node over P2P"""
+P2PInterface: A high-level interface object for communicating to a node over P2P
+P2PDataStore: A p2p interface class that keeps a store of transactions and blocks
+              and can respond correctly to getdata and getheaders messages"""
 import asyncore
 from collections import defaultdict
 from io import BytesIO
@@ -86,7 +88,7 @@ class P2PConnection(asyncore.dispatcher):
         self.network = net
         self.disconnect = False
 
-        logger.info('Connecting to Bitcoin Node: %s:%d' % (self.dstaddr, self.dstport))
+        logger.debug('Connecting to Bitcoin Node: %s:%d' % (self.dstaddr, self.dstport))
 
         try:
             self.connect((dstaddr, dstport))
@@ -356,10 +358,22 @@ class P2PInterface(P2PConnection):
         wait_until(test_function, timeout=timeout, lock=mininode_lock)
 
     def wait_for_getdata(self, timeout=60):
+        """Waits for a getdata message.
+
+        Receiving any getdata message will satisfy the predicate. the last_message["getdata"]
+        value must be explicitly cleared before calling this method, or this will return
+        immediately with success. TODO: change this method to take a hash value and only
+        return true if the correct block/tx has been requested."""
         test_function = lambda: self.last_message.get("getdata")
         wait_until(test_function, timeout=timeout, lock=mininode_lock)
 
     def wait_for_getheaders(self, timeout=60):
+        """Waits for a getheaders message.
+
+        Receiving any getheaders message will satisfy the predicate. the last_message["getheaders"]
+        value must be explicitly cleared before calling this method, or this will return
+        immediately with success. TODO: change this method to take a hash value and only
+        return true if the correct block header has been requested."""
         test_function = lambda: self.last_message.get("getheaders")
         wait_until(test_function, timeout=timeout, lock=mininode_lock)
 
@@ -440,3 +454,138 @@ def network_thread_join(timeout=10):
     for thread in network_threads:
         thread.join(timeout)
         assert not thread.is_alive()
+
+class P2PDataStore(P2PInterface):
+    """A P2P data store class.
+
+    Keeps a block and transaction store and responds correctly to getdata and getheaders requests."""
+
+    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 = ''
+        # store of txs. key is txid, value is a CTransaction object
+        self.tx_store = {}
+        self.getdata_requests = []
+
+    def on_getdata(self, message):
+        """Check for the tx/block in our stores and if found, reply with an inv message."""
+        for inv in message.inv:
+            self.getdata_requests.append(inv.hash)
+            if (inv.type & MSG_TYPE_MASK) == MSG_TX and inv.hash in self.tx_store.keys():
+                self.send_message(msg_tx(self.tx_store[inv.hash]))
+            elif (inv.type & MSG_TYPE_MASK) == MSG_BLOCK and inv.hash in self.block_store.keys():
+                self.send_message(msg_block(self.block_store[inv.hash]))
+            else:
+                logger.debug('getdata message type {} received.'.format(hex(inv.type)))
+
+    def on_getheaders(self, message):
+        """Search back through our block store for the locator, and reply with a headers message if found."""
+
+        locator, hash_stop = message.locator, message.hashstop
+
+        # Assume that the most recent block added is the tip
+        if not self.block_store:
+            return
+
+        headers_list = [self.block_store[self.last_block_hash]]
+        maxheaders = 2000
+        while headers_list[-1].sha256 not in locator.vHave:
+            # Walk back through the block store, adding headers to headers_list
+            # as we go.
+            prev_block_hash = headers_list[-1].hashPrevBlock
+            if prev_block_hash in self.block_store:
+                prev_block_header = self.block_store[prev_block_hash]
+                headers_list.append(prev_block_header)
+                if prev_block_header.sha256 == hash_stop:
+                    # if this is the hashstop header, stop here
+                    break
+            else:
+                logger.debug('block hash {} not found in block store'.format(hex(prev_block_hash)))
+                break
+
+        # Truncate the list if there are too many headers
+        headers_list = headers_list[:-maxheaders - 1:-1]
+        response = msg_headers(headers_list)
+
+        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, rpc, success=True, request_block=True, reject_code=None, reject_reason=None, timeout=60):
+        """Send blocks to test node and test whether the tip advances.
+
+         - add all blocks to our block_store
+         - send a headers message for the final block
+         - the on_getheaders handler will ensure that any getheaders are responded to
+         - if request_block is True: wait for getdata for each of the blocks. The on_getdata handler will
+           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"""
+
+        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([blocks[-1]]))
+
+        if request_block:
+            wait_until(lambda: blocks[-1].sha256 in self.getdata_requests, timeout=timeout, lock=mininode_lock)
+
+        if success:
+            wait_until(lambda: rpc.getbestblockhash() == blocks[-1].hash, timeout=timeout)
+        else:
+            assert rpc.getbestblockhash() != blocks[-1].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)
+
+    def send_txs_and_test(self, txs, rpc, success=True, reject_code=None, reject_reason=None):
+        """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: assert that the tx is accepted to the mempool
+         - if success is False: assert that the tx is not accepted to the mempool
+         - if reject_code and reject_reason are set: assert that the correct reject message is received."""
+
+        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))
+
+        self.sync_with_ping()
+
+        raw_mempool = rpc.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)
+
+        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)