refactor(rpc): migrate composite RPCs to client conversion table, drop ParseInt{32,64}V(), lay foundation to drop ParseBoolV()#7099
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
|
This pull request has conflicts, please rebase. |
1 similar comment
|
This pull request has conflicts, please rebase. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR tightens and unifies RPC parameter parsing: boolean parameters now require JSON true/false by default (with optional permissive parsing via -deprecatedrpc=permissive_bool), many ParseInt32V/ParseInt64V/ParseDoubleV uses are replaced by UniValue::getInt/getDouble/getBool and related accessors, the vRPCConvertParams conversion table is extended with many subcommands, several Evo/quorum/protx RPCs now accept block identifiers as either hash or height strings, and unit/functional tests were updated to use explicit booleans/integers. Sequence Diagram(s)sequenceDiagram
participant Client as Client (CLI / JSON-RPC)
participant RPC as RPC Server
participant Conv as vRPCConvertParams
participant Util as ParseBoolV / UniValue accessors
participant Handler as RPC Handler (protx / quorum / evo / ...)
Client->>RPC: send RPC request (method, params)
RPC->>Conv: lookup method-specific conversions
Conv-->>RPC: mapped param types/indices
RPC->>Util: validate & parse params (getInt/getBool/ParseBoolV)
alt -deprecatedrpc=permissive_bool enabled
Util-->>Handler: coerced values (strings/numbers -> bool/int)
else
Util-->>Handler: strict JSON types or error (require JSON boolean)
end
Handler->>RPC: execute command logic (may query chain/index)
RPC->>Client: return result or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
doc/release-notes-7099.md (1)
4-16:⚠️ Potential issue | 🟠 MajorPlease document the quoted-number break as well.
This note only covers boolean inputs, but the same PR also removes server-side quoted-number coercion in composite RPCs by swapping
ParseInt{32,64}V()forgetInt<...>()in places likesrc/rpc/masternode.cpp,src/rpc/evo.cpp, andsrc/rpc/governance.cpp. Raw JSON requests such as"count":"1"or"height":"1000"will now fail too, so the release notes should call that out explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/release-notes-7099.md` around lines 4 - 16, Update the release note paragraph to also document the quoted-number break: explain that server-side quoted-number coercion was removed (mention the swap from ParseInt32V/ParseInt64V to getInt<...>()), and that raw JSON string numbers like "count":"1" or "height":"1000" in composite RPCs (e.g., in src/rpc/masternode.cpp, src/rpc/evo.cpp, src/rpc/governance.cpp) will now be rejected; state that this affects the same RPCs listed and that using unquoted numeric literals is required (and that -deprecatedrpc=permissive_bool does not relax numeric coercion), so callers must send numbers unquoted to avoid failures.
🧹 Nitpick comments (3)
src/test/rpc_tests.cpp (1)
525-569: Please add explicit coverage for the new bool-validation behavior.These edits keep the happy path passing, but they do not prove the actual behavior change. Adding one assertion that
bls generate 1fails without-deprecatedrpc=permissive_bool, plus one compatibility-path assertion with that flag enabled, would protect bothParseBoolV()branches from regressing silently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/rpc_tests.cpp` around lines 525 - 569, Update the rpc_bls test to explicitly exercise the new bool-validation branches: add a check that CallRPC(std::string("bls generate 1")) throws (e.g., BOOST_CHECK_THROW) to cover the strict ParseBoolV path, and add a complementary check that invoking CallRPC with the deprecated-permissive bool mode (simulate enabling "-deprecatedrpc=permissive_bool" for the RPC context) allows "bls generate 1" to succeed and returns the expected scheme/public fields; reference the rpc_bls test and the CallRPC helper when adding these assertions so both ParseBoolV branches are covered.src/rpc/quorums.cpp (1)
862-866: Consider usinggetInt<int32_t>()for type consistency.The variable
rateis declared asint32_t, butgetInt<int>()is used. While these are typically equivalent, usinggetInt<int32_t>()would be more explicit and consistent with the declared type.🔧 Suggested fix
- int32_t rate = request.params[1].getInt<int>(); + int32_t rate = request.params[1].getInt<int32_t>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/quorums.cpp` around lines 862 - 866, The variable `rate` is declared as int32_t but is retrieved using request.params[1].getInt<int>(), causing a type mismatch in intent; change the call to request.params[1].getInt<int32_t>() so the fetched value matches the declared type `rate` (in the block that performs the bounds check and throws JSONRPCError on invalid rate).src/rpc/client.cpp (1)
320-339: Add conversion table entry forquorum platformsignsubmit parameter.The
quorum platformsigncommand has asubmitboolean parameter at position 4, but no conversion table entry exists for it. Bothquorum signandquorum platformsignfollow the same pattern usingParseBoolV(request.params[3], "submit"), so the conversion table should include an entry forquorum platformsignfor consistency.🔧 Suggested addition
{ "quorum sign", 5, "submit" }, + { "quorum platformsign", 4, "submit" }, { "quorum verify", 1, "llmqType" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/client.cpp` around lines 320 - 339, The conversion table in client.cpp is missing an entry for the "quorum platformsign" command's boolean submit parameter; add a table row analogous to the existing "quorum sign" entry referencing the 4th parameter position (index 4 in human terms, param index 3 when parsed) and name "submit" so that the code path using ParseBoolV(request.params[3], "submit") is covered; locate the conversion table near the other "quorum ..." entries and add { "quorum platformsign", 4, "submit" } (mirroring the "quorum sign" entry) to ensure consistent parameter conversion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@doc/release-notes-7099.md`:
- Around line 4-16: Update the release note paragraph to also document the
quoted-number break: explain that server-side quoted-number coercion was removed
(mention the swap from ParseInt32V/ParseInt64V to getInt<...>()), and that raw
JSON string numbers like "count":"1" or "height":"1000" in composite RPCs (e.g.,
in src/rpc/masternode.cpp, src/rpc/evo.cpp, src/rpc/governance.cpp) will now be
rejected; state that this affects the same RPCs listed and that using unquoted
numeric literals is required (and that -deprecatedrpc=permissive_bool does not
relax numeric coercion), so callers must send numbers unquoted to avoid
failures.
---
Nitpick comments:
In `@src/rpc/client.cpp`:
- Around line 320-339: The conversion table in client.cpp is missing an entry
for the "quorum platformsign" command's boolean submit parameter; add a table
row analogous to the existing "quorum sign" entry referencing the 4th parameter
position (index 4 in human terms, param index 3 when parsed) and name "submit"
so that the code path using ParseBoolV(request.params[3], "submit") is covered;
locate the conversion table near the other "quorum ..." entries and add {
"quorum platformsign", 4, "submit" } (mirroring the "quorum sign" entry) to
ensure consistent parameter conversion.
In `@src/rpc/quorums.cpp`:
- Around line 862-866: The variable `rate` is declared as int32_t but is
retrieved using request.params[1].getInt<int>(), causing a type mismatch in
intent; change the call to request.params[1].getInt<int32_t>() so the fetched
value matches the declared type `rate` (in the block that performs the bounds
check and throws JSONRPCError on invalid rate).
In `@src/test/rpc_tests.cpp`:
- Around line 525-569: Update the rpc_bls test to explicitly exercise the new
bool-validation branches: add a check that CallRPC(std::string("bls generate
1")) throws (e.g., BOOST_CHECK_THROW) to cover the strict ParseBoolV path, and
add a complementary check that invoking CallRPC with the deprecated-permissive
bool mode (simulate enabling "-deprecatedrpc=permissive_bool" for the RPC
context) allows "bls generate 1" to succeed and returns the expected
scheme/public fields; reference the rpc_bls test and the CallRPC helper when
adding these assertions so both ParseBoolV branches are covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cbf3bc36-77bf-483d-9417-f6ed108cca63
📒 Files selected for processing (11)
doc/release-notes-7099.mdsrc/rpc/client.cppsrc/rpc/evo.cppsrc/rpc/governance.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/rpc/util.cppsrc/rpc/util.hsrc/test/rpc_tests.cpptest/functional/feature_llmq_connections.pytest/functional/feature_llmq_dkgerrors.py
💤 Files with no reviewable changes (1)
- src/rpc/util.h
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7d0e58560
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/rpc/util.cpp`:
- Around line 125-151: ParseBoolV currently leaves strBool empty when
permissive_bool is enabled and v is neither bool, num, nor str; add an explicit
branch to handle unexpected UniValue types before calling ToLower to avoid using
an uninitialized/empty string. Inside the permissive_bool block (in ParseBoolV)
after the if/else if chain that checks v.isBool(), v.isNum(), and v.isStr(), add
an else that throws a JSONRPCError (using the same RPC_INVALID_PARAMETER code
and strName) describing the unsupported type (include the UniValue type or a
short description) so we fail fast instead of falling through to ToLower and the
later comparisons that rely on strBool.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 275b0cfb-c32e-4681-9b78-b48f452170f8
📒 Files selected for processing (10)
doc/release-notes-7099.mdsrc/rpc/client.cppsrc/rpc/evo.cppsrc/rpc/governance.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/rpc/util.cppsrc/rpc/util.hsrc/test/rpc_tests.cpptest/functional/feature_llmq_connections.py
💤 Files with no reviewable changes (1)
- src/rpc/util.h
✅ Files skipped from review due to trivial changes (1)
- doc/release-notes-7099.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/rpc/masternode.cpp
- src/test/rpc_tests.cpp
- src/rpc/governance.cpp
- src/rpc/evo.cpp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2d327da4e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/rpc/quorums.cpp (1)
905-918:⚠️ Potential issue | 🟡 MinorValidate
dataMaskis within the valid range before casting touint16_t.The RPC help text documents
dataMaskas accepting only1,2, or3, but the code casts the parameter directly without validation. This allows invalid values like-1(wraps to65535),65536(wraps to0), or4to pass through and be sent on the wire.💡 Suggested fix
- uint16_t nDataMask = static_cast<uint16_t>(request.params[3].getInt<int>()); + const int data_mask = request.params[3].getInt<int>(); + if (data_mask < 1 || data_mask > 3) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid dataMask. Must be 1, 2, or 3"); + } + const uint16_t nDataMask = static_cast<uint16_t>(data_mask);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/quorums.cpp` around lines 905 - 918, The code casts request.params[3] directly to uint16_t (nDataMask) allowing out-of-range/wrapped values; before casting, read the raw integer (e.g., int64_t rawDataMask = request.params[3].getInt<int64_t>()), validate it is one of the allowed values (1, 2, or 3) and throw JSONRPCError(RPC_INVALID_PARAMETER, "dataMask invalid") if not, then assign nDataMask = static_cast<uint16_t>(rawDataMask); keep the existing logic that checks nDataMask & llmq::CQuorumDataRequest::ENCRYPTED_CONTRIBUTIONS and uses ParseHashV/proTxHash unchanged.
♻️ Duplicate comments (1)
src/rpc/util.cpp (1)
127-143:⚠️ Potential issue | 🟡 MinorReject unsupported types before normalizing
strBool.
null, arrays, and objects still fall through withstrBool == "", so callers getnot ''instead of the actual bad input type.💡 Suggested fix
if (std23::ranges::contains(gArgs.GetArgs("-deprecatedrpc"), "permissive_bool")) { std::string strBool; if (v.isBool()) return v.get_bool(); else if (v.isNum()) strBool = ToString(v.getInt<int>()); else if (v.isStr()) strBool = v.get_str(); + else + throw JSONRPCError(RPC_INVALID_PARAMETER, + strprintf("%s must be a boolean, number, or string (not '%s')", + strName, uvTypeName(v.type()))); strBool = ToLower(strBool);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/util.cpp` around lines 127 - 143, In permissive bool handling (the block gated by std23::ranges::contains(gArgs.GetArgs("-deprecatedrpc"), "permissive_bool")), reject unsupported JSON types before you normalize strBool: after checking v.isBool(), v.isNum(), and v.isStr() but before calling ToLower(strBool), add an else branch that throws JSONRPCError(RPC_INVALID_PARAMETER, /*message*/ ) for null/array/object inputs (using v.type()/a string describing the actual type or the raw value) so callers receive an error describing the bad input type instead of getting an empty-string error; keep existing true/false/yes/no/1/0 logic and use the same RPC_INVALID_PARAMETER and strName symbols in the message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/rpc/evo.cpp`:
- Around line 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.
In `@src/rpc/quorums.cpp`:
- Around line 281-284: Read the raw integer from request.params into a signed
integer first (e.g., rawLlmq = request.params[0].getInt<int64_t>()), validate it
fits the unsigned-backend range before casting (check rawLlmq >= 0 && rawLlmq <=
std::numeric_limits<uint8_t>::max()), then cast to Consensus::LLMQType and call
Params().GetLLMQ(llmqType) as before; apply the same raw-before-cast pattern to
every place using static_cast<Consensus::LLMQType> (the other occurrences you
noted) to avoid narrowing. For nDataMask, read the raw int into a signed type,
check it is one of the documented valid values (1, 2, or 3) and within uint16_t
range (0..65535) before casting to uint16_t, then proceed with existing logic.
Ensure you update all referenced usages (llmqType and nDataMask) so validation
occurs on the raw value prior to any static_cast.
---
Outside diff comments:
In `@src/rpc/quorums.cpp`:
- Around line 905-918: The code casts request.params[3] directly to uint16_t
(nDataMask) allowing out-of-range/wrapped values; before casting, read the raw
integer (e.g., int64_t rawDataMask = request.params[3].getInt<int64_t>()),
validate it is one of the allowed values (1, 2, or 3) and throw
JSONRPCError(RPC_INVALID_PARAMETER, "dataMask invalid") if not, then assign
nDataMask = static_cast<uint16_t>(rawDataMask); keep the existing logic that
checks nDataMask & llmq::CQuorumDataRequest::ENCRYPTED_CONTRIBUTIONS and uses
ParseHashV/proTxHash unchanged.
---
Duplicate comments:
In `@src/rpc/util.cpp`:
- Around line 127-143: In permissive bool handling (the block gated by
std23::ranges::contains(gArgs.GetArgs("-deprecatedrpc"), "permissive_bool")),
reject unsupported JSON types before you normalize strBool: after checking
v.isBool(), v.isNum(), and v.isStr() but before calling ToLower(strBool), add an
else branch that throws JSONRPCError(RPC_INVALID_PARAMETER, /*message*/ ) for
null/array/object inputs (using v.type()/a string describing the actual type or
the raw value) so callers receive an error describing the bad input type instead
of getting an empty-string error; keep existing true/false/yes/no/1/0 logic and
use the same RPC_INVALID_PARAMETER and strName symbols in the message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1a473dd8-2f44-4a18-9a33-9b4a369f9755
📒 Files selected for processing (10)
doc/release-notes-7099.mdsrc/rpc/client.cppsrc/rpc/evo.cppsrc/rpc/governance.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/rpc/util.cppsrc/rpc/util.hsrc/test/rpc_tests.cpptest/functional/feature_llmq_connections.py
💤 Files with no reviewable changes (1)
- src/rpc/util.h
✅ Files skipped from review due to trivial changes (1)
- doc/release-notes-7099.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/rpc/governance.cpp
- src/rpc/masternode.cpp
| try { | ||
| return ParseHashV(v, strName); | ||
| } catch (...) { | ||
| int h = ParseInt32V(v, strName); | ||
| int h = v.isNum() ? v.getInt<int>() : LocaleIndependentAtoi<int>(v.get_str()); | ||
| if (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; |
There was a problem hiding this comment.
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.
| const Consensus::LLMQType llmqType{static_cast<Consensus::LLMQType>(request.params[0].getInt<int>())}; | ||
| if (!Params().GetLLMQ(llmqType).has_value()) { | ||
| throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid LLMQ type"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the Consensus::LLMQType definition
find . -name "*.h" -o -name "*.hpp" | head -20Repository: dashpay/dash
Length of output: 468
🏁 Script executed:
# Look for the LLMQ type definition
rg "enum.*LLMQType|class.*LLMQType" -A 5 -B 2Repository: dashpay/dash
Length of output: 1121
🏁 Script executed:
# Examine the specific file mentioned in the scratchpad
fd "params.h" -p "llmq"Repository: dashpay/dash
Length of output: 147
🏁 Script executed:
# Get the structure of the repo
git ls-files | grep -E "(llmq|quorums)" | head -20Repository: dashpay/dash
Length of output: 530
🏁 Script executed:
# Examine the quorums.cpp file around the flagged lines
wc -l src/rpc/quorums.cppRepository: dashpay/dash
Length of output: 80
🏁 Script executed:
# Look at lines 281-284 and surrounding context
sed -n '275,290p' src/rpc/quorums.cppRepository: dashpay/dash
Length of output: 754
🏁 Script executed:
# Check all flagged lines in the review comment
echo "=== Lines 596-597 ===" && sed -n '590,605p' src/rpc/quorums.cppRepository: dashpay/dash
Length of output: 731
🏁 Script executed:
# Continue checking flagged lines
echo "=== Lines 665-669 ===" && sed -n '660,675p' src/rpc/quorums.cpp
echo "=== Lines 718-720 ===" && sed -n '715,725p' src/rpc/quorums.cppRepository: dashpay/dash
Length of output: 1267
🏁 Script executed:
# Check remaining flagged lines
echo "=== Lines 747-749 ===" && sed -n '742,755p' src/rpc/quorums.cpp
echo "=== Lines 783-785 ===" && sed -n '778,790p' src/rpc/quorums.cppRepository: dashpay/dash
Length of output: 1283
🏁 Script executed:
# Check final flagged lines
echo "=== Lines 820-823 ===" && sed -n '815,830p' src/rpc/quorums.cpp
echo "=== Lines 903-904 ===" && sed -n '898,910p' src/rpc/quorums.cppRepository: dashpay/dash
Length of output: 1479
🏁 Script executed:
# Now examine the GetLLMQ validation function
fd "chainparams" -e cpp -e h | xargs rg "GetLLMQ" -A 10 -B 2 | head -80Repository: dashpay/dash
Length of output: 4466
🏁 Script executed:
# Check what getInt returns and its range
rg "getInt<int>" src/rpc/quorums.cpp | head -5Repository: dashpay/dash
Length of output: 377
🏁 Script executed:
# Verify the exact values that LLMQType can have
cat src/llmq/params.h | grep -A 10 "enum class LLMQType"Repository: dashpay/dash
Length of output: 571
🏁 Script executed:
# Understand what getInt<int>() returns - check if there are docs or type constraints
rg "getInt<int>" -B 2 -A 2 src/univalue | head -30Repository: dashpay/dash
Length of output: 1432
🏁 Script executed:
# Check the dataMask validation concern - look at line 905 and nearby
sed -n '900,915p' src/rpc/quorums.cppRepository: dashpay/dash
Length of output: 826
🏁 Script executed:
# Look for any validation of dataMask values
rg "dataMask|nDataMask" src/rpc/quorums.cpp -B 2 -A 2Repository: dashpay/dash
Length of output: 1206
🏁 Script executed:
# Check if there's any validation of request.params[3] before the cast
sed -n '900,920p' src/rpc/quorums.cppRepository: dashpay/dash
Length of output: 991
🏁 Script executed:
# Verify the semantics: what happens when you cast 257 (0x101) to uint8_t?
python3 << 'EOF'
# Simulate the cast behavior
test_values = [1, 2, 3, 255, 257, -1, 256, 512]
for val in test_values:
# Simulate int to uint8_t cast
narrowed = val & 0xFF
print(f"int({val}) -> uint8_t: {narrowed} (0x{narrowed:02x})")
EOFRepository: dashpay/dash
Length of output: 292
🏁 Script executed:
# Verify available LLMQ types
rg "LLMQ_.*=" src/llmq/params.h | head -10Repository: dashpay/dash
Length of output: 832
🏁 Script executed:
# Final check: Verify the order of operations one more time for clarity
echo "=== Verification of casting before validation ===" && sed -n '281,284p' src/rpc/quorums.cpp
echo "" && echo "=== GetLLMQ implementation ===" && sed -n '1,30p' src/chainparams.cpp | tail -20Repository: dashpay/dash
Length of output: 1169
🏁 Script executed:
# Check if there's any pre-validation of the raw int value before casting
rg -B 5 "static_cast<Consensus::LLMQType>" src/rpc/quorums.cpp | head -40Repository: dashpay/dash
Length of output: 1783
Validate raw llmqType before casting to Consensus::LLMQType.
Consensus::LLMQType is uint8_t-backed, so values like 257 or -255 can narrow to valid enum values (e.g., 257 → 1) during the cast before GetLLMQ() validation. This allows invalid RPC inputs to pass validation by coincidentally matching a real quorum type. Store the raw int, validate its range against known types, then cast.
Also applies to: 596-597, 665-669, 718-720, 747-749, 783-785, 820-823, 903-904. Additionally, line 905 has the same issue with nDataMask being cast from int to uint16_t without validation despite documented valid values being only 1, 2, or 3.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rpc/quorums.cpp` around lines 281 - 284, Read the raw integer from
request.params into a signed integer first (e.g., rawLlmq =
request.params[0].getInt<int64_t>()), validate it fits the unsigned-backend
range before casting (check rawLlmq >= 0 && rawLlmq <=
std::numeric_limits<uint8_t>::max()), then cast to Consensus::LLMQType and call
Params().GetLLMQ(llmqType) as before; apply the same raw-before-cast pattern to
every place using static_cast<Consensus::LLMQType> (the other occurrences you
noted) to avoid narrowing. For nDataMask, read the raw int into a signed type,
check it is one of the documented valid values (1, 2, or 3) and within uint16_t
range (0..65535) before casting to uint16_t, then proceed with existing logic.
Ensure you update all referenced usages (llmqType and nDataMask) so validation
occurs on the raw value prior to any static_cast.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72b6c04dd3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/rpc/evo.cpp (1)
1590-1593:⚠️ Potential issue | 🟡 MinorPreserve structured RPC error codes for malformed block identifiers.
Line 1592 and Line 1651 throw
std::runtime_error, which dropsRPC_INVALID_PARAMETERsemantics in these RPC parsing paths. Keep these failures asJSONRPCError(RPC_INVALID_PARAMETER, ...)so clients receive stable error codes.💡 Proposed 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()));#!/bin/bash # Verify these helper throw sites and downstream call paths. rg -n "ParseBlock\\(|ParseBlockIndex\\(" src/rpc/evo.cpp rg -n "must be a block hash or chain height" src/rpc/evo.cpp -C2 rg -n "throw std::runtime_error\\(" src/rpc/evo.cpp -C1Also applies to: 1649-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 1590 - 1593, Replace the throw std::runtime_error(...) occurrences in the RPC block-parsing paths (e.g., in ParseBlock/ParseBlockIndex helpers where the code computes h from v and checks range) with JSONRPCError(RPC_INVALID_PARAMETER, ...) so RPC clients receive the structured RPC_INVALID_PARAMETER code; preserve the existing error message text (strprintf("%s must be a block hash or chain height and not %s", strName, v.getValStr())) when constructing the JSONRPCError and update both failing sites referenced in the review.src/rpc/quorums.cpp (1)
281-284:⚠️ Potential issue | 🟠 MajorValidate raw numeric inputs before narrowing casts to
LLMQType/uint16_t.At Line 281 (and the similar callsites listed),
getInt<int>()is cast before validation. If the enum backing type is narrower, out-of-range values can wrap and potentially passGetLLMQ()checks. Line 905 has the same pattern fornDataMask, and it also needs explicit domain validation (1,2, or3) before casting.💡 Proposed fix pattern
+ const int raw_llmq_type = request.params[0].getInt<int>(); + if (raw_llmq_type < 0 || raw_llmq_type > std::numeric_limits<uint8_t>::max()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid LLMQ type"); + } - const Consensus::LLMQType llmqType{static_cast<Consensus::LLMQType>(request.params[0].getInt<int>())}; + const Consensus::LLMQType llmqType{static_cast<Consensus::LLMQType>(raw_llmq_type)};- uint16_t nDataMask = static_cast<uint16_t>(request.params[3].getInt<int>()); + const int raw_data_mask = request.params[3].getInt<int>(); + if (raw_data_mask != 1 && raw_data_mask != 2 && raw_data_mask != 3) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid dataMask. Valid values: 1, 2, or 3"); + } + const uint16_t nDataMask = static_cast<uint16_t>(raw_data_mask);#!/bin/bash # Verify enum backing and all narrowing cast callsites. rg -n "enum class LLMQType" src/llmq/params.h -C2 rg -n "static_cast<Consensus::LLMQType>\\(request\\.params\\[[0-9]+\\]\\.getInt<int>\\(\\)\\)" src/rpc/quorums.cpp rg -n "nDataMask\\s*=\\s*static_cast<uint16_t>\\(request\\.params\\[3\\]\\.getInt<int>\\(\\)\\)" src/rpc/quorums.cppAlso applies to: 596-597, 665-666, 718-719, 747-748, 783-784, 820-821, 902-905
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/quorums.cpp` around lines 281 - 284, The code narrows raw numeric inputs with static_cast to Consensus::LLMQType (e.g., the llmqType assignment) and to uint16_t (nDataMask) without validating the original int, risking wraparound; change each site (where you see static_cast<Consensus::LLMQType>(request.params[..].getInt<int>()) and the nDataMask static_cast) to first read the int into a local int (e.g., raw = request.params[i].getInt<int>()), validate that raw lies within the valid domain (for LLMQ use the permitted enum range/values returned by Params().GetLLMQ or explicit min/max of Consensus::LLMQType before casting, and for nDataMask ensure raw is one of 1,2,3) and only then perform the static_cast and proceed (update error messages to use RPC_INVALID_PARAMETER when validation fails); apply this pattern to all listed callsites (including the llmqType occurrences and the nDataMask handling).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/rpc/evo.cpp`:
- Around line 1590-1593: Replace the throw std::runtime_error(...) occurrences
in the RPC block-parsing paths (e.g., in ParseBlock/ParseBlockIndex helpers
where the code computes h from v and checks range) with
JSONRPCError(RPC_INVALID_PARAMETER, ...) so RPC clients receive the structured
RPC_INVALID_PARAMETER code; preserve the existing error message text
(strprintf("%s must be a block hash or chain height and not %s", strName,
v.getValStr())) when constructing the JSONRPCError and update both failing sites
referenced in the review.
In `@src/rpc/quorums.cpp`:
- Around line 281-284: The code narrows raw numeric inputs with static_cast to
Consensus::LLMQType (e.g., the llmqType assignment) and to uint16_t (nDataMask)
without validating the original int, risking wraparound; change each site (where
you see static_cast<Consensus::LLMQType>(request.params[..].getInt<int>()) and
the nDataMask static_cast) to first read the int into a local int (e.g., raw =
request.params[i].getInt<int>()), validate that raw lies within the valid domain
(for LLMQ use the permitted enum range/values returned by Params().GetLLMQ or
explicit min/max of Consensus::LLMQType before casting, and for nDataMask ensure
raw is one of 1,2,3) and only then perform the static_cast and proceed (update
error messages to use RPC_INVALID_PARAMETER when validation fails); apply this
pattern to all listed callsites (including the llmqType occurrences and the
nDataMask handling).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a94b8b19-ab68-4833-9cd1-c79a7f14b4ad
📒 Files selected for processing (12)
doc/release-notes-7099.mdsrc/rpc/client.cppsrc/rpc/evo.cppsrc/rpc/governance.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/rpc/util.cppsrc/rpc/util.hsrc/test/rpc_tests.cpptest/functional/feature_llmq_connections.pytest/functional/feature_llmq_dkgerrors.pytest/functional/feature_llmq_evo.py
💤 Files with no reviewable changes (1)
- src/rpc/util.h
🚧 Files skipped from review as they are similar to previous changes (3)
- src/rpc/masternode.cpp
- test/functional/feature_llmq_dkgerrors.py
- src/rpc/governance.cpp
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Well-structured PR migrating compound RPCs to the client conversion table with a clean deprecation path for permissive boolean parsing. One correctness issue identified: LocaleIndependentAtoi silently accepts trailing non-digit characters in the new string-based block identifier parsing, which should use ParseInt32 for strict validation.
Reviewed commit: 72b6c04
🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/rpc/evo.cpp`:
- [SUGGESTION] lines 1583-1654: LocaleIndependentAtoi silently accepts trailing junk in ParseBlock/ParseBlockIndex
Both `ParseBlock` (line 1590) and `ParseBlockIndex` (line 1649) use `LocaleIndependentAtoi<int>()` for the string-to-height fallback. Unlike `ParseInt32` (which validates that ALL characters are consumed via `ToIntegral`'s `first_nonmatching == end` check), `LocaleIndependentAtoi` only checks for `result_out_of_range` — trailing non-digit characters are silently ignored.
Practical impact: an input like `"123abc"` is silently treated as height 123 instead of producing an error. The more concerning case is in `ParseBlockIndex`, where a valid 64-char hex hash that passes `ParseHashV` but fails `LookupBlockIndex` (unknown hash) falls into the catch block — if that hash starts with decimal digits (e.g., `"12345abcdef..."`) it would be silently interpreted as a height.
The old code used `v.getInt<int>()` which only accepted JSON numbers (strict). Since this PR changes these parameters from `RPCArg::Type::NUM` to `RPCArg::Type::STR` to support hash-or-height input, the string path is new functionality. Using `ParseInt32` instead of `LocaleIndependentAtoi` would provide strict validation consistent with the rest of the codebase.
UdjinM6
dismissed
their stale review
LocaleIndependentAtoi is not restrictive enough
|
develop: this PR: pls consider 1d152e1 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddfb25ee00
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/rpc/evo.cpp (1)
1645-1658:⚠️ Potential issue | 🟠 MajorDon't let the height fallback swallow unknown block hashes.
ParseBlockIndex()wrapsLookupBlockIndex()in the same catch-all asParseHashV(). A syntactically valid but unknown hash therefore gets caught and re-reported as “must be a block hash or chain height”, which changes the message and RPC error behavior seen by clients.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/evo.cpp` around lines 1645 - 1658, The current ParseBlockIndex implementation catches all exceptions from ParseHashV and LookupBlockIndex, causing an unknown-but-valid hash error to be swallowed and reclassified as a height parsing error; update the code so only parse failures fall through to the height-fallback: either move the try/catch to wrap only the ParseHashV call or, if keeping the broader try, detect and rethrow the "Block ... not found" runtime_error from LookupBlockIndex so it is returned unchanged. Refer to ParseBlockIndex, ParseHashV, LookupBlockIndex and chainman.ActiveChain().Height() when making the change.src/rpc/util.cpp (1)
127-143:⚠️ Potential issue | 🟡 MinorHandle non-scalar permissive-bool inputs explicitly.
With
-deprecatedrpc=permissive_bool,null/array/object values still fall through as"", so callers get a misleadingnot ''error instead of a type error.Suggested fix
if (v.isBool()) return v.get_bool(); else if (v.isNum()) strBool = ToString(v.getInt<int>()); else if (v.isStr()) strBool = v.get_str(); + else + throw JSONRPCError(RPC_INVALID_PARAMETER, + strprintf("%s must be a boolean, number, or string (not %s)", + strName, uvTypeName(v.type())));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/util.cpp` around lines 127 - 143, The permissive_bool branch treats non-scalar UniValue (null/array/object) as an empty string and yields a misleading "not ''" error; update the block that checks v (used in the permissive_bool handling) to explicitly detect and reject non-scalar inputs (e.g., v.isNull(), v.isArray(), v.isObject()) before converting to string, and throw a clear JSONRPCError using strName (instead of falling through to ToLower/ToString) so callers get a proper type/invalid-parameter error for null/array/object rather than the empty-string parse message.
🧹 Nitpick comments (1)
src/test/rpc_tests.cpp (1)
532-563: Add one regression for rejected legacy0/1booleans.These assertions cover the new happy path, but the behavior change here is that composite RPC bools are strict by default. One
BOOST_CHECK_THROW(CallRPC("bls generate 1"), std::runtime_error)-style check would pin down the rejection path and catch accidental reintroduction of permissive parsing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/rpc_tests.cpp` around lines 532 - 563, Add regression asserts that numeric legacy booleans are rejected: insert BOOST_CHECK_THROW(CallRPC(std::string("bls generate 1")), std::runtime_error) and BOOST_CHECK_THROW(CallRPC(std::string("bls generate 0")), std::runtime_error) near the other bls generate tests so the CallRPC(...) path is exercised and will fail if permissive parsing is reintroduced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/rpc/evo.cpp`:
- Around line 1590-1595: The numeric-path branch (v.isNum()) sets h from
v.getInt<int>() but skips the bounds check against
chainman.ActiveChain().Height(), allowing invalid heights to be used by
ParseBlock() / ParseBlockIndex(); after assigning h in the v.isNum() branch of
both locations (the block handling around ParseBlock() and the other helper
around ParseBlockIndex()), perform the same validation (h >= 1 && h <=
chainman.ActiveChain().Height()) and throw the identical
std::runtime_error(strprintf("%s must be a block hash or chain height and not
%s", strName, v.getValStr())) when out of range so both numeric and string
inputs are rejected consistently before indexing ActiveChain().
---
Duplicate comments:
In `@src/rpc/evo.cpp`:
- Around line 1645-1658: The current ParseBlockIndex implementation catches all
exceptions from ParseHashV and LookupBlockIndex, causing an unknown-but-valid
hash error to be swallowed and reclassified as a height parsing error; update
the code so only parse failures fall through to the height-fallback: either move
the try/catch to wrap only the ParseHashV call or, if keeping the broader try,
detect and rethrow the "Block ... not found" runtime_error from LookupBlockIndex
so it is returned unchanged. Refer to ParseBlockIndex, ParseHashV,
LookupBlockIndex and chainman.ActiveChain().Height() when making the change.
In `@src/rpc/util.cpp`:
- Around line 127-143: The permissive_bool branch treats non-scalar UniValue
(null/array/object) as an empty string and yields a misleading "not ''" error;
update the block that checks v (used in the permissive_bool handling) to
explicitly detect and reject non-scalar inputs (e.g., v.isNull(), v.isArray(),
v.isObject()) before converting to string, and throw a clear JSONRPCError using
strName (instead of falling through to ToLower/ToString) so callers get a proper
type/invalid-parameter error for null/array/object rather than the empty-string
parse message.
---
Nitpick comments:
In `@src/test/rpc_tests.cpp`:
- Around line 532-563: Add regression asserts that numeric legacy booleans are
rejected: insert BOOST_CHECK_THROW(CallRPC(std::string("bls generate 1")),
std::runtime_error) and BOOST_CHECK_THROW(CallRPC(std::string("bls generate
0")), std::runtime_error) near the other bls generate tests so the CallRPC(...)
path is exercised and will fail if permissive parsing is reintroduced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b224fed8-4748-46d9-ab3d-41cc56c92806
📒 Files selected for processing (7)
doc/release-notes-7099.mdsrc/rpc/client.cppsrc/rpc/evo.cppsrc/rpc/util.cppsrc/rpc/util.hsrc/test/rpc_tests.cpptest/functional/feature_llmq_connections.py
💤 Files with no reviewable changes (1)
- src/rpc/util.h
🚧 Files skipped from review as they are similar to previous changes (2)
- doc/release-notes-7099.md
- test/functional/feature_llmq_connections.py
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
The incremental push correctly addresses the prior finding about trailing-junk acceptance by switching from LocaleIndependentAtoi to ParseInt32. However, the refactored if/else-if structure in both ParseBlock and ParseBlockIndex accidentally moved the height-range validation into the string-only branch. Numeric JSON inputs now bypass bounds checking entirely, creating a null pointer dereference crash for out-of-range heights.
Reviewed commit: ddfb25e
🔴 2 blocking
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/rpc/evo.cpp`:
- [BLOCKING] lines 1590-1596: ParseBlock: height range check skipped for numeric inputs → null deref crash
The range validation `h < 1 || h > chainman.ActiveChain().Height()` is inside the `else if` branch and only executes for string inputs parsed via `ParseInt32()`. When the RPC parameter is a JSON number (`v.isNum() == true`), `h` is assigned via `getInt<int>()` but never bounds-checked. For `h > Height()` or `h < 0`, `CChain::operator[]` returns `nullptr` (see `chain.h:446`), and the subsequent `->phashBlock` dereference crashes the node. For `h == 0` (genesis), this is also a behavioral change from the old code which rejected heights below 1.
The fix is to separate parse validation from range validation: parsing errors throw immediately, but the range check must run unconditionally after both branches.
- [BLOCKING] lines 1653-1659: ParseBlockIndex: identical range-check bypass → null deref or silent misbehavior
Same control-flow bug as `ParseBlock`. When `v.isNum()` is true, `h` bypasses the range check. `chainman.ActiveChain()[h]` returns `nullptr` for out-of-range heights. While `protx_listdiff` happens to check for null downstream, this weakens the contract of `ParseBlockIndex` and the `protx_diff` caller does not guard against null — it would crash.
We cannot simply get rid of `ParseBoolV` because upstream parsing logic is much more stricter, constituting a breaking change. This requires us to therefore follow the deprecation process.
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
The incremental push correctly resolves both prior blocking findings. ParseBlock now separates parse-fail detection (bool fail) from the range check, ensuring the bounds validation h < 1 || h > chainman.ActiveChain().Height() runs unconditionally for both numeric and string inputs. ParseBlockIndex is cleanly refactored to delegate to ParseBlock rather than duplicating the height-parsing logic. The RPC::Type::NUM → STR arg type changes for protx diff/listdiff and evodb_verify/repair are correct and necessary to support the hash-or-height input pattern.
Reviewed commit: 2d5817e
| 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())); | ||
| } |
There was a problem hiding this comment.
Since ParseBlock already handles both hash and height parsing and throws on failure, the try/catch can be dropped:
| 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.
| return false; | ||
| if (!v.isBool()) { | ||
| throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be a JSON boolean. " /* Continued */ | ||
| "Pass -deprecatedrpc=permissive_bool to allow legacy boolean parsing.", strName)); |
There was a problem hiding this comment.
somehow -deprecatedrpc=permissive_bool doesn't work for masternode winners "1"
May be I am doing something wrong?
See 079a5f2
…_bool 02c4ed4 test: expand permissive_bool test coverage (UdjinM6) 00d18f8 test: add functional tests for -deprecatedrpc=permissive_bool (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Follow-up for #7099 ## What was done? Added functional tests for #7099 ## How Has This Been Tested? See new functional tests inside `rpc_deprecated.py` ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 02c4ed4 Tree-SHA512: 758cb89d4f9afa20171084b7ded8054177266539ea7ca0e868ce77ab5b62b151340ec98465632743caa22acd5f61d5d6dca3bb128c9f4887a48d4f43694c8ecf
5 participants
Additional Information
util/std23.h) and helpers (util/helpers.h) #7098Breaking Changes
See release notes.
Checklist