From 1a22a096c6e86406c712b5f9a1f611618a48effa Mon Sep 17 00:00:00 2001 From: Akihito Nakano Date: Thu, 28 Jan 2021 23:31:06 +0000 Subject: [PATCH] Fix clippy errors on tests (#2160) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Issue Addressed There are some clippy error on tests. ## Proposed Changes Enable clippy check on tests and fix the errors. 💪 --- Makefile | 2 +- beacon_node/beacon_chain/src/eth1_chain.rs | 32 +- .../src/validator_pubkey_cache.rs | 1 + beacon_node/client/src/config.rs | 1 - beacon_node/eth1/src/deposit_cache.rs | 4 +- beacon_node/eth1/src/service.rs | 1 - beacon_node/eth2_libp2p/src/discovery/mod.rs | 10 +- .../eth2_libp2p/src/peer_manager/peerdb.rs | 8 +- .../eth2_libp2p/src/peer_manager/score.rs | 3 + beacon_node/eth2_libp2p/tests/common/mod.rs | 3 +- beacon_node/eth2_libp2p/tests/rpc_tests.rs | 19 +- .../network/src/attestation_service/mod.rs | 1 + .../src/attestation_service/tests/mod.rs | 816 +++++++++--------- beacon_node/store/src/iter.rs | 2 +- common/account_utils/src/lib.rs | 28 +- common/eth2_network_config/src/lib.rs | 4 +- common/lockfile/src/lib.rs | 2 +- consensus/int_to_bytes/src/lib.rs | 1 - .../src/proto_array_fork_choice.rs | 2 +- consensus/ssz/tests/tests.rs | 6 + consensus/ssz_types/src/bitfield.rs | 6 +- .../src/per_block_processing/tests.rs | 4 +- .../swap_or_not_shuffle/src/shuffle_list.rs | 1 + consensus/tree_hash/src/merkle_hasher.rs | 6 +- consensus/types/src/beacon_state/tests.rs | 31 +- consensus/types/src/chain_spec.rs | 1 + consensus/types/src/tree_hash_impls.rs | 9 +- remote_signer/backend/src/lib.rs | 4 +- slasher/tests/attester_slashings.rs | 2 +- slasher/tests/proposer_slashings.rs | 2 +- slasher/tests/random.rs | 2 +- slasher/tests/wrap_around.rs | 4 +- .../src/attestation_tests.rs | 6 +- .../slashing_protection/src/block_tests.rs | 2 +- .../slashing_protection/src/lib.rs | 1 + validator_client/src/fork_service.rs | 2 +- 36 files changed, 513 insertions(+), 516 deletions(-) diff --git a/Makefile b/Makefile index b5df59798f..797dbdd650 100644 --- a/Makefile +++ b/Makefile @@ -119,7 +119,7 @@ test-full: cargo-fmt test-release test-debug test-ef # Lints the code for bad style and potentially unsafe arithmetic using Clippy. # Clippy lints are opt-in per-crate for now. By default, everything is allowed except for performance and correctness lints. lint: - cargo clippy --all -- -D warnings + cargo clippy --all --tests -- -D warnings # Runs the makefile in the `ef_tests` repo. # diff --git a/beacon_node/beacon_chain/src/eth1_chain.rs b/beacon_node/beacon_chain/src/eth1_chain.rs index bffc6b70a9..ed74ff6e41 100644 --- a/beacon_node/beacon_chain/src/eth1_chain.rs +++ b/beacon_node/beacon_chain/src/eth1_chain.rs @@ -668,7 +668,6 @@ fn is_candidate_block(block: &Eth1Block, period_start: u64, spec: &ChainSpec) -> mod test { use super::*; use environment::null_logger; - use std::iter::FromIterator; use types::{test_utils::DepositTestTask, MinimalEthSpec}; type E = MinimalEthSpec; @@ -1042,10 +1041,7 @@ mod test { let votes_to_consider = get_eth1_data_vec(slots, 0); - let votes = collect_valid_votes( - &state, - &HashMap::from_iter(votes_to_consider.clone().into_iter()), - ); + let votes = collect_valid_votes(&state, &votes_to_consider.into_iter().collect()); assert_eq!( votes.len(), 0, @@ -1068,10 +1064,8 @@ mod test { .collect::>() .into(); - let votes = collect_valid_votes( - &state, - &HashMap::from_iter(votes_to_consider.clone().into_iter()), - ); + let votes = + collect_valid_votes(&state, &votes_to_consider.clone().into_iter().collect()); assert_votes!( votes, votes_to_consider[0..slots as usize / 4].to_vec(), @@ -1099,10 +1093,7 @@ mod test { .collect::>() .into(); - let votes = collect_valid_votes( - &state, - &HashMap::from_iter(votes_to_consider.clone().into_iter()), - ); + let votes = collect_valid_votes(&state, &votes_to_consider.into_iter().collect()); assert_votes!( votes, // There should only be one value if there's a duplicate @@ -1150,8 +1141,7 @@ mod test { assert_eq!( // Favour the highest block number when there are no votes. vote_data(&no_votes[2]), - find_winning_vote(Eth1DataVoteCount::from_iter(no_votes.into_iter())) - .expect("should find winner") + find_winning_vote(no_votes.into_iter().collect()).expect("should find winner") ); } @@ -1162,8 +1152,7 @@ mod test { assert_eq!( // Favour the highest block number when there are equal votes. vote_data(&votes[2]), - find_winning_vote(Eth1DataVoteCount::from_iter(votes.into_iter())) - .expect("should find winner") + find_winning_vote(votes.into_iter().collect()).expect("should find winner") ); } @@ -1174,8 +1163,7 @@ mod test { assert_eq!( // Favour the highest vote over the highest block number. vote_data(&votes[3]), - find_winning_vote(Eth1DataVoteCount::from_iter(votes.into_iter())) - .expect("should find winner") + find_winning_vote(votes.into_iter().collect()).expect("should find winner") ); } @@ -1186,8 +1174,7 @@ mod test { assert_eq!( // Favour the highest block number for tying votes. vote_data(&votes[3]), - find_winning_vote(Eth1DataVoteCount::from_iter(votes.into_iter())) - .expect("should find winner") + find_winning_vote(votes.into_iter().collect()).expect("should find winner") ); } @@ -1198,8 +1185,7 @@ mod test { assert_eq!( // Favour the highest block number for tying votes. vote_data(&votes[0]), - find_winning_vote(Eth1DataVoteCount::from_iter(votes.into_iter())) - .expect("should find winner") + find_winning_vote(votes.into_iter().collect()).expect("should find winner") ); } } diff --git a/beacon_node/beacon_chain/src/validator_pubkey_cache.rs b/beacon_node/beacon_chain/src/validator_pubkey_cache.rs index 03977b2410..e72a89ce70 100644 --- a/beacon_node/beacon_chain/src/validator_pubkey_cache.rs +++ b/beacon_node/beacon_chain/src/validator_pubkey_cache.rs @@ -238,6 +238,7 @@ mod test { builder.build() } + #[allow(clippy::needless_range_loop)] fn check_cache_get(cache: &ValidatorPubkeyCache, keypairs: &[Keypair]) { let validator_count = keypairs.len(); diff --git a/beacon_node/client/src/config.rs b/beacon_node/client/src/config.rs index 865724f3ac..c2d78dd539 100644 --- a/beacon_node/client/src/config.rs +++ b/beacon_node/client/src/config.rs @@ -160,7 +160,6 @@ fn ensure_dir_exists(path: PathBuf) -> Result { #[cfg(test)] mod tests { use super::*; - use toml; #[test] fn serde() { diff --git a/beacon_node/eth1/src/deposit_cache.rs b/beacon_node/eth1/src/deposit_cache.rs index cd16bfbf94..5b6ffec1c7 100644 --- a/beacon_node/eth1/src/deposit_cache.rs +++ b/beacon_node/eth1/src/deposit_cache.rs @@ -397,7 +397,7 @@ pub mod tests { .expect("should get the four from the full tree"); assert_eq!( deposits.len(), - 4 as usize, + 4_usize, "should get 4 deposits from full tree" ); assert_eq!( @@ -418,7 +418,7 @@ pub mod tests { .expect("should get the half tree"); assert_eq!( deposits.len(), - 4 as usize, + 4_usize, "should get 4 deposits from half tree" ); assert_eq!( diff --git a/beacon_node/eth1/src/service.rs b/beacon_node/eth1/src/service.rs index 43eee28e6c..20e1f5027b 100644 --- a/beacon_node/eth1/src/service.rs +++ b/beacon_node/eth1/src/service.rs @@ -1179,7 +1179,6 @@ async fn download_eth1_block( #[cfg(test)] mod tests { use super::*; - use toml; use types::MainnetEthSpec; #[test] diff --git a/beacon_node/eth2_libp2p/src/discovery/mod.rs b/beacon_node/eth2_libp2p/src/discovery/mod.rs index c22c7d6cdc..74df116517 100644 --- a/beacon_node/eth2_libp2p/src/discovery/mod.rs +++ b/beacon_node/eth2_libp2p/src/discovery/mod.rs @@ -964,8 +964,10 @@ mod tests { async fn build_discovery() -> Discovery { let keypair = libp2p::identity::Keypair::generate_secp256k1(); - let mut config = NetworkConfig::default(); - config.discovery_port = unused_port(); + let config = NetworkConfig { + discovery_port: unused_port(), + ..Default::default() + }; let enr_key: CombinedKey = CombinedKey::from_libp2p(&keypair).unwrap(); let enr: Enr = build_enr::(&enr_key, &config, EnrForkId::default()).unwrap(); let log = build_log(slog::Level::Debug, false); @@ -1055,7 +1057,7 @@ mod tests { discovery.queued_queries.push_back(QueryType::FindPeers); discovery .queued_queries - .push_back(QueryType::Subnet(subnet_query.clone())); + .push_back(QueryType::Subnet(subnet_query)); // Process Subnet query and FindPeers afterwards. assert!(discovery.process_queue()); } @@ -1101,7 +1103,7 @@ mod tests { // Unwanted enr for the given grouped query let enr3 = make_enr(vec![3]); - let enrs: Vec = vec![enr1.clone(), enr2.clone(), enr3.clone()]; + let enrs: Vec = vec![enr1.clone(), enr2, enr3]; let results = discovery .process_completed_queries(QueryResult(query, Ok(enrs))) .unwrap(); diff --git a/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs b/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs index 379505d807..b60a55d831 100644 --- a/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs +++ b/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs @@ -645,6 +645,7 @@ mod tests { } #[test] + #[allow(clippy::float_cmp)] fn test_peer_connected_successfully() { let mut pdb = get_db(); let random_peer = PeerId::random(); @@ -745,7 +746,7 @@ mod tests { assert!(the_best.is_some()); // Consistency check let best_peers = pdb.best_peers_by_status(PeerInfo::is_connected); - assert_eq!(the_best, best_peers.iter().next().map(|p| p.0)); + assert_eq!(the_best.unwrap(), best_peers.get(0).unwrap().0); } #[test] @@ -839,7 +840,7 @@ mod tests { pdb.notify_disconnect(&random_peer2); pdb.disconnect_and_ban(&random_peer3); pdb.notify_disconnect(&random_peer3); - pdb.connect_ingoing(&random_peer, multiaddr.clone(), None); + pdb.connect_ingoing(&random_peer, multiaddr, None); assert_eq!(pdb.disconnected_peers, pdb.disconnected_peers().count()); assert_eq!( pdb.banned_peers_count.banned_peers(), @@ -1021,10 +1022,11 @@ mod tests { } #[test] + #[allow(clippy::float_cmp)] fn test_trusted_peers_score() { let trusted_peer = PeerId::random(); let log = build_log(slog::Level::Debug, false); - let mut pdb: PeerDB = PeerDB::new(vec![trusted_peer.clone()], &log); + let mut pdb: PeerDB = PeerDB::new(vec![trusted_peer], &log); pdb.connect_ingoing(&trusted_peer, "/ip4/0.0.0.0".parse().unwrap(), None); diff --git a/beacon_node/eth2_libp2p/src/peer_manager/score.rs b/beacon_node/eth2_libp2p/src/peer_manager/score.rs index e185f61014..38fecad3af 100644 --- a/beacon_node/eth2_libp2p/src/peer_manager/score.rs +++ b/beacon_node/eth2_libp2p/src/peer_manager/score.rs @@ -346,6 +346,7 @@ mod tests { use super::*; #[test] + #[allow(clippy::float_cmp)] fn test_reputation_change() { let mut score = Score::default(); @@ -375,6 +376,7 @@ mod tests { } #[test] + #[allow(clippy::float_cmp)] fn test_ban_time() { let mut score = RealScore::default(); let now = Instant::now(); @@ -402,6 +404,7 @@ mod tests { } #[test] + #[allow(clippy::float_cmp)] fn test_ignored_gossipsub_score() { let mut score = Score::default(); score.update_gossipsub_score(GOSSIPSUB_GREYLIST_THRESHOLD, true); diff --git a/beacon_node/eth2_libp2p/tests/common/mod.rs b/beacon_node/eth2_libp2p/tests/common/mod.rs index d13efade26..e0aba71f65 100644 --- a/beacon_node/eth2_libp2p/tests/common/mod.rs +++ b/beacon_node/eth2_libp2p/tests/common/mod.rs @@ -126,8 +126,7 @@ pub async fn build_libp2p_instance( #[allow(dead_code)] pub fn get_enr(node: &LibP2PService) -> Enr { - let enr = node.swarm.local_enr().clone(); - enr + node.swarm.local_enr() } // Returns `n` libp2p peers in fully connected topology. diff --git a/beacon_node/eth2_libp2p/tests/rpc_tests.rs b/beacon_node/eth2_libp2p/tests/rpc_tests.rs index 29abd14fe3..e054964e16 100644 --- a/beacon_node/eth2_libp2p/tests/rpc_tests.rs +++ b/beacon_node/eth2_libp2p/tests/rpc_tests.rs @@ -17,6 +17,7 @@ type E = MinimalEthSpec; // Tests the STATUS RPC message #[test] +#[allow(clippy::single_match)] fn test_status_rpc() { // set up the logging. The level and enabled logging or not let log_level = Level::Debug; @@ -113,6 +114,7 @@ fn test_status_rpc() { // Tests a streamed BlocksByRange RPC Message #[test] +#[allow(clippy::single_match)] fn test_blocks_by_range_chunked_rpc() { // set up the logging. The level and enabled logging or not let log_level = Level::Trace; @@ -199,7 +201,7 @@ fn test_blocks_by_range_chunked_rpc() { warn!(log, "Receiver got request"); for _ in 1..=messages_to_send { receiver.swarm.send_successful_response( - peer_id.clone(), + peer_id, id, rpc_response.clone(), ); @@ -340,8 +342,8 @@ fn test_blocks_by_range_chunked_rpc_terminates_correctly() { messages_sent += 1; let (peer_id, stream_id) = message_info.as_ref().unwrap(); receiver.swarm.send_successful_response( - peer_id.clone(), - stream_id.clone(), + *peer_id, + *stream_id, rpc_response.clone(), ); debug!(log, "Sending message {}", messages_sent); @@ -365,6 +367,7 @@ fn test_blocks_by_range_chunked_rpc_terminates_correctly() { // Tests an empty response to a BlocksByRange RPC Message #[test] +#[allow(clippy::single_match)] fn test_blocks_by_range_single_empty_rpc() { // set up the logging. The level and enabled logging or not let log_level = Level::Trace; @@ -448,7 +451,7 @@ fn test_blocks_by_range_single_empty_rpc() { for _ in 1..=messages_to_send { receiver.swarm.send_successful_response( - peer_id.clone(), + peer_id, id, rpc_response.clone(), ); @@ -480,6 +483,7 @@ fn test_blocks_by_range_single_empty_rpc() { // which is greater than the Snappy frame size. Hence, this test // serves to test the snappy framing format as well. #[test] +#[allow(clippy::single_match)] fn test_blocks_by_root_chunked_rpc() { // set up the logging. The level and enabled logging or not let log_level = Level::Debug; @@ -565,7 +569,7 @@ fn test_blocks_by_root_chunked_rpc() { for _ in 1..=messages_to_send { receiver.swarm.send_successful_response( - peer_id.clone(), + peer_id, id, rpc_response.clone(), ); @@ -715,8 +719,8 @@ fn test_blocks_by_root_chunked_rpc_terminates_correctly() { messages_sent += 1; let (peer_id, stream_id) = message_info.as_ref().unwrap(); receiver.swarm.send_successful_response( - peer_id.clone(), - stream_id.clone(), + *peer_id, + *stream_id, rpc_response.clone(), ); debug!(log, "Sending message {}", messages_sent); @@ -740,6 +744,7 @@ fn test_blocks_by_root_chunked_rpc_terminates_correctly() { // Tests a Goodbye RPC message #[test] +#[allow(clippy::single_match)] fn test_goodbye_rpc() { // set up the logging. The level and enabled logging or not let log_level = Level::Trace; diff --git a/beacon_node/network/src/attestation_service/mod.rs b/beacon_node/network/src/attestation_service/mod.rs index 5510bbc818..da031877ac 100644 --- a/beacon_node/network/src/attestation_service/mod.rs +++ b/beacon_node/network/src/attestation_service/mod.rs @@ -20,6 +20,7 @@ use types::{Attestation, EthSpec, Slot, SubnetId, ValidatorSubscription}; use crate::metrics; +#[cfg(test)] mod tests; /// The minimum number of slots ahead that we attempt to discover peers for a subscription. If the diff --git a/beacon_node/network/src/attestation_service/tests/mod.rs b/beacon_node/network/src/attestation_service/tests/mod.rs index b9587a6601..2462f2c94e 100644 --- a/beacon_node/network/src/attestation_service/tests/mod.rs +++ b/beacon_node/network/src/attestation_service/tests/mod.rs @@ -1,432 +1,424 @@ -#[cfg(test)] -mod tests { - use super::super::*; - use beacon_chain::{ - builder::{BeaconChainBuilder, Witness}, - eth1_chain::CachingEth1Backend, - }; - use futures::Stream; - use genesis::{generate_deterministic_keypairs, interop_genesis_state}; - use lazy_static::lazy_static; - use matches::assert_matches; - use slog::Logger; - use sloggers::{null::NullLoggerBuilder, Build}; - use slot_clock::{SlotClock, SystemTimeSlotClock}; - use std::time::{Duration, SystemTime}; - use store::config::StoreConfig; - use store::{HotColdDB, MemoryStore}; - use tempfile::tempdir; - use types::{CommitteeIndex, EthSpec, MinimalEthSpec}; +use super::*; +use beacon_chain::{ + builder::{BeaconChainBuilder, Witness}, + eth1_chain::CachingEth1Backend, +}; +use futures::Stream; +use genesis::{generate_deterministic_keypairs, interop_genesis_state}; +use lazy_static::lazy_static; +use matches::assert_matches; +use slog::Logger; +use sloggers::{null::NullLoggerBuilder, Build}; +use slot_clock::{SlotClock, SystemTimeSlotClock}; +use std::time::{Duration, SystemTime}; +use store::config::StoreConfig; +use store::{HotColdDB, MemoryStore}; +use tempfile::tempdir; +use types::{CommitteeIndex, EthSpec, MinimalEthSpec}; - const SLOT_DURATION_MILLIS: u64 = 400; +const SLOT_DURATION_MILLIS: u64 = 400; - type TestBeaconChainType = Witness< - SystemTimeSlotClock, - CachingEth1Backend, - MinimalEthSpec, - MemoryStore, - MemoryStore, - >; +type TestBeaconChainType = Witness< + SystemTimeSlotClock, + CachingEth1Backend, + MinimalEthSpec, + MemoryStore, + MemoryStore, +>; - pub struct TestBeaconChain { - chain: Arc>, - } +pub struct TestBeaconChain { + chain: Arc>, +} - impl TestBeaconChain { - pub fn new_with_system_clock() -> Self { - let data_dir = tempdir().expect("should create temporary data_dir"); - let spec = MinimalEthSpec::default_spec(); +impl TestBeaconChain { + pub fn new_with_system_clock() -> Self { + let data_dir = tempdir().expect("should create temporary data_dir"); + let spec = MinimalEthSpec::default_spec(); - let keypairs = generate_deterministic_keypairs(1); + let keypairs = generate_deterministic_keypairs(1); - let log = get_logger(); - let store = - HotColdDB::open_ephemeral(StoreConfig::default(), spec.clone(), log.clone()) - .unwrap(); - - let (shutdown_tx, _) = futures::channel::mpsc::channel(1); - - let chain = Arc::new( - BeaconChainBuilder::new(MinimalEthSpec) - .logger(log.clone()) - .custom_spec(spec.clone()) - .store(Arc::new(store)) - .data_dir(data_dir.path().to_path_buf()) - .genesis_state( - interop_genesis_state::(&keypairs, 0, &spec) - .expect("should generate interop state"), - ) - .expect("should build state using recent genesis") - .dummy_eth1_backend() - .expect("should build dummy backend") - .slot_clock(SystemTimeSlotClock::new( - Slot::new(0), - Duration::from_secs(recent_genesis_time()), - Duration::from_millis(SLOT_DURATION_MILLIS), - )) - .shutdown_sender(shutdown_tx) - .monitor_validators(true, vec![], log) - .build() - .expect("should build"), - ); - Self { chain } - } - } - - pub fn recent_genesis_time() -> u64 { - SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .unwrap() - .as_secs() - } - - fn get_logger() -> Logger { - NullLoggerBuilder.build().expect("logger should build") - } - - lazy_static! { - static ref CHAIN: TestBeaconChain = TestBeaconChain::new_with_system_clock(); - } - - fn get_attestation_service() -> AttestationService { let log = get_logger(); - let config = NetworkConfig::default(); + let store = + HotColdDB::open_ephemeral(StoreConfig::default(), spec.clone(), log.clone()).unwrap(); - let beacon_chain = CHAIN.chain.clone(); + let (shutdown_tx, _) = futures::channel::mpsc::channel(1); - AttestationService::new(beacon_chain, &config, &log) - } - - fn get_subscription( - validator_index: u64, - attestation_committee_index: CommitteeIndex, - slot: Slot, - committee_count_at_slot: u64, - ) -> ValidatorSubscription { - let is_aggregator = true; - ValidatorSubscription { - validator_index, - attestation_committee_index, - slot, - committee_count_at_slot, - is_aggregator, - } - } - - fn get_subscriptions( - validator_count: u64, - slot: Slot, - committee_count_at_slot: u64, - ) -> Vec { - (0..validator_count) - .map(|validator_index| { - get_subscription( - validator_index, - validator_index, - slot, - committee_count_at_slot, + let chain = Arc::new( + BeaconChainBuilder::new(MinimalEthSpec) + .logger(log.clone()) + .custom_spec(spec.clone()) + .store(Arc::new(store)) + .data_dir(data_dir.path().to_path_buf()) + .genesis_state( + interop_genesis_state::(&keypairs, 0, &spec) + .expect("should generate interop state"), ) - }) - .collect() + .expect("should build state using recent genesis") + .dummy_eth1_backend() + .expect("should build dummy backend") + .slot_clock(SystemTimeSlotClock::new( + Slot::new(0), + Duration::from_secs(recent_genesis_time()), + Duration::from_millis(SLOT_DURATION_MILLIS), + )) + .shutdown_sender(shutdown_tx) + .monitor_validators(true, vec![], log) + .build() + .expect("should build"), + ); + Self { chain } } +} - // gets a number of events from the subscription service, or returns none if it times out after a number - // of slots - async fn get_events + Unpin>( - stream: &mut S, - num_events: Option, - num_slots_before_timeout: u32, - ) -> Vec { - let mut events = Vec::new(); +pub fn recent_genesis_time() -> u64 { + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs() +} - let collect_stream_fut = async { - loop { - if let Some(result) = stream.next().await { - events.push(result); - if let Some(num) = num_events { - if events.len() == num { - return; - } +fn get_logger() -> Logger { + NullLoggerBuilder.build().expect("logger should build") +} + +lazy_static! { + static ref CHAIN: TestBeaconChain = TestBeaconChain::new_with_system_clock(); +} + +fn get_attestation_service() -> AttestationService { + let log = get_logger(); + let config = NetworkConfig::default(); + + let beacon_chain = CHAIN.chain.clone(); + + AttestationService::new(beacon_chain, &config, &log) +} + +fn get_subscription( + validator_index: u64, + attestation_committee_index: CommitteeIndex, + slot: Slot, + committee_count_at_slot: u64, +) -> ValidatorSubscription { + let is_aggregator = true; + ValidatorSubscription { + validator_index, + attestation_committee_index, + slot, + committee_count_at_slot, + is_aggregator, + } +} + +fn get_subscriptions( + validator_count: u64, + slot: Slot, + committee_count_at_slot: u64, +) -> Vec { + (0..validator_count) + .map(|validator_index| { + get_subscription( + validator_index, + validator_index, + slot, + committee_count_at_slot, + ) + }) + .collect() +} + +// gets a number of events from the subscription service, or returns none if it times out after a number +// of slots +async fn get_events + Unpin>( + stream: &mut S, + num_events: Option, + num_slots_before_timeout: u32, +) -> Vec { + let mut events = Vec::new(); + + let collect_stream_fut = async { + loop { + if let Some(result) = stream.next().await { + events.push(result); + if let Some(num) = num_events { + if events.len() == num { + return; } } } - }; - - tokio::select! { - _ = collect_stream_fut => {return events} - _ = tokio::time::sleep( - Duration::from_millis(SLOT_DURATION_MILLIS) * num_slots_before_timeout, - ) => { return events; } - } - } - - #[tokio::test] - async fn subscribe_current_slot_wait_for_unsubscribe() { - // subscription config - let validator_index = 1; - let committee_index = 1; - // Keep a low subscription slot so that there are no additional subnet discovery events. - let subscription_slot = 0; - let committee_count = 1; - - // create the attestation service and subscriptions - let mut attestation_service = get_attestation_service(); - let current_slot = attestation_service - .beacon_chain - .slot_clock - .now() - .expect("Could not get current slot"); - - let subscriptions = vec![get_subscription( - validator_index, - committee_index, - current_slot + Slot::new(subscription_slot), - committee_count, - )]; - - // submit the subscriptions - attestation_service - .validator_subscriptions(subscriptions) - .unwrap(); - - // not enough time for peer discovery, just subscribe, unsubscribe - let subnet_id = SubnetId::compute_subnet::( - current_slot + Slot::new(subscription_slot), - committee_index, - committee_count, - &attestation_service.beacon_chain.spec, - ) - .unwrap(); - let expected = vec![ - AttServiceMessage::Subscribe(subnet_id), - AttServiceMessage::Unsubscribe(subnet_id), - ]; - - // Wait for 1 slot duration to get the unsubscription event - let events = get_events(&mut attestation_service, None, 1).await; - assert_matches!( - events[..3], - [AttServiceMessage::DiscoverPeers(_), AttServiceMessage::Subscribe(_any1), AttServiceMessage::EnrAdd(_any3)] - ); - - // If the long lived and short lived subnets are the same, there should be no more events - // as we don't resubscribe already subscribed subnets. - if !attestation_service.random_subnets.contains(&subnet_id) { - assert_eq!(expected[..], events[3..]); } - // Should be subscribed to only 1 long lived subnet after unsubscription. - assert_eq!(attestation_service.subscription_count(), 1); - } + }; - /// Test to verify that we are not unsubscribing to a subnet before a required subscription. - #[tokio::test] - async fn test_same_subnet_unsubscription() { - // subscription config - let validator_index = 1; - let committee_count = 1; - - // Makes 2 validator subscriptions to the same subnet but at different slots. - // There should be just 1 unsubscription event for the later slot subscription (subscription_slot2). - let subscription_slot1 = 0; - let subscription_slot2 = 1; - let com1 = 1; - let com2 = 0; - - // create the attestation service and subscriptions - let mut attestation_service = get_attestation_service(); - let current_slot = attestation_service - .beacon_chain - .slot_clock - .now() - .expect("Could not get current slot"); - - let sub1 = get_subscription( - validator_index, - com1, - current_slot + Slot::new(subscription_slot1), - committee_count, - ); - - let sub2 = get_subscription( - validator_index, - com2, - current_slot + Slot::new(subscription_slot2), - committee_count, - ); - - let subnet_id1 = SubnetId::compute_subnet::( - current_slot + Slot::new(subscription_slot1), - com1, - committee_count, - &attestation_service.beacon_chain.spec, - ) - .unwrap(); - - let subnet_id2 = SubnetId::compute_subnet::( - current_slot + Slot::new(subscription_slot2), - com2, - committee_count, - &attestation_service.beacon_chain.spec, - ) - .unwrap(); - - // Assert that subscriptions are different but their subnet is the same - assert_ne!(sub1, sub2); - assert_eq!(subnet_id1, subnet_id2); - - // submit the subscriptions - attestation_service - .validator_subscriptions(vec![sub1, sub2]) - .unwrap(); - - // Unsubscription event should happen at slot 2 (since subnet id's are the same, unsubscription event should be at higher slot + 1) - // Get all events for 1 slot duration (unsubscription event should happen after 2 slot durations). - let events = get_events(&mut attestation_service, None, 1).await; - assert_matches!( - events[..3], - [AttServiceMessage::DiscoverPeers(_), AttServiceMessage::Subscribe(_any1), AttServiceMessage::EnrAdd(_any3)] - ); - - let expected = AttServiceMessage::Subscribe(subnet_id1); - - // Should be still subscribed to 1 long lived and 1 short lived subnet if both are different. - if !attestation_service.random_subnets.contains(&subnet_id1) { - assert_eq!(expected, events[3]); - assert_eq!(attestation_service.subscription_count(), 2); - } else { - assert_eq!(attestation_service.subscription_count(), 1); + tokio::select! { + _ = collect_stream_fut => {return events} + _ = tokio::time::sleep( + Duration::from_millis(SLOT_DURATION_MILLIS) * num_slots_before_timeout, + ) => { return events; } } - - // Get event for 1 more slot duration, we should get the unsubscribe event now. - let unsubscribe_event = get_events(&mut attestation_service, None, 1).await; - - // If the long lived and short lived subnets are different, we should get an unsubscription event. - if !attestation_service.random_subnets.contains(&subnet_id1) { - assert_eq!( - [AttServiceMessage::Unsubscribe(subnet_id1)], - unsubscribe_event[..] - ); - } - - // Should be subscribed to only 1 long lived subnet after unsubscription. - assert_eq!(attestation_service.subscription_count(), 1); - } - - #[tokio::test] - async fn subscribe_all_random_subnets() { - let attestation_subnet_count = MinimalEthSpec::default_spec().attestation_subnet_count; - let subscription_slot = 10; - let subscription_count = attestation_subnet_count; - let committee_count = 1; - - // create the attestation service and subscriptions - let mut attestation_service = get_attestation_service(); - let current_slot = attestation_service - .beacon_chain - .slot_clock - .now() - .expect("Could not get current slot"); - - let subscriptions = get_subscriptions( - subscription_count, - current_slot + subscription_slot, - committee_count, - ); - - // submit the subscriptions - attestation_service - .validator_subscriptions(subscriptions) - .unwrap(); - - let events = get_events(&mut attestation_service, None, 3).await; - let mut discover_peer_count = 0; - let mut enr_add_count = 0; - let mut unexpected_msg_count = 0; - - for event in &events { - match event { - AttServiceMessage::DiscoverPeers(_) => { - discover_peer_count = discover_peer_count + 1 - } - AttServiceMessage::Subscribe(_any_subnet) => {} - AttServiceMessage::EnrAdd(_any_subnet) => enr_add_count = enr_add_count + 1, - _ => unexpected_msg_count = unexpected_msg_count + 1, - } - } - - // The bulk discovery request length should be equal to validator_count - let bulk_discovery_event = events.last().unwrap(); - if let AttServiceMessage::DiscoverPeers(d) = bulk_discovery_event { - assert_eq!(d.len(), attestation_subnet_count as usize); - } else { - panic!("Unexpected event {:?}", bulk_discovery_event); - } - - // 64 `DiscoverPeer` requests of length 1 corresponding to random subnets - // and 1 `DiscoverPeer` request corresponding to bulk subnet discovery. - assert_eq!(discover_peer_count, subscription_count + 1); - assert_eq!(attestation_service.subscription_count(), 64); - assert_eq!(enr_add_count, 64); - assert_eq!(unexpected_msg_count, 0); - // test completed successfully - } - - #[tokio::test] - async fn subscribe_all_random_subnets_plus_one() { - let attestation_subnet_count = MinimalEthSpec::default_spec().attestation_subnet_count; - let subscription_slot = 10; - // the 65th subscription should result in no more messages than the previous scenario - let subscription_count = attestation_subnet_count + 1; - let committee_count = 1; - - // create the attestation service and subscriptions - let mut attestation_service = get_attestation_service(); - let current_slot = attestation_service - .beacon_chain - .slot_clock - .now() - .expect("Could not get current slot"); - - let subscriptions = get_subscriptions( - subscription_count, - current_slot + subscription_slot, - committee_count, - ); - - // submit the subscriptions - attestation_service - .validator_subscriptions(subscriptions) - .unwrap(); - - let events = get_events(&mut attestation_service, None, 3).await; - let mut discover_peer_count = 0; - let mut enr_add_count = 0; - let mut unexpected_msg_count = 0; - - for event in &events { - match event { - AttServiceMessage::DiscoverPeers(_) => { - discover_peer_count = discover_peer_count + 1 - } - AttServiceMessage::Subscribe(_any_subnet) => {} - AttServiceMessage::EnrAdd(_any_subnet) => enr_add_count = enr_add_count + 1, - _ => unexpected_msg_count = unexpected_msg_count + 1, - } - } - - // The bulk discovery request length shouldn't exceed max attestation_subnet_count - let bulk_discovery_event = events.last().unwrap(); - if let AttServiceMessage::DiscoverPeers(d) = bulk_discovery_event { - assert_eq!(d.len(), attestation_subnet_count as usize); - } else { - panic!("Unexpected event {:?}", bulk_discovery_event); - } - // 64 `DiscoverPeer` requests of length 1 corresponding to random subnets - // and 1 `DiscoverPeer` request corresponding to the bulk subnet discovery. - // For the 65th subscription, the call to `subscribe_to_random_subnets` is not made because we are at capacity. - assert_eq!(discover_peer_count, 64 + 1); - assert_eq!(attestation_service.subscription_count(), 64); - assert_eq!(enr_add_count, 64); - assert_eq!(unexpected_msg_count, 0); - } +} + +#[tokio::test] +async fn subscribe_current_slot_wait_for_unsubscribe() { + // subscription config + let validator_index = 1; + let committee_index = 1; + // Keep a low subscription slot so that there are no additional subnet discovery events. + let subscription_slot = 0; + let committee_count = 1; + + // create the attestation service and subscriptions + let mut attestation_service = get_attestation_service(); + let current_slot = attestation_service + .beacon_chain + .slot_clock + .now() + .expect("Could not get current slot"); + + let subscriptions = vec![get_subscription( + validator_index, + committee_index, + current_slot + Slot::new(subscription_slot), + committee_count, + )]; + + // submit the subscriptions + attestation_service + .validator_subscriptions(subscriptions) + .unwrap(); + + // not enough time for peer discovery, just subscribe, unsubscribe + let subnet_id = SubnetId::compute_subnet::( + current_slot + Slot::new(subscription_slot), + committee_index, + committee_count, + &attestation_service.beacon_chain.spec, + ) + .unwrap(); + let expected = vec![ + AttServiceMessage::Subscribe(subnet_id), + AttServiceMessage::Unsubscribe(subnet_id), + ]; + + // Wait for 1 slot duration to get the unsubscription event + let events = get_events(&mut attestation_service, None, 1).await; + assert_matches!( + events[..3], + [AttServiceMessage::DiscoverPeers(_), AttServiceMessage::Subscribe(_any1), AttServiceMessage::EnrAdd(_any3)] + ); + + // If the long lived and short lived subnets are the same, there should be no more events + // as we don't resubscribe already subscribed subnets. + if !attestation_service.random_subnets.contains(&subnet_id) { + assert_eq!(expected[..], events[3..]); + } + // Should be subscribed to only 1 long lived subnet after unsubscription. + assert_eq!(attestation_service.subscription_count(), 1); +} + +/// Test to verify that we are not unsubscribing to a subnet before a required subscription. +#[tokio::test] +async fn test_same_subnet_unsubscription() { + // subscription config + let validator_index = 1; + let committee_count = 1; + + // Makes 2 validator subscriptions to the same subnet but at different slots. + // There should be just 1 unsubscription event for the later slot subscription (subscription_slot2). + let subscription_slot1 = 0; + let subscription_slot2 = 1; + let com1 = 1; + let com2 = 0; + + // create the attestation service and subscriptions + let mut attestation_service = get_attestation_service(); + let current_slot = attestation_service + .beacon_chain + .slot_clock + .now() + .expect("Could not get current slot"); + + let sub1 = get_subscription( + validator_index, + com1, + current_slot + Slot::new(subscription_slot1), + committee_count, + ); + + let sub2 = get_subscription( + validator_index, + com2, + current_slot + Slot::new(subscription_slot2), + committee_count, + ); + + let subnet_id1 = SubnetId::compute_subnet::( + current_slot + Slot::new(subscription_slot1), + com1, + committee_count, + &attestation_service.beacon_chain.spec, + ) + .unwrap(); + + let subnet_id2 = SubnetId::compute_subnet::( + current_slot + Slot::new(subscription_slot2), + com2, + committee_count, + &attestation_service.beacon_chain.spec, + ) + .unwrap(); + + // Assert that subscriptions are different but their subnet is the same + assert_ne!(sub1, sub2); + assert_eq!(subnet_id1, subnet_id2); + + // submit the subscriptions + attestation_service + .validator_subscriptions(vec![sub1, sub2]) + .unwrap(); + + // Unsubscription event should happen at slot 2 (since subnet id's are the same, unsubscription event should be at higher slot + 1) + // Get all events for 1 slot duration (unsubscription event should happen after 2 slot durations). + let events = get_events(&mut attestation_service, None, 1).await; + assert_matches!( + events[..3], + [AttServiceMessage::DiscoverPeers(_), AttServiceMessage::Subscribe(_any1), AttServiceMessage::EnrAdd(_any3)] + ); + + let expected = AttServiceMessage::Subscribe(subnet_id1); + + // Should be still subscribed to 1 long lived and 1 short lived subnet if both are different. + if !attestation_service.random_subnets.contains(&subnet_id1) { + assert_eq!(expected, events[3]); + assert_eq!(attestation_service.subscription_count(), 2); + } else { + assert_eq!(attestation_service.subscription_count(), 1); + } + + // Get event for 1 more slot duration, we should get the unsubscribe event now. + let unsubscribe_event = get_events(&mut attestation_service, None, 1).await; + + // If the long lived and short lived subnets are different, we should get an unsubscription event. + if !attestation_service.random_subnets.contains(&subnet_id1) { + assert_eq!( + [AttServiceMessage::Unsubscribe(subnet_id1)], + unsubscribe_event[..] + ); + } + + // Should be subscribed to only 1 long lived subnet after unsubscription. + assert_eq!(attestation_service.subscription_count(), 1); +} + +#[tokio::test] +async fn subscribe_all_random_subnets() { + let attestation_subnet_count = MinimalEthSpec::default_spec().attestation_subnet_count; + let subscription_slot = 10; + let subscription_count = attestation_subnet_count; + let committee_count = 1; + + // create the attestation service and subscriptions + let mut attestation_service = get_attestation_service(); + let current_slot = attestation_service + .beacon_chain + .slot_clock + .now() + .expect("Could not get current slot"); + + let subscriptions = get_subscriptions( + subscription_count, + current_slot + subscription_slot, + committee_count, + ); + + // submit the subscriptions + attestation_service + .validator_subscriptions(subscriptions) + .unwrap(); + + let events = get_events(&mut attestation_service, None, 3).await; + let mut discover_peer_count = 0; + let mut enr_add_count = 0; + let mut unexpected_msg_count = 0; + + for event in &events { + match event { + AttServiceMessage::DiscoverPeers(_) => discover_peer_count += 1, + AttServiceMessage::Subscribe(_any_subnet) => {} + AttServiceMessage::EnrAdd(_any_subnet) => enr_add_count += 1, + _ => unexpected_msg_count += 1, + } + } + + // The bulk discovery request length should be equal to validator_count + let bulk_discovery_event = events.last().unwrap(); + if let AttServiceMessage::DiscoverPeers(d) = bulk_discovery_event { + assert_eq!(d.len(), attestation_subnet_count as usize); + } else { + panic!("Unexpected event {:?}", bulk_discovery_event); + } + + // 64 `DiscoverPeer` requests of length 1 corresponding to random subnets + // and 1 `DiscoverPeer` request corresponding to bulk subnet discovery. + assert_eq!(discover_peer_count, subscription_count + 1); + assert_eq!(attestation_service.subscription_count(), 64); + assert_eq!(enr_add_count, 64); + assert_eq!(unexpected_msg_count, 0); + // test completed successfully +} + +#[tokio::test] +async fn subscribe_all_random_subnets_plus_one() { + let attestation_subnet_count = MinimalEthSpec::default_spec().attestation_subnet_count; + let subscription_slot = 10; + // the 65th subscription should result in no more messages than the previous scenario + let subscription_count = attestation_subnet_count + 1; + let committee_count = 1; + + // create the attestation service and subscriptions + let mut attestation_service = get_attestation_service(); + let current_slot = attestation_service + .beacon_chain + .slot_clock + .now() + .expect("Could not get current slot"); + + let subscriptions = get_subscriptions( + subscription_count, + current_slot + subscription_slot, + committee_count, + ); + + // submit the subscriptions + attestation_service + .validator_subscriptions(subscriptions) + .unwrap(); + + let events = get_events(&mut attestation_service, None, 3).await; + let mut discover_peer_count = 0; + let mut enr_add_count = 0; + let mut unexpected_msg_count = 0; + + for event in &events { + match event { + AttServiceMessage::DiscoverPeers(_) => discover_peer_count += 1, + AttServiceMessage::Subscribe(_any_subnet) => {} + AttServiceMessage::EnrAdd(_any_subnet) => enr_add_count += 1, + _ => unexpected_msg_count += 1, + } + } + + // The bulk discovery request length shouldn't exceed max attestation_subnet_count + let bulk_discovery_event = events.last().unwrap(); + if let AttServiceMessage::DiscoverPeers(d) = bulk_discovery_event { + assert_eq!(d.len(), attestation_subnet_count as usize); + } else { + panic!("Unexpected event {:?}", bulk_discovery_event); + } + // 64 `DiscoverPeer` requests of length 1 corresponding to random subnets + // and 1 `DiscoverPeer` request corresponding to the bulk subnet discovery. + // For the 65th subscription, the call to `subscribe_to_random_subnets` is not made because we are at capacity. + assert_eq!(discover_peer_count, 64 + 1); + assert_eq!(attestation_service.subscription_count(), 64); + assert_eq!(enr_add_count, 64); + assert_eq!(unexpected_msg_count, 0); } diff --git a/beacon_node/store/src/iter.rs b/beacon_node/store/src/iter.rs index 5a97a39170..54b1f5df69 100644 --- a/beacon_node/store/src/iter.rs +++ b/beacon_node/store/src/iter.rs @@ -432,7 +432,7 @@ mod test { let state_b_root = Hash256::from_low_u64_be(slots_per_historical_root as u64 * 2); store.put_state(&state_a_root, &state_a).unwrap(); - store.put_state(&state_b_root, &state_b.clone()).unwrap(); + store.put_state(&state_b_root, &state_b).unwrap(); let iter = StateRootsIterator::new(store, &state_b); diff --git a/common/account_utils/src/lib.rs b/common/account_utils/src/lib.rs index f969db7020..152b6ba465 100644 --- a/common/account_utils/src/lib.rs +++ b/common/account_utils/src/lib.rs @@ -202,36 +202,24 @@ mod test { #[test] fn test_strip_off() { - let expected = "hello world".as_bytes().to_vec(); + let expected = b"hello world".to_vec(); + assert_eq!(strip_off_newlines(b"hello world\n".to_vec()), expected); assert_eq!( - strip_off_newlines("hello world\n".as_bytes().to_vec()), + strip_off_newlines(b"hello world\n\n\n\n".to_vec()), expected ); + assert_eq!(strip_off_newlines(b"hello world\r".to_vec()), expected); assert_eq!( - strip_off_newlines("hello world\n\n\n\n".as_bytes().to_vec()), + strip_off_newlines(b"hello world\r\r\r\r\r".to_vec()), expected ); + assert_eq!(strip_off_newlines(b"hello world\r\n".to_vec()), expected); assert_eq!( - strip_off_newlines("hello world\r".as_bytes().to_vec()), - expected - ); - assert_eq!( - strip_off_newlines("hello world\r\r\r\r\r".as_bytes().to_vec()), - expected - ); - assert_eq!( - strip_off_newlines("hello world\r\n".as_bytes().to_vec()), - expected - ); - assert_eq!( - strip_off_newlines("hello world\r\n\r\n".as_bytes().to_vec()), - expected - ); - assert_eq!( - strip_off_newlines("hello world".as_bytes().to_vec()), + strip_off_newlines(b"hello world\r\n\r\n".to_vec()), expected ); + assert_eq!(strip_off_newlines(b"hello world".to_vec()), expected); } #[test] diff --git a/common/eth2_network_config/src/lib.rs b/common/eth2_network_config/src/lib.rs index f8ac56be29..07bf9a8f4d 100644 --- a/common/eth2_network_config/src/lib.rs +++ b/common/eth2_network_config/src/lib.rs @@ -247,8 +247,8 @@ mod tests { #[test] fn hard_coded_nets_work() { for net in HARDCODED_NETS { - let config = - Eth2NetworkConfig::from_hardcoded_net(net).expect(&format!("{:?}", net.name)); + let config = Eth2NetworkConfig::from_hardcoded_net(net) + .unwrap_or_else(|_| panic!("{:?}", net.name)); if net.name == "mainnet" || net.name == "toledo" || net.name == "pyrmont" { // Ensure we can parse the YAML config to a chain spec. diff --git a/common/lockfile/src/lib.rs b/common/lockfile/src/lib.rs index 54c7c54ca1..28e5b90a0e 100644 --- a/common/lockfile/src/lib.rs +++ b/common/lockfile/src/lib.rs @@ -110,7 +110,7 @@ mod test { let _lockfile = File::create(&path).unwrap(); - let lock = Lockfile::new(path.clone()).unwrap(); + let lock = Lockfile::new(path).unwrap(); assert!(lock.file_existed()); } diff --git a/consensus/int_to_bytes/src/lib.rs b/consensus/int_to_bytes/src/lib.rs index 8d8b236cae..589c72d249 100644 --- a/consensus/int_to_bytes/src/lib.rs +++ b/consensus/int_to_bytes/src/lib.rs @@ -78,7 +78,6 @@ pub fn int_to_bytes96(int: u64) -> Vec { #[cfg(test)] mod tests { use super::*; - use hex; use std::{fs::File, io::prelude::*, path::PathBuf}; use yaml_rust::yaml; diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 252c6db89c..c3bffb9c2d 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -386,7 +386,7 @@ mod test_compute_deltas { state_root, target_root: finalized_root, current_epoch_shuffling_id: junk_shuffling_id.clone(), - next_epoch_shuffling_id: junk_shuffling_id.clone(), + next_epoch_shuffling_id: junk_shuffling_id, justified_epoch: genesis_epoch, finalized_epoch: genesis_epoch, }) diff --git a/consensus/ssz/tests/tests.rs b/consensus/ssz/tests/tests.rs index 049ee30c83..16712fcb07 100644 --- a/consensus/ssz/tests/tests.rs +++ b/consensus/ssz/tests/tests.rs @@ -83,6 +83,7 @@ mod round_trip { } #[test] + #[allow(clippy::zero_prefixed_literal)] fn fixed_len_struct_encoding() { let items: Vec = vec![ FixedLen { a: 0, b: 0, c: 0 }, @@ -142,6 +143,7 @@ mod round_trip { } #[test] + #[allow(clippy::zero_prefixed_literal)] fn offset_into_fixed_bytes() { let bytes = vec![ // 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 @@ -172,6 +174,7 @@ mod round_trip { } #[test] + #[allow(clippy::zero_prefixed_literal)] fn first_offset_skips_byte() { let bytes = vec![ // 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 @@ -186,6 +189,7 @@ mod round_trip { } #[test] + #[allow(clippy::zero_prefixed_literal)] fn variable_len_struct_encoding() { let items: Vec = vec![ VariableLen { @@ -274,6 +278,7 @@ mod round_trip { } #[test] + #[allow(clippy::zero_prefixed_literal)] fn offsets_decreasing() { let bytes = vec![ // 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 @@ -296,6 +301,7 @@ mod round_trip { } #[test] + #[allow(clippy::zero_prefixed_literal)] fn two_variable_len_options_encoding() { let s = TwoVariableLenOptions { a: 42, diff --git a/consensus/ssz_types/src/bitfield.rs b/consensus/ssz_types/src/bitfield.rs index df9523c877..89f1272a3d 100644 --- a/consensus/ssz_types/src/bitfield.rs +++ b/consensus/ssz_types/src/bitfield.rs @@ -777,17 +777,17 @@ mod bitlist { fn ssz_encode() { assert_eq!( BitList0::with_capacity(0).unwrap().as_ssz_bytes(), - vec![0b0000_00001], + vec![0b0000_0001], ); assert_eq!( BitList1::with_capacity(0).unwrap().as_ssz_bytes(), - vec![0b0000_00001], + vec![0b0000_0001], ); assert_eq!( BitList1::with_capacity(1).unwrap().as_ssz_bytes(), - vec![0b0000_00010], + vec![0b0000_0010], ); assert_eq!( diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index a0dcd66c48..471ca011dd 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -387,7 +387,7 @@ fn invalid_attestation_wrong_justified_checkpoint() { index: 0, reason: AttestationInvalid::WrongJustifiedCheckpoint { state: Checkpoint { - epoch: Epoch::from(2 as u64), + epoch: Epoch::from(2_u64), root: Hash256::zero(), }, attestation: Checkpoint { @@ -878,7 +878,7 @@ fn invalid_proposer_slashing_proposal_epoch_mismatch() { index: 0, reason: ProposerSlashingInvalid::ProposalSlotMismatch( Slot::from(0_u64), - Slot::from(128 as u64) + Slot::from(128_u64) ) }) ); diff --git a/consensus/swap_or_not_shuffle/src/shuffle_list.rs b/consensus/swap_or_not_shuffle/src/shuffle_list.rs index 652f03c5b3..41e91beb23 100644 --- a/consensus/swap_or_not_shuffle/src/shuffle_list.rs +++ b/consensus/swap_or_not_shuffle/src/shuffle_list.rs @@ -181,6 +181,7 @@ mod tests { } #[test] + #[allow(clippy::assertions_on_constants)] fn sanity_check_constants() { assert!(TOTAL_SIZE > SEED_SIZE); assert!(TOTAL_SIZE > PIVOT_VIEW_SIZE); diff --git a/consensus/tree_hash/src/merkle_hasher.rs b/consensus/tree_hash/src/merkle_hasher.rs index 02c349eb8e..cebf0b33b5 100644 --- a/consensus/tree_hash/src/merkle_hasher.rs +++ b/consensus/tree_hash/src/merkle_hasher.rs @@ -397,7 +397,7 @@ mod test { let merklizer_root_individual_3_bytes = { let mut m = MerkleHasher::with_depth(depth); - for bytes in reference_bytes.clone().chunks(3) { + for bytes in reference_bytes.chunks(3) { m.write(bytes).expect("should process byte"); } m.finish().expect("should finish") @@ -426,7 +426,7 @@ mod test { /// of leaves and a depth. fn compare_reference_with_len(leaves: u64, depth: usize) { let leaves = (0..leaves) - .map(|i| Hash256::from_low_u64_be(i)) + .map(Hash256::from_low_u64_be) .collect::>(); compare_with_reference(&leaves, depth) } @@ -435,7 +435,7 @@ mod test { /// results. fn compare_new_with_leaf_count(num_leaves: u64, depth: usize) { let leaves = (0..num_leaves) - .map(|i| Hash256::from_low_u64_be(i)) + .map(Hash256::from_low_u64_be) .collect::>(); let from_depth = { diff --git a/consensus/types/src/beacon_state/tests.rs b/consensus/types/src/beacon_state/tests.rs index 9c2df38d4d..c14bd94fcd 100644 --- a/consensus/types/src/beacon_state/tests.rs +++ b/consensus/types/src/beacon_state/tests.rs @@ -1,6 +1,7 @@ #![cfg(test)] use super::*; use crate::test_utils::*; +use std::ops::Mul; ssz_and_tree_hash_tests!(FoundationBeaconState); @@ -49,13 +50,13 @@ fn test_beacon_proposer_index() { // Test where we have two validators per slot. // 0th candidate should be chosen every time. - let state = build_state(T::slots_per_epoch() as usize * 2); + let state = build_state((T::slots_per_epoch() as usize).mul(2)); for i in 0..T::slots_per_epoch() { test(&state, Slot::from(i), 0); } // Test with two validators per slot, first validator has zero balance. - let mut state = build_state(T::slots_per_epoch() as usize * 2); + let mut state = build_state((T::slots_per_epoch() as usize).mul(2)); let slot0_candidate0 = ith_candidate(&state, Slot::new(0), 0, &spec); state.validators[slot0_candidate0].effective_balance = 0; test(&state, Slot::new(0), 1); @@ -74,8 +75,8 @@ fn beacon_proposer_index() { /// 1. Using the cache before it's built fails. /// 2. Using the cache after it's build passes. /// 3. Using the cache after it's dropped fails. -fn test_cache_initialization<'a, T: EthSpec>( - state: &'a mut BeaconState, +fn test_cache_initialization( + state: &mut BeaconState, relative_epoch: RelativeEpoch, spec: &ChainSpec, ) { @@ -126,7 +127,7 @@ fn cache_initialization() { } fn test_clone_config(base_state: &BeaconState, clone_config: CloneConfig) { - let state = base_state.clone_with(clone_config.clone()); + let state = base_state.clone_with(clone_config); if clone_config.committee_caches { state .committee_cache(RelativeEpoch::Previous) @@ -260,6 +261,7 @@ fn tree_hash_cache() { mod committees { use super::*; use crate::beacon_state::MinimalEthSpec; + use std::ops::{Add, Div}; use swap_or_not_shuffle::shuffle_list; fn execute_committee_consistency_test( @@ -295,7 +297,10 @@ mod committees { // of committees in an epoch. assert_eq!( beacon_committees.len() as u64, - state.get_epoch_committee_count(relative_epoch).unwrap() / T::slots_per_epoch() + state + .get_epoch_committee_count(relative_epoch) + .unwrap() + .div(T::slots_per_epoch()) ); for (committee_index, bc) in beacon_committees.iter().enumerate() { @@ -372,10 +377,11 @@ mod committees { fn committee_consistency_test_suite(cached_epoch: RelativeEpoch) { let spec = T::default_spec(); - let validator_count = spec.max_committees_per_slot - * T::slots_per_epoch() as usize - * spec.target_committee_size - + 1; + let validator_count = spec + .max_committees_per_slot + .mul(T::slots_per_epoch() as usize) + .mul(spec.target_committee_size) + .add(1); committee_consistency_test::(validator_count as usize, Epoch::new(0), cached_epoch); @@ -387,7 +393,10 @@ mod committees { committee_consistency_test::( validator_count as usize, - T::genesis_epoch() + T::slots_per_historical_root() as u64 * T::slots_per_epoch() * 4, + T::genesis_epoch() + + (T::slots_per_historical_root() as u64) + .mul(T::slots_per_epoch()) + .mul(4), cached_epoch, ); } diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 9621c347da..9f7dd43f07 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -405,6 +405,7 @@ mod tests { let _ = ChainSpec::mainnet(); } + #[allow(clippy::useless_vec)] fn test_domain(domain_type: Domain, raw_domain: u32, spec: &ChainSpec) { let previous_version = [0, 0, 0, 1]; let current_version = [0, 0, 0, 2]; diff --git a/consensus/types/src/tree_hash_impls.rs b/consensus/types/src/tree_hash_impls.rs index 749a33b5da..df0ba2ed2a 100644 --- a/consensus/types/src/tree_hash_impls.rs +++ b/consensus/types/src/tree_hash_impls.rs @@ -137,9 +137,11 @@ mod test { #[test] fn zeroed_validator() { - let mut v = Validator::default(); - v.activation_eligibility_epoch = Epoch::from(0u64); - v.activation_epoch = Epoch::from(0u64); + let v = Validator { + activation_eligibility_epoch: Epoch::from(0u64), + activation_epoch: Epoch::from(0u64), + ..Default::default() + }; test_validator_tree_hash(&v); } @@ -153,6 +155,7 @@ mod test { } #[test] + #[allow(clippy::assertions_on_constants)] pub fn smallvec_size_check() { // If this test fails we need to go and reassess the length of the `SmallVec` in // `cached_tree_hash::TreeHashCache`. If the size of the `SmallVec` is too slow we're going diff --git a/remote_signer/backend/src/lib.rs b/remote_signer/backend/src/lib.rs index 951d5a6ff7..9631f00b1d 100644 --- a/remote_signer/backend/src/lib.rs +++ b/remote_signer/backend/src/lib.rs @@ -135,7 +135,7 @@ pub mod tests_commons { pub fn assert_backend_new_error(matches: &ArgMatches, error_msg: &str) { match Backend::new(matches, &get_null_logger()) { Ok(_) => panic!("This invocation to Backend::new() should return error"), - Err(e) => assert_eq!(e.to_string(), error_msg), + Err(e) => assert_eq!(e, error_msg), } } } @@ -188,7 +188,7 @@ pub mod backend_new { match result { Ok(_) => panic!("This invocation to Backend::new() should return error"), - Err(e) => assert_eq!(e.to_string(), "Storage Raw Dir: PermissionDenied",), + Err(e) => assert_eq!(e, "Storage Raw Dir: PermissionDenied",), } } diff --git a/slasher/tests/attester_slashings.rs b/slasher/tests/attester_slashings.rs index d4a4223fcf..3b541e7a8f 100644 --- a/slasher/tests/attester_slashings.rs +++ b/slasher/tests/attester_slashings.rs @@ -107,7 +107,7 @@ fn surrounds_existing_single_val_single_chunk() { fn surrounds_existing_multi_vals_single_chunk() { let validators = vec![0, 16, 1024, 300_000, 300_001]; let att1 = indexed_att(validators.clone(), 1, 2, 0); - let att2 = indexed_att(validators.clone(), 0, 3, 0); + let att2 = indexed_att(validators, 0, 3, 0); let slashings = hashset![att_slashing(&att2, &att1)]; slasher_test_indiv(&[att1, att2], &slashings, 3); } diff --git a/slasher/tests/proposer_slashings.rs b/slasher/tests/proposer_slashings.rs index 4174e3b636..f584bacfba 100644 --- a/slasher/tests/proposer_slashings.rs +++ b/slasher/tests/proposer_slashings.rs @@ -9,7 +9,7 @@ use types::{Epoch, EthSpec}; fn empty_pruning() { let tempdir = tempdir().unwrap(); let config = Config::new(tempdir.path().into()); - let slasher = Slasher::::open(config.clone(), logger()).unwrap(); + let slasher = Slasher::::open(config, logger()).unwrap(); slasher.prune_database(Epoch::new(0)).unwrap(); } diff --git a/slasher/tests/random.rs b/slasher/tests/random.rs index ab82af0bda..47fce46f4f 100644 --- a/slasher/tests/random.rs +++ b/slasher/tests/random.rs @@ -62,7 +62,7 @@ fn random_test(seed: u64, test_config: TestConfig) { .choose_multiple(&mut rng, num_attesters) .copied() .collect::>(); - attesting_indices.sort(); + attesting_indices.sort_unstable(); // If checking slashings, generate valid attestations in range. let (source, target) = if check_slashings { diff --git a/slasher/tests/wrap_around.rs b/slasher/tests/wrap_around.rs index 6029fb6c1d..1480704e6b 100644 --- a/slasher/tests/wrap_around.rs +++ b/slasher/tests/wrap_around.rs @@ -30,7 +30,7 @@ fn attestation_pruning_empty_wrap_around() { // Add an attestation that would be surrounded with the modulo considered slasher.accept_attestation(indexed_att( - v.clone(), + v, 2 * history_length - 3, 2 * history_length - 2, 1, @@ -48,7 +48,7 @@ fn pruning_with_map_full() { config.history_length = 1024; config.max_db_size_mbs = 1; - let slasher = Slasher::open(config.clone(), logger()).unwrap(); + let slasher = Slasher::open(config, logger()).unwrap(); let v = vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]; diff --git a/validator_client/slashing_protection/src/attestation_tests.rs b/validator_client/slashing_protection/src/attestation_tests.rs index ded026a3da..c66a67b70a 100644 --- a/validator_client/slashing_protection/src/attestation_tests.rs +++ b/validator_client/slashing_protection/src/attestation_tests.rs @@ -306,7 +306,7 @@ fn invalid_surrounding_from_first_source() { let second = attestation_data_builder(3, 4); StreamTest { cases: vec![ - Test::single(first.clone()), + Test::single(first), Test::single(second.clone()), Test::single(attestation_data_builder(2, 5)).expect_invalid_att( InvalidAttestation::NewSurroundsPrev { @@ -326,8 +326,8 @@ fn invalid_surrounding_multiple_votes() { let third = attestation_data_builder(2, 3); StreamTest { cases: vec![ - Test::single(first.clone()), - Test::single(second.clone()), + Test::single(first), + Test::single(second), Test::single(third.clone()), Test::single(attestation_data_builder(0, 4)).expect_invalid_att( InvalidAttestation::NewSurroundsPrev { diff --git a/validator_client/slashing_protection/src/block_tests.rs b/validator_client/slashing_protection/src/block_tests.rs index 7fe9a21b71..a1f634ef07 100644 --- a/validator_client/slashing_protection/src/block_tests.rs +++ b/validator_client/slashing_protection/src/block_tests.rs @@ -69,7 +69,7 @@ fn valid_same_block_different_validator() { registered_validators: vec![pubkey(0), pubkey(1)], cases: vec![ Test::with_pubkey(pubkey(0), block.clone()), - Test::with_pubkey(pubkey(1), block.clone()), + Test::with_pubkey(pubkey(1), block), ], } .run() diff --git a/validator_client/slashing_protection/src/lib.rs b/validator_client/slashing_protection/src/lib.rs index b209a515d3..774cbc9340 100644 --- a/validator_client/slashing_protection/src/lib.rs +++ b/validator_client/slashing_protection/src/lib.rs @@ -119,6 +119,7 @@ mod test { use super::*; #[test] + #[allow(clippy::eq_op)] fn signing_root_partial_eq() { let h0 = SigningRoot(Hash256::zero()); let h1 = SigningRoot(Hash256::repeat_byte(1)); diff --git a/validator_client/src/fork_service.rs b/validator_client/src/fork_service.rs index 82c827b08a..0787fa80f8 100644 --- a/validator_client/src/fork_service.rs +++ b/validator_client/src/fork_service.rs @@ -88,7 +88,7 @@ impl ForkServiceBuilder { eth2::Url::parse("http://127.0.0.1").unwrap(), ))]; let mut beacon_nodes = BeaconNodeFallback::new(candidates, spec, log.clone()); - beacon_nodes.set_slot_clock(slot_clock.clone()); + beacon_nodes.set_slot_clock(slot_clock); Self { fork: Some(types::Fork::default()),