Skip to content

refactor(rpc): migrate composite RPCs to client conversion table, drop ParseInt{32,64}V(), lay foundation to drop ParseBoolV() #7099

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 7 commits into from
Mar 21, 2026
Merged

refactor(rpc): migrate composite RPCs to client conversion table, drop ParseInt{32,64}V(), lay foundation to drop ParseBoolV() #7099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fd6ccaf
fix: don't string quote numerical arguments
kwvg Mar 14, 2026
635ecd9
refactor: remove `ParseInt{32,64}V()` usage in Dash-specific code
kwvg Mar 14, 2026
e2ee1f7
refactor: drop array parsing logic in `quorum rotationinfo`
kwvg Mar 14, 2026
ac537fc
fix: adapt `ParseBlockIndex` to work with stringy integers, update docs
kwvg Mar 18, 2026
53a66ca
refactor: drop unused `ParseInt{32,64}V()`, unresolvable `ParseDouble…
kwvg Jan 10, 2026
11bf976
refactor: add composite RPC bools to conversion table, deprecate old fn
kwvg Mar 14, 2026
2d5817e
doc: add release notes
kwvg Mar 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions doc/release-notes-7099.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Updated RPCs
------------

* Dash RPCs will no longer permit submitting strings to boolean input fields in line with validation
enforced on upstream RPCs, where this is already the case. Requests must now use unquoted `true`
or `false`.

* The following RPCs are affected, `bls generate`, `bls fromsecret`, `coinjoinsalt generate`,
`coinjoinsalt set`, `masternode connect`, `protx diff`, `protx list`, `protx register`,
`protx register_legacy`, `protx register_evo`, `protx register_fund`, `protx register_fund_legacy`,
`protx register_fund_evo`, `protx revoke`, `protx update_registrar`, `protx update_registrar_legacy`,
`protx update_service`, `protx update_service_evo`, `quorum info`, `quorum platformsign`,
`quorum rotationinfo`, `quorum sign`.

* This restriction can be relaxed by setting `-deprecatedrpc=permissive_bool` at runtime
but is liable to be removed in future versions of Dash Core.
56 changes: 56 additions & 0 deletions src/rpc/client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,25 +256,81 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "verifyislock", 3, "maxHeight" },
{ "submitchainlock", 2, "blockHeight" },
{ "mnauth", 0, "nodeId" },
// Compound RPCs (note: index position is offset by one to account for subcommand)
{ "bls generate", 1, "legacy" },
{ "bls fromsecret", 2, "legacy" },
{ "coinjoinsalt generate", 1, "overwrite" },
{ "coinjoinsalt set", 2, "overwrite" },
{ "gobject list-prepared", 1, "count" },
{ "gobject prepare", 2, "revision" },
{ "gobject prepare", 3, "time" },
{ "gobject prepare", 7, "outputIndex" },
{ "gobject submit", 2, "revision" },
{ "gobject submit", 3, "time" },
{ "masternode connect", 2, "v2transport" },
{ "masternode payments", 2, "count" },
{ "masternode winners", 1, "count" },
{ "protx diff", 3, "extended" },
Comment thread
kwvg marked this conversation as resolved.
{ "protx list", 2, "detailed" },
{ "protx list", 3, "height" },
{ "protx register", 2, "collateralIndex" },
{ "protx register", 3, "coreP2PAddrs", true },
{ "protx register", 10, "submit" },
{ "protx register_legacy", 2, "collateralIndex" },
{ "protx register_legacy", 3, "coreP2PAddrs", true },
{ "protx register_legacy", 10, "submit" },
{ "protx register_evo", 2, "collateralIndex" },
{ "protx register_evo", 3, "coreP2PAddrs", true },
{ "protx register_evo", 10, "platformP2PAddrs", true },
{ "protx register_evo", 11, "platformHTTPSAddrs", true },
{ "protx register_evo", 13, "submit" },
{ "protx register_fund", 2, "coreP2PAddrs", true },
{ "protx register_fund", 9, "submit" },
{ "protx register_fund_legacy", 2, "coreP2PAddrs", true },
{ "protx register_fund_legacy", 9, "submit" },
{ "protx register_fund_evo", 2, "coreP2PAddrs", true },
{ "protx register_fund_evo", 9, "platformP2PAddrs", true },
{ "protx register_fund_evo", 10, "platformHTTPSAddrs", true },
{ "protx register_fund_evo", 12, "submit" },
{ "protx register_prepare", 2, "collateralIndex" },
{ "protx register_prepare", 3, "coreP2PAddrs", true },
{ "protx register_prepare_legacy", 2, "collateralIndex" },
{ "protx register_prepare_legacy", 3, "coreP2PAddrs", true },
{ "protx register_prepare_evo", 2, "collateralIndex" },
{ "protx register_prepare_evo", 3, "coreP2PAddrs", true },
{ "protx register_prepare_evo", 10, "platformP2PAddrs", true },
{ "protx register_prepare_evo", 11, "platformHTTPSAddrs", true },
{ "protx revoke", 3, "reason" },
{ "protx revoke", 5, "submit" },
{ "protx update_registrar", 6, "submit" },
{ "protx update_registrar_legacy", 6, "submit" },
{ "protx update_service", 2, "coreP2PAddrs", true },
{ "protx update_service", 6, "submit" },
{ "protx update_service_evo", 2, "coreP2PAddrs", true },
{ "protx update_service_evo", 5, "platformP2PAddrs", true },
{ "protx update_service_evo", 6, "platformHTTPSAddrs", true },
{ "protx update_service_evo", 9, "submit" },
{ "quorum dkgsimerror", 2, "rate" },
{ "quorum dkgstatus", 1, "detail_level" },
{ "quorum getdata", 1, "nodeId" },
{ "quorum getdata", 2, "llmqType" },
{ "quorum getdata", 4, "dataMask" },
{ "quorum getrecsig", 1, "llmqType" },
{ "quorum hasrecsig", 1, "llmqType" },
{ "quorum info", 1, "llmqType" },
{ "quorum info", 3, "includeSkShare" },
{ "quorum isconflicting", 1, "llmqType" },
{ "quorum listextended", 1, "height" },
{ "quorum list", 1, "count" },
{ "quorum memberof", 2, "scanQuorumsCount" },
{ "quorum platformsign", 4, "submit" },
{ "quorum rotationinfo", 2, "extraShare" },
{ "quorum rotationinfo", 3, "baseBlockHashes" },
{ "quorum selectquorum", 1, "llmqType" },
{ "quorum sign", 1, "llmqType" },
{ "quorum sign", 5, "submit" },
Comment thread
kwvg marked this conversation as resolved.
{ "quorum verify", 1, "llmqType" },
{ "quorum verify", 6, "signHeight" },
};
// clang-format on

Expand Down
69 changes: 37 additions & 32 deletions src/rpc/evo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request,
paramIdx++;
} else {
uint256 collateralHash(ParseHashV(request.params[paramIdx], "collateralHash"));
int32_t collateralIndex = ParseInt32V(request.params[paramIdx + 1], "collateralIndex");
int32_t collateralIndex = request.params[paramIdx + 1].getInt<int>();
if (collateralHash.IsNull() || collateralIndex < 0) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("invalid hash or index: %s-%d", collateralHash.ToString(), collateralIndex));
}
Expand Down Expand Up @@ -1277,7 +1277,7 @@ static RPCHelpMan protx_revoke()
CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[1].get_str(), "operatorKey");

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

LOCK2(wallet->cs_wallet, ::cs_main);
int height = !request.params[2].isNull() ? ParseInt32V(request.params[2], "height") : chainman.ActiveChain().Height();
int height = !request.params[2].isNull() ? request.params[2].getInt<int>() : chainman.ActiveChain().Height();
if (height < 1 || height > chainman.ActiveChain().Height()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid height specified");
}
Expand Down Expand Up @@ -1495,7 +1495,7 @@ static RPCHelpMan protx_list()
#else
LOCK(::cs_main);
#endif
int height = !request.params[2].isNull() ? ParseInt32V(request.params[2], "height") : chainman.ActiveChain().Height();
int height = !request.params[2].isNull() ? request.params[2].getInt<int>() : chainman.ActiveChain().Height();
if (height < 1 || height > chainman.ActiveChain().Height()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid height specified");
}
Expand Down Expand Up @@ -1587,20 +1587,43 @@ static uint256 ParseBlock(const UniValue& v, const ChainstateManager& chainman,
try {
return ParseHashV(v, strName);
} catch (...) {
int h = ParseInt32V(v, strName);
if (h < 1 || h > chainman.ActiveChain().Height())
bool fail{false}; int32_t h{0};
if (v.isNum()) {
h = v.getInt<int>();
} else if (!ParseInt32(v.get_str(), &h)) {
fail = true;
}
if (fail || h < 1 || h > chainman.ActiveChain().Height()) {
throw std::runtime_error(strprintf("%s must be a block hash or chain height and not %s", strName, v.getValStr()));
}
return *chainman.ActiveChain()[h]->phashBlock;
Comment on lines 1587 to 1599
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Keep malformed block identifiers on RPC_INVALID_PARAMETER.

These helpers catch ParseHashV()'s typed RPC error and then rethrow std::runtime_error, so bad baseBlock/block/startBlock/stopBlock inputs lose their structured RPC error code.

💡 Suggested fix 
-            throw std::runtime_error(strprintf("%s must be a block hash or chain height and not %s", strName, v.getValStr()));
+            throw JSONRPCError(RPC_INVALID_PARAMETER,
+                               strprintf("%s must be a block hash or chain height and not %s",
+                                         strName, v.getValStr()));

Apply the same change in both helpers.

Also applies to: 1642-1652

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/rpc/evo.cpp` around lines 1587 - 1593, The catch blocks that currently
convert ParseHashV exceptions into std::runtime_error lose the original RPC
error code; update the handlers (the catch in the helper that calls ParseHashV
and the analogous one around lines 1642-1652) to throw a JSONRPCError with
RPC_INVALID_PARAMETER and the same formatted message instead of
std::runtime_error (i.e., replace the throw std::runtime_error(...) with throw
JSONRPCError(RPC_INVALID_PARAMETER, strprintf(...))), so malformed block
identifiers retain the structured RPC error code; reference ParseHashV,
RPC_INVALID_PARAMETER, JSONRPCError, and chainman.ActiveChain() when making the
change.

}
}

