Skip to content

Commit 65366a4

Browse files
PastaPastaPastaPastaPastaPasta
committed
Merge dashpay#7099: refactor(rpc): migrate composite RPCs to client conversion table, drop ParseInt{32,64}V(), lay foundation to drop ParseBoolV()
2d5817e doc: add release notes (Kittywhiskers Van Gogh) 11bf976 refactor: add composite RPC bools to conversion table, deprecate old fn (Kittywhiskers Van Gogh) 53a66ca refactor: drop unused `ParseInt{32,64}V()`, unresolvable `ParseDoubleV()` (Kittywhiskers Van Gogh) ac537fc fix: adapt `ParseBlockIndex` to work with stringy integers, update docs (Kittywhiskers Van Gogh) e2ee1f7 refactor: drop array parsing logic in `quorum rotationinfo` (Kittywhiskers Van Gogh) 635ecd9 refactor: remove `ParseInt{32,64}V()` usage in Dash-specific code (Kittywhiskers Van Gogh) fd6ccaf fix: don't string quote numerical arguments (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependent on dashpay#7098 ## Breaking Changes See release notes. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [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 2d5817e UdjinM6: The issue with a swallowed exception exists on develop actually, can be fixed in a follow-up PR. utACK 2d5817e Tree-SHA512: 2432af51fca2e2ac5b1272d244c96588a9122a82428bdb1a6a9c926eecfa3292f0767749b6d1c67632cc9b8ba7942c266cdc5e0a9ddd06a27b167fe0df28808a
2 parents 1d212a1 + 2d5817e commit 65366a4

File tree

12 files changed

+181
-123
lines changed

12 files changed

+181
-123
lines changed

‎doc/release-notes-7099.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
Updated RPCs
2+
------------
3+
4+
* Dash RPCs will no longer permit submitting strings to boolean input fields in line with validation
5+
enforced on upstream RPCs, where this is already the case. Requests must now use unquoted `true`
6+
or `false`.
7+
8+
* The following RPCs are affected, `bls generate`, `bls fromsecret`, `coinjoinsalt generate`,
9+
`coinjoinsalt set`, `masternode connect`, `protx diff`, `protx list`, `protx register`,
10+
`protx register_legacy`, `protx register_evo`, `protx register_fund`, `protx register_fund_legacy`,
11+
`protx register_fund_evo`, `protx revoke`, `protx update_registrar`, `protx update_registrar_legacy`,
12+
`protx update_service`, `protx update_service_evo`, `quorum info`, `quorum platformsign`,
13+
`quorum rotationinfo`, `quorum sign`.
14+
15+
* This restriction can be relaxed by setting `-deprecatedrpc=permissive_bool` at runtime
16+
but is liable to be removed in future versions of Dash Core.

