fix(drive-abci): make zero quorum windows and active-signer counts unrepresentable

packages/rs-drive-abci/src/execution/platform_events/core_chain_lock/verify_chain_lock_locally/v0/mod.rs

@@ -66,11 +66,14 @@ where
 
         let window_width = quorum_config.window;
 
-        // The last block in the window where the quorums would be the same
-        let last_block_in_window = platform_state.last_committed_core_height()
-            - platform_state.last_committed_core_height() % window_width
-            + window_width
-            - 1;
+        // The last block in the window where the quorums would be the same. The window is a
+        // `NonZeroU32`, so the remainder cannot divide by zero, and a window ending past the
+        // largest core height wraps instead of overflowing where overflow is checked. For every
+        // window this equals the wrapping `u32` arithmetic of release builds, which the protocol
+        // versions selecting this method validate their chains with.
+        let last_block_in_window = (platform_state.last_committed_core_height()
+            - platform_state.last_committed_core_height() % window_width)
+            .wrapping_add(window_width.get() - 1);
 
         let verification_height = chain_lock_height.saturating_sub(SIGN_OFFSET);
 
@@ -235,12 +238,21 @@ where
 
 #[cfg(test)]
 mod tests {
+    use crate::config::ChainLockConfig;
     use crate::execution::platform_events::core_chain_lock::verify_chain_lock_locally::v0::CHAIN_LOCK_REQUEST_ID_PREFIX;
-    use crate::test::helpers::setup::TestPlatformBuilder;
-    use dpp::bls_signatures::{Bls12381G2Impl, Pairing};
+    use crate::platform_types::platform_state::{PlatformState, PlatformStateV0Methods};
+    use crate::platform_types::signature_verification_quorum_set::{
+        SignatureVerificationQuorumSet, SignatureVerificationQuorumSetV0Methods, VerificationQuorum,
+    };
+    use crate::rpc::core::MockCoreRPCLike;
+    use crate::test::helpers::setup::{TempPlatform, TestPlatformBuilder};
+    use dpp::block::block_info::BlockInfo;
+    use dpp::bls_signatures::{Bls12381G2Impl, Pairing, SecretKey, SignatureSchemes};
     use dpp::dashcore::hashes::{sha256d, Hash, HashEngine};
-    use dpp::dashcore::{BlockHash, ChainLock, QuorumSigningRequestId};
+    use dpp::dashcore::{BlockHash, ChainLock, QuorumHash, QuorumSigningRequestId};
+    use dpp::dashcore_rpc::json::QuorumType;
     use dpp::version::PlatformVersion;
+    use std::num::{NonZeroU16, NonZeroU32};
 
     #[test]
     fn verify_request_id() {
@@ -371,4 +383,127 @@ mod tests {
             msg
         );
     }
+
+    /// The secret key of the only chain lock quorum held by `state_with_one_chain_lock_quorum`
+    fn quorum_secret_key() -> SecretKey<Bls12381G2Impl> {
+        let mut key_bytes = [0u8; 32];
+        key_bytes[0] = 1;
+        key_bytes[31] = 1;
+        SecretKey::<Bls12381G2Impl>::from_be_bytes(&key_bytes).expect("valid secret key")
+    }
+
+    const QUORUM_HASH: [u8; 32] = [1u8; 32];
+
+    /// `platform`'s state with last committed core height `core_height`, holding one llmq_test
+    /// chain lock quorum with a 24 block DKG interval
+    fn state_with_one_chain_lock_quorum(
+        platform: &TempPlatform<MockCoreRPCLike>,
+        core_height: u32,
+    ) -> PlatformState {
+        let mut state = platform.platform.state.load().as_ref().clone();
+        state.set_last_committed_block_info(None);
+        state.set_genesis_block_info(Some(BlockInfo {
+            core_height,
+            ..Default::default()
+        }));
+
+        let mut quorum_set = SignatureVerificationQuorumSet::V0(
+            ChainLockConfig {
+                quorum_type: QuorumType::LlmqTest,
+                quorum_size: 3,
+                quorum_window: NonZeroU32::new(24).unwrap(),
+                quorum_active_signers: NonZeroU16::new(2).unwrap(),
+                quorum_rotation: false,
+            }
+            .into(),
+        );
+        quorum_set.set_current_quorums(
+            [(
+                QuorumHash::from_byte_array(QUORUM_HASH),
+                VerificationQuorum {
+                    index: None,
+                    public_key: quorum_secret_key().public_key(),
+                },
+            )]
+            .into_iter()
+            .collect(),
+        );
+        state.set_chain_lock_validating_quorums(quorum_set);
+        state
+    }
+
+    /// A llmq_test chain lock at `height` signed by the quorum of
+    /// `state_with_one_chain_lock_quorum`
+    fn sign_chain_lock(height: u32) -> ChainLock {
+        let block_hash = BlockHash::from_byte_array([7u8; 32]);
+
+        let mut engine = QuorumSigningRequestId::engine();
+        engine.input(&[CHAIN_LOCK_REQUEST_ID_PREFIX.len() as u8]);
+        engine.input(CHAIN_LOCK_REQUEST_ID_PREFIX.as_bytes());
+        engine.input(height.to_le_bytes().as_slice());
+        let request_id = QuorumSigningRequestId::from_engine(engine);
+
+        let mut reversed_quorum_hash = QUORUM_HASH;
+        reversed_quorum_hash.reverse();
+
+        let mut engine = sha256d::Hash::engine();
+        engine.input(&[QuorumType::LlmqTest as u8]);
+        engine.input(reversed_quorum_hash.as_slice());
+        engine.input(request_id.as_byte_array());
+        engine.input(block_hash.as_byte_array());
+        let message_digest = sha256d::Hash::from_engine(engine);
+
+        let signature = quorum_secret_key()
+            .sign(SignatureSchemes::Basic, message_digest.as_byte_array())
+            .expect("signs");
+
+        ChainLock {
+            block_height: height,
+            block_hash,
+            signature: signature.as_raw_value().to_compressed().into(),
+        }
+    }
+
+    fn chain_lock_test_platform() -> TempPlatform<MockCoreRPCLike> {
+        TestPlatformBuilder::new()
+            .with_latest_protocol_version()
+            .build_with_mock_rpc()
+            .set_initial_state_structure()
+    }
+
+    #[test]
+    fn should_leave_chain_lock_to_core_without_overflowing_when_quorum_window_ends_past_largest_core_height(
+    ) {
+        let platform = chain_lock_test_platform();
+        let platform_version = PlatformVersion::latest();
+
+        // The quorum verifies its chain locks locally while its window ends below u32::MAX
+        let state = state_with_one_chain_lock_quorum(&platform, 1106);
+        assert_eq!(
+            platform
+                .platform
+                .verify_chain_lock_locally_v0(0, &state, &sign_chain_lock(1110), platform_version)
+                .expect("verification runs"),
+            Some(true)
+        );
+
+        // The 24 block window holding core height u32::MAX - 5 would end past u32::MAX. Release
+        // builds have always wrapped its last block to 7, below any verification height near
+        // the core height, so the chain lock is left to Core; that must stay so, and must not
+        // overflow in builds that check arithmetic.
+        let core_height = u32::MAX - 5;
+        let state = state_with_one_chain_lock_quorum(&platform, core_height);
+        assert_eq!(
+            platform
+                .platform
+                .verify_chain_lock_locally_v0(
+                    0,
+                    &state,
+                    &sign_chain_lock(core_height + 3),
+                    platform_version
+                )
+                .expect("verification runs"),
+            None
+        );
+    }
 }