static const CBlockIndex* ParseBlockIndex(const UniValue& v, const ChainstateManager& chainman, const std::string& strName) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);

try {
const auto hash{ParseBlock(v, chainman, strName)};
const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(hash);
if (!pindex) {
throw std::runtime_error(strprintf("Block %s with hash %s not found", strName, v.getValStr()));
}
return pindex;
} catch (...) {
// Same phrasing as ParseBlock() as it can parse heights
throw std::runtime_error(strprintf("%s must be a block hash or chain height and not %s", strName, v.getValStr()));
}
Comment on lines +1607 to +1617
Copy link
Copy Markdown

@UdjinM6 UdjinM6 Mar 20, 2026

Choose a reason for hiding this comment

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

Since ParseBlock already handles both hash and height parsing and throws on failure, the try/catch can be dropped:

Suggested change
try {
const auto hash{ParseBlock(v, chainman, strName)};
const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(hash);
if (!pindex) {
throw std::runtime_error(strprintf("Block %s with hash %s not found", strName, v.getValStr()));
}
return pindex;
} catch (...) {
// Same phrasing as ParseBlock() as it can parse heights
throw std::runtime_error(strprintf("%s must be a block hash or chain height and not %s", strName, v.getValStr()));
}
const auto hash{ParseBlock(v, chainman, strName)};
const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(hash);
if (!pindex) {
throw std::runtime_error(strprintf("Block %s with hash %s not found", strName, v.getValStr()));
}
return pindex;

