fix: index rotated quorums by quorum_index and rebuild per cycle

The `Vec<QualifiedQuorumEntry>` had no size bound and no uniqueness check on `quorum_index`. Combined with QRInfo still being requested on every new block (will change soon), repeated `feed_qr_info` calls within a single rotation cycle accumulated duplicate entries that the lookup then returned as stale matches.

- Switch the inner container to `BTreeMap<u16, QualifiedQuorumEntry>` keyed by `quorum_index` so insertion replaces by key, lookup is direct, and re-feeds of the same cycle are not causing wrong behaviour anymore.
- Introduce `build_cycle_quorum_map` to centralize map construction with index validation and count checks. Both branches of `feed_qr_info` use it to rebuild the per-cycle map from scratch.
- Migrates `tests/data/test_DML_diffs/masternode_list_engine.hex` to the new format.

Author: xdustinface

dash/src/sml/masternode_list_engine/message_request_verification.rs

@@ -1,3 +1,5 @@
+use std::collections::BTreeMap;
+
 use hashes::Hash;
 
 use crate::hash_types::QuorumOrderingHash;
@@ -12,7 +14,7 @@ impl MasternodeListEngine {
     fn is_lock_potential_quorums(
         &self,
         instant_lock: &InstantLock,
-    ) -> Result<&Vec<QualifiedQuorumEntry>, MessageVerificationError> {
+    ) -> Result<&BTreeMap<u16, QualifiedQuorumEntry>, MessageVerificationError> {
         // Retrieve the cycle hash from the Instant Lock
         let cycle_hash = instant_lock.cyclehash;
 
@@ -29,11 +31,11 @@ impl MasternodeListEngine {
             .ok_or(MessageVerificationError::CycleHashNotPresent(cycle_hash))?;
 
         tracing::debug!("Found {} quorums for cyclehash {}", quorums.len(), cycle_hash);
-        for q in quorums.iter() {
+        for (idx, q) in quorums.iter() {
             tracing::debug!(
-                "  Quorum: hash={}, index={:?}, height at block_container={:?}",
+                "  Quorum: index={}, hash={}, height at block_container={:?}",
+                idx,
                 q.quorum_entry.quorum_hash,
-                q.quorum_entry.quorum_index,
                 self.block_container.get_height(&q.quorum_entry.quorum_hash)
             );
         }
@@ -126,16 +128,13 @@ impl MasternodeListEngine {
             quorum_index
         );
 
-        // Find the quorum by its quorum_index field.
-        let quorum = quorums
-            .iter()
-            .find(|q| q.quorum_entry.quorum_index == Some(quorum_index as i16))
-            .ok_or({
-                MessageVerificationError::QuorumIndexNotFound(
-                    quorum_index as u16,
-                    instant_lock.cyclehash,
-                )
-            })?;
+        // Look up the quorum by index.
+        let quorum = quorums.get(&(quorum_index as u16)).ok_or({
+            MessageVerificationError::QuorumIndexNotFound(
+                quorum_index as u16,
+                instant_lock.cyclehash,
+            )
+        })?;
 
         tracing::debug!(
             "IS lock selected quorum: hash={}, index={:?}, public_key={}, verified={:?}",
@@ -540,50 +539,6 @@ mod tests {
         mn_list_engine.verify_chain_lock(&chain_lock).expect("expected to verify chain lock");
     }
 
-    /// Test that quorums are looked up by their quorum_index field, not by array position.
-    #[test]
-    pub fn is_lock_quorum_lookup_by_quorum_index_not_array_position() {
-        let block_hex =
-            include_str!("../../../tests/data/test_DML_diffs/masternode_list_engine.hex");
-        let data = hex::decode(block_hex).expect("decode hex");
-        let mut mn_list_engine: MasternodeListEngine =
-            bincode::decode_from_slice(&data, bincode::config::standard())
-                .expect("expected to decode")
-                .0;
-
-        let lock_data = hex::decode("01018d53e7997ead57409750942af0d5e0aafc06f852a9a52308f4781b6a8220298f00000000c6f9d8c63dd15937ea70aaddb7890daad42c91bf6818e2bf76d183d6f2d9215b4b5f84978fad9dde7ab52bdcc0674be891e9029cc1ef0cb01200000000000000a27c98836c4c04653ab81eb4e07ddfc2c8c2c1036b75247969c05a4f25451cd78913a971f1899d9f2bddec9cf8e0104004f72f20c2856453e5aa3bcd2a8200670ec28feda38f67cc400fc72ef1966956656ec0765478c9d16e9a9e470c07f9ed").expect("expected valid hex");
-        let lock: InstantLock = deserialize(lock_data.as_slice()).expect("expected to deserialize");
-
-        // Get the original result
-        let (original_quorum, _, original_index) =
-            mn_list_engine.is_lock_quorum(&lock).expect("expected to get quorum");
-        let expected_quorum_hash = original_quorum.quorum_entry.quorum_hash;
-        let expected_quorum_index = original_quorum.quorum_entry.quorum_index;
-        assert_eq!(original_index, 23);
-        assert_eq!(expected_quorum_index, Some(23));
-
-        // Reverse the quorum array order so array positions no longer match quorum indices
-        let cycle_hash = lock.cyclehash;
-        if let Some(quorums) = mn_list_engine.rotated_quorums_per_cycle.get_mut(&cycle_hash) {
-            quorums.reverse();
-
-            // Verify the quorum with index 23 is no longer at position 23
-            let quorum_at_pos_23 = quorums.get(23);
-            assert!(
-                quorum_at_pos_23.is_none()
-                    || quorum_at_pos_23.unwrap().quorum_entry.quorum_index != Some(23),
-                "after reversing, quorum at position 23 should not have quorum_index 23"
-            );
-        }
-
-        // The lookup should still find the correct quorum by its quorum_index field
-        let (found_quorum, _, found_index) =
-            mn_list_engine.is_lock_quorum(&lock).expect("expected to find quorum by index");
-        assert_eq!(found_index, 23);
-        assert_eq!(found_quorum.quorum_entry.quorum_hash, expected_quorum_hash);
-        assert_eq!(found_quorum.quorum_entry.quorum_index, expected_quorum_index);
-    }
-
     /// Test that QuorumIndexNotFound error is returned when the required quorum index is missing.
     #[test]
     pub fn is_lock_quorum_not_found_error() {
@@ -607,7 +562,7 @@ mod tests {
         // Remove the quorum with index 23 from the cycle
         let cycle_hash = lock.cyclehash;
         if let Some(quorums) = mn_list_engine.rotated_quorums_per_cycle.get_mut(&cycle_hash) {
-            quorums.retain(|q| q.quorum_entry.quorum_index != Some(23));
+            quorums.remove(&23);
         }
 
         // Now the lookup should return QuorumIndexNotFound

dash/src/sml/masternode_list_engine/mod.rs

@@ -173,7 +173,7 @@ pub struct MasternodeListEngine {
     pub block_container: MasternodeListEngineBlockContainer,
     pub masternode_lists: BTreeMap<CoreBlockHeight, MasternodeList>,
     pub known_snapshots: BTreeMap<BlockHash, QuorumSnapshot>,
-    pub rotated_quorums_per_cycle: BTreeMap<BlockHash, Vec<QualifiedQuorumEntry>>,
+    pub rotated_quorums_per_cycle: BTreeMap<BlockHash, BTreeMap<u16, QualifiedQuorumEntry>>,
     #[allow(clippy::type_complexity)]
     pub quorum_statuses: BTreeMap<
         LLMQType,
@@ -198,6 +198,38 @@ impl Default for MasternodeListEngine {
     }
 }
 
+/// Builds a per-cycle quorum map keyed by `quorum_index`.
+/// Rejects missing, negative, or duplicate indices and validates the final count.
+#[cfg(feature = "quorum_validation")]
+fn build_cycle_quorum_map(
+    quorums: Vec<QualifiedQuorumEntry>,
+    rotation_quorum_type: LLMQType,
+) -> Result<BTreeMap<u16, QualifiedQuorumEntry>, QuorumValidationError> {
+    let mut map = BTreeMap::new();
+    for quorum in quorums {
+        let quorum_index = quorum.quorum_entry.quorum_index.ok_or(
+            QuorumValidationError::RequiredQuorumIndexNotPresent(quorum.quorum_entry.quorum_hash),
+        )?;
+        let key = u16::try_from(quorum_index).map_err(|_| {
+            QuorumValidationError::RequiredQuorumIndexNotPresent(quorum.quorum_entry.quorum_hash)
+        })?;
+        if map.contains_key(&key) {
+            return Err(QuorumValidationError::CorruptedCodeExecution(format!(
+                "duplicate quorum_index {key} in rotation cycle"
+            )));
+        }
+        map.insert(key, quorum);
+    }
+    let expected = rotation_quorum_type.active_quorum_count() as usize;
+    if map.len() != expected {
+        return Err(QuorumValidationError::CorruptedCodeExecution(format!(
+            "rotated quorums per cycle count mismatch: expected {expected}, got {}",
+            map.len()
+        )));
+    }
+    Ok(map)
+}
+
 impl MasternodeListEngine {
     /// Creates a new MasternodeListEngine with the specified network configuration.
     ///
@@ -384,7 +416,7 @@ impl MasternodeListEngine {
         self.rotated_quorums_per_cycle
             .values()
             .find_map(|qualified_entries| {
-                qualified_entries.iter().find(|qualified_entry| {
+                qualified_entries.values().find(|qualified_entry| {
                     qualified_entry.quorum_entry.quorum_hash == quorum_entry.quorum_hash
                         && qualified_entry.quorum_entry.llmq_type == quorum_entry.llmq_type
                 })
@@ -628,7 +660,7 @@ impl MasternodeListEngine {
             self.apply_diff(mn_list_diff_tip, None, verify_tip_non_rotated_quorums, sigs)?;
 
         #[cfg(feature = "quorum_validation")]
-        let qualified_last_commitment_per_index = last_commitment_per_index
+        let mut qualified_last_commitment_per_index = last_commitment_per_index
             .into_iter()
             .map(|quorum_entry| {
                 if let Some(qualified_quorum_entry) =
@@ -685,27 +717,22 @@ impl MasternodeListEngine {
                 LLMQEntryVerificationStatus,
             )> = Vec::new();
 
-            let mut qualified_rotated_quorums_per_cycle =
-                qualified_last_commitment_per_index.first().map(|quorum_entry| {
-                    self.rotated_quorums_per_cycle
-                        .entry(quorum_entry.quorum_entry.quorum_hash)
-                        .or_default()
-                });
+            let cycle_key =
+                qualified_last_commitment_per_index.first().map(|q| q.quorum_entry.quorum_hash);
 
-            for mut rotated_quorum in qualified_last_commitment_per_index {
+            for rotated_quorum in qualified_last_commitment_per_index.iter_mut() {
                 tracing::debug!(
-                    "  Current cycle quorum: hash={}, index={:?}",
+                    "  Current cycle quorum: hash={}, raw_quorum_index={:?}, map_key={:?}",
                     rotated_quorum.quorum_entry.quorum_hash,
-                    rotated_quorum.quorum_entry.quorum_index
+                    rotated_quorum.quorum_entry.quorum_index,
+                    rotated_quorum.quorum_entry.quorum_index.and_then(|i| u16::try_from(i).ok())
                 );
 
                 rotated_quorum.verified = validation_statuses
                     .get(&rotated_quorum.quorum_entry.quorum_hash)
                     .cloned()
                     .unwrap_or_default();
 
-                qualified_rotated_quorums_per_cycle.as_mut().unwrap().push(rotated_quorum.clone());
-
                 // Store status updates separately to prevent multiple mutable borrows
                 let masternode_lists_having_quorum_hash_for_quorum_type =
                     self.quorum_statuses.entry(rotated_quorum.quorum_entry.llmq_type).or_default();
@@ -727,6 +754,14 @@ impl MasternodeListEngine {
                 *status = rotated_quorum.verified.clone();
             }
 
+            if let Some(key) = cycle_key {
+                let cycle_map = build_cycle_quorum_map(
+                    qualified_last_commitment_per_index,
+                    rotation_quorum_type,
+                )?;
+                *self.rotated_quorums_per_cycle.entry(key).or_default() = cycle_map;
+            }
+
             // Apply collected updates after iteration to avoid borrow conflicts
             for (heights, quorum_type, quorum_hash, new_status) in updates {
                 for height in heights {
@@ -816,14 +851,12 @@ impl MasternodeListEngine {
                     }
                 }
             }
-        } else if let Some(qualified_rotated_quorums_per_cycle) =
-            qualified_last_commitment_per_index.first().map(|quorum_entry| {
-                self.rotated_quorums_per_cycle
-                    .entry(quorum_entry.quorum_entry.quorum_hash)
-                    .or_default()
-            })
+        } else if let Some(cycle_key) =
+            qualified_last_commitment_per_index.first().map(|q| q.quorum_entry.quorum_hash)
         {
-            *qualified_rotated_quorums_per_cycle = qualified_last_commitment_per_index;
+            let cycle_map =
+                build_cycle_quorum_map(qualified_last_commitment_per_index, rotation_quorum_type)?;
+            *self.rotated_quorums_per_cycle.entry(cycle_key).or_default() = cycle_map;
         }
 
         #[cfg(not(feature = "quorum_validation"))]
@@ -1065,7 +1098,7 @@ impl MasternodeListEngine {
                     && let Some(cycle_quorums) = self.rotated_quorums_per_cycle.get(&cycle_hash)
                 {
                     // Only update rotating quorum statuses based on last commitment entries
-                    for quorum in cycle_quorums {
+                    for quorum in cycle_quorums.values() {
                         if let Some(quorum_entry) =
                             hash_to_quorum_entries.get_mut(&quorum.quorum_entry.quorum_hash)
                         {
@@ -1134,9 +1167,8 @@ impl MasternodeListEngine {
 
 #[cfg(test)]
 mod tests {
-    use crate::BlockHash;
-    use crate::Network;
     use crate::consensus::deserialize;
+    use crate::hashes::Hash;
     use crate::network::message_qrinfo::QRInfo;
     use crate::network::message_sml::MnListDiff;
     use crate::prelude::CoreBlockHeight;
@@ -1149,10 +1181,87 @@ mod tests {
     use crate::sml::masternode_list_engine::{
         MasternodeListEngine, MasternodeListEngineBlockContainer,
     };
-    use crate::sml::quorum_entry::qualified_quorum_entry::VerifyingChainLockSignaturesType;
+    use crate::sml::quorum_entry::qualified_quorum_entry::{
+        QualifiedQuorumEntry, VerifyingChainLockSignaturesType,
+    };
     use crate::sml::quorum_validation_error::ClientDataRetrievalError;
+    #[cfg(feature = "quorum_validation")]
+    use crate::sml::quorum_validation_error::QuorumValidationError;
+    use crate::{BlockHash, Network};
     use std::collections::BTreeMap;
 
+    #[cfg(feature = "quorum_validation")]
+    use {
+        super::build_cycle_quorum_map,
+        crate::QuorumHash,
+        crate::bls_sig_utils::{BLSPublicKey, BLSSignature},
+        crate::hash_types::QuorumVVecHash,
+        crate::transaction::special_transaction::quorum_commitment::QuorumEntry,
+    };
+
+    #[cfg(feature = "quorum_validation")]
+    fn make_qualified_quorum_entry(
+        llmq_type: LLMQType,
+        quorum_index: Option<i16>,
+    ) -> QualifiedQuorumEntry {
+        QuorumEntry {
+            version: 2,
+            llmq_type,
+            quorum_hash: QuorumHash::all_zeros(),
+            quorum_index,
+            signers: vec![true],
+            valid_members: vec![true],
+            quorum_public_key: BLSPublicKey::from([0; 48]),
+            quorum_vvec_hash: QuorumVVecHash::all_zeros(),
+            threshold_sig: BLSSignature::from([0; 96]),
+            all_commitment_aggregated_signature: BLSSignature::from([0; 96]),
+        }
+        .into()
+    }
+
+    #[cfg(feature = "quorum_validation")]
+    #[test]
+    fn build_cycle_quorum_map_edge_cases() {
+        let ty = LLMQType::LlmqtypeTest;
+        assert_eq!(ty.active_quorum_count(), 2, "test assumes active_quorum_count == 2");
+
+        // Valid: two quorums with distinct indices
+        let quorums = vec![
+            make_qualified_quorum_entry(ty, Some(0)),
+            make_qualified_quorum_entry(ty, Some(1)),
+        ];
+        let map = build_cycle_quorum_map(quorums, ty).expect("valid quorums should succeed");
+        assert_eq!(map.len(), 2);
+        assert!(map.contains_key(&0) && map.contains_key(&1));
+
+        // Missing index is rejected
+        let quorums =
+            vec![make_qualified_quorum_entry(ty, Some(0)), make_qualified_quorum_entry(ty, None)];
+        let err = build_cycle_quorum_map(quorums, ty).expect_err("missing index should fail");
+        assert!(matches!(err, QuorumValidationError::RequiredQuorumIndexNotPresent(_)));
+
+        // Negative index is rejected
+        let quorums = vec![
+            make_qualified_quorum_entry(ty, Some(0)),
+            make_qualified_quorum_entry(ty, Some(-1)),
+        ];
+        let err = build_cycle_quorum_map(quorums, ty).expect_err("negative index should fail");
+        assert!(matches!(err, QuorumValidationError::RequiredQuorumIndexNotPresent(_)));
+
+        // Duplicate index is rejected
+        let quorums = vec![
+            make_qualified_quorum_entry(ty, Some(0)),
+            make_qualified_quorum_entry(ty, Some(0)),
+        ];
+        let err = build_cycle_quorum_map(quorums, ty).expect_err("duplicate index should fail");
+        assert!(matches!(err, QuorumValidationError::CorruptedCodeExecution(_)));
+
+        // Wrong count is rejected
+        let quorums = vec![make_qualified_quorum_entry(ty, Some(0))];
+        let err = build_cycle_quorum_map(quorums, ty).expect_err("wrong count should fail");
+        assert!(matches!(err, QuorumValidationError::CorruptedCodeExecution(_)));
+    }
+
     fn verify_masternode_list_quorums(
         mn_list_engine: &MasternodeListEngine,
         masternode_list: &MasternodeList,
@@ -1413,9 +1522,12 @@ mod tests {
                 .0;
 
         for (cycle_hash, quorums) in mn_list_engine.rotated_quorums_per_cycle.iter() {
-            for (i, quorum) in quorums.iter().enumerate() {
+            for (index, quorum) in quorums.iter() {
                 mn_list_engine.validate_quorum(quorum).unwrap_or_else(|_| {
-                    panic!("expected to validate quorum {} in cycle hash {}", i, cycle_hash)
+                    panic!(
+                        "expected to validate quorum at index {} in cycle hash {}",
+                        index, cycle_hash
+                    )
                 });
             }
         }
@@ -1433,7 +1545,7 @@ mod tests {
 
         for quorums in mn_list_engine.rotated_quorums_per_cycle.values() {
             mn_list_engine
-                .validate_rotation_cycle_quorums(quorums.iter().collect::<Vec<_>>().as_slice())
+                .validate_rotation_cycle_quorums(quorums.values().collect::<Vec<_>>().as_slice())
                 .expect("expected to validated quorums");
         }
     }