fix: reversed quorum hash

[shumkov]

Summary by CodeRabbit

• New Features

  • Introduced enhanced data types for improved quorum and network security processing.

• Refactor

  • Streamlined code organization and formatting across multiple components.
  • Improved error handling and unified internal type usage for more consistent node and quorum management.

These updates strengthen the system’s maintainability and reliability without altering visible user functionality.

[coderabbitai]

Walkthrough

The changes focus on reorganizing import statements and reformatting function implementations to enhance readability and consistency. New hash types have been introduced in the hash types module, and existing types in the MasternodeListEngine have been updated from BlockHash to BlockCompatibleRawHash. In addition, method parameters and error handling logic have been streamlined, and extraneous print statements from tests have been removed. The improvements maintain the existing functionality while ensuring type safety and clearer code structure.

Changes

File(s)	Change Summary
dash/src/blockdata/transaction/special_transaction/asset_unlock/qualified_asset_unlock.rs dash/src/blockdata/transaction/special_transaction/quorum_commitment.rs	Reordered and reformatted import statements; updated function bodies (size, consensus_decode) using block style; removed extraneous print statements from tests.
dash/src/hash_types.rs	Added new public structures QuorumHash for different hash configurations; introduced new type alias BlockCompatibleRawHash; extended hash encoding via impl_hashencode!; reformatted trait/method bodies for clarity.
dash/src/sml/masternode_list_engine/message_request_verification.rs	Updated method calls to use .as_raw_hash() for parameter conversion; streamlined error handling in verify_chain_lock by adjusting conditional checks.
dash/src/sml/masternode_list_engine/mod.rs dash/src/sml/masternode_list_engine/non_rotated_quorum_construction.rs dash/src/sml/masternode_list_engine/rotated_quorum_construction.rs	Changed struct fields and method signatures to use BlockCompatibleRawHash instead of BlockHash; updated internal logic to convert and handle raw hash values consistently, including in error generation.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant MLE as MasternodeListEngine
    participant Store as BlockHeights Map

    Client->>MLE: feed_block_height(height, block_hash)
    MLE->>MLE: Convert block_hash using to_raw_hash()
    MLE->>Store: Update block_heights[converted_hash] = height
    Client->>MLE: request_quorum_entry_height(quorum_entry)
    MLE->>MLE: Convert quorum_entry.quorum_hash using as_raw_hash()
    MLE->>Store: Retrieve block height for converted_hash
    MLE-->>Client: Return block height or error

Loading

sequenceDiagram
    participant ChainLock
    participant MLE as MasternodeListEngine
    participant Request as VerificationRequest

    ChainLock->>MLE: verify_chain_lock(chain_lock, before, request_id)
    MLE->>MLE: Convert cycle_hash using as_raw_hash()
    MLE->>MLE: Process verification and streamline error handling
    MLE-->>ChainLock: Return verification result

Loading

Suggested reviewers

• QuantumExplorer

Poem

I’m a rabbit in the code meadow,
Hopping through lines where clean winds blow.
Imports neatly gathered in a row,
Quorum hashes and types now imbue a glow.
With each reformatted, bug-free leap,
I celebrate this code—joyful and deep!
🐰✨

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9cb1184 and db0ec75.

📒 Files selected for processing (7)

• dash/src/blockdata/transaction/special_transaction/asset_unlock/qualified_asset_unlock.rs (6 hunks)
• dash/src/blockdata/transaction/special_transaction/quorum_commitment.rs (6 hunks)
• dash/src/hash_types.rs (14 hunks)
• dash/src/sml/masternode_list_engine/message_request_verification.rs (5 hunks)
• dash/src/sml/masternode_list_engine/mod.rs (20 hunks)
• dash/src/sml/masternode_list_engine/non_rotated_quorum_construction.rs (3 hunks)
• dash/src/sml/masternode_list_engine/rotated_quorum_construction.rs (6 hunks)

🔇 Additional comments (52)

dash/src/blockdata/transaction/special_transaction/asset_unlock/qualified_asset_unlock.rs (5)

1-14: Verify Global PR Objective: Fix for Reversed Quorum Hash

Although this file does not directly manipulate quorum hash values, the PR title indicates a fix regarding a reversed quorum hash. Please double-check that any changes to hash byte ordering (if needed) have been applied in the appropriate modules (for example, in the QuorumHash implementation).

---

29-37: Import Reordering for Improved Readability

The reordering of import statements (lines 29–37) rearranges dependencies (e.g. moving SpecialTransactionBasePayloadEncodable, BLSSignature and TransactionPayload) into a clearer logical grouping. This enhances maintainability without altering functionality.

