refactor: minor masternode list improvements based on #51 review (#54)

* 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

Author: shumkov

dash/src/network/message_sml.rs

@@ -64,56 +64,7 @@ pub struct MnListDiff {
     pub quorums_chainlock_signatures: Vec<QuorumCLSigObject>,
 }
 
-impl Encodable for MnListDiff {
-    fn consensus_encode<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
-        let mut len = 0;
-        len += self.version.consensus_encode(w)?;
-        len += self.base_block_hash.consensus_encode(w)?;
-        len += self.block_hash.consensus_encode(w)?;
-        len += self.total_transactions.consensus_encode(w)?;
-        len += self.merkle_hashes.consensus_encode(w)?;
-        len += self.merkle_flags.consensus_encode(w)?;
-        len += self.coinbase_tx.consensus_encode(w)?;
-        len += self.deleted_masternodes.consensus_encode(w)?;
-        len += self.new_masternodes.consensus_encode(w)?;
-        len += self.deleted_quorums.consensus_encode(w)?;
-        len += self.new_quorums.consensus_encode(w)?;
-        len += self.quorums_chainlock_signatures.consensus_encode(w)?;
-        Ok(len)
-    }
-}
-
-impl Decodable for MnListDiff {
-    fn consensus_decode<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
-        let version = u16::consensus_decode(r)?;
-        let base_block_hash = BlockHash::consensus_decode(r)?;
-        let block_hash = BlockHash::consensus_decode(r)?;
-        let total_transactions = u32::consensus_decode(r)?;
-        let merkle_hashes = Vec::<MerkleRootMasternodeList>::consensus_decode(r)?;
-        let merkle_flags = Vec::<u8>::consensus_decode(r)?;
-        let coinbase_tx = Transaction::consensus_decode(r)?;
-        let deleted_masternodes = Vec::<ProTxHash>::consensus_decode(r)?;
-        let new_masternodes = Vec::<MasternodeListEntry>::consensus_decode(r)?;
-        let deleted_quorums = Vec::<DeletedQuorum>::consensus_decode(r)?;
-        let new_quorums = Vec::<QuorumEntry>::consensus_decode(r)?;
-        let quorums_chainlock_signatures = Vec::<QuorumCLSigObject>::consensus_decode(r)?;
-
-        Ok(MnListDiff {
-            version,
-            base_block_hash,
-            block_hash,
-            total_transactions,
-            merkle_hashes,
-            merkle_flags,
-            coinbase_tx,
-            deleted_masternodes,
-            new_masternodes,
-            deleted_quorums,
-            new_quorums,
-            quorums_chainlock_signatures,
-        })
-    }
-}
+impl_consensus_encoding!(MnListDiff, version, base_block_hash, block_hash, total_transactions, merkle_hashes, merkle_flags, coinbase_tx, deleted_masternodes, new_masternodes, deleted_quorums, new_quorums, quorums_chainlock_signatures);
 
 #[derive(PartialEq, Eq, Clone, Debug)]
 #[cfg_attr(feature = "bincode", derive(Encode, Decode))]
@@ -131,7 +82,6 @@ impl_consensus_encoding!(QuorumCLSigObject, signature, index_set);
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
 pub struct DeletedQuorum {
-    // TODO: Make it enum
     pub llmq_type: LLMQType,
     pub quorum_hash: QuorumHash,
 }

dash/src/sml/llmq_entry_verification.rs

@@ -2,7 +2,7 @@ use core::fmt::{Display, Formatter};
 
 #[cfg(feature = "bincode")]
 use bincode::{Decode, Encode};
-
+use crate::BlockHash;
 use crate::prelude::CoreBlockHeight;
 use crate::sml::quorum_validation_error::QuorumValidationError;
 
@@ -13,7 +13,7 @@ use crate::sml::quorum_validation_error::QuorumValidationError;
 pub enum LLMQEntryVerificationSkipStatus {
     NotMarkedForVerification,
     MissedList(CoreBlockHeight),
-    UnknownBlock([u8; 32]),
+    UnknownBlock(BlockHash),
     OtherContext(String),
 }
 
