From dbce9c90c65bb22b6ce7b0bd6ddf590532b2e68d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Jun 2025 15:58:02 +0000 Subject: [PATCH 1/8] test: add helper for `register{,_evo,_legacy}` calls We store a fair amount of information about the masternode already in MasternodeInfo, so we should try and leverage that as best we can and require the user only input the values we don't store and they need to override. --- .../feature_dip3_deterministicmns.py | 2 +- .../test_framework/test_framework.py | 56 +++++++++++++++---- 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py index 5e012624ea32..733b0e57a2d1 100755 --- a/test/functional/feature_dip3_deterministicmns.py +++ b/test/functional/feature_dip3_deterministicmns.py @@ -244,7 +244,7 @@ def register_fund_mn(self, node, mn: MasternodeInfo): # create a protx MN which refers to an existing collateral def register_mn(self, node, mn: MasternodeInfo): node.sendtoaddress(mn.fundsAddr, 0.001) - proTxHash = node.protx('register' if softfork_active(node, 'v19') else 'register_legacy', mn.collateral_txid, mn.collateral_vout, '127.0.0.1:%d' % mn.nodePort, mn.ownerAddr, mn.pubKeyOperator, mn.votingAddr, mn.operator_reward, mn.rewards_address, mn.fundsAddr) + proTxHash = mn.register(node, True) mn.set_params(proTxHash=proTxHash) self.generate(node, 1, sync_fun=self.no_op) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index f547ee084edb..09691636e26d 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1186,6 +1186,8 @@ def get_collateral_vout(self, node: TestNode, txid: str, should_throw: bool = Tr return -1 def __init__(self, evo: bool, legacy: bool): + if evo and legacy: + raise AssertionError("EvoNodes are not allowed to use legacy scheme") self.evo = evo self.legacy = legacy @@ -1204,6 +1206,7 @@ def set_params(self, proTxHash: Optional[str] = None, operator_reward: Optional[ def set_node(self, nodeIdx: int, friendlyName: Optional[str] = None): self.nodeIdx = nodeIdx + self.nodePort = p2p_port(nodeIdx) self.friendlyName = friendlyName or f"mn-{'evo' if self.evo else 'reg'}-{self.nodeIdx}" def get_node(self, test: BitcoinTestFramework) -> TestNode: @@ -1213,6 +1216,42 @@ def get_node(self, test: BitcoinTestFramework) -> TestNode: raise AssertionError(f"Node at pos {self.nodeIdx} not present, did you start the node?") return test.nodes[self.nodeIdx] + def register(self, node: TestNode, submit: bool = True, collateral_txid: Optional[str] = None, collateral_vout: Optional[int] = None, + ipAndPort: Optional[str] = None, ownerAddr: Optional[str] = None, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, + operator_reward: Optional[int] = None, rewards_address: Optional[str] = None, fundsAddr: Optional[str] = None, + platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, platform_http_port: Optional[int] = None) -> str: + # EvoNode-specific fields are ignored for regular masternodes + if self.evo: + if platform_node_id is None: + raise AssertionError("EvoNode but platform_node_id is missing, must be specified!") + if platform_p2p_port is None: + raise AssertionError("EvoNode but platform_p2p_port is missing, must be specified!") + if platform_http_port is None: + raise AssertionError("EvoNode but platform_http_port is missing, must be specified!") + + # Common arguments shared between regular masternodes and EvoNodes + args = [ + collateral_txid or self.collateral_txid, + collateral_vout or self.collateral_vout, + ipAndPort or f'127.0.0.1:{self.nodePort}', + ownerAddr or self.ownerAddr, + pubKeyOperator or self.pubKeyOperator, + votingAddr or self.votingAddr, + operator_reward or self.operator_reward, + rewards_address or self.rewards_address, + ] + address_funds = fundsAddr or self.fundsAddr + + # Construct final command and arguments + if self.evo: + command = "register_evo" + args = args + [platform_node_id, platform_p2p_port, platform_http_port, address_funds, submit] # type: ignore + else: + command = "register_legacy" if self.legacy else "register" + args = args + [address_funds, submit] # type: ignore + + return node.protx(command, *args) + class DashTestFramework(BitcoinTestFramework): def set_test_params(self): """Tests must this method to change default values for number of nodes, topology, etc""" @@ -1381,8 +1420,8 @@ def dynamically_prepare_masternode(self, idx, node_p2p_port, evo=False, rnd=None mn.generate_addresses(self.nodes[0]) platform_node_id = hash160(b'%d' % rnd).hex() if rnd is not None else hash160(b'%d' % node_p2p_port).hex() - platform_p2p_port = '%d' % (node_p2p_port + 101) - platform_http_port = '%d' % (node_p2p_port + 102) + platform_p2p_port = node_p2p_port + 101 + platform_http_port = node_p2p_port + 102 outputs = {mn.collateral_address: mn.get_collateral_value(), mn.fundsAddr: 1} collateral_txid = self.nodes[0].sendmany("", outputs) @@ -1393,11 +1432,9 @@ def dynamically_prepare_masternode(self, idx, node_p2p_port, evo=False, rnd=None ipAndPort = '127.0.0.1:%d' % node_p2p_port operatorReward = idx - protx_result = None - if evo: - protx_result = self.nodes[0].protx("register_evo", collateral_txid, collateral_vout, ipAndPort, mn.ownerAddr, mn.pubKeyOperator, mn.votingAddr, operatorReward, mn.rewards_address, platform_node_id, platform_p2p_port, platform_http_port, mn.fundsAddr, True) - else: - protx_result = self.nodes[0].protx("register_legacy" if mn.legacy else "register", collateral_txid, collateral_vout, ipAndPort, mn.ownerAddr, mn.pubKeyOperator, mn.votingAddr, operatorReward, mn.rewards_address, mn.fundsAddr, True) + # platform_node_id, platform_p2p_port and platform_http_port are ignored for regular masternodes + protx_result = mn.register(self.nodes[0], True, collateral_txid=collateral_txid, collateral_vout=collateral_vout, ipAndPort=ipAndPort, operator_reward=operatorReward, + platform_node_id=platform_node_id, platform_p2p_port=platform_p2p_port, platform_http_port=platform_http_port) self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block mn.bury_tx(self, genIdx=0, txid=protx_result, depth=1) @@ -1466,9 +1503,8 @@ def prepare_masternode(self, idx): mn.ownerAddr, mn.pubKeyOperator, mn.votingAddr, operatorReward, mn.rewards_address, mn.fundsAddr, submit) else: self.generate(self.nodes[0], 1, sync_fun=self.no_op) - protx_result = self.nodes[0].protx('register_legacy' if mn.legacy else 'register', txid, collateral_vout, ipAndPort, - mn.ownerAddr, mn.pubKeyOperator, mn.votingAddr, operatorReward, mn.rewards_address, mn.fundsAddr, submit) - + protx_result = mn.register(self.nodes[0], submit, collateral_txid=txid, collateral_vout=collateral_vout, ipAndPort=ipAndPort, + operator_reward=operatorReward) if submit: proTxHash = protx_result else: From aa96ace89af0d9e26308b22688ed8721d9dfb6f8 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Jun 2025 15:56:57 +0000 Subject: [PATCH 2/8] test: add helper for `register_fund{,_evo,_legacy}` calls --- .../feature_dip3_deterministicmns.py | 2 +- .../test_framework/test_framework.py | 38 ++++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py index 733b0e57a2d1..6f7c06cb59b5 100755 --- a/test/functional/feature_dip3_deterministicmns.py +++ b/test/functional/feature_dip3_deterministicmns.py @@ -237,7 +237,7 @@ def create_mn_collateral(self, node, mn: MasternodeInfo): # register a protx MN and also fund it (using collateral inside ProRegTx) def register_fund_mn(self, node, mn: MasternodeInfo): node.sendtoaddress(mn.fundsAddr, mn.get_collateral_value() + 0.001) - txid = node.protx('register_fund' if softfork_active(node, 'v19') else 'register_fund_legacy', mn.collateral_address, '127.0.0.1:%d' % mn.nodePort, mn.ownerAddr, mn.pubKeyOperator, mn.votingAddr, mn.operator_reward, mn.rewards_address, mn.fundsAddr) + txid = mn.register_fund(node, True) vout = mn.get_collateral_vout(node, txid) mn.set_params(proTxHash=txid, collateral_txid=txid, collateral_vout=vout) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 09691636e26d..23cef04b0ac2 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1252,6 +1252,41 @@ def register(self, node: TestNode, submit: bool = True, collateral_txid: Optiona return node.protx(command, *args) + def register_fund(self, node: TestNode, submit: bool = True, collateral_address: Optional[str] = None, ipAndPort: Optional[str] = None, + ownerAddr: Optional[str] = None, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, + operator_reward: Optional[int] = None, rewards_address: Optional[str] = None, fundsAddr: Optional[str] = None, + platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, platform_http_port: Optional[int] = None) -> str: + # EvoNode-specific fields are ignored for regular masternodes + if self.evo: + if platform_node_id is None: + raise AssertionError("EvoNode but platform_node_id is missing, must be specified!") + if platform_p2p_port is None: + raise AssertionError("EvoNode but platform_p2p_port is missing, must be specified!") + if platform_http_port is None: + raise AssertionError("EvoNode but platform_http_port is missing, must be specified!") + + # Common arguments shared between regular masternodes and EvoNodes + args = [ + collateral_address or self.collateral_address, + ipAndPort or f'127.0.0.1:{self.nodePort}', + ownerAddr or self.ownerAddr, + pubKeyOperator or self.pubKeyOperator, + votingAddr or self.votingAddr, + operator_reward or self.operator_reward, + rewards_address or self.rewards_address, + ] + address_funds = fundsAddr or self.fundsAddr + + # Construct final command and arguments + if self.evo: + command = "register_fund_evo" + args = args + [platform_node_id, platform_p2p_port, platform_http_port, address_funds, submit] # type: ignore + else: + command = "register_fund_legacy" if self.legacy else "register_fund" + args = args + [address_funds, submit] # type: ignore + + return node.protx(command, *args) + class DashTestFramework(BitcoinTestFramework): def set_test_params(self): """Tests must this method to change default values for number of nodes, topology, etc""" @@ -1499,8 +1534,7 @@ def prepare_masternode(self, idx): submit = (idx % 4) < 2 if register_fund: - protx_result = self.nodes[0].protx('register_fund_legacy' if mn.legacy else 'register_fund', mn.collateral_address, ipAndPort, - mn.ownerAddr, mn.pubKeyOperator, mn.votingAddr, operatorReward, mn.rewards_address, mn.fundsAddr, submit) + protx_result = mn.register_fund(self.nodes[0], submit, ipAndPort=ipAndPort, operator_reward=operatorReward) else: self.generate(self.nodes[0], 1, sync_fun=self.no_op) protx_result = mn.register(self.nodes[0], submit, collateral_txid=txid, collateral_vout=collateral_vout, ipAndPort=ipAndPort, From 6ba685f53fd22399a2f6c36602125aaa249b11c4 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 11 Jun 2025 14:23:36 +0000 Subject: [PATCH 3/8] test: add helper for `revoke` calls --- test/functional/feature_dip3_v19.py | 14 ++++++-------- .../test_framework/test_framework.py | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/test/functional/feature_dip3_v19.py b/test/functional/feature_dip3_v19.py index 9ba756874041..5860063ddef3 100755 --- a/test/functional/feature_dip3_v19.py +++ b/test/functional/feature_dip3_v19.py @@ -94,10 +94,8 @@ def run_test(self): assert evo_info_3 is not None self.dynamically_evo_update_service(evo_info_0, 9, should_be_rejected=True) - revoke_protx = self.mninfo[-1].proTxHash - revoke_keyoperator = self.mninfo[-1].keyOperator - self.log.info(f"Trying to revoke proTx:{revoke_protx}") - self.test_revoke_protx(evo_info_3.nodeIdx, revoke_protx, revoke_keyoperator) + self.log.info(f"Trying to revoke proTx:{self.mninfo[-1].proTxHash}") + self.test_revoke_protx(evo_info_3.nodeIdx, self.mninfo[-1]) self.mine_quorum(llmq_type_name='llmq_test', llmq_type=100) @@ -115,14 +113,14 @@ def run_test(self): self.wait_for_chainlocked_block_all_nodes(self.nodes[0].getbestblockhash()) - def test_revoke_protx(self, node_idx, revoke_protx, revoke_keyoperator): + def test_revoke_protx(self, node_idx, revoke_mn: MasternodeInfo): funds_address = self.nodes[0].getnewaddress() fund_txid = self.nodes[0].sendtoaddress(funds_address, 1) self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block tip = self.generate(self.nodes[0], 1)[0] assert_equal(self.nodes[0].getrawtransaction(fund_txid, 1, tip)['confirmations'], 1) - protx_result = self.nodes[0].protx('revoke', revoke_protx, revoke_keyoperator, 1, funds_address) + protx_result = revoke_mn.revoke(self.nodes[0], reason=1, fundsAddr=funds_address) self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block tip = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0] assert_equal(self.nodes[0].getrawtransaction(protx_result, 1, tip)['confirmations'], 1) @@ -131,9 +129,9 @@ def test_revoke_protx(self, node_idx, revoke_protx, revoke_keyoperator): self.wait_until(lambda: self.nodes[node_idx].getconnectioncount() == 0) self.connect_nodes(node_idx, 0) self.sync_all() - self.log.info(f"Successfully revoked={revoke_protx}") + self.log.info(f"Successfully revoked={revoke_mn.proTxHash}") for mn in self.mninfo: # type: MasternodeInfo - if mn.proTxHash == revoke_protx: + if mn.proTxHash == revoke_mn.proTxHash: self.mninfo.remove(mn) return diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 23cef04b0ac2..2b23423ebed3 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1287,6 +1287,25 @@ def register_fund(self, node: TestNode, submit: bool = True, collateral_address: return node.protx(command, *args) + def revoke(self, node: TestNode, reason: int, fundsAddr: Optional[str] = None) -> str: + # Update commands should be run from the appropriate MasternodeInfo instance, we do not allow overriding some values for this reason + if self.proTxHash is None: + raise AssertionError("proTxHash not set, did you call set_params()") + if self.keyOperator is None: + raise AssertionError("keyOperator not set, did you call generate_addresses()") + + args = [ + self.proTxHash, + self.keyOperator, + reason, + ] + + # fundsAddr is an optional field that results in different behavior if omitted, so we don't fallback here + if fundsAddr is not None: + args = args + [fundsAddr] + + return node.protx('revoke', *args) + class DashTestFramework(BitcoinTestFramework): def set_test_params(self): """Tests must this method to change default values for number of nodes, topology, etc""" From 1493d67ab99d7a0f836b22b8d909f20ac625c3ce Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 11 Jun 2025 14:23:55 +0000 Subject: [PATCH 4/8] test: add helper for `update_registrar{,_legacy}` calls --- .../feature_dip3_deterministicmns.py | 4 ++-- .../test_framework/test_framework.py | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py index 6f7c06cb59b5..44dc9a8ef7ee 100755 --- a/test/functional/feature_dip3_deterministicmns.py +++ b/test/functional/feature_dip3_deterministicmns.py @@ -213,7 +213,7 @@ def run_test(self): assert old_voting_address != new_voting_address # also check if funds from payout address are used when no fee source address is specified node.sendtoaddress(mn.rewards_address, 0.001) - node.protx('update_registrar' if softfork_active(node, 'v19') else 'update_registrar_legacy', mn.proTxHash, "", new_voting_address, "") + mn.update_registrar(node, pubKeyOperator="", votingAddr=new_voting_address, rewards_address="") self.generate(node, 1) new_dmnState = mn.get_node(self).masternode("status")["dmnState"] new_voting_address_from_rpc = new_dmnState["votingAddress"] @@ -263,7 +263,7 @@ def spend_mn_collateral(self, mn: MasternodeInfo, with_dummy_input_output=False) def update_mn_payee(self, mn: MasternodeInfo, payee): self.nodes[0].sendtoaddress(mn.fundsAddr, 0.001) - self.nodes[0].protx('update_registrar' if softfork_active(self.nodes[0], 'v19') else 'update_registrar_legacy', mn.proTxHash, '', '', payee, mn.fundsAddr) + mn.update_registrar(self.nodes[0], pubKeyOperator="", votingAddr="", rewards_address=payee, fundsAddr=mn.fundsAddr) self.generate(self.nodes[0], 1) info = self.nodes[0].protx('info', mn.proTxHash) assert info['state']['payoutAddress'] == payee diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 2b23423ebed3..fa82b07b548a 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1306,6 +1306,26 @@ def revoke(self, node: TestNode, reason: int, fundsAddr: Optional[str] = None) - return node.protx('revoke', *args) + def update_registrar(self, node: TestNode, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, + rewards_address: Optional[str] = None, fundsAddr: Optional[str] = None) -> str: + # Update commands should be run from the appropriate MasternodeInfo instance, we do not allow overriding proTxHash for this reason + if self.proTxHash is None: + raise AssertionError("proTxHash not set, did you call set_params()") + + command = 'update_registrar_legacy' if self.legacy else 'update_registrar' + args = [ + self.proTxHash, + pubKeyOperator or self.pubKeyOperator, + votingAddr or self.votingAddr, + rewards_address or self.rewards_address, + ] + + # fundsAddr is an optional field that results in different behavior if omitted, so we don't fallback here + if fundsAddr is not None: + args = args + [fundsAddr] + + return node.protx(command, *args) + class DashTestFramework(BitcoinTestFramework): def set_test_params(self): """Tests must this method to change default values for number of nodes, topology, etc""" From 968698b626238defdc0b9aed7acb0e60d6f1524b Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Jun 2025 15:59:44 +0000 Subject: [PATCH 5/8] test: add helper for `update_service{,_evo}` calls --- .../feature_dip3_deterministicmns.py | 4 +- test/functional/feature_llmq_simplepose.py | 2 +- .../test_framework/test_framework.py | 47 +++++++++++++++++-- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py index 44dc9a8ef7ee..13756db49462 100755 --- a/test/functional/feature_dip3_deterministicmns.py +++ b/test/functional/feature_dip3_deterministicmns.py @@ -270,7 +270,7 @@ def update_mn_payee(self, mn: MasternodeInfo, payee): def test_protx_update_service(self, mn: MasternodeInfo): self.nodes[0].sendtoaddress(mn.fundsAddr, 0.001) - self.nodes[0].protx('update_service', mn.proTxHash, '127.0.0.2:%d' % mn.nodePort, mn.keyOperator, "", mn.fundsAddr) + mn.update_service(self.nodes[0], ipAndPort=f'127.0.0.2:{mn.nodePort}') self.generate(self.nodes[0], 1) for node in self.nodes: protx_info = node.protx('info', mn.proTxHash) @@ -279,7 +279,7 @@ def test_protx_update_service(self, mn: MasternodeInfo): assert_equal(mn_list['%s-%d' % (mn.collateral_txid, mn.collateral_vout)]['address'], '127.0.0.2:%d' % mn.nodePort) # undo - self.nodes[0].protx('update_service', mn.proTxHash, '127.0.0.1:%d' % mn.nodePort, mn.keyOperator, "", mn.fundsAddr) + mn.update_service(self.nodes[0]) self.generate(self.nodes[0], 1, sync_fun=self.no_op) def assert_mnlists(self, mns): diff --git a/test/functional/feature_llmq_simplepose.py b/test/functional/feature_llmq_simplepose.py index 12d92c063c41..191f092b1a63 100755 --- a/test/functional/feature_llmq_simplepose.py +++ b/test/functional/feature_llmq_simplepose.py @@ -208,7 +208,7 @@ def repair_masternodes(self, restart): if check_banned(self.nodes[0], mn) or check_punished(self.nodes[0], mn): addr = self.nodes[0].getnewaddress() self.nodes[0].sendtoaddress(addr, 0.1) - self.nodes[0].protx('update_service', mn.proTxHash, f'127.0.0.1:{mn.nodePort}', mn.keyOperator, "", addr) + mn.update_service(self.nodes[0], fundsAddr=addr) if restart: self.stop_node(mn.nodeIdx) self.start_masternode(mn) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index fa82b07b548a..ab868af27c47 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1326,6 +1326,42 @@ def update_registrar(self, node: TestNode, pubKeyOperator: Optional[str] = None, return node.protx(command, *args) + def update_service(self, node: TestNode, ipAndPort: Optional[str] = None, platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, + platform_http_port: Optional[int] = None, address_operator: Optional[str] = None, fundsAddr: Optional[str] = None) -> str: + # Update commands should be run from the appropriate MasternodeInfo instance, we do not allow overriding some values for this reason + if self.proTxHash is None: + raise AssertionError("proTxHash not set, did you call set_params()") + if self.keyOperator is None: + raise AssertionError("keyOperator not set, did you call generate_addresses()") + + # EvoNode-specific fields are ignored for regular masternodes + if self.evo: + if platform_node_id is None: + raise AssertionError("EvoNode but platform_node_id is missing, must be specified!") + if platform_p2p_port is None: + raise AssertionError("EvoNode but platform_p2p_port is missing, must be specified!") + if platform_http_port is None: + raise AssertionError("EvoNode but platform_http_port is missing, must be specified!") + + # Common arguments shared between regular masternodes and EvoNodes + args = [ + self.proTxHash, + ipAndPort or f'127.0.0.1:{self.nodePort}', + self.keyOperator, + ] + address_funds = fundsAddr or self.fundsAddr + address_operator = address_operator or "" + + # Construct final command and arguments + if self.evo: + command = "update_service_evo" + args = args + [platform_node_id, platform_p2p_port, platform_http_port, address_operator, address_funds] # type: ignore + else: + command = "update_service" + args = args + [address_operator, address_funds] # type: ignore + + return node.protx(command, *args) + class DashTestFramework(BitcoinTestFramework): def set_test_params(self): """Tests must this method to change default values for number of nodes, topology, etc""" @@ -1527,8 +1563,8 @@ def dynamically_evo_update_service(self, evo_info: MasternodeInfo, rnd=None, sho # For the sake of the test, generate random nodeid, p2p and http platform values r = rnd if rnd is not None else random.randint(21000, 65000) platform_node_id = hash160(b'%d' % r).hex() - platform_p2p_port = '%d' % (r + 1) - platform_http_port = '%d' % (r + 2) + platform_p2p_port = r + 1 + platform_http_port = r + 2 fund_txid = self.nodes[0].sendtoaddress(funds_address, 1) self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block @@ -1536,7 +1572,7 @@ def dynamically_evo_update_service(self, evo_info: MasternodeInfo, rnd=None, sho protx_success = False try: - protx_result = self.nodes[0].protx('update_service_evo', evo_info.proTxHash, f'127.0.0.1:{evo_info.nodePort}', evo_info.keyOperator, platform_node_id, platform_p2p_port, platform_http_port, operator_reward_address, funds_address) + protx_result = evo_info.update_service(self.nodes[0], f'127.0.0.1:{evo_info.nodePort}', platform_node_id, platform_p2p_port, platform_http_port, operator_reward_address, funds_address) self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block evo_info.bury_tx(self, genIdx=0, txid=protx_result, depth=1) self.log.info("Updated EvoNode %s: platformNodeID=%s, platformP2PPort=%s, platformHTTPPort=%s" % (evo_info.proTxHash, platform_node_id, platform_p2p_port, platform_http_port)) @@ -1583,12 +1619,13 @@ def prepare_masternode(self, idx): else: proTxHash = self.nodes[0].sendrawtransaction(protx_result) + mn.set_params(proTxHash=proTxHash, operator_reward=operatorReward, collateral_txid=txid, collateral_vout=collateral_vout, nodePort=port) + if operatorReward > 0: self.generate(self.nodes[0], 1, sync_fun=self.no_op) operatorPayoutAddress = self.nodes[0].getnewaddress() - self.nodes[0].protx('update_service', proTxHash, ipAndPort, mn.keyOperator, operatorPayoutAddress, mn.fundsAddr) + mn.update_service(self.nodes[0], ipAndPort=ipAndPort, address_operator=operatorPayoutAddress) - mn.set_params(proTxHash=proTxHash, operator_reward=operatorReward, collateral_txid=txid, collateral_vout=collateral_vout, nodePort=port) self.mninfo.append(mn) self.log.info("Prepared MN %d: collateral_txid=%s, collateral_vout=%d, protxHash=%s" % (idx, txid, collateral_vout, proTxHash)) From 43318a45928e6ee3823e4718a4b79944fb816ff8 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 11 Jun 2025 15:54:44 +0000 Subject: [PATCH 6/8] rpc: extend the ability to withhold submission to all ProTxes *Not* submitting a transaction gives test the ability to check if a call will result in a *valid* transaction, it also lets us examine the raw transaction, manipulate it and check if transaction checks at broadcast time will catch it. This is relevant for extended addresses (specifically, ProUpServTx) but has been extended to all ProTx-creating RPCs for consistency's sake. --- src/rpc/evo.cpp | 66 ++++++++++++++----- .../feature_dip3_deterministicmns.py | 12 ++-- test/functional/feature_dip3_v19.py | 2 +- test/functional/feature_llmq_simplepose.py | 2 +- .../test_framework/test_framework.py | 32 +++++---- 5 files changed, 75 insertions(+), 39 deletions(-) diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 50befd1f9670..06a0ea34227d 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -320,7 +320,7 @@ static void SignSpecialTxPayloadByHash(const CMutableTransaction& tx, SpecialTxP payload.sig = key.Sign(hash, use_legacy); } -static std::string SignAndSendSpecialTx(const JSONRPCRequest& request, CChainstateHelper& chain_helper, const ChainstateManager& chainman, const CMutableTransaction& tx, bool fSubmit = true) +static std::string SignAndSendSpecialTx(const JSONRPCRequest& request, CChainstateHelper& chain_helper, const ChainstateManager& chainman, const CMutableTransaction& tx, bool fSubmit) { { LOCK(cs_main); @@ -539,9 +539,9 @@ static RPCHelpMan protx_register_fund_evo() }, { RPCResult{"if \"submit\" is not set or set to true", - RPCResult::Type::STR_HEX, "txid", "The transaction id"}, + RPCResult::Type::STR_HEX, "txid", "The transaction id"}, RPCResult{"if \"submit\" is set to false", - RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"}, + RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"}, }, RPCExamples{ HelpExampleCli("protx", "register_fund_evo \"" + EXAMPLE_ADDRESS[0] + "\" \"1.2.3.4:1234\" \"" + EXAMPLE_ADDRESS[1] + "\" \"93746e8731c57f87f79b3620a7982924e2931717d49540a85864bd543de11c43fb868fd63e501a1db37e19ed59ae6db4\" \"" + EXAMPLE_ADDRESS[1] + "\" 0 \"" + EXAMPLE_ADDRESS[0] + "\" \"f2dbd9b0a1f541a7c44d34a58674d0262f5feca5\" 22821 22822")}, @@ -578,9 +578,9 @@ static RPCHelpMan protx_register_evo() }, { RPCResult{"if \"submit\" is not set or set to true", - RPCResult::Type::STR_HEX, "txid", "The transaction id"}, + RPCResult::Type::STR_HEX, "txid", "The transaction id"}, RPCResult{"if \"submit\" is set to false", - RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"}, + RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"}, }, RPCExamples{ HelpExampleCli("protx", "register_evo \"0123456701234567012345670123456701234567012345670123456701234567\" 0 \"1.2.3.4:1234\" \"" + EXAMPLE_ADDRESS[1] + "\" \"93746e8731c57f87f79b3620a7982924e2931717d49540a85864bd543de11c43fb868fd63e501a1db37e19ed59ae6db4\" \"" + EXAMPLE_ADDRESS[1] + "\" 0 \"" + EXAMPLE_ADDRESS[0] + "\" \"f2dbd9b0a1f541a7c44d34a58674d0262f5feca5\" 22821 22822")}, @@ -885,7 +885,7 @@ static RPCHelpMan protx_register_submit() ptx.vchSig = opt_vchSig.value(); SetTxPayload(tx, ptx); - return SignAndSendSpecialTx(request, chain_helper, chainman, tx); + return SignAndSendSpecialTx(request, chain_helper, chainman, tx, /*fSubmit=*/true); }, }; } @@ -903,9 +903,13 @@ static RPCHelpMan protx_update_service() GetRpcArg("operatorKey"), GetRpcArg("operatorPayoutAddress"), GetRpcArg("feeSourceAddress"), + GetRpcArg("submit"), }, - RPCResult{ - RPCResult::Type::STR_HEX, "txid", "The transaction id" + { + RPCResult{"if \"submit\" is not set or set to true", + RPCResult::Type::STR_HEX, "txid", "The transaction id"}, + RPCResult{"if \"submit\" is set to false", + RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"}, }, RPCExamples{ HelpExampleCli("protx", "update_service \"0123456701234567012345670123456701234567012345670123456701234567\" \"1.2.3.4:1234\" 5a2e15982e62f1e0b7cf9783c64cf7e3af3f90a52d6c40f6f95d624c0b1621cd") @@ -935,9 +939,14 @@ static RPCHelpMan protx_update_service_evo() GetRpcArg("platformHTTPPort"), GetRpcArg("operatorPayoutAddress"), GetRpcArg("feeSourceAddress"), + GetRpcArg("submit"), + }, + { + RPCResult{"if \"submit\" is not set or set to true", + RPCResult::Type::STR_HEX, "txid", "The transaction id"}, + RPCResult{"if \"submit\" is set to false", + RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"}, }, - RPCResult{ - RPCResult::Type::STR_HEX, "txid", "The transaction id"}, RPCExamples{ HelpExampleCli("protx", "update_service_evo \"0123456701234567012345670123456701234567012345670123456701234567\" \"1.2.3.4:1234\" \"5a2e15982e62f1e0b7cf9783c64cf7e3af3f90a52d6c40f6f95d624c0b1621cd\" \"f2dbd9b0a1f541a7c44d34a58674d0262f5feca5\" 22821 22822")}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue @@ -1043,6 +1052,11 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques } } + bool fSubmit{true}; + if (!request.params[paramIdx + 2].isNull()) { + fSubmit = ParseBoolV(request.params[paramIdx + 2], "submit"); + } + FundSpecialTx(*wallet, tx, ptx, feeSource); const bool isV19active = DeploymentActiveAfter(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip();), @@ -1050,7 +1064,7 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques SignSpecialTxPayloadByHash(tx, ptx, keyOperator, !isV19active); SetTxPayload(tx, ptx); - return SignAndSendSpecialTx(request, chain_helper, chainman, tx); + return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit); } static RPCHelpMan protx_update_registrar_wrapper(const bool specific_legacy_bls_scheme) @@ -1070,9 +1084,13 @@ static RPCHelpMan protx_update_registrar_wrapper(const bool specific_legacy_bls_ GetRpcArg("votingAddress_update"), GetRpcArg("payoutAddress_update"), GetRpcArg("feeSourceAddress"), + GetRpcArg("submit"), }, - RPCResult{ - RPCResult::Type::STR_HEX, "txid", "The transaction id" + { + RPCResult{"if \"submit\" is not set or set to true", + RPCResult::Type::STR_HEX, "txid", "The transaction id"}, + RPCResult{"if \"submit\" is set to false", + RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"}, }, RPCExamples{ HelpExampleCli("protx", rpc_example) @@ -1149,11 +1167,16 @@ static RPCHelpMan protx_update_registrar_wrapper(const bool specific_legacy_bls_ throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Dash address: ") + request.params[4].get_str()); } + bool fSubmit{true}; + if (!request.params[5].isNull()) { + fSubmit = ParseBoolV(request.params[5], "submit"); + } + FundSpecialTx(*wallet, tx, ptx, feeSourceDest); SignSpecialTxPayloadByHash(tx, ptx, dmn->pdmnState->keyIDOwner, *wallet); SetTxPayload(tx, ptx); - return SignAndSendSpecialTx(request, chain_helper, chainman, tx); + return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit); }, }; } @@ -1181,9 +1204,13 @@ static RPCHelpMan protx_revoke() GetRpcArg("operatorKey"), GetRpcArg("reason"), GetRpcArg("feeSourceAddress"), + GetRpcArg("submit"), }, - RPCResult{ - RPCResult::Type::STR_HEX, "txid", "The transaction id" + { + RPCResult{"if \"submit\" is not set or set to true", + RPCResult::Type::STR_HEX, "txid", "The transaction id"}, + RPCResult{"if \"submit\" is set to false", + RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"}, }, RPCExamples{ HelpExampleCli("protx", "revoke \"0123456701234567012345670123456701234567012345670123456701234567\" \"072f36a77261cdd5d64c32d97bac417540eddca1d5612f416feb07ff75a8e240\"") @@ -1248,10 +1275,15 @@ static RPCHelpMan protx_revoke() throw JSONRPCError(RPC_INTERNAL_ERROR, "No payout or fee source addresses found, can't revoke"); } + bool fSubmit{true}; + if (!request.params[4].isNull()) { + fSubmit = ParseBoolV(request.params[4], "submit"); + } + SignSpecialTxPayloadByHash(tx, ptx, keyOperator, !isV19active); SetTxPayload(tx, ptx); - return SignAndSendSpecialTx(request, chain_helper, chainman, tx); + return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit); }, }; } diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py index 13756db49462..0b7c8c4f1203 100755 --- a/test/functional/feature_dip3_deterministicmns.py +++ b/test/functional/feature_dip3_deterministicmns.py @@ -213,7 +213,7 @@ def run_test(self): assert old_voting_address != new_voting_address # also check if funds from payout address are used when no fee source address is specified node.sendtoaddress(mn.rewards_address, 0.001) - mn.update_registrar(node, pubKeyOperator="", votingAddr=new_voting_address, rewards_address="") + mn.update_registrar(node, submit=True, pubKeyOperator="", votingAddr=new_voting_address, rewards_address="") self.generate(node, 1) new_dmnState = mn.get_node(self).masternode("status")["dmnState"] new_voting_address_from_rpc = new_dmnState["votingAddress"] @@ -237,14 +237,14 @@ def create_mn_collateral(self, node, mn: MasternodeInfo): # register a protx MN and also fund it (using collateral inside ProRegTx) def register_fund_mn(self, node, mn: MasternodeInfo): node.sendtoaddress(mn.fundsAddr, mn.get_collateral_value() + 0.001) - txid = mn.register_fund(node, True) + txid = mn.register_fund(node, submit=True) vout = mn.get_collateral_vout(node, txid) mn.set_params(proTxHash=txid, collateral_txid=txid, collateral_vout=vout) # create a protx MN which refers to an existing collateral def register_mn(self, node, mn: MasternodeInfo): node.sendtoaddress(mn.fundsAddr, 0.001) - proTxHash = mn.register(node, True) + proTxHash = mn.register(node, submit=True) mn.set_params(proTxHash=proTxHash) self.generate(node, 1, sync_fun=self.no_op) @@ -263,14 +263,14 @@ def spend_mn_collateral(self, mn: MasternodeInfo, with_dummy_input_output=False) def update_mn_payee(self, mn: MasternodeInfo, payee): self.nodes[0].sendtoaddress(mn.fundsAddr, 0.001) - mn.update_registrar(self.nodes[0], pubKeyOperator="", votingAddr="", rewards_address=payee, fundsAddr=mn.fundsAddr) + mn.update_registrar(self.nodes[0], submit=True, pubKeyOperator="", votingAddr="", rewards_address=payee, fundsAddr=mn.fundsAddr) self.generate(self.nodes[0], 1) info = self.nodes[0].protx('info', mn.proTxHash) assert info['state']['payoutAddress'] == payee def test_protx_update_service(self, mn: MasternodeInfo): self.nodes[0].sendtoaddress(mn.fundsAddr, 0.001) - mn.update_service(self.nodes[0], ipAndPort=f'127.0.0.2:{mn.nodePort}') + mn.update_service(self.nodes[0], submit=True, ipAndPort=f'127.0.0.2:{mn.nodePort}') self.generate(self.nodes[0], 1) for node in self.nodes: protx_info = node.protx('info', mn.proTxHash) @@ -279,7 +279,7 @@ def test_protx_update_service(self, mn: MasternodeInfo): assert_equal(mn_list['%s-%d' % (mn.collateral_txid, mn.collateral_vout)]['address'], '127.0.0.2:%d' % mn.nodePort) # undo - mn.update_service(self.nodes[0]) + mn.update_service(self.nodes[0], submit=True) self.generate(self.nodes[0], 1, sync_fun=self.no_op) def assert_mnlists(self, mns): diff --git a/test/functional/feature_dip3_v19.py b/test/functional/feature_dip3_v19.py index 5860063ddef3..8bd87667cc5c 100755 --- a/test/functional/feature_dip3_v19.py +++ b/test/functional/feature_dip3_v19.py @@ -120,7 +120,7 @@ def test_revoke_protx(self, node_idx, revoke_mn: MasternodeInfo): tip = self.generate(self.nodes[0], 1)[0] assert_equal(self.nodes[0].getrawtransaction(fund_txid, 1, tip)['confirmations'], 1) - protx_result = revoke_mn.revoke(self.nodes[0], reason=1, fundsAddr=funds_address) + protx_result = revoke_mn.revoke(self.nodes[0], submit=True, reason=1, fundsAddr=funds_address) self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block tip = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0] assert_equal(self.nodes[0].getrawtransaction(protx_result, 1, tip)['confirmations'], 1) diff --git a/test/functional/feature_llmq_simplepose.py b/test/functional/feature_llmq_simplepose.py index 191f092b1a63..d5ea7288ac01 100755 --- a/test/functional/feature_llmq_simplepose.py +++ b/test/functional/feature_llmq_simplepose.py @@ -208,7 +208,7 @@ def repair_masternodes(self, restart): if check_banned(self.nodes[0], mn) or check_punished(self.nodes[0], mn): addr = self.nodes[0].getnewaddress() self.nodes[0].sendtoaddress(addr, 0.1) - mn.update_service(self.nodes[0], fundsAddr=addr) + mn.update_service(self.nodes[0], submit=True, fundsAddr=addr) if restart: self.stop_node(mn.nodeIdx) self.start_masternode(mn) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index ab868af27c47..d374df40e7fa 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1216,7 +1216,7 @@ def get_node(self, test: BitcoinTestFramework) -> TestNode: raise AssertionError(f"Node at pos {self.nodeIdx} not present, did you start the node?") return test.nodes[self.nodeIdx] - def register(self, node: TestNode, submit: bool = True, collateral_txid: Optional[str] = None, collateral_vout: Optional[int] = None, + def register(self, node: TestNode, submit: bool, collateral_txid: Optional[str] = None, collateral_vout: Optional[int] = None, ipAndPort: Optional[str] = None, ownerAddr: Optional[str] = None, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, operator_reward: Optional[int] = None, rewards_address: Optional[str] = None, fundsAddr: Optional[str] = None, platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, platform_http_port: Optional[int] = None) -> str: @@ -1252,7 +1252,7 @@ def register(self, node: TestNode, submit: bool = True, collateral_txid: Optiona return node.protx(command, *args) - def register_fund(self, node: TestNode, submit: bool = True, collateral_address: Optional[str] = None, ipAndPort: Optional[str] = None, + def register_fund(self, node: TestNode, submit: bool, collateral_address: Optional[str] = None, ipAndPort: Optional[str] = None, ownerAddr: Optional[str] = None, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, operator_reward: Optional[int] = None, rewards_address: Optional[str] = None, fundsAddr: Optional[str] = None, platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, platform_http_port: Optional[int] = None) -> str: @@ -1287,7 +1287,7 @@ def register_fund(self, node: TestNode, submit: bool = True, collateral_address: return node.protx(command, *args) - def revoke(self, node: TestNode, reason: int, fundsAddr: Optional[str] = None) -> str: + def revoke(self, node: TestNode, submit: bool, reason: int, fundsAddr: Optional[str] = None) -> str: # Update commands should be run from the appropriate MasternodeInfo instance, we do not allow overriding some values for this reason if self.proTxHash is None: raise AssertionError("proTxHash not set, did you call set_params()") @@ -1302,11 +1302,13 @@ def revoke(self, node: TestNode, reason: int, fundsAddr: Optional[str] = None) - # fundsAddr is an optional field that results in different behavior if omitted, so we don't fallback here if fundsAddr is not None: - args = args + [fundsAddr] + args = args + [fundsAddr, submit] # type: ignore + elif submit is not True: + raise AssertionError("Cannot withhold transaction if relying on fundsAddr fallback behavior") return node.protx('revoke', *args) - def update_registrar(self, node: TestNode, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, + def update_registrar(self, node: TestNode, submit: bool, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, rewards_address: Optional[str] = None, fundsAddr: Optional[str] = None) -> str: # Update commands should be run from the appropriate MasternodeInfo instance, we do not allow overriding proTxHash for this reason if self.proTxHash is None: @@ -1322,11 +1324,13 @@ def update_registrar(self, node: TestNode, pubKeyOperator: Optional[str] = None, # fundsAddr is an optional field that results in different behavior if omitted, so we don't fallback here if fundsAddr is not None: - args = args + [fundsAddr] + args = args + [fundsAddr, submit] # type: ignore + elif submit is not True: + raise AssertionError("Cannot withhold transaction if relying on fundsAddr fallback behavior") return node.protx(command, *args) - def update_service(self, node: TestNode, ipAndPort: Optional[str] = None, platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, + def update_service(self, node: TestNode, submit: bool, ipAndPort: Optional[str] = None, platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, platform_http_port: Optional[int] = None, address_operator: Optional[str] = None, fundsAddr: Optional[str] = None) -> str: # Update commands should be run from the appropriate MasternodeInfo instance, we do not allow overriding some values for this reason if self.proTxHash is None: @@ -1355,10 +1359,10 @@ def update_service(self, node: TestNode, ipAndPort: Optional[str] = None, platfo # Construct final command and arguments if self.evo: command = "update_service_evo" - args = args + [platform_node_id, platform_p2p_port, platform_http_port, address_operator, address_funds] # type: ignore + args = args + [platform_node_id, platform_p2p_port, platform_http_port, address_operator, address_funds, submit] # type: ignore else: command = "update_service" - args = args + [address_operator, address_funds] # type: ignore + args = args + [address_operator, address_funds, submit] # type: ignore return node.protx(command, *args) @@ -1543,7 +1547,7 @@ def dynamically_prepare_masternode(self, idx, node_p2p_port, evo=False, rnd=None operatorReward = idx # platform_node_id, platform_p2p_port and platform_http_port are ignored for regular masternodes - protx_result = mn.register(self.nodes[0], True, collateral_txid=collateral_txid, collateral_vout=collateral_vout, ipAndPort=ipAndPort, operator_reward=operatorReward, + protx_result = mn.register(self.nodes[0], submit=True, collateral_txid=collateral_txid, collateral_vout=collateral_vout, ipAndPort=ipAndPort, operator_reward=operatorReward, platform_node_id=platform_node_id, platform_p2p_port=platform_p2p_port, platform_http_port=platform_http_port) self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block @@ -1572,7 +1576,7 @@ def dynamically_evo_update_service(self, evo_info: MasternodeInfo, rnd=None, sho protx_success = False try: - protx_result = evo_info.update_service(self.nodes[0], f'127.0.0.1:{evo_info.nodePort}', platform_node_id, platform_p2p_port, platform_http_port, operator_reward_address, funds_address) + protx_result = evo_info.update_service(self.nodes[0], True, f'127.0.0.1:{evo_info.nodePort}', platform_node_id, platform_p2p_port, platform_http_port, operator_reward_address, funds_address) self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block evo_info.bury_tx(self, genIdx=0, txid=protx_result, depth=1) self.log.info("Updated EvoNode %s: platformNodeID=%s, platformP2PPort=%s, platformHTTPPort=%s" % (evo_info.proTxHash, platform_node_id, platform_p2p_port, platform_http_port)) @@ -1609,10 +1613,10 @@ def prepare_masternode(self, idx): submit = (idx % 4) < 2 if register_fund: - protx_result = mn.register_fund(self.nodes[0], submit, ipAndPort=ipAndPort, operator_reward=operatorReward) + protx_result = mn.register_fund(self.nodes[0], submit=submit, ipAndPort=ipAndPort, operator_reward=operatorReward) else: self.generate(self.nodes[0], 1, sync_fun=self.no_op) - protx_result = mn.register(self.nodes[0], submit, collateral_txid=txid, collateral_vout=collateral_vout, ipAndPort=ipAndPort, + protx_result = mn.register(self.nodes[0], submit=submit, collateral_txid=txid, collateral_vout=collateral_vout, ipAndPort=ipAndPort, operator_reward=operatorReward) if submit: proTxHash = protx_result @@ -1624,7 +1628,7 @@ def prepare_masternode(self, idx): if operatorReward > 0: self.generate(self.nodes[0], 1, sync_fun=self.no_op) operatorPayoutAddress = self.nodes[0].getnewaddress() - mn.update_service(self.nodes[0], ipAndPort=ipAndPort, address_operator=operatorPayoutAddress) + mn.update_service(self.nodes[0], submit=True, ipAndPort=ipAndPort, address_operator=operatorPayoutAddress) self.mninfo.append(mn) self.log.info("Prepared MN %d: collateral_txid=%s, collateral_vout=%d, protxHash=%s" % (idx, txid, collateral_vout, proTxHash)) From 5aeec2ad27ba844aa4469d342f32ebdfa6221d69 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 15 Jun 2025 12:26:14 +0000 Subject: [PATCH 7/8] test: validate that the 'submit' argument works as intended --- doc/release-notes-6720.md | 4 ++ .../test_framework/test_framework.py | 55 +++++++++++++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 doc/release-notes-6720.md diff --git a/doc/release-notes-6720.md b/doc/release-notes-6720.md new file mode 100644 index 000000000000..69936aeb26ca --- /dev/null +++ b/doc/release-notes-6720.md @@ -0,0 +1,4 @@ +Updated RPCs +------------ + +* A new optional field `submit` has been introduced to the `protx revoke`, `protx update_registrar`, `protx update_service` RPCs. It behaves identically to `submit` in `protx register` or `protx register_fund`. diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index d374df40e7fa..798faa4e1a70 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1242,6 +1242,11 @@ def register(self, node: TestNode, submit: bool, collateral_txid: Optional[str] ] address_funds = fundsAddr or self.fundsAddr + # Flip a coin and decide if we will submit the transaction directly or use sendrawtransaction + use_srt: bool = bool(random.getrandbits(1)) and submit + if use_srt: + submit = False + # Construct final command and arguments if self.evo: command = "register_evo" @@ -1250,7 +1255,11 @@ def register(self, node: TestNode, submit: bool, collateral_txid: Optional[str] command = "register_legacy" if self.legacy else "register" args = args + [address_funds, submit] # type: ignore - return node.protx(command, *args) + ret: str = node.protx(command, *args) + if use_srt: + ret = node.sendrawtransaction(ret) + + return ret def register_fund(self, node: TestNode, submit: bool, collateral_address: Optional[str] = None, ipAndPort: Optional[str] = None, ownerAddr: Optional[str] = None, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, @@ -1265,6 +1274,11 @@ def register_fund(self, node: TestNode, submit: bool, collateral_address: Option if platform_http_port is None: raise AssertionError("EvoNode but platform_http_port is missing, must be specified!") + # Flip a coin and decide if we will submit the transaction directly or use sendrawtransaction + use_srt: bool = bool(random.getrandbits(1)) and submit + if use_srt: + submit = False + # Common arguments shared between regular masternodes and EvoNodes args = [ collateral_address or self.collateral_address, @@ -1285,7 +1299,11 @@ def register_fund(self, node: TestNode, submit: bool, collateral_address: Option command = "register_fund_legacy" if self.legacy else "register_fund" args = args + [address_funds, submit] # type: ignore - return node.protx(command, *args) + ret: str = node.protx(command, *args) + if use_srt: + ret = node.sendrawtransaction(ret) + + return ret def revoke(self, node: TestNode, submit: bool, reason: int, fundsAddr: Optional[str] = None) -> str: # Update commands should be run from the appropriate MasternodeInfo instance, we do not allow overriding some values for this reason @@ -1294,6 +1312,11 @@ def revoke(self, node: TestNode, submit: bool, reason: int, fundsAddr: Optional[ if self.keyOperator is None: raise AssertionError("keyOperator not set, did you call generate_addresses()") + # Flip a coin and decide if we will submit the transaction directly or use sendrawtransaction + use_srt: bool = bool(random.getrandbits(1)) and submit and (fundsAddr is not None) + if use_srt: + submit = False + args = [ self.proTxHash, self.keyOperator, @@ -1306,7 +1329,11 @@ def revoke(self, node: TestNode, submit: bool, reason: int, fundsAddr: Optional[ elif submit is not True: raise AssertionError("Cannot withhold transaction if relying on fundsAddr fallback behavior") - return node.protx('revoke', *args) + ret: str = node.protx('revoke', *args) + if use_srt: + ret = node.sendrawtransaction(ret) + + return ret def update_registrar(self, node: TestNode, submit: bool, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, rewards_address: Optional[str] = None, fundsAddr: Optional[str] = None) -> str: @@ -1314,6 +1341,11 @@ def update_registrar(self, node: TestNode, submit: bool, pubKeyOperator: Optiona if self.proTxHash is None: raise AssertionError("proTxHash not set, did you call set_params()") + # Flip a coin and decide if we will submit the transaction directly or use sendrawtransaction + use_srt: bool = bool(random.getrandbits(1)) and submit and (fundsAddr is not None) + if use_srt: + submit = False + command = 'update_registrar_legacy' if self.legacy else 'update_registrar' args = [ self.proTxHash, @@ -1328,7 +1360,11 @@ def update_registrar(self, node: TestNode, submit: bool, pubKeyOperator: Optiona elif submit is not True: raise AssertionError("Cannot withhold transaction if relying on fundsAddr fallback behavior") - return node.protx(command, *args) + ret: str = node.protx(command, *args) + if use_srt: + ret = node.sendrawtransaction(ret) + + return ret def update_service(self, node: TestNode, submit: bool, ipAndPort: Optional[str] = None, platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, platform_http_port: Optional[int] = None, address_operator: Optional[str] = None, fundsAddr: Optional[str] = None) -> str: @@ -1347,6 +1383,11 @@ def update_service(self, node: TestNode, submit: bool, ipAndPort: Optional[str] if platform_http_port is None: raise AssertionError("EvoNode but platform_http_port is missing, must be specified!") + # Flip a coin and decide if we will submit the transaction directly or use sendrawtransaction + use_srt: bool = bool(random.getrandbits(1)) and submit + if use_srt: + submit = False + # Common arguments shared between regular masternodes and EvoNodes args = [ self.proTxHash, @@ -1364,7 +1405,11 @@ def update_service(self, node: TestNode, submit: bool, ipAndPort: Optional[str] command = "update_service" args = args + [address_operator, address_funds, submit] # type: ignore - return node.protx(command, *args) + ret: str = node.protx(command, *args) + if use_srt: + ret = node.sendrawtransaction(ret) + + return ret class DashTestFramework(BitcoinTestFramework): def set_test_params(self): From 28aaf1d430f58ce3b8b2751a113ab08a6c13d6f8 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 12 Jun 2025 17:32:05 +0000 Subject: [PATCH 8/8] test: allow `assert_raises_rpc_error` with helper functions --- .../feature_dip3_deterministicmns.py | 1 + .../test_framework/test_framework.py | 83 ++++++++++++++++--- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py index 0b7c8c4f1203..3684033a8f45 100755 --- a/test/functional/feature_dip3_deterministicmns.py +++ b/test/functional/feature_dip3_deterministicmns.py @@ -238,6 +238,7 @@ def create_mn_collateral(self, node, mn: MasternodeInfo): def register_fund_mn(self, node, mn: MasternodeInfo): node.sendtoaddress(mn.fundsAddr, mn.get_collateral_value() + 0.001) txid = mn.register_fund(node, submit=True) + assert txid is not None vout = mn.get_collateral_vout(node, txid) mn.set_params(proTxHash=txid, collateral_txid=txid, collateral_vout=vout) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 798faa4e1a70..89d024e691c2 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -35,8 +35,6 @@ ser_compact_size, ser_string, tx_from_hex, - - ) from .script import hash160 from .p2p import NetworkThread @@ -46,6 +44,7 @@ MAX_NODES, append_config, assert_equal, + assert_raises_rpc_error, check_json_precision, copy_datadir, force_finish_mnsync, @@ -1219,7 +1218,11 @@ def get_node(self, test: BitcoinTestFramework) -> TestNode: def register(self, node: TestNode, submit: bool, collateral_txid: Optional[str] = None, collateral_vout: Optional[int] = None, ipAndPort: Optional[str] = None, ownerAddr: Optional[str] = None, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, operator_reward: Optional[int] = None, rewards_address: Optional[str] = None, fundsAddr: Optional[str] = None, - platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, platform_http_port: Optional[int] = None) -> str: + platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, platform_http_port: Optional[int] = None, + expected_assert_code: Optional[int] = None, expected_assert_msg: Optional[str] = None) -> Optional[str]: + if (expected_assert_code and not expected_assert_msg) or (not expected_assert_code and expected_assert_msg): + raise AssertionError("Intending to use assert_raises_rpc_error() but didn't specify code and message") + # EvoNode-specific fields are ignored for regular masternodes if self.evo: if platform_node_id is None: @@ -1242,8 +1245,11 @@ def register(self, node: TestNode, submit: bool, collateral_txid: Optional[str] ] address_funds = fundsAddr or self.fundsAddr + # Use assert_raises_rpc_error if we expect to error out + use_assert: bool = bool(expected_assert_code and expected_assert_msg) + # Flip a coin and decide if we will submit the transaction directly or use sendrawtransaction - use_srt: bool = bool(random.getrandbits(1)) and submit + use_srt: bool = bool(random.getrandbits(1)) and submit and not use_assert if use_srt: submit = False @@ -1255,6 +1261,10 @@ def register(self, node: TestNode, submit: bool, collateral_txid: Optional[str] command = "register_legacy" if self.legacy else "register" args = args + [address_funds, submit] # type: ignore + if use_assert: + assert_raises_rpc_error(expected_assert_code, expected_assert_msg, node.protx, command, *args) + return None + ret: str = node.protx(command, *args) if use_srt: ret = node.sendrawtransaction(ret) @@ -1264,7 +1274,11 @@ def register(self, node: TestNode, submit: bool, collateral_txid: Optional[str] def register_fund(self, node: TestNode, submit: bool, collateral_address: Optional[str] = None, ipAndPort: Optional[str] = None, ownerAddr: Optional[str] = None, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, operator_reward: Optional[int] = None, rewards_address: Optional[str] = None, fundsAddr: Optional[str] = None, - platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, platform_http_port: Optional[int] = None) -> str: + platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, platform_http_port: Optional[int] = None, + expected_assert_code: Optional[int] = None, expected_assert_msg: Optional[str] = None) -> Optional[str]: + if (expected_assert_code and not expected_assert_msg) or (not expected_assert_code and expected_assert_msg): + raise AssertionError("Intending to use assert_raises_rpc_error() but didn't specify code and message") + # EvoNode-specific fields are ignored for regular masternodes if self.evo: if platform_node_id is None: @@ -1274,8 +1288,11 @@ def register_fund(self, node: TestNode, submit: bool, collateral_address: Option if platform_http_port is None: raise AssertionError("EvoNode but platform_http_port is missing, must be specified!") + # Use assert_raises_rpc_error if we expect to error out + use_assert: bool = bool(expected_assert_code and expected_assert_msg) + # Flip a coin and decide if we will submit the transaction directly or use sendrawtransaction - use_srt: bool = bool(random.getrandbits(1)) and submit + use_srt: bool = bool(random.getrandbits(1)) and submit and not use_assert if use_srt: submit = False @@ -1299,24 +1316,36 @@ def register_fund(self, node: TestNode, submit: bool, collateral_address: Option command = "register_fund_legacy" if self.legacy else "register_fund" args = args + [address_funds, submit] # type: ignore + if use_assert: + assert_raises_rpc_error(expected_assert_code, expected_assert_msg, node.protx, command, *args) + return None + ret: str = node.protx(command, *args) if use_srt: ret = node.sendrawtransaction(ret) return ret - def revoke(self, node: TestNode, submit: bool, reason: int, fundsAddr: Optional[str] = None) -> str: + def revoke(self, node: TestNode, submit: bool, reason: int, fundsAddr: Optional[str] = None, + expected_assert_code: Optional[int] = None, expected_assert_msg: Optional[str] = None) -> Optional[str]: + if (expected_assert_code and not expected_assert_msg) or (not expected_assert_code and expected_assert_msg): + raise AssertionError("Intending to use assert_raises_rpc_error() but didn't specify code and message") + # Update commands should be run from the appropriate MasternodeInfo instance, we do not allow overriding some values for this reason if self.proTxHash is None: raise AssertionError("proTxHash not set, did you call set_params()") if self.keyOperator is None: raise AssertionError("keyOperator not set, did you call generate_addresses()") + # Use assert_raises_rpc_error if we expect to error out + use_assert: bool = bool(expected_assert_code and expected_assert_msg) + # Flip a coin and decide if we will submit the transaction directly or use sendrawtransaction - use_srt: bool = bool(random.getrandbits(1)) and submit and (fundsAddr is not None) + use_srt: bool = bool(random.getrandbits(1)) and submit and (fundsAddr is not None) and not use_assert if use_srt: submit = False + command = 'revoke' args = [ self.proTxHash, self.keyOperator, @@ -1329,20 +1358,31 @@ def revoke(self, node: TestNode, submit: bool, reason: int, fundsAddr: Optional[ elif submit is not True: raise AssertionError("Cannot withhold transaction if relying on fundsAddr fallback behavior") - ret: str = node.protx('revoke', *args) + if use_assert: + assert_raises_rpc_error(expected_assert_code, expected_assert_msg, node.protx, command, *args) + return None + + ret: str = node.protx(command, *args) if use_srt: ret = node.sendrawtransaction(ret) return ret def update_registrar(self, node: TestNode, submit: bool, pubKeyOperator: Optional[str] = None, votingAddr: Optional[str] = None, - rewards_address: Optional[str] = None, fundsAddr: Optional[str] = None) -> str: + rewards_address: Optional[str] = None, fundsAddr: Optional[str] = None, + expected_assert_code: Optional[int] = None, expected_assert_msg: Optional[str] = None) -> Optional[str]: + if (expected_assert_code and not expected_assert_msg) or (not expected_assert_code and expected_assert_msg): + raise AssertionError("Intending to use assert_raises_rpc_error() but didn't specify code and message") + # Update commands should be run from the appropriate MasternodeInfo instance, we do not allow overriding proTxHash for this reason if self.proTxHash is None: raise AssertionError("proTxHash not set, did you call set_params()") + # Use assert_raises_rpc_error if we expect to error out + use_assert: bool = bool(expected_assert_code and expected_assert_msg) + # Flip a coin and decide if we will submit the transaction directly or use sendrawtransaction - use_srt: bool = bool(random.getrandbits(1)) and submit and (fundsAddr is not None) + use_srt: bool = bool(random.getrandbits(1)) and submit and (fundsAddr is not None) and not use_assert if use_srt: submit = False @@ -1360,6 +1400,10 @@ def update_registrar(self, node: TestNode, submit: bool, pubKeyOperator: Optiona elif submit is not True: raise AssertionError("Cannot withhold transaction if relying on fundsAddr fallback behavior") + if use_assert: + assert_raises_rpc_error(expected_assert_code, expected_assert_msg, node.protx, command, *args) + return None + ret: str = node.protx(command, *args) if use_srt: ret = node.sendrawtransaction(ret) @@ -1367,7 +1411,11 @@ def update_registrar(self, node: TestNode, submit: bool, pubKeyOperator: Optiona return ret def update_service(self, node: TestNode, submit: bool, ipAndPort: Optional[str] = None, platform_node_id: Optional[str] = None, platform_p2p_port: Optional[int] = None, - platform_http_port: Optional[int] = None, address_operator: Optional[str] = None, fundsAddr: Optional[str] = None) -> str: + platform_http_port: Optional[int] = None, address_operator: Optional[str] = None, fundsAddr: Optional[str] = None, + expected_assert_code: Optional[int] = None, expected_assert_msg: Optional[str] = None) -> Optional[str]: + if (expected_assert_code and not expected_assert_msg) or (not expected_assert_code and expected_assert_msg): + raise AssertionError("Intending to use assert_raises_rpc_error() but didn't specify code and message") + # Update commands should be run from the appropriate MasternodeInfo instance, we do not allow overriding some values for this reason if self.proTxHash is None: raise AssertionError("proTxHash not set, did you call set_params()") @@ -1383,8 +1431,11 @@ def update_service(self, node: TestNode, submit: bool, ipAndPort: Optional[str] if platform_http_port is None: raise AssertionError("EvoNode but platform_http_port is missing, must be specified!") + # Use assert_raises_rpc_error if we expect to error out + use_assert: bool = bool(expected_assert_code and expected_assert_msg) + # Flip a coin and decide if we will submit the transaction directly or use sendrawtransaction - use_srt: bool = bool(random.getrandbits(1)) and submit + use_srt: bool = bool(random.getrandbits(1)) and submit and not use_assert if use_srt: submit = False @@ -1405,6 +1456,10 @@ def update_service(self, node: TestNode, submit: bool, ipAndPort: Optional[str] command = "update_service" args = args + [address_operator, address_funds, submit] # type: ignore + if use_assert: + assert_raises_rpc_error(expected_assert_code, expected_assert_msg, node.protx, command, *args) + return None + ret: str = node.protx(command, *args) if use_srt: ret = node.sendrawtransaction(ret) @@ -1594,6 +1649,7 @@ def dynamically_prepare_masternode(self, idx, node_p2p_port, evo=False, rnd=None # platform_node_id, platform_p2p_port and platform_http_port are ignored for regular masternodes protx_result = mn.register(self.nodes[0], submit=True, collateral_txid=collateral_txid, collateral_vout=collateral_vout, ipAndPort=ipAndPort, operator_reward=operatorReward, platform_node_id=platform_node_id, platform_p2p_port=platform_p2p_port, platform_http_port=platform_http_port) + assert protx_result is not None self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block mn.bury_tx(self, genIdx=0, txid=protx_result, depth=1) @@ -1622,6 +1678,7 @@ def dynamically_evo_update_service(self, evo_info: MasternodeInfo, rnd=None, sho protx_success = False try: protx_result = evo_info.update_service(self.nodes[0], True, f'127.0.0.1:{evo_info.nodePort}', platform_node_id, platform_p2p_port, platform_http_port, operator_reward_address, funds_address) + assert protx_result is not None self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block evo_info.bury_tx(self, genIdx=0, txid=protx_result, depth=1) self.log.info("Updated EvoNode %s: platformNodeID=%s, platformP2PPort=%s, platformHTTPPort=%s" % (evo_info.proTxHash, platform_node_id, platform_p2p_port, platform_http_port))