packages/rs-drive-abci/src/execution/storage/fetch_platform_state/mod.rs

@@ -34,3 +34,146 @@ impl<C> Platform<C> {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::error::Error;
+    use crate::platform_types::platform::Platform;
+    use crate::platform_types::platform_state::platform_state_for_saving::PlatformStateForSaving;
+    use crate::platform_types::platform_state::PlatformStateV0Methods;
+    use crate::platform_types::signature_verification_quorum_set::SignatureVerificationQuorumSetV0Methods;
+    use crate::rpc::core::MockCoreRPCLike;
+    use crate::test::fixture::platform_state::{
+        PLATFORM_STATE_V3_TESTNET, PLATFORM_STATE_V8_DEVNET,
+    };
+    use crate::test::helpers::setup::TestPlatformBuilder;
+    use bincode::config::{BigEndian, Configuration, NoLimit, Varint};
+    use dpp::dashcore_rpc::json::QuorumType;
+    use dpp::version::PlatformVersion;
+    use dpp::ProtocolError;
+    use std::ops::Deref;
+
+    /// The bincode configuration platform states are saved with
+    const PLATFORM_BINCODE_CONFIG: Configuration<BigEndian, Varint, NoLimit> =
+        bincode::config::standard()
+            .with_big_endian()
+            .with_no_limit();
+
+    /// A saved quorum config: its type, active signers, rotation and window
+    type SavedQuorumConfig = (QuorumType, u16, bool, u32);
+
+    /// A testnet state in the first saved format and a devnet state in the second, each with
+    /// the chain lock quorum config it was saved with
+    fn fixtures() -> [(&'static str, SavedQuorumConfig); 2] {
+        [
+            (
+                PLATFORM_STATE_V3_TESTNET.deref(),
+                (QuorumType::Llmq50_60, 24, false, 24),
+            ),
+            (
+                PLATFORM_STATE_V8_DEVNET.deref(),
+                (QuorumType::LlmqDevnet, 4, false, 24),
+            ),
+        ]
+    }
+
+    /// The bytes of `fixture` with its chain lock quorum config, saved as `saved`, holding
+    /// `active_signers` and `window`, encoded the way a `u16` and a `u32` were
+    fn with_chain_lock_quorum_config(
+        fixture: &str,
+        saved: SavedQuorumConfig,
+        active_signers: u16,
+        window: u32,
+    ) -> Vec<u8> {
+        let encode = |config: SavedQuorumConfig| {
+            bincode::encode_to_vec(config, PLATFORM_BINCODE_CONFIG).unwrap()
+        };
+        let saved_bytes = encode(saved);
+        let mut bytes = hex::decode(fixture).unwrap();
+        let positions: Vec<usize> = bytes
+            .windows(saved_bytes.len())
+            .enumerate()
+            .filter_map(|(position, window)| (window == saved_bytes).then_some(position))
+            .collect();
+        assert_eq!(
+            positions.len(),
+            1,
+            "the chain lock quorum config is saved once"
+        );
+        let (quorum_type, _, rotation, _) = saved;
+        bytes.splice(
+            positions[0]..positions[0] + saved_bytes.len(),
+            encode((quorum_type, active_signers, rotation, window)),
+        );
+        bytes
+    }
+
+    #[test]
+    fn should_save_historical_platform_states_with_unchanged_bytes() {
+        for (fixture, _) in fixtures() {
+            let bytes = hex::decode(fixture).unwrap();
+            let (saved, read): (PlatformStateForSaving, _) =
+                bincode::decode_from_slice(&bytes, PLATFORM_BINCODE_CONFIG).unwrap();
+            assert_eq!(read, bytes.len());
+            assert_eq!(
+                bincode::encode_to_vec(&saved, PLATFORM_BINCODE_CONFIG).unwrap(),
+                bytes
+            );
+        }
+    }
+
+    #[test]
+    fn should_refuse_to_restore_saved_state_holding_zero_chain_lock_active_signers_or_window() {
+        // A restarted node reads its chain lock quorum config from its saved state rather than from
+        // its config. Choosing a rotated quorum takes the logarithm of the active signers and chain
+        // lock verification divides by the window, so a saved 0 in either must stop the node from
+        // loading that state instead of reaching them.
+        let platform = TestPlatformBuilder::new().build_with_mock_rpc();
+        let restore = |bytes: &[u8], platform_version: &PlatformVersion| {
+            let transaction = platform.drive.grove.start_transaction();
+            platform
+                .drive
+                .store_platform_state_bytes(bytes, Some(&transaction), platform_version)
+                .unwrap();
+            Platform::<MockCoreRPCLike>::fetch_platform_state(
+                &platform.drive,
+                Some(&transaction),
+                platform_version,
+            )
+        };
+
+        // Protocol version 13 reads the saved state with fetch_platform_state v0, 14 with v1
+        for platform_version in [PlatformVersion::get(13).unwrap(), PlatformVersion::latest()] {
+            for (fixture, saved) in fixtures() {
+                let state = restore(
+                    &with_chain_lock_quorum_config(fixture, saved, 1, 1),
+                    platform_version,
+                )
+                .unwrap()
+                .unwrap();
+                let restored = state.chain_lock_validating_quorums().config();
+                assert_eq!(
+                    (restored.active_signers.get(), restored.window.get()),
+                    (1, 1)
+                );
+
+                for (active_signers, window) in [(0, saved.3), (saved.1, 0)] {
+                    let result = restore(
+                        &with_chain_lock_quorum_config(fixture, saved, active_signers, window),
+                        platform_version,
+                    );
+                    assert!(
+                        matches!(
+                            &result,
+                            Err(Error::Protocol(ProtocolError::PlatformDeserializationError(message)))
+                                if message.contains("NonZeroTypeIsZero")
+                        ),
+                        "protocol version {}, {:?}, active signers {active_signers}, window {window}: got {result:?}",
+                        platform_version.protocol_version,
+                        saved.0
+                    );
+                }
+            }
+        }
+    }
+}