@@ -27,7 +27,7 @@ impl Display for LLMQEntryVerificationSkipStatus {
                     format!("MissedList({})", block_height)
                 }
                 LLMQEntryVerificationSkipStatus::UnknownBlock(block_hash) => {
-                    format!("UnknownBlock({})", hex::encode(block_hash))
+                    format!("UnknownBlock({})", block_hash)
                 }
                 LLMQEntryVerificationSkipStatus::OtherContext(message) => {
                     format!("OtherContext({message})")

dash/src/sml/llmq_type/llmq_indexed_hash.rs

@@ -1,4 +1,3 @@
-pub(crate) mod llmq_indexed_hash;
 mod network;
 pub mod rotation;
 

dash/src/sml/llmq_type/mod.rs

@@ -76,12 +76,13 @@ impl MasternodeList {
         }
 
         // Create and return the new MasternodeList
-        Ok(MasternodeList::new(
+        let builder = MasternodeList::build(
             updated_masternodes,
             updated_quorums,
             diff.block_hash,
             diff_end_height,
-            true, // Assume quorums are active
-        ))
+        );
+
+        Ok(builder.build())
     }
 }

dash/src/sml/masternode_list/apply_diff.rs

@@ -0,0 +1,62 @@
+use crate::hash_types::{MerkleRootMasternodeList, MerkleRootQuorums};
+use crate::sml::llmq_type::LLMQType;
+use crate::sml::masternode_list::MasternodeList;
+use crate::sml::masternode_list_entry::qualified_masternode_list_entry::QualifiedMasternodeListEntry;
+use crate::sml::quorum_entry::qualified_quorum_entry::QualifiedQuorumEntry;
+use crate::{BlockHash, ProTxHash, QuorumHash};
+use std::collections::BTreeMap;
+
+pub struct MasternodeListBuilder {
+    pub block_hash: BlockHash,
+    pub block_height: u32,
+    pub masternode_merkle_root: Option<MerkleRootMasternodeList>,
+    pub llmq_merkle_root: Option<MerkleRootQuorums>,
+    pub masternodes: BTreeMap<ProTxHash, QualifiedMasternodeListEntry>,
+    pub quorums: BTreeMap<LLMQType, BTreeMap<QuorumHash, QualifiedQuorumEntry>>,
+}
+
+impl MasternodeListBuilder {
+    pub fn new(
+        masternodes: BTreeMap<ProTxHash, QualifiedMasternodeListEntry>,
+        quorums: BTreeMap<LLMQType, BTreeMap<QuorumHash, QualifiedQuorumEntry>>,
+        block_hash: BlockHash,
+        block_height: u32,
+    ) -> Self {
+        Self {
+            quorums,
+            block_hash,
+            block_height,
+            masternode_merkle_root: None,
+            llmq_merkle_root: None,
+            masternodes,
+        }
+    }
+
+    pub fn with_merkle_roots(
+        mut self,
+        masternode_merkle_root: MerkleRootMasternodeList,
+        llmq_merkle_root: Option<MerkleRootQuorums>,
+    ) -> Self {
+        self.masternode_merkle_root = Some(masternode_merkle_root);
+        self.llmq_merkle_root = llmq_merkle_root;
+        self
+    }
+
+    pub fn build(self) -> MasternodeList {
+        let mut list = MasternodeList {
+            block_hash: self.block_hash,
+            known_height: self.block_height,
+            masternode_merkle_root: self.masternode_merkle_root,
+            llmq_merkle_root: self.llmq_merkle_root,
+            masternodes: self.masternodes,
+            quorums: self.quorums,
+        };
+
+        if self.masternode_merkle_root.is_none() {
+            list.masternode_merkle_root = list.calculate_masternodes_merkle_root(self.block_height);
+            list.llmq_merkle_root = list.calculate_llmq_merkle_root();
+        }
+
+        list
+    }
+}

dash/src/sml/masternode_list/builder.rs

@@ -31,7 +31,7 @@ impl MasternodeList {
             self.known_height,
             hex::encode(self.block_hash),
             self.masternode_merkle_root.map_or("None".to_string(), hex::encode),
-            self.masternode_count(),
+            self.masternodes.len(),
             self.llmq_merkle_root.map_or("None".to_string(), hex::encode),
             self.quorums_short_description()
         )

