feat: validate masternode list merkle root

dash/src/sml/address.rs

@@ -40,7 +40,7 @@ impl Decodable for SocketAddr {
 
         let ipv6 = Ipv6Addr::from_bits(ip);
 
-        if let Some(ipv4) = ipv6.to_ipv4() {
+        if let Some(ipv4) = ipv6.to_ipv4_mapped() {
             Ok(SocketAddr::V4(SocketAddrV4::new(ipv4, port)))
         } else {
             Ok(SocketAddr::V6(SocketAddrV6::new(ipv6, port, 0, 0)))
@@ -86,6 +86,20 @@ mod tests {
         assert_eq!(writer, decoded_writer);
     }
 
+    #[test]
+    fn encode_decode_unspecified_preserves_bytes() {
+        // An all-zero (`::`) address must round-trip to the same 16 zero bytes. Decoding it as
+        // IPv4 `0.0.0.0` would re-encode with the `::ffff:` mapped prefix and corrupt the bytes,
+        // which in turn breaks the masternode entry hash for entries with an unset service.
+        let original = [0u8; 18];
+        let mut reader = &original[..];
+        let decoded = SocketAddr::consensus_decode(&mut reader).unwrap();
+
+        let mut writer = Vec::new();
+        decoded.consensus_encode(&mut writer).unwrap();
+        assert_eq!(writer, original);
+    }
+
     #[test]
     fn encode_decode_ipv6() {
         let address = SocketAddr::V6(SocketAddrV6::new(

dash/src/sml/error.rs

@@ -3,6 +3,7 @@ use bincode::{Decode, Encode};
 use thiserror::Error;
 
 use crate::BlockHash;
+use crate::hash_types::MerkleRootMasternodeList;
 
 #[derive(Debug, Error, Clone, PartialEq, Eq, Ord, PartialOrd, Hash)]
 #[cfg_attr(feature = "bincode", derive(Encode, Decode))]
@@ -50,4 +51,16 @@ pub enum SmlError {
     /// Error indicating the quorum signature set is incomplete (some slots were not filled).
     #[error("Incomplete quorum signature set; not all slots were filled")]
     IncompleteSignatureSet,
+
+    /// The recomputed masternode list Merkle root does not match the root committed in the
+    /// coinbase transaction, so the diff must be rejected and the list must not advance.
+    #[error(
+        "Masternode list Merkle root mismatch at height {block_height} (block {block_hash}): coinbase committed {expected}, recomputed {calculated}"
+    )]
+    MasternodeListRootMismatch {
+        block_hash: BlockHash,
+        block_height: u32,
+        expected: MerkleRootMasternodeList,
+        calculated: MerkleRootMasternodeList,
+    },
 }

dash/src/sml/masternode_list/apply_diff.rs

@@ -161,29 +161,68 @@ impl MasternodeList {
             diff_end_height,
         );
 
-        Ok((builder.build(), rotating_sig))
+        let updated_list = builder.build();
+
+        updated_list.validate_mn_list_root(&diff.coinbase_tx, diff_end_height)?;
+
+        Ok((updated_list, rotating_sig))
     }
 }
 
 #[cfg(test)]
 mod tests {
+    use std::collections::BTreeMap;
+
     use super::*;
     use crate::consensus::deserialize;
     use crate::sml::masternode_list::from_diff::TryFromWithBlockHashLookup;
 
-    #[test]
-    fn apply_diff_post_v20_requires_chainlock_signatures() {
-        // Create base list from first diff
+    /// Builds a base list from the from-genesis capture fixture, rewriting its coinbase root so it
+    /// passes production validation (the captured subset does not reproduce the mainnet full-list
+    /// root the fixture commits to).
+    fn consistent_base_list(height: u32) -> MasternodeList {
         let base_diff_bytes: &[u8] =
             include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin");
-        let base_diff: MnListDiff = deserialize(base_diff_bytes).expect("expected to deserialize");
-
-        let base_list = MasternodeList::try_from_with_block_hash_lookup(
+        let mut base_diff: MnListDiff =
+            deserialize(base_diff_bytes).expect("expected to deserialize");
+        let masternodes = base_diff
+            .new_masternodes
+            .iter()
+            .map(|entry| (entry.pro_reg_tx_hash.reverse(), entry.clone().into()))
+            .collect();
+        let assembled =
+            MasternodeList::build(masternodes, BTreeMap::new(), base_diff.block_hash, height)
+                .build();
+        MasternodeList::rewrite_coinbase_mn_list_root(
+            &mut base_diff.coinbase_tx,
+            &assembled,
+            height,
+        );
+        MasternodeList::try_from_with_block_hash_lookup(
             base_diff,
-            |_| Some(2_227_096),
+            |_| Some(height),
             Network::Mainnet,
         )
-        .expect("expected to create base list");
+        .expect("expected to create base list")
+    }
+
+    /// Rewrites a diff's coinbase root to the value applying it on top of `base` produces.
+    fn make_diff_consistent(base: &MasternodeList, diff: &mut MnListDiff, height: u32) {
+        let mut masternodes = base.masternodes.clone();
+        for pro_tx_hash in &diff.deleted_masternodes {
+            masternodes.remove(&pro_tx_hash.reverse());
+        }
+        for new_mn in &diff.new_masternodes {
+            masternodes.insert(new_mn.pro_reg_tx_hash.reverse(), new_mn.clone().into());
+        }
+        let assembled =
+            MasternodeList::build(masternodes, BTreeMap::new(), diff.block_hash, height).build();
+        MasternodeList::rewrite_coinbase_mn_list_root(&mut diff.coinbase_tx, &assembled, height);
+    }
+
+    #[test]
+    fn apply_diff_post_v20_requires_chainlock_signatures() {
+        let base_list = consistent_base_list(2_227_096);
 
         // Load second diff and clear signatures
         let diff_bytes: &[u8] =
@@ -205,18 +244,8 @@ mod tests {
 
     #[test]
     fn apply_diff_pre_v20_allows_missing_chainlock_signatures() {
-        // Create base list from first diff at pre-V20 height
-        let base_diff_bytes: &[u8] =
-            include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin");
-        let base_diff: MnListDiff = deserialize(base_diff_bytes).expect("expected to deserialize");
-
         let base_height = 1_800_000u32;
-        let base_list = MasternodeList::try_from_with_block_hash_lookup(
-            base_diff,
-            |_| Some(base_height),
-            Network::Mainnet,
-        )
-        .expect("expected to create base list");
+        let base_list = consistent_base_list(base_height);
 
         // Load second diff and clear signatures
         let diff_bytes: &[u8] =
@@ -231,6 +260,8 @@ mod tests {
         let pre_v20_height = 1_900_000u32;
         assert!(pre_v20_height < Network::Mainnet.v20_activation_height());
 
+        make_diff_consistent(&base_list, &mut diff, pre_v20_height);
+
         let result = base_list.apply_diff(diff, pre_v20_height, None, Network::Mainnet);
 
         assert!(
@@ -239,4 +270,24 @@ mod tests {
             result.err()
         );
     }
+
+    #[test]
+    fn apply_diff_rejects_coinbase_mn_list_root_mismatch() {
+        let base_list = consistent_base_list(1_900_000);
+
+        let diff_bytes: &[u8] =
+            include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_2227096_2241332.bin");
+        let mut diff: MnListDiff = deserialize(diff_bytes).expect("expected to deserialize");
+        diff.base_block_hash = base_list.block_hash;
+        diff.quorums_chainlock_signatures.clear();
+
+        // No coinbase-root rewrite: the fixture's committed root must not match the assembled list.
+        let result = base_list.apply_diff(diff, 1_900_000, None, Network::Mainnet);
+
+        assert!(
+            matches!(result, Err(SmlError::MasternodeListRootMismatch { .. })),
+            "apply_diff must reject a diff whose coinbase root disagrees with the assembled list: {:?}",
+            result
+        );
+    }
 }

dash/src/sml/masternode_list/from_diff.rs

@@ -87,6 +87,8 @@ impl TryFromWithBlockHashLookup<MnListDiff> for MasternodeList {
             return Err(SmlError::IncompleteMnListDiff);
         }
 
+        let coinbase_tx = diff.coinbase_tx.clone();
+
         // Populate masternode and quorum maps
         let masternodes = diff
             .new_masternodes
@@ -143,15 +145,14 @@ impl TryFromWithBlockHashLookup<MnListDiff> for MasternodeList {
             },
         );
 
-        // Construct `MasternodeList`
-        Ok(MasternodeList {
-            block_hash: diff.block_hash,
-            known_height,
-            masternode_merkle_root: diff.merkle_hashes.first().cloned(),
-            llmq_merkle_root: None, // Adjust based on real data availability
-            masternodes,
-            quorums,
-        })
+        // Construct `MasternodeList`, recomputing the Merkle roots over the assembled entry set
+        // instead of trusting any value carried in the diff.
+        let masternode_list =
+            MasternodeList::build(masternodes, quorums, diff.block_hash, known_height).build();
+
+        masternode_list.validate_mn_list_root(&coinbase_tx, known_height)?;
+
+        Ok(masternode_list)
     }
 }
 
@@ -160,6 +161,22 @@ mod tests {
     use super::*;
     use crate::consensus::deserialize;
 
+    /// Rewrites the diff's coinbase masternode list root to match the list its own
+    /// `new_masternodes` set produces. The from-genesis capture fixture commits a root over the
+    /// full mainnet list which the captured subset does not reproduce, so without this it would be
+    /// rejected by root validation even though the unrelated chainlock-signature behaviour under
+    /// test is correct.
+    fn make_from_genesis_diff_consistent(diff: &mut MnListDiff, height: u32) {
+        let masternodes = diff
+            .new_masternodes
+            .iter()
+            .map(|entry| (entry.pro_reg_tx_hash.reverse(), entry.clone().into()))
+            .collect();
+        let assembled =
+            MasternodeList::build(masternodes, BTreeMap::new(), diff.block_hash, height).build();
+        MasternodeList::rewrite_coinbase_mn_list_root(&mut diff.coinbase_tx, &assembled, height);
+    }
+
     #[test]
     fn post_v20_requires_chainlock_signatures() {
         let mn_list_diff_bytes: &[u8] =
@@ -200,6 +217,8 @@ mod tests {
         let pre_v20_height = 1_900_000;
         assert!(pre_v20_height < Network::Mainnet.v20_activation_height());
 
+        make_from_genesis_diff_consistent(&mut diff, pre_v20_height);
+
         let result = MasternodeList::try_from_with_block_hash_lookup(
             diff,
             |_| Some(pre_v20_height),
@@ -212,4 +231,25 @@ mod tests {
             result.err()
         );
     }
+
+    #[test]
+    fn rejects_coinbase_mn_list_root_mismatch() {
+        let mn_list_diff_bytes: &[u8] =
+            include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin");
+        let diff: MnListDiff = deserialize(mn_list_diff_bytes).expect("expected to deserialize");
+
+        // The capture fixture's coinbase commits a root over the full mainnet list that the
+        // captured `new_masternodes` subset does not reproduce, so building it must hard-reject.
+        let result = MasternodeList::try_from_with_block_hash_lookup(
+            diff,
+            |_| Some(1_900_000),
+            Network::Mainnet,
+        );
+
+        assert!(
+            matches!(result, Err(SmlError::MasternodeListRootMismatch { .. })),
+            "a diff whose coinbase root disagrees with its entry set must be rejected: {:?}",
+            result
+        );
+    }
 }

dash/src/sml/masternode_list/merkle_roots.rs

@@ -2,6 +2,7 @@ use hashes::{Hash, sha256d};
 
 use crate::Transaction;
 use crate::hash_types::{MerkleRootMasternodeList, MerkleRootQuorums};
+use crate::sml::error::SmlError;
 use crate::sml::masternode_list::MasternodeList;
 use crate::transaction::special_transaction::TransactionPayload;
 
@@ -44,31 +45,68 @@ pub fn merkle_root_from_hashes(hashes: Vec<sha256d::Hash>) -> Option<sha256d::Ha
 }
 
 impl MasternodeList {
-    /// Validates whether the stored masternode list Merkle root matches the one in the coinbase transaction.
-    ///
-    /// This function compares the calculated masternode Merkle root with the one provided
-    /// in the coinbase transaction payload to verify the integrity of the masternode list.
-    ///
-    /// # Parameters
-    ///
-    /// - `coinbase_transaction`: The coinbase transaction containing the expected Merkle root.
-    ///
-    /// # Returns
-    ///
-    /// - `true` if the Merkle root matches.
-    /// - `false` otherwise.
-    pub fn has_valid_mn_list_root(&self, coinbase_transaction: &Transaction) -> bool {
-        let Some(TransactionPayload::CoinbasePayloadType(coinbase_payload)) =
-            &coinbase_transaction.special_transaction_payload
-        else {
-            return false;
-        };
-        // we need to check that the coinbase is in the transaction hashes we got back
-        // and is in the merkle block
-        if let Some(mn_merkle_root) = self.masternode_merkle_root {
-            coinbase_payload.merkle_root_masternode_list == mn_merkle_root
+    /// Extracts the masternode list Merkle root committed in a coinbase transaction's payload.
+    fn coinbase_mn_list_root(
+        coinbase_transaction: &Transaction,
+    ) -> Option<MerkleRootMasternodeList> {
+        match &coinbase_transaction.special_transaction_payload {
+            Some(TransactionPayload::CoinbasePayloadType(coinbase_payload)) => {
+                Some(coinbase_payload.merkle_root_masternode_list)
+            }
+            _ => None,
+        }
+    }
+
+    /// Recomputes the masternode list Merkle root over the fully assembled list and verifies it
+    /// against the root committed in the coinbase transaction. The list must never be advanced on
+    /// mismatch.
+    ///
+    /// The recomputation is done from scratch over the current entry set rather than trusting any
+    /// stored root, so this is the authoritative check applied right after a list is built from a
+    /// diff.
+    pub fn validate_mn_list_root(
+        &self,
+        coinbase_transaction: &Transaction,
+        block_height: u32,
+    ) -> Result<(), SmlError> {
+        let expected = Self::coinbase_mn_list_root(coinbase_transaction)
+            .ok_or(SmlError::IncompleteMnListDiff)?;
+
+        let calculated = self
+            .calculate_masternodes_merkle_root(block_height)
+            .unwrap_or_else(|| MerkleRootMasternodeList::from_raw_hash(sha256d::Hash::all_zeros()));
+
+        if expected == calculated {
+            Ok(())
         } else {
-            false
+            Err(SmlError::MasternodeListRootMismatch {
+                block_hash: self.block_hash,
+                block_height,
+                expected,
+                calculated,
+            })
+        }
+    }
+
+    /// Overwrites the masternode list Merkle root committed in a diff's coinbase transaction with
+    /// the value that recomputing over `assembled` produces. The historical capture fixtures in
+    /// this crate carry coinbase roots that were computed by Dash Core over a full list which the
+    /// captured `new_masternodes` set does not reproduce, so they would otherwise be rejected by
+    /// the production root validation. This makes such a fixture self-consistent for tests that
+    /// exercise unrelated quorum and chainlock logic.
+    #[cfg(test)]
+    pub(crate) fn rewrite_coinbase_mn_list_root(
+        coinbase_transaction: &mut Transaction,
+        assembled: &MasternodeList,
+        block_height: u32,
+    ) {
+        let root = assembled
+            .calculate_masternodes_merkle_root(block_height)
+            .unwrap_or_else(|| MerkleRootMasternodeList::from_raw_hash(sha256d::Hash::all_zeros()));
+        if let Some(TransactionPayload::CoinbasePayloadType(payload)) =
+            &mut coinbase_transaction.special_transaction_payload
+        {
+            payload.merkle_root_masternode_list = root;
         }
     }