Skip to content

test: move sync_blocks and sync_mempool functions to test_framework.py #19208

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

Merged
merged 1 commit into from
Jun 21, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions test/functional/feature_backwards_compatibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
from test_framework.util import (
adjust_bitcoin_conf_for_pre_17,
assert_equal,
sync_blocks,
sync_mempools,
)


Expand Down Expand Up @@ -68,7 +66,7 @@ def setup_nodes(self):
def run_test(self):
self.nodes[0].generatetoaddress(101, self.nodes[0].getnewaddress())

sync_blocks(self.nodes)
self.sync_blocks()

# Sanity check the test framework:
res = self.nodes[self.num_nodes - 1].getblockchaininfo()
Expand All @@ -93,17 +91,17 @@ def run_test(self):
# Create a confirmed transaction, receiving coins
address = wallet.getnewaddress()
self.nodes[0].sendtoaddress(address, 10)
sync_mempools(self.nodes)
self.sync_mempools()
self.nodes[0].generate(1)
sync_blocks(self.nodes)
self.sync_blocks()
# Create a conflicting transaction using RBF
return_address = self.nodes[0].getnewaddress()
tx1_id = self.nodes[1].sendtoaddress(return_address, 1)
tx2_id = self.nodes[1].bumpfee(tx1_id)["txid"]
# Confirm the transaction
sync_mempools(self.nodes)
self.sync_mempools()
self.nodes[0].generate(1)
sync_blocks(self.nodes)
self.sync_blocks()
# Create another conflicting transaction using RBF
tx3_id = self.nodes[1].sendtoaddress(return_address, 1)
tx4_id = self.nodes[1].bumpfee(tx3_id)["txid"]
Expand Down
4 changes: 2 additions & 2 deletions test/functional/rpc_getblockfilter.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal, assert_is_hex_string, assert_raises_rpc_error,
connect_nodes, disconnect_nodes, sync_blocks
connect_nodes, disconnect_nodes
)

FILTER_TYPES = ["basic"]
Expand All @@ -30,7 +30,7 @@ def run_test(self):

# Reorg node 0 to a new chain
connect_nodes(self.nodes[0], 1)
sync_blocks(self.nodes)
self.sync_blocks()

assert_equal(self.nodes[0].getblockcount(), 4)
chain1_hashes = [self.nodes[0].getblockhash(block_height) for block_height in range(4)]
Expand Down
59 changes: 48 additions & 11 deletions test/functional/test_framework/test_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
disconnect_nodes,
get_datadir_path,
initialize_datadir,
sync_blocks,
sync_mempools,
)


Expand Down Expand Up @@ -541,15 +539,54 @@ def join_network(self):
connect_nodes(self.nodes[1], 2)
self.sync_all()

def sync_blocks(self, nodes=None, **kwargs):
sync_blocks(nodes or self.nodes, **kwargs)

def sync_mempools(self, nodes=None, **kwargs):
sync_mempools(nodes or self.nodes, **kwargs)

