fix: multiple help string for RPC result #6886
Conversation
- masternode (composite command) - masternode connect - masternode count - masternode winners
Fixed: - `setcoinjoinrounds` - `setcoinjoinamount` - `coinjoinsalt` - composite command `coinjoin`
knst
changed the title
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughThe PR replaces many placeholder RPCResults{} entries with explicit RPCResult descriptors across multiple RPC modules (src/rpc/coinjoin.cpp, evo.cpp, governance.cpp, masternode.cpp, node.cpp, quorums.cpp, rawtransaction.cpp, and src/wallet/rpc/wallet.cpp). Changes declare concrete result types (NONE, BOOL, STR, OBJ, ARR), nested schemas, conditional branches, and ELISION placeholders for detailed fields. An anonymous quorum_sign_result schema was introduced and reused. No function signatures or runtime control flow/logic were modified; only RPCHelpMan result metadata and help/return schemas were updated. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
src/rpc/rawtransaction.cpp (1)
361-373: Use OBJ_DYN for dynamic per-txid resultsThe returned object is a dynamic map keyed by txid. Use OBJ_DYN to accurately document dynamic keys.
- RPCResult{ - RPCResult::Type::OBJ, "", "", + RPCResult{ + RPCResult::Type::OBJ_DYN, "", "", { {"if verbose is not set or set to false", RPCResult::Type::STR_HEX, "txid", "The serialized, hex-encoded data for 'txid'"}, {"if verbose is set to true", RPCResult::Type::OBJ, "txid", "The decoded network-serialized transaction.", { {RPCResult::Type::ELISION, "", "The layout is the same as the output of getrawtransaction."}, }}, {"if tx is unknown", RPCResult::Type::STR, "txid", "None"}, }, },src/rpc/governance.cpp (1)
523-539: Fix typo in vote results description“successfull” → “successful”.
- {RPCResult::Type::STR, "overall", "Total amount of successfull and failed votes"}, + {RPCResult::Type::STR, "overall", "Total amount of successful and failed votes"},src/rpc/quorums.cpp (1)
1109-1110: Nit: fix wording in verifyislock helpUse “InstantSend” instead of “instantsent”.
- RPCResult{RPCResult::Type::BOOL, "", "Returns true if the instantsent is valid"}, + RPCResult{RPCResult::Type::BOOL, "", "Returns true if the InstantSend lock is valid"},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/rpc/coinjoin.cpp(2 hunks)src/rpc/evo.cpp(5 hunks)src/rpc/governance.cpp(14 hunks)src/rpc/masternode.cpp(6 hunks)src/rpc/node.cpp(1 hunks)src/rpc/quorums.cpp(16 hunks)src/rpc/rawtransaction.cpp(1 hunks)src/wallet/rpc/wallet.cpp(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/rpc/node.cppsrc/wallet/rpc/wallet.cppsrc/rpc/rawtransaction.cppsrc/rpc/quorums.cppsrc/rpc/coinjoin.cppsrc/rpc/masternode.cppsrc/rpc/evo.cppsrc/rpc/governance.cpp
🧠 Learnings (1)
📓 Common learnings
Learnt from: knst
PR: dashpay/dash#6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.
| RPCResult::Type::OBJ, "", "", | ||
| { | ||
| {RPCResult::Type::NUM, "baseHeight", "height of base (starting) block"}, | ||
| {RPCResult::Type::NUM, "blockHeight", "height of target (ending) block"}, | ||
| {RPCResult::Type::ARR, "addedMNs", "added masternodes", | ||
| { | ||
| {RPCResult::Type::OBJ, "", "", | ||
| { | ||
| // TODO: list fields of output for RPC help instead ELISION | ||
| {RPCResult::Type::ELISION, "", ""} | ||
| }}, | ||
| }, | ||
| }, | ||
| {RPCResult::Type::ARR, "removedMns", "added masternodes", | ||
| { | ||
| {RPCResult::Type::STR_HEX, "protx", "ProTx of removed masternode"}, | ||
| }, | ||
| }, | ||
| {RPCResult::Type::ARR, "addedMNs", "added masternodes", | ||
| { | ||
| {RPCResult::Type::OBJ, "", "", | ||
| { | ||
| {RPCResult::Type::OBJ, "protx", "ProTx of updated masternode", | ||
| { | ||
| // TODO: list fields of output for RPC help instead ELISION | ||
| {RPCResult::Type::ELISION, "", ""} | ||
| }}, | ||
| }}, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| RPCExamples{""}, |
There was a problem hiding this comment.
Correct protx listdiff help structure
The new help mixes up field names (removedMns vs actual removedMNs) and repeats addedMNs where updatedMNs should be, so callers now see keys that don’t exist. Please align the schema with the actual payload.
Suggested fix:
- {RPCResult::Type::ARR, "removedMns", "added masternodes",
+ {RPCResult::Type::ARR, "removedMNs", "Removed masternodes",
{
{RPCResult::Type::STR_HEX, "protx", "ProTx of removed masternode"},
},
},
- {RPCResult::Type::ARR, "addedMNs", "added masternodes",
+ {RPCResult::Type::ARR, "updatedMNs", "Updated masternodes",
{
- {RPCResult::Type::OBJ, "", "",
- {
- {RPCResult::Type::OBJ, "protx", "ProTx of updated masternode",
- {
- // TODO: list fields of output for RPC help instead ELISION
- {RPCResult::Type::ELISION, "", ""}
- }},
- }},
+ {RPCResult::Type::OBJ_DYN, "", "Keys are ProTx hashes, values are state diffs",
+ {
+ // TODO: document fields emitted by CDeterministicMNStateDiff::ToJson()
+ {RPCResult::Type::ELISION, "", ""}
+ }},
},
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RPCResult::Type::OBJ, "", "", | |
| { | |
| {RPCResult::Type::NUM, "baseHeight", "height of base (starting) block"}, | |
| {RPCResult::Type::NUM, "blockHeight", "height of target (ending) block"}, | |
| {RPCResult::Type::ARR, "addedMNs", "added masternodes", | |
| { | |
| {RPCResult::Type::OBJ, "", "", | |
| { | |
| // TODO: list fields of output for RPC help instead ELISION | |
| {RPCResult::Type::ELISION, "", ""} | |
| }}, | |
| }, | |
| }, | |
| {RPCResult::Type::ARR, "removedMns", "added masternodes", | |
| { | |
| {RPCResult::Type::STR_HEX, "protx", "ProTx of removed masternode"}, | |
| }, | |
| }, | |
| {RPCResult::Type::ARR, "addedMNs", "added masternodes", | |
| { | |
| {RPCResult::Type::OBJ, "", "", | |
| { | |
| {RPCResult::Type::OBJ, "protx", "ProTx of updated masternode", | |
| { | |
| // TODO: list fields of output for RPC help instead ELISION | |
| {RPCResult::Type::ELISION, "", ""} | |
| }}, | |
| }}, | |
| }, | |
| }, | |
| }, | |
| }, | |
| RPCExamples{""}, | |
| RPCResult::Type::OBJ, "", "", | |
| { | |
| {RPCResult::Type::NUM, "baseHeight", "height of base (starting) block"}, | |
| {RPCResult::Type::NUM, "blockHeight", "height of target (ending) block"}, | |
| {RPCResult::Type::ARR, "addedMNs", "added masternodes", | |
| { | |
| {RPCResult::Type::OBJ, "", "", | |
| { | |
| // TODO: list fields of output for RPC help instead ELISION | |
| {RPCResult::Type::ELISION, "", ""} | |
| }}, | |
| }, | |
| }, | |
| {RPCResult::Type::ARR, "removedMNs", "Removed masternodes", | |
| { | |
| {RPCResult::Type::STR_HEX, "protx", "ProTx of removed masternode"}, | |
| }, | |
| }, | |
| {RPCResult::Type::ARR, "updatedMNs", "Updated masternodes", | |
| { | |
| {RPCResult::Type::OBJ_DYN, "", "Keys are ProTx hashes, values are state diffs", | |
| { | |
| // TODO: document fields emitted by CDeterministicMNStateDiff::ToJson() | |
| {RPCResult::Type::ELISION, "", ""} | |
| }}, | |
| }, | |
| }, | |
| }, | |
| }, | |
| RPCExamples{""}, |
🤖 Prompt for AI Agents
In src/rpc/evo.cpp around lines 1658-1690 the RPC result schema for protx
listdiff is wrong: field names/casing and a duplicated "addedMNs" entry should
match the actual payload. Replace the "removedMns" entry with "removedMNs"
(correct casing) and rename the second duplicated "addedMNs" to "updatedMNs";
ensure the array element types match payload (removedMNs: STR_HEX "protx";
updatedMNs: OBJ with a nested OBJ "protx" as shown) and keep ELISION
placeholders only where appropriate so the help output reflects the real keys.
src/rpc/masternode.cpp
Outdated
| {RPCResult::Type::OBJ, "details", "Amount of enabled Masternodes", | ||
| {{RPCResult::Type::OBJ, "", "", | ||
| { | ||
| {RPCResult::Type::OBJ, "regular", "Details for regular masternodes", | ||
| { | ||
| {RPCResult::Type::NUM, "total", "Total amount regular Masternodes"}, | ||
| {RPCResult::Type::NUM, "enabled", "Amount of enabled regular Masternodes"} | ||
| }}, | ||
| {RPCResult::Type::OBJ, "evo", "Details for Evo nodes", | ||
| { | ||
| {RPCResult::Type::NUM, "total", "Total amount Evo nodes"}, | ||
| {RPCResult::Type::NUM, "enabled", "Amount of enabled Evo nodes"} | ||
| }}, | ||
| }}, | ||
| }} |
There was a problem hiding this comment.
Match the documented key name to the actual response
The help now advertises a details object, but the handler still returns detailed. That regression breaks consumers leaning on the new docs.
Please rename the documented key:
- {RPCResult::Type::OBJ, "details", "Amount of enabled Masternodes",
+ {RPCResult::Type::OBJ, "detailed", "Breakdown of masternodes by type",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {RPCResult::Type::OBJ, "details", "Amount of enabled Masternodes", | |
| {{RPCResult::Type::OBJ, "", "", | |
| { | |
| {RPCResult::Type::OBJ, "regular", "Details for regular masternodes", | |
| { | |
| {RPCResult::Type::NUM, "total", "Total amount regular Masternodes"}, | |
| {RPCResult::Type::NUM, "enabled", "Amount of enabled regular Masternodes"} | |
| }}, | |
| {RPCResult::Type::OBJ, "evo", "Details for Evo nodes", | |
| { | |
| {RPCResult::Type::NUM, "total", "Total amount Evo nodes"}, | |
| {RPCResult::Type::NUM, "enabled", "Amount of enabled Evo nodes"} | |
| }}, | |
| }}, | |
| }} | |
| {RPCResult::Type::OBJ, "detailed", "Breakdown of masternodes by type", | |
| {{RPCResult::Type::OBJ, "", "", | |
| { | |
| {RPCResult::Type::OBJ, "regular", "Details for regular masternodes", | |
| { | |
| {RPCResult::Type::NUM, "total", "Total amount regular Masternodes"}, | |
| {RPCResult::Type::NUM, "enabled", "Amount of enabled regular Masternodes"} | |
| }}, | |
| {RPCResult::Type::OBJ, "evo", "Details for Evo nodes", | |
| { | |
| {RPCResult::Type::NUM, "total", "Total amount Evo nodes"}, | |
| {RPCResult::Type::NUM, "enabled", "Amount of enabled Evo nodes"} | |
| }}, | |
| }}, | |
| }} |
🤖 Prompt for AI Agents
In src/rpc/masternode.cpp around lines 95-109, the help JSON documents the key
as "details" but the handler returns "detailed"; update the documented key name
to "detailed" so it matches the actual response (replace "details" with
"detailed" in the RPCResult structure and adjust any associated descriptions to
remain accurate).
src/rpc/evo.cpp
Outdated
| {RPCResult::Type::ARR, "addedMNs", "added masternodes", | ||
| { | ||
| {RPCResult::Type::OBJ, "", "", | ||
| { | ||
| {RPCResult::Type::OBJ, "protx", "ProTx of updated masternode", | ||
| { | ||
| // TODO: list fields of output for RPC help instead ELISION | ||
| {RPCResult::Type::ELISION, "", ""} | ||
| }}, | ||
| }}, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
This is supposed to be updated masternodes? Appears to create a duplicate addedMNs currently?
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/rpc/governance.cpp (3)
46-55: Duplicate: Incorrect description for "mode=all" resultAs flagged in the previous review, the description "The transaction hash in hex" for "mode = all" is incorrect. The RPC returns
govman->ToString(), which is a free-form summary string, not a transaction hash.
83-94: Duplicate: Misaligned result structure and unrelated alternativeAs flagged in the previous review, this RPC help block has two issues:
- Returns a JSON string (
u.write()), not an object with ELISION- Includes an unrelated "for mode = all" alternative when this RPC has no mode parameter
724-735: Duplicate: Incorrect type for error stringAs flagged in the previous review, line 733 should use
RPCResult::Type::STRinstead ofRPCResult::Type::STR_HEXfor the error message, as the implementation returns plain text error strings, not hex-encoded data.
🧹 Nitpick comments (1)
src/rpc/governance.cpp (1)
341-341: Clarify the result descriptionThe description "A string that is serialized, hex-encoded data for the gobject" is misleading. The RPC returns only the governance object hash (line 439:
govobj.GetHash().ToString()), not the full serialized governance object.Apply this diff to clarify the description:
- RPCResult{RPCResult::Type::STR_HEX, "data", "A string that is serialized, hex-encoded data for the gobject"}, + RPCResult{RPCResult::Type::STR_HEX, "hash", "The governance object hash"},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/rpc/governance.cpp(14 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/rpc/governance.cpp
🔇 Additional comments (7)
src/rpc/governance.cpp (7)
119-124: LGTM!The RPCResult structure correctly describes the return value of this RPC, which returns an object with an "Object status" field set to "OK" when validation succeeds.
283-292: LGTM!The RPCResult structure correctly describes an array of governance objects. The use of ELISION for the object fields is appropriate given the complexity of the governance object schema.
549-549: LGTM!Good use of the shared
vote_resultsconstant to provide consistent result documentation acrossgobject vote-manyandgobject vote-aliasRPCs, eliminating duplication.Also applies to: 602-602
781-789: LGTM!The use of ELISION is appropriate here given the complexity of the governance object structure with multiple voting result objects (FundingResult, ValidResult, DeleteResult, EndorsedResult) and numerous fields.
885-890: LGTM!The RPCResult structure correctly describes a map where vote hashes are keys and their human-friendly string representations are values (line 923:
vote.ToString(...)).
956-956: LGTM!Using
RPCResult::Type::NONEis appropriate for the top-levelgobjectcommand, which serves as a dispatcher and always throws an error directing users to valid subcommands.
978-978: LGTM!The RPCResult type and description correctly reflect the implementation, which returns a success message string (line 1044: "Voted successfully") or throws an error.
knst
force-pushed
the
fix-multiple-rpc-result
branch
from
69bd4f7 to
976e4dc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/rpc/masternode.cpp (2)
95-95: Match the documented key name to the actual response.The help documents
detailsbut the handler returnsdetailedat line 139. This mismatch will break API consumers relying on the documented schema.Rename the documented key to match the implementation:
- {RPCResult::Type::OBJ, "details", "Number of enabled Masternodes", + {RPCResult::Type::OBJ, "detailed", "Breakdown of masternodes by type",
264-269: Document dynamic keys correctly.The RPC returns an object with dynamic keys (block heights as strings) mapping to payment strings, not a literal
"height"member. The current schema is misleading.Use
OBJ_DYNto indicate dynamic keys:- RPCResult{ - RPCResult::Type::OBJ, "", "Details about a specific deterministic masternode", - { - {RPCResult::Type::STR, "height", "payment string"} - } - }, + RPCResult{ + RPCResult::Type::OBJ_DYN, "", "Map of block height (as string) to payment information", + { + {RPCResult::Type::STR, "height", "Comma-separated list of payees for the block at this height"} + } + },
🧹 Nitpick comments (1)
src/rpc/quorums.cpp (1)
245-250: Consider adding a TODO comment for consistency.The ELISION placeholder is appropriate for the complex output from
BuildQuorumInfo, but unlike other uses in this file (e.g., lines 301, 309, 313), there's no TODO comment indicating this should be documented fully later.Add a TODO for consistency with other ELISION uses:
{ - {RPCResult::Type::ELISION, "", ""} + // TODO: Document full quorum info structure instead of ELISION + {RPCResult::Type::ELISION, "", ""} },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/rpc/masternode.cpp(6 hunks)src/rpc/quorums.cpp(16 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/rpc/masternode.cppsrc/rpc/quorums.cpp
🔇 Additional comments (12)
src/rpc/masternode.cpp (4)
52-54: LGTM!The RPCResult schema correctly documents the string return value that matches the implementation at line 80.
183-189: LGTM with TODO noted.The ELISION placeholder is appropriate here given the complex output structure. Ensure the TODO is tracked for proper documentation in a future update.
491-491: LGTM!
RPCResult::Type::NONEis appropriate for a help command that doesn't return structured data.
528-534: LGTM with TODO noted.The ELISION placeholder is appropriate for this multi-mode function with varied output schemas. Track the TODO for implementing proper per-mode type validators.
src/rpc/quorums.cpp (8)
538-553: LGTM!The
quorum_sign_resulttype definition cleanly documents the two-branch return pattern (bool when submitting, detailed object when not submitting) and matches the implementation inquorum_sign_helper.
567-567: LGTM!Both
quorum_signandquorum_platformsigncorrectly reference the sharedquorum_sign_resulttype, eliminating duplication and providing consistent documentation.Also applies to: 594-594
288-316: LGTM!The schema appropriately balances explicit documentation of top-level fields with ELISION placeholders (marked with TODOs) for deeply nested structures that need fuller documentation later.
424-434: LGTM!The schema correctly documents the array structure, appropriately uses ELISION with a reference to
quorum infohelp, and explicitly lists the additional fields (isValidMember,memberIndex) added to each object.
779-787: LGTM!The schema now correctly documents both
quorumHashandrecoveryMembersfields, addressing the previous review feedback. The implementation at lines 809-816 matches this schema.
832-832: LGTM!The
RPCResult::Type::NONEat line 832 correctly documents the void return, and the implementation at line 848 appropriately returnsNullUniValue.Also applies to: 848-848
632-632: LGTM!All boolean result definitions (
quorum_verify,quorum_hasrecsig,quorum_isconflicting,quorum_getdata,verifychainlock,verifyislock) correctly document their return types and match their implementations.Also applies to: 686-686, 751-751, 869-869, 1049-1049, 1108-1108
1031-1031: LGTM!
RPCResult::Type::NONEis appropriate for the help command, consistent with the pattern used in other RPC help functions.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/rpc/evo.cpp (1)
1657-1689: Fix field name casing mismatch in help schema.Line 1671 documents the field as
"removedMns"(lowercase 'n'), but the actual implementation at line 1735 uses"removedMNs"(capital 'N'). This inconsistency will mislead users relying on the help output.Apply this diff to fix the casing:
- {RPCResult::Type::ARR, "removedMns", "added masternodes", + {RPCResult::Type::ARR, "removedMNs", "Removed masternodes",Additionally, line 1671's description says "added masternodes" when it should say "removed masternodes".
src/rpc/governance.cpp (2)
87-98: Remove incorrect "mode = all" alternative and fix return type.This RPC does not accept a
modeparameter, so the "for mode = all" alternative is incorrect. Additionally, the implementation returns a JSON string (line 110:u.write()), not an object with ELISION.Apply this diff:
- { - RPCResult{ - RPCResult::Type::OBJ, "", "Deserialized governance object", - { - // TODO: list fields of output for RPC help instead ELISION - {RPCResult::Type::ELISION, "", ""} - } - }, - RPCResult{"for mode = all", - RPCResult::Type::STR, "", "The transaction hash in hex" - }, - }, + RPCResult{ + RPCResult::Type::STR, "", "JSON string of the deserialized governance object" + },
525-540: Fix typo in help text.Line 529 contains "successfull" which should be "successful".
Apply this diff:
- {RPCResult::Type::STR, "overall", "Total amount of successfull and failed votes"}, + {RPCResult::Type::STR, "overall", "Total amount of successful and failed votes"},
🧹 Nitpick comments (3)
src/rpc/evo.cpp (1)
1520-1525: Consider documenting the detailed fields.While the ELISION placeholder is acceptable for this PR, the TODO suggests the full object schema should be documented. The actual return includes fields like
proTxHash,collateralHash,ownerAddress,votingAddress,payoutAddress,state, etc. fromBuildDMNListEntry.src/rpc/governance.cpp (1)
286-295: Consider documenting the governance object fields.While the ELISION placeholder is acceptable, the actual return includes fields from
Governance::Object::ToJson()such asparentHash,revision,time,collateralHash,dataHex, etc.src/rpc/quorums.cpp (1)
245-250: Consider documenting quorum info fields.While ELISION is acceptable,
BuildQuorumInforeturns fields likeheight,type,quorumHash,quorumIndex,minedBlock,members,quorumPublicKey, and optionallysecretKeyShare.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/rpc/evo.cpp(5 hunks)src/rpc/governance.cpp(14 hunks)src/rpc/quorums.cpp(16 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/rpc/evo.cppsrc/rpc/governance.cppsrc/rpc/quorums.cpp
🔇 Additional comments (12)
src/rpc/evo.cpp (3)
1407-1416: LGTM! Result schema correctly documents both return shapes.The help now properly reflects that
protx listreturns either an array of strings (ProTx hashes) whendetailed=false, or an array of objects whendetailed=true.
1788-1788: LGTM! Correct use of NONE type for help command.The
protx_helpcommand always throws an error and never returns a value, soRPCResult::Type::NONEis the appropriate type.
1878-1878: LGTM! Correct use of NONE type for help command.The
bls_helpcommand follows the same pattern asprotx_helpand correctly usesRPCResult::Type::NONE.src/rpc/governance.cpp (4)
46-59: LGTM! Result schema correctly documents both modes.The help now properly distinguishes between
mode=json(returns structured object) andmode=all(returns free-form summary string).
123-128: LGTM! Result schema matches implementation.The help correctly documents that a successful check returns an object with the "Object status" field.
727-738: LGTM! Error result type is correct.The help now correctly uses
RPCResult::Type::STR(notSTR_HEX) for plain text error messages.
177-177: LGTM! Result description is accurate.The help correctly states that the RPC returns the collateral transaction id.
src/rpc/quorums.cpp (5)
288-316: LGTM! Well-structured result schema.The help provides a clear high-level structure with ELISION placeholders for complex nested fields that can be documented later. This is a good improvement over the previous empty
RPCResults{}.
538-553: LGTM! Reusable result schema is well-designed.The anonymous namespace constant
quorum_sign_resultis a good pattern for sharing result schemas betweenquorum_signandquorum_platformsign. The two-branch structure correctly documents the conditional return based on thesubmitparameter.
779-787: LGTM! Result schema now includes recoveryMembers.The help correctly documents both the
quorumHashand therecoveryMembersarray, matching the actual implementation.
632-632: LGTM! Boolean return types are accurate.All the boolean result types correctly match their implementations:
quorum_verify(line 672)quorum_hasrecsig(line 701)quorum_isconflicting(line 766)quorum_getdata(lines 899-901)verifychainlock(lines 1092-1093)verifyislock(lines 1167-1168)Also applies to: 686-686, 751-751, 869-869, 1049-1049, 1108-1108
832-832: LGTM! Correct use of NONE type.The
quorum_dkgsimerrorcommand correctly usesRPCResult::Type::NONEand returnsNullUniValue(line 848), as it performs an action without a meaningful return value.Also applies to: 848-848
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/rpc/governance.cpp (1)
87-98: Fix gobject_deserialize result schemaThis RPC returns
u.write()(a JSON string), yet the help still advertises an object schema and even a “mode = all” alternative that the handler never produces. Update the result toType::STRwith an accurate description and drop the unused alternative so the help matches reality.- { - RPCResult{ - RPCResult::Type::OBJ, "", "Deserialized governance object", - { - // TODO: list fields of output for RPC help instead ELISION - {RPCResult::Type::ELISION, "", ""} - } - }, - RPCResult{"for mode = all", - RPCResult::Type::STR, "", "The transaction hash in hex" - }, - }, + RPCResult{ + RPCResult::Type::STR, "", "JSON string of the deserialized governance object" + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/rpc/governance.cpp(14 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/rpc/governance.cpp
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/rpc/governance.cpp (1)
87-98: gobject_deserialize: Fix result type and remove unrelated alternativeThis RPC returns a JSON string (u.write()), not an object; there is no “mode = all” branch here. Update help to a single STR result and drop the unrelated alternative.
Apply:
- { - RPCResult{ - RPCResult::Type::OBJ, "", "Deserialized governance object", - { - // TODO: list fields of output for RPC help instead ELISION - {RPCResult::Type::ELISION, "", ""} - } - }, - RPCResult{"for mode = all", - RPCResult::Type::STR, "", "The transaction hash in hex" - }, - }, + RPCResult{ + RPCResult::Type::STR, "", "JSON string of the deserialized governance object" + },
🧹 Nitpick comments (2)
src/rpc/governance.cpp (2)
286-295: gobject_list-prepared: Consider documenting object fieldsArray of objects with ELISION is acceptable; documenting key fields (e.g., data hex/string, txid, time) would improve UX.
784-792: gobject get: Replace ELISION with explicit fields for clarityHelp currently hides structure. Consider enumerating top-level fields (with nested vote subtotals) to match the actual response.
Proposed help change:
- { - RPCResult{ - RPCResult::Type::OBJ, "", "", - { - // TODO: list fields of output for RPC help instead ELISION - {RPCResult::Type::ELISION, "", ""} - } - }, - }, + RPCResult{ + RPCResult::Type::OBJ, "", "Governance object", + { + {RPCResult::Type::STR_HEX, "DataHex", "Hex-encoded object data"}, + {RPCResult::Type::STR, "DataString", "Plain-text object data"}, + {RPCResult::Type::STR_HEX, "Hash", "Governance object hash"}, + {RPCResult::Type::STR_HEX, "CollateralHash", "Collateral transaction id (txid)"}, + {RPCResult::Type::NUM, "ObjectType", "Numeric object type"}, + {RPCResult::Type::NUM, "CreationTime", "UNIX timestamp"}, + {RPCResult::Type::STR, "SigningMasternode", /*optional=*/true, "Collateral outpoint if present"}, + {RPCResult::Type::OBJ, "FundingResult", "Funding vote tallies", + { + {RPCResult::Type::NUM, "AbsoluteYesCount", ""}, + {RPCResult::Type::NUM, "YesCount", ""}, + {RPCResult::Type::NUM, "NoCount", ""}, + {RPCResult::Type::NUM, "AbstainCount", ""}, + } + }, + {RPCResult::Type::OBJ, "ValidResult", "Validity vote tallies", + { + {RPCResult::Type::NUM, "AbsoluteYesCount", ""}, + {RPCResult::Type::NUM, "YesCount", ""}, + {RPCResult::Type::NUM, "NoCount", ""}, + {RPCResult::Type::NUM, "AbstainCount", ""}, + } + }, + {RPCResult::Type::OBJ, "DeleteResult", "Deletion vote tallies", + { + {RPCResult::Type::NUM, "AbsoluteYesCount", ""}, + {RPCResult::Type::NUM, "YesCount", ""}, + {RPCResult::Type::NUM, "NoCount", ""}, + {RPCResult::Type::NUM, "AbstainCount", ""}, + } + }, + {RPCResult::Type::OBJ, "EndorsedResult", "Endorsement vote tallies", + { + {RPCResult::Type::NUM, "AbsoluteYesCount", ""}, + {RPCResult::Type::NUM, "YesCount", ""}, + {RPCResult::Type::NUM, "NoCount", ""}, + {RPCResult::Type::NUM, "AbstainCount", ""}, + } + }, + {RPCResult::Type::BOOL, "fLocalValidity", ""}, + {RPCResult::Type::STR, "IsValidReason", ""}, + {RPCResult::Type::BOOL, "fCachedValid", ""}, + {RPCResult::Type::BOOL, "fCachedFunding", ""}, + {RPCResult::Type::BOOL, "fCachedDelete", ""}, + {RPCResult::Type::BOOL, "fCachedEndorsed", ""}, + } + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/rpc/governance.cpp(14 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/rpc/governance.cpp
🔇 Additional comments (10)
src/rpc/governance.cpp (10)
46-59: gobject_count: Help schema matches behaviorJSON vs “all” branches and field types look correct. Good inclusion of “other”.
123-128: gobject_check: Result object matches returned fieldsReturns {"Object status": "OK"} on success; help reflects this accurately.
177-177: gobject_prepare: Return type and description correctedSTR_HEX txid matches tx->GetHash().ToString() return.
525-540: vote_results schema: Reusable and clearGood consolidation; optional errorMessage correctly marked.
552-552: gobject vote-many: Uses shared vote_results schemaConsistent with actual return object from VoteWithMasternodes.
605-605: gobject vote-alias: Uses shared vote_results schemaMatches VoteWithMasternodes return.
727-738: gobject list/diff: Valid/invalid branches documented correctlyOBJ on success and plain STR error path align with implementation.
888-893: gobject getcurrentvotes: Schema matches map of votehash->stringResult type and description align with loop producing vote.ToString(...).
959-959: gobject (dispatcher): NONE result is appropriateHelp for command wrapper returning an error on invalid command is correct.
981-982: voteraw: Return type aligns with success pathPlain string result (“Voted successfully”) matches help.
- gobject (composite command) - gobject check - gobject count - gobject deserialize - gobject get - gobject getcurrentvotes - gobject list - gobject listprepared - gobject prepare - gobject submit - gobject vote-alias - gobject vote-many - gobject voteraw
Fixed RPC: - bls (composite command) - protx (composite command) - protx list - protx listdiff
- quorum (composite command) - quorum dkgstats - quorum info - quorum sign - quorum platformsign - quorum verify - quorum hasrecsig - quorum isconflicting - quorum selectquorum - quorum dkgsimerror - quorum getdata - quorum verifychainlock - quorum verifyislock
Co-authored-by: thephez <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: UdjinM6 <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
knst
force-pushed
the
fix-multiple-rpc-result
branch
from
c353c53 to
690ff3a
Compare
I squashed old fixes; and applied udjin's comments |
Co-authored-by: thephez <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/rpc/evo.cpp (1)
1657-1690: Fix field name casing and updatedMNs structure.Two issues remain unresolved from past reviews:
Field name casing mismatch: Line 1671 documents
"removedMns"but the actual code at line 1735 returns"removedMNs"(capital M and N).Incorrect updatedMNs structure: The help shows a nested object with a
"protx"field, but the actual implementation (lines 1743-1745) creates an array of objects where each object has dynamic keys (ProTx hashes) mapping to state diff values.Apply this diff to fix both issues:
- {RPCResult::Type::ARR, "removedMns", "Removed masternodes", + {RPCResult::Type::ARR, "removedMNs", "Removed masternodes", { {RPCResult::Type::STR_HEX, "protx", "ProTx of removed masternode"}, }, }, {RPCResult::Type::ARR, "updatedMNs", "Updated masternodes", { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::OBJ, "protx", "ProTx of updated masternode", - { - // TODO: list fields of output for RPC help instead ELISION - {RPCResult::Type::ELISION, "", ""} - }}, - }}, + {RPCResult::Type::OBJ_DYN, "", "Keys are ProTx hashes", + { + // TODO: document CDeterministicMNStateDiff::ToJson() fields + {RPCResult::Type::ELISION, "", ""} + }}, }, },src/rpc/masternode.cpp (1)
90-111: Critical: Fix the documented key name to match implementation.The help documentation at line 95 declares the key as
"details", but the implementation at line 139 returns"detailed". This mismatch breaks API consumers relying on the documented schema.Apply this diff:
- {RPCResult::Type::OBJ, "details", "Breakdown of masternodes by type", + {RPCResult::Type::OBJ, "detailed", "Breakdown of masternodes by type",Based on past review comments.
🧹 Nitpick comments (2)
src/rpc/governance.cpp (2)
514-529: Improve vote_results schema accuracy.Two issues with the schema:
Line 518: The description says "Total number of" but the actual value (line 500) is a formatted string like
"Voted successfully 5 time(s) and failed 2 time(s).", not a number.Lines 519-526: The "detail" field schema uses a nested OBJ with field name "protx", but the actual implementation (line 496) uses dynamic keys (
proTxHash.ToString()). This is misleading—the schema should useOBJ_DYNto indicate dynamic keys.Apply this diff:
namespace { const RPCResult vote_results{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR, "overall", "Total number of successful and failed votes"}, - {RPCResult::Type::OBJ, "detail", "Detailed information for each vote", - { - {RPCResult::Type::OBJ, "protx", "ProTx of masternode for voting", - { - {RPCResult::Type::STR, "result", "Result of voting: {success|failed}"}, - {RPCResult::Type::STR, "errorMessage", /*optional=*/true, "Error message if failed"}, - }}, - }}, + {RPCResult::Type::STR, "overall", "Summary of successful and failed votes"}, + {RPCResult::Type::OBJ_DYN, "detail", "Detailed voting results for each masternode (keys are proTxHash values)", + { + {RPCResult::Type::STR, "result", "Voting result: \"success\" or \"failed\""}, + {RPCResult::Type::STR, "errorMessage", /*optional=*/true, "Error message if voting failed"}, + }}, }, }; } // anonymous namespace
716-727: Minor style inconsistency in conditional result descriptions.The conditional result descriptions use capitalized "If" (lines 717, 724), while other conditional results in this file use lowercase (e.g., "if object valid" at line 166, "for mode = json" at line 47). Consider using lowercase for consistency with the rest of the file.
Optional diff for consistency:
{ - RPCResult{"If request is valid", + RPCResult{"if request is valid", RPCResult::Type::OBJ, "hash", "Object details", { // TODO: list fields of output for RPC help instead ELISION {RPCResult::Type::ELISION, "", ""} }, }, - RPCResult{"If request is invalid", + RPCResult{"if request is invalid", RPCResult::Type::STR, "", "Error string" }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/rpc/evo.cpp(5 hunks)src/rpc/governance.cpp(14 hunks)src/rpc/masternode.cpp(6 hunks)src/rpc/quorums.cpp(16 hunks)src/rpc/rawtransaction.cpp(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/rpc/rawtransaction.cpp
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/rpc/quorums.cppsrc/rpc/evo.cppsrc/rpc/governance.cppsrc/rpc/masternode.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build container / Build container
🔇 Additional comments (23)
src/rpc/evo.cpp (4)
1407-1416: LGTM! Conditional result schema correctly documents both output shapes.The schema properly reflects that
protx listreturns an array of strings (ProTx hashes) whendetailed=falseand an array of objects whendetailed=true. The ELISION placeholder appropriately indicates fields to be documented later.
1520-1526: LGTM! ELISION placeholder appropriately marks detailed object for future documentation.The result schema correctly identifies the return type as an object. The ELISION placeholder is appropriate given the complexity of the detailed masternode information returned by this RPC.
1788-1788: LGTM! Type::NONE is appropriate for help commands.The
protx_helpcommand only throws an error to display available commands and doesn't return structured data, soType::NONEis correct.
1878-1878: LGTM! Type::NONE is appropriate for help commands.Consistent with
protx_help, thebls_helpcommand only provides help text and doesn't return structured data, soType::NONEis correct.src/rpc/masternode.cpp (4)
52-54: LGTM!The RPCResult correctly documents the string status return value that matches the implementation at line 80.
183-189: LGTM!The ELISION placeholder is appropriate given the complex and partially dynamic return structure (lines 199-215). The TODO comment correctly flags this as temporary until proper type validators are implemented.
491-491: LGTM!Using
RPCResult::Type::NONEis correct since this help function always throws an error (line 495) and never returns a value.
528-534: LGTM!The ELISION placeholder is appropriate since this function returns different structures depending on the mode parameter (lines 617-701): simple key-value pairs for some modes vs. nested JSON objects for others. The TODO comment correctly indicates this is temporary pending proper type validation.
src/rpc/quorums.cpp (7)
245-250: LGTM: Appropriate use of ELISION for complex outputThe result schema correctly declares the return type as an object. Using
ELISIONis appropriate here sinceBuildQuorumInforeturns a complex, variable structure depending on parameters likeincludeMembersandincludeSkShare.
288-316: Well-structured schema with acknowledged gapsThe result schema provides a clear structure for the DKG status output. The TODO comments at lines 301, 308, and 312 appropriately acknowledge that
ELISIONplaceholders should eventually be replaced with detailed field descriptions.
538-553: Excellent: Reusable two-branch result schemaThe
quorum_sign_resultschema elegantly handles the conditional return type based on thesubmitparameter. Defining it once in an anonymous namespace and reusing it for bothquorum_signandquorum_platformsign(lines 567, 594) demonstrates good design and DRY principles.
779-787: Good: recoveryMembers now properly documentedThe
recoveryMembersfield is now correctly included in the result schema, matching the actual implementation (lines 811-816). This addresses the previous concern noted in past review comments.
632-632: LGTM: Boolean result types correctly declaredAll six functions (
quorum_verify,quorum_hasrecsig,quorum_isconflicting,quorum_getdata,verifychainlock,verifyislock) now correctly declareRPCResult::Type::BOOLas their return type, accurately reflecting their implementations.Also applies to: 686-686, 751-751, 869-869, 1049-1049, 1108-1108
832-832: Correct: NONE result with explicit NullUniValue returnThe result type correctly reflects that this command has no meaningful return value on success. The explicit
return NullUniValue;on line 848 properly handles the success path.Also applies to: 848-848
1031-1031: Correct: NONE result for error-only handlerThe
NONEresult type is appropriate since the handler always throwsJSONRPCError(line 1035) and never returns a value.src/rpc/governance.cpp (8)
46-59: LGTM!The RPCResult definitions correctly describe both output modes. The object schema for json mode accurately lists all returned fields, and the string mode properly indicates it returns a human-friendly summary.
166-166: LGTM!The result type and description correctly match the implementation, which returns the collateral transaction id.
275-284: LGTM!The ARR of OBJ structure correctly describes the list of governance objects returned. The ELISION placeholder with TODO comment is acceptable for deferring detailed field schemas.
333-333: LGTM!The result correctly describes the governance object hash returned by the handler.
541-541: Good reuse of the vote_results schema.Both RPCs correctly reference the shared
vote_resultsschema, which promotes consistency. The handler implementations match the schema structure.Note: This depends on the recommended improvements to the
vote_resultsschema (lines 514-529).Also applies to: 594-594
773-781: LGTM!The OBJ type correctly matches the handler's return value. The ELISION placeholder with TODO comment appropriately defers the detailed field schema.
877-882: LGTM!The
OBJ_DYNtype correctly represents the dynamic vote hash keys, and the description accurately explains the structure. The value schema correctly indicates string representations of votes.
948-948: LGTM!Both result types are correct:
- Line 948:
NONEis appropriate for thegobjectdispatcher command that only throws errors.- Line 970:
STRcorrectly matches the success message returned by the handler.Also applies to: 970-970
| RPCResult{"if object is valid", | ||
| RPCResult::Type::OBJ, "", "", | ||
| { | ||
| {RPCResult::Type::STR, "Object status", "OK"}, | ||
| } | ||
| }, |
There was a problem hiding this comment.
Fix misleading field description.
The description parameter should describe what the field contains, not show the literal value. Line 115 currently has "OK" as the description, but it should explain that this field contains the validation status.
Apply this diff:
{
- {RPCResult::Type::STR, "Object status", "OK"},
+ {RPCResult::Type::STR, "Object status", "Validation status (\"OK\" if valid)"},
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RPCResult{"if object is valid", | |
| RPCResult::Type::OBJ, "", "", | |
| { | |
| {RPCResult::Type::STR, "Object status", "OK"}, | |
| } | |
| }, | |
| RPCResult{"if object is valid", | |
| RPCResult::Type::OBJ, "", "", | |
| { | |
| {RPCResult::Type::STR, "Object status", "Validation status (\"OK\" if valid)"}, | |
| } | |
| }, |
🤖 Prompt for AI Agents
In src/rpc/governance.cpp around lines 112 to 117, the RPCResult entry uses a
misleading description label for the "Object status" field by setting the
description to the literal "OK"; change that description to a brief explanatory
string indicating it contains the object's validation/verification status (for
example "validation status, e.g., 'OK' or an error code") so the description
explains the content rather than showing a sample value.
| RPCResult{ | ||
| RPCResult::Type::OBJ_DYN, "", "Keys are block heights (as strings); values describe the payees for that height", | ||
| { | ||
| {RPCResult::Type::STR, "payee", "Payee for the height"} | ||
| } | ||
| }, |
There was a problem hiding this comment.
Clarify that values are comma-separated payment strings, not single payees.
The description at line 267 says "Payee for the height", but the implementation returns a comma-separated string that can include multiple addresses: the masternode owner payout, operator reward payout (line 233), and superblock payees (lines 241-251). This will mislead API consumers expecting a single payee.
Apply this diff to clarify:
- {RPCResult::Type::STR, "payee", "Payee for the height"}
+ {RPCResult::Type::STR, "payments", "Comma-separated list of payment addresses (masternode payout, operator reward if applicable, and superblock payees if triggered)"}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RPCResult{ | |
| RPCResult::Type::OBJ_DYN, "", "Keys are block heights (as strings); values describe the payees for that height", | |
| { | |
| {RPCResult::Type::STR, "payee", "Payee for the height"} | |
| } | |
| }, | |
| RPCResult{ | |
| RPCResult::Type::OBJ_DYN, "", "Keys are block heights (as strings); values describe the payees for that height", | |
| { | |
| {RPCResult::Type::STR, "payments", "Comma-separated list of payment addresses (masternode payout, operator reward if applicable, and superblock payees if triggered)"} | |
| } | |
| }, |
🤖 Prompt for AI Agents
In src/rpc/masternode.cpp around lines 264 to 269, the RPC result description
currently says "Payee for the height" but the implementation returns a
comma-separated string containing potentially multiple payment entries
(masternode owner payout, operator reward payout, and superblock payees). Update
the RPCResult description to indicate that values are comma-separated payment
strings (e.g., "Comma-separated payment strings: masternode owner payout,
operator reward payout, and any superblock payees") so API consumers understand
multiple payees can be present.
| RPCResult{ | ||
| RPCResult::Type::ARR, "quorums", "", | ||
| { | ||
| {RPCResult::Type::OBJ, "", "Quorum Info", | ||
| { | ||
| {RPCResult::Type::ELISION, "", "See for details help for `quorum info`"}, | ||
| {RPCResult::Type::BOOL, "isValidMember", ""}, | ||
| {RPCResult::Type::NUM, "memberIndex", ""}, | ||
| }}, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Fix grammatical awkwardness in help text
Line 429 has awkward phrasing: "See for details help for `quorum info`"
Apply this diff to improve the grammar:
- {RPCResult::Type::ELISION, "", "See for details help for `quorum info`"},
+ {RPCResult::Type::ELISION, "", "See help for `quorum info` for details"},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RPCResult{ | |
| RPCResult::Type::ARR, "quorums", "", | |
| { | |
| {RPCResult::Type::OBJ, "", "Quorum Info", | |
| { | |
| {RPCResult::Type::ELISION, "", "See for details help for `quorum info`"}, | |
| {RPCResult::Type::BOOL, "isValidMember", ""}, | |
| {RPCResult::Type::NUM, "memberIndex", ""}, | |
| }}, | |
| }, | |
| }, | |
| RPCResult{ | |
| RPCResult::Type::ARR, "quorums", "", | |
| { | |
| {RPCResult::Type::OBJ, "", "Quorum Info", | |
| { | |
| {RPCResult::Type::ELISION, "", "See help for `quorum info` for details"}, | |
| {RPCResult::Type::BOOL, "isValidMember", ""}, | |
| {RPCResult::Type::NUM, "memberIndex", ""}, | |
| }}, | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In src/rpc/quorums.cpp around lines 424 to 434, the ELISION help text "See for
details help for `quorum info`" is grammatically awkward; replace that string
with a corrected phrase such as "See help for details on `quorum info`" (or "See
`quorum info` help for details") so the message reads naturally. Update only the
RPCResult::Type::ELISION value accordingly.
There was a problem hiding this comment.
Best would be:
"See quorum info help for details"
There was a problem hiding this comment.
@thephez Perfect! I'll update the suggestion with your preferred wording:
{RPCResult::Type::ELISION, "", "See quorum info help for details"},This reads more naturally and maintains consistency with how other help text references are typically formatted in the codebase.
4 participants
Issue being fixed or feature implemented
For many dash specific RPC we use stub
RPCResultsinstead real documentation for returned values.It was possible because some important changes from backport bitcoin#20495 has been skipped.
What was done?
Replaced all stub RPCResults{} in RPC code to real help, or map with ELISION inside.
For some RPC replaced ELISION to real description.
How Has This Been Tested?
See PR #6887: done extra testing with backports bitcoin#20495, bitcoin#25161, bitcoin#25170, bitcoin#24408, bitcoin#25237 and fixed all failures in functional tests.
Breaking Changes
N/A
Checklist: