From 750f627a903cfca5166ee88732e1bcaff0140634 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 6 May 2023 21:36:45 -0500 Subject: [PATCH] fix: improved reliability of functional tests that activates dip24 (#5313) ## Issue being fixed or feature implemented Functional tests in CI and locally often fails without a good reason (pretty randomly) ## What was done? It was re-implemented `get_recovered_sig` and updated `create_raw_tx` for better selection/change output. ## How Has This Been Tested? Run functional times with bug amount of parrallel jobs: ``` test/functional/test_runner.py -j 20 feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py ``` Without these changes usually 2-3 instance fails. With these changes all failures happened only for `p2p_addrv2_relay.py` and `mempool_unbroadcast.py`. Beside feature_llmq_is_conflicts.py improved stability of `interface_zmq_dash.py` also. ## Breaking Changes No breaking changes ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone --------- Co-authored-by: UdjinM6 --- .../feature_llmq_is_cl_conflicts.py | 8 +--- test/functional/interface_zmq_dash.py | 3 +- .../test_framework/test_framework.py | 48 ++++++++++--------- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/test/functional/feature_llmq_is_cl_conflicts.py b/test/functional/feature_llmq_is_cl_conflicts.py index dd3deaa902..d9e7af580c 100755 --- a/test/functional/feature_llmq_is_cl_conflicts.py +++ b/test/functional/feature_llmq_is_cl_conflicts.py @@ -349,13 +349,7 @@ class LLMQ_IS_CL_Conflicts(DashTestFramework): request_id = hash256(request_id_buf)[::-1].hex() message_hash = block.hash - quorum_member = None - for mn in self.mninfo: - res = mn.node.quorum('sign', 100, request_id, message_hash) - if res and quorum_member is None: - quorum_member = mn - - recSig = self.get_recovered_sig(request_id, message_hash, node=quorum_member.node) + recSig = self.get_recovered_sig(request_id, message_hash) clsig = msg_clsig(height, block.sha256, hex_str_to_bytes(recSig['sig'])) return clsig diff --git a/test/functional/interface_zmq_dash.py b/test/functional/interface_zmq_dash.py index 6b5848f6fb..aca7323bc2 100755 --- a/test/functional/interface_zmq_dash.py +++ b/test/functional/interface_zmq_dash.py @@ -190,7 +190,8 @@ class DashZMQTest (DashTestFramework): def validate_recovered_sig(request_id, msg_hash): # Make sure the recovered sig exists by RPC - rpc_recovered_sig = self.get_recovered_sig(request_id, msg_hash) + self.wait_for_recovered_sig(request_id, msg_hash) + rpc_recovered_sig = self.mninfo[0].node.quorum('getrecsig', 100, request_id, msg_hash) # Validate hashrecoveredsig zmq_recovered_sig_hash = self.subscribers[ZMQPublisher.hash_recovered_sig].receive().read(32).hex() assert_equal(zmq_recovered_sig_hash, msg_hash) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 8741b59f00..a638f51870 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -46,6 +46,7 @@ from .util import ( get_datadir_path, hex_str_to_bytes, initialize_datadir, + make_change, p2p_port, set_node_times, set_timeout_scale, @@ -1394,6 +1395,7 @@ class DashTestFramework(BitcoinTestFramework): def create_raw_tx(self, node_from, node_to, amount, min_inputs, max_inputs): assert min_inputs <= max_inputs # fill inputs + fee = 0.001 inputs = [] balances = node_from.listunspent() in_amount = 0.0 @@ -1405,7 +1407,7 @@ class DashTestFramework(BitcoinTestFramework): input['vout'] = tx['vout'] in_amount += float(tx['amount']) inputs.append(input) - elif in_amount > amount: + elif in_amount >= amount + fee: break elif len(inputs) < max_inputs: input = {} @@ -1424,14 +1426,11 @@ class DashTestFramework(BitcoinTestFramework): assert len(inputs) >= min_inputs assert len(inputs) <= max_inputs - assert in_amount >= amount + assert in_amount >= amount + fee # fill outputs + outputs = make_change(node_from, satoshi_round(in_amount), satoshi_round(amount), satoshi_round(fee)) receiver_address = node_to.getnewaddress() - change_address = node_from.getnewaddress() - fee = 0.001 - outputs = {} outputs[receiver_address] = satoshi_round(amount) - outputs[change_address] = satoshi_round(in_amount - amount - fee) rawtx = node_from.createrawtransaction(inputs, outputs) ret = node_from.signrawtransactionwithwallet(rawtx) decoded = node_from.decoderawtransaction(ret['hex']) @@ -1461,17 +1460,12 @@ class DashTestFramework(BitcoinTestFramework): message_hash = tx.hash llmq_type = 103 if deterministic else 104 - quorum_member = None - for mn in self.mninfo: - res = mn.node.quorum('sign', llmq_type, request_id, message_hash) - if (res and quorum_member is None): - quorum_member = mn - rec_sig = self.get_recovered_sig(request_id, message_hash, node=quorum_member.node, llmq_type=llmq_type) + rec_sig = self.get_recovered_sig(request_id, message_hash, llmq_type=llmq_type) if deterministic: - block_count = quorum_member.node.getblockcount() - cycle_hash = int(quorum_member.node.getblockhash(block_count - (block_count % 24)), 16) + block_count = self.mninfo[0].node.getblockcount() + cycle_hash = int(self.mninfo[0].node.getblockhash(block_count - (block_count % 24)), 16) islock = msg_isdlock(1, inputs, tx.sha256, cycle_hash, hex_str_to_bytes(rec_sig['sig'])) else: islock = msg_islock(inputs, tx.sha256, hex_str_to_bytes(rec_sig['sig'])) @@ -1903,16 +1897,26 @@ class DashTestFramework(BitcoinTestFramework): time.sleep(1) self.log.info('Moved from block %d to %d' % (cur_block, self.nodes[0].getblockcount())) - def get_recovered_sig(self, rec_sig_id, rec_sig_msg_hash, llmq_type=100, node=None): + def wait_for_recovered_sig(self, rec_sig_id, rec_sig_msg_hash, llmq_type=100, timeout=10): + def check_recovered_sig(): + self.bump_mocktime(1) + for mn in self.mninfo: + if not mn.node.quorum("hasrecsig", llmq_type, rec_sig_id, rec_sig_msg_hash): + return False + return True + wait_until(check_recovered_sig, timeout=timeout, sleep=1) + + def get_recovered_sig(self, rec_sig_id, rec_sig_msg_hash, llmq_type=100): # Note: recsigs aren't relayed to regular nodes by default, # make sure to pick a mn as a node to query for recsigs. - node = self.mninfo[0].node if node is None else node - stop_time = time.time() + 10 * self.options.timeout_scale - while time.time() < stop_time: - try: - return node.quorum('getrecsig', llmq_type, rec_sig_id, rec_sig_msg_hash) - except JSONRPCException: - time.sleep(0.1) + try: + quorumHash = self.mninfo[0].node.quorum("selectquorum", llmq_type, rec_sig_id)["quorumHash"] + for mn in self.mninfo: + mn.node.quorum("sign", llmq_type, rec_sig_id, rec_sig_msg_hash, quorumHash) + self.wait_for_recovered_sig(rec_sig_id, rec_sig_msg_hash, llmq_type, 10) + return self.mninfo[0].node.quorum("getrecsig", llmq_type, rec_sig_id, rec_sig_msg_hash) + except JSONRPCException as e: + self.log.info(f"getrecsig failed with '{e}'") assert False def get_quorum_masternodes(self, q, llmq_type=100):