diff --git a/account_manager/src/lib.rs b/account_manager/src/lib.rs index b0f3525b1..719d5728a 100644 --- a/account_manager/src/lib.rs +++ b/account_manager/src/lib.rs @@ -156,7 +156,7 @@ fn run_new_validator_subcommand( }) .map(|password| { // Trim the line feed from the end of the password file, if present. - if password.ends_with("\n") { + if password.ends_with('\n') { password[0..password.len() - 1].to_string() } else { password @@ -337,7 +337,7 @@ fn deposit_validators( .map(|_| event_loop) }) // Web3 gives errors if the event loop is dropped whilst performing requests. - .map(|event_loop| drop(event_loop)) + .map(drop) } /// For the given `ValidatorDirectory`, submit a deposit transaction to the `web3` node. @@ -367,7 +367,7 @@ fn deposit_validator( .into_future() .and_then(move |(voting_keypair, deposit_data)| { let pubkey_1 = voting_keypair.pk.clone(); - let pubkey_2 = voting_keypair.pk.clone(); + let pubkey_2 = voting_keypair.pk; let web3_1 = web3.clone(); let web3_2 = web3.clone(); @@ -421,7 +421,7 @@ fn deposit_validator( to: Some(deposit_contract), gas: Some(U256::from(DEPOSIT_GAS)), gas_price: None, - value: Some(U256::from(from_gwei(deposit_amount))), + value: Some(from_gwei(deposit_amount)), data: Some(deposit_data.into()), nonce: None, condition: None, diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 4edf1a832..4a2708830 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -25,6 +25,7 @@ use state_processing::{ per_block_processing, per_slot_processing, BlockProcessingError, BlockSignatureStrategy, }; use std::borrow::Cow; +use std::cmp::Ordering; use std::fs; use std::io::prelude::*; use std::sync::Arc; @@ -512,65 +513,67 @@ impl BeaconChain { pub fn state_at_slot(&self, slot: Slot) -> Result, Error> { let head_state = self.head()?.beacon_state; - if slot == head_state.slot { - Ok(head_state) - } else if slot > head_state.slot { - if slot > head_state.slot + T::EthSpec::slots_per_epoch() { - warn!( - self.log, - "Skipping more than an epoch"; - "head_slot" => head_state.slot, - "request_slot" => slot - ) - } - - let start_slot = head_state.slot; - let task_start = Instant::now(); - let max_task_runtime = Duration::from_millis(self.spec.milliseconds_per_slot); - - let head_state_slot = head_state.slot; - let mut state = head_state; - while state.slot < slot { - // Do not allow and forward state skip that takes longer than the maximum task duration. - // - // This is a protection against nodes doing too much work when they're not synced - // to a chain. - if task_start + max_task_runtime < Instant::now() { - return Err(Error::StateSkipTooLarge { - start_slot, - requested_slot: slot, - max_task_runtime, - }); + match slot.cmp(&head_state.slot) { + Ordering::Equal => Ok(head_state), + Ordering::Greater => { + if slot > head_state.slot + T::EthSpec::slots_per_epoch() { + warn!( + self.log, + "Skipping more than an epoch"; + "head_slot" => head_state.slot, + "request_slot" => slot + ) } - // Note: supplying some `state_root` when it is known would be a cheap and easy - // optimization. - match per_slot_processing(&mut state, None, &self.spec) { - Ok(()) => (), - Err(e) => { - warn!( - self.log, - "Unable to load state at slot"; - "error" => format!("{:?}", e), - "head_slot" => head_state_slot, - "requested_slot" => slot - ); - return Err(Error::NoStateForSlot(slot)); - } - }; - } - Ok(state) - } else { - let state_root = self - .rev_iter_state_roots()? - .take_while(|(_root, current_slot)| *current_slot >= slot) - .find(|(_root, current_slot)| *current_slot == slot) - .map(|(root, _slot)| root) - .ok_or_else(|| Error::NoStateForSlot(slot))?; + let start_slot = head_state.slot; + let task_start = Instant::now(); + let max_task_runtime = Duration::from_millis(self.spec.milliseconds_per_slot); - Ok(self - .get_state_caching(&state_root, Some(slot))? - .ok_or_else(|| Error::NoStateForSlot(slot))?) + let head_state_slot = head_state.slot; + let mut state = head_state; + while state.slot < slot { + // Do not allow and forward state skip that takes longer than the maximum task duration. + // + // This is a protection against nodes doing too much work when they're not synced + // to a chain. + if task_start + max_task_runtime < Instant::now() { + return Err(Error::StateSkipTooLarge { + start_slot, + requested_slot: slot, + max_task_runtime, + }); + } + + // Note: supplying some `state_root` when it is known would be a cheap and easy + // optimization. + match per_slot_processing(&mut state, None, &self.spec) { + Ok(()) => (), + Err(e) => { + warn!( + self.log, + "Unable to load state at slot"; + "error" => format!("{:?}", e), + "head_slot" => head_state_slot, + "requested_slot" => slot + ); + return Err(Error::NoStateForSlot(slot)); + } + }; + } + Ok(state) + } + Ordering::Less => { + let state_root = self + .rev_iter_state_roots()? + .take_while(|(_root, current_slot)| *current_slot >= slot) + .find(|(_root, current_slot)| *current_slot == slot) + .map(|(root, _slot)| root) + .ok_or_else(|| Error::NoStateForSlot(slot))?; + + Ok(self + .get_state_caching(&state_root, Some(slot))? + .ok_or_else(|| Error::NoStateForSlot(slot))?) + } } } @@ -638,7 +641,7 @@ impl BeaconChain { let head_state = &self.head()?.beacon_state; let mut state = if epoch(slot) == epoch(head_state.slot) { - self.head()?.beacon_state.clone() + self.head()?.beacon_state } else { self.state_at_slot(slot)? }; @@ -671,7 +674,7 @@ impl BeaconChain { let head_state = &self.head()?.beacon_state; let mut state = if epoch == as_epoch(head_state.slot) { - self.head()?.beacon_state.clone() + self.head()?.beacon_state } else { self.state_at_slot(epoch.start_slot(T::EthSpec::slots_per_epoch()))? }; @@ -1754,9 +1757,9 @@ impl BeaconChain { let mut dump = vec![]; let mut last_slot = CheckPoint { - beacon_block: self.head()?.beacon_block.clone(), + beacon_block: self.head()?.beacon_block, beacon_block_root: self.head()?.beacon_block_root, - beacon_state: self.head()?.beacon_state.clone(), + beacon_state: self.head()?.beacon_state, beacon_state_root: self.head()?.beacon_state_root, }; diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 5738d494e..fc951635f 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -448,7 +448,7 @@ where let fork_choice = if let Some(persisted_beacon_chain) = &self.persisted_beacon_chain { ForkChoice::from_ssz_container( persisted_beacon_chain.fork_choice.clone(), - store.clone(), + store, block_root_tree, ) .map_err(|e| format!("Unable to decode fork choice from db: {:?}", e))? @@ -462,7 +462,7 @@ where .ok_or_else(|| "fork_choice_backend requires a genesis_block_root")?; let backend = ThreadSafeReducedTree::new( - store.clone(), + store, block_root_tree, &finalized_checkpoint.beacon_block, finalized_checkpoint.beacon_block_root, @@ -626,7 +626,7 @@ mod test { #[test] fn recent_genesis() { let validator_count = 8; - let genesis_time = 13371337; + let genesis_time = 13_371_337; let log = get_logger(); let store = Arc::new(MemoryStore::open()); @@ -641,7 +641,7 @@ mod test { let chain = BeaconChainBuilder::new(MinimalEthSpec) .logger(log.clone()) - .store(store.clone()) + .store(store) .store_migrator(NullMigrator) .genesis_state(genesis_state) .expect("should build state using recent genesis") @@ -662,7 +662,7 @@ mod test { assert_eq!(state.slot, Slot::new(0), "should start from genesis"); assert_eq!( - state.genesis_time, 13371337, + state.genesis_time, 13_371_337, "should have the correct genesis time" ); assert_eq!( diff --git a/beacon_node/beacon_chain/src/eth1_chain.rs b/beacon_node/beacon_chain/src/eth1_chain.rs index 9b7f77204..e52192397 100644 --- a/beacon_node/beacon_chain/src/eth1_chain.rs +++ b/beacon_node/beacon_chain/src/eth1_chain.rs @@ -8,6 +8,7 @@ use rand::prelude::*; use slog::{crit, debug, error, trace, Logger}; use ssz_derive::{Decode, Encode}; use state_processing::per_block_processing::get_new_eth1_data; +use std::cmp::Ordering; use std::collections::HashMap; use std::iter::DoubleEndedIterator; use std::iter::FromIterator; @@ -343,8 +344,7 @@ impl> Eth1ChainBackend for CachingEth1Backend voting_period_start_seconds) - .skip(eth1_follow_distance as usize) - .next() + .nth(eth1_follow_distance as usize) .map(|block| { trace!( self.log, @@ -392,21 +392,21 @@ impl> Eth1ChainBackend for CachingEth1Backend deposit_count { - Err(Error::DepositIndexTooHigh) - } else if deposit_index == deposit_count { - Ok(vec![]) - } else { - let next = deposit_index; - let last = std::cmp::min(deposit_count, next + T::MaxDeposits::to_u64()); + match deposit_index.cmp(&deposit_count) { + Ordering::Greater => Err(Error::DepositIndexTooHigh), + Ordering::Equal => Ok(vec![]), + Ordering::Less => { + let next = deposit_index; + let last = std::cmp::min(deposit_count, next + T::MaxDeposits::to_u64()); - self.core - .deposits() - .read() - .cache - .get_deposits(next, last, deposit_count, DEPOSIT_TREE_DEPTH) - .map_err(|e| Error::BackendError(format!("Failed to get deposits: {:?}", e))) - .map(|(_deposit_root, deposits)| deposits) + self.core + .deposits() + .read() + .cache + .get_deposits(next, last, deposit_count, DEPOSIT_TREE_DEPTH) + .map_err(|e| Error::BackendError(format!("Failed to get deposits: {:?}", e))) + .map(|(_deposit_root, deposits)| deposits) + } } } @@ -775,7 +775,7 @@ mod test { let deposits_for_inclusion = eth1_chain .deposits_for_block_inclusion(&state, &random_eth1_data(), spec) - .expect(&format!("should find deposit for {}", i)); + .unwrap_or_else(|_| panic!("should find deposit for {}", i)); let expected_len = std::cmp::min(i - initial_deposit_index, max_deposits as usize); @@ -853,7 +853,7 @@ mod test { state.slot = Slot::new(period * 1_000 + period / 2); // Add 50% of the votes so a lookup is required. - for _ in 0..period / 2 + 1 { + for _ in 0..=period / 2 { state .eth1_data_votes .push(random_eth1_data()) @@ -932,7 +932,7 @@ mod test { state.slot = Slot::new(period / 2); // Add 50% of the votes so a lookup is required. - for _ in 0..period / 2 + 1 { + for _ in 0..=period / 2 { state .eth1_data_votes .push(random_eth1_data()) @@ -1090,7 +1090,7 @@ mod test { eth1_block.number, *new_eth1_data .get(ð1_block.clone().eth1_data().unwrap()) - .expect(&format!( + .unwrap_or_else(|| panic!( "new_eth1_data should have expected block #{}", eth1_block.number )) @@ -1135,8 +1135,8 @@ mod test { let votes = collect_valid_votes( &state, - HashMap::from_iter(new_eth1_data.clone().into_iter()), - HashMap::from_iter(all_eth1_data.clone().into_iter()), + HashMap::from_iter(new_eth1_data.into_iter()), + HashMap::from_iter(all_eth1_data.into_iter()), ); assert_eq!( votes.len(), @@ -1164,7 +1164,7 @@ mod test { let votes = collect_valid_votes( &state, HashMap::from_iter(new_eth1_data.clone().into_iter()), - HashMap::from_iter(all_eth1_data.clone().into_iter()), + HashMap::from_iter(all_eth1_data.into_iter()), ); assert_votes!( votes, @@ -1196,8 +1196,8 @@ mod test { let votes = collect_valid_votes( &state, - HashMap::from_iter(new_eth1_data.clone().into_iter()), - HashMap::from_iter(all_eth1_data.clone().into_iter()), + HashMap::from_iter(new_eth1_data.into_iter()), + HashMap::from_iter(all_eth1_data.into_iter()), ); assert_votes!( votes, @@ -1230,12 +1230,12 @@ mod test { .expect("should have some eth1 data") .clone(); - state.eth1_data_votes = vec![non_new_eth1_data.0.clone()].into(); + state.eth1_data_votes = vec![non_new_eth1_data.0].into(); let votes = collect_valid_votes( &state, - HashMap::from_iter(new_eth1_data.clone().into_iter()), - HashMap::from_iter(all_eth1_data.clone().into_iter()), + HashMap::from_iter(new_eth1_data.into_iter()), + HashMap::from_iter(all_eth1_data.into_iter()), ); assert_votes!( @@ -1268,8 +1268,8 @@ mod test { let votes = collect_valid_votes( &state, - HashMap::from_iter(new_eth1_data.clone().into_iter()), - HashMap::from_iter(all_eth1_data.clone().into_iter()), + HashMap::from_iter(new_eth1_data.into_iter()), + HashMap::from_iter(all_eth1_data.into_iter()), ); assert_votes!( diff --git a/beacon_node/beacon_chain/src/fork_choice.rs b/beacon_node/beacon_chain/src/fork_choice.rs index b8d0e4a46..3a727a1f6 100644 --- a/beacon_node/beacon_chain/src/fork_choice.rs +++ b/beacon_node/beacon_chain/src/fork_choice.rs @@ -294,7 +294,7 @@ impl ForkChoice { /// Returns a `SszForkChoice` which contains the current state of `Self`. pub fn as_ssz_container(&self) -> SszForkChoice { SszForkChoice { - genesis_block_root: self.genesis_block_root.clone(), + genesis_block_root: self.genesis_block_root, justified_checkpoint: self.justified_checkpoint.read().clone(), best_justified_checkpoint: self.best_justified_checkpoint.read().clone(), backend_bytes: self.backend.as_bytes(), diff --git a/beacon_node/beacon_chain/src/head_tracker.rs b/beacon_node/beacon_chain/src/head_tracker.rs index 850f9629c..7bae0ce62 100644 --- a/beacon_node/beacon_chain/src/head_tracker.rs +++ b/beacon_node/beacon_chain/src/head_tracker.rs @@ -59,10 +59,10 @@ impl HeadTracker { let slots_len = ssz_container.slots.len(); if roots_len != slots_len { - return Err(Error::MismatchingLengths { + Err(Error::MismatchingLengths { roots_len, slots_len, - }); + }) } else { let map = HashMap::from_iter( ssz_container diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index fbf437fd1..337878d4c 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -173,7 +173,7 @@ impl BeaconChainHarness> { let chain = BeaconChainBuilder::new(eth_spec_instance) .logger(log.clone()) - .custom_spec(spec.clone()) + .custom_spec(spec) .store(store.clone()) .store_migrator( as Migrate<_, E>>::new(store)) .resume_from_db(Eth1Config::default()) @@ -236,7 +236,6 @@ where self.chain .state_at_slot(state_slot) .expect("should find state for slot") - .clone() }; // Determine the first slot where a block should be built. diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index d35fbeb46..611763e9b 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -151,7 +151,7 @@ where let builder = BeaconChainBuilder::new(eth_spec_instance) .logger(context.log.clone()) - .store(store.clone()) + .store(store) .store_migrator(store_migrator) .custom_spec(spec.clone()); @@ -223,7 +223,7 @@ where Box::new(future) } ClientGenesis::RemoteNode { server, .. } => { - let future = Bootstrapper::connect(server.to_string(), &context.log) + let future = Bootstrapper::connect(server, &context.log) .map_err(|e| { format!("Failed to initialize bootstrap client: {}", e) }) @@ -306,14 +306,14 @@ where .ok_or_else(|| "http_server requires a libp2p network sender")?; let network_info = rest_api::NetworkInfo { - network_service: network.clone(), - network_chan: network_send.clone(), + network_service: network, + network_chan: network_send, }; let (exit_signal, listening_addr) = rest_api::start_server( &client_config.rest_api, &context.executor, - beacon_chain.clone(), + beacon_chain, network_info, client_config .create_db_path() @@ -529,7 +529,7 @@ where spec, context.log, ) - .map_err(|e| format!("Unable to open database: {:?}", e).to_string())?; + .map_err(|e| format!("Unable to open database: {:?}", e))?; self.store = Some(Arc::new(store)); Ok(self) } @@ -557,8 +557,8 @@ where { /// Specifies that the `Client` should use a `DiskStore` database. pub fn simple_disk_store(mut self, path: &Path) -> Result { - let store = SimpleDiskStore::open(path) - .map_err(|e| format!("Unable to open database: {:?}", e).to_string())?; + let store = + SimpleDiskStore::open(path).map_err(|e| format!("Unable to open database: {:?}", e))?; self.store = Some(Arc::new(store)); Ok(self) } @@ -660,7 +660,7 @@ where .ok_or_else(|| "caching_eth1_backend requires a store".to_string())?; let backend = if let Some(eth1_service_from_genesis) = self.eth1_service { - eth1_service_from_genesis.update_config(config.clone())?; + eth1_service_from_genesis.update_config(config)?; // This cache is not useful because it's first (earliest) block likely the block that // triggered genesis. diff --git a/beacon_node/client/src/notifier.rs b/beacon_node/client/src/notifier.rs index 378582100..faaf8464c 100644 --- a/beacon_node/client/src/notifier.rs +++ b/beacon_node/client/src/notifier.rs @@ -17,7 +17,7 @@ pub const WARN_PEER_COUNT: usize = 1; const SECS_PER_MINUTE: f64 = 60.0; const SECS_PER_HOUR: f64 = 3600.0; const SECS_PER_DAY: f64 = 86400.0; // non-leap -const SECS_PER_WEEK: f64 = 604800.0; // non-leap +const SECS_PER_WEEK: f64 = 604_800.0; // non-leap const DAYS_PER_WEEK: f64 = 7.0; const HOURS_PER_DAY: f64 = 24.0; const MINUTES_PER_HOUR: f64 = 60.0; @@ -166,13 +166,14 @@ pub fn spawn_notifier( .then(move |result| { match result { Ok(()) => Ok(()), - Err(e) => Ok(error!( + Err(e) => { + error!( log_3, "Notifier failed to notify"; "error" => format!("{:?}", e) - )) - } - }); + ); + Ok(()) + } } }); let (exit_signal, exit) = exit_future::signal(); context diff --git a/beacon_node/eth1/src/block_cache.rs b/beacon_node/eth1/src/block_cache.rs index 088d8dcfd..4bcd23740 100644 --- a/beacon_node/eth1/src/block_cache.rs +++ b/beacon_node/eth1/src/block_cache.rs @@ -224,7 +224,7 @@ mod tests { ); } - let mut cache_2 = cache.clone(); + let mut cache_2 = cache; cache_2.truncate(17); assert_eq!( cache_2.blocks.len(), diff --git a/beacon_node/eth1/src/deposit_cache.rs b/beacon_node/eth1/src/deposit_cache.rs index b3fe67b7e..a0f9e9996 100644 --- a/beacon_node/eth1/src/deposit_cache.rs +++ b/beacon_node/eth1/src/deposit_cache.rs @@ -1,6 +1,7 @@ use crate::DepositLog; use eth2_hashing::hash; use ssz_derive::{Decode, Encode}; +use std::cmp::Ordering; use tree_hash::TreeHash; use types::{Deposit, Hash256, DEPOSIT_TREE_DEPTH}; @@ -196,24 +197,26 @@ impl DepositCache { /// - If a log with index `log.index - 1` is not already present in `self` (ignored when empty). /// - If a log with `log.index` is already known, but the given `log` is distinct to it. pub fn insert_log(&mut self, log: DepositLog) -> Result<(), Error> { - if log.index == self.logs.len() as u64 { - let deposit = Hash256::from_slice(&log.deposit_data.tree_hash_root()); - self.leaves.push(deposit); - self.logs.push(log); - self.deposit_tree.push_leaf(deposit)?; - self.deposit_roots.push(self.deposit_tree.root()); - Ok(()) - } else if log.index < self.logs.len() as u64 { - if self.logs[log.index as usize] == log { + match log.index.cmp(&(self.logs.len() as u64)) { + Ordering::Equal => { + let deposit = Hash256::from_slice(&log.deposit_data.tree_hash_root()); + self.leaves.push(deposit); + self.logs.push(log); + self.deposit_tree.push_leaf(deposit)?; + self.deposit_roots.push(self.deposit_tree.root()); Ok(()) - } else { - Err(Error::DuplicateDistinctLog(log.index)) } - } else { - Err(Error::NonConsecutive { + Ordering::Less => { + if self.logs[log.index as usize] == log { + Ok(()) + } else { + Err(Error::DuplicateDistinctLog(log.index)) + } + } + Ordering::Greater => Err(Error::NonConsecutive { log_index: log.index, expected: self.logs.len(), - }) + }), } } @@ -312,14 +315,12 @@ impl DepositCache { .logs .binary_search_by(|deposit| deposit.block_number.cmp(&block_number)); match index { - Ok(index) => return self.logs.get(index).map(|x| x.index + 1), - Err(next) => { - return Some( - self.logs - .get(next.saturating_sub(1)) - .map_or(0, |x| x.index + 1), - ) - } + Ok(index) => self.logs.get(index).map(|x| x.index + 1), + Err(next) => Some( + self.logs + .get(next.saturating_sub(1)) + .map_or(0, |x| x.index + 1), + ), } } @@ -329,7 +330,7 @@ impl DepositCache { /// and queries the `deposit_roots` map to get the corresponding `deposit_root`. pub fn get_deposit_root_from_cache(&self, block_number: u64) -> Option { let index = self.get_deposit_count_from_cache(block_number)?; - Some(self.deposit_roots.get(index as usize)?.clone()) + Some(*self.deposit_roots.get(index as usize)?) } } diff --git a/beacon_node/eth1/src/http.rs b/beacon_node/eth1/src/http.rs index 0b2e3a32b..130195245 100644 --- a/beacon_node/eth1/src/http.rs +++ b/beacon_node/eth1/src/http.rs @@ -136,7 +136,7 @@ pub fn get_deposit_count( timeout, ) .and_then(|result| match result { - None => Err(format!("Deposit root response was none")), + None => Err("Deposit root response was none".to_string()), Some(bytes) => { if bytes.is_empty() { Ok(None) @@ -173,7 +173,7 @@ pub fn get_deposit_root( timeout, ) .and_then(|result| match result { - None => Err(format!("Deposit root response was none")), + None => Err("Deposit root response was none".to_string()), Some(bytes) => { if bytes.is_empty() { Ok(None) diff --git a/beacon_node/eth1/src/service.rs b/beacon_node/eth1/src/service.rs index 383df1d4a..292f777fe 100644 --- a/beacon_node/eth1/src/service.rs +++ b/beacon_node/eth1/src/service.rs @@ -613,7 +613,7 @@ impl Service { Ok(BlockCacheUpdateOutcome::Success { blocks_imported, - head_block_number: cache_4.clone().block_cache.read().highest_block_number(), + head_block_number: cache_4.block_cache.read().highest_block_number(), }) }) } diff --git a/beacon_node/eth1/tests/test.rs b/beacon_node/eth1/tests/test.rs index d8597b984..32f793aad 100644 --- a/beacon_node/eth1/tests/test.rs +++ b/beacon_node/eth1/tests/test.rs @@ -758,7 +758,7 @@ mod fast { log, ); let n = 10; - let deposits: Vec<_> = (0..n).into_iter().map(|_| random_deposit_data()).collect(); + let deposits: Vec<_> = (0..n).map(|_| random_deposit_data()).collect(); for deposit in &deposits { deposit_contract .deposit(runtime, deposit.clone()) diff --git a/beacon_node/eth2-libp2p/src/behaviour.rs b/beacon_node/eth2-libp2p/src/behaviour.rs index a9e697473..608cccf7d 100644 --- a/beacon_node/eth2-libp2p/src/behaviour.rs +++ b/beacon_node/eth2-libp2p/src/behaviour.rs @@ -57,7 +57,7 @@ impl Behaviour { net_conf: &NetworkConfig, log: &slog::Logger, ) -> error::Result { - let local_peer_id = local_key.public().clone().into_peer_id(); + let local_peer_id = local_key.public().into_peer_id(); let behaviour_log = log.new(o!()); let ping_config = PingConfig::new() @@ -74,7 +74,7 @@ impl Behaviour { Ok(Behaviour { eth2_rpc: RPC::new(log.clone()), - gossipsub: Gossipsub::new(local_peer_id.clone(), net_conf.gs_config.clone()), + gossipsub: Gossipsub::new(local_peer_id, net_conf.gs_config.clone()), discovery: Discovery::new(local_key, net_conf, log)?, ping: Ping::new(ping_config), identify, diff --git a/beacon_node/eth2-libp2p/src/rpc/handler.rs b/beacon_node/eth2-libp2p/src/rpc/handler.rs index e7b225e16..4f2908c93 100644 --- a/beacon_node/eth2-libp2p/src/rpc/handler.rs +++ b/beacon_node/eth2-libp2p/src/rpc/handler.rs @@ -145,7 +145,7 @@ where // When terminating a stream, report the stream termination to the requesting user via // an RPC error let error = RPCErrorResponse::ServerError(ErrorMessage { - error_message: "Request timed out".as_bytes().to_vec(), + error_message: b"Request timed out".to_vec(), }); // The stream termination type is irrelevant, this will terminate the @@ -510,7 +510,7 @@ where // notify the user return Ok(Async::Ready(ProtocolsHandlerEvent::Custom( RPCEvent::Error( - stream_id.get_ref().clone(), + *stream_id.get_ref(), RPCError::Custom("Stream timed out".into()), ), ))); diff --git a/beacon_node/eth2-libp2p/tests/common/mod.rs b/beacon_node/eth2-libp2p/tests/common/mod.rs index a4fa7db59..d945383ae 100644 --- a/beacon_node/eth2-libp2p/tests/common/mod.rs +++ b/beacon_node/eth2-libp2p/tests/common/mod.rs @@ -43,8 +43,7 @@ pub fn build_libp2p_instance( ) -> LibP2PService { let config = build_config(port, boot_nodes, secret_key); // launch libp2p service - let libp2p_service = LibP2PService::new(config.clone(), log.clone()).unwrap(); - libp2p_service + LibP2PService::new(config, log.clone()).unwrap() } #[allow(dead_code)] @@ -64,10 +63,10 @@ pub fn build_full_mesh(log: slog::Logger, n: usize, start_port: Option) -> .map(|x| get_enr(&x).multiaddr()[1].clone()) .collect(); - for i in 0..n { - for j in i..n { + for (i, node) in nodes.iter_mut().enumerate().take(n) { + for (j, multiaddr) in multiaddrs.iter().enumerate().skip(i) { if i != j { - match libp2p::Swarm::dial_addr(&mut nodes[i].swarm, multiaddrs[j].clone()) { + match libp2p::Swarm::dial_addr(&mut node.swarm, multiaddr.clone()) { Ok(()) => debug!(log, "Connected"), Err(_) => error!(log, "Failed to connect"), }; diff --git a/beacon_node/eth2-libp2p/tests/gossipsub_tests.rs b/beacon_node/eth2-libp2p/tests/gossipsub_tests.rs index 349d754d5..37d38f986 100644 --- a/beacon_node/eth2-libp2p/tests/gossipsub_tests.rs +++ b/beacon_node/eth2-libp2p/tests/gossipsub_tests.rs @@ -62,7 +62,7 @@ fn test_gossipsub_forward() { // Every node except the corner nodes are connected to 2 nodes. if subscribed_count == (num_nodes * 2) - 2 { node.swarm.publish( - &vec![Topic::new(topic.into_string())], + &[Topic::new(topic.into_string())], pubsub_message.clone(), ); } @@ -96,45 +96,37 @@ fn test_gossipsub_full_mesh_publish() { let mut received_count = 0; tokio::run(futures::future::poll_fn(move || -> Result<_, ()> { for node in nodes.iter_mut() { - loop { - match node.poll().unwrap() { - Async::Ready(Some(Libp2pEvent::PubsubMessage { - topics, message, .. - })) => { - assert_eq!(topics.len(), 1); - // Assert topic is the published topic - assert_eq!( - topics.first().unwrap(), - &TopicHash::from_raw(publishing_topic.clone()) - ); - // Assert message received is the correct one - assert_eq!(message, pubsub_message.clone()); - received_count += 1; - if received_count == num_nodes - 1 { - return Ok(Async::Ready(())); - } - } - _ => break, + while let Async::Ready(Some(Libp2pEvent::PubsubMessage { + topics, message, .. + })) = node.poll().unwrap() + { + assert_eq!(topics.len(), 1); + // Assert topic is the published topic + assert_eq!( + topics.first().unwrap(), + &TopicHash::from_raw(publishing_topic.clone()) + ); + // Assert message received is the correct one + assert_eq!(message, pubsub_message.clone()); + received_count += 1; + if received_count == num_nodes - 1 { + return Ok(Async::Ready(())); } } } - loop { - match publishing_node.poll().unwrap() { - Async::Ready(Some(Libp2pEvent::PeerSubscribed(_, topic))) => { - // Received topics is one of subscribed eth2 topics - assert!(topic.clone().into_string().starts_with("/eth2/")); - // Publish on beacon block topic - if topic == TopicHash::from_raw("/eth2/beacon_block/ssz") { - subscribed_count += 1; - if subscribed_count == num_nodes - 1 { - publishing_node.swarm.publish( - &vec![Topic::new(topic.into_string())], - pubsub_message.clone(), - ); - } - } + while let Async::Ready(Some(Libp2pEvent::PeerSubscribed(_, topic))) = + publishing_node.poll().unwrap() + { + // Received topics is one of subscribed eth2 topics + assert!(topic.clone().into_string().starts_with("/eth2/")); + // Publish on beacon block topic + if topic == TopicHash::from_raw("/eth2/beacon_block/ssz") { + subscribed_count += 1; + if subscribed_count == num_nodes - 1 { + publishing_node + .swarm + .publish(&[Topic::new(topic.into_string())], pubsub_message.clone()); } - _ => break, } } Ok(Async::NotReady) diff --git a/beacon_node/eth2-libp2p/tests/rpc_tests.rs b/beacon_node/eth2-libp2p/tests/rpc_tests.rs index 91390b4e7..279cc7569 100644 --- a/beacon_node/eth2-libp2p/tests/rpc_tests.rs +++ b/beacon_node/eth2-libp2p/tests/rpc_tests.rs @@ -3,6 +3,7 @@ use eth2_libp2p::rpc::methods::*; use eth2_libp2p::rpc::*; use eth2_libp2p::{Libp2pEvent, RPCEvent}; use slog::{warn, Level}; +use std::sync::atomic::{AtomicBool, Ordering::Relaxed}; use std::sync::{Arc, Mutex}; use std::time::Duration; use tokio::prelude::*; @@ -106,20 +107,19 @@ fn test_status_rpc() { }); // execute the futures and check the result - let test_result = Arc::new(Mutex::new(false)); + let test_result = Arc::new(AtomicBool::new(false)); let error_result = test_result.clone(); let thread_result = test_result.clone(); tokio::run( sender_future .select(receiver_future) .timeout(Duration::from_millis(1000)) - .map_err(move |_| *error_result.lock().unwrap() = false) + .map_err(move |_| error_result.store(false, Relaxed)) .map(move |result| { - *thread_result.lock().unwrap() = result.0; - () + thread_result.store(result.0, Relaxed); }), ); - assert!(*test_result.lock().unwrap()); + assert!(test_result.load(Relaxed)); } #[test] @@ -236,20 +236,19 @@ fn test_blocks_by_range_chunked_rpc() { }); // execute the futures and check the result - let test_result = Arc::new(Mutex::new(false)); + let test_result = Arc::new(AtomicBool::new(false)); let error_result = test_result.clone(); let thread_result = test_result.clone(); tokio::run( sender_future .select(receiver_future) .timeout(Duration::from_millis(1000)) - .map_err(move |_| *error_result.lock().unwrap() = false) + .map_err(move |_| error_result.store(false, Relaxed)) .map(move |result| { - *thread_result.lock().unwrap() = result.0; - () + thread_result.store(result.0, Relaxed); }), ); - assert!(*test_result.lock().unwrap()); + assert!(test_result.load(Relaxed)); } #[test] @@ -359,20 +358,19 @@ fn test_blocks_by_range_single_empty_rpc() { }); // execute the futures and check the result - let test_result = Arc::new(Mutex::new(false)); + let test_result = Arc::new(AtomicBool::new(false)); let error_result = test_result.clone(); let thread_result = test_result.clone(); tokio::run( sender_future .select(receiver_future) .timeout(Duration::from_millis(1000)) - .map_err(move |_| *error_result.lock().unwrap() = false) + .map_err(move |_| error_result.store(false, Relaxed)) .map(move |result| { - *thread_result.lock().unwrap() = result.0; - () + thread_result.store(result.0, Relaxed); }), ); - assert!(*test_result.lock().unwrap()); + assert!(test_result.load(Relaxed)); } #[test] @@ -486,20 +484,19 @@ fn test_blocks_by_root_chunked_rpc() { }); // execute the futures and check the result - let test_result = Arc::new(Mutex::new(false)); + let test_result = Arc::new(AtomicBool::new(false)); let error_result = test_result.clone(); let thread_result = test_result.clone(); tokio::run( sender_future .select(receiver_future) .timeout(Duration::from_millis(1000)) - .map_err(move |_| *error_result.lock().unwrap() = false) + .map_err(move |_| error_result.store(false, Relaxed)) .map(move |result| { - *thread_result.lock().unwrap() = result.0; - () + thread_result.store(result.0, Relaxed); }), ); - assert!(*test_result.lock().unwrap()); + assert!(test_result.load(Relaxed)); } #[test] @@ -558,18 +555,17 @@ fn test_goodbye_rpc() { }); // execute the futures and check the result - let test_result = Arc::new(Mutex::new(false)); + let test_result = Arc::new(AtomicBool::new(false)); let error_result = test_result.clone(); let thread_result = test_result.clone(); tokio::run( sender_future .select(receiver_future) .timeout(Duration::from_millis(1000)) - .map_err(move |_| *error_result.lock().unwrap() = false) + .map_err(move |_| error_result.store(false, Relaxed)) .map(move |result| { - *thread_result.lock().unwrap() = result.0; - () + thread_result.store(result.0, Relaxed); }), ); - assert!(*test_result.lock().unwrap()); + assert!(test_result.load(Relaxed)); } diff --git a/beacon_node/genesis/src/common.rs b/beacon_node/genesis/src/common.rs index 9353ad33a..5c4447738 100644 --- a/beacon_node/genesis/src/common.rs +++ b/beacon_node/genesis/src/common.rs @@ -19,7 +19,7 @@ pub fn genesis_deposits( let depth = spec.deposit_contract_tree_depth as usize; let mut tree = MerkleTree::create(&[], depth); for (i, deposit_leaf) in deposit_root_leaves.iter().enumerate() { - if let Err(_) = tree.push_leaf(*deposit_leaf, depth) { + if tree.push_leaf(*deposit_leaf, depth).is_err() { return Err(String::from("Failed to push leaf")); } diff --git a/beacon_node/genesis/tests/tests.rs b/beacon_node/genesis/tests/tests.rs index d3030720a..bf9a8ce89 100644 --- a/beacon_node/genesis/tests/tests.rs +++ b/beacon_node/genesis/tests/tests.rs @@ -59,7 +59,6 @@ fn basic() { spec.min_genesis_active_validator_count = 8; let deposits = (0..spec.min_genesis_active_validator_count + 2) - .into_iter() .map(|i| { deposit_contract.deposit_helper::( generate_deterministic_keypair(i as usize), @@ -73,7 +72,7 @@ fn basic() { }) .collect::>(); - let deposit_future = deposit_contract.deposit_multiple(deposits.clone()); + let deposit_future = deposit_contract.deposit_multiple(deposits); let wait_future = service.wait_for_genesis_state::(update_interval, spec.clone()); diff --git a/beacon_node/network/src/message_handler.rs b/beacon_node/network/src/message_handler.rs index 4b1215287..a7c8ed40b 100644 --- a/beacon_node/network/src/message_handler.rs +++ b/beacon_node/network/src/message_handler.rs @@ -229,7 +229,7 @@ impl MessageHandler { .on_block_gossip(peer_id.clone(), block); // TODO: Apply more sophisticated validation and decoding logic if should_forward_on { - self.propagate_message(id, peer_id.clone()); + self.propagate_message(id, peer_id); } } Err(e) => { diff --git a/beacon_node/network/src/message_processor.rs b/beacon_node/network/src/message_processor.rs index f77f8e6ac..dc4f91d2f 100644 --- a/beacon_node/network/src/message_processor.rs +++ b/beacon_node/network/src/message_processor.rs @@ -203,7 +203,7 @@ impl MessageProcessor { ); self.network - .disconnect(peer_id.clone(), GoodbyeReason::IrrelevantNetwork); + .disconnect(peer_id, GoodbyeReason::IrrelevantNetwork); } else if remote.head_slot > self.chain.slot().unwrap_or_else(|_| Slot::from(0u64)) + FUTURE_SLOT_TOLERANCE { @@ -219,7 +219,7 @@ impl MessageProcessor { "reason" => "different system clocks or genesis time" ); self.network - .disconnect(peer_id.clone(), GoodbyeReason::IrrelevantNetwork); + .disconnect(peer_id, GoodbyeReason::IrrelevantNetwork); } else if remote.finalized_epoch <= local.finalized_epoch && remote.finalized_root != Hash256::zero() && local.finalized_root != Hash256::zero() @@ -239,7 +239,7 @@ impl MessageProcessor { "reason" => "different finalized chain" ); self.network - .disconnect(peer_id.clone(), GoodbyeReason::IrrelevantNetwork); + .disconnect(peer_id, GoodbyeReason::IrrelevantNetwork); } else if remote.finalized_epoch < local.finalized_epoch { // The node has a lower finalized epoch, their chain is not useful to us. There are two // cases where a node can have a lower finalized epoch: @@ -512,7 +512,7 @@ impl MessageProcessor { // Inform the sync manager to find parents for this block trace!(self.log, "Block with unknown parent received"; "peer_id" => format!("{:?}",peer_id)); - self.send_to_sync(SyncMessage::UnknownBlock(peer_id, Box::new(block.clone()))); + self.send_to_sync(SyncMessage::UnknownBlock(peer_id, Box::new(block))); SHOULD_FORWARD_GOSSIP_BLOCK } BlockProcessingOutcome::FutureSlot { diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs index b1bbc3bbe..7936e23d6 100644 --- a/beacon_node/network/src/service.rs +++ b/beacon_node/network/src/service.rs @@ -263,7 +263,7 @@ fn network_service( id, source, message, - topics: _, + .. } => { message_handler_send .try_send(HandlerMessage::PubsubMessage(id, source, message)) diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index a470ce9c6..f2cd7a970 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -66,7 +66,7 @@ impl SyncNetworkContext { "count" => request.count, "peer" => format!("{:?}", peer_id) ); - self.send_rpc_request(peer_id.clone(), RPCRequest::BlocksByRange(request)) + self.send_rpc_request(peer_id, RPCRequest::BlocksByRange(request)) } pub fn blocks_by_root_request( @@ -81,7 +81,7 @@ impl SyncNetworkContext { "count" => request.block_roots.len(), "peer" => format!("{:?}", peer_id) ); - self.send_rpc_request(peer_id.clone(), RPCRequest::BlocksByRoot(request)) + self.send_rpc_request(peer_id, RPCRequest::BlocksByRoot(request)) } pub fn downvote_peer(&mut self, peer_id: PeerId) { @@ -91,7 +91,7 @@ impl SyncNetworkContext { "peer" => format!("{:?}", peer_id) ); // TODO: Implement reputation - self.disconnect(peer_id.clone(), GoodbyeReason::Fault); + self.disconnect(peer_id, GoodbyeReason::Fault); } fn disconnect(&mut self, peer_id: PeerId, reason: GoodbyeReason) { diff --git a/beacon_node/network/src/sync/range_sync/batch.rs b/beacon_node/network/src/sync/range_sync/batch.rs index c6f8c4477..45a7672e4 100644 --- a/beacon_node/network/src/sync/range_sync/batch.rs +++ b/beacon_node/network/src/sync/range_sync/batch.rs @@ -62,16 +62,16 @@ impl PendingBatches { let peer_request = batch.current_peer.clone(); self.peer_requests .entry(peer_request) - .or_insert_with(|| HashSet::new()) + .or_insert_with(HashSet::new) .insert(request_id); self.batches.insert(request_id, batch) } - pub fn remove(&mut self, request_id: &RequestId) -> Option> { - if let Some(batch) = self.batches.remove(request_id) { + pub fn remove(&mut self, request_id: RequestId) -> Option> { + if let Some(batch) = self.batches.remove(&request_id) { if let Entry::Occupied(mut entry) = self.peer_requests.entry(batch.current_peer.clone()) { - entry.get_mut().remove(request_id); + entry.get_mut().remove(&request_id); if entry.get().is_empty() { entry.remove(); @@ -85,8 +85,8 @@ impl PendingBatches { /// Adds a block to the batches if the request id exists. Returns None if there is no batch /// matching the request id. - pub fn add_block(&mut self, request_id: &RequestId, block: BeaconBlock) -> Option<()> { - let batch = self.batches.get_mut(request_id)?; + pub fn add_block(&mut self, request_id: RequestId, block: BeaconBlock) -> Option<()> { + let batch = self.batches.get_mut(&request_id)?; batch.downloaded_blocks.push(block); Some(()) } @@ -101,7 +101,7 @@ impl PendingBatches { pub fn remove_batch_by_peer(&mut self, peer_id: &PeerId) -> Option> { let request_ids = self.peer_requests.get(peer_id)?; - let request_id = request_ids.iter().next()?.clone(); - self.remove(&request_id) + let request_id = *request_ids.iter().next()?; + self.remove(request_id) } } diff --git a/beacon_node/network/src/sync/range_sync/chain.rs b/beacon_node/network/src/sync/range_sync/chain.rs index be2fcf2c8..ddeaf0583 100644 --- a/beacon_node/network/src/sync/range_sync/chain.rs +++ b/beacon_node/network/src/sync/range_sync/chain.rs @@ -144,11 +144,11 @@ impl SyncingChain { ) -> Option { if let Some(block) = beacon_block { // This is not a stream termination, simply add the block to the request - self.pending_batches.add_block(&request_id, block.clone())?; - return Some(ProcessingResult::KeepChain); + self.pending_batches.add_block(request_id, block.clone())?; + Some(ProcessingResult::KeepChain) } else { // A stream termination has been sent. This batch has ended. Process a completed batch. - let batch = self.pending_batches.remove(&request_id)?; + let batch = self.pending_batches.remove(request_id)?; Some(self.process_completed_batch(chain.clone(), network, batch, log)) } } @@ -433,7 +433,7 @@ impl SyncingChain { return true; } } - return false; + false } /// Returns a peer if there exists a peer which does not currently have a pending request. @@ -500,10 +500,10 @@ impl SyncingChain { &mut self, network: &mut SyncNetworkContext, peer_id: &PeerId, - request_id: &RequestId, + request_id: RequestId, log: &slog::Logger, ) -> Option { - if let Some(batch) = self.pending_batches.remove(&request_id) { + if let Some(batch) = self.pending_batches.remove(request_id) { warn!(log, "Batch failed. RPC Error"; "id" => batch.id, "retries" => batch.retries, "peer" => format!("{:?}", peer_id)); Some(self.failed_batch(network, batch, log)) diff --git a/beacon_node/network/src/sync/range_sync/range.rs b/beacon_node/network/src/sync/range_sync/range.rs index 8f38b90c0..98ef6e621 100644 --- a/beacon_node/network/src/sync/range_sync/range.rs +++ b/beacon_node/network/src/sync/range_sync/range.rs @@ -188,7 +188,7 @@ impl RangeSync { debug!(self.log, "Adding peer to the existing head chain peer pool"; "head_root" => format!("{}",remote.head_root), "head_slot" => remote.head_slot, "peer_id" => format!("{:?}", peer_id)); // add the peer to the head's pool - chain.add_peer(network, peer_id.clone(), &self.log); + chain.add_peer(network, peer_id, &self.log); } else { // There are no other head chains that match this peer's status, create a new one, and let start_slot = std::cmp::min(local_info.head_slot, remote_finalized_slot); @@ -305,29 +305,28 @@ impl RangeSync { /// retries. In this case, we need to remove the chain and re-status all the peers. fn remove_peer(&mut self, network: &mut SyncNetworkContext, peer_id: &PeerId) { let log_ref = &self.log; - match self.chains.head_finalized_request(|chain| { - if chain.peer_pool.remove(peer_id) { - // this chain contained the peer - while let Some(batch) = chain.pending_batches.remove_batch_by_peer(peer_id) { - if let ProcessingResult::RemoveChain = - chain.failed_batch(network, batch, log_ref) - { - // a single batch failed, remove the chain - return Some(ProcessingResult::RemoveChain); + if let Some((index, ProcessingResult::RemoveChain)) = + self.chains.head_finalized_request(|chain| { + if chain.peer_pool.remove(peer_id) { + // this chain contained the peer + while let Some(batch) = chain.pending_batches.remove_batch_by_peer(peer_id) { + if let ProcessingResult::RemoveChain = + chain.failed_batch(network, batch, log_ref) + { + // a single batch failed, remove the chain + return Some(ProcessingResult::RemoveChain); + } } + // peer removed from chain, no batch failed + Some(ProcessingResult::KeepChain) + } else { + None } - // peer removed from chain, no batch failed - Some(ProcessingResult::KeepChain) - } else { - None - } - }) { - Some((index, ProcessingResult::RemoveChain)) => { - // the chain needed to be removed - debug!(self.log, "Chain being removed due to failed batch"); - self.chains.remove_chain(network, index, &self.log); - } - _ => {} // chain didn't need to be removed, ignore + }) + { + // the chain needed to be removed + debug!(self.log, "Chain being removed due to failed batch"); + self.chains.remove_chain(network, index, &self.log); } } @@ -344,7 +343,7 @@ impl RangeSync { // check that this request is pending let log_ref = &self.log; match self.chains.head_finalized_request(|chain| { - chain.inject_error(network, &peer_id, &request_id, log_ref) + chain.inject_error(network, &peer_id, request_id, log_ref) }) { Some((_, ProcessingResult::KeepChain)) => {} // error handled chain persists Some((index, ProcessingResult::RemoveChain)) => { diff --git a/beacon_node/rest_api/src/helpers.rs b/beacon_node/rest_api/src/helpers.rs index e64c1e459..26de0a819 100644 --- a/beacon_node/rest_api/src/helpers.rs +++ b/beacon_node/rest_api/src/helpers.rs @@ -145,7 +145,7 @@ pub fn state_at_slot( // I'm not sure if this `.clone()` will be optimized out. If not, it seems unnecessary. Ok(( beacon_chain.head()?.beacon_state_root, - beacon_chain.head()?.beacon_state.clone(), + beacon_chain.head()?.beacon_state, )) } else { let root = state_root_at_slot(beacon_chain, slot)?; @@ -209,7 +209,7 @@ pub fn state_root_at_slot( // // Use `per_slot_processing` to advance the head state to the present slot, // assuming that all slots do not contain a block (i.e., they are skipped slots). - let mut state = beacon_chain.head()?.beacon_state.clone(); + let mut state = beacon_chain.head()?.beacon_state; let spec = &T::EthSpec::default_spec(); for _ in state.slot.as_u64()..slot.as_u64() { diff --git a/beacon_node/rest_api/src/lib.rs b/beacon_node/rest_api/src/lib.rs index 1473c5e72..221d96e25 100644 --- a/beacon_node/rest_api/src/lib.rs +++ b/beacon_node/rest_api/src/lib.rs @@ -54,6 +54,8 @@ pub struct NetworkInfo { pub network_chan: mpsc::UnboundedSender, } +// Allowing more than 7 arguments. +#[allow(clippy::too_many_arguments)] pub fn start_server( config: &Config, executor: &TaskExecutor, diff --git a/beacon_node/rest_api/src/router.rs b/beacon_node/rest_api/src/router.rs index 8c30ff3f0..d9fd63ce5 100644 --- a/beacon_node/rest_api/src/router.rs +++ b/beacon_node/rest_api/src/router.rs @@ -19,6 +19,8 @@ where Box::new(item.into_future()) } +// Allowing more than 7 arguments. +#[allow(clippy::too_many_arguments)] pub fn route( req: Request, beacon_chain: Arc>, diff --git a/beacon_node/rest_api/tests/test.rs b/beacon_node/rest_api/tests/test.rs index 987b8e326..1d438253a 100644 --- a/beacon_node/rest_api/tests/test.rs +++ b/beacon_node/rest_api/tests/test.rs @@ -47,8 +47,7 @@ fn get_randao_reveal( .head() .expect("should get head") .beacon_state - .fork - .clone(); + .fork; let proposer_index = beacon_chain .block_proposer(slot) .expect("should get proposer index"); @@ -69,8 +68,7 @@ fn sign_block( .head() .expect("should get head") .beacon_state - .fork - .clone(); + .fork; let proposer_index = beacon_chain .block_proposer(block.slot) .expect("should get proposer index"); @@ -91,11 +89,7 @@ fn validator_produce_attestation() { .client .beacon_chain() .expect("client should have beacon chain"); - let state = beacon_chain - .head() - .expect("should get head") - .beacon_state - .clone(); + let state = beacon_chain.head().expect("should get head").beacon_state; let validator_index = 0; let duties = state @@ -169,7 +163,7 @@ fn validator_produce_attestation() { remote_node .http .validator() - .publish_attestation(attestation.clone()), + .publish_attestation(attestation), ) .expect("should publish attestation"); assert!( @@ -344,7 +338,7 @@ fn validator_block_post() { remote_node .http .validator() - .produce_block(slot, randao_reveal.clone()), + .produce_block(slot, randao_reveal), ) .expect("should fetch block from http api"); @@ -360,12 +354,12 @@ fn validator_block_post() { ); } - sign_block(beacon_chain.clone(), &mut block, spec); + sign_block(beacon_chain, &mut block, spec); let block_root = block.canonical_root(); let publish_status = env .runtime() - .block_on(remote_node.http.validator().publish_block(block.clone())) + .block_on(remote_node.http.validator().publish_block(block)) .expect("should publish block"); if cfg!(not(feature = "fake_crypto")) { @@ -419,7 +413,7 @@ fn validator_block_get() { .expect("client should have beacon chain"); let slot = Slot::new(1); - let randao_reveal = get_randao_reveal(beacon_chain.clone(), slot, spec); + let randao_reveal = get_randao_reveal(beacon_chain, slot, spec); let block = env .runtime() diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 61d6f7db3..b68e1841e 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -268,7 +268,7 @@ pub fn get_configs( if eth2_config.spec_constants != client_config.spec_constants { crit!(log, "Specification constants do not match."; "client_config" => client_config.spec_constants.to_string(), - "eth2_config" => eth2_config.spec_constants.to_string() + "eth2_config" => eth2_config.spec_constants ); return Err("Specification constant mismatch".into()); } diff --git a/beacon_node/store/benches/benches.rs b/beacon_node/store/benches/benches.rs index 5a8675ecf..eac31c36b 100644 --- a/beacon_node/store/benches/benches.rs +++ b/beacon_node/store/benches/benches.rs @@ -24,7 +24,6 @@ fn get_state(validator_count: usize) -> BeaconState { } state.validators = (0..validator_count) - .into_iter() .collect::>() .par_iter() .map(|&i| Validator { @@ -77,7 +76,7 @@ fn all_benches(c: &mut Criterion) { .sample_size(10), ); - let inner_state = state.clone(); + let inner_state = state; c.bench( &format!("{}_validators", validator_count), Benchmark::new("encode/beacon_state/committee_cache[0]", move |b| { diff --git a/beacon_node/store/examples/ssz_encode_state_container.rs b/beacon_node/store/examples/ssz_encode_state_container.rs index f36f85d06..d44f7b4e9 100644 --- a/beacon_node/store/examples/ssz_encode_state_container.rs +++ b/beacon_node/store/examples/ssz_encode_state_container.rs @@ -27,7 +27,6 @@ fn get_state(validator_count: usize) -> BeaconState { } state.validators = (0..validator_count) - .into_iter() .collect::>() .par_iter() .map(|&i| Validator { diff --git a/beacon_node/store/src/block_at_slot.rs b/beacon_node/store/src/block_at_slot.rs index 292d7b383..dbb2c946e 100644 --- a/beacon_node/store/src/block_at_slot.rs +++ b/beacon_node/store/src/block_at_slot.rs @@ -1,5 +1,6 @@ use super::*; use ssz::{Decode, DecodeError}; +use std::cmp::Ordering; fn get_block_bytes, E: EthSpec>( store: &T, @@ -45,12 +46,10 @@ fn get_at_preceeding_slot, E: EthSpec>( if let Some(bytes) = get_block_bytes::<_, E>(store, root)? { let this_slot = read_slot_from_block_bytes(&bytes)?; - if this_slot == slot { - break Ok(Some((root, bytes))); - } else if this_slot < slot { - break Ok(None); - } else { - root = read_parent_root_from_block_bytes(&bytes)?; + match this_slot.cmp(&slot) { + Ordering::Equal => break Ok(Some((root, bytes))), + Ordering::Less => break Ok(None), + Ordering::Greater => root = read_parent_root_from_block_bytes(&bytes)?, } } else { break Ok(None); diff --git a/beacon_node/store/src/block_root_tree.rs b/beacon_node/store/src/block_root_tree.rs index 76950069c..dba1b1c24 100644 --- a/beacon_node/store/src/block_root_tree.rs +++ b/beacon_node/store/src/block_root_tree.rs @@ -237,7 +237,7 @@ mod test { .add_block_root(int_hash(i), int_hash(i - 1), Slot::new(i)) .expect("add_block_root ok"); - let expected = (1..i + 1) + let expected = (1..=i) .rev() .map(|j| (int_hash(j), Slot::new(j))) .collect::>(); @@ -262,12 +262,12 @@ mod test { .add_block_root(int_hash(i), int_hash(i - step_length), Slot::new(i)) .expect("add_block_root ok"); - let sparse_expected = (1..i + 1) + let sparse_expected = (1..=i) .rev() .step_by(step_length as usize) .map(|j| (int_hash(j), Slot::new(j))) .collect_vec(); - let every_slot_expected = (1..i + 1) + let every_slot_expected = (1..=i) .rev() .map(|j| { let nearest = 1 + (j - 1) / step_length * step_length; @@ -343,10 +343,9 @@ mod test { // Check that advancing the finalized root onto one side completely removes the other // side. - let fin_tree = tree.clone(); + let fin_tree = tree; let prune_point = num_blocks / 2; let remaining_fork1_blocks = all_fork1_blocks - .clone() .into_iter() .take_while(|(_, slot)| *slot >= prune_point) .collect_vec(); diff --git a/beacon_node/store/src/chunked_vector.rs b/beacon_node/store/src/chunked_vector.rs index 4dd05d716..b876545c7 100644 --- a/beacon_node/store/src/chunked_vector.rs +++ b/beacon_node/store/src/chunked_vector.rs @@ -185,7 +185,7 @@ pub trait Field: Copy { .values .first() .cloned() - .ok_or(ChunkError::MissingGenesisValue.into()) + .ok_or_else(|| ChunkError::MissingGenesisValue.into()) } /// Store the given `value` as the genesis value for this field, unless stored already. @@ -685,7 +685,7 @@ mod test { ]; assert_eq!( - stitch(chunks.clone(), 2, 6, chunk_size, 12, 99).unwrap(), + stitch(chunks, 2, 6, chunk_size, 12, 99).unwrap(), vec![99, 99, 2, 3, 4, 5, 99, 99, 99, 99, 99, 99] ); } @@ -707,7 +707,7 @@ mod test { ); assert_eq!( - stitch(chunks.clone(), 2, 10, chunk_size, 8, default).unwrap(), + stitch(chunks, 2, 10, chunk_size, 8, default).unwrap(), vec![v(8), v(9), v(2), v(3), v(4), v(5), v(6), v(7)] ); } diff --git a/beacon_node/store/src/forwards_iter.rs b/beacon_node/store/src/forwards_iter.rs index bcec40935..1c2431fc3 100644 --- a/beacon_node/store/src/forwards_iter.rs +++ b/beacon_node/store/src/forwards_iter.rs @@ -20,9 +20,9 @@ pub struct SimpleForwardsBlockRootsIterator { /// Fusion of the above two approaches to forwards iteration. Fast and efficient. pub enum HybridForwardsBlockRootsIterator { PreFinalization { - iter: FrozenForwardsBlockRootsIterator, + iter: Box>, /// Data required by the `PostFinalization` iterator when we get to it. - continuation_data: Option<(BeaconState, Hash256)>, + continuation_data: Box, Hash256)>>, }, PostFinalization { iter: SimpleForwardsBlockRootsIterator, @@ -99,13 +99,13 @@ impl HybridForwardsBlockRootsIterator { if start_slot < latest_restore_point_slot { PreFinalization { - iter: FrozenForwardsBlockRootsIterator::new( + iter: Box::new(FrozenForwardsBlockRootsIterator::new( store, start_slot, latest_restore_point_slot, spec, - ), - continuation_data: Some((end_state, end_block_root)), + )), + continuation_data: Box::new(Some((end_state, end_block_root))), } } else { PostFinalization { diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index c9acf7631..90d9b7441 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -145,14 +145,15 @@ impl Store for HotColdDB { let current_split_slot = store.get_split_slot(); if frozen_head.slot < current_split_slot { - Err(HotColdDBError::FreezeSlotError { + return Err(HotColdDBError::FreezeSlotError { current_split_slot, proposed_split_slot: frozen_head.slot, - })?; + } + .into()); } if frozen_head.slot % E::slots_per_epoch() != 0 { - Err(HotColdDBError::FreezeSlotUnaligned(frozen_head.slot))?; + return Err(HotColdDBError::FreezeSlotUnaligned(frozen_head.slot).into()); } // 1. Copy all of the states between the head and the split slot, from the hot DB @@ -574,7 +575,7 @@ impl HotColdDB { let key = Self::restore_point_key(restore_point_index); RestorePointHash::db_get(&self.cold_db, &key)? .map(|r| r.state_root) - .ok_or(HotColdDBError::MissingRestorePointHash(restore_point_index).into()) + .ok_or_else(|| HotColdDBError::MissingRestorePointHash(restore_point_index).into()) } /// Store the state root of a restore point. diff --git a/beacon_node/store/src/iter.rs b/beacon_node/store/src/iter.rs index 07078acee..c17ee70d7 100644 --- a/beacon_node/store/src/iter.rs +++ b/beacon_node/store/src/iter.rs @@ -345,7 +345,7 @@ mod test { state_b.state_roots[0] = state_a_root; store.put_state(&state_a_root, &state_a).unwrap(); - let iter = BlockRootsIterator::new(store.clone(), &state_b); + let iter = BlockRootsIterator::new(store, &state_b); assert!( iter.clone().any(|(_root, slot)| slot == 0), @@ -394,7 +394,7 @@ mod test { store.put_state(&state_a_root, &state_a).unwrap(); store.put_state(&state_b_root, &state_b).unwrap(); - let iter = StateRootsIterator::new(store.clone(), &state_b); + let iter = StateRootsIterator::new(store, &state_b); assert!( iter.clone().any(|(_root, slot)| slot == 0), diff --git a/beacon_node/store/src/memory_store.rs b/beacon_node/store/src/memory_store.rs index 169575651..59caf9eaa 100644 --- a/beacon_node/store/src/memory_store.rs +++ b/beacon_node/store/src/memory_store.rs @@ -47,11 +47,7 @@ impl Store for MemoryStore { fn get_bytes(&self, col: &str, key: &[u8]) -> Result>, Error> { let column_key = Self::get_key_for_col(col, key); - Ok(self - .db - .read() - .get(&column_key) - .and_then(|val| Some(val.clone()))) + Ok(self.db.read().get(&column_key).cloned()) } /// Puts a key in the database. diff --git a/beacon_node/store/src/migrate.rs b/beacon_node/store/src/migrate.rs index 2d6cd604b..ffe518a89 100644 --- a/beacon_node/store/src/migrate.rs +++ b/beacon_node/store/src/migrate.rs @@ -60,13 +60,12 @@ impl> Migrate for BlockingMigrator { } } +type MpscSender = mpsc::Sender<(Hash256, BeaconState)>; + /// Migrator that runs a background thread to migrate state from the hot to the cold database. pub struct BackgroundMigrator { db: Arc>, - tx_thread: Mutex<( - mpsc::Sender<(Hash256, BeaconState)>, - thread::JoinHandle<()>, - )>, + tx_thread: Mutex<(MpscSender, thread::JoinHandle<()>)>, } impl Migrate, E> for BackgroundMigrator { diff --git a/eth2/lmd_ghost/src/reduced_tree.rs b/eth2/lmd_ghost/src/reduced_tree.rs index 547bdf247..d0798afd2 100644 --- a/eth2/lmd_ghost/src/reduced_tree.rs +++ b/eth2/lmd_ghost/src/reduced_tree.rs @@ -8,6 +8,7 @@ use itertools::Itertools; use parking_lot::RwLock; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; +use std::cmp::Ordering; use std::collections::HashMap; use std::fmt; use std::marker::PhantomData; @@ -182,13 +183,15 @@ impl ReducedTreeSsz { } } - pub fn to_reduced_tree( + pub fn into_reduced_tree( self, store: Arc, block_root_tree: Arc, ) -> Result> { if self.node_hashes.len() != self.nodes.len() { - Error::InvalidReducedTreeSsz("node_hashes and nodes should have equal length".into()); + return Err(Error::InvalidReducedTreeSsz( + "node_hashes and nodes should have equal length".to_string(), + )); } let nodes: HashMap<_, _> = self .node_hashes @@ -740,16 +743,19 @@ where if a_slot < self.root.1 || b_slot < self.root.1 { None } else { - if a_slot < b_slot { - for _ in a_slot.as_u64()..b_slot.as_u64() { - b_root = b_iter.next()?.0; + match a_slot.cmp(&b_slot) { + Ordering::Less => { + for _ in a_slot.as_u64()..b_slot.as_u64() { + b_root = b_iter.next()?.0; + } } - } else if a_slot > b_slot { - for _ in b_slot.as_u64()..a_slot.as_u64() { - a_root = a_iter.next()?.0; + Ordering::Greater => { + for _ in b_slot.as_u64()..a_slot.as_u64() { + a_root = a_iter.next()?.0; + } } + Ordering::Equal => (), } - Some((a_root, b_root)) } } @@ -876,7 +882,7 @@ where block_root_tree: Arc, ) -> Result { let reduced_tree_ssz = ReducedTreeSsz::from_ssz_bytes(bytes)?; - Ok(reduced_tree_ssz.to_reduced_tree(store, block_root_tree)?) + Ok(reduced_tree_ssz.into_reduced_tree(store, block_root_tree)?) } } @@ -1013,8 +1019,7 @@ mod tests { ); let ssz_tree = ReducedTreeSsz::from_reduced_tree(&tree); let bytes = tree.as_bytes(); - let recovered_tree = - ReducedTree::from_bytes(&bytes, store.clone(), block_root_tree).unwrap(); + let recovered_tree = ReducedTree::from_bytes(&bytes, store, block_root_tree).unwrap(); let recovered_ssz = ReducedTreeSsz::from_reduced_tree(&recovered_tree); assert_eq!(ssz_tree, recovered_ssz); diff --git a/eth2/operation_pool/src/max_cover.rs b/eth2/operation_pool/src/max_cover.rs index 15d528e45..8188d6939 100644 --- a/eth2/operation_pool/src/max_cover.rs +++ b/eth2/operation_pool/src/max_cover.rs @@ -167,7 +167,7 @@ mod test { HashSet::from_iter(vec![5, 6, 7, 8]), // 4, 4* HashSet::from_iter(vec![0, 1, 2, 3, 4]), // 5* ]; - let cover = maximum_cover(sets.clone(), 3); + let cover = maximum_cover(sets, 3); assert_eq!(quality(&cover), 11); } @@ -182,7 +182,7 @@ mod test { HashSet::from_iter(vec![1, 5, 6, 8]), HashSet::from_iter(vec![1, 7, 11, 19]), ]; - let cover = maximum_cover(sets.clone(), 5); + let cover = maximum_cover(sets, 5); assert_eq!(quality(&cover), 19); assert_eq!(cover.len(), 5); } diff --git a/eth2/state_processing/benches/benches.rs b/eth2/state_processing/benches/benches.rs index c277e6734..f03c571ad 100644 --- a/eth2/state_processing/benches/benches.rs +++ b/eth2/state_processing/benches/benches.rs @@ -311,11 +311,7 @@ fn bench_block( ) .expect("should get indexed attestation"); - ( - local_spec.clone(), - local_state.clone(), - indexed_attestation.clone(), - ) + (local_spec.clone(), local_state.clone(), indexed_attestation) }, |(spec, ref mut state, indexed_attestation)| { black_box( @@ -349,11 +345,7 @@ fn bench_block( ) .expect("should get indexed attestation"); - ( - local_spec.clone(), - local_state.clone(), - indexed_attestation.clone(), - ) + (local_spec.clone(), local_state.clone(), indexed_attestation) }, |(spec, ref mut state, indexed_attestation)| { black_box( @@ -373,7 +365,7 @@ fn bench_block( ); let local_block = block.clone(); - let local_state = state.clone(); + let local_state = state; c.bench( &title, Benchmark::new("get_attesting_indices", move |b| { @@ -409,7 +401,7 @@ fn bench_block( .sample_size(10), ); - let local_block = block.clone(); + let local_block = block; c.bench( &title, Benchmark::new("ssz_block_len", move |b| { diff --git a/eth2/state_processing/tests/tests.rs b/eth2/state_processing/tests/tests.rs index 77bc14ba3..83f8cdd16 100644 --- a/eth2/state_processing/tests/tests.rs +++ b/eth2/state_processing/tests/tests.rs @@ -29,7 +29,7 @@ where F: FnMut(&mut BlockBuilder), G: FnMut(&mut BeaconBlock), { - let (mut block, state) = get_block::(mutate_builder); + let (mut block, mut state) = get_block::(mutate_builder); /* * Control check to ensure the valid block should pass verification. @@ -79,7 +79,7 @@ where assert_eq!( per_block_processing( - &mut state.clone(), + &mut state, &block, None, BlockSignatureStrategy::VerifyBulk, diff --git a/eth2/types/benches/benches.rs b/eth2/types/benches/benches.rs index dd66c7174..e38b9ded3 100644 --- a/eth2/types/benches/benches.rs +++ b/eth2/types/benches/benches.rs @@ -22,7 +22,6 @@ fn get_state(validator_count: usize) -> BeaconState { } state.validators = (0..validator_count) - .into_iter() .collect::>() .par_iter() .map(|&i| Validator { @@ -91,7 +90,7 @@ fn all_benches(c: &mut Criterion) { .sample_size(10), ); - let inner_state = state.clone(); + let inner_state = state; c.bench( &format!("{}_validators", validator_count), Benchmark::new("clone_without_caches/beacon_state", move |b| { diff --git a/eth2/types/src/attestation.rs b/eth2/types/src/attestation.rs index e1b2078e9..66306ba18 100644 --- a/eth2/types/src/attestation.rs +++ b/eth2/types/src/attestation.rs @@ -71,13 +71,13 @@ impl Attestation { if self .aggregation_bits .get(committee_position) - .map_err(|e| Error::SszTypesError(e))? + .map_err(Error::SszTypesError)? { Err(Error::AlreadySigned(committee_position)) } else { self.aggregation_bits .set(committee_position, true) - .map_err(|e| Error::SszTypesError(e))?; + .map_err(Error::SszTypesError)?; let message = self.data.tree_hash_root(); let domain = spec.get_domain(self.data.target.epoch, Domain::BeaconAttester, fork); diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index dd483a774..ac17d6d04 100644 --- a/eth2/types/src/beacon_state.rs +++ b/eth2/types/src/beacon_state.rs @@ -897,7 +897,7 @@ impl BeaconState { .enumerate() .skip(self.pubkey_cache.len()) { - let success = self.pubkey_cache.insert(validator.pubkey.clone().into(), i); + let success = self.pubkey_cache.insert(validator.pubkey.clone(), i); if !success { return Err(Error::PubkeyCacheInconsistent); } @@ -957,7 +957,7 @@ impl BeaconState { validator .pubkey .decompress() - .map_err(|e| Error::InvalidValidatorPubkey(e)) + .map_err(Error::InvalidValidatorPubkey) } else { Ok(()) } diff --git a/eth2/types/src/test_utils/builders/testing_attestation_builder.rs b/eth2/types/src/test_utils/builders/testing_attestation_builder.rs index 3d877f881..a0e34128c 100644 --- a/eth2/types/src/test_utils/builders/testing_attestation_builder.rs +++ b/eth2/types/src/test_utils/builders/testing_attestation_builder.rs @@ -24,9 +24,8 @@ impl TestingAttestationBuilder { let mut aggregation_bits_len = committee.len(); - match test_task { - AttestationTestTask::BadAggregationBitfieldLen => aggregation_bits_len += 1, - _ => (), + if test_task == AttestationTestTask::BadAggregationBitfieldLen { + aggregation_bits_len += 1 } let mut aggregation_bits = BitList::with_capacity(aggregation_bits_len).unwrap(); diff --git a/eth2/types/src/test_utils/builders/testing_beacon_state_builder.rs b/eth2/types/src/test_utils/builders/testing_beacon_state_builder.rs index 04f22fa89..333f221ad 100644 --- a/eth2/types/src/test_utils/builders/testing_beacon_state_builder.rs +++ b/eth2/types/src/test_utils/builders/testing_beacon_state_builder.rs @@ -15,7 +15,7 @@ pub const KEYPAIRS_FILE: &str = "keypairs.raw_keypairs"; /// `./keypairs.raw_keypairs`. pub fn keypairs_path() -> PathBuf { let dir = dirs::home_dir() - .and_then(|home| Some(home.join(".lighthouse"))) + .map(|home| (home.join(".lighthouse"))) .unwrap_or_else(|| PathBuf::from("")); dir.join(KEYPAIRS_FILE) } @@ -42,7 +42,7 @@ impl TestingBeaconStateBuilder { /// If the file does not contain enough keypairs or is invalid. pub fn from_default_keypairs_file_if_exists(validator_count: usize, spec: &ChainSpec) -> Self { let dir = dirs::home_dir() - .and_then(|home| Some(home.join(".lighthouse"))) + .map(|home| home.join(".lighthouse")) .unwrap_or_else(|| PathBuf::from("")); let file = dir.join(KEYPAIRS_FILE); diff --git a/eth2/types/src/test_utils/builders/testing_proposer_slashing_builder.rs b/eth2/types/src/test_utils/builders/testing_proposer_slashing_builder.rs index bfd4bd334..182236a36 100644 --- a/eth2/types/src/test_utils/builders/testing_proposer_slashing_builder.rs +++ b/eth2/types/src/test_utils/builders/testing_proposer_slashing_builder.rs @@ -30,7 +30,7 @@ impl TestingProposerSlashingBuilder { let slot = Slot::new(0); let hash_1 = Hash256::from([1; 32]); let hash_2 = if test_task == ProposerSlashingTestTask::ProposalsIdentical { - hash_1.clone() + hash_1 } else { Hash256::from([2; 32]) }; diff --git a/eth2/utils/bls/src/aggregate_signature.rs b/eth2/utils/bls/src/aggregate_signature.rs index 146fbfd5d..8d930f0cc 100644 --- a/eth2/utils/bls/src/aggregate_signature.rs +++ b/eth2/utils/bls/src/aggregate_signature.rs @@ -99,9 +99,10 @@ impl AggregateSignature { for byte in bytes { if *byte != 0 { let sig = RawAggregateSignature::from_bytes(&bytes).map_err(|_| { - DecodeError::BytesInvalid( - format!("Invalid AggregateSignature bytes: {:?}", bytes).to_string(), - ) + DecodeError::BytesInvalid(format!( + "Invalid AggregateSignature bytes: {:?}", + bytes + )) })?; return Ok(Self { diff --git a/eth2/utils/bls/src/public_key.rs b/eth2/utils/bls/src/public_key.rs index 095d1989f..42ed592ef 100644 --- a/eth2/utils/bls/src/public_key.rs +++ b/eth2/utils/bls/src/public_key.rs @@ -39,7 +39,7 @@ impl PublicKey { /// Converts compressed bytes to PublicKey pub fn from_bytes(bytes: &[u8]) -> Result { let pubkey = RawPublicKey::from_bytes(&bytes).map_err(|_| { - DecodeError::BytesInvalid(format!("Invalid PublicKey bytes: {:?}", bytes).to_string()) + DecodeError::BytesInvalid(format!("Invalid PublicKey bytes: {:?}", bytes)) })?; Ok(PublicKey(pubkey)) diff --git a/eth2/utils/bls/src/signature.rs b/eth2/utils/bls/src/signature.rs index b3912f735..b6ba37017 100644 --- a/eth2/utils/bls/src/signature.rs +++ b/eth2/utils/bls/src/signature.rs @@ -81,9 +81,7 @@ impl Signature { for byte in bytes { if *byte != 0 { let raw_signature = RawSignature::from_bytes(&bytes).map_err(|_| { - DecodeError::BytesInvalid( - format!("Invalid Signature bytes: {:?}", bytes).to_string(), - ) + DecodeError::BytesInvalid(format!("Invalid Signature bytes: {:?}", bytes)) })?; return Ok(Signature { signature: raw_signature, diff --git a/eth2/utils/bls/src/signature_set.rs b/eth2/utils/bls/src/signature_set.rs index 27eebe1af..10ef6d65f 100644 --- a/eth2/utils/bls/src/signature_set.rs +++ b/eth2/utils/bls/src/signature_set.rs @@ -158,17 +158,17 @@ fn aggregate_public_keys<'a>(public_keys: &'a [Cow<'a, G1Point>]) -> G1Point { } pub trait G1Ref { - fn g1_ref<'a>(&'a self) -> Cow<'a, G1Point>; + fn g1_ref(&self) -> Cow<'_, G1Point>; } impl G1Ref for AggregatePublicKey { - fn g1_ref<'a>(&'a self) -> Cow<'a, G1Point> { + fn g1_ref(&self) -> Cow<'_, G1Point> { Cow::Borrowed(&self.as_raw().point) } } impl G1Ref for PublicKey { - fn g1_ref<'a>(&'a self) -> Cow<'a, G1Point> { + fn g1_ref(&self) -> Cow<'_, G1Point> { Cow::Borrowed(&self.as_raw().point) } } diff --git a/eth2/utils/cached_tree_hash/src/test.rs b/eth2/utils/cached_tree_hash/src/test.rs index 68173fd6a..0e3679d9f 100644 --- a/eth2/utils/cached_tree_hash/src/test.rs +++ b/eth2/utils/cached_tree_hash/src/test.rs @@ -131,17 +131,15 @@ fn variable_list_h256_test(leaves_and_skips: Vec<(u64, bool)>) -> for (end, (_, update_cache)) in leaves_and_skips.into_iter().enumerate() { list = VariableList::new(leaves[..end].to_vec()).unwrap(); - if update_cache { - if list + if update_cache + && list .recalculate_tree_hash_root(&mut cache) .unwrap() .as_bytes() != &list.tree_hash_root()[..] - { - return false; - } + { + return false; } } - true } diff --git a/eth2/utils/deposit_contract/src/lib.rs b/eth2/utils/deposit_contract/src/lib.rs index a235d0ff7..ba473032c 100644 --- a/eth2/utils/deposit_contract/src/lib.rs +++ b/eth2/utils/deposit_contract/src/lib.rs @@ -58,7 +58,7 @@ mod tests { let spec = &E::default_spec(); let keypair = generate_deterministic_keypair(42); - let deposit = get_deposit(keypair.clone(), spec); + let deposit = get_deposit(keypair, spec); let data = eth1_tx_data(&deposit).expect("should produce tx data"); diff --git a/eth2/utils/eth2_interop_keypairs/src/lib.rs b/eth2/utils/eth2_interop_keypairs/src/lib.rs index 6fa303553..94b0b207f 100644 --- a/eth2/utils/eth2_interop_keypairs/src/lib.rs +++ b/eth2/utils/eth2_interop_keypairs/src/lib.rs @@ -123,8 +123,7 @@ fn string_to_bytes(string: &str) -> Result, String> { /// Uses this as reference: /// https://github.com/ethereum/eth2.0-pm/blob/9a9dbcd95e2b8e10287797bd768014ab3d842e99/interop/mocked_start/keygen_10_validators.yaml pub fn keypairs_from_yaml_file(path: PathBuf) -> Result, String> { - let file = - File::open(path.clone()).map_err(|e| format!("Unable to open YAML key file: {}", e))?; + let file = File::open(path).map_err(|e| format!("Unable to open YAML key file: {}", e))?; serde_yaml::from_reader::<_, Vec>(file) .map_err(|e| format!("Could not parse YAML: {:?}", e))? diff --git a/eth2/utils/eth2_testnet_config/src/lib.rs b/eth2/utils/eth2_testnet_config/src/lib.rs index 57575fead..2c1d455dd 100644 --- a/eth2/utils/eth2_testnet_config/src/lib.rs +++ b/eth2/utils/eth2_testnet_config/src/lib.rs @@ -227,7 +227,7 @@ mod tests { let genesis_state = Some(BeaconState::new(42, eth1_data, spec)); let yaml_config = Some(YamlConfig::from_spec::(spec)); - do_test::(boot_enr, genesis_state.clone(), yaml_config.clone()); + do_test::(boot_enr, genesis_state, yaml_config); do_test::(None, None, None); } @@ -237,13 +237,13 @@ mod tests { yaml_config: Option, ) { let temp_dir = TempDir::new("eth2_testnet_test").expect("should create temp dir"); - let base_dir = PathBuf::from(temp_dir.path().join("my_testnet")); + let base_dir = temp_dir.path().join("my_testnet"); let deposit_contract_address = "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413".to_string(); let deposit_contract_deploy_block = 42; let testnet: Eth2TestnetConfig = Eth2TestnetConfig { - deposit_contract_address: deposit_contract_address.clone(), - deposit_contract_deploy_block: deposit_contract_deploy_block, + deposit_contract_address, + deposit_contract_deploy_block, boot_enr, genesis_state, yaml_config, diff --git a/eth2/utils/lighthouse_metrics/src/lib.rs b/eth2/utils/lighthouse_metrics/src/lib.rs index 7c229b592..c7d312a4f 100644 --- a/eth2/utils/lighthouse_metrics/src/lib.rs +++ b/eth2/utils/lighthouse_metrics/src/lib.rs @@ -1,3 +1,4 @@ +#![allow(clippy::needless_doctest_main)] //! A wrapper around the `prometheus` crate that provides a global, `lazy_static` metrics registry //! and functions to add and use the following components (more info at //! [Prometheus docs](https://prometheus.io/docs/concepts/metric_types/)): diff --git a/eth2/utils/ssz/src/decode.rs b/eth2/utils/ssz/src/decode.rs index abcebda07..db80c2409 100644 --- a/eth2/utils/ssz/src/decode.rs +++ b/eth2/utils/ssz/src/decode.rs @@ -99,7 +99,7 @@ impl<'a> SszDecoderBuilder<'a> { let previous_offset = self .offsets .last() - .and_then(|o| Some(o.offset)) + .map(|o| o.offset) .unwrap_or_else(|| BYTES_PER_LENGTH_OFFSET); if (previous_offset > offset) || (offset > self.bytes.len()) { @@ -179,7 +179,7 @@ impl<'a> SszDecoderBuilder<'a> { /// b: Vec, /// } /// -/// fn main() { +/// fn ssz_decoding_example() { /// let foo = Foo { /// a: 42, /// b: vec![1, 3, 3, 7] diff --git a/eth2/utils/ssz/src/decode/impls.rs b/eth2/utils/ssz/src/decode/impls.rs index 1a12e7e19..39b7fa3c1 100644 --- a/eth2/utils/ssz/src/decode/impls.rs +++ b/eth2/utils/ssz/src/decode/impls.rs @@ -207,9 +207,10 @@ impl Decode for bool { match bytes[0] { 0b0000_0000 => Ok(false), 0b0000_0001 => Ok(true), - _ => Err(DecodeError::BytesInvalid( - format!("Out-of-range for boolean: {}", bytes[0]).to_string(), - )), + _ => Err(DecodeError::BytesInvalid(format!( + "Out-of-range for boolean: {}", + bytes[0] + ))), } } } diff --git a/eth2/utils/ssz/src/encode.rs b/eth2/utils/ssz/src/encode.rs index 52b3d9bfd..91281e050 100644 --- a/eth2/utils/ssz/src/encode.rs +++ b/eth2/utils/ssz/src/encode.rs @@ -64,7 +64,7 @@ pub trait Encode { /// b: Vec, /// } /// -/// fn main() { +/// fn ssz_encode_example() { /// let foo = Foo { /// a: 42, /// b: vec![1, 3, 3, 7] diff --git a/eth2/utils/ssz/src/lib.rs b/eth2/utils/ssz/src/lib.rs index 115633889..89e707395 100644 --- a/eth2/utils/ssz/src/lib.rs +++ b/eth2/utils/ssz/src/lib.rs @@ -17,7 +17,7 @@ //! b: Vec, //! } //! -//! fn main() { +//! fn ssz_encode_decode_example() { //! let foo = Foo { //! a: 42, //! b: vec![1, 3, 3, 7] diff --git a/eth2/utils/ssz/tests/tests.rs b/eth2/utils/ssz/tests/tests.rs index 26f2f53ef..f82e5d6e3 100644 --- a/eth2/utils/ssz/tests/tests.rs +++ b/eth2/utils/ssz/tests/tests.rs @@ -2,6 +2,7 @@ use ethereum_types::H256; use ssz::{Decode, DecodeError, Encode}; use ssz_derive::{Decode, Encode}; +#[allow(clippy::zero_prefixed_literal)] mod round_trip { use super::*; diff --git a/eth2/utils/ssz_types/src/bitfield.rs b/eth2/utils/ssz_types/src/bitfield.rs index cc01d40c7..d18267ee2 100644 --- a/eth2/utils/ssz_types/src/bitfield.rs +++ b/eth2/utils/ssz_types/src/bitfield.rs @@ -739,6 +739,7 @@ mod bitvector { } #[cfg(test)] +#[allow(clippy::cognitive_complexity)] mod bitlist { use super::*; use crate::BitList; @@ -937,7 +938,7 @@ mod bitlist { fn test_set_unset(num_bits: usize) { let mut bitfield = BitList1024::with_capacity(num_bits).unwrap(); - for i in 0..num_bits + 1 { + for i in 0..=num_bits { if i < num_bits { // Starts as false assert_eq!(bitfield.get(i), Ok(false)); @@ -1023,10 +1024,7 @@ mod bitlist { vec![0b1111_1111, 0b0000_0000] ); bitfield.set(8, true).unwrap(); - assert_eq!( - bitfield.clone().into_raw_bytes(), - vec![0b1111_1111, 0b0000_0001] - ); + assert_eq!(bitfield.into_raw_bytes(), vec![0b1111_1111, 0b0000_0001]); } #[test] diff --git a/eth2/utils/ssz_types/src/fixed_vector.rs b/eth2/utils/ssz_types/src/fixed_vector.rs index f9c896331..eb63d6d39 100644 --- a/eth2/utils/ssz_types/src/fixed_vector.rs +++ b/eth2/utils/ssz_types/src/fixed_vector.rs @@ -261,15 +261,15 @@ mod test { #[test] fn new() { let vec = vec![42; 5]; - let fixed: Result, _> = FixedVector::new(vec.clone()); + let fixed: Result, _> = FixedVector::new(vec); assert!(fixed.is_err()); let vec = vec![42; 3]; - let fixed: Result, _> = FixedVector::new(vec.clone()); + let fixed: Result, _> = FixedVector::new(vec); assert!(fixed.is_err()); let vec = vec![42; 4]; - let fixed: Result, _> = FixedVector::new(vec.clone()); + let fixed: Result, _> = FixedVector::new(vec); assert!(fixed.is_ok()); } @@ -299,7 +299,7 @@ mod test { assert_eq!(&fixed[..], &vec![42, 42, 42, 0][..]); let vec = vec![]; - let fixed: FixedVector = FixedVector::from(vec.clone()); + let fixed: FixedVector = FixedVector::from(vec); assert_eq!(&fixed[..], &vec![0, 0, 0, 0][..]); } diff --git a/eth2/utils/ssz_types/src/variable_list.rs b/eth2/utils/ssz_types/src/variable_list.rs index feb656745..c03331512 100644 --- a/eth2/utils/ssz_types/src/variable_list.rs +++ b/eth2/utils/ssz_types/src/variable_list.rs @@ -247,15 +247,15 @@ mod test { #[test] fn new() { let vec = vec![42; 5]; - let fixed: Result, _> = VariableList::new(vec.clone()); + let fixed: Result, _> = VariableList::new(vec); assert!(fixed.is_err()); let vec = vec![42; 3]; - let fixed: Result, _> = VariableList::new(vec.clone()); + let fixed: Result, _> = VariableList::new(vec); assert!(fixed.is_ok()); let vec = vec![42; 4]; - let fixed: Result, _> = VariableList::new(vec.clone()); + let fixed: Result, _> = VariableList::new(vec); assert!(fixed.is_ok()); } @@ -285,7 +285,7 @@ mod test { assert_eq!(&fixed[..], &vec![42, 42, 42][..]); let vec = vec![]; - let fixed: VariableList = VariableList::from(vec.clone()); + let fixed: VariableList = VariableList::from(vec); assert_eq!(&fixed[..], &vec![][..]); } diff --git a/eth2/utils/tree_hash_derive/src/lib.rs b/eth2/utils/tree_hash_derive/src/lib.rs index ab61cf08e..b15e9ceb2 100644 --- a/eth2/utils/tree_hash_derive/src/lib.rs +++ b/eth2/utils/tree_hash_derive/src/lib.rs @@ -46,7 +46,7 @@ fn get_hashable_fields_and_their_caches<'a>( /// /// Return `Some(cache_field_name)` if the field has a cached tree hash attribute, /// or `None` otherwise. -fn get_cache_field_for<'a>(field: &'a syn::Field) -> Option { +fn get_cache_field_for(field: &syn::Field) -> Option { use syn::{MetaList, NestedMeta}; let parsed_attrs = cached_tree_hash_attr_metas(&field.attrs); diff --git a/lcli/src/deploy_deposit_contract.rs b/lcli/src/deploy_deposit_contract.rs index f40849c35..76662309a 100644 --- a/lcli/src/deploy_deposit_contract.rs +++ b/lcli/src/deploy_deposit_contract.rs @@ -94,12 +94,12 @@ pub fn run(mut env: Environment, matches: &ArgMatches) -> Result< info!("Writing config to {:?}", output_dir); - let mut spec = lighthouse_testnet_spec(env.core_context().eth2_config.spec.clone()); + let mut spec = lighthouse_testnet_spec(env.core_context().eth2_config.spec); spec.min_genesis_time = min_genesis_time; spec.min_genesis_active_validator_count = min_genesis_active_validator_count; let testnet_config: Eth2TestnetConfig = Eth2TestnetConfig { - deposit_contract_address: format!("{}", deposit_contract.address()), + deposit_contract_address: deposit_contract.address(), deposit_contract_deploy_block: deploy_block.as_u64(), boot_enr: None, genesis_state: None, @@ -152,7 +152,7 @@ pub fn parse_password(matches: &ArgMatches) -> Result, String> { }) .map(|password| { // Trim the linefeed from the end. - if password.ends_with("\n") { + if password.ends_with('\n') { password[0..password.len() - 1].to_string() } else { password diff --git a/lcli/src/refund_deposit_contract.rs b/lcli/src/refund_deposit_contract.rs index 1684a5d89..d413b7f5c 100644 --- a/lcli/src/refund_deposit_contract.rs +++ b/lcli/src/refund_deposit_contract.rs @@ -110,7 +110,7 @@ pub fn run(mut env: Environment, matches: &ArgMatches) -> Result< env.runtime() .block_on(future) - .map_err(|()| format!("Failed to send transaction"))?; + .map_err(|()| "Failed to send transaction".to_string())?; Ok(()) } diff --git a/lcli/src/transition_blocks.rs b/lcli/src/transition_blocks.rs index 23e19acae..fb583eeb5 100644 --- a/lcli/src/transition_blocks.rs +++ b/lcli/src/transition_blocks.rs @@ -34,8 +34,8 @@ pub fn run_transition_blocks(matches: &ArgMatches) -> Result<(), Str let post_state = do_transition(pre_state, block)?; - let mut output_file = File::create(output_path.clone()) - .map_err(|e| format!("Unable to create output file: {:?}", e))?; + let mut output_file = + File::create(output_path).map_err(|e| format!("Unable to create output file: {:?}", e))?; output_file .write_all(&post_state.as_ssz_bytes()) diff --git a/tests/beacon_chain_sim/src/main.rs b/tests/beacon_chain_sim/src/main.rs index 4eea1cd24..e03232e01 100644 --- a/tests/beacon_chain_sim/src/main.rs +++ b/tests/beacon_chain_sim/src/main.rs @@ -78,7 +78,7 @@ fn async_sim( let spec = &mut env.eth2_config.spec; - spec.milliseconds_per_slot = spec.milliseconds_per_slot / speed_up_factor; + spec.milliseconds_per_slot /= speed_up_factor; spec.eth1_follow_distance = 16; spec.seconds_per_day = eth1_block_time.as_secs() * spec.eth1_follow_distance * 2; spec.min_genesis_time = 0; diff --git a/validator_client/src/attestation_service.rs b/validator_client/src/attestation_service.rs index 45cd99798..cf3507cd7 100644 --- a/validator_client/src/attestation_service.rs +++ b/validator_client/src/attestation_service.rs @@ -194,8 +194,9 @@ impl AttestationService { .into_iter() .for_each(|duty| { if let Some(committee_index) = duty.attestation_committee_index { - let validator_duties = - committee_indices.entry(committee_index).or_insert(vec![]); + let validator_duties = committee_indices + .entry(committee_index) + .or_insert_with(|| vec![]); validator_duties.push(duty); } diff --git a/validator_client/src/duties_service.rs b/validator_client/src/duties_service.rs index ea1f1841a..eb91f0a41 100644 --- a/validator_client/src/duties_service.rs +++ b/validator_client/src/duties_service.rs @@ -443,7 +443,7 @@ impl DutiesService { /// Attempt to download the duties of all managed validators for the given `epoch`. fn update_epoch(self, epoch: Epoch) -> impl Future { let service_1 = self.clone(); - let service_2 = self.clone(); + let service_2 = self; let pubkeys = service_1.validator_store.voting_pubkeys(); service_1 diff --git a/validator_client/src/validator_directory.rs b/validator_client/src/validator_directory.rs index 15d2edd9a..66f58b3b1 100644 --- a/validator_client/src/validator_directory.rs +++ b/validator_client/src/validator_directory.rs @@ -384,12 +384,11 @@ mod tests { "withdrawal keypair should be as expected" ); assert!( - created_dir + !created_dir .deposit_data .clone() .expect("should have data") - .len() - > 0, + .is_empty(), "should have some deposit data" );