From 07745db249e8212e278a56a66c3abf6b13635109 Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Sat, 9 Feb 2019 09:25:11 -0800 Subject: [PATCH 1/3] Rename enum Config to enum PohServiceConfig --- src/banking_stage.rs | 6 +++--- src/poh_service.rs | 31 ++++++++++++++----------------- src/tpu.rs | 6 +++--- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/banking_stage.rs b/src/banking_stage.rs index d2a34cbd3c0b6c..bd0f1ff7296260 100644 --- a/src/banking_stage.rs +++ b/src/banking_stage.rs @@ -8,7 +8,7 @@ use crate::counter::Counter; use crate::entry::Entry; use crate::packet::Packets; use crate::poh_recorder::{PohRecorder, PohRecorderError}; -use crate::poh_service::{Config, PohService}; +use crate::poh_service::{PohService, PohServiceConfig}; use crate::result::{Error, Result}; use crate::service::Service; use crate::sigverify_stage::VerifiedPackets; @@ -51,7 +51,7 @@ impl BankingStage { pub fn new( bank: &Arc, verified_receiver: Receiver, - config: Config, + config: PohServiceConfig, last_entry_id: &Hash, max_tick_height: u64, leader_id: Pubkey, @@ -338,7 +338,7 @@ mod tests { let (banking_stage, entry_receiver) = BankingStage::new( &bank, verified_receiver, - Config::Sleep(Duration::from_millis(1)), + PohServiceConfig::Sleep(Duration::from_millis(1)), &bank.last_id(), std::u64::MAX, genesis_block.bootstrap_leader_id, diff --git a/src/poh_service.rs b/src/poh_service.rs index 5f59f351a6ad58..82f597ef87d09f 100644 --- a/src/poh_service.rs +++ b/src/poh_service.rs @@ -15,7 +15,7 @@ use std::time::Duration; pub const NUM_TICKS_PER_SECOND: usize = 10; #[derive(Copy, Clone)] -pub enum Config { +pub enum PohServiceConfig { /// * `Tick` - Run full PoH thread. Tick is a rough estimate of how many hashes to roll before /// transmitting a new entry. Tick(usize), @@ -24,10 +24,10 @@ pub enum Config { Sleep(Duration), } -impl Default for Config { - fn default() -> Config { +impl Default for PohServiceConfig { + fn default() -> PohServiceConfig { // TODO: Change this to Tick to enable PoH - Config::Sleep(Duration::from_millis(1000 / NUM_TICKS_PER_SECOND as u64)) + PohServiceConfig::Sleep(Duration::from_millis(1000 / NUM_TICKS_PER_SECOND as u64)) } } @@ -48,7 +48,7 @@ impl PohService { pub fn new( poh_recorder: PohRecorder, - config: Config, + config: PohServiceConfig, to_validator_sender: TpuRotationSender, ) -> Self { // PohService is a headless producer, so when it exits it should notify the banking stage. @@ -80,14 +80,14 @@ impl PohService { fn tick_producer( poh: &mut PohRecorder, - config: Config, + config: PohServiceConfig, poh_exit: &AtomicBool, to_validator_sender: &TpuRotationSender, ) -> Result<()> { let max_tick_height = poh.max_tick_height(); loop { match config { - Config::Tick(num) => { + PohServiceConfig::Tick(num) => { for _ in 1..num { let res = poh.hash(); if let Err(e) = res { @@ -99,7 +99,7 @@ impl PohService { } } } - Config::Sleep(duration) => { + PohServiceConfig::Sleep(duration) => { sleep(duration); } } @@ -128,18 +128,12 @@ impl Service for PohService { #[cfg(test)] mod tests { - use super::{Config, PohService}; + use super::*; use crate::bank::Bank; use crate::genesis_block::GenesisBlock; - use crate::poh_recorder::PohRecorder; - use crate::result::Result; - use crate::service::Service; use crate::test_tx::test_tx; use solana_sdk::hash::hash; - use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::channel; - use std::sync::Arc; - use std::thread::{Builder, JoinHandle}; #[test] fn test_poh_service() { @@ -173,8 +167,11 @@ mod tests { const HASHES_PER_TICK: u64 = 2; let (sender, _) = channel(); - let poh_service = - PohService::new(poh_recorder, Config::Tick(HASHES_PER_TICK as usize), sender); + let poh_service = PohService::new( + poh_recorder, + PohServiceConfig::Tick(HASHES_PER_TICK as usize), + sender, + ); // get some events let mut hashes = 0; diff --git a/src/tpu.rs b/src/tpu.rs index 2d8dabe4be1abc..6991192aedf58b 100644 --- a/src/tpu.rs +++ b/src/tpu.rs @@ -7,7 +7,7 @@ use crate::broadcast_service::BroadcastService; use crate::cluster_info::ClusterInfo; use crate::cluster_info_vote_listener::ClusterInfoVoteListener; use crate::fetch_stage::FetchStage; -use crate::poh_service::Config; +use crate::poh_service::PohServiceConfig; use crate::service::Service; use crate::sigverify_stage::SigVerifyStage; use crate::streamer::BlobSender; @@ -77,7 +77,7 @@ impl Tpu { #[allow(clippy::too_many_arguments)] pub fn new( bank: &Arc, - tick_duration: Config, + tick_duration: PohServiceConfig, transactions_sockets: Vec, broadcast_socket: UdpSocket, cluster_info: Arc>, @@ -143,7 +143,7 @@ impl Tpu { pub fn switch_to_leader( &mut self, bank: &Arc, - tick_duration: Config, + tick_duration: PohServiceConfig, transactions_sockets: Vec, broadcast_socket: UdpSocket, cluster_info: Arc>, From 8418f14223c1836fcace37a9ee826565cc3245b2 Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Sat, 9 Feb 2019 09:20:43 -0800 Subject: [PATCH 2/3] Purge Default::default() --- bench-tps/src/bench.rs | 2 +- benches/banking_stage.rs | 5 +++-- benches/bloom.rs | 4 ++-- programs/native/storage/src/lib.rs | 6 +++--- sdk/src/budget_transaction.rs | 4 ++-- sdk/src/system_transaction.rs | 2 +- sdk/src/transaction.rs | 10 +++++----- src/banking_stage.rs | 10 +++++----- src/bloom.rs | 2 +- src/fullnode.rs | 9 +++++---- src/leader_scheduler.rs | 2 +- src/packet.rs | 2 +- src/rpc_pubsub.rs | 16 +++++++++------- src/sigverify.rs | 2 +- src/status_cache.rs | 14 +++++++------- src/window_service.rs | 9 ++------- 16 files changed, 49 insertions(+), 50 deletions(-) diff --git a/bench-tps/src/bench.rs b/bench-tps/src/bench.rs index 143367e0e76b69..3aa402b7639be4 100644 --- a/bench-tps/src/bench.rs +++ b/bench-tps/src/bench.rs @@ -342,7 +342,7 @@ pub fn fund_keys(client: &mut ThinClient, source: &Keypair, dests: &[Keypair], t .map(|(k, m)| { ( k.clone(), - SystemTransaction::new_move_many(k, &m, Default::default(), 0), + SystemTransaction::new_move_many(k, &m, Hash::default(), 0), ) }) .collect(); diff --git a/benches/banking_stage.rs b/benches/banking_stage.rs index 75e6d062315747..9d39cb8a481c81 100644 --- a/benches/banking_stage.rs +++ b/benches/banking_stage.rs @@ -10,6 +10,7 @@ use solana::entry::Entry; use solana::genesis_block::GenesisBlock; use solana::last_id_queue::MAX_ENTRY_IDS; use solana::packet::to_packets_chunked; +use solana::poh_service::PohServiceConfig; use solana_sdk::hash::hash; use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::{KeypairUtil, Signature}; @@ -103,7 +104,7 @@ fn bench_banking_stage_multi_accounts(bencher: &mut Bencher) { let (_stage, signal_receiver) = BankingStage::new( &bank, verified_receiver, - Default::default(), + PohServiceConfig::default(), &genesis_block.last_id(), std::u64::MAX, genesis_block.bootstrap_leader_id, @@ -212,7 +213,7 @@ fn bench_banking_stage_multi_programs(bencher: &mut Bencher) { let (_stage, signal_receiver) = BankingStage::new( &bank, verified_receiver, - Default::default(), + PohServiceConfig::default(), &genesis_block.last_id(), std::u64::MAX, genesis_block.bootstrap_leader_id, diff --git a/benches/bloom.rs b/benches/bloom.rs index ab52b133444c88..851b9064dc2fbf 100644 --- a/benches/bloom.rs +++ b/benches/bloom.rs @@ -15,7 +15,7 @@ use test::Bencher; #[ignore] fn bench_bits_set(bencher: &mut Bencher) { let mut bits: BitVec = BitVec::new_fill(false, 38_340_234 as u64); - let mut hasher: FnvHasher = Default::default(); + let mut hasher = FnvHasher::default(); bencher.iter(|| { let idx = hasher.finish() % bits.len(); @@ -30,7 +30,7 @@ fn bench_bits_set(bencher: &mut Bencher) { #[ignore] fn bench_bits_set_hasher(bencher: &mut Bencher) { let bits: BitVec = BitVec::new_fill(false, 38_340_234 as u64); - let mut hasher: FnvHasher = Default::default(); + let mut hasher = FnvHasher::default(); bencher.iter(|| { let idx = hasher.finish() % bits.len(); diff --git a/programs/native/storage/src/lib.rs b/programs/native/storage/src/lib.rs index e436b35ea76c0a..397956830e7627 100644 --- a/programs/native/storage/src/lib.rs +++ b/programs/native/storage/src/lib.rs @@ -226,7 +226,7 @@ mod test { #[test] fn test_storage_tx() { let keypair = Keypair::new(); - let mut accounts = [(keypair.pubkey(), Default::default())]; + let mut accounts = [(keypair.pubkey(), Account::default())]; let mut keyed_accounts = create_keyed_accounts(&mut accounts); assert!(entrypoint(&id(), &mut keyed_accounts, &[], 42).is_err()); } @@ -258,7 +258,7 @@ mod test { #[test] fn test_invalid_accounts_len() { let keypair = Keypair::new(); - let mut accounts = [Default::default()]; + let mut accounts = [Account::default()]; let tx = StorageTransaction::new_mining_proof( &keypair, @@ -269,7 +269,7 @@ mod test { ); assert!(test_transaction(&tx, &mut accounts).is_err()); - let mut accounts = [Default::default(), Default::default(), Default::default()]; + let mut accounts = [Account::default(), Account::default(), Account::default()]; assert!(test_transaction(&tx, &mut accounts).is_err()); } diff --git a/sdk/src/budget_transaction.rs b/sdk/src/budget_transaction.rs index 7b5c9da0442854..3524c016540169 100644 --- a/sdk/src/budget_transaction.rs +++ b/sdk/src/budget_transaction.rs @@ -232,13 +232,13 @@ mod tests { fn test_serialize_claim() { let expr = BudgetExpr::Pay(Payment { tokens: 0, - to: Default::default(), + to: Pubkey::default(), }); let instruction = Instruction::NewBudget(expr); let instructions = vec![transaction::Instruction::new(0, &instruction, vec![])]; let claim0 = Transaction { account_keys: vec![], - last_id: Default::default(), + last_id: Hash::default(), signatures: vec![], program_ids: vec![], instructions, diff --git a/sdk/src/system_transaction.rs b/sdk/src/system_transaction.rs index 1827f31abe12c6..4ee52a4c8d9a6b 100644 --- a/sdk/src/system_transaction.rs +++ b/sdk/src/system_transaction.rs @@ -133,7 +133,7 @@ mod tests { let t2 = Keypair::new(); let moves = vec![(t1.pubkey(), 1), (t2.pubkey(), 2)]; - let tx = SystemTransaction::new_move_many(&from, &moves, Default::default(), 0); + let tx = SystemTransaction::new_move_many(&from, &moves, Hash::default(), 0); assert_eq!(tx.account_keys[0], from.pubkey()); assert_eq!(tx.account_keys[1], t1.pubkey()); assert_eq!(tx.account_keys[2], t2.pubkey()); diff --git a/sdk/src/transaction.rs b/sdk/src/transaction.rs index dd33cb49448f60..07bf0edcc3438a 100644 --- a/sdk/src/transaction.rs +++ b/sdk/src/transaction.rs @@ -412,7 +412,7 @@ mod tests { let tx = Transaction::new_with_instructions( &[&key], &[key1, key2], - Default::default(), + Hash::default(), 0, vec![prog1, prog2], instructions, @@ -447,7 +447,7 @@ mod tests { let tx = Transaction::new_with_instructions( &[&key], &[], - Default::default(), + Hash::default(), 0, vec![], instructions, @@ -461,12 +461,12 @@ mod tests { let tx = Transaction::new_with_instructions( &[&key], &[], - Default::default(), + Hash::default(), 0, - vec![Default::default()], + vec![Pubkey::default()], instructions, ); - assert_eq!(*tx.program_id(0), Default::default()); + assert_eq!(*tx.program_id(0), Pubkey::default()); assert!(!tx.verify_refs()); } diff --git a/src/banking_stage.rs b/src/banking_stage.rs index bd0f1ff7296260..86cda6b51b8f32 100644 --- a/src/banking_stage.rs +++ b/src/banking_stage.rs @@ -293,7 +293,7 @@ mod tests { let (banking_stage, _entry_receiver) = BankingStage::new( &bank, verified_receiver, - Default::default(), + PohServiceConfig::default(), &bank.last_id(), std::u64::MAX, genesis_block.bootstrap_leader_id, @@ -315,7 +315,7 @@ mod tests { let (banking_stage, entry_receiver) = BankingStage::new( &bank, verified_receiver, - Default::default(), + PohServiceConfig::default(), &bank.last_id(), std::u64::MAX, genesis_block.bootstrap_leader_id, @@ -367,7 +367,7 @@ mod tests { let (banking_stage, entry_receiver) = BankingStage::new( &bank, verified_receiver, - Default::default(), + PohServiceConfig::default(), &bank.last_id(), std::u64::MAX, genesis_block.bootstrap_leader_id, @@ -425,7 +425,7 @@ mod tests { let (banking_stage, entry_receiver) = BankingStage::new( &bank, verified_receiver, - Default::default(), + PohServiceConfig::default(), &bank.last_id(), std::u64::MAX, genesis_block.bootstrap_leader_id, @@ -494,7 +494,7 @@ mod tests { let (banking_stage, _entry_receiver) = BankingStage::new( &bank, verified_receiver, - Default::default(), + PohServiceConfig::default(), &bank.last_id(), max_tick_height, genesis_block.bootstrap_leader_id, diff --git a/src/bloom.rs b/src/bloom.rs index c6b943b6bdc845..b81c395f854d0b 100644 --- a/src/bloom.rs +++ b/src/bloom.rs @@ -25,7 +25,7 @@ impl Bloom { Bloom { keys, bits, - _phantom: Default::default(), + _phantom: PhantomData::default(), } } /// create filter optimal for num size given the `false_rate` diff --git a/src/fullnode.rs b/src/fullnode.rs index 535b2514812354..b09ad1f4e020a9 100644 --- a/src/fullnode.rs +++ b/src/fullnode.rs @@ -7,6 +7,7 @@ use crate::counter::Counter; use crate::genesis_block::GenesisBlock; use crate::gossip_service::GossipService; use crate::leader_scheduler::LeaderSchedulerConfig; +use crate::poh_service::PohServiceConfig; use crate::rpc::JsonRpcService; use crate::rpc_pubsub::PubSubService; use crate::service::Service; @@ -77,8 +78,8 @@ impl Default for FullnodeConfig { voting_disabled: false, entry_stream: None, storage_rotate_count: NUM_HASHES_FOR_STORAGE_ROTATE, - leader_scheduler_config: Default::default(), - ledger_config: Default::default(), + leader_scheduler_config: LeaderSchedulerConfig::default(), + ledger_config: BlocktreeConfig::default(), } } } @@ -271,7 +272,7 @@ impl Fullnode { ); let tpu = Tpu::new( &Arc::new(bank.copy_for_tpu()), - Default::default(), + PohServiceConfig::default(), node.sockets .tpu .iter() @@ -389,7 +390,7 @@ impl Fullnode { self.to_validator_receiver = to_validator_receiver; self.node_services.tpu.switch_to_leader( &Arc::new(self.bank.copy_for_tpu()), - Default::default(), + PohServiceConfig::default(), self.tpu_sockets .iter() .map(|s| s.try_clone().expect("Failed to clone TPU sockets")) diff --git a/src/leader_scheduler.rs b/src/leader_scheduler.rs index 2351c85d55ef45..46b4a929ae8b08 100644 --- a/src/leader_scheduler.rs +++ b/src/leader_scheduler.rs @@ -399,7 +399,7 @@ impl LeaderScheduler { impl Default for LeaderScheduler { fn default() -> Self { - let config = Default::default(); + let config = LeaderSchedulerConfig::default(); Self::new(&config) } } diff --git a/src/packet.rs b/src/packet.rs index e502b09ea3f1ad..682d98d87c24fe 100644 --- a/src/packet.rs +++ b/src/packet.rs @@ -224,7 +224,7 @@ pub fn to_packets_chunked(xs: &[T], chunks: usize) -> Vec>>>; +type RpcSignatureSubscriptions = + RwLock>>>; pub struct RpcSubscriptions { - account_subscriptions: RwLock>>>, - signature_subscriptions: - RwLock>>>, + account_subscriptions: RpcAccountSubscriptions, + signature_subscriptions: RpcSignatureSubscriptions, } impl Default for RpcSubscriptions { fn default() -> Self { RpcSubscriptions { - account_subscriptions: Default::default(), - signature_subscriptions: Default::default(), + account_subscriptions: RpcAccountSubscriptions::default(), + signature_subscriptions: RpcSignatureSubscriptions::default(), } } } @@ -260,9 +262,9 @@ struct RpcSolPubSubImpl { impl RpcSolPubSubImpl { fn new(bank: Arc>) -> Self { RpcSolPubSubImpl { - uid: Default::default(), + uid: Arc::new(atomic::AtomicUsize::default()), bank, - subscription: Arc::new(Default::default()), + subscription: Arc::new(RpcSubscriptions::default()), } } diff --git a/src/sigverify.rs b/src/sigverify.rs index 2dc0f277aa33ab..bad2e83008289e 100644 --- a/src/sigverify.rs +++ b/src/sigverify.rs @@ -442,7 +442,7 @@ mod tests { .write() .unwrap() .packets - .resize(0, Default::default()); + .resize(0, Packet::default()); for _ in 0..num_packets_per_batch { packets.write().unwrap().packets.push(packet.clone()); } diff --git a/src/status_cache.rs b/src/status_cache.rs index d182a6ffdef157..e1fa0e9a1c3f78 100644 --- a/src/status_cache.rs +++ b/src/status_cache.rs @@ -135,7 +135,7 @@ mod tests { #[test] fn test_has_signature() { - let sig = Default::default(); + let sig = Signature::default(); let last_id = hash(Hash::default().as_ref()); let mut status_cache = BankStatusCache::new(&last_id); assert_eq!(status_cache.has_signature(&sig), false); @@ -147,7 +147,7 @@ mod tests { #[test] fn test_has_signature_checkpoint() { - let sig = Default::default(); + let sig = Signature::default(); let last_id = hash(Hash::default().as_ref()); let mut first = BankStatusCache::new(&last_id); first.add(&sig); @@ -164,7 +164,7 @@ mod tests { #[test] fn test_has_signature_merged1() { - let sig = Default::default(); + let sig = Signature::default(); let last_id = hash(Hash::default().as_ref()); let mut first = BankStatusCache::new(&last_id); first.add(&sig); @@ -178,7 +178,7 @@ mod tests { #[test] fn test_has_signature_merged2() { - let sig = Default::default(); + let sig = Signature::default(); let last_id = hash(Hash::default().as_ref()); let mut first = BankStatusCache::new(&last_id); first.add(&sig); @@ -192,7 +192,7 @@ mod tests { #[test] fn test_failure_status() { - let sig = Default::default(); + let sig = Signature::default(); let last_id = hash(Hash::default().as_ref()); let mut first = StatusCache::new(&last_id); first.add(&sig); @@ -206,7 +206,7 @@ mod tests { #[test] fn test_clear_signatures() { - let sig = Default::default(); + let sig = Signature::default(); let last_id = hash(Hash::default().as_ref()); let mut first = StatusCache::new(&last_id); first.add(&sig); @@ -222,7 +222,7 @@ mod tests { } #[test] fn test_clear_signatures_all() { - let sig = Default::default(); + let sig = Signature::default(); let last_id = hash(Hash::default().as_ref()); let mut first = StatusCache::new(&last_id); first.add(&sig); diff --git a/src/window_service.rs b/src/window_service.rs index e77f719c94827b..b301192b9c3407 100644 --- a/src/window_service.rs +++ b/src/window_service.rs @@ -290,13 +290,8 @@ mod test { leader_node.sockets.tvu.into_iter().map(Arc::new).collect(); let t_responder = responder("window_send_test", blob_sockets[0].clone(), r_responder); let mut msgs = Vec::new(); - let blobs = make_consecutive_blobs( - &me_id, - 14u64, - 0, - Default::default(), - &leader_node.info.gossip, - ); + let blobs = + make_consecutive_blobs(&me_id, 14u64, 0, Hash::default(), &leader_node.info.gossip); for v in 0..10 { let i = 9 - v; From 58ea538be1e24d51df08e63c588ec300c9e07998 Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Sat, 9 Feb 2019 09:13:35 -0800 Subject: [PATCH 3/3] Ban Default::default() --- ci/nits.sh | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/ci/nits.sh b/ci/nits.sh index 49c9aebbe071f4..12b58f6665b61b 100755 --- a/ci/nits.sh +++ b/ci/nits.sh @@ -7,7 +7,7 @@ set -e cd "$(dirname "$0")/.." source ci/_ -# please don't print from --lib... +# Logging hygiene: Please don't print from --lib, use the `log` crate instead declare prints=( 'print!' 'println!' @@ -15,7 +15,16 @@ declare prints=( 'eprintln!' ) -if _ git grep "${prints[@]/#/-e }" src -then +if _ git grep "${prints[@]/#/-e }" src; then exit 1 fi + + +# Code readability: please be explicit about the type instead of using +# Default::default() +# +# Ref: https://github.com/solana-labs/solana/issues/2630 +if _ git grep 'Default::default()' -- '*.rs'; then + exit 1 +fi +