def sync_all(self, nodes=None, **kwargs):
self.sync_blocks(nodes, **kwargs)
self.sync_mempools(nodes, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

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

why is this changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed **kwargs which are not used.

Copy link
Member

Choose a reason for hiding this comment

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

I think they might come in handy in the future. Is there a reason they will be unused forever?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel it's better to add it when it will be used in instead of putting it there up front just because some time in the future it might be used. What do you think?

def sync_blocks(self, nodes=None, wait=1, timeout=60):
"""
Wait until everybody has the same tip.
sync_blocks needs to be called with an rpc_connections set that has least
Copy link
Member

Choose a reason for hiding this comment

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

I have a question - it's probably not a big deal to require at least one node to be updated, but would it be appropriate to have an optional blockhash parameter and enforce that hash instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see a use case like that in current code. But I think one benefit your idea will give is to shorten test run time if the block we need to sync to is not usually the tip.

one node already synced to the latest, stable tip, otherwise there's a
chance it might return before all nodes are stably synced.
"""
rpc_connections = nodes or self.nodes
timeout = int(timeout * self.options.timeout_factor)
stop_time = time.time() + timeout
while time.time() <= stop_time:
best_hash = [x.getbestblockhash() for x in rpc_connections]
if best_hash.count(best_hash[0]) == len(rpc_connections):
return
# Check that each peer has at least one connection
assert (all([len(x.getpeerinfo()) for x in rpc_connections]))
time.sleep(wait)
raise AssertionError("Block sync timed out after {}s:{}".format(
timeout,
"".join("\n {!r}".format(b) for b in best_hash),
))

def sync_mempools(self, nodes=None, wait=1, timeout=60, flush_scheduler=True):
"""
Wait until everybody has the same transactions in their memory
pools
"""
rpc_connections = nodes or self.nodes
timeout = int(timeout * self.options.timeout_factor)
stop_time = time.time() + timeout
while time.time() <= stop_time:
pool = [set(r.getrawmempool()) for r in rpc_connections]
if pool.count(pool[0]) == len(rpc_connections):
if flush_scheduler:
for r in rpc_connections:
r.syncwithvalidationinterfacequeue()
return
# Check that each peer has at least one connection
assert (all([len(x.getpeerinfo()) for x in rpc_connections]))
time.sleep(wait)
raise AssertionError("Mempool sync timed out after {}s:{}".format(
timeout,
"".join("\n {!r}".format(m) for m in pool),
))

def sync_all(self, nodes=None):
self.sync_blocks(nodes)
self.sync_mempools(nodes)

# Private helper methods. These should not be accessed by the subclass test scripts.

Expand Down
44 changes: 0 additions & 44 deletions test/functional/test_framework/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,50 +420,6 @@ def connect_nodes(from_connection, node_num):
wait_until(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo()))


def sync_blocks(rpc_connections, *, wait=1, timeout=60):
"""
Wait until everybody has the same tip.

sync_blocks needs to be called with an rpc_connections set that has least
one node already synced to the latest, stable tip, otherwise there's a
chance it might return before all nodes are stably synced.
"""
stop_time = time.time() + timeout
while time.time() <= stop_time:
best_hash = [x.getbestblockhash() for x in rpc_connections]
if best_hash.count(best_hash[0]) == len(rpc_connections):
return
# Check that each peer has at least one connection
assert (all([len(x.getpeerinfo()) for x in rpc_connections]))
time.sleep(wait)
raise AssertionError("Block sync timed out after {}s:{}".format(
timeout,
"".join("\n {!r}".format(b) for b in best_hash),
))


def sync_mempools(rpc_connections, *, wait=1, timeout=60, flush_scheduler=True):
"""
Wait until everybody has the same transactions in their memory
pools
"""
stop_time = time.time() + timeout
while time.time() <= stop_time:
pool = [set(r.getrawmempool()) for r in rpc_connections]
if pool.count(pool[0]) == len(rpc_connections):
if flush_scheduler:
for r in rpc_connections:
r.syncwithvalidationinterfacequeue()
return
# Check that each peer has at least one connection
assert (all([len(x.getpeerinfo()) for x in rpc_connections]))
time.sleep(wait)
raise AssertionError("Mempool sync timed out after {}s:{}".format(
timeout,
"".join("\n {!r}".format(m) for m in pool),
))


# Transaction/Block functions
#############################

Expand Down
3 changes: 1 addition & 2 deletions test/functional/wallet_balance.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
assert_equal,
assert_raises_rpc_error,
connect_nodes,
sync_blocks,
)


Expand Down Expand Up @@ -264,7 +263,7 @@ def test_balances(*, fee_node_1=0):
# Now confirm tx_orig
self.restart_node(1, ['-persistmempool=0'])
connect_nodes(self.nodes[0], 1)
sync_blocks(self.nodes)
self.sync_blocks()
self.nodes[1].sendrawtransaction(tx_orig)
self.nodes[1].generatetoaddress(1, ADDRESS_WATCHONLY)
self.sync_all()
Expand Down