Skip to content

Commit 44c8dda

Browse files
PastaPastaPastaPastaPastaPasta
committed
Merge #6720: test: implement helper functions for ProTx creation, allow working with assert_raises_rpc_error, add ability to withhold submission to all ProTx creation RPCs
28aaf1d test: allow `assert_raises_rpc_error` with helper functions (Kittywhiskers Van Gogh) 5aeec2a test: validate that the 'submit' argument works as intended (Kittywhiskers Van Gogh) 43318a4 rpc: extend the ability to withhold submission to all ProTxes (Kittywhiskers Van Gogh) 968698b test: add helper for `update_service{,_evo}` calls (Kittywhiskers Van Gogh) 1493d67 test: add helper for `update_registrar{,_legacy}` calls (Kittywhiskers Van Gogh) 6ba685f test: add helper for `revoke` calls (Kittywhiskers Van Gogh) aa96ace test: add helper for `register_fund{,_evo,_legacy}` calls (Kittywhiskers Van Gogh) dbce9c9 test: add helper for `register{,_evo,_legacy}` calls (Kittywhiskers Van Gogh) Pull request description: ## Motivation Continuing on the work of [dash#6718](#6718), this pull request aims to do two things: * Leverage the amount of information stored in `MasternodeInfo` to make ProTx creation simpler by only needing to specify the values that need to be _updated_ instead of needing to read most of the fields in `MasternodeInfo` to construct the RPC call * This also comes with the bonus of `MasternodeInfo` knowing _which_ RPC call to make depending on whether the node is legacy regular, basic regular or basic evonode. * As ordering is managed by `register{,_fund}`/`revoke`/`update_{registrar,service}()`, the arguments can be supplied in any desired order (or in RPC call order as the arguments have been arranged in that way except for `submit`, which is placed earlier and must be specified mandatorily). * Allow testing both failure and success cases by introducing both support for `assert_raises_rpc_error` in these functions _and_ implementing support for withholding submission of a ProTx for non-ProRegTxes. These changes benefit the functional tests needed for extended addresses, where we need to test for _changes_ in input validation logic based on deployment status, evaluating both success and failure cases. For instance, this pull request reduces the amount of logic in `Node` (in `rpc_netinfo.py`, introduced in [dash#6674](#6674)) by a fair amount ([diff](https://github.com/dashpay/dash/compare/c788531d35c48820af951799ff8b67ee2dadb8b3..232eb493484b5632b48320d6bbce6c459ee17602#diff-8bacb0d5b8909182c16ac6696c69f805c5cddc3deebc8eb4c0fac77b37aabd5d)) and could see further reduction in future iterations. ## Additional Information * Depends on #6718 * Dependency for #6674 * As `fundsAddr` is an optional field in some ProTx RPCs that trigger different behavior if not supplied ([source](https://github.com/dashpay/dash/blob/ee34525451fbc3efffba6afadf812d2b7f5b7782/src/rpc/evo.cpp#L1237-L1246)), special handling has been introduced to make sure that if this behavior is desired, the argument itself is omitted as replacing it with a blank string does not trigger the intended behavior. * This means that for `revoke()` and `update_registrar()`, transaction withholding (setting `submit` to `false`) is not allowed as this would require setting `fundsAddr` to an empty string, which does not trigger the expected fallback behavior. * This does not extend to `update_service()` where a blank string is submitted as `protx update_service` does not seem to behave adversely when supplied a blank string. * To test that the `submit` argument works as intended in newly updated and existing RPCs, if `submit` is `True` and no expected error code and message are supplied, at random, `submit` is set to `False` and the hex serialized signed transaction output is broadcast using `sendrawtransaction` (see `use_srd` variable). ## Breaking Changes * 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`. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK 28aaf1d UdjinM6: utACK 28aaf1d Tree-SHA512: 2346c3ebcd9b4384a4884f0ad2e1f675fc23fc15575973109233770f006e1af98d3c22c0bbb819589a6f97e277a580e4f90b39d940fbe99f8a86961783d72e8e
2 parents e8f0b62 + 28aaf1d commit 44c8dda