The current catch (...) swallows the more specific "not found" error when a valid hash isn't in the index, replacing it with the generic "must be a block hash or chain height" message. Dropping it fixes that.

}

static RPCHelpMan protx_diff()
{
return RPCHelpMan{"protx diff",
"\nCalculates a diff between two deterministic masternode lists. The result also contains proof data.\n",
{
{"baseBlock", RPCArg::Type::NUM, RPCArg::Optional::NO, "The starting block height."},
{"block", RPCArg::Type::NUM, RPCArg::Optional::NO, "The ending block height."},
{"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height."},
{"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height."},
Comment thread
UdjinM6 marked this conversation as resolved.
{"extended", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Show additional fields."},
},
CSimplifiedMNListDiff::GetJsonHelp(/*key=*/"", /*optional=*/false),
Expand Down Expand Up @@ -1635,31 +1658,13 @@ static RPCHelpMan protx_diff()
};
}

static const CBlockIndex* ParseBlockIndex(const UniValue& v, const ChainstateManager& chainman, const std::string& strName) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);

try {
uint256 hash = ParseHashV(v, strName);
const CBlockIndex* pblockindex = chainman.m_blockman.LookupBlockIndex(hash);
if (!pblockindex)
throw std::runtime_error(strprintf("Block %s with hash %s not found", strName, v.getValStr()));
return pblockindex;
} catch (...) {
int h = ParseInt32V(v, strName);
if (h < 1 || h > chainman.ActiveChain().Height())
throw std::runtime_error(strprintf("%s must be a chain height and not %s", strName, v.getValStr()));
return chainman.ActiveChain()[h];
}
}

