fix: require chainlock signatures only after V20 activation (#405)

Pre-V20 blocks have no chainlock signatures in `MnListDiff` responses, but we currently incorrectly trigger failures
when they are missing below v20 activation height.

V20 activation heights (from Dash Core src/chainparams.cpp):
- Mainnet: 1,987,776
- Testnet: 905,100

Added `v20_activation_height()` to `NetworkExt` and check in
`apply_diff` and `from_diff` to only require signatures for post-V20 blocks.

See DIP-0029.

Author: xdustinface

dash/src/network/constants.rs

@@ -69,6 +69,10 @@ pub const PROTOCOL_VERSION: u32 = 70237;
 pub trait NetworkExt {
     /// The known dash genesis block hash for mainnet and testnet
     fn known_genesis_block_hash(&self) -> Option<BlockHash>;
+
+    /// V20 activation height when quorumsCLSigs was introduced (protocol 70230).
+    /// See DIP-0029 and Dash Core src/chainparams.cpp.
+    fn v20_activation_height(&self) -> u32;
 }
 
 impl NetworkExt for Network {
@@ -99,6 +103,15 @@ impl NetworkExt for Network {
             _ => None,
         }
     }
+
+    fn v20_activation_height(&self) -> u32 {
+        match self {
+            Network::Dash => 1_987_776,
+            Network::Testnet => 905_100,
+            // Devnet and regtest activate V20 immediately
+            _ => 0,
+        }
+    }
 }
 
 /// Flags to indicate which network services a node supports.

dash/src/sml/masternode_list/apply_diff.rs

@@ -1,4 +1,5 @@
 use crate::bls_sig_utils::BLSSignature;
+use crate::network::constants::NetworkExt;
 use crate::network::message_sml::MnListDiff;
 use crate::prelude::CoreBlockHeight;
 use crate::sml::error::SmlError;
@@ -9,6 +10,7 @@ use crate::sml::masternode_list::MasternodeList;
 use crate::sml::quorum_entry::qualified_quorum_entry::{
     QualifiedQuorumEntry, VerifyingChainLockSignaturesType,
 };