‎src/rpc/client.cpp‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,25 +256,81 @@ static const CRPCConvertParam vRPCConvertParams[] =
256256
{ "verifyislock", 3, "maxHeight" },
257257
{ "submitchainlock", 2, "blockHeight" },
258258
{ "mnauth", 0, "nodeId" },
259+
// Compound RPCs (note: index position is offset by one to account for subcommand)
260+
{ "bls generate", 1, "legacy" },
261+
{ "bls fromsecret", 2, "legacy" },
262+
{ "coinjoinsalt generate", 1, "overwrite" },
263+
{ "coinjoinsalt set", 2, "overwrite" },
264+
{ "gobject list-prepared", 1, "count" },
265+
{ "gobject prepare", 2, "revision" },
266+
{ "gobject prepare", 3, "time" },
267+
{ "gobject prepare", 7, "outputIndex" },
268+
{ "gobject submit", 2, "revision" },
269+
{ "gobject submit", 3, "time" },
270+
{ "masternode connect", 2, "v2transport" },
271+
{ "masternode payments", 2, "count" },
272+
{ "masternode winners", 1, "count" },
273+
{ "protx diff", 3, "extended" },
274+
{ "protx list", 2, "detailed" },
275+
{ "protx list", 3, "height" },
276+
{ "protx register", 2, "collateralIndex" },
259277
{ "protx register", 3, "coreP2PAddrs", true },
278+
{ "protx register", 10, "submit" },
279+
{ "protx register_legacy", 2, "collateralIndex" },
260280
{ "protx register_legacy", 3, "coreP2PAddrs", true },
281+
{ "protx register_legacy", 10, "submit" },
282+
{ "protx register_evo", 2, "collateralIndex" },
261283
{ "protx register_evo", 3, "coreP2PAddrs", true },
262284
{ "protx register_evo", 10, "platformP2PAddrs", true },
263285
{ "protx register_evo", 11, "platformHTTPSAddrs", true },
286+
{ "protx register_evo", 13, "submit" },
264287
{ "protx register_fund", 2, "coreP2PAddrs", true },
288+
{ "protx register_fund", 9, "submit" },
265289
{ "protx register_fund_legacy", 2, "coreP2PAddrs", true },
290+
{ "protx register_fund_legacy", 9, "submit" },
266291
{ "protx register_fund_evo", 2, "coreP2PAddrs", true },
267292
{ "protx register_fund_evo", 9, "platformP2PAddrs", true },
268293
{ "protx register_fund_evo", 10, "platformHTTPSAddrs", true },
294+
{ "protx register_fund_evo", 12, "submit" },
295+
{ "protx register_prepare", 2, "collateralIndex" },
269296
{ "protx register_prepare", 3, "coreP2PAddrs", true },
297+
{ "protx register_prepare_legacy", 2, "collateralIndex" },
270298
{ "protx register_prepare_legacy", 3, "coreP2PAddrs", true },
299+
{ "protx register_prepare_evo", 2, "collateralIndex" },
271300
{ "protx register_prepare_evo", 3, "coreP2PAddrs", true },
272301
{ "protx register_prepare_evo", 10, "platformP2PAddrs", true },
273302
{ "protx register_prepare_evo", 11, "platformHTTPSAddrs", true },
303+
{ "protx revoke", 3, "reason" },
304+
{ "protx revoke", 5, "submit" },
305+
{ "protx update_registrar", 6, "submit" },
306+
{ "protx update_registrar_legacy", 6, "submit" },
274307
{ "protx update_service", 2, "coreP2PAddrs", true },
308+
{ "protx update_service", 6, "submit" },
275309
{ "protx update_service_evo", 2, "coreP2PAddrs", true },
276310
{ "protx update_service_evo", 5, "platformP2PAddrs", true },
277311
{ "protx update_service_evo", 6, "platformHTTPSAddrs", true },
312+
{ "protx update_service_evo", 9, "submit" },
313+
{ "quorum dkgsimerror", 2, "rate" },
314+
{ "quorum dkgstatus", 1, "detail_level" },
315+
{ "quorum getdata", 1, "nodeId" },
316+
{ "quorum getdata", 2, "llmqType" },
317+
{ "quorum getdata", 4, "dataMask" },
318+
{ "quorum getrecsig", 1, "llmqType" },
319+
{ "quorum hasrecsig", 1, "llmqType" },
320+
{ "quorum info", 1, "llmqType" },
321+
{ "quorum info", 3, "includeSkShare" },
322+
{ "quorum isconflicting", 1, "llmqType" },
323+
{ "quorum listextended", 1, "height" },
324+
{ "quorum list", 1, "count" },
325+
{ "quorum memberof", 2, "scanQuorumsCount" },
326+
{ "quorum platformsign", 4, "submit" },
327+
{ "quorum rotationinfo", 2, "extraShare" },
328+
{ "quorum rotationinfo", 3, "baseBlockHashes" },
329+
{ "quorum selectquorum", 1, "llmqType" },
330+
{ "quorum sign", 1, "llmqType" },
331+
{ "quorum sign", 5, "submit" },
332+
{ "quorum verify", 1, "llmqType" },
333+
{ "quorum verify", 6, "signHeight" },
278334
};
279335
// clang-format on
280336

‎src/rpc/evo.cpp‎

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request,
713713
paramIdx++;
714714
} else {
715715
uint256 collateralHash(ParseHashV(request.params[paramIdx], "collateralHash"));
716-
int32_t collateralIndex = ParseInt32V(request.params[paramIdx + 1], "collateralIndex");
716+
int32_t collateralIndex = request.params[paramIdx + 1].getInt<int>();
717717
if (collateralHash.IsNull() || collateralIndex < 0) {
718718
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("invalid hash or index: %s-%d", collateralHash.ToString(), collateralIndex));
719719
}
@@ -1277,7 +1277,7 @@ static RPCHelpMan protx_revoke()
12771277
CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[1].get_str(), "operatorKey");
12781278

12791279
if (!request.params[2].isNull()) {
1280-
int32_t nReason = ParseInt32V(request.params[2], "reason");
1280+
int32_t nReason = request.params[2].getInt<int>();
12811281
if (nReason < 0 || nReason > CProUpRevTx::REASON_LAST) {
12821282
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("invalid reason %d, must be between 0 and %d", nReason, CProUpRevTx::REASON_LAST));
12831283
}
@@ -1462,7 +1462,7 @@ static RPCHelpMan protx_list()
14621462
bool detailed = !request.params[1].isNull() ? ParseBoolV(request.params[1], "detailed") : false;
14631463

