feat: Deterministic Masternode List and Quorum Rotation (DIP24) support #51
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request upgrades multiple CI workflows and refines numerous core components. GitHub Actions versions are bumped from v2 to v4; Cargo files are updated with a new workspace resolver and MSRV changes. In the Dash codebase, many structs now conditionally derive bincode serialization traits, new BLS key conversion utilities are introduced, and additional hash and quorum types and methods are implemented. The SML modules have been reorganized with new masternode list engine and quorum validation functionality. Finally, various macros and dependencies in the hashes module are corrected or extended. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant M as MasternodeListEngine
participant V as Quorum Validator
participant ML as MasternodeList
C->>M: initialize_with_diff_to_height(diff, height, network)
M->>ML: Build/update masternode list from diff
C->>M: feed_qr_info(qr_info, verify_rotated_quorums)
M->>V: Validate quorum entries
V-->>M: Return validation results
M->>M: Apply diff and update state
M-->>C: Return latest masternode list & quorum hashes
Suggested reviewers
Poem
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🔭 Outside diff range comments (3)
dash/src/blockdata/transaction/special_transaction/provider_update_registrar.rs (1)
58-58: 💡 Verification agent🧩 Analysis chain
Address the TODO comment about payload signature length.
The TODO comment indicates uncertainty about the signature length. This should be clarified and documented.
Would you like me to help investigate and document the expected signature length?
🏁 Script executed:
#!/bin/bash # Search for BLS signature length usage in the codebase rg -A 2 'payload_sig.*=.*Vec::new\(\)' || rg -A 2 'payload_sig:.*Vec<u8>'Length of output: 684
Clarify and Document the Expected BLS Signature Length
It appears that the field
payload_sig: Vec<u8>is defined in bothprovider_registration.rsandprovider_update_registrar.rs, with the latter containing a TODO comment about whether the signature is a BLS signature with a length of 96 bytes. Since our current codebase does not definitively document the expected size, please verify against your BLS library or specification. If 96 bytes is correct, update the comment to clearly state that the payload signature should always be 96 bytes; if not, document the accurate expected length and rationale.dash/src/consensus/encode.rs (1)
1485-1485:⚠️ Potential issueFix the unused variable warning.
The pipeline failure indicates that the
bytes_writtenvariable is unused.Apply this diff to fix the warning:
- let bytes_written = write_compact_size(&mut buffer, value).expect("Failed to write"); + write_compact_size(&mut buffer, value).expect("Failed to write");🧰 Tools
🪛 GitHub Actions: Continuous integration
[warning] 1485-1485: unused variable:
bytes_written.dash/src/blockdata/transaction/mod.rs (1)
351-351: 🛠️ Refactor suggestionReplace deprecated usage of
encode_signing_data_to.Line 351 references a soon-to-be-removed API. Update it to
SighashCache::legacy_encode_signing_data_toto avoid warnings:- let _ = self.encode_signing_data_to(&mut engine, input_index, script_pubkey, sighash_u32); + let _ = SighashCache::new(self).legacy_encode_signing_data_to( + &mut engine, + input_index, + script_pubkey, + sighash_u32 + );🧰 Tools
🪛 GitHub Actions: Continuous integration
[warning] 351-351: use of deprecated method
blockdata::transaction::Transaction::encode_signing_data_to: Use SighashCache::legacy_encode_signing_data_to instead.
🧹 Nitpick comments (62)
dash/src/sml/masternode_list/peer_addresses.rs (1)
35-50: Consider optimizing the sorting implementation.The current implementation clones the hash vector and performs multiple BLAKE3 hash computations during sorting. This could be optimized for better performance.
Consider this optimization:
- let registration_transaction_hashes: Vec<_> = self.masternodes.keys().cloned().collect(); - let mut sorted_hashes = registration_transaction_hashes.clone(); - sorted_hashes.sort_by(|hash1, hash2| { - let nonce_le = nonce.to_le_bytes(); - let mut hash1_nonce = hash1.to_byte_array().to_vec(); - hash1_nonce.extend_from_slice(&nonce_le); - let mut hash2_nonce = hash2.to_byte_array().to_vec(); - hash2_nonce.extend_from_slice(&nonce_le); - let hash1_blake = blake3::hash(&hash1_nonce).into(); - let hash2_blake = blake3::hash(&hash2_nonce).into(); - if reverse_cmp_sup(hash1_blake, hash2_blake) { - Ordering::Greater - } else { - Ordering::Less - } - }); + let nonce_le = nonce.to_le_bytes(); + let mut sorted_hashes: Vec<_> = self.masternodes.keys() + .map(|hash| { + let mut hash_nonce = hash.to_byte_array().to_vec(); + hash_nonce.extend_from_slice(&nonce_le); + (hash, blake3::hash(&hash_nonce)) + }) + .collect(); + sorted_hashes.sort_by(|(hash1, blake1), (hash2, blake2)| { + if reverse_cmp_sup((*blake1).into(), (*blake2).into()) { + Ordering::Greater + } else { + Ordering::Less + } + });dash/src/sml/quorum_entry/helpers.rs (1)
24-39: Consider using Result instead of expect for error handling.The implementation is well-structured with efficient capacity pre-allocation and clear documentation. However, the use of
expectfor error handling could be improved.Consider returning a Result instead of panicking:
- pub fn ordering_hash_for_request_id(&self, request_id: [u8; 32]) -> [u8; 32] { + pub fn ordering_hash_for_request_id(&self, request_id: [u8; 32]) -> Result<[u8; 32], std::io::Error> { let llmq_type = VarInt(self.quorum_entry.llmq_type as u64); let mut writer = Vec::with_capacity(llmq_type.len() + 64); // Encode LLMQ type - llmq_type.consensus_encode(&mut writer).expect("Encoding failed"); + llmq_type.consensus_encode(&mut writer)?; // Encode Quorum Hash writer.extend_from_slice(&self.quorum_entry.quorum_hash.to_byte_array()); // Encode Request ID writer.extend_from_slice(&request_id); // Compute double SHA-256 hash - sha256d::Hash::hash(&writer).to_byte_array() + Ok(sha256d::Hash::hash(&writer).to_byte_array()) }dash/src/sml/masternode_list/apply_diff.rs (1)
31-86: Consider adding debug logging for better observability.Adding debug logs would help track the changes being applied to the masternode list, making it easier to diagnose issues in production.
pub fn apply_diff( &self, diff: MnListDiff, diff_end_height: CoreBlockHeight, ) -> Result<MasternodeList, SmlError> { + log::debug!( + "Applying diff with {} deleted and {} new masternodes", + diff.deleted_masternodes.len(), + diff.new_masternodes.len() + ); // Ensure the base block hash matches if self.block_hash != diff.base_block_hash { + log::debug!( + "Base block hash mismatch. Expected: {}, found: {}", + self.block_hash, + diff.base_block_hash + ); return Err(SmlError::BaseBlockHashMismatch { expected: self.block_hash, found: diff.base_block_hash, }); }dash/src/network/message.rs (1)
541-562: Clean up unused imports in test module.Multiple unused imports were detected in the test module. While these don't affect production code, cleaning them up would improve code maintainability.
-use std::net::Ipv4Addr; -use super::message_network::{Reject, RejectReason, VersionMessage}; -use crate::bip152::BlockTransactionsRequest; -use crate::blockdata::block::{self, Block}; -use crate::blockdata::script::ScriptBuf; -use crate::blockdata::transaction::Transaction; -use crate::internal_macros::hex; -use crate::network::address::{AddrV2, AddrV2Message, Address}; -use crate::network::message_blockdata::{GetBlocksMessage, GetHeadersMessage, Inventory}; -use crate::network::message_bloom::{BloomFlags, FilterAdd, FilterLoad}; -use crate::network::message_compact_blocks::{GetBlockTxn, SendCmpct}; -use crate::network::message_filter::{ - CFCheckpt, CFHeaders, CFilter, GetCFCheckpt, GetCFHeaders, GetCFilters, -};🧰 Tools
🪛 GitHub Actions: Continuous integration
[warning] 541-541: unused import:
std::net::Ipv4Addr.
[warning] 548-548: unused imports:
RejectReason,Reject, andVersionMessage.
[warning] 550-550: unused import:
crate::bip152::BlockTransactionsRequest.
[warning] 551-551: unused imports:
Blockandself.
[warning] 552-552: unused import:
crate::blockdata::script::ScriptBuf.
[warning] 553-553: unused import:
crate::blockdata::transaction::Transaction.
[warning] 555-555: unused import:
crate::internal_macros::hex.
[warning] 556-556: unused imports:
AddrV2Message,AddrV2, andAddress.
[warning] 558-558: unused imports:
GetBlocksMessage,GetHeadersMessage, andInventory.
[warning] 559-559: unused imports:
BloomFlags,FilterAdd, andFilterLoad.
[warning] 560-560: unused imports:
GetBlockTxnandSendCmpct.
[warning] 562-562: unused imports:
CFCheckpt,CFHeaders,CFilter,GetCFCheckpt,GetCFHeaders, andGetCFilters.dash/src/sml/quorum_entry/hash.rs (3)
18-23: Consider propagating encoding errors instead of usingexpect.The function could panic if encoding fails. Consider propagating the error using
Resultinstead.-pub fn calculate_entry_hash(&self) -> QuorumEntryHash { +pub fn calculate_entry_hash(&self) -> Result<QuorumEntryHash, std::io::Error> { let mut writer = Vec::new(); - self.consensus_encode(&mut writer).expect("encoding failed"); - QuorumEntryHash::hash(&writer) + self.consensus_encode(&mut writer)?; + Ok(QuorumEntryHash::hash(&writer)) }
40-57: Consider propagating encoding errors instead of usingexpect.The function has multiple points where it could panic if encoding fails. Consider propagating the errors using
Resultinstead.-pub fn commitment_data(&self) -> Vec<u8> { +pub fn commitment_data(&self) -> Result<Vec<u8>, std::io::Error> { let mut buffer: Vec<u8> = Vec::new(); - self.llmq_type.consensus_encode(&mut buffer).expect("encoding failed"); - self.quorum_hash.consensus_encode(&mut buffer).expect("encoding failed"); - write_compact_size(&mut buffer, self.valid_members.len() as u32).expect("encoding failed"); - write_fixed_bitset( + self.llmq_type.consensus_encode(&mut buffer)?; + self.quorum_hash.consensus_encode(&mut buffer)?; + write_compact_size(&mut buffer, self.valid_members.len() as u32)?; + write_fixed_bitset( &mut buffer, self.valid_members.as_slice(), self.valid_members.iter().len(), - ) - .expect("encoding failed"); - self.quorum_public_key.consensus_encode(&mut buffer).expect("encoding failed"); - self.quorum_vvec_hash.consensus_encode(&mut buffer).expect("encoding failed"); + )?; + self.quorum_public_key.consensus_encode(&mut buffer)?; + self.quorum_vvec_hash.consensus_encode(&mut buffer)?; - buffer + Ok(buffer) }
67-70: Consider propagating encoding errors instead of usingexpect.Since this function depends on
commitment_datawhich can panic, it should also handle potential errors.-pub fn calculate_commitment_hash(&self) -> QuorumCommitmentHash { +pub fn calculate_commitment_hash(&self) -> Result<QuorumCommitmentHash, std::io::Error> { - let commitment_data = self.commitment_data(); - QuorumCommitmentHash::hash(&commitment_data) + let commitment_data = self.commitment_data()?; + Ok(QuorumCommitmentHash::hash(&commitment_data)) }dash/src/sml/llmq_type/rotation.rs (3)
4-9: Add documentation for the enum and its variants.Consider adding documentation to explain the purpose of the enum and its variants.
+/// Represents different states of the LLMQ (Long-Living Masternode Quorums). pub enum LLMQQuarterType { + /// Represents the state at height minus 3 cycles. AtHeightMinus3Cycles, + /// Represents the state at height minus 2 cycles. AtHeightMinus2Cycles, + /// Represents the state at height minus 1 cycle. AtHeightMinusCycle, + /// Represents the new state. New, }
11-15: Add documentation and consider explicit lifetime bounds.Consider adding documentation and making the lifetime bounds more explicit.
+/// Represents different types of LLMQ quarter reconstruction. #[derive(Clone, Copy)] -pub enum LLMQQuarterReconstructionType<'a: 'b, 'b> { +pub enum LLMQQuarterReconstructionType<'a, 'b> +where + 'a: 'b, +{ + /// Represents a snapshot reconstruction. Snapshot, + /// Represents a new reconstruction with previous quarters data. New { previous_quarters: [&'b Vec<Vec<&'a QualifiedMasternodeListEntry>>; 3] }, }
17-20: Add documentation for the enum and its variants.Consider adding documentation to explain the purpose of the enum and its variants.
+/// Represents different types of LLMQ quarter usage. pub enum LLMQQuarterUsageType<'a> { + /// Represents a snapshot usage with quorum snapshot data. Snapshot(QuorumSnapshot), + /// Represents a new usage with qualified masternode list entries. New(Vec<Vec<&'a QualifiedMasternodeListEntry>>), }dash/src/sml/llmq_entry_verification.rs (2)
13-18: Add documentation for the enum and its variants.Consider adding documentation to explain the purpose of the enum and its variants.
+/// Represents different statuses for skipping LLMQ entry verification. pub enum LLMQEntryVerificationSkipStatus { + /// Entry was not marked for verification. NotMarkedForVerification, + /// List was missed at the specified block height. MissedList(CoreBlockHeight), + /// Block with the specified hash is unknown. UnknownBlock([u8; 32]), + /// Verification was skipped for another reason specified in the message. OtherContext(String), }
42-48: Add documentation for the enum and its variants.Consider adding documentation to explain the purpose of the enum and its variants.
+/// Represents different statuses of LLMQ entry verification. pub enum LLMQEntryVerificationStatus { #[default] + /// Entry has not been verified yet (default state). Unknown, + /// Entry has been successfully verified. Verified, + /// Entry verification was skipped for a specific reason. Skipped(LLMQEntryVerificationSkipStatus), + /// Entry verification failed with a specific error. Invalid(QuorumValidationError), }dash/Cargo.toml (2)
22-22: Consider avoiding non-essential default features.Adding
bincodeandquorum_validationto the default feature set implicitly opts-in all consumers, even if they don’t need the associated functionality. You might consider leaving them as optional features unless you expect them to be universally required.
71-73: Pin or track the git-based dependency.Relying on a git branch for
bls-signaturescould introduce risk if the branch changes or force-pushes occur. For reproducible builds, consider pinning to a commit hash or using a published release.dash/src/blockdata/transaction/mod.rs (1)
1261-1272: Test could assert round-trip equality.The new test verifies deserialization and serialization for a coinbase transaction in a DML context. Consider adding an assertion that the re-serialized bytes match the original to ensure full fidelity of the round-trip.
dash/src/sml/quorum_validation_error.rs (1)
99-107: Conversion implementations facilitate seamless error chaining.Using
FromforSmlErrorandClientDataRetrievalErrorsimplifies propagation. Check that these conversions preserve sufficient context for debugging.hashes/src/internal_macros.rs (1)
160-174: Borrow decoding is correct, but consider clone overhead.Decoding into a borrowed slice and then converting it via
try_into()is ideal. The final.clone()on theHashis likely cheap, but if performance is critical, consider zero-copy alternatives if feasible.dash/src/sml/llmq_type/mod.rs (1)
14-22: Consider documenting parameter usage more thoroughly.
TheDKGParamsstruct is clear in naming, but it would be helpful to include short doc comments for each field, describing its meaning and valid ranges (e.g. minimum/maximum forbad_votes_threshold).dash/src/sml/quorum_entry/qualified_quorum_entry.rs (1)
31-44: Alternative default verification status.
Currently, the conversion fromQuorumEntrydefaults the status toSkipped(NotMarkedForVerification). Consider introducing a more neutral status like “Unverified” if future logic requires a distinction between truly skipped vs. newly created.dash/src/network/constants.rs (1)
137-175: Refactor repeated hex decode for genesis block.
The repeated decode+reverse blocks for mainnet and testnet might be extracted into a helper function or constant to reduce duplication.- let mut block_hash = - hex::decode("00000bafbc94add7...").expect("expected valid hex"); - block_hash.reverse(); - Some(BlockHash::from_byte_array(block_hash.try_into()...)) + fn parse_and_reverse_hex(hash_str: &str) -> BlockHash { + let mut block_hash = hex::decode(hash_str).expect("expected valid hex"); + block_hash.reverse(); + BlockHash::from_byte_array(block_hash.try_into().expect("expected 32 bytes")) + } ...dash/src/sml/quorum_entry/quorum_modifier_type.rs (3)
63-65: Consider handling encoding errors more gracefully instead of unwrapping.
Although writing to an in-memory buffer typically shouldn't fail, usingunwrap()will panic on rare I/O-related errors. A more robust approach would be returning aResultif those edge cases need handling.-VarInt(*llmq_type as u64).consensus_encode(&mut writer).unwrap(); +VarInt(*llmq_type as u64).consensus_encode(&mut writer) + .map_err(|e| QuorumValidationError::DataEncodableError(e.to_string()))?;Also applies to: 69-73
60-77: Potential duplication for encoding LLMQ type.
Both branches inbuild_llmq_hashencode the LLMQ type in the same way. Consider extracting a helper function to reduce duplication.
110-119: Check logging or instrumentation for missing chain lock signatures.
Whenknown_chain_locks.get(&work_block_hash)is not found, the code correctly returnsQuorumValidationError::RequiredChainLockNotPresent. You might also add logs or metrics here to facilitate debugging in production.dash/src/sml/quorum_entry/validation.rs (1)
42-56: Consider returning an error on failing public key parse.
Currently, the code logs parse failures but still proceeds with fewer public keys. If a critical fraction of keys can’t parse, you might prefer returning an error to avoid misrepresenting the “participating set.”dash/src/sml/masternode_list/is_lock_methods.rs (3)
34-45: Question the use of BTreeMap for ordering.
You build aBTreeMapkeyed by(entry, ordering_hash)only to convert it to aVec<_>and sort again. Directly accumulating into a vector might be more efficient.- let ordered_quorum_map = - quorums_for_is.into_iter().fold(BTreeMap::new(), |mut acc, entry| { - let mut ordering_hash = entry.ordering_hash_for_request_id(request_id); - ordering_hash.reverse(); - acc.insert(entry, ordering_hash); - acc - }); - let mut ordered_quorums: Vec<_> = ordered_quorum_map.into_iter().collect(); + let mut ordered_quorums: Vec<(QualifiedQuorumEntry, [u8; 32])> = + quorums_for_is.into_iter() + .map(|entry| { + let mut ordering_hash = entry.ordering_hash_for_request_id(request_id); + ordering_hash.reverse(); + (entry, ordering_hash) + }) + .collect();
35-35: Unify repetitive hash reversal in ordering logic.
Bothordered_quorums_for_is_lockandquorum_entry_for_lock_request_idreverse the ordering hash. Consider introducing a small helper function (e.g.,reverse_ordering_hash) to reduce code duplication.Also applies to: 92-93
66-67: Ensure cloned quorum references are used cautiously.
Cloning theQualifiedQuorumEntryis convenient for returning ownership. However, if the struct is large or includes non-trivial fields, consider returning a reference or Arc instead to avoid unnecessary copying.dash/src/network/message_qrinfo.rs (4)
14-28: Consider providing a default value forextra_share.
TheGetQRInfostruct documentsextra_shareas optional/false by default. For clarity, you could implement aDefaulttrait or otherwise ensure that outside code benefits from an explicit default behavior.
30-63: Large struct warning forQRInfo.
QRInfohas numerous fields, including optional ones for extended snapshots. This design may become unwieldy if additional fields are introduced in the future. Consider extracting some fields into specialized sub-structs or grouping them in a cohesive manner to enhance maintainability.
206-290: Consider aligning enum representation with decode type.
MNSkipListModeis declared with#[repr(u32)]but is decoded from ani32. For consistency and clarity, you might switch to#[repr(i32)]or decode it as au32, then error on out-of-range values.
292-318: Test coverage could be enhanced.
The test verifies that aQRInfomessage can be deserialized, but it only checks themagicfield. Consider asserting values in the decodedQRInfoto ensure fields match expectations after deserialization.dash/src/sml/masternode_list/mod.rs (1)
38-80:newandwith_merkle_rootsoverlap in functionality.
Both methods support constructing aMasternodeList; one calculates merkle roots, the other accepts them. Consider providing a single constructor that accepts optional merkle roots, or centralizing logic so that the code is less prone to duplication of the root calculation.hashes/src/util.rs (3)
130-131: Repeated example in doc comments.Consider referencing the earlier example or consolidating both to reduce duplication.
330-341: Macro for no-ord newtype.Consider deduplicating logic with
hash_newtype!to streamline maintenance and reduce complexity.
495-520:hash_newtype_struct_no_ord!mirrorshash_newtype_struct.Extracting common code would reduce duplication and simplify future changes.
dash/src/sml/masternode_list/merkle_roots.rs (2)
8-44: Implementation ofmerkle_root_from_hashes.Current approach using floating-point
(0.5 * len as f64).ceil()works, but consider integer division or careful bounds checking to avoid floating-point edge cases in critical code.
146-173:hashes_for_merkle_root.In some future scenario, reintroducing a per-height entry hash function could restore dynamic masternode hashing. For now the static approach is fine.
dash/src/network/message_sml.rs (1)
175-178: Unused variabledeserialized.Consider referencing the
deserializedobject or removing it to avoid warnings.- let deserialized: MnListDiff = + let _deserialized: MnListDiff =or handle it as needed in tests.
🧰 Tools
🪛 GitHub Actions: Continuous integration
[warning] 175-175: unused variable:
deserialized.dash/src/bls_sig_utils.rs (5)
45-53: Validate BLSPublicKey conversion with invalid slices
You map errors toQuorumValidationError::InvalidBLSPublicKey. Confirm that downstream code tests invalid key scenarios. This helps ensure well-formed error handling.
55-63: Similar coverage for reference-based conversion
The same caution applies toTryFrom<&BLSPublicKey>. Test how the code behaves if the byte slice is invalid or truncated, ensuring meaningful error messages surface.
133-147: MultiSignature conversions
This code is nearly identical to the basic signature approach. To reduce duplication and potential for drift, consider a macro or shared function. Otherwise, confirm thorough coverage in tests.
165-179: AggregateSignature conversions
Error-handling mirrors your previous pattern. The suggestion for code reuse or macros also applies.
181-195: AggregateSignature conversions (references)
Again, consider a generalized approach to avoid code duplication. At a minimum, ensure thorough negative tests for clarity.dash/src/hash_types.rs (1)
277-299: Inconsistent docstrings for ConfirmedHashHashedWithProRegTx
The doc says “Create a ConfirmedHash from a string” instead of referencingConfirmedHashHashedWithProRegTx. Rename for clarity. Implementation itself looks fine, but do confirm that yourhash_members_*methods produce the desired combined hash in all cases.dash/src/sml/masternode_list_engine/validation.rs (3)
21-23: Consider returning aResultor logging the validation outcome.Currently, this method calls
validate_quorumand immediately passes any error toupdate_quorum_status. If external callers need to handle errors differently, returning aResultmight be more flexible. Otherwise, confirm thatupdate_quorum_statusfully covers the desired failure-handling logic.
45-80: Potential duplication withvalidate_quorum.This function replicates logic (structure checks, rotating type checks, signer filtering) that partly overlaps with
validate_quorum. Consider refactoring to reduce duplication and maintain a single source of truth for shared validation steps.
82-170: Refactor repetitive validation flow.Much of this function’s code mirrors the checks for structure validation, rotating quorum types, and extracting signers. Consider extracting these shared validations into a helper to reduce complexity.
dash/src/blockdata/transaction/special_transaction/quorum_commitment.rs (1)
69-117: Validate structure thoroughly checks thresholds and zeroed keys.Verifies signers, valid_members, and pubkey/signature fields. Consider also ensuring
signers.len()≥ the threshold for clarity, though the existing checks likely cover the same ground.dash/src/sml/masternode_list_engine/rotated_quorum_construction.rs (5)
1-15: Consider removing or referencing the never-used items.The pipeline logs show warnings about methods such as
find_rotated_masternodes_for_quorumandapply_skip_strategy_of_typenever being used. If these methods are part of future functionality, consider referencing them from existing code or adding tests so they won't be flagged as unused. Otherwise, removing them can help maintain a leaner codebase.🧰 Tools
🪛 GitHub Actions: Continuous integration
[warning] 11-11: methods
masternode_list_and_height_for_block_hash_8_blocks_agoandfind_non_rotated_masternodes_for_quorumare never used.
16-26: Refine doc comments for increased clarity.The doc comments are thorough but can be simplified. For instance, the function’s overview can be summarized in fewer lines, focusing on the core logic. This would help future contributors quickly scan and grasp the function purpose.
44-53: Ensure consistent error handling for unexpected quorum index out of bounds.Here, you rely on
ok_or(...)for indexing the array of rotated members. Double-check whether there's a scenario in which thequorum_indexmight exceed the returned vector length or become negative. If so, more explicit checks could provide clearer diagnostic messages.
56-104: Validate concurrency/parallelization potential.
find_rotated_masternodes_for_quorumsiterates over potentially many quorums and performs repeated map lookups. If performance is a concern, you might parallelize these lookups or cache them in a single pass to reduce time complexity. Ensure thread-safety if concurrency is introduced in the future.
148-198: Ensure reusability by extracting repeated reconstruction logic.There is a repeated pattern of calling
self.quorum_quarter_members_by_reconstruction_type(...)with different intervals: 3c, 2c, c, etc. Extracting this pattern into a helper function could streamline the code and make changes more centralized.dash/src/sml/masternode_list/masternode_helpers.rs (6)
10-16: Enhance documentation formasternode_for.It's not immediately obvious what the returned optional masternode signifies in all cases (e.g., if the entry might be invalid). Mention scenarios and usage patterns for clarity.
18-22: Check for additional constraints on validity.
has_valid_masternodeonly checksis_valid. If future validity checks involve complex logic (like specific states or transitions), consider centralizing them to ensure consistency across the codebase.
27-42: Optimize address matching logic.The code checks IPv4 vs. IPv6 by branching. If this check is performance-critical, consider storing addresses in a uniform representation (e.g., a standard 16-byte array) to reduce branching. Only do so if profiling indicates a bottleneck.
52-57: Clarify naming for 'reversed' hash collections.The methods
reversed_pro_reg_tx_hashes_cloned()andreversed_pro_reg_tx_hashes()simply collect or reference keys, but the phrase “reversed” might be ambiguous: It doesn’t obviously reverse them. If “reversed” is relevant to a known domain detail, add a doc comment clarifying the rationale.
65-75: Consider modularizing custom comparisons.
provider_tx_ordered_hashes()uses a custom comparatorreverse_cmp_sup. If similar logic is expected to be used for other ordering constraints, consider placingreverse_cmp_supin a shared utility or trait.
80-86: Compare entire arrays or partial fields?
compare_masternodes()sorts and compares entire masternode entries. In the future, partial comparisons (by block height or rank) might be needed. Provide a dedicated comparator or a partial-eq function to prevent rewriting or misusing comparison logic.dash/src/sml/masternode_list_engine/mod.rs (3)
26-38: Consider private vs. public field designations inMasternodeListEngine.All fields (e.g.,
block_hashes,block_heights) are markedpub, which can expose them more than necessary. Evaluate whether some fields should be private and exposed through methods for improved encapsulation.
186-206: Guard against large insertion loops inrequest_qr_info_cl_sigs.When processing multiple heights, ensure that memory usage is controlled. If
qr_inforeferences a large number of heights, consider streaming or chunking the operation to avoid storing or processing too much data at once.
208-297: Evaluate repeated diffs application logic.
feed_qr_infocallsapply_diffmultiple times in quick succession. Certain edge cases (e.g., partial success of one diff, then failure of a subsequent diff) might leave the engine in an inconsistent state. Consider adding rollback or a “transactional” approach to keep the engine consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (77)
.github/workflows/fuzz.yml(3 hunks).github/workflows/rust.yml(1 hunks)Cargo.toml(1 hunks)README.md(1 hunks)dash/Cargo.toml(3 hunks)dash/src/blockdata/script/owned.rs(2 hunks)dash/src/blockdata/transaction/mod.rs(4 hunks)dash/src/blockdata/transaction/outpoint.rs(2 hunks)dash/src/blockdata/transaction/special_transaction/asset_lock.rs(2 hunks)dash/src/blockdata/transaction/special_transaction/asset_unlock/qualified_asset_unlock.rs(4 hunks)dash/src/blockdata/transaction/special_transaction/asset_unlock/request_info.rs(2 hunks)dash/src/blockdata/transaction/special_transaction/asset_unlock/unqualified_asset_unlock.rs(2 hunks)dash/src/blockdata/transaction/special_transaction/coinbase.rs(1 hunks)dash/src/blockdata/transaction/special_transaction/mod.rs(2 hunks)dash/src/blockdata/transaction/special_transaction/provider_registration.rs(2 hunks)dash/src/blockdata/transaction/special_transaction/provider_update_registrar.rs(2 hunks)dash/src/blockdata/transaction/special_transaction/provider_update_revocation.rs(2 hunks)dash/src/blockdata/transaction/special_transaction/provider_update_service.rs(2 hunks)dash/src/blockdata/transaction/special_transaction/quorum_commitment.rs(11 hunks)dash/src/blockdata/transaction/txin.rs(2 hunks)dash/src/blockdata/transaction/txout.rs(1 hunks)dash/src/blockdata/witness.rs(2 hunks)dash/src/bls_sig_utils.rs(2 hunks)dash/src/consensus/encode.rs(2 hunks)dash/src/consensus/serde.rs(1 hunks)dash/src/dip9.rs(1 hunks)dash/src/hash_types.rs(6 hunks)dash/src/lib.rs(1 hunks)dash/src/network/constants.rs(3 hunks)dash/src/network/message.rs(5 hunks)dash/src/network/message_qrinfo.rs(1 hunks)dash/src/network/message_sml.rs(5 hunks)dash/src/network/mod.rs(1 hunks)dash/src/sml/error.rs(1 hunks)dash/src/sml/llmq_entry_verification.rs(1 hunks)dash/src/sml/llmq_type/llmq_indexed_hash.rs(1 hunks)dash/src/sml/llmq_type/mod.rs(1 hunks)dash/src/sml/llmq_type/network.rs(1 hunks)dash/src/sml/llmq_type/rotation.rs(1 hunks)dash/src/sml/masternode_list/apply_diff.rs(1 hunks)dash/src/sml/masternode_list/debug_helpers.rs(1 hunks)dash/src/sml/masternode_list/from_diff.rs(1 hunks)dash/src/sml/masternode_list/is_lock_methods.rs(1 hunks)dash/src/sml/masternode_list/masternode_helpers.rs(1 hunks)dash/src/sml/masternode_list/merkle_roots.rs(1 hunks)dash/src/sml/masternode_list/mod.rs(1 hunks)dash/src/sml/masternode_list/peer_addresses.rs(1 hunks)dash/src/sml/masternode_list/quorum_helpers.rs(1 hunks)dash/src/sml/masternode_list/rotated_quorums_info.rs(1 hunks)dash/src/sml/masternode_list/scores_for_quorum.rs(1 hunks)dash/src/sml/masternode_list_engine/mod.rs(1 hunks)dash/src/sml/masternode_list_engine/non_rotated_quorum_construction.rs(1 hunks)dash/src/sml/masternode_list_engine/rotated_quorum_construction.rs(1 hunks)dash/src/sml/masternode_list_engine/validation.rs(1 hunks)dash/src/sml/masternode_list_entry/hash.rs(1 hunks)dash/src/sml/masternode_list_entry/helpers.rs(1 hunks)dash/src/sml/masternode_list_entry/mod.rs(4 hunks)dash/src/sml/masternode_list_entry/qualified_masternode_list_entry.rs(1 hunks)dash/src/sml/masternode_list_entry/score.rs(1 hunks)dash/src/sml/mod.rs(1 hunks)dash/src/sml/order_option.rs(1 hunks)dash/src/sml/quorum_entry/hash.rs(1 hunks)dash/src/sml/quorum_entry/helpers.rs(1 hunks)dash/src/sml/quorum_entry/mod.rs(1 hunks)dash/src/sml/quorum_entry/qualified_quorum_entry.rs(1 hunks)dash/src/sml/quorum_entry/quorum_modifier_type.rs(1 hunks)dash/src/sml/quorum_entry/validation.rs(1 hunks)dash/src/sml/quorum_validation_error.rs(1 hunks)dash/tests/psbt.rs(0 hunks)fuzz/generate-files.sh(3 hunks)hashes/Cargo.toml(1 hunks)hashes/embedded/Cargo.toml(1 hunks)hashes/src/bincode_macros.rs(1 hunks)hashes/src/internal_macros.rs(1 hunks)hashes/src/lib.rs(1 hunks)hashes/src/sha256t.rs(1 hunks)hashes/src/util.rs(5 hunks)
💤 Files with no reviewable changes (1)
- dash/tests/psbt.rs
✅ Files skipped from review due to trivial changes (6)
- dash/src/sml/order_option.rs
- Cargo.toml
- dash/src/sml/quorum_entry/mod.rs
- dash/src/dip9.rs
- hashes/src/sha256t.rs
- hashes/embedded/Cargo.toml
🧰 Additional context used
🪛 GitHub Actions: Continuous integration
dash/src/lib.rs
[warning] 48-48: unexpected cfg condition name: bench.
[warning] 53-53: unexpected cfg condition name: fuzzing.
[warning] 65-65: unexpected cfg condition name: bench.
[warning] 79-79: unexpected cfg condition value: bls-signatures.
[warning] 215-215: unexpected cfg condition name: bench.
[warning] 218-218: unexpected cfg condition name: bench.
dash/src/blockdata/transaction/special_transaction/provider_registration.rs
[warning] 205-205: unused import: hashes::hex::FromHex.
dash/src/blockdata/transaction/mod.rs
[warning] 351-351: use of deprecated method blockdata::transaction::Transaction::encode_signing_data_to: Use SighashCache::legacy_encode_signing_data_to instead.
dash/src/consensus/encode.rs
[warning] 1485-1485: unused variable: bytes_written.
dash/src/sml/masternode_list/debug_helpers.rs
[warning] 175-175: unused variable: deserialized.
dash/src/network/message.rs
[warning] 541-541: unused import: std::net::Ipv4Addr.
[warning] 548-548: unused imports: RejectReason, Reject, and VersionMessage.
[warning] 550-550: unused import: crate::bip152::BlockTransactionsRequest.
[warning] 551-551: unused imports: Block and self.
[warning] 552-552: unused import: crate::blockdata::script::ScriptBuf.
[warning] 553-553: unused import: crate::blockdata::transaction::Transaction.
[warning] 555-555: unused import: crate::internal_macros::hex.
[warning] 556-556: unused imports: AddrV2Message, AddrV2, and Address.
[warning] 558-558: unused imports: GetBlocksMessage, GetHeadersMessage, and Inventory.
[warning] 559-559: unused imports: BloomFlags, FilterAdd, and FilterLoad.
[warning] 560-560: unused imports: GetBlockTxn and SendCmpct.
[warning] 562-562: unused imports: CFCheckpt, CFHeaders, CFilter, GetCFCheckpt, GetCFHeaders, and GetCFilters.
dash/src/sml/masternode_list_engine/mod.rs
[error] 450-450: unresolved import bincode: use of unresolved module or unlinked crate bincode.
[error] 463-463: failed to resolve: use of unresolved module or unlinked crate bincode.
[error] 561-561: failed to resolve: use of unresolved module or unlinked crate bincode.
[error] 582-582: failed to resolve: use of unresolved module or unlinked crate bincode.
dash/src/sml/masternode_list_engine/rotated_quorum_construction.rs
[warning] 11-11: methods masternode_list_and_height_for_block_hash_8_blocks_ago and find_non_rotated_masternodes_for_quorum are never used.
[warning] 27-27: associated items find_rotated_masternodes_for_quorum, find_rotated_masternodes_for_quorums, masternode_list_entry_members_for_rotated_quorum, add_quorum_members_from_quarter, quorum_quarter_members_by_reconstruction_type, and apply_skip_strategy_of_type are never used.
dash/src/network/message_sml.rs
[warning] 175-175: unused variable: deserialized.
🔇 Additional comments (163)
README.md (1)
69-69: LGTM! MSRV update is clear and consistent.The update to MSRV from Rust 1.60 to 1.80 is well-documented and aligns with the changes in CI workflows.
hashes/src/lib.rs (1)
123-123: LGTM! New module for bincode serialization.The addition of the
bincode_macrosmodule is well-placed and aligns with the optional bincode serialization feature.dash/src/lib.rs (1)
212-212: LGTM! Semantic type alias for block heights.The addition of
CoreBlockHeighttype alias provides better semantic meaning for block heights throughout the codebase.dash/src/network/mod.rs (1)
49-49: LGTM! New module for quorum-related info messages.The addition of the
message_qrinfomodule is well-placed among other message modules and correctly gated behind thestdfeature.dash/src/sml/masternode_list_entry/helpers.rs (1)
4-7: LGTM! Clear and well-documented implementation.The method provides a simple way to determine if legacy BLS keys are used based on the version number. The comment clearly explains the logic.
.github/workflows/rust.yml (1)
29-29:❓ Verification inconclusive
Verify compatibility with Rust 1.80.0.
The changes update the minimum supported Rust version (MSRV) to 1.80.0 and the checkout action to v4. Please ensure that:
- All dependencies are compatible with Rust 1.80.0
- No breaking changes are introduced by the new Rust version
Run the following script to check dependency compatibility:
Also applies to: 34-34
🏁 Script executed:
#!/bin/bash # Description: Check dependency compatibility with Rust 1.80.0 # Get all dependencies and their versions cargo metadata --format-version 1 | jq -r '.packages[] | select(.name != "dash") | "\(.name)@\(.version)"' # Check if any dependencies have MSRV > 1.80.0 cargo metadata --format-version 1 | \ jq -r '.packages[] | select(.rust_version != null) | "\(.name)@\(.version): \(.rust_version)"'Length of output: 433
Manual verification required: Cargo environment missing
The compatibility check script did not produce any output because the
cargocommand wasn’t found. Please ensure that the script is executed in an environment with Rust and Cargo installed. Once you have access to a proper environment, verify the following:
- Dependency Compatibility: Confirm that all dependencies support Rust 1.80.0 by checking that none require a higher minimum Rust version.
- Breaking Changes: Ensure that upgrading to Rust 1.80.0 does not introduce any breaking changes.
- Checkout Action Validation: Verify that updating the checkout action to v4 functions as expected.
Re-run the provided script in an environment where Cargo is available, or perform manual checks to validate these points.
dash/src/sml/masternode_list_entry/score.rs (1)
6-14: LGTM! Well-structured implementation with clear documentation.The implementation follows good practices:
- Early returns for invalid conditions
- Clear variable names
- Proper use of Option type
dash/src/blockdata/transaction/special_transaction/provider_update_revocation.rs (1)
39-40: LGTM! Consistent bincode serialization support added.The changes correctly add bincode serialization support through conditional compilation, maintaining consistency with other files in the codebase.
Also applies to: 53-53
dash/src/blockdata/transaction/special_transaction/asset_lock.rs (1)
25-26: LGTM! Consistent bincode serialization support added.The changes correctly add bincode serialization support through conditional compilation, maintaining consistency with other files in the codebase.
Also applies to: 41-41
dash/src/blockdata/transaction/txout.rs (1)
23-24: LGTM! Consistent bincode serialization support added.The changes correctly add bincode serialization support through conditional compilation, maintaining consistency with other files in the codebase.
Also applies to: 33-33
dash/src/blockdata/transaction/txin.rs (1)
23-24:✅ Verification successful
Verify witness field serialization behavior with bincode.
The witness field has a special note about its serialization behavior with the existing Encodable/Decodable traits. Please verify that this special handling is correctly maintained when using bincode serialization.
Also applies to: 35-35, 48-52
🏁 Script executed:
#!/bin/bash # Description: Search for any special handling of witness serialization in bincode-related code # Search for witness-related serialization code rg -A 5 'witness.*serializ|serializ.*witness' # Search for any bincode-specific witness handling ast-grep --pattern 'impl $_ for Witness { $$$ }'Length of output: 20507
Action: Confirm bincode witness serialization behavior
After reviewing the repository and test outputs, it appears that the witness field’s special serialization handling is maintained when bincode serialization is enabled. In particular:
- The conditional import in
dash/src/blockdata/transaction/txin.rs(lines 23–24) correctly brings inbincode::{Decode, Encode}.- The implementations in
dash/src/blockdata/witness.rs(including the custom consensus encode/decode logic) remain intact.- Test cases in files such as
dash/tests/serde.rsvalidate that witness serialization (and deserialization) via bincode produces the expected output, mirroring the special handling already applied in consensus serialization.Please ensure that any edge cases related to the witness field are covered by tests. Otherwise, no additional changes appear necessary.
dash/src/sml/masternode_list/peer_addresses.rs (2)
30-34: LGTM! Well-documented method signature.The method signature and documentation are clear and comprehensive, explaining the purpose, parameters, and behavior of the function.
51-63: LGTM! Efficient filtering and output generation.The implementation correctly:
- Limits the output size using
min- Filters for valid masternodes
- Returns socket addresses for valid entries
dash/src/blockdata/transaction/outpoint.rs (2)
27-28: LGTM! Clean bincode feature imports.The imports are correctly guarded by the
bincodefeature flag.
37-37: LGTM! Proper bincode trait derivation.The
EncodeandDecodetraits are correctly derived with feature flag protection.dash/src/blockdata/transaction/special_transaction/asset_unlock/request_info.rs (2)
23-24: LGTM! Clean bincode feature imports.The imports are correctly guarded by the
bincodefeature flag.
35-35: LGTM! Proper bincode trait derivation.The
EncodeandDecodetraits are correctly derived with feature flag protection, maintaining compatibility with existing serde attributes.dash/src/consensus/serde.rs (1)
17-17: LGTM! Clean import statement.Removed redundant Error import as it's already available through the ConsensusError alias.
dash/src/blockdata/witness.rs (1)
11-12: LGTM! Clean addition of bincode serialization support.The changes correctly add bincode serialization support to the
Witnessstruct while preserving existing functionality. The conditional compilation ensures the feature is only available when explicitly enabled.Also applies to: 34-34
dash/src/blockdata/transaction/special_transaction/coinbase.rs (1)
21-22: LGTM! Clean addition of bincode serialization support.The changes correctly add bincode serialization support to the
CoinbasePayloadstruct while preserving existing functionality. The conditional compilation ensures the feature is only available when explicitly enabled.Also applies to: 28-28, 35-35
hashes/Cargo.toml (2)
30-30: LGTM! Fixed hyphenation in dependency flags.The changes correctly update
default_featurestodefault-featuresto align with Cargo's preferred format.Also applies to: 37-37
41-41:❓ Verification inconclusive
Verify the stability of bincode release candidate.
The bincode dependency is using a release candidate version (2.0.0-rc.3). While this is acceptable for development, consider the implications of using a pre-release version in production.
🌐 Web query:
What is the latest stable version of bincode?💡 Result:
The latest stable version of Bincode as of February 2025 remains v1.3.3, while its prerelease v2.x series has reached v2.0.0-rc.3 under active development[1][2][3].
Key details:
- v1.x (stable): Last released in mid-2021[5], maintained separately from ongoing v2 development[3].
- v2.x (prerelease):
- Latest release:
v2.0.0-rc.3with improved error handling and performance optimizations[2][4].- Breaking changes: Requires explicit configuration via
Configurationstruct[4][6].- Features: Native serde support through compatibility modules[7], improved cross-platform testing[8].
For production use requiring stability, v1.x remains recommended until v2 reaches full stability[3][5]. Developers wanting modern features can use v2 via explicit version pinning (
bincode = "=2.0.0-rc.3")[1][4].Citations:
- 1: https://lib.rs/crates/redb-bincode
- 2: https://github.com/bincode-org/bincode/releases
- 3: https://github.com/bincode-org/bincode
- 4: https://docs.rs/bincode/
- 5: https://rust.libhunt.com/compare-bincode-vs-serde
- 6: https://crates.io/crates/bitcode/0.3.4
- 7: https://crates.io/crates/bitcode/0.2.1
- 8: https://en.wikipedia.org/wiki/CURL
Subject: Reevaluate bincode Release Candidate Usage
- In
hashes/Cargo.toml(line 41) the dependency is declared as:bincode = { version= "2.0.0-rc.3", optional = true }- As of February 2025, the latest stable release of bincode is v1.3.3, while v2.0.0-rc.3 remains a prerelease.
- Using a prerelease version is suitable if you specifically require the improved error handling, performance enhancements, or new features provided in the v2 series. However, for production environments where stability is paramount, it is worth considering pinning to the stable v1.x series.
- Please confirm whether the benefits of v2.0.0-rc.3 justify its use over the more stable v1.3.3, and update the dependency version accordingly or document the decision if the prerelease is necessary.
.github/workflows/fuzz.yml (2)
42-43: LGTM! Updated GitHub Actions to latest versions.The changes correctly update the GitHub Actions from v2 to v4, which brings the latest features and security improvements.
Also applies to: 59-59, 69-70
53-53:❓ Verification inconclusive
Verify compatibility with updated Rust toolchain.
The Rust toolchain has been updated from 1.65 to 1.80. This is a significant version bump that might introduce breaking changes.
🌐 Web query:
What are the breaking changes between Rust 1.65 and 1.80?💡 Result:
Rust versions between 1.65 (2022) and 1.80 (2024) introduced several notable backward-incompatible changes affecting type inference, standard library APIs, and language syntax:
Type System Changes
- Type inference breakages
Rust 1.80 introduced ambiguity in type inference due to new standard library implementations (e.g.,FromIterator for Box<str>), breaking crates relying on prior unambiguous inference[1][3][8]. This notably affected older versions of the widely usedtimecrate[3][8].Standard Library
- Concurrency primitives
std::sync::ReentrantLockGuard<T>lost itsSynctrait implementation ifT: !Sync, impacting thread-safety assumptions for types likeStdoutLockandStderrLock[8].- WASI target renaming
Thewasm32-wasitarget was replaced bywasm32-wasip1, requiring migration for WASI-compatible projects[6][7].Language Syntax
- Exclusive range patterns
1.80 stabilized exclusive ranges (a..b) in pattern matching, replacing older workarounds with separate constants for inclusive endpoints[5][8]. Example:match n { ..K => "", // Now valid K..M => "k", // Replaces K..=M-1 // ... }Other Breaking Adjustments
- Deprecated target removal
Thewasm32-wasitarget alias was phased out with warnings starting in Rust 1.81[6].- Panic hook type renaming
std::panic::PanicInfobecame an alias forstd::panic::PanicHookInfo, with deprecation warnings introduced in later versions[7].These changes reflect Rust’s evolving balance between stability and necessary improvements—type inference adjustments highlight unavoidable trade-offs when enhancing compiler capabilities[1][3]. Developers are advised to monitor future-incompat warnings and update dependencies proactively when upgrading toolchains[3][8].
Citations:
- 1: https://users.rust-lang.org/t/understanding-the-rust-1-80-backward-incompatibility/116752
- 2: https://www.ncameron.org/blog/rust-through-the-ages/
- 3: https://internals.rust-lang.org/t/type-inference-breakage-in-1-80-has-not-been-handled-well/21374
- 4: https://blog.rust-lang.org
- 5: https://blog.rust-lang.org/2024/07/25/Rust-1.80.0.html
- 6: https://github.com/rust-lang/rust/blob/master/RELEASES.md
- 7: https://doc.rust-lang.org/beta/releases.html
- 8: https://releases.rs/docs/1.80.0/
- 9: qarmin/czkawka#1341
- 10: https://releases.rs
Ensure Compatibility with Rust 1.80
The workflow now specifies
toolchain: 1.80in.github/workflows/fuzz.yml(line 53). Since updating from Rust 1.65 to 1.80 introduces several breaking changes—such as adjustments to type inference (e.g., issues related toFromIterator for Box<str>), modifications in standard library APIs (e.g., concurrency primitives and target renamings), and syntax improvements in range patterns—it’s important to verify that these changes do not adversely affect our project. Please double-check that:
- The updated toolchain doesn’t break type inference or cause unexpected behavior in parts of the code relying on previous Rust semantics.
- Any dependencies or build scripts aren’t affected by the changes in standard library APIs.
- CI and integration tests pass without compatibility issues.
dash/src/sml/llmq_type/llmq_indexed_hash.rs (3)
1-5: LGTM! Well-structured struct definition.The struct is well-designed with appropriate field types and derives all the necessary traits for hash comparison and manipulation.
7-9: LGTM! Clean constructor implementation.The constructor is concise and follows Rust's idiomatic style.
11-16: LGTM! Useful conversion implementations.The From trait implementations provide convenient ways to create LLMQIndexedHash from different tuple formats, with proper type conversion from usize to u32.
dash/src/blockdata/transaction/special_transaction/provider_update_service.rs (1)
37-38: LGTM! Clean bincode feature implementation.The conditional implementation of bincode serialization is well-structured and consistent with the codebase's approach to feature flags.
Also applies to: 52-52
dash/src/blockdata/script/owned.rs (1)
7-8: LGTM! Clean bincode feature implementation.The conditional implementation of bincode serialization is well-structured and consistent with the codebase's approach to feature flags.
Also applies to: 31-31
dash/src/blockdata/transaction/special_transaction/mod.rs (2)
26-27: LGTM! Clean bincode feature integration.The conditional import of bincode traits is well-structured.
64-64: LGTM! Proper conditional attribute usage.The conditional attribute for bincode serialization is correctly applied to the enum.
dash/src/sml/mod.rs (1)
2-10: LGTM! Well-structured module organization.The new module structure improves code organization by:
- Separating error handling into its own module
- Isolating LLMQ (Long-Living Masternode Quorum) related functionality
- Breaking down masternode list functionality into focused components
hashes/src/bincode_macros.rs (2)
1-37: LGTM! Well-implemented bincode serialization macro.The macro provides a clean and type-safe implementation for bincode serialization:
- Proper handling of generic parameters
- Efficient encoding using byte arrays
- Robust error handling in decoding
- Type safety checks in borrow_decode
39-45: LGTM! Clean empty implementation for disabled feature.The empty macro implementation when bincode is disabled prevents compilation errors.
dash/src/blockdata/transaction/special_transaction/provider_update_registrar.rs (1)
32-33: LGTM! Consistent bincode integration.The bincode serialization support is added consistently with other files.
Also applies to: 47-47
fuzz/generate-files.sh (1)
66-67:✅ Verification successful
LGTM! GitHub Actions versions have been updated to v4.
The changes upgrade the GitHub Actions versions from v2 to v4, which is a good practice to ensure the latest features and security updates.
Let's verify that v4 is indeed the latest version for these actions:
Also applies to: 83-83, 93-94
🌐 Web query:
What is the latest version of actions/checkout, actions/cache, actions/upload-artifact, and actions/download-artifact?💡 Result:
The latest versions of key GitHub Actions as of February 2025 are:
Core Actions Versions
Action Latest Version Key Details actions/checkout v4Default version for repository cloning[5][9], supports Node16 runtime[1] actions/cache v4Integrates with new cache service APIs[10], required upgrade by Feb 2025[2] actions/upload-artifact v410x faster uploads with immediate API availability[7] actions/download-artifact v4Supports cross-repo/workflow downloads[7][8] Key upgrade considerations:
- Cache v3/v2 users must migrate to v4 before Feb 2025 due to backend service sunset[2][10]
- Artifact actions (upload/download) gained URL outputs and real-time availability in v4[7]
- Checkout v3+ uses Node16 instead of deprecated Node12[1]
For code implementation:
steps: - uses: actions/checkout@v4 # Required first step for repo access[5][9] - uses: actions/cache@v4 # With new cache key patterns[6][10] with: path: ~/.npm key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }} - uses: actions/upload-artifact@v4 # After build steps[7] with: name: build-output path: dist/ - uses: actions/download-artifact@v4 # For cross-job sharing[8] with: name: build-outputCitations:
- 1: https://blog.eidinger.info/why-and-how-to-adopt-actionscheckoutv3-in-your-github-action-workflow
- 2: actions/toolkit#1890
- 3: https://github.com/actions/upload-pages-artifact
- 4: https://github.com/marketplace/actions/artifact-downloader
- 5: https://spacelift.io/blog/github-actions-checkout
- 6: https://graphite.dev/guides/github-actions-caching
- 7: https://github.blog/news-insights/product-news/get-started-with-v4-of-github-actions-artifacts/
- 8: https://docs.github.com/actions/using-workflows/storing-workflow-data-as-artifacts
- 9: https://graphite.dev/guides/github-actions-checkout
- 10: https://github.com/actions/cache
GitHub Actions Versions Updated Correctly to v4
The web query confirms that all core GitHub Actions—checkout, cache, upload-artifact, and download-artifact—are now at their latest version, v4, as of February 2025. The update in
fuzz/generate-files.sh(lines 66-67, 83, and 93-94) correctly reflects this upgrade, ensuring the latest features and security updates are leveraged.
- Verified that
actions/checkout@v4andactions/cache@v4are current.- Confirmed that artifact actions (upload/download) should also use v4.
LGTM!
dash/src/blockdata/transaction/special_transaction/asset_unlock/unqualified_asset_unlock.rs (1)
25-26: LGTM! Conditional bincode serialization support added.The changes correctly implement conditional compilation for bincode serialization using feature flags. This is a clean way to make serialization optional while maintaining backward compatibility.
Also applies to: 39-39
dash/src/sml/masternode_list/apply_diff.rs (1)
31-86: LGTM! Well-structured implementation ofapply_diff.The implementation is clean, efficient, and handles all cases properly. The code follows a logical flow with clear error handling and proper cloning of data structures.
dash/src/network/message.rs (1)
33-33: LGTM! Clean integration of QR info message types.The new message types are well-integrated into the existing network message framework, maintaining consistency with the established patterns for command strings, serialization, and deserialization.
Also applies to: 244-247, 303-304, 390-391, 523-527
dash/src/blockdata/transaction/special_transaction/provider_registration.rs (2)
37-38: LGTM!The bincode import is correctly guarded by the feature flag and only imports the necessary traits.
63-63: LGTM!The bincode derive attribute is correctly guarded by the feature flag and derives the necessary traits.
dash/src/sml/masternode_list_entry/mod.rs (9)
1-4: Modules look well-organized.Defining separate modules for hash, helpers, qualified masternode entries, and score helps maintain clarity and modularity. No immediate concerns.
12-12: Good import forConfirmedHash.Including
ConfirmedHashis consistent with the newly introduced optionalconfirmed_hashfield.
17-19: Conditional derivations are appropriate.The
#[cfg_attr(feature = "bincode", ...)]and#[cfg_attr(feature = "serde", ...)]attributes properly gate encoding/decoding under respective features.
31-31: Variant tag widened tou16.Writing
0u16and1u16for enum tags is fine if you anticipate more than 256 variants. Confirm backward compatibility with existing data if this was a breaking change fromu8.Would you like a script to search for any usage assumptions about the old
u8encoding?Also applies to: 36-36
75-77: Introducing optionalconfirmed_hashfield.Changing
confirmed_hashtoOption<ConfirmedHash>adds flexibility but may require extra checks in code relying on a guaranteed value. Ensure that downstream logic accounts forNone.Also applies to: 81-81
89-89: DerivingOrdandPartialOrd.Sorting
MasternodeListEntrybypro_reg_tx_hashis straightforward. Make sure other fields don’t need to be part of the sort criteria. Thebincodefeature gating is consistent with the rest.Also applies to: 91-92, 95-96, 99-100
108-112: Conditional encoding ofNoneas zeroed bytes.Encoding
[0; 32]forNoneand the actual hash forSome(confirmed_hash)is consistent and clear.
117-119: Version check formn_type.Conditionally encoding
mn_typewhenversion >= 2appears logical. Verify older nodes can handle an absence of the field if they encounter version < 2.
128-130: DecodingOption<ConfirmedHash>.The zero-check when decoding aligns well with the encoding logic. This approach ensures backwards compatibility with previously zeroed fields.
dash/src/sml/masternode_list/rotated_quorums_info.rs (1)
1-56: Validate reversed hash usage and lifetime considerations.The new
usage_infomethod effectively classifies used vs. unused masternodes. However, confirm that reversing thepro_reg_tx_hash(line 25) is correct for your endianness or domain logic. Also verify that returning references to these ephemeral vectors is acceptable for your design.dash/src/blockdata/transaction/special_transaction/asset_unlock/qualified_asset_unlock.rs (4)
25-26: Conditional bincode support.Enabling
Encode/Decodeunderbincodefeature is consistent with the rest of the codebase.
51-51: Deriving encode/decode forAssetUnlockPayload.Good approach, ensures consistent serialization when the feature is active.
188-188: Reversingquorum_hash.Reversing the
quorum_hashin tests ensures consistency with on-chain or internal endianness. Just confirm it matches the intended protocol format.Also applies to: 203-203
208-208: Hex-encoded payload output.Switching to
hex::encodefor serialized payload is clear and convenient for debugging or comparisons. No issues found.Also applies to: 210-210
dash/src/consensus/encode.rs (2)
49-50: LGTM!The new imports are correctly organized and related to the new implementations.
683-684: LGTM!The
impl_vec!macro is correctly used to generate serialization code for new types:
u16andu32for basic numeric typesQuorumEntryandQuorumCLSigObjectfor quorum-related typesMnListDiffandQuorumSnapshotfor masternode list typesAlso applies to: 691-692, 698-699
dash/src/sml/masternode_list/debug_helpers.rs (2)
6-25: LGTM!The
Debugimplementation is well-structured and handles optional fields properly usingunwrap_orwith appropriate default values.
27-52: LGTM!The helper methods are well-implemented:
short_descriptionprovides a clear, formatted overview of the masternode listquorums_short_descriptionefficiently builds the quorum description usingfold- The string formatting uses proper indentation for readability
dash/src/sml/error.rs (1)
7-43: LGTM!The
SmlErrorenum is well-designed:
- Comprehensive error variants with descriptive messages
- Appropriate trait derivations including
Error,Debug,Clone, etc.- Correct use of conditional compilation for optional features
- Clear documentation for each variant
dash/src/sml/llmq_type/network.rs (1)
4-40: LGTM!The
Networkimplementation is well-structured:
- Clear mapping of network variants to LLMQ types
- Consistent pattern matching across all methods
- Logical grouping of related functionality (
is_llmq_type,isd_llmq_type,chain_locks_type,platform_type)dash/Cargo.toml (2)
33-34: Validate that new features and dependencies won’t cause conflicts.The
quorum_validationfeature depends onbls, andbincodedepends on both a direct bincode dependency anddashcore_hashes/bincode. Ensure these dependencies do not cause version conflicts or duplication in consumer projects.
66-66:❓ Verification inconclusive
Confirm downgrading of
blsfulto3.0.0-pre6.Switching from
3.0.0-pre8to3.0.0-pre6might revert fixes or features. Verify that this version is stable and doesn’t introduce regressions.
🏁 Script executed:
#!/bin/bash # Checking known security advisories and differences for `blsful` or relevant crates. cargo auditLength of output: 109
Manual verification required for
blsfuldowngradeThe current downgrade from
3.0.0-pre8to3.0.0-pre6raises concerns about potential regressions. Since the automatedcargo auditcheck did not run in the current environment (thecargocommand was not found), please verify manually that this version is stable and does not reintroduce issues.
- File:
dash/Cargo.toml, Line: 66- Action Required: Run
cargo auditlocally (or in an environment where Cargo is available) to check for advisories and ensure that no critical fixes or features are lost with this version downgrade.dash/src/blockdata/transaction/mod.rs (2)
37-38: Import for bincode is straightforward.This conditional import is consistent with adding bincode-based serialization logic. No immediate issues detected.
163-164: Conditional bincode derivation looks good.Deriving
Encode/Decodeunder thebincodefeature cleanly implements optional serialization support.dash/src/sml/quorum_validation_error.rs (3)
1-2: Imports for bincode are aligned with the new feature.Conditionally importing
Decode/Encodeallows the type to remain flexible across builds.
10-20:ClientDataRetrievalErrorenumeration is well-structured.It neatly represents critical missing-block and missing-coinbase scenarios.
22-98: Broad error coverage inQuorumValidationError.The enum captures a wide range of quorum validation failures. Ensure each variant is well-tested to confirm error handling covers all edge cases.
dash/src/sml/masternode_list_engine/non_rotated_quorum_construction.rs (3)
1-9: Imports look good.All imported items appear relevant for the functionality implemented below. No obvious redundancies.
10-26: Check behavior for low block heights.Using
height.saturating_sub(8)avoids underflow, but it silently defaults to0ifheight < 8, which might be incorrect if you expect an error for insufficient history. Ensure this is the intended behavior, and consider returning an error ifheight < 8.
28-46: Implementation appears sound.The logic to retrieve the masternode list, compute a modifier type, and filter valid masternodes is clear. Returning an empty vector when no valid masternodes are found is acceptable, unless you need explicit error handling in such cases.
hashes/src/internal_macros.rs (2)
141-148: Efficient encoding implementation.Leveraging
as_byte_array().encodeis a concise way to implementbincode::Encode. This approach avoids consumingself, which is good for reusability.
150-158: Straightforward decoding logic.Decoding into a fixed array and then constructing the Hash is succinct. No issues found.
dash/src/sml/masternode_list_entry/qualified_masternode_list_entry.rs (4)
1-24: Struct definition is well-organized.The derived traits and optional
confirmed_hash_hashed_with_pro_reg_txfield align with the expected usage pattern, supporting bothbincodeandserde.
26-30: Ordering relies on underlying entry ordering.Delegating
cmpsimplifies maintaining consistent ordering. This is good, assumingMasternodeListEntryhas a correct ordinal definition.
32-34: Partial ordering is consistent.
partial_cmpdefers tocmp, ensuring consistency withOrd.
36-54: Conversion logic is clear.The
Fromimplementation properly calculatesentry_hashand conditionally derivesconfirmed_hash_hashed_with_pro_reg_tx. This reuse of existing methods is concise and maintainable.dash/src/sml/llmq_type/mod.rs (5)
1-3: All good on module definitions.
The module declarations and visibility settings look appropriate. No issues found.
24-36: Ensure consistency inLLMQParamsfields.
While the struct is well-defined, it might be beneficial to confirm each field’s interplay (e.g.thresholdvs.min_size) through additional doc comments or usage examples. This helps future contributors understand the constraints without needing to dig into external references.
445-447: Validate default DKG rotation parameters for non-Devnet networks.
Currently,dkg_rotation_paramsreturnsDKG_DEVNET_DIP_0024forDevnet, otherwiseDKG_60_75. Verify correctness for Testnet/Mainnet usage or consider separate logic for each network.
342-359: Review defaulting toLLMQ_DEVNETfor unknown.
Withinparams(), theLlmqtypeUnknownbranch returnsLLMQ_DEVNET. This fallback may be unexpected. Confirm or handle unknown types more cautiously.
454-462: Check completeness of rotating quorum types.
is_rotating_quorum_type()currently includes onlyLlmqtype60_75,LlmqtypeDevnetDIP0024, andLlmqtypeTestDIP0024. Ensure other rotating quorums will not be missed.dash/src/sml/quorum_entry/qualified_quorum_entry.rs (3)
1-2: No concerns regarding bincode usage.
The conditional import ofEncodeandDecodeis correctly gated behind thebincodefeature.
16-29: Struct design is consistent.
QualifiedQuorumEntryneatly wraps the extra metadata for verification status and computed hashes. The approach is straightforward and maintains good separation fromQuorumEntry.
59-76: Validation statuses are clearly distinguished.
Theupdate_quorum_statusmethod nicely covers both success and multiple error states. However, consider unit testing each path (missing blocks, missing masternode list, etc.) to ensure coverage.dash/src/network/constants.rs (2)
48-49: Properly gated bincode derives.
The#[cfg(feature = "bincode")]usage looks correct, ensuring serialization code is only included when needed.
82-82: Network enum bincode derivation is consistent.
Good job aligning with the pattern used elsewhere in the codebase for conditional encoding/decoding.dash/src/sml/quorum_entry/validation.rs (1)
101-109: Well-structured threshold signature verification.
This method cleanly converts public key and signature intoblsfultypes and handles errors. The design aligns well with the separation of concerns for threshold signature checks.dash/src/network/message_qrinfo.rs (2)
1-13: Imports and feature-gated dependencies look good.
No immediate issues found with these imports or feature-flagged usage ofbincode.
65-139: Encoding/decoding logic is well-organized.
The encode/decode logic correctly distinguishes optional fields by writing/reading a marker byte. This approach is consistent with the rest of the codebase and no critical issues stand out.dash/src/sml/masternode_list/from_diff.rs (3)
1-21: Trait declarations are clear and useful for typed block-hash lookups.
The definitions ofTryFromWithBlockHashLookupandTryIntoWithBlockHashLookupnicely encapsulate logic for conversions tied to block hash data. No issues noted.
23-52: Forwarding implementation is consistent.
impl<T, U> TryIntoWithBlockHashLookup<U> for Tuses the underlyingTryFromWithBlockHashLookuptrait. This pattern is straightforward and follows typical Rust approaches to extend conversions.
54-118: Potential pitfalls inMasternodeListconstruction fromMnListDiff.
- The code uses
entry.pro_reg_tx_hash.reverse()when populating masternodes, which can introduce confusion or mismatches if other parts of the codebase expect an unreversed hash.- Only the first element of
diff.merkle_hashesis stored inmasternode_merkle_root. If multiple merkle hashes are significant, consider clarifying or validating that one is enough.dash/src/sml/masternode_list/mod.rs (2)
1-11: Module organization and partial re-exports look good.
No concerns found with the structure or naming of the submodules.
23-36: Hash reversal might lead to confusion.
A comment notes thatpro_tx_hashis reversed. This can be error-prone if other parts of the code rely on a standard endianness. Consider removing or documenting the reversal unless there’s a strong justification.hashes/src/util.rs (4)
115-116: Doc comment additions look fine.
149-149: Additional usage hint.No functional impact; good for clarity.
210-243: Newtype methods appear well-structured.Implementation of
from_raw_hash,to_raw_hash,as_raw_hash, andreverseis straightforward and correct.
245-326: Robust trait implementation for bridging $hash and the newtype.The conversions and
Hashtrait implementation look consistent with no obvious issues.dash/src/sml/masternode_list/merkle_roots.rs (7)
1-2: New import statements.No concerns; standard library usage.
3-3: Import ofTransaction.No issue; consistent with usage in the file.
46-74:has_valid_mn_list_rootmethod logic.Ensure the early
return falsecovers all unexpected coinbase payload variants. Code seems correct for now.
76-108:has_valid_llmq_list_rootmethod.Validation is straightforward; logs are commented out but could be useful in troubleshooting.
110-130:calculate_masternodes_merkle_root.Leverages
merkle_root_from_hashescorrectly and maps result to the typed root.
132-144:calculate_llmq_merkle_root.Uses the same logic pattern; consistent approach for LLMQ root.
175-192:hashes_for_quorum_merkle_root.Sorting ensures deterministic Merkle root generation. Implementation looks correct.
dash/src/network/message_sml.rs (10)
1-4: Updated use statements and feature-gated imports.Adds bincode/serde features for broader serialization support; no issues.
Also applies to: 7-7, 10-13
36-38: Added derive attributes for MnListDiff.Enabling bincode and serde derivations is consistent with the rest of the codebase.
62-65: New fields inMnListDiff:new_quorumsandquorums_chainlock_signatures.This extends coverage for LLMQ data; straightforward addition.
67-83:impl Encodable for MnListDiff.Logic accumulates the encoded length correctly, matching the struct fields.
86-100:impl Decodable for MnListDiff.Decodes fields in the correct order; consistent with the encoding.
101-115: Construction ofMnListDifffrom decoded fields.Straightforward bridging from decode steps to struct creation.
119-125: NewQuorumCLSigObjectstruct with bincode/serde.Captures signature and index set. No immediate issues.
127-127: Consensus encoding macro for QuorumCLSigObject.Matches established pattern for macros.
129-137:DeletedQuorumextends toLLMQTypeandQuorumHash.Provides more semantic clarity than raw
u8or hash type.
148-150: Additional imports and references forserialize/deserializeandMnListDiff.No concerns; these are necessary for testing and usage.
dash/src/sml/masternode_list/scores_for_quorum.rs (3)
13-28: Verify reversed ordering for top N masternodes
By callingscore_dictionary.into_values().rev().take(...), you're retrieving the highest scores first only ifScoreHashsorting is ascending. It looks intentional, but please confirm this reversed iteration aligns with the desired logic (i.e., highest-scoring entries for your quorum).
30-48: Validate partition indexing
You use a running indexito look upactive_quorum_members.get(i)and default tofalseif out of range. Verify that short or mismatched snapshots should truly categorize those masternodes as “unused.” If not, consider explicit checks or error handling wheniexceeds the length.
75-85: Wrapper function reliance
scores_for_quorumrelies entirely onscores_for_quorum_for_masternodes. Ensure you have test coverage confirming this method’s output matches expectations, since it adds no additional logic.dash/src/bls_sig_utils.rs (6)
20-23: Feature-gated imports
Conditionally importingbincodeandblsfulunder respective features looks good. This keeps the module lightweight when these features aren’t needed.
36-41: Check test coverage for zeroed keys
Your#[cfg_attr(feature = "bincode", derive(Encode, Decode))]plusis_zeroed()method is a nice addition. Make sure to include tests that confirm the behavior ofis_zeroed()with both all-zero keys and non-zero keys.
95-99: Consistency with BLSPublicKey
BLSSignaturealso providesis_zeroed(). Verify usage in your codebase to see where zeroed signatures could appear in the flow and confirm this is accounted for in your validation logic.
101-115: Check invalid compressed data for BLSSignature
When converting fromBLSSignaturetoblsful::Signature, you rely onfrom_compressed(...).into_option(). If the conversion fails, you returnInvalidBLSSignature. Ensure you have tests verifying that corrupted or invalid data triggers this logic.
117-131: Reference-based signature conversion
Similar to the owned case, make sure to confirm partial or invalid signature data is properly handled.
149-163: MultiSignature conversions (references)
Same pattern: repeated logic for the reference variant. Confirm uniform coverage for error scenarios.dash/src/hash_types.rs (9)
74-75: Imports and ordering
Bringing instd::cmp::Orderingandhashes::{sha256, sha256d, hash160, hash_newtype, Hash, hash_newtype_no_ord}is consistent. No concerns here.
129-130: Forward-derivation of Merkle roots
#[hash_newtype(forward)]forMerkleRootMasternodeListandMerkleRootQuorumsneatly passes internal hash calls. Looks fine.Also applies to: 132-133
145-150: Additional newtypes
Defining multiple newtypes (ProTxHash,ConfirmedHash, etc.) fosters type safety. Check that no call site expects these to be interchangeable inadvertently.
153-157: ScoreHash declared with no default ordering
Usinghash_newtype_no_ord!forScoreHashis a good approach when you have customOrdlogic. Ensure no existing code depends on defaultEqorOrdfor hashing or sorting.
159-169: Reverse byte comparison for ScoreHash
Reversing endianness to compare forOrdis presumably intentional. Validate that this big-endian logic, ensuring higher numeric values rank higher, is the correct approach for your chain’s consensus or scoring.
171-175: PartialOrd consistent with Ord
partial_cmpdelegates tocmpso they remain consistent. This is standard practice.
179-179: QuorumHash alias
You’ve aliasedQuorumHashtoBlockHash, presumably to preserve existing references. Just confirm this redirection does not create confusion where logic differs between blocks and quorums.
203-207: Serialization macros
impl_hashencode!usage for new hash types likeConfirmedHashHashedWithProRegTxandQuorumModifierHashis consistent with the rest. No issues noted.
258-262: Default for ConfirmedHash
Setting[0; 32]as a default might indicate an ‘uninitialized’ or ‘missing’ state. Validate whether 0 is a safe fallback that won't cause unexpected collisions in logic.dash/src/sml/masternode_list_engine/validation.rs (3)
1-9: Imports look fine.These imports and uses appear consistent with the file’s new functionality.
10-20: Functionfind_valid_masternodes_for_quorumlogic appears clean.The branching on
is_rotating_quorum_type()looks appropriate for delegating to the correct helper function. Just ensure each helper consistently returns a unified error type to avoid unexpected partial states.
25-43: Check usage ofsigners.get(i)?.In standard Rust,
Vec::get(i)returns anOption<&bool>. Ensure that this is properly transformed into aResultwith a meaningful error if the index is out of range, or confirm that the.get(i)?is using a custom type that indeed returns aResult.dash/src/sml/masternode_list/quorum_helpers.rs (10)
1-8: New imports are consistent with file scope.No issues noted. These imports align with the new functionality for quorum management.
9-27: Functionquorum_hashesusage of excludes looks correct.The conditional logic for empty vs. non-empty
exclude_quorum_typesis straightforward. The functional style with iterators is clean.
29-47: Functionnon_rotating_quorum_hashesis clear and concise.Filtering out rotated quorums and excluded types is correct. No concerns here.
49-67: Functionrotating_quorum_hashesmirrors the non-rotating variant well.Behavior is consistent. Ensuring it captures all rotating types except those in
exclude_quorum_typeslooks good.
69-83: Functionquorum_entry_of_type_for_quorum_hashis straightforward.Returning
Optionfrom the nested map access is normal. No concerns.
85-99: Functionquorum_entry_of_type_for_quorum_hash_mutproperly matches its immutable counterpart.This is a direct mutable reference version. Implementation is consistent.
101-111: Functionquorums_countis simple and effective.Counting entries over each LLMQ type is straightforward.
113-127:platform_llmq_with_quorum_hashlogic is minimal.Clones the retrieved quorum entry for usage elsewhere. Looks good.
129-142: Check whethernetwork.isd_llmq_type()covers all rotating quorums.If multiple LLMQ types can rotate, confirm you aren’t missing others. Otherwise, usage is consistent with your design.
144-157:has_unverified_regular_quorumsmirrors the rotating variant.This check is also restricted to the single
isd_llmq_type(). Confirm that’s the intended scope for “regular quorums.”dash/src/blockdata/transaction/special_transaction/quorum_commitment.rs (8)
20-21: Conditional bincode usage aligned with feature gate.Having
[cfg(feature = "bincode")]for serialization is consistent with the approach in the rest of the code.
30-33: Imports forLLMQTypeandQuorumValidationError.These new imports fit logically and are used by the
QuorumEntrystruct’s validation.
42-44: NewQuorumEntrystruct includesLLMQTypefield.Switching from a numeric type to an enum is clearer and less error-prone. Good improvement.
50-53: Renamed fields and new BLSSignature usage.Using separate fields (
threshold_sig,all_commitment_aggregated_signature) makes the code more descriptive.
55-68:QuorumEntry::size()method is consistent with the new fields.Properly accounts for signers/valid_members bitset lengths, plus any version-specific fields.
120-132:Encodableimplementation reflects new fields accurately.Version-based handling for
quorum_indexis done properly, with bitset writing for signers and valid_members.
144-144:DecodableforQuorumEntryremains consistent.Handles version-based reading of
quorum_indexand the new fields for BLSSignature usage.
187-187:QuorumCommitmentPayloadreferences the updatedQuorumEntry.Replacing the older finalization struct with the new
QuorumEntryaligns well with the rest of the changes.dash/src/sml/masternode_list_engine/rotated_quorum_construction.rs (4)
31-43: Guard against potential underflow incycle_base_height.
cycle_base_height = quorum_block_height - quorum_index as u32may underflow ifquorum_indexis unexpectedly larger thanquorum_block_height. Consider checking or saturating the subtraction to handle edge cases gracefully.
116-134: Check for negative block heights when subtracting multiple cycles.This logic repeatedly subtracts
cycle_lengthand8(line 130). Ifcycle_base_heightis smaller than these subtractions, it could produce a negative integer, which is not valid for block heights. Consider an additional safeguard that ensures non-negative results before inserting them into the BTreeSet.
200-216: Document the behavior for missing quarter indexes.
add_quorum_members_from_quarterquietly returns ifquarter.get(index)isNone. Document whether this is an expected scenario or if it indicates an anomaly. Logging or an error might be beneficial when the array is unexpectedly small.
231-295: Review snapshot presence checks for reliability.If a snapshot is missing or corrupt, the logic returns an error. Confirm that callers handle these errors properly and that no partial states remain in the engine. Consider rollback or cleanup logic if intermediate states could become inconsistent.
dash/src/sml/masternode_list/masternode_helpers.rs (2)
59-63: Confirm expected lexical ordering with.reverse().Here, you sort by comparing the reversed byte arrays. Make sure that reversing each hash (i.e., big-endian vs. little-endian concerns) is indeed the correct domain logic. If you need a standard numeric ordering, consider converting each hash to a big-endian integer before comparing.
89-99: Handle potential merges or expansions inreverse_cmp_sup.As soon as the code grows to handle more hashing or a different hash length,
reverse_cmp_supmight become insufficient or misleading if the data structure changes. Keep this function in sync with future expansions to the hash format.dash/src/sml/masternode_list_engine/mod.rs (2)
104-145: Validate aggregator usage for block height requests.You collect multiple diffs in
request_qr_info_block_heightsand feed them torequest_mn_list_diff_heights. This approach can produce partial updates if an error occurs part-way. Check whether partial insertion of block heights can cause inconsistent internal state.
303-394: Review error handling inapply_difffor base block hash retrieval.The function returns an error if the base block hash is not found in
block_heightsormasternode_lists(lines 338–344). Double-check upstream calls to ensure the base block hash is always fed prior to callingapply_diff, or handle the missing data gracefully if not.
| /// A snapshot of quorum-related information at a given cycle height. | ||
| /// | ||
| /// Fields: | ||
| /// - `mn_skip_list_mode`: A 4-byte signed integer representing the mode of the skip list. | ||
| /// - `active_quorum_members_count`: A compact-size unsigned integer representing the number of active quorum members. | ||
| /// - `active_quorum_members`: A bitset (stored as a Vec<u8>) with length = | ||
| /// (active_quorum_members_count + 7) / 8. | ||
| /// - `mn_skip_list_size`: A compact-size unsigned integer representing the number of skip list entries. | ||
| /// - `mn_skip_list`: An array of 4-byte signed integers, one per skip list entry. | ||
| #[derive(PartialEq, Eq, Clone, Debug)] | ||
| #[cfg_attr(feature = "bincode", derive(Encode, Decode))] | ||
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | ||
| #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))] | ||
| pub struct QuorumSnapshot { | ||
| pub skip_list_mode: MNSkipListMode, | ||
| pub active_quorum_members: Vec<bool>, // Bitset, length = (active_quorum_members_count + 7) / 8 | ||
| pub skip_list: Vec<u32>, // Array of int32_t | ||
| } | ||
|
|
||
| impl Display for QuorumSnapshot { | ||
| fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { | ||
| let active_members_display: String = self | ||
| .active_quorum_members | ||
| .iter() | ||
| .map(|&member| if member { '■' } else { 'x' }) // Use `■` for true, `x` for false | ||
| .collect(); | ||
|
|
||
| let skip_list = self.skip_list.iter().map(|i| i.to_string()).collect::<Vec<_>>().join(","); | ||
|
|
||
| write!( | ||
| f, | ||
| "mode: {} members: [{}] skip_list: [{}]", | ||
| active_members_display, self.skip_list_mode, skip_list | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| impl Encodable for QuorumSnapshot { | ||
| fn consensus_encode<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> { | ||
| let mut len = 0; | ||
| len += self.skip_list_mode.consensus_encode(w)?; | ||
| len += write_fixed_bitset( | ||
| w, | ||
| self.active_quorum_members.as_slice(), | ||
| self.active_quorum_members.iter().len(), | ||
| )?; | ||
| len += self.skip_list.consensus_encode(w)?; | ||
| Ok(len) | ||
| } | ||
| } | ||
|
|
||
| impl Decodable for QuorumSnapshot { | ||
| fn consensus_decode<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> { | ||
| let mn_skip_list_mode = MNSkipListMode::consensus_decode(r)?; | ||
| let active_quorum_members_count = read_compact_size(r)?; | ||
| let active_quorum_members = read_fixed_bitset(r, active_quorum_members_count as usize)?; | ||
| let mn_skip_list = Vec::consensus_decode(r)?; | ||
| Ok(QuorumSnapshot { | ||
| skip_list_mode: mn_skip_list_mode, | ||
| active_quorum_members, | ||
| skip_list: mn_skip_list, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Mismatch between documented and actual integer type in QuorumSnapshot::skip_list.
The documentation states the skip list contains 4-byte signed integers, but the code uses Vec<u32>. This discrepancy can lead to silent bugs if negative values are intended.
- pub skip_list: Vec<u32>, // Array of int32_t
+ pub skip_list: Vec<i32>, // Array of int32_t📝 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.
| /// A snapshot of quorum-related information at a given cycle height. | |
| /// | |
| /// Fields: | |
| /// - `mn_skip_list_mode`: A 4-byte signed integer representing the mode of the skip list. | |
| /// - `active_quorum_members_count`: A compact-size unsigned integer representing the number of active quorum members. | |
| /// - `active_quorum_members`: A bitset (stored as a Vec<u8>) with length = | |
| /// (active_quorum_members_count + 7) / 8. | |
| /// - `mn_skip_list_size`: A compact-size unsigned integer representing the number of skip list entries. | |
| /// - `mn_skip_list`: An array of 4-byte signed integers, one per skip list entry. | |
| #[derive(PartialEq, Eq, Clone, Debug)] | |
| #[cfg_attr(feature = "bincode", derive(Encode, Decode))] | |
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | |
| #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))] | |
| pub struct QuorumSnapshot { | |
| pub skip_list_mode: MNSkipListMode, | |
| pub active_quorum_members: Vec<bool>, // Bitset, length = (active_quorum_members_count + 7) / 8 | |
| pub skip_list: Vec<u32>, // Array of int32_t | |
| } | |
| impl Display for QuorumSnapshot { | |
| fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { | |
| let active_members_display: String = self | |
| .active_quorum_members | |
| .iter() | |
| .map(|&member| if member { '■' } else { 'x' }) // Use `■` for true, `x` for false | |
| .collect(); | |
| let skip_list = self.skip_list.iter().map(|i| i.to_string()).collect::<Vec<_>>().join(","); | |
| write!( | |
| f, | |
| "mode: {} members: [{}] skip_list: [{}]", | |
| active_members_display, self.skip_list_mode, skip_list | |
| ) | |
| } | |
| } | |
| impl Encodable for QuorumSnapshot { | |
| fn consensus_encode<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> { | |
| let mut len = 0; | |
| len += self.skip_list_mode.consensus_encode(w)?; | |
| len += write_fixed_bitset( | |
| w, | |
| self.active_quorum_members.as_slice(), | |
| self.active_quorum_members.iter().len(), | |
| )?; | |
| len += self.skip_list.consensus_encode(w)?; | |
| Ok(len) | |
| } | |
| } | |
| impl Decodable for QuorumSnapshot { | |
| fn consensus_decode<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> { | |
| let mn_skip_list_mode = MNSkipListMode::consensus_decode(r)?; | |
| let active_quorum_members_count = read_compact_size(r)?; | |
| let active_quorum_members = read_fixed_bitset(r, active_quorum_members_count as usize)?; | |
| let mn_skip_list = Vec::consensus_decode(r)?; | |
| Ok(QuorumSnapshot { | |
| skip_list_mode: mn_skip_list_mode, | |
| active_quorum_members, | |
| skip_list: mn_skip_list, | |
| }) | |
| } | |
| } | |
| /// A snapshot of quorum-related information at a given cycle height. | |
| /// | |
| /// Fields: | |
| /// - `mn_skip_list_mode`: A 4-byte signed integer representing the mode of the skip list. | |
| /// - `active_quorum_members_count`: A compact-size unsigned integer representing the number of active quorum members. | |
| /// - `active_quorum_members`: A bitset (stored as a Vec<u8>) with length = | |
| /// (active_quorum_members_count + 7) / 8. | |
| /// - `mn_skip_list_size`: A compact-size unsigned integer representing the number of skip list entries. | |
| /// - `mn_skip_list`: An array of 4-byte signed integers, one per skip list entry. | |
| #[derive(PartialEq, Eq, Clone, Debug)] | |
| #[cfg_attr(feature = "bincode", derive(Encode, Decode))] | |
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | |
| #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))] | |
| pub struct QuorumSnapshot { | |
| pub skip_list_mode: MNSkipListMode, | |
| pub active_quorum_members: Vec<bool>, // Bitset, length = (active_quorum_members_count + 7) / 8 | |
| - pub skip_list: Vec<u32>, // Array of int32_t | |
| + pub skip_list: Vec<i32>, // Array of int32_t | |
| } | |
| impl Display for QuorumSnapshot { | |
| fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { | |
| let active_members_display: String = self | |
| .active_quorum_members | |
| .iter() | |
| .map(|&member| if member { '■' } else { 'x' }) // Use `■` for true, `x` for false | |
| .collect(); | |
| let skip_list = self.skip_list.iter().map(|i| i.to_string()).collect::<Vec<_>>().join(","); | |
| write!( | |
| f, | |
| "mode: {} members: [{}] skip_list: [{}]", | |
| active_members_display, self.skip_list_mode, skip_list | |
| ) | |
| } | |
| } | |
| impl Encodable for QuorumSnapshot { | |
| fn consensus_encode<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> { | |
| let mut len = 0; | |
| len += self.skip_list_mode.consensus_encode(w)?; | |
| len += write_fixed_bitset( | |
| w, | |
| self.active_quorum_members.as_slice(), | |
| self.active_quorum_members.iter().len(), | |
| )?; | |
| len += self.skip_list.consensus_encode(w)?; | |
| Ok(len) | |
| } | |
| } | |
| impl Decodable for QuorumSnapshot { | |
| fn consensus_decode<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> { | |
| let mn_skip_list_mode = MNSkipListMode::consensus_decode(r)?; | |
| let active_quorum_members_count = read_compact_size(r)?; | |
| let active_quorum_members = read_fixed_bitset(r, active_quorum_members_count as usize)?; | |
| let mn_skip_list = Vec::consensus_decode(r)?; | |
| Ok(QuorumSnapshot { | |
| skip_list_mode: mn_skip_list_mode, | |
| active_quorum_members, | |
| skip_list: mn_skip_list, | |
| }) | |
| } | |
| } |
| fn apply_skip_strategy_of_type<'a>( | ||
| skip_type: LLMQQuarterUsageType, | ||
| used_at_h_masternodes: Vec<&'a QualifiedMasternodeListEntry>, | ||
| unused_at_h_masternodes: Vec<&'a QualifiedMasternodeListEntry>, | ||
| quorum_modifier: QuorumModifierHash, | ||
| quorum_count: usize, | ||
| quarter_size: usize, | ||
| ) -> Vec<Vec<&'a QualifiedMasternodeListEntry>> { | ||
| let sorted_used_mns_list = MasternodeList::scores_for_quorum_for_masternodes( | ||
| used_at_h_masternodes, | ||
| quorum_modifier, | ||
| false, | ||
| ); | ||
| let sorted_unused_mns_list = MasternodeList::scores_for_quorum_for_masternodes( | ||
| unused_at_h_masternodes, | ||
| quorum_modifier, | ||
| false, | ||
| ); | ||
| // if matches!(skip_type, LLMQQuarterUsageType::New(_)) { | ||
| // let used_masternodes_string = sorted_used_mns_list.values().rev().map(|m| m.masternode_list_entry.pro_reg_tx_hash.reverse().to_string().split_at(4).0.to_string()).collect::<Vec<_>>().join("|"); | ||
| // let unused_masternodes_string = sorted_unused_mns_list.values().rev().map(|m| m.masternode_list_entry.pro_reg_tx_hash.reverse().to_string().split_at(4).0.to_string()).collect::<Vec<_>>().join("|"); | ||
| // | ||
| // println!("used masternodes [{}]", used_masternodes_string); | ||
| // println!("unused masternodes [{}]", unused_masternodes_string); | ||
| // } | ||
| let sorted_combined_mns_list = Vec::from_iter( | ||
| sorted_unused_mns_list | ||
| .into_values() | ||
| .rev() | ||
| .chain(sorted_used_mns_list.into_values().rev()), | ||
| ); | ||
| match skip_type { | ||
| LLMQQuarterUsageType::Snapshot(snapshot) => { | ||
| match snapshot.skip_list_mode { | ||
| MNSkipListMode::NoSkipping => sorted_combined_mns_list | ||
| .chunks(quarter_size) | ||
| .map(|chunk| chunk.to_vec()) | ||
| .collect(), | ||
| MNSkipListMode::SkipFirst => { | ||
| let mut first_entry_index = 0; | ||
| let processed_skip_list = | ||
| Vec::from_iter(snapshot.skip_list.into_iter().map(|s| { | ||
| if first_entry_index == 0 { | ||
| first_entry_index = s; | ||
| s | ||
| } else { | ||
| first_entry_index + s | ||
| } | ||
| })); | ||
| let mut idx = 0; | ||
| let mut skip_idx = 0; | ||
| (0..quorum_count) | ||
| .map(|_| { | ||
| let mut quarter = Vec::with_capacity(quarter_size); | ||
| while quarter.len() < quarter_size { | ||
| let index = (idx + 1) % sorted_combined_mns_list.len(); | ||
| if skip_idx < processed_skip_list.len() | ||
| && idx == processed_skip_list[skip_idx] as usize | ||
| { | ||
| skip_idx += 1; | ||
| } else { | ||
| quarter.push(sorted_combined_mns_list[idx]); | ||
| } | ||
| idx = index | ||
| } | ||
| quarter | ||
| }) | ||
| .collect() | ||
| } | ||
| MNSkipListMode::SkipExcept => (0..quorum_count) | ||
| .map(|i| { | ||
| snapshot | ||
| .skip_list | ||
| .iter() | ||
| .filter_map(|unskipped| { | ||
| sorted_combined_mns_list.get(*unskipped as usize) | ||
| }) | ||
| .take(quarter_size) | ||
| .cloned() | ||
| .collect() | ||
| }) | ||
| .collect(), | ||
| MNSkipListMode::SkipAll => { | ||
| // TODO: do we need to impl smth in this strategy ? | ||
| // warn!("skip_mode SkipAll not supported yet"); | ||
| vec![Vec::<&QualifiedMasternodeListEntry>::new(); quorum_count] | ||
| } | ||
| } | ||
| } | ||
| LLMQQuarterUsageType::New(mut used_indexed_masternodes) => { | ||
| let mut quarter_quorum_members = | ||
| vec![Vec::<&QualifiedMasternodeListEntry>::new(); quorum_count]; | ||
| let mut skip_list = Vec::<i32>::new(); | ||
| let mut first_skipped_index = 0i32; | ||
| let mut idx = 0i32; | ||
| for i in 0..quorum_count { | ||
| let masternodes_used_at_h_indexed_at_i = used_indexed_masternodes | ||
| .get_mut(i) | ||
| .expect("expected to get index i quorum used indexed masternodes"); | ||
| let used_mns_count = masternodes_used_at_h_indexed_at_i.len(); | ||
| let sorted_combined_mns_list_len = sorted_combined_mns_list.len(); | ||
| let mut updated = false; | ||
| let initial_loop_idx = idx; | ||
| while quarter_quorum_members[i].len() < quarter_size | ||
| && used_mns_count + quarter_quorum_members[i].len() | ||
| < sorted_combined_mns_list_len | ||
| { | ||
| let mn = sorted_combined_mns_list.get(idx as usize).unwrap(); | ||
| if masternodes_used_at_h_indexed_at_i.iter().any(|node| { | ||
| mn.masternode_list_entry.pro_reg_tx_hash | ||
| == node.masternode_list_entry.pro_reg_tx_hash | ||
| }) { | ||
| if first_skipped_index == 0 { | ||
| first_skipped_index = idx; | ||
| } | ||
| skip_list.push(idx); | ||
| } else { | ||
| masternodes_used_at_h_indexed_at_i.push(mn); | ||
| quarter_quorum_members[i].push(mn); | ||
| updated = true; | ||
| } | ||
| idx += 1; | ||
| if idx == sorted_combined_mns_list_len as i32 { | ||
| idx = 0; | ||
| } | ||
| if idx == initial_loop_idx { | ||
| if !updated { | ||
| // warn!("there are not enough MNs then required for quarter size: ({})", quarter_size); | ||
| return quarter_quorum_members; | ||
| } | ||
| updated = false; | ||
| } | ||
| } | ||
| } | ||
| quarter_quorum_members | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce the complexity of apply_skip_strategy_of_type.
This method is quite large, handling multiple skip scenarios. Consider splitting it into smaller dedicated functions—one function per skip mode—to improve readability and testing. The nested loops and index adjustments in lines 350–444 can be especially tricky to maintain.
| #[cfg(test)] | ||
| mod tests { | ||
|
|
||
| use bincode::Decode; |
There was a problem hiding this comment.
Address usage of bincode in tests.
The pipeline fails when trying to import or decode with bincode in the test module (line 450, 463, 561, 582). If these tests require bincode, add a proper dev-dependency entry (e.g., [dev-dependencies] bincode = "...") in Cargo.toml.
🧰 Tools
🪛 GitHub Actions: Continuous integration
[error] 450-450: unresolved import bincode: use of unresolved module or unlinked crate bincode.
| #[cfg(feature = "bincode")] | ||
| use bincode::{Decode, Encode}; | ||
| #[cfg(feature = "serde")] | ||
| use serde::{Deserialize, Serialize}; |
There was a problem hiding this comment.
Resolve missing bincode dependency.
Multiple pipeline errors indicate unresolved references to bincode. Ensure that bincode is included in the project’s dependencies. For example, in your Cargo.toml:
[dependencies]
+ bincode = "x.y.z"📝 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.
| #[cfg(feature = "bincode")] | |
| use bincode::{Decode, Encode}; | |
| #[cfg(feature = "serde")] | |
| use serde::{Deserialize, Serialize}; | |
| [dependencies] | |
| bincode = "x.y.z" |
QuantumExplorer
changed the title
|
Merging for now, @shumkov will review when he can. |
| bincode = { version= "2.0.0-rc.3", optional = true } | ||
| bitflags = "2.6.0" | ||
| blsful = { version = "3.0.0-pre8", optional = true } | ||
| blsful = { version = "3.0.0-pre6", optional = true } |
There was a problem hiding this comment.
why did we downgrade blsful?
| size | ||
| } | ||
|
|
||
| pub fn validate_structure(&self) -> Result<(), QuorumValidationError> { |
There was a problem hiding this comment.
There might be a situation when the validation function could throw an error while running validation logic. In this case error handling will be complicated, and you won't be able to use ?. Shall we just return an optional error as Ok?
| pub enum LLMQEntryVerificationSkipStatus { | ||
| NotMarkedForVerification, | ||
| MissedList(CoreBlockHeight), | ||
| UnknownBlock([u8; 32]), |
| self.provider_tx_ordered_hashes().eq(&list.provider_tx_ordered_hashes()) | ||
| } | ||
|
|
||
| pub fn compare_masternodes(&self, list: MasternodeList) -> bool { |
There was a problem hiding this comment.
let set1: HashSet<_> = self.masternodes.values().cloned().collect();
let set2: HashSet<_> = list.masternodes.values().cloned().collect();
set1 == set2
It will be O(n) instead of O(n log n)
There was a problem hiding this comment.
Oh god, they already in BTreeMap. So even easier just use eq
| } | ||
| list | ||
| } | ||
| pub fn with_merkle_roots( |
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | ||
| #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))] | ||
| #[cfg_attr(feature = "bincode", derive(Encode, Decode))] | ||
| pub struct MasternodeListEngine { |
There was a problem hiding this comment.
what is this for? any documentation?
* refactor: use `BlockHash` instead of byte array * refactor: remove unnecessary vector allocation and sorting * refactor: use Hash primitive * refactor: introduce `MasternodeListBuilder` * refactor: use macros for encoding/decoding traits * refactor: remove unused struct * refactor: remove unused methods * refactor: add helper methods for MN list builder * refactor: remove unused import
Summary by CodeRabbit
New Features
Chores
Refactor