Remove quickcheck in favour of proptest (#8471)

Consolidate our property-testing around `proptest`. This PR was written with Copilot and manually tweaked.


Co-Authored-By: Michael Sproul <michael@sproul.xyz>

Co-Authored-By: Michael Sproul <michael@sigmaprime.io>

Cargo.lock

@@ -3084,16 +3084,6 @@ dependencies = [
  "syn 2.0.110",
 ]
 
-[[package]]
-name = "env_logger"
-version = "0.8.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a19187fea3ac7e84da7dacf48de0c45d63c6a76f9490dae389aead16c243fce3"
-dependencies = [
- "log",
- "regex",
-]
-
 [[package]]
 name = "environment"
 version = "0.1.2"
@@ -5525,8 +5515,7 @@ dependencies = [
  "network_utils",
  "parking_lot",
  "prometheus-client",
- "quickcheck",
+ "proptest",
- "quickcheck_macros",
  "rand 0.9.2",
  "regex",
  "serde",
@@ -5833,8 +5822,7 @@ dependencies = [
  "alloy-primitives",
  "ethereum_hashing",
  "fixed_bytes",
- "quickcheck",
+ "proptest",
- "quickcheck_macros",
  "safe_arith",
 ]
 
@@ -7222,28 +7210,6 @@ dependencies = [
  "unsigned-varint 0.8.0",
 ]
 
-[[package]]
-name = "quickcheck"
-version = "1.0.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "588f6378e4dd99458b60ec275b4477add41ce4fa9f64dcba6f15adccb19b50d6"
-dependencies = [
- "env_logger",
- "log",
- "rand 0.8.5",
-]
-
-[[package]]
-name = "quickcheck_macros"
-version = "1.1.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f71ee38b42f8459a88d3362be6f9b841ad2d5421844f61eb1c59c11bff3ac14a"
-dependencies = [
- "proc-macro2",
- "quote",
- "syn 2.0.110",
-]
-
 [[package]]
 name = "quinn"
 version = "0.11.9"

Cargo.toml

@@ -201,9 +201,8 @@ parking_lot = "0.12"
 paste = "1"
 pretty_reqwest_error = { path = "common/pretty_reqwest_error" }
 prometheus = { version = "0.13", default-features = false }
+proptest = "1"
 proto_array = { path = "consensus/proto_array" }
-quickcheck = "1"
-quickcheck_macros = "1"
 quote = "1"
 r2d2 = "0.8"
 rand = "0.9.0"

beacon_node/lighthouse_network/Cargo.toml

@@ -72,6 +72,5 @@ features = [
 [dev-dependencies]
 async-channel = { workspace = true }
 logging = { workspace = true }
-quickcheck = { workspace = true }
+proptest = { workspace = true }
-quickcheck_macros = { workspace = true }
 tempfile = { workspace = true }

beacon_node/lighthouse_network/src/peer_manager/mod.rs

@@ -2975,8 +2975,7 @@ mod tests {
         use crate::peer_manager::tests::build_peer_manager_with_trusted_peers;
         use crate::rpc::{MetaData, MetaDataV3};
         use libp2p::PeerId;
-        use quickcheck::{Arbitrary, Gen, TestResult};
+        use proptest::prelude::*;
-        use quickcheck_macros::quickcheck;
         use std::collections::HashSet;
         use tokio::runtime::Runtime;
         use types::{DataColumnSubnetId, Unsigned};
@@ -2994,159 +2993,202 @@ mod tests {
             custody_subnets: HashSet<DataColumnSubnetId>,
         }
 
-        impl Arbitrary for PeerCondition {
+        fn peer_condition_strategy() -> impl Strategy<Value = PeerCondition> {
-            fn arbitrary(g: &mut Gen) -> Self {
+            let attestation_len = <E as EthSpec>::SubnetBitfieldLength::to_usize();
-                let attestation_net_bitfield = {
+            let sync_committee_len = <E as EthSpec>::SyncCommitteeSubnetCount::to_usize();
-                    let len = <E as EthSpec>::SubnetBitfieldLength::to_usize();
+            let spec = E::default_spec();
-                    let mut bitfield = Vec::with_capacity(len);
+            let total_subnet_count = spec.data_column_sidecar_subnet_count;
-                    for _ in 0..len {
+            let custody_requirement = spec.custody_requirement;
-                        bitfield.push(bool::arbitrary(g));
-                    }
-                    bitfield
-                };
 
-                let sync_committee_net_bitfield = {
+            // Create the pool of available subnet IDs
-                    let len = <E as EthSpec>::SyncCommitteeSubnetCount::to_usize();
+            let available_subnets: Vec<u64> = (custody_requirement..total_subnet_count).collect();
-                    let mut bitfield = Vec::with_capacity(len);
+            let max_custody_subnets = available_subnets.len();
-                    for _ in 0..len {
-                        bitfield.push(bool::arbitrary(g));
-                    }
-                    bitfield
-                };
 
-                let spec = E::default_spec();
+            // Trusted peer probability constants - 1 in 5 peers should be trusted (20%)
-                let custody_subnets = {
+            const TRUSTED_PEER_WEIGHT_FALSE: u32 = 4;
-                    let total_subnet_count = spec.data_column_sidecar_subnet_count;
+            const TRUSTED_PEER_WEIGHT_TRUE: u32 = 1;
-                    let custody_subnet_count = u64::arbitrary(g) % (total_subnet_count + 1); // 0 to 128
-                    (spec.custody_requirement..total_subnet_count)
-                        .filter(|_| bool::arbitrary(g))
-                        .map(DataColumnSubnetId::new)
-                        .take(custody_subnet_count as usize)
-                        .collect()
-                };
 
-                PeerCondition {
+            (
-                    peer_id: PeerId::random(),
+                proptest::collection::vec(any::<bool>(), attestation_len),
-                    outgoing: bool::arbitrary(g),
+                proptest::collection::vec(any::<bool>(), sync_committee_len),
-                    attestation_net_bitfield,
+                any::<f64>(),
-                    sync_committee_net_bitfield,
+                any::<bool>(),
-                    score: f64::arbitrary(g),
+                any::<f64>(),
-                    trusted: bool::arbitrary(g),
+                // Weight trusted peers to avoid test rejection due to too many trusted peers
-                    gossipsub_score: f64::arbitrary(g),
+                prop_oneof![
-                    custody_subnets,
+                    TRUSTED_PEER_WEIGHT_FALSE => Just(false),
-                }
+                    TRUSTED_PEER_WEIGHT_TRUE => Just(true),
-            }
+                ],
+                0..=max_custody_subnets,
+            )
+                .prop_flat_map(
+                    move |(
+                        attestation_net_bitfield,
+                        sync_committee_net_bitfield,
+                        score,
+                        outgoing,
+                        gossipsub_score,
+                        trusted,
+                        custody_subnet_count,
+                    )| {
+                        // Use proptest's subsequence to select a random subset of subnets
+                        let custody_subnets_strategy = proptest::sample::subsequence(
+                            available_subnets.clone(),
+                            custody_subnet_count,
+                        );
+
+                        (
+                            Just(attestation_net_bitfield),
+                            Just(sync_committee_net_bitfield),
+                            Just(score),
+                            Just(outgoing),
+                            Just(gossipsub_score),
+                            Just(trusted),
+                            custody_subnets_strategy,
+                        )
+                    },
+                )
+                .prop_map(
+                    |(
+                        attestation_net_bitfield,
+                        sync_committee_net_bitfield,
+                        score,
+                        outgoing,
+                        gossipsub_score,
+                        trusted,
+                        custody_subnets_vec,
+                    )| {
+                        let custody_subnets: HashSet<DataColumnSubnetId> = custody_subnets_vec
+                            .into_iter()
+                            .map(DataColumnSubnetId::new)
+                            .collect();
+
+                        PeerCondition {
+                            peer_id: PeerId::random(),
+                            outgoing,
+                            attestation_net_bitfield,
+                            sync_committee_net_bitfield,
+                            score,
+                            trusted,
+                            gossipsub_score,
+                            custody_subnets,
+                        }
+                    },
+                )
         }
 
-        #[quickcheck]
+        // Upper bound for testing peer pruning - we test with at least the target number
-        fn prune_excess_peers(peer_conditions: Vec<PeerCondition>) -> TestResult {
+        // and up to 50% more than the target to verify pruning behavior.
-            let target_peer_count = DEFAULT_TARGET_PEERS;
+        const MAX_TEST_PEERS: usize = 300;
-            let spec = E::default_spec();
-            if peer_conditions.len() < target_peer_count {
-                return TestResult::discard();
-            }
-            let trusted_peers: Vec<_> = peer_conditions
-                .iter()
-                .filter_map(|p| if p.trusted { Some(p.peer_id) } else { None })
-                .collect();
-            // If we have a high percentage of trusted peers, it is very difficult to reason about
-            // the expected results of the pruning.
-            if trusted_peers.len() > peer_conditions.len() / 3_usize {
-                return TestResult::discard();
-            }
-            let rt = Runtime::new().unwrap();
 
-            rt.block_on(async move {
+        proptest! {
-                // Collect all the trusted peers
+            #[test]
-                let mut peer_manager =
+            fn prune_excess_peers(peer_conditions in proptest::collection::vec(peer_condition_strategy(), DEFAULT_TARGET_PEERS..=MAX_TEST_PEERS)) {
-                    build_peer_manager_with_trusted_peers(trusted_peers, target_peer_count).await;
+                let target_peer_count = DEFAULT_TARGET_PEERS;
+                let spec = E::default_spec();
 
-                // Create peers based on the randomly generated conditions.
+                let trusted_peers: Vec<_> = peer_conditions
-                for condition in &peer_conditions {
-                    let mut attnets = crate::types::EnrAttestationBitfield::<E>::new();
-                    let mut syncnets = crate::types::EnrSyncCommitteeBitfield::<E>::new();
-
-                    if condition.outgoing {
-                        peer_manager.inject_connect_outgoing(
-                            &condition.peer_id,
-                            "/ip4/0.0.0.0".parse().unwrap(),
-                            None,
-                        );
-                    } else {
-                        peer_manager.inject_connect_ingoing(
-                            &condition.peer_id,
-                            "/ip4/0.0.0.0".parse().unwrap(),
-                            None,
-                        );
-                    }
-
-                    for (i, value) in condition.attestation_net_bitfield.iter().enumerate() {
-                        attnets.set(i, *value).unwrap();
-                    }
-
-                    for (i, value) in condition.sync_committee_net_bitfield.iter().enumerate() {
-                        syncnets.set(i, *value).unwrap();
-                    }
-
-                    let subnets_per_custody_group =
-                        spec.data_column_sidecar_subnet_count / spec.number_of_custody_groups;
-                    let metadata = MetaDataV3 {
-                        seq_number: 0,
-                        attnets,
-                        syncnets,
-                        custody_group_count: condition.custody_subnets.len() as u64
-                            / subnets_per_custody_group,
-                    };
-
-                    let mut peer_db = peer_manager.network_globals.peers.write();
-                    let peer_info = peer_db.peer_info_mut(&condition.peer_id).unwrap();
-                    peer_info.set_meta_data(MetaData::V3(metadata));
-                    peer_info.set_gossipsub_score(condition.gossipsub_score);
-                    peer_info.add_to_score(condition.score);
-                    peer_info.set_custody_subnets(condition.custody_subnets.clone());
-
-                    for subnet in peer_info.long_lived_subnets() {
-                        peer_db.add_subscription(&condition.peer_id, subnet);
-                    }
-                }
-
-                // Perform the heartbeat.
-                peer_manager.heartbeat();
-
-                // The minimum number of connected peers cannot be less than the target peer count
-                // or submitted peers.
-
-                let expected_peer_count = target_peer_count.min(peer_conditions.len());
-                // Trusted peers could make this larger however.
-                let no_of_trusted_peers = peer_conditions
                     .iter()
-                    .filter(|condition| condition.trusted)
+                    .filter_map(|p| if p.trusted { Some(p.peer_id) } else { None })
-                    .count();
+                    .collect();
-                let expected_peer_count = expected_peer_count.max(no_of_trusted_peers);
+                // If we have a high percentage of trusted peers, it is very difficult to reason about
+                // the expected results of the pruning.
+                prop_assume!(trusted_peers.len() <= peer_conditions.len() / 3_usize);
 
-                let target_peer_condition =
+                let rt = Runtime::new().unwrap();
-                    peer_manager.network_globals.connected_or_dialing_peers()
-                        == expected_peer_count;
 
-                // It could be that we reach our target outbound limit and are unable to prune any
+                let result = rt.block_on(async move {
-                // extra, which violates the target_peer_condition.
+                    // Collect all the trusted peers
-                let outbound_peers = peer_manager.network_globals.connected_outbound_only_peers();
+                    let mut peer_manager =
-                let hit_outbound_limit = outbound_peers == peer_manager.target_outbound_peers();
+                        build_peer_manager_with_trusted_peers(trusted_peers, target_peer_count).await;
 
-                // No trusted peers should be disconnected
+                    // Create peers based on the randomly generated conditions.
-                let trusted_peer_disconnected = peer_conditions.iter().any(|condition| {
+                    for condition in &peer_conditions {
-                    condition.trusted
+                        let mut attnets = crate::types::EnrAttestationBitfield::<E>::new();
-                        && !peer_manager
+                        let mut syncnets = crate::types::EnrSyncCommitteeBitfield::<E>::new();
-                            .network_globals
+
-                            .peers
+                        if condition.outgoing {
-                            .read()
+                            peer_manager.inject_connect_outgoing(
-                            .is_connected(&condition.peer_id)
+                                &condition.peer_id,
+                                "/ip4/0.0.0.0".parse().unwrap(),
+                                None,
+                            );
+                        } else {
+                            peer_manager.inject_connect_ingoing(
+                                &condition.peer_id,
+                                "/ip4/0.0.0.0".parse().unwrap(),
+                                None,
+                            );
+                        }
+
+                        for (i, value) in condition.attestation_net_bitfield.iter().enumerate() {
+                            attnets.set(i, *value).unwrap();
+                        }
+
+                        for (i, value) in condition.sync_committee_net_bitfield.iter().enumerate() {
+                            syncnets.set(i, *value).unwrap();
+                        }
+
+                        let subnets_per_custody_group =
+                            spec.data_column_sidecar_subnet_count / spec.number_of_custody_groups;
+                        let metadata = MetaDataV3 {
+                            seq_number: 0,
+                            attnets,
+                            syncnets,
+                            custody_group_count: condition.custody_subnets.len() as u64
+                                / subnets_per_custody_group,
+                        };
+
+                        let mut peer_db = peer_manager.network_globals.peers.write();
+                        let peer_info = peer_db.peer_info_mut(&condition.peer_id).unwrap();
+                        peer_info.set_meta_data(MetaData::V3(metadata));
+                        peer_info.set_gossipsub_score(condition.gossipsub_score);
+                        peer_info.add_to_score(condition.score);
+                        peer_info.set_custody_subnets(condition.custody_subnets.clone());
+
+                        for subnet in peer_info.long_lived_subnets() {
+                            peer_db.add_subscription(&condition.peer_id, subnet);
+                        }
+                    }
+
+                    // Perform the heartbeat.
+                    peer_manager.heartbeat();
+
+                    // The minimum number of connected peers cannot be less than the target peer count
+                    // or submitted peers.
+
+                    let expected_peer_count = target_peer_count.min(peer_conditions.len());
+                    // Trusted peers could make this larger however.
+                    let no_of_trusted_peers = peer_conditions
+                        .iter()
+                        .filter(|condition| condition.trusted)
+                        .count();
+                    let expected_peer_count = expected_peer_count.max(no_of_trusted_peers);
+
+                    let target_peer_condition =
+                        peer_manager.network_globals.connected_or_dialing_peers()
+                            == expected_peer_count;
+
+                    // It could be that we reach our target outbound limit and are unable to prune any
+                    // extra, which violates the target_peer_condition.
+                    let outbound_peers = peer_manager.network_globals.connected_outbound_only_peers();
+                    let hit_outbound_limit = outbound_peers == peer_manager.target_outbound_peers();
+
+                    // No trusted peers should be disconnected
+                    let trusted_peer_disconnected = peer_conditions.iter().any(|condition| {
+                        condition.trusted
+                            && !peer_manager
+                                .network_globals
+                                .peers
+                                .read()
+                                .is_connected(&condition.peer_id)
+                    });
+
+                    (target_peer_condition || hit_outbound_limit) && !trusted_peer_disconnected
                 });
 
-                TestResult::from_bool(
+                prop_assert!(result);
-                    (target_peer_condition || hit_outbound_limit) && !trusted_peer_disconnected,
+            }
-                )
-            })
         }
     }
 

consensus/merkle_proof/Cargo.toml

@@ -14,5 +14,4 @@ fixed_bytes = { workspace = true }
 safe_arith = { workspace = true }
 
 [dev-dependencies]
-quickcheck = { workspace = true }
+proptest = { workspace = true }
-quickcheck_macros = { workspace = true }

consensus/merkle_proof/src/lib.rs

@@ -413,50 +413,70 @@ impl From<InvalidSnapshot> for MerkleTreeError {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use quickcheck::TestResult;
-    use quickcheck_macros::quickcheck;
 
-    /// Check that we can:
+    use proptest::prelude::*;
-    /// 1. Build a MerkleTree from arbitrary leaves and an arbitrary depth.
-    /// 2. Generate valid proofs for all of the leaves of this MerkleTree.
-    #[quickcheck]
-    fn quickcheck_create_and_verify(int_leaves: Vec<u64>, depth: usize) -> TestResult {
-        if depth > MAX_TREE_DEPTH || int_leaves.len() > 2usize.pow(depth as u32) {
-            return TestResult::discard();
-        }
 
-        let leaves: Vec<_> = int_leaves.into_iter().map(H256::from_low_u64_be).collect();
+    // Limit test depth to avoid generating huge trees. Depth 10 = 1024 max leaves.
-        let merkle_tree = MerkleTree::create(&leaves, depth);
+    const TEST_MAX_DEPTH: usize = 10;
-        let merkle_root = merkle_tree.hash();
 
-        let proofs_ok = (0..leaves.len()).all(|i| {
+    fn merkle_leaves_strategy(max_depth: usize) -> impl Strategy<Value = (Vec<u64>, usize)> {
-            let (leaf, branch) = merkle_tree
+        (0..=max_depth).prop_flat_map(|depth| {
-                .generate_proof(i, depth)
+            let max_leaves = 2usize.pow(depth as u32);
-                .expect("should generate proof");
+            (
-            leaf == leaves[i] && verify_merkle_proof(leaf, &branch, depth, i, merkle_root)
+                proptest::collection::vec(any::<u64>(), 0..=max_leaves),
-        });
+                Just(depth),
-
+            )
-        TestResult::from_bool(proofs_ok)
+        })
     }
 
-    #[quickcheck]
+    fn merkle_leaves_strategy_min_depth(
-    fn quickcheck_push_leaf_and_verify(int_leaves: Vec<u64>, depth: usize) -> TestResult {
+        max_depth: usize,
-        if depth == 0 || depth > MAX_TREE_DEPTH || int_leaves.len() > 2usize.pow(depth as u32) {
+        min_depth: usize,
-            return TestResult::discard();
+    ) -> impl Strategy<Value = (Vec<u64>, usize)> {
+        (min_depth..=max_depth).prop_flat_map(|depth| {
+            let max_leaves = 2usize.pow(depth as u32);
+            (
+                proptest::collection::vec(any::<u64>(), 0..=max_leaves),
+                Just(depth),
+            )
+        })
+    }
+
+    proptest::proptest! {
+        /// Check that we can:
+        /// 1. Build a MerkleTree from arbitrary leaves and an arbitrary depth.
+        /// 2. Generate valid proofs for all of the leaves of this MerkleTree.
+        #[test]
+        fn proptest_create_and_verify((int_leaves, depth) in merkle_leaves_strategy(TEST_MAX_DEPTH)) {
+            let leaves: Vec<_> = int_leaves.into_iter().map(H256::from_low_u64_be).collect();
+            let merkle_tree = MerkleTree::create(&leaves, depth);
+            let merkle_root = merkle_tree.hash();
+
+            let proofs_ok = (0..leaves.len()).all(|i| {
+                let (leaf, branch) = merkle_tree
+                    .generate_proof(i, depth)
+                    .expect("should generate proof");
+                leaf == leaves[i] && verify_merkle_proof(leaf, &branch, depth, i, merkle_root)
+            });
+
+            proptest::prop_assert!(proofs_ok);
         }
 
-        let leaves_iter = int_leaves.into_iter().map(H256::from_low_u64_be);
+        #[test]
-        let mut merkle_tree = MerkleTree::create(&[], depth);
+        fn proptest_push_leaf_and_verify((int_leaves, depth) in merkle_leaves_strategy_min_depth(TEST_MAX_DEPTH, 1)) {
+            let leaves_iter = int_leaves.into_iter().map(H256::from_low_u64_be);
+            let mut merkle_tree = MerkleTree::create(&[], depth);
 
-        let proofs_ok = leaves_iter.enumerate().all(|(i, leaf)| {
+            let proofs_ok = leaves_iter.enumerate().all(|(i, leaf)| {
-            assert_eq!(merkle_tree.push_leaf(leaf, depth), Ok(()));
+                assert_eq!(merkle_tree.push_leaf(leaf, depth), Ok(()));
-            let (stored_leaf, branch) = merkle_tree
+                let (stored_leaf, branch) = merkle_tree
-                .generate_proof(i, depth)
+                    .generate_proof(i, depth)
-                .expect("should generate proof");
+                    .expect("should generate proof");
-            stored_leaf == leaf && verify_merkle_proof(leaf, &branch, depth, i, merkle_tree.hash())
+                stored_leaf == leaf && verify_merkle_proof(leaf, &branch, depth, i, merkle_tree.hash())
-        });
+            });
 
-        TestResult::from_bool(proofs_ok)
+            proptest::prop_assert!(proofs_ok);
+        }
     }
 
     #[test]