14641464
LOCK2(wallet->cs_wallet, ::cs_main);
1465-
int height = !request.params[2].isNull() ? ParseInt32V(request.params[2], "height") : chainman.ActiveChain().Height();
1465+
int height = !request.params[2].isNull() ? request.params[2].getInt<int>() : chainman.ActiveChain().Height();
14661466
if (height < 1 || height > chainman.ActiveChain().Height()) {
14671467
throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid height specified");
14681468
}
@@ -1495,7 +1495,7 @@ static RPCHelpMan protx_list()
14951495
#else
14961496
LOCK(::cs_main);
14971497
#endif
1498-
int height = !request.params[2].isNull() ? ParseInt32V(request.params[2], "height") : chainman.ActiveChain().Height();
1498+
int height = !request.params[2].isNull() ? request.params[2].getInt<int>() : chainman.ActiveChain().Height();
14991499
if (height < 1 || height > chainman.ActiveChain().Height()) {
15001500
throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid height specified");
15011501
}
@@ -1587,20 +1587,43 @@ static uint256 ParseBlock(const UniValue& v, const ChainstateManager& chainman,
15871587
try {
15881588
return ParseHashV(v, strName);
15891589
} catch (...) {
1590-
int h = ParseInt32V(v, strName);
1591-
if (h < 1 || h > chainman.ActiveChain().Height())
1590+
bool fail{false}; int32_t h{0};
1591+
if (v.isNum()) {
1592+
h = v.getInt<int>();
1593+
} else if (!ParseInt32(v.get_str(), &h)) {
1594+
fail = true;
1595+
}
1596+
if (fail || h < 1 || h > chainman.ActiveChain().Height()) {
15921597
throw std::runtime_error(strprintf("%s must be a block hash or chain height and not %s", strName, v.getValStr()));
1598+
}
15931599
return *chainman.ActiveChain()[h]->phashBlock;
15941600
}
15951601
}
15961602

