diff --git a/Cargo.lock b/Cargo.lock index a1ad2ab5ba..6cc99a659f 100644 --- a/Cargo.lock +++ b/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", - "quickcheck_macros", + "proptest", "rand 0.9.2", "regex", "serde", @@ -5833,8 +5822,7 @@ dependencies = [ "alloy-primitives", "ethereum_hashing", "fixed_bytes", - "quickcheck", - "quickcheck_macros", + "proptest", "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" diff --git a/Cargo.toml b/Cargo.toml index 713fbf25d8..f4ce67d0bf 100644 --- a/Cargo.toml +++ b/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" diff --git a/beacon_node/lighthouse_network/Cargo.toml b/beacon_node/lighthouse_network/Cargo.toml index 035452e4b2..6963b7c2dd 100644 --- a/beacon_node/lighthouse_network/Cargo.toml +++ b/beacon_node/lighthouse_network/Cargo.toml @@ -72,6 +72,5 @@ features = [ [dev-dependencies] async-channel = { workspace = true } logging = { workspace = true } -quickcheck = { workspace = true } -quickcheck_macros = { workspace = true } +proptest = { workspace = true } tempfile = { workspace = true } diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index ad16bb0421..dfa8b374e9 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/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 quickcheck_macros::quickcheck; + use proptest::prelude::*; use std::collections::HashSet; use tokio::runtime::Runtime; use types::{DataColumnSubnetId, Unsigned}; @@ -2994,159 +2993,202 @@ mod tests { custody_subnets: HashSet, } - impl Arbitrary for PeerCondition { - fn arbitrary(g: &mut Gen) -> Self { - let attestation_net_bitfield = { - let len = ::SubnetBitfieldLength::to_usize(); - let mut bitfield = Vec::with_capacity(len); - for _ in 0..len { - bitfield.push(bool::arbitrary(g)); - } - bitfield - }; + fn peer_condition_strategy() -> impl Strategy { + let attestation_len = ::SubnetBitfieldLength::to_usize(); + let sync_committee_len = ::SyncCommitteeSubnetCount::to_usize(); + let spec = E::default_spec(); + let total_subnet_count = spec.data_column_sidecar_subnet_count; + let custody_requirement = spec.custody_requirement; - let sync_committee_net_bitfield = { - let len = ::SyncCommitteeSubnetCount::to_usize(); - let mut bitfield = Vec::with_capacity(len); - for _ in 0..len { - bitfield.push(bool::arbitrary(g)); - } - bitfield - }; + // Create the pool of available subnet IDs + let available_subnets: Vec = (custody_requirement..total_subnet_count).collect(); + let max_custody_subnets = available_subnets.len(); - let spec = E::default_spec(); - let custody_subnets = { - let total_subnet_count = spec.data_column_sidecar_subnet_count; - 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() - }; + // Trusted peer probability constants - 1 in 5 peers should be trusted (20%) + const TRUSTED_PEER_WEIGHT_FALSE: u32 = 4; + const TRUSTED_PEER_WEIGHT_TRUE: u32 = 1; - PeerCondition { - peer_id: PeerId::random(), - outgoing: bool::arbitrary(g), - attestation_net_bitfield, - sync_committee_net_bitfield, - score: f64::arbitrary(g), - trusted: bool::arbitrary(g), - gossipsub_score: f64::arbitrary(g), - custody_subnets, - } - } + ( + proptest::collection::vec(any::(), attestation_len), + proptest::collection::vec(any::(), sync_committee_len), + any::(), + any::(), + any::(), + // Weight trusted peers to avoid test rejection due to too many trusted peers + prop_oneof![ + 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 = 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] - fn prune_excess_peers(peer_conditions: Vec) -> TestResult { - let target_peer_count = DEFAULT_TARGET_PEERS; - 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(); + // Upper bound for testing peer pruning - we test with at least the target number + // and up to 50% more than the target to verify pruning behavior. + const MAX_TEST_PEERS: usize = 300; - rt.block_on(async move { - // Collect all the trusted peers - let mut peer_manager = - build_peer_manager_with_trusted_peers(trusted_peers, target_peer_count).await; + proptest! { + #[test] + fn prune_excess_peers(peer_conditions in proptest::collection::vec(peer_condition_strategy(), DEFAULT_TARGET_PEERS..=MAX_TEST_PEERS)) { + let target_peer_count = DEFAULT_TARGET_PEERS; + let spec = E::default_spec(); - // Create peers based on the randomly generated conditions. - for condition in &peer_conditions { - let mut attnets = crate::types::EnrAttestationBitfield::::new(); - let mut syncnets = crate::types::EnrSyncCommitteeBitfield::::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 + let trusted_peers: Vec<_> = peer_conditions .iter() - .filter(|condition| condition.trusted) - .count(); - let expected_peer_count = expected_peer_count.max(no_of_trusted_peers); + .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. + prop_assume!(trusted_peers.len() <= peer_conditions.len() / 3_usize); - let target_peer_condition = - peer_manager.network_globals.connected_or_dialing_peers() - == expected_peer_count; + let rt = Runtime::new().unwrap(); - // 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(); + let result = rt.block_on(async move { + // Collect all the trusted peers + let mut peer_manager = + build_peer_manager_with_trusted_peers(trusted_peers, target_peer_count).await; - // 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) + // Create peers based on the randomly generated conditions. + for condition in &peer_conditions { + let mut attnets = crate::types::EnrAttestationBitfield::::new(); + let mut syncnets = crate::types::EnrSyncCommitteeBitfield::::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) + .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( - (target_peer_condition || hit_outbound_limit) && !trusted_peer_disconnected, - ) - }) + prop_assert!(result); + } } } diff --git a/consensus/merkle_proof/Cargo.toml b/consensus/merkle_proof/Cargo.toml index d750c05406..5ba8a1b949 100644 --- a/consensus/merkle_proof/Cargo.toml +++ b/consensus/merkle_proof/Cargo.toml @@ -14,5 +14,4 @@ fixed_bytes = { workspace = true } safe_arith = { workspace = true } [dev-dependencies] -quickcheck = { workspace = true } -quickcheck_macros = { workspace = true } +proptest = { workspace = true } diff --git a/consensus/merkle_proof/src/lib.rs b/consensus/merkle_proof/src/lib.rs index bf075ec15a..494c73d05c 100644 --- a/consensus/merkle_proof/src/lib.rs +++ b/consensus/merkle_proof/src/lib.rs @@ -413,50 +413,70 @@ impl From for MerkleTreeError { #[cfg(test)] mod tests { use super::*; - use quickcheck::TestResult; - use quickcheck_macros::quickcheck; - /// 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. - #[quickcheck] - fn quickcheck_create_and_verify(int_leaves: Vec, depth: usize) -> TestResult { - if depth > MAX_TREE_DEPTH || int_leaves.len() > 2usize.pow(depth as u32) { - return TestResult::discard(); - } + use proptest::prelude::*; - 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(); + // Limit test depth to avoid generating huge trees. Depth 10 = 1024 max leaves. + const TEST_MAX_DEPTH: usize = 10; - 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) - }); - - TestResult::from_bool(proofs_ok) + fn merkle_leaves_strategy(max_depth: usize) -> impl Strategy, usize)> { + (0..=max_depth).prop_flat_map(|depth| { + let max_leaves = 2usize.pow(depth as u32); + ( + proptest::collection::vec(any::(), 0..=max_leaves), + Just(depth), + ) + }) } - #[quickcheck] - fn quickcheck_push_leaf_and_verify(int_leaves: Vec, depth: usize) -> TestResult { - if depth == 0 || depth > MAX_TREE_DEPTH || int_leaves.len() > 2usize.pow(depth as u32) { - return TestResult::discard(); + fn merkle_leaves_strategy_min_depth( + max_depth: usize, + min_depth: usize, + ) -> impl Strategy, usize)> { + (min_depth..=max_depth).prop_flat_map(|depth| { + let max_leaves = 2usize.pow(depth as u32); + ( + proptest::collection::vec(any::(), 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); - let mut merkle_tree = MerkleTree::create(&[], depth); + #[test] + 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)| { - assert_eq!(merkle_tree.push_leaf(leaf, depth), Ok(())); - let (stored_leaf, branch) = merkle_tree - .generate_proof(i, depth) - .expect("should generate proof"); - stored_leaf == leaf && verify_merkle_proof(leaf, &branch, depth, i, merkle_tree.hash()) - }); + let proofs_ok = leaves_iter.enumerate().all(|(i, leaf)| { + assert_eq!(merkle_tree.push_leaf(leaf, depth), Ok(())); + let (stored_leaf, branch) = merkle_tree + .generate_proof(i, depth) + .expect("should generate proof"); + stored_leaf == leaf && verify_merkle_proof(leaf, &branch, depth, i, merkle_tree.hash()) + }); - TestResult::from_bool(proofs_ok) + proptest::prop_assert!(proofs_ok); + } } #[test]