-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add quorum rpc methods to support legacy behavour #62
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
Conversation
WalkthroughThe changes span three modules. In the hash types file, import statements have been reordered and function bodies reformatted for improved readability without modifying logic. The RPC client now updates the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client Code
participant API as RpcApi Implementation
participant Q as Quorum Provider
C->>API: get_quorum_info_reversed(llmq_type, quorum_hash)
API->>Q: get_quorum_info(llmq_type, quorum_hash)
Q-->>API: QuorumInfo
API->>API: Reverse quorum_hash in result
API-->>C: QuorumInfoResult
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🔇 Additional comments (2)
✨ Finishing Touches
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rpc-client/src/client.rs (1)
449-452: Check method naming for clarity
The function name “get_instant_locks” matches returning multiple locks. Implementation calls"getislocks"and returns aVec<String>with no apparent issues.Consider adding a doc comment or clarifying the naming to emphasize that this method fetches multiple locks.
🛑 Comments failed to post (1)
rpc-client/src/client.rs (1)
1421-1426:
⚠️ Potential issueDuplicate or incomplete definition of
get_protx_info
This code block redefines the method in the same scope and references a stray"listdiff"snippet, causing a likely syntax error.Apply the following diff to remove or properly merge the incomplete method:
- /// Returns a returns detailed information about a deterministic masternode - fn get_protx_info( - &self, - protx_hash: &ProTxHash, - block_hash: Option<&BlockHash>, - ) -> Result<json::ProTxInfo> { - let mut args = ["info".into(), into_json(protx_hash.to_hex())?, opt_into_json(block_hash)?]; - self.call::<json::ProTxInfo>("protx", handle_defaults(&mut args, &[null()])) - let mut args = ["listdiff".into(), into_json(base_block)?, into_json(block)?]; - self.call::<json::MasternodeListDiff>( - "protx", - handle_defaults(&mut args, &[null(), null()]), - ) - }Make sure only the valid
get_protx_infoimplementation remains at lines 1439–1446.📝 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./// Returns a diff and a proof between two masternode list fn get_protx_diff(&self, base_block: u32, block: u32) -> Result<json::MasternodeDiff> { let mut args = ["diff".into(), into_json(base_block)?, into_json(block)?]; self.call::<json::MasternodeDiff>("protx", handle_defaults(&mut args, &[null()])) } // Ensure that only the valid `get_protx_info` implementation remains here (lines 1439–1446). // The duplicate/incomplete `get_protx_info` definition (shown below) has been removed: // // - /// Returns a returns detailed information about a deterministic masternode // - fn get_protx_info( // - &self, // - protx_hash: &ProTxHash, // - block_hash: Option<&BlockHash>, // - ) -> Result<json::ProTxInfo> { // - let mut args = ["info".into(), into_json(protx_hash.to_hex())?, opt_into_json(block_hash)?]; // - self.call::<json::ProTxInfo>("protx", handle_defaults(&mut args, &[null()])) // - let mut args = ["listdiff".into(), into_json(base_block)?, into_json(block)?]; // - self.call::<json::MasternodeListDiff>( // - "protx", // - handle_defaults(&mut args, &[null(), null()]), // - ) // - }
#51 fixes wrong QuorumHash (it supposed to reverse from / to hex string conversion). Unfortunately, we have a lot of legacy code that depends on buggy logic. This PR adds additional RPC methods which support legacy behaviour
Close #61
Summary by CodeRabbit
New Features
Tests
These changes improve the functionality and reliability of the RPC interface while ensuring enhanced data processing consistency.