Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qa: Add test for orphan handling #13003

Merged
merged 2 commits into from Apr 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 12 additions & 10 deletions test/functional/feature_block.py
Expand Up @@ -82,10 +82,7 @@ def set_test_params(self):
def run_test(self):
node = self.nodes[0] # convenience reference to the node

# reconnect_p2p() expects the network thread to be running
network_thread_start()

self.reconnect_p2p()
self.bootstrap_p2p() # Add one p2p connection to the node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers: The updates in feature_block.py just tweak the network_thread_start/network_thread_join calls to happen in a more logical order, and be consistent with other tests. This is internal test cleanup which is not directly related to the other changes in this PR.


self.block_heights = {}
self.coinbase_key = CECKey()
Expand Down Expand Up @@ -1296,18 +1293,23 @@ def update_block(self, block_number, new_transactions):
self.blocks[block_number] = block
return block

def reconnect_p2p(self):
def bootstrap_p2p(self):
"""Add a P2P connection to the node.

The node gets disconnected several times in this test. This helper
method reconnects the p2p and restarts the network thread."""

network_thread_join()
self.nodes[0].disconnect_p2ps()
Helper to connect and wait for version handshake."""
self.nodes[0].add_p2p_connection(P2PDataStore())
network_thread_start()
self.nodes[0].p2p.wait_for_verack()

def reconnect_p2p(self):
"""Tear down and bootstrap the P2P connection to the node.

The node gets disconnected several times in this test. This helper
method reconnects the p2p and restarts the network thread."""
self.nodes[0].disconnect_p2ps()
network_thread_join()
self.bootstrap_p2p()

def sync_blocks(self, blocks, success=True, reject_code=None, reject_reason=None, request_block=True, reconnect=False, timeout=60):
"""Sends blocks to test node. Syncs and verifies that tip has advanced to most recent block.

Expand Down
109 changes: 97 additions & 12 deletions test/functional/p2p_invalid_tx.py
Expand Up @@ -6,24 +6,48 @@

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.messages import (
COIN,
COutPoint,
CTransaction,
CTxIn,
CTxOut,
)
from test_framework.mininode import network_thread_start, P2PDataStore, network_thread_join
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
wait_until,
)

class InvalidTxRequestTest(BitcoinTestFramework):

class InvalidTxRequestTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1
self.setup_clean_chain = True
self.extra_args = [["-whitelist=127.0.0.1"]]

def bootstrap_p2p(self, *, num_connections=1):
"""Add a P2P connection to the node.

Helper to connect and wait for version handshake."""
for _ in range(num_connections):
self.nodes[0].add_p2p_connection(P2PDataStore())
network_thread_start()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be logical to start the network thread adding connections, right? Just asking because it's weird to me to see shutdown order not be the reverse of initialization order like:

network_thread_start()
add_p2p_connection()
...
disconnect_p2ps()
network_thread_join()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In test/functional/README.md it says "Call network_thread_start() after all P2PInterface objects are created ..."

self.nodes[0].p2p.wait_for_verack()

def reconnect_p2p(self, **kwargs):
"""Tear down and bootstrap the P2P connection to the node.

The node gets disconnected several times in this test. This helper
method reconnects the p2p and restarts the network thread."""
self.nodes[0].disconnect_p2ps()
network_thread_join()
self.bootstrap_p2p(**kwargs)

def run_test(self):
# Add p2p connection to node0
node = self.nodes[0] # convenience reference to the node
node.add_p2p_connection(P2PDataStore())

network_thread_start()
node.p2p.wait_for_verack()
self.bootstrap_p2p() # Add one p2p connection to the node

best_block = self.nodes[0].getbestblockhash()
tip = int(best_block, 16)
Expand All @@ -44,12 +68,73 @@ def run_test(self):

# b'\x64' is OP_NOTIF
# Transaction will be rejected with code 16 (REJECT_INVALID)
# and we get disconnected immediately
self.log.info('Test a transaction that is rejected')
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)')
node.p2p.send_txs_and_test([tx1], node, success=False, expect_disconnect=True)

# Make two p2p connections to provide the node with orphans
# * p2ps[0] will send valid orphan txs (one with low fee)
# * p2ps[1] will send an invalid orphan tx (and is later disconnected for that)
self.reconnect_p2p(num_connections=2)