dash/src/sml/masternode_list/debug_helpers.rs

@@ -1,29 +1,14 @@
-use std::cmp::Ordering;
 use std::net::IpAddr;
-
-use hashes::Hash;
-
 use crate::ProTxHash;
 use crate::sml::masternode_list::MasternodeList;
-use crate::sml::masternode_list_entry::qualified_masternode_list_entry::QualifiedMasternodeListEntry;
 
 impl MasternodeList {
-    pub fn masternode_for(
-        &self,
-        pro_reg_tx_hash: &ProTxHash,
-    ) -> Option<&QualifiedMasternodeListEntry> {
-        self.masternodes.get(pro_reg_tx_hash)
-    }
-
     pub fn has_valid_masternode(&self, pro_reg_tx_hash: &ProTxHash) -> bool {
         self.masternodes
             .get(pro_reg_tx_hash)
             .map_or(false, |node| node.masternode_list_entry.is_valid)
     }
 
-    pub fn has_masternode(&self, pro_reg_tx_hash: &ProTxHash) -> bool {
-        self.masternodes.get(pro_reg_tx_hash).is_some()
-    }
     pub fn has_masternode_at_location(&self, address: [u8; 16], port: u16) -> bool {
         self.masternodes.values().any(|node| {
             match node.masternode_list_entry.service_address.ip() {
@@ -40,50 +25,10 @@ impl MasternodeList {
             }
         })
     }
-    pub fn masternode_count(&self) -> usize { self.masternodes.len() }
-
-    pub fn masternode_by_pro_reg_tx_hash(
-        &self,
-        registration_hash: &ProTxHash,
-    ) -> Option<QualifiedMasternodeListEntry> {
-        self.masternodes.get(registration_hash).cloned()
-    }
 
-    pub fn reversed_pro_reg_tx_hashes_cloned(&self) -> Vec<ProTxHash> {
-        self.masternodes.keys().cloned().collect()
-    }
     pub fn reversed_pro_reg_tx_hashes(&self) -> Vec<&ProTxHash> {
         self.masternodes.keys().collect()
     }
-
-    pub fn sorted_reversed_pro_reg_tx_hashes(&self) -> Vec<&ProTxHash> {
-        let mut hashes = self.reversed_pro_reg_tx_hashes();
-        hashes.sort_by(|&s1, &s2| s2.reverse().cmp(&s1.reverse()));
-        hashes
-    }
-
-    pub fn provider_tx_ordered_hashes(&self) -> Vec<ProTxHash> {
-        let mut vec = Vec::from_iter(self.masternodes.keys().cloned());
-        vec.sort_by(|hash1, hash2| {
-            if reverse_cmp_sup(hash1.to_byte_array(), hash2.to_byte_array()) {
-                Ordering::Greater
-            } else {
-                Ordering::Less
-            }
-        });
-        vec
-    }
-    pub fn compare_provider_tx_ordered_hashes(&self, list: MasternodeList) -> bool {
-        self.provider_tx_ordered_hashes().eq(&list.provider_tx_ordered_hashes())
-    }
-
-    pub fn compare_masternodes(&self, list: MasternodeList) -> bool {
-        let mut vec1 = Vec::from_iter(self.masternodes.values());
-        vec1.sort();
-        let mut vec2 = Vec::from_iter(list.masternodes.values());
-        vec2.sort();
-        vec1.eq(&vec2)
-    }
 }
 
 pub fn reverse_cmp_sup(lhs: [u8; 32], rhs: [u8; 32]) -> bool {

dash/src/sml/masternode_list/masternode_helpers.rs

@@ -20,7 +20,7 @@ use crate::transaction::special_transaction::TransactionPayload;
 /// - `Some([u8; 32])`: The computed Merkle root if at least one hash is provided.
 /// - `None`: If the input vector is empty.
 #[inline]
-pub fn merkle_root_from_hashes(hashes: Vec<[u8; 32]>) -> Option<[u8; 32]> {
+pub fn merkle_root_from_hashes(hashes: Vec<sha256d::Hash>) -> Option<sha256d::Hash> {
     let length = hashes.len();
     let mut level = hashes;
     match length {
@@ -29,12 +29,12 @@ pub fn merkle_root_from_hashes(hashes: Vec<[u8; 32]>) -> Option<[u8; 32]> {
             while level.len() != 1 {
                 let len = level.len();
                 let mut higher_level =
-                    Vec::<[u8; 32]>::with_capacity((0.5 * len as f64).ceil() as usize);
+                    Vec::<sha256d::Hash>::with_capacity((0.5 * len as f64).ceil() as usize);
                 for pair in level.chunks(2) {
                     let mut buffer = Vec::with_capacity(64);
-                    buffer.extend_from_slice(&pair[0]);
-                    buffer.extend_from_slice(pair.get(1).unwrap_or(&pair[0]));
-                    higher_level.push(sha256d::Hash::hash(&buffer).to_byte_array());
+                    buffer.extend_from_slice(pair[0].as_byte_array());
+                    buffer.extend_from_slice(pair.get(1).unwrap_or(&pair[0]).as_byte_array());
+                    higher_level.push(sha256d::Hash::hash(&buffer));
                 }
                 level = higher_level;
             }
@@ -126,7 +126,7 @@ impl MasternodeList {
     ) -> Option<MerkleRootMasternodeList> {
         self.hashes_for_merkle_root(block_height)
             .and_then(merkle_root_from_hashes)
-            .map(|hash| MerkleRootMasternodeList::from_byte_array(hash))
+            .map(MerkleRootMasternodeList::from_raw_hash)
     }
 
     /// Computes the Merkle root for the LLMQ (Long-Living Masternode Quorum) list.
@@ -140,7 +140,7 @@ impl MasternodeList {
     /// - `None`: If no quorum commitment hashes are available.
     pub fn calculate_llmq_merkle_root(&self) -> Option<MerkleRootQuorums> {
         merkle_root_from_hashes(self.hashes_for_quorum_merkle_root())
-            .map(|hash| MerkleRootQuorums::from_byte_array(hash))
+            .map(MerkleRootQuorums::from_raw_hash)
     }
 
     /// Retrieves the list of hashes required to compute the masternode list Merkle root.
@@ -154,21 +154,16 @@ impl MasternodeList {
     ///
     /// # Returns
     ///
-    /// - `Some(Vec<[u8; 32]>)`: A sorted list of masternode entry hashes.
+    /// - `Some(Vec<sha256d::Hash>)`: A sorted list of masternode entry hashes.
     /// - `None`: If the block height is invalid (`u32::MAX`).
-    pub fn hashes_for_merkle_root(&self, block_height: u32) -> Option<Vec<[u8; 32]>> {
+    pub fn hashes_for_merkle_root(&self, block_height: u32) -> Option<Vec<sha256d::Hash>> {
         (block_height != u32::MAX).then_some({
             let mut pro_tx_hashes = self.reversed_pro_reg_tx_hashes();
             pro_tx_hashes.sort_by(|&s1, &s2| s1.reverse().cmp(&s2.reverse()));
             pro_tx_hashes
                 .into_iter()
                 .map(|hash| self.masternodes[hash].entry_hash)
                 .collect::<Vec<_>>()
-            //this was the following: (with entry_hash_at)
-            // pro_tx_hashes
-            //     .into_iter()
-            //     .map(|hash| (&self.masternodes[hash]).entry_hash_at(block_height))
-            //     .collect::<Vec<_>>()
         })
     }
 
@@ -180,11 +175,11 @@ impl MasternodeList {
     /// # Returns
     ///
     /// - `Vec<[u8; 32]>`: A sorted list of quorum commitment hashes.
-    pub fn hashes_for_quorum_merkle_root(&self) -> Vec<[u8; 32]> {
+    pub fn hashes_for_quorum_merkle_root(&self) -> Vec<sha256d::Hash> {
         let mut llmq_commitment_hashes = self
             .quorums
             .values()
-            .flat_map(|q_map| q_map.values().map(|entry| entry.entry_hash.to_byte_array()))
+            .flat_map(|q_map| q_map.values().map(|entry| entry.entry_hash.to_raw_hash()))
             .collect::<Vec<_>>();
         llmq_commitment_hashes.sort();
         llmq_commitment_hashes