File tree

6 files changed

+338
-51
lines changed

6 files changed

+338
-51
lines changed

‎doc/release-notes-6720.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Updated RPCs
2+
------------
3+
4+
* 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`.

‎src/rpc/evo.cpp‎

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ static void SignSpecialTxPayloadByHash(const CMutableTransaction& tx, SpecialTxP
320320
payload.sig = key.Sign(hash, use_legacy);
321321
}
322322

323-
static std::string SignAndSendSpecialTx(const JSONRPCRequest& request, CChainstateHelper& chain_helper, const ChainstateManager& chainman, const CMutableTransaction& tx, bool fSubmit = true)
323+
static std::string SignAndSendSpecialTx(const JSONRPCRequest& request, CChainstateHelper& chain_helper, const ChainstateManager& chainman, const CMutableTransaction& tx, bool fSubmit)
324324
{
325325
{
326326
LOCK(cs_main);
@@ -551,9 +551,9 @@ static RPCHelpMan protx_register_fund_evo()
551551
},
552552
{
553553
RPCResult{"if \"submit\" is not set or set to true",
554-
RPCResult::Type::STR_HEX, "txid", "The transaction id"},
554+
RPCResult::Type::STR_HEX, "txid", "The transaction id"},
555555
RPCResult{"if \"submit\" is set to false",
556-
RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"},
556+
RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"},
557557
},
558558
RPCExamples{
559559
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")},
@@ -590,9 +590,9 @@ static RPCHelpMan protx_register_evo()
590590
},
591591
{
592592
RPCResult{"if \"submit\" is not set or set to true",
593-
RPCResult::Type::STR_HEX, "txid", "The transaction id"},
593+
RPCResult::Type::STR_HEX, "txid", "The transaction id"},
594594
RPCResult{"if \"submit\" is set to false",
595-
RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"},
595+
RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"},
596596
},
597597
RPCExamples{
598598
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")},
@@ -897,7 +897,7 @@ static RPCHelpMan protx_register_submit()
897897
ptx.vchSig = opt_vchSig.value();
898898

899899
SetTxPayload(tx, ptx);
900-
return SignAndSendSpecialTx(request, chain_helper, chainman, tx);
900+
return SignAndSendSpecialTx(request, chain_helper, chainman, tx, /*fSubmit=*/true);
901901
},
902902
};
903903
}
@@ -915,9 +915,13 @@ static RPCHelpMan protx_update_service()
915915
GetRpcArg("operatorKey"),
916916
GetRpcArg("operatorPayoutAddress"),
917917
GetRpcArg("feeSourceAddress"),
918+
GetRpcArg("submit"),
918919
},
919-
RPCResult{
920-
RPCResult::Type::STR_HEX, "txid", "The transaction id"
920+
{
921+
RPCResult{"if \"submit\" is not set or set to true",
922+
RPCResult::Type::STR_HEX, "txid", "The transaction id"},
923+
RPCResult{"if \"submit\" is set to false",
924+
RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"},
921925
},
922926
RPCExamples{
923927
HelpExampleCli("protx", "update_service \"0123456701234567012345670123456701234567012345670123456701234567\" \"1.2.3.4:1234\" 5a2e15982e62f1e0b7cf9783c64cf7e3af3f90a52d6c40f6f95d624c0b1621cd")
@@ -947,9 +951,14 @@ static RPCHelpMan protx_update_service_evo()
947951
GetRpcArg("platformHTTPPort"),
948952
GetRpcArg("operatorPayoutAddress"),
949953
GetRpcArg("feeSourceAddress"),
954+
GetRpcArg("submit"),
955+
},
956+
{
957+
RPCResult{"if \"submit\" is not set or set to true",
958+
RPCResult::Type::STR_HEX, "txid", "The transaction id"},
959+
RPCResult{"if \"submit\" is set to false",
960+
RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"},
950961
},
951-
RPCResult{
952-
RPCResult::Type::STR_HEX, "txid", "The transaction id"},
953962
RPCExamples{
954963
HelpExampleCli("protx", "update_service_evo \"0123456701234567012345670123456701234567012345670123456701234567\" \"1.2.3.4:1234\" \"5a2e15982e62f1e0b7cf9783c64cf7e3af3f90a52d6c40f6f95d624c0b1621cd\" \"f2dbd9b0a1f541a7c44d34a58674d0262f5feca5\" 22821 22822")},
955964
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
@@ -1055,14 +1064,19 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques
10551064
}
10561065
}
10571066

1067+
bool fSubmit{true};
1068+
if (!request.params[paramIdx + 2].isNull()) {
1069+
fSubmit = ParseBoolV(request.params[paramIdx + 2], "submit");
1070+
}
1071+
10581072
FundSpecialTx(*wallet, tx, ptx, feeSource);
10591073

10601074
const bool isV19active = DeploymentActiveAfter(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip();),
10611075
Params().GetConsensus(), Consensus::DEPLOYMENT_V19);
10621076
SignSpecialTxPayloadByHash(tx, ptx, keyOperator, !isV19active);
10631077
SetTxPayload(tx, ptx);
10641078

1065-
return SignAndSendSpecialTx(request, chain_helper, chainman, tx);
1079+
return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit);
10661080
}
10671081

10681082
static RPCHelpMan protx_update_registrar_wrapper(const bool specific_legacy_bls_scheme)
@@ -1083,9 +1097,13 @@ static RPCHelpMan protx_update_registrar_wrapper(const bool specific_legacy_bls_
10831097
GetRpcArg("votingAddress_update"),
10841098
GetRpcArg("payoutAddress_update"),
10851099
GetRpcArg("feeSourceAddress"),
1100+
GetRpcArg("submit"),
10861101
},
1087-
RPCResult{
1088-
RPCResult::Type::STR_HEX, "txid", "The transaction id"
1102+
{
1103+
RPCResult{"if \"submit\" is not set or set to true",
1104+
RPCResult::Type::STR_HEX, "txid", "The transaction id"},
1105+
RPCResult{"if \"submit\" is set to false",
1106+
RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"},
10891107
},
10901108
RPCExamples{
10911109
HelpExampleCli("protx", rpc_example)
@@ -1166,11 +1184,16 @@ static RPCHelpMan protx_update_registrar_wrapper(const bool specific_legacy_bls_
11661184
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Dash address: ") + request.params[4].get_str());
11671185
}
11681186

1187+
bool fSubmit{true};
1188+
if (!request.params[5].isNull()) {
1189+
fSubmit = ParseBoolV(request.params[5], "submit");
1190+
}
1191+
11691192
FundSpecialTx(*wallet, tx, ptx, feeSourceDest);
11701193
SignSpecialTxPayloadByHash(tx, ptx, dmn->pdmnState->keyIDOwner, *wallet);
11711194
SetTxPayload(tx, ptx);
11721195

1173-
return SignAndSendSpecialTx(request, chain_helper, chainman, tx);
1196+
return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit);
11741197
},
11751198
};
11761199
}
@@ -1198,9 +1221,13 @@ static RPCHelpMan protx_revoke()
11981221
GetRpcArg("operatorKey"),
11991222
GetRpcArg("reason"),
12001223
GetRpcArg("feeSourceAddress"),
1224+
GetRpcArg("submit"),
12011225
},
1202-
RPCResult{
1203-
RPCResult::Type::STR_HEX, "txid", "The transaction id"
1226+
{
1227+
RPCResult{"if \"submit\" is not set or set to true",
1228+
RPCResult::Type::STR_HEX, "txid", "The transaction id"},
1229+
RPCResult{"if \"submit\" is set to false",
1230+
RPCResult::Type::STR_HEX, "hex", "The serialized signed ProTx in hex format"},
12041231
},
12051232
RPCExamples{
12061233
HelpExampleCli("protx", "revoke \"0123456701234567012345670123456701234567012345670123456701234567\" \"072f36a77261cdd5d64c32d97bac417540eddca1d5612f416feb07ff75a8e240\"")
@@ -1265,10 +1292,15 @@ static RPCHelpMan protx_revoke()
12651292
throw JSONRPCError(RPC_INTERNAL_ERROR, "No payout or fee source addresses found, can't revoke");
12661293
}
12671294

1295+
bool fSubmit{true};
1296+
if (!request.params[4].isNull()) {
1297+
fSubmit = ParseBoolV(request.params[4], "submit");
1298+
}
1299+
12681300
SignSpecialTxPayloadByHash(tx, ptx, keyOperator, !isV19active);
12691301
SetTxPayload(tx, ptx);
12701302

1271-
return SignAndSendSpecialTx(request, chain_helper, chainman, tx);
1303+
return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit);
12721304
},
12731305
};
12741306
}

‎test/functional/feature_dip3_deterministicmns.py‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ def run_test(self):
213213
assert old_voting_address != new_voting_address
214214
# also check if funds from payout address are used when no fee source address is specified
215215
node.sendtoaddress(mn.rewards_address, 0.001)
216-
node.protx('update_registrar' if softfork_active(node, 'v19') else 'update_registrar_legacy', mn.proTxHash, "", new_voting_address, "")
216+
mn.update_registrar(node, submit=True, pubKeyOperator="", votingAddr=new_voting_address, rewards_address="")
217217
self.generate(node, 1)
218218
new_dmnState = mn.get_node(self).masternode("status")["dmnState"]
219219
new_voting_address_from_rpc = new_dmnState["votingAddress"]
@@ -237,14 +237,15 @@ def create_mn_collateral(self, node, mn: MasternodeInfo):
237237
# register a protx MN and also fund it (using collateral inside ProRegTx)
238238
def register_fund_mn(self, node, mn: MasternodeInfo):
239239
node.sendtoaddress(mn.fundsAddr, mn.get_collateral_value() + 0.001)
240-
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)
240+
txid = mn.register_fund(node, submit=True)
241+
assert txid is not None
241242
vout = mn.get_collateral_vout(node, txid)
242243
mn.set_params(proTxHash=txid, collateral_txid=txid, collateral_vout=vout)
243244

244245
# create a protx MN which refers to an existing collateral
245246
def register_mn(self, node, mn: MasternodeInfo):
246247
node.sendtoaddress(mn.fundsAddr, 0.001)
247-
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)
248+
proTxHash = mn.register(node, submit=True)
248249
mn.set_params(proTxHash=proTxHash)
249250
self.generate(node, 1, sync_fun=self.no_op)
250251

@@ -263,14 +264,14 @@ def spend_mn_collateral(self, mn: MasternodeInfo, with_dummy_input_output=False)
263264

264265
def update_mn_payee(self, mn: MasternodeInfo, payee):
265266
self.nodes[0].sendtoaddress(mn.fundsAddr, 0.001)
266-
self.nodes[0].protx('update_registrar' if softfork_active(self.nodes[0], 'v19') else 'update_registrar_legacy', mn.proTxHash, '', '', payee, mn.fundsAddr)
267+
mn.update_registrar(self.nodes[0], submit=True, pubKeyOperator="", votingAddr="", rewards_address=payee, fundsAddr=mn.fundsAddr)
267268
self.generate(self.nodes[0], 1)
268269
info = self.nodes[0].protx('info', mn.proTxHash)
269270
assert info['state']['payoutAddress'] == payee
270271

271272
def test_protx_update_service(self, mn: MasternodeInfo):
272273
self.nodes[0].sendtoaddress(mn.fundsAddr, 0.001)
273-
self.nodes[0].protx('update_service', mn.proTxHash, '127.0.0.2:%d' % mn.nodePort, mn.keyOperator, "", mn.fundsAddr)
274+
mn.update_service(self.nodes[0], submit=True, ipAndPort=f'127.0.0.2:{mn.nodePort}')
274275
self.generate(self.nodes[0], 1)
275276
for node in self.nodes:
276277
protx_info = node.protx('info', mn.proTxHash)
@@ -279,7 +280,7 @@ def test_protx_update_service(self, mn: MasternodeInfo):
279280
assert_equal(mn_list['%s-%d' % (mn.collateral_txid, mn.collateral_vout)]['address'], '127.0.0.2:%d' % mn.nodePort)
280281

281282
# undo
282-
self.nodes[0].protx('update_service', mn.proTxHash, '127.0.0.1:%d' % mn.nodePort, mn.keyOperator, "", mn.fundsAddr)
283+
mn.update_service(self.nodes[0], submit=True)
283284
self.generate(self.nodes[0], 1, sync_fun=self.no_op)
284285

285286
def assert_mnlists(self, mns):

‎test/functional/feature_dip3_v19.py‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,8 @@ def run_test(self):
9595
assert evo_info_3 is not None
9696
self.dynamically_evo_update_service(evo_info_0, 9, should_be_rejected=True)
9797

98-
revoke_protx = self.mninfo[-1].proTxHash
99-
revoke_keyoperator = self.mninfo[-1].keyOperator
100-
self.log.info(f"Trying to revoke proTx:{revoke_protx}")
101-
self.test_revoke_protx(evo_info_3.nodeIdx, revoke_protx, revoke_keyoperator)
98+
self.log.info(f"Trying to revoke proTx:{self.mninfo[-1].proTxHash}")
99+
self.test_revoke_protx(evo_info_3.nodeIdx, self.mninfo[-1])
102100

103101
self.mine_quorum(llmq_type_name='llmq_test', llmq_type=100)
104102

@@ -116,14 +114,14 @@ def run_test(self):
116114

117115
self.wait_for_chainlocked_block_all_nodes(self.nodes[0].getbestblockhash())
118116

119-
def test_revoke_protx(self, node_idx, revoke_protx, revoke_keyoperator):
117+
def test_revoke_protx(self, node_idx, revoke_mn: MasternodeInfo):
120118
funds_address = self.nodes[0].getnewaddress()
121119
fund_txid = self.nodes[0].sendtoaddress(funds_address, 1)
122120
self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block
123121
tip = self.generate(self.nodes[0], 1)[0]
124122
assert_equal(self.nodes[0].getrawtransaction(fund_txid, 1, tip)['confirmations'], 1)
125123

126-
protx_result = self.nodes[0].protx('revoke', revoke_protx, revoke_keyoperator, 1, funds_address)
124+
protx_result = revoke_mn.revoke(self.nodes[0], submit=True, reason=1, fundsAddr=funds_address)
127125
self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block
128126
tip = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0]
129127
assert_equal(self.nodes[0].getrawtransaction(protx_result, 1, tip)['confirmations'], 1)
@@ -132,9 +130,9 @@ def test_revoke_protx(self, node_idx, revoke_protx, revoke_keyoperator):
132130
self.wait_until(lambda: self.nodes[node_idx].getconnectioncount() == 0)
133131
self.connect_nodes(node_idx, 0)
134132
self.sync_all()
135-
self.log.info(f"Successfully revoked={revoke_protx}")
133+
self.log.info(f"Successfully revoked={revoke_mn.proTxHash}")
136134
for mn in self.mninfo: # type: MasternodeInfo
137-
if mn.proTxHash == revoke_protx:
135+
if mn.proTxHash == revoke_mn.proTxHash:
138136
self.mninfo.remove(mn)
139137
return
140138

‎test/functional/feature_llmq_simplepose.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ def repair_masternodes(self, restart):
208208
if check_banned(self.nodes[0], mn) or check_punished(self.nodes[0], mn):
209209
addr = self.nodes[0].getnewaddress()
210210
self.nodes[0].sendtoaddress(addr, 0.1)
211-
self.nodes[0].protx('update_service', mn.proTxHash, f'127.0.0.1:{mn.nodePort}', mn.keyOperator, "", addr)
211+
mn.update_service(self.nodes[0], submit=True, fundsAddr=addr)
212212
if restart:
213213
self.stop_node(mn.nodeIdx)
214214
self.start_masternode(mn)

0 commit comments

Comments
 (0)