---

67-69: Block-Style Formatting for the size() Method

The size() method now employs a block-style format:

pub fn size(&self) -> usize {
    self.base.size() + self.request_info.size() + 96
}

This improves clarity and consistency with similar methods in the codebase.

---

134-138: Enhanced Readability in consensus_decode

The reformatted return block in the consensus_decode implementation:

Ok(AssetUnlockPayload {
    base,
    request_info,
    quorum_sig,
})

clearly shows the construction of the AssetUnlockPayload and improves readability without impacting logic.

---

228-232: Removal of Debug Print Statements in Tests

The removal (or commenting out) of extraneous print statements in test cases (lines 228–232) cleans up the test output. This is a welcome clean-up that does not affect functionality.

dash/src/blockdata/transaction/special_transaction/quorum_commitment.rs (6)

25-33: Reordering of Import Statements

The repositioning of import statements—including the conditional import for bincode::{Decode, Encode} (lines 31–32)—improves organization and consistency. There are no functional changes.

---

146-150: Verification of quorum_hash Decoding

The quorum_hash is decoded directly using its consensus_decode method:

let quorum_hash = QuorumHash::consensus_decode(r)?;

Given the PR title ("fix: reversed quorum hash"), please verify that the underlying implementation of QuorumHash::consensus_decode properly handles byte order as intended by the consensus rules. Adding a dedicated test for byte order verification could be beneficial.

---

149-153: Clear Block-Style for Conditional Decoding of quorum_index

The refactored block for decoding the optional quorum_index based on the version:

let quorum_index = if version == 2 || version == 4 {
    Some(i16::consensus_decode(r)?)
} else {
    None
};

improves clarity and makes it easier to verify that only the expected versions include a quorum index.

---

194-196: Refined Block-Style Formatting in size() Method

The updated implementation of the size() method in QuorumCommitmentPayload:

pub fn size(&self) -> usize {
    2 + 4 + self.finalization_commitment.size()
}

enhances readability and aligns with similar formatting improvements throughout the codebase.

---

213-218: Consistent Block Formatting in consensus_decode for Payload

The consensus_decode method for QuorumCommitmentPayload now uses a clear block-style return:

Ok(QuorumCommitmentPayload {
    version,
    height,
    finalization_commitment,
})

This change improves code clarity without altering functional behavior.

---

224-297: Comprehensive Test Coverage in Quorum Commitment Module

The test suite in this file (lines 224–297) continues to validate both the consensus encoding/decoding and the serialization round-trip for quorum commitment payloads and entries. These tests ensure that changes in formatting preserve functional behavior.

dash/src/sml/masternode_list_engine/rotated_quorum_construction.rs (5)

31-37: Good fix for quorum hash handling.

The change from directly using quorum.quorum_entry.quorum_hash to using quorum.quorum_entry.quorum_hash.as_raw_hash() when querying block_heights and to_raw_hash().into() for error handling ensures consistent hash orientation throughout the codebase. This matches the PR title's intention to fix reversed quorum hash issues.

---

66-71: Consistent fix applied to another instance.

This change ensures consistent hash handling in the find_rotated_masternodes_for_quorums method, aligning with the previous fix.

---

123-128: Consistent hash handling in required_cl_sig_heights.

The same pattern of using as_raw_hash() for lookups and to_raw_hash().into() for error handling is correctly applied here as well.

---

142-147: Consistent fix for another quorum hash usage.

Good to see the same pattern applied consistently throughout the file.

---

287-289: Improved code formatting.

The match expression is now more consistently formatted with braces, improving readability.

dash/src/sml/masternode_list_engine/message_request_verification.rs (4)

19-22: Good fix for cycle hash lookup.

Using cycle_hash.as_raw_hash() instead of &cycle_hash ensures the hash orientation is correct when accessing rotated_quorums_per_cycle.

---

97-99: Improved comment formatting.

The comment alignment has been improved, making the code more readable.

---

278-280: Streamlined parameter passing.

Changed from passing &before to passing before directly to verify_chain_lock_with_masternode_list, which is correct since the parameter is expected to be passed by value, not by reference.

---

300-303: Improved error handling structure.

Streamlined the conditional logic for error handling, combining related checks which makes the code more readable and maintainable.

dash/src/hash_types.rs (5)

87-93: Good addition of QuorumHash type and BlockCompatibleRawHash alias.

Introducing a dedicated QuorumHash type with the hash_newtype(forward) attribute and a BlockCompatibleRawHash type alias improves type safety and makes the code more explicit about the expected hash types. This is key to fixing the reversed quorum hash issue mentioned in the PR title.