+use dash_network::Network;
 
 impl MasternodeList {
     /// Applies an `MnListDiff` to update the current masternode list.
@@ -40,6 +42,7 @@ impl MasternodeList {
         diff: MnListDiff,
         diff_end_height: CoreBlockHeight,
         previous_chain_lock_sigs: Option<[BLSSignature; 3]>,
+        network: Network,
     ) -> Result<(MasternodeList, Option<BLSSignature>), SmlError> {
         // Ensure the base block hash matches
         if self.block_hash != diff.base_block_hash {
@@ -89,8 +92,12 @@ impl MasternodeList {
             }
         }
 
-        // Verify all slots have been filled
-        if quorum_sig_lookup.iter().any(Option::is_none) {
+        // quorumsCLSigs only exists after V20 activation (protocol 70230).
+        // Pre-V20 blocks have no chainlock signatures. See DIP-0029.
+        let signatures_available = !quorum_sig_lookup.iter().any(Option::is_none);
+        let signatures_required = diff_end_height >= network.v20_activation_height();
+
+        if signatures_required && !signatures_available {
             return Err(SmlError::IncompleteSignatureSet);
         }
 
@@ -105,37 +112,40 @@ impl MasternodeList {
                     let entry_hash = new_quorum.calculate_entry_hash();
                     let verifying_chain_lock_signature =
                         if new_quorum.llmq_type.is_rotating_quorum_type() {
-                            if rotating_sig.is_none() {
-                                if let Some(sig) = quorum_sig_lookup[idx] {
-                                    rotating_sig = Some(*sig)
-                                } else {
-                                    return Err(SmlError::IncompleteSignatureSet);
-                                }
+                            if rotating_sig.is_none()
+                                && let Some(sig) = quorum_sig_lookup.get(idx).copied().flatten()
+                            {
+                                rotating_sig = Some(*sig);
                             }
-                            if let Some(previous_chain_lock_sigs) = previous_chain_lock_sigs {
-                                if let Some(sig) = quorum_sig_lookup[idx] {
-                                    Some(VerifyingChainLockSignaturesType::Rotating([
-                                        previous_chain_lock_sigs[0],
-                                        previous_chain_lock_sigs[1],
-                                        previous_chain_lock_sigs[2],
-                                        *sig,
-                                    ]))
+                            if signatures_available {
+                                if let Some(previous_chain_lock_sigs) = previous_chain_lock_sigs {
+                                    quorum_sig_lookup.get(idx).copied().flatten().map(|sig| {
+                                        VerifyingChainLockSignaturesType::Rotating([
+                                            previous_chain_lock_sigs[0],
+                                            previous_chain_lock_sigs[1],
+                                            previous_chain_lock_sigs[2],
+                                            *sig,
+                                        ])
+                                    })
                                 } else {
-                                    return Err(SmlError::IncompleteSignatureSet);
+                                    None
                                 }
                             } else {
                                 None
                             }
                         } else {
-                            quorum_sig_lookup[idx]
+                            quorum_sig_lookup
+                                .get(idx)
+                                .copied()
+                                .flatten()
                                 .copied()
                                 .map(VerifyingChainLockSignaturesType::NonRotating)
                         };
                     QualifiedQuorumEntry {
                         quorum_entry: new_quorum,
                         verified: LLMQEntryVerificationStatus::Skipped(
                             LLMQEntryVerificationSkipStatus::NotMarkedForVerification,
-                        ), // Default to unverified
+                        ),
                         commitment_hash,
                         entry_hash,
                         verifying_chain_lock_signature,
@@ -155,3 +165,80 @@ impl MasternodeList {
         Ok((builder.build(), rotating_sig))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::consensus::deserialize;
+    use crate::network::constants::NetworkExt;
+    use crate::sml::masternode_list::from_diff::TryFromWithBlockHashLookup;
+
+    #[test]
+    fn apply_diff_post_v20_requires_chainlock_signatures() {
+        // Create base list from first diff
+        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(
+            base_diff,
+            |_| Some(2_227_096),
+            Network::Dash,
+        )
+        .expect("expected to create base list");
+
+        // Load second diff and clear signatures
+        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.quorums_chainlock_signatures.clear();
+
+        // Height 2241332 is post-V20 on mainnet (1,987,776)
+        let post_v20_height = 2_241_332;
+        assert!(post_v20_height >= Network::Dash.v20_activation_height());
+
+        let result = base_list.apply_diff(diff, post_v20_height, None, Network::Dash);
+
+        assert!(
+            matches!(result, Err(SmlError::IncompleteSignatureSet)),
+            "Post-V20 apply_diff should require chainlock signatures"
+        );
+    }
+
+    #[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::Dash,
+        )
+        .expect("expected to create base list");
+
+        // Load second diff and clear signatures
+        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");
+
+        // Fix base_block_hash to match our base list
+        diff.base_block_hash = base_list.block_hash;
+        diff.quorums_chainlock_signatures.clear();
+
+        // Use a pre-V20 height on mainnet
+        let pre_v20_height = 1_900_000u32;
+        assert!(pre_v20_height < Network::Dash.v20_activation_height());
+
+        let result = base_list.apply_diff(diff, pre_v20_height, None, Network::Dash);
+
+        assert!(
+            result.is_ok(),
+            "Pre-V20 apply_diff should allow missing chainlock signatures: {:?}",
+            result.err()
+        );
+    }
+}

dash/src/sml/masternode_list/from_diff.rs

@@ -107,8 +107,11 @@ impl TryFromWithBlockHashLookup<MnListDiff> for MasternodeList {
             }
         }
 
-        // Verify all slots have been filled
-        if quorum_sig_lookup.iter().any(Option::is_none) {
+        // quorumsCLSigs only exists after V20 activation (protocol 70230).
+        // Pre-V20 blocks have no chainlock signatures. See DIP-0029.
+        if known_height >= network.v20_activation_height()
+            && quorum_sig_lookup.iter().any(Option::is_none)
+        {
             return Err(SmlError::IncompleteSignatureSet);
         }
 
@@ -127,7 +130,10 @@ impl TryFromWithBlockHashLookup<MnListDiff> for MasternodeList {
                         ),
                         commitment_hash,
                         entry_hash,
-                        verifying_chain_lock_signature: quorum_sig_lookup[idx]
+                        verifying_chain_lock_signature: quorum_sig_lookup
+                            .get(idx)
+                            .copied()
+                            .flatten()
                             .copied()
                             .map(VerifyingChainLockSignaturesType::NonRotating),
                     }
@@ -147,3 +153,63 @@ impl TryFromWithBlockHashLookup<MnListDiff> for MasternodeList {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::consensus::deserialize;
+    use crate::network::constants::NetworkExt;
+
+    #[test]
+    fn post_v20_requires_chainlock_signatures() {
+        let mn_list_diff_bytes: &[u8] =
+            include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin");
+        let mut diff: MnListDiff =
+            deserialize(mn_list_diff_bytes).expect("expected to deserialize");
+
+        // Clear signatures to simulate missing data
+        diff.quorums_chainlock_signatures.clear();
+
+        // Height 2227096 is post-V20 on mainnet (1,987,776)
+        let post_v20_height = 2_227_096;
+        assert!(post_v20_height >= Network::Dash.v20_activation_height());
+
+        let result = MasternodeList::try_from_with_block_hash_lookup(
+            diff,
+            |_| Some(post_v20_height),
+            Network::Dash,
+        );
+
+        assert!(
+            matches!(result, Err(SmlError::IncompleteSignatureSet)),
+            "Post-V20 blocks should require chainlock signatures"
+        );
+    }
+
+    #[test]
+    fn pre_v20_allows_missing_chainlock_signatures() {
+        let mn_list_diff_bytes: &[u8] =
+            include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin");
+        let mut diff: MnListDiff =
+            deserialize(mn_list_diff_bytes).expect("expected to deserialize");
+
+        // Clear signatures to simulate pre-V20 data
+        diff.quorums_chainlock_signatures.clear();
+
+        // Use a pre-V20 height on mainnet (V20 at 1,987,776)
+        let pre_v20_height = 1_900_000;
+        assert!(pre_v20_height < Network::Dash.v20_activation_height());
+
+        let result = MasternodeList::try_from_with_block_hash_lookup(
+            diff,
+            |_| Some(pre_v20_height),
+            Network::Dash,
+        );
+
+        assert!(
+            result.is_ok(),
+            "Pre-V20 blocks should allow missing chainlock signatures: {:?}",
+            result.err()
+        );
+    }
+}

dash/src/sml/masternode_list_engine/mod.rs

@@ -909,6 +909,7 @@ impl MasternodeListEngine {
                 masternode_list_diff.clone(),
                 diff_end_height,
                 previous_chain_lock_sigs,
+                self.network,
             )?;
             if verify_quorums {
                 // We should go through all quorums of the masternode list to update those that were not yet verified
@@ -978,6 +979,7 @@ impl MasternodeListEngine {
                 masternode_list_diff.clone(),
                 diff_end_height,
                 None,
+                self.network,
             )?;
             if verify_quorums {
                 return Err(SmlError::FeatureNotTurnedOn(
@@ -1436,4 +1438,55 @@ mod tests {
                 .expect("expected to validated quorums");
         }
     }
+
+    #[test]
+    fn feed_qr_info_rejects_post_v20_with_missing_chainlock_signatures() {
+        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");
+        let mut masternode_list_engine =
+            MasternodeListEngine::initialize_with_diff_to_height(diff, 2227096, Network::Dash)
+                .expect("expected to start engine");
+
+        let block_container_bytes: &[u8] =
+            include_bytes!("../../../tests/data/test_DML_diffs/block_container_2240504.dat");
+        let block_container: MasternodeListEngineBlockContainer =
+            bincode::decode_from_slice(block_container_bytes, bincode::config::standard())
+                .expect("expected to decode")
+                .0;
+        let mn_list_diffs_bytes: &[u8] =
+            include_bytes!("../../../tests/data/test_DML_diffs/mnlistdiffs_2240504.dat");
+        let mn_list_diffs: BTreeMap<(CoreBlockHeight, CoreBlockHeight), MnListDiff> =
+            bincode::decode_from_slice(mn_list_diffs_bytes, bincode::config::standard())
+                .expect("expected to decode")
+                .0;
+        let qr_info_bytes: &[u8] =
+            include_bytes!("../../../tests/data/test_DML_diffs/qrinfo_2240504.dat");
+        let mut qr_info: QRInfo =
+            bincode::decode_from_slice(qr_info_bytes, bincode::config::standard())
+                .expect("expected to decode")
+                .0;
+
+        masternode_list_engine.block_container = block_container;
+
+        for ((_start_height, height), diff) in mn_list_diffs.into_iter() {
+            masternode_list_engine
+                .apply_diff(diff, Some(height), false, None)
+                .expect("expected to apply diff");
+        }
+
+        // Clear chainlock signatures to simulate missing data for post-V20 block
+        qr_info.mn_list_diff_at_h_minus_2c.quorums_chainlock_signatures.clear();
+
+        // feed_qr_info should fail for post-V20 blocks with missing signatures
+        let result = masternode_list_engine
+            .feed_qr_info::<fn(&BlockHash) -> Result<u32, ClientDataRetrievalError>>(
+                qr_info, false, false, None,
+            );
+
+        assert!(
+            result.is_err(),
+            "Post-V20 feed_qr_info should reject missing chainlock signatures"
+        );
+    }
 }