static RPCHelpMan protx_listdiff()
{
return RPCHelpMan{"protx listdiff",
"\nCalculate a full MN list diff between two masternode lists.\n",
{
{"baseBlock", RPCArg::Type::NUM, RPCArg::Optional::NO, "The starting block height."},
{"block", RPCArg::Type::NUM, RPCArg::Optional::NO, "The ending block height."},
{"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height."},
{"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height."},
Comment thread
UdjinM6 marked this conversation as resolved.
},
RPCResult {
RPCResult::Type::OBJ, "", "",
Expand Down Expand Up @@ -1834,8 +1839,8 @@ static RPCHelpMan evodb_verify()
"This is a read-only operation that does not modify the database.\n"
"If no heights are specified, defaults to the full range from DIP0003 activation to chain tip.\n",
{
{"startBlock", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The starting block height (defaults to DIP0003 activation height)."},
{"stopBlock", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The ending block height (defaults to current chain tip)."},
{"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height)."},
{"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip)."},
Comment thread
UdjinM6 marked this conversation as resolved.
},
RPCResult{
RPCResult::Type::OBJ, "", "",
Expand Down Expand Up @@ -1872,8 +1877,8 @@ static RPCHelpMan evodb_repair()
"If verification fails, recalculates diffs from blockchain data and replaces corrupted records.\n"
"If no heights are specified, defaults to the full range from DIP0003 activation to chain tip.\n",
{
{"startBlock", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The starting block height (defaults to DIP0003 activation height)."},
{"stopBlock", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The ending block height (defaults to current chain tip)."},
{"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height)."},
{"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip)."},
Comment thread
UdjinM6 marked this conversation as resolved.
},
RPCResult{
RPCResult::Type::OBJ, "", "",
Expand Down
12 changes: 6 additions & 6 deletions src/rpc/governance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ static RPCHelpMan gobject_prepare()
hashParent = ParseHashV(request.params[0], "parent-hash");
}

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

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

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

// GET THE PARAMETERS FROM USER

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

CGovernanceObject govobj(hashParent, nRevision, nTime, txidFee, strDataHex);
Expand Down
4 changes: 2 additions & 2 deletions src/rpc/masternode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ static RPCHelpMan masternode_winners()
std::string strFilter;

if (!request.params[0].isNull()) {
nCount = LocaleIndependentAtoi<int>(request.params[0].get_str());
nCount = request.params[0].getInt<int>();
}

if (!request.params[1].isNull()) {
Expand Down Expand Up @@ -378,7 +378,7 @@ static RPCHelpMan masternode_payments()
}
}

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

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