---

100-106: Consistent type definitions for non-x11 feature.

Good to see the same pattern applied for both feature configurations, ensuring consistent behavior regardless of which hash implementation is used.

---

210-211: Added hash encoding for QuorumHash.

Properly including QuorumHash in the impl_hashencode! macro ensures the new type has consistent serialization/deserialization behavior with other hash types.

---

186-189: Improved formatting for PartialOrd implementation.

Using braces instead of a single-line expression makes the code more consistent with the rest of the file.

---

229-266: Consistent method formatting throughout.

Converting single-line expressions to use braces for method bodies makes the code style more consistent. This improves readability without changing the functionality.

dash/src/sml/masternode_list_engine/non_rotated_quorum_construction.rs (4)

1-9: Updated imports to support the new hash types.

Added imports for BlockCompatibleRawHash and BlockHash to support the function signature changes and error handling in this file.

---

14-15: Improved parameter type for hash handling.

Changed the parameter type from &BlockHash to &BlockCompatibleRawHash, which is more appropriate since it's compatible with both quorum and block hashes. This supports the overall goal of fixing reversed hash issues.

---

25-28: Updated error handling for hash type conversion.

The error handling now properly converts the raw hash to a BlockHash using from_raw_hash, ensuring the error message contains the correct hash representation.

---

36-38: Consistent hash access method.

Changed from passing &quorum.quorum_entry.quorum_hash to using quorum.quorum_entry.quorum_hash.as_raw_hash(), which aligns with the changes in other files and ensures consistent hash orientation.

dash/src/sml/masternode_list_engine/mod.rs (23)

9-9: Use of BlockCompatibleRawHash import looks consistent.
No issues with introducing this new hash type.

---

20-20: Importing MasternodeList is appropriate.
This aligns with the usage of MasternodeList throughout the file.

---

25-28: Feature-gated imports are properly isolated.
Using bincode and serde under feature flags is a good practice that keeps optional dependencies cleanly separated.

---

36-36: Switched type to BTreeMap<BlockCompatibleRawHash, CoreBlockHeight>.
This update cleanly matches the design shift to use raw hashes.

---

40-40: rotated_quorums_per_cycle now keyed by raw hash.
Consistent with the new approach to store raw hashes for block references.

---

67-72: default_for_network provides a succinct constructor.
No issues; the code is straightforward and maintainable.

---

83-88: Insertion using to_raw_hash() in initialize_with_diff_to_height().
This consistently tracks block-to-height mapping with the raw hash.

---

122-124: Filtering based on block_heights lookup.
Returning true when the hash is not found is logical for the intended control flow.

---

150-152: Filtering logic for non-rotating quorums.
Again, using as_raw_hash() consistently for lookups.

---

178-180: masternode_list_for_block_hash retrieves height using raw hash.
The approach is correct and aligns with the overall refactor.

---

183-189: feed_block_height now takes a BlockCompatibleRawHash.
Storing both in block_heights and block_hashes ensures bidirectional lookups remain intact.

---

198-198: Callback signature updated to accept &BlockCompatibleRawHash.
No concerns here; this is consistent with the rest of the changes.

---

257-259: Ensuring quorum hash is known before feeding block height.
Good defensive check; maintains consistency with raw hash usage.

---

273-283: request_mn_list_diff_heights applying the same raw-hash pattern.
All updated calls align with the new type strategy and error handling remains intact.

---

356-356: Lookups for mn_list_diff_h and mn_list_diff_tip use raw hashes.
These lines confirm consistent usage of as_raw_hash() for height retrieval.

Also applies to: 360-360

---

409-413: Storing rotating quorums by raw hash.
No issues; preserves the new type approach for quorum references.

---

538-545: Fallback case storing qualified_last_commitment_per_index.
Logic neatly injects rotating quorums under the raw-hash key.

---

600-602: base_block_hash lookup using as_raw_hash().
Matches the approach used throughout for typed-to-raw conversions.

---

614-615: Block hash fallback in apply_diff.
Graceful error handling if the map lookup fails.

---

714-714: Inserting the diff-end height by raw hash.
Continues to keep block_heights and block_hashes synced.

---

765-767: Rotation cycle check now uses as_raw_hash().
Ensures uniform usage for all block-hash lookups.

---

841-841: Imported additional LLMQ types.
The usage is clear and no further concerns identified.

---

879-881: Applying as_raw_hash() in test.
Ensures consistent type usage in the verification test logic.

✨ Finishing Touches

• 📝 Generate Docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.