1603+
static const CBlockIndex* ParseBlockIndex(const UniValue& v, const ChainstateManager& chainman, const std::string& strName) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
1604+
{
1605+
AssertLockHeld(::cs_main);
1606+
1607+
try {
1608+
const auto hash{ParseBlock(v, chainman, strName)};
1609+
const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(hash);
1610+
if (!pindex) {
1611+
throw std::runtime_error(strprintf("Block %s with hash %s not found", strName, v.getValStr()));
1612+
}
1613+
return pindex;
1614+
} catch (...) {
1615+
// Same phrasing as ParseBlock() as it can parse heights
1616+
throw std::runtime_error(strprintf("%s must be a block hash or chain height and not %s", strName, v.getValStr()));
1617+
}
1618+
}
1619+
15971620
static RPCHelpMan protx_diff()
15981621
{
15991622
return RPCHelpMan{"protx diff",
16001623
"\nCalculates a diff between two deterministic masternode lists. The result also contains proof data.\n",
16011624
{
1602-
{"baseBlock", RPCArg::Type::NUM, RPCArg::Optional::NO, "The starting block height."},
1603-
{"block", RPCArg::Type::NUM, RPCArg::Optional::NO, "The ending block height."},
1625+
{"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height."},
1626+
{"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height."},
16041627
{"extended", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Show additional fields."},
16051628
},
16061629
CSimplifiedMNListDiff::GetJsonHelp(/*key=*/"", /*optional=*/false),
@@ -1635,31 +1658,13 @@ static RPCHelpMan protx_diff()
16351658
};
16361659
}
16371660

1638-
static const CBlockIndex* ParseBlockIndex(const UniValue& v, const ChainstateManager& chainman, const std::string& strName) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
1639-
{
1640-
AssertLockHeld(::cs_main);
1641-
1642-
try {
1643-
uint256 hash = ParseHashV(v, strName);
1644-
const CBlockIndex* pblockindex = chainman.m_blockman.LookupBlockIndex(hash);
1645-
if (!pblockindex)
1646-
throw std::runtime_error(strprintf("Block %s with hash %s not found", strName, v.getValStr()));
1647-
return pblockindex;
1648-
} catch (...) {
1649-
int h = ParseInt32V(v, strName);
1650-
if (h < 1 || h > chainman.ActiveChain().Height())
1651-
throw std::runtime_error(strprintf("%s must be a chain height and not %s", strName, v.getValStr()));
1652-
return chainman.ActiveChain()[h];
1653-
}
1654-
}
1655-
16561661
static RPCHelpMan protx_listdiff()
16571662
{
16581663
return RPCHelpMan{"protx listdiff",
16591664
"\nCalculate a full MN list diff between two masternode lists.\n",
16601665
{
1661-
{"baseBlock", RPCArg::Type::NUM, RPCArg::Optional::NO, "The starting block height."},
1662-
{"block", RPCArg::Type::NUM, RPCArg::Optional::NO, "The ending block height."},
1666+
{"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height."},
1667+
{"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height."},
16631668
},
16641669
RPCResult {
16651670
RPCResult::Type::OBJ, "", "",
@@ -1834,8 +1839,8 @@ static RPCHelpMan evodb_verify()
18341839
"This is a read-only operation that does not modify the database.\n"
18351840
"If no heights are specified, defaults to the full range from DIP0003 activation to chain tip.\n",
18361841
{
1837-
{"startBlock", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The starting block height (defaults to DIP0003 activation height)."},
1838-
{"stopBlock", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The ending block height (defaults to current chain tip)."},
1842+
{"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height)."},
1843+
{"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip)."},
18391844
},
18401845
RPCResult{
18411846
RPCResult::Type::OBJ, "", "",
@@ -1872,8 +1877,8 @@ static RPCHelpMan evodb_repair()
18721877
"If verification fails, recalculates diffs from blockchain data and replaces corrupted records.\n"
18731878
"If no heights are specified, defaults to the full range from DIP0003 activation to chain tip.\n",
18741879
{
1875-
{"startBlock", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The starting block height (defaults to DIP0003 activation height)."},
1876-
{"stopBlock", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The ending block height (defaults to current chain tip)."},
1880+
{"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height)."},
1881+
{"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip)."},
18771882
},
18781883
RPCResult{
18791884
RPCResult::Type::OBJ, "", "",

‎src/rpc/governance.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ static RPCHelpMan gobject_prepare()
176176
hashParent = ParseHashV(request.params[0], "parent-hash");
177177
}
178178

179-
int nRevision = ParseInt32V(request.params[1], "revision");
180-
int64_t nTime = ParseInt64V(request.params[2], "time");
179+
int nRevision = request.params[1].getInt<int>();
180+
int64_t nTime = request.params[2].getInt<int64_t>();
181181
std::string strDataHex = request.params[3].get_str();
182182

183183
// CREATE A NEW COLLATERAL TRANSACTION FOR THIS SPECIFIC OBJECT
@@ -225,7 +225,7 @@ static RPCHelpMan gobject_prepare()
225225
outpoint.SetNull();
226226
if (!request.params[5].isNull() && !request.params[6].isNull()) {
227227
uint256 collateralHash(ParseHashV(request.params[5], "outputHash"));
228-
int32_t collateralIndex = ParseInt32V(request.params[6], "outputIndex");
228+
int32_t collateralIndex = request.params[6].getInt<int>();
229229
if (collateralHash.IsNull() || collateralIndex < 0) {
230230
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("invalid hash or index: %s-%d", collateralHash.ToString(), collateralIndex));
231231
}
@@ -278,7 +278,7 @@ static RPCHelpMan gobject_list_prepared()
278278
if (!wallet) return UniValue::VNULL;
279279
EnsureWalletIsUnlocked(*wallet);
280280

281-
int64_t nCount = request.params.empty() ? 10 : ParseInt64V(request.params[0], "count");
281+
int64_t nCount = request.params.empty() ? 10 : request.params[0].getInt<int64_t>();
282282
if (nCount < 0) {
283283
throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative count");
284284
}
@@ -361,8 +361,8 @@ static RPCHelpMan gobject_submit()
361361

362362
// GET THE PARAMETERS FROM USER
363363

364-
int nRevision = ParseInt32V(request.params[1], "revision");
365-
int64_t nTime = ParseInt64V(request.params[2], "time");
364+
int nRevision = request.params[1].getInt<int>();
365+
int64_t nTime = request.params[2].getInt<int64_t>();
366366
std::string strDataHex = request.params[3].get_str();
367367

368368
CGovernanceObject govobj(hashParent, nRevision, nTime, txidFee, strDataHex);

‎src/rpc/masternode.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ static RPCHelpMan masternode_winners()
288288
std::string strFilter;
289289

290290
if (!request.params[0].isNull()) {
291-
nCount = LocaleIndependentAtoi<int>(request.params[0].get_str());
291+
nCount = request.params[0].getInt<int>();
292292
}
293293

294294
if (!request.params[1].isNull()) {
@@ -378,7 +378,7 @@ static RPCHelpMan masternode_payments()
378378
}
379379
}
380380

381-
int64_t nCount = request.params.size() > 1 ? ParseInt64V(request.params[1], "count") : 1;
381+
int64_t nCount = request.params.size() > 1 ? request.params[1].getInt<int64_t>() : 1;
382382

383383
// A temporary vector which is used to sort results properly (there is no "reverse" in/for UniValue)
384384
std::vector<UniValue> vecPayments;

0 commit comments

Comments
 (0)