self.log.info('Test orphan transaction handling ... ')
# Create a root transaction that we withold until all dependend transactions
# are sent out and in the orphan cache
tx_withhold = CTransaction()
tx_withhold.vin.append(CTxIn(outpoint=COutPoint(block1.vtx[0].sha256, 0)))
tx_withhold.vout.append(CTxOut(nValue=50 * COIN - 12000, scriptPubKey=b'\x51'))
tx_withhold.calc_sha256()

# Our first orphan tx with some outputs to create further orphan txs
tx_orphan_1 = CTransaction()
tx_orphan_1.vin.append(CTxIn(outpoint=COutPoint(tx_withhold.sha256, 0)))
tx_orphan_1.vout = [CTxOut(nValue=10 * COIN, scriptPubKey=b'\x51')] * 3
tx_orphan_1.calc_sha256()

# A valid transaction with low fee
tx_orphan_2_no_fee = CTransaction()
tx_orphan_2_no_fee.vin.append(CTxIn(outpoint=COutPoint(tx_orphan_1.sha256, 0)))
tx_orphan_2_no_fee.vout.append(CTxOut(nValue=10 * COIN, scriptPubKey=b'\x51'))

# A valid transaction with sufficient fee
tx_orphan_2_valid = CTransaction()
tx_orphan_2_valid.vin.append(CTxIn(outpoint=COutPoint(tx_orphan_1.sha256, 1)))
tx_orphan_2_valid.vout.append(CTxOut(nValue=10 * COIN - 12000, scriptPubKey=b'\x51'))
tx_orphan_2_valid.calc_sha256()

# An invalid transaction with negative fee
tx_orphan_2_invalid = CTransaction()
tx_orphan_2_invalid.vin.append(CTxIn(outpoint=COutPoint(tx_orphan_1.sha256, 2)))
tx_orphan_2_invalid.vout.append(CTxOut(nValue=11 * COIN, scriptPubKey=b'\x51'))

self.log.info('Send the orphans ... ')
# Send valid orphan txs from p2ps[0]
node.p2p.send_txs_and_test([tx_orphan_1, tx_orphan_2_no_fee, tx_orphan_2_valid], node, success=False)
# Send invalid tx from p2ps[1]
node.p2ps[1].send_txs_and_test([tx_orphan_2_invalid], node, success=False)

assert_equal(0, node.getmempoolinfo()['size']) # Mempool should be empty
assert_equal(2, len(node.getpeerinfo())) # p2ps[1] is still connected

self.log.info('Send the withhold tx ... ')
node.p2p.send_txs_and_test([tx_withhold], node, success=True)

# Transactions that should end up in the mempool
expected_mempool = {
t.hash
for t in [
tx_withhold, # The transaction that is the root for all orphans
tx_orphan_1, # The orphan transaction that splits the coins
tx_orphan_2_valid, # The valid transaction (with sufficient fee)
]
}
# Transactions that do not end up in the mempool
# tx_orphan_no_fee, because it has too low fee (p2ps[0] is not disconnected for relaying that tx)
# tx_orphan_invaid, because it has negative fee (p2ps[1] is disconnected for relaying that tx)

# Verify valid transaction
tx1 = create_transaction(block1.vtx[0], 0, b'', 50 * COIN - 12000)
node.p2p.send_txs_and_test([tx1], node, success=True)
wait_until(lambda: 1 == len(node.getpeerinfo()), timeout=12) # p2ps[1] is no longer connected
assert_equal(expected_mempool, set(node.getrawmempool()))


if __name__ == '__main__':
Expand Down
11 changes: 7 additions & 4 deletions test/functional/test_framework/mininode.py
Expand Up @@ -554,13 +554,13 @@ def send_blocks_and_test(self, blocks, rpc, success=True, request_block=True, re
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):
def send_txs_and_test(self, txs, rpc, success=True, expect_disconnect=False, 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"success is False" line unintentionally deleted?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like including both "if success is ..." lines would be repetitive.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a little repetitive, but I think the description is vague without that line because "not asserting" and "asserting not" mean different things.

- 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."""

with mininode_lock:
Expand All @@ -573,7 +573,10 @@ def send_txs_and_test(self, txs, rpc, success=True, reject_code=None, reject_rea
for tx in txs:
self.send_message(msg_tx(tx))

self.sync_with_ping()
if expect_disconnect:
self.wait_for_disconnect()
else:
self.sync_with_ping()

raw_mempool = rpc.getrawmempool()
if success:
Expand Down