-
-
Notifications
You must be signed in to change notification settings - Fork 353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
checkifmncollateral rpc added #1427
Conversation
WalkthroughA delightful addition has been made to the system, allowing for masternode collateral existence checks. This enhancement includes the implementation of a new function in Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (3)
- src/rpc/misc.cpp (3 hunks)
- src/rpc/server.cpp (1 hunks)
- src/rpc/server.h (1 hunks)
Additional comments not posted (3)
src/rpc/server.h (1)
221-222
: LGTM! The declaration ofcheckifmncollateral
correctly follows the conventions for RPC command functions.src/rpc/server.cpp (1)
347-348
: LGTM! The addition ofcheckifmncollateral
to the RPC commands under the "mobile" category is correctly implemented.src/rpc/misc.cpp (1)
1772-1772
: Ensure the RPC command category aligns with its functionality.The
checkifmncollateral
function is added under the "mobile" category. If this functionality is specifically designed for mobile applications or has broader use, consider placing it in a more appropriate category or providing additional context in the help documentation.
src/rpc/misc.cpp
Outdated
UniValue checkifmncollateral(const JSONRPCRequest& request) | ||
{ | ||
if (request.fHelp || request.params.size() != 2) | ||
throw std::runtime_error( | ||
"checkifmncollateral\n" | ||
"\nReturns bool value.\n" | ||
"\nArguments:\n" | ||
" \"txHash\"\n" | ||
" \"index\"\n" | ||
+ HelpExampleCli("checkifmncollateral", "\"b476ed2b374bb081ea51d111f68f0136252521214e213d119b8dc67b92f5a390\"" "\"0\" ") | ||
+ HelpExampleRpc("checkifmncollateral", "\"b476ed2b374bb081ea51d111f68f0136252521214e213d119b8dc67b92f5a390\"" "\"0\" ") | ||
); | ||
|
||
std::string strTxId; | ||
int index; | ||
|
||
try { | ||
strTxId = request.params[0].get_str(); | ||
index = std::stol(request.params[1].get_str()); | ||
} catch (std::logic_error const & e) { | ||
throw std::runtime_error(std::string("An exception occurred while parsing parameters: ") + e.what()); | ||
} | ||
|
||
uint256 txid = uint256S(strTxId); | ||
|
||
CTransactionRef tx; | ||
uint256 hashBlock; | ||
if(!GetTransaction(txid, tx, Params().GetConsensus(), hashBlock, true)) | ||
throw std::runtime_error("Unable to get the transction"); | ||
|
||
auto mnList = deterministicMNManager->GetListAtChainTip(); | ||
COutPoint o(txid, index); | ||
|
||
bool mnexists = false; | ||
if (deterministicMNManager->IsProTxWithCollateral(tx, index) || mnList.HasMNByCollateral(o)) { | ||
mnexists = true; | ||
} | ||
|
||
return UniValue(mnexists); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for invalid parameters.
The function checkifmncollateral
lacks error handling for cases where the transaction hash is invalid or the index is out of bounds. Consider adding checks to ensure the transaction exists and the index is within the valid range.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/rpc/misc.cpp (3 hunks)
Additional comments not posted (1)
src/rpc/misc.cpp (1)
1434-1468
: Review of the new RPC commandcheckifmncollateral
.
- Error Handling: The function correctly throws an exception if the number of parameters is incorrect or if the parameters cannot be parsed. This is good practice for robust error handling.
- Transaction Retrieval: The function uses
GetTransaction
to retrieve the transaction from the transaction ID provided. It correctly checks if the transaction exists.- Masternode Collateral Check: It checks if the transaction output is used as collateral either directly or in the masternode list at the chain tip. This is done using
IsProTxWithCollateral
andHasMNByCollateral
, which are appropriate for this check.- Return Value: The function returns a boolean indicating if the output is used as collateral, which aligns with the function's purpose.
Overall, the implementation of the
checkifmncollateral
function appears correct and adheres to the expected functionality as described in the PR. It properly integrates with the existing masternode management systems and provides a clear, boolean output for RPC clients.
* checkifmncollateral rpc added * review comments applied
No description provided.