Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Commit

Permalink
Clippy cleanup for all targets and nighly rust (also support 1.44.0) (#…
Browse files Browse the repository at this point in the history
…10445)

* address warnings from 'rustup run beta cargo clippy --workspace'

minor refactoring in:
- cli/src/cli.rs
- cli/src/offline/blockhash_query.rs
- logger/src/lib.rs
- runtime/src/accounts_db.rs

expect some performance improvement AccountsDB::clean_accounts()

* address warnings from 'rustup run beta cargo clippy --workspace --tests'

* address warnings from 'rustup run nightly cargo clippy --workspace --all-targets'

* rustfmt

* fix warning stragglers

* properly fix clippy warnings test_vote_subscribe()
replace ref-to-arc with ref parameters where arc not cloned

* Remove lock around JsonRpcRequestProcessor (#10417)

automerge

* make ancestors parameter optional to avoid forcing construction of empty hash maps

Co-authored-by: Greg Fitzgerald <greg@solana.com>
  • Loading branch information
svenski123 and garious authored Jun 9, 2020
1 parent fa3a6c5 commit e23340d
Show file tree
Hide file tree
Showing 63 changed files with 259 additions and 309 deletions.
2 changes: 1 addition & 1 deletion banking-bench/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ fn main() {
bank.clear_signatures();
}

let mut verified: Vec<_> = to_packets_chunked(&transactions.clone(), packets_per_chunk);
let mut verified: Vec<_> = to_packets_chunked(&transactions, packets_per_chunk);
let ledger_path = get_tmp_ledger_path!();
{
let blockstore = Arc::new(
Expand Down
29 changes: 11 additions & 18 deletions cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -754,25 +754,18 @@ pub fn parse_command(
("airdrop", Some(matches)) => {
let faucet_port = matches
.value_of("faucet_port")
.unwrap()
.ok_or_else(|| CliError::BadParameter("Missing faucet port".to_string()))?
.parse()
.or_else(|err| {
Err(CliError::BadParameter(format!(
"Invalid faucet port: {}",
err
)))
})?;

let faucet_host = if let Some(faucet_host) = matches.value_of("faucet_host") {
Some(solana_net_utils::parse_host(faucet_host).or_else(|err| {
Err(CliError::BadParameter(format!(
"Invalid faucet host: {}",
err
)))
})?)
} else {
None
};
.map_err(|err| CliError::BadParameter(format!("Invalid faucet port: {}", err)))?;

let faucet_host = matches
.value_of("faucet_host")
.map(|faucet_host| {
solana_net_utils::parse_host(faucet_host).map_err(|err| {
CliError::BadParameter(format!("Invalid faucet host: {}", err))
})
})
.transpose()?;
let pubkey = pubkey_of_signer(matches, "to", wallet_manager)?;
let signers = if pubkey.is_some() {
vec![]
Expand Down
19 changes: 8 additions & 11 deletions cli/src/cluster_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ pub fn process_get_epoch_info(
commitment_config: CommitmentConfig,
) -> ProcessResult {
let epoch_info: CliEpochInfo = rpc_client
.get_epoch_info_with_commitment(commitment_config.clone())?
.get_epoch_info_with_commitment(commitment_config)?
.into();
Ok(config.output_format.formatted_string(&epoch_info))
}
Expand All @@ -673,15 +673,15 @@ pub fn process_get_slot(
rpc_client: &RpcClient,
commitment_config: CommitmentConfig,
) -> ProcessResult {
let slot = rpc_client.get_slot_with_commitment(commitment_config.clone())?;
let slot = rpc_client.get_slot_with_commitment(commitment_config)?;
Ok(slot.to_string())
}

pub fn process_get_epoch(
rpc_client: &RpcClient,
commitment_config: CommitmentConfig,
) -> ProcessResult {
let epoch_info = rpc_client.get_epoch_info_with_commitment(commitment_config.clone())?;
let epoch_info = rpc_client.get_epoch_info_with_commitment(commitment_config)?;
Ok(epoch_info.epoch.to_string())
}

Expand Down Expand Up @@ -868,7 +868,7 @@ pub fn process_supply(
commitment_config: CommitmentConfig,
print_accounts: bool,
) -> ProcessResult {
let supply_response = rpc_client.supply_with_commitment(commitment_config.clone())?;
let supply_response = rpc_client.supply_with_commitment(commitment_config)?;
let mut supply: CliSupply = supply_response.value.into();
supply.print_accounts = print_accounts;
Ok(config.output_format.formatted_string(&supply))
Expand All @@ -878,16 +878,15 @@ pub fn process_total_supply(
rpc_client: &RpcClient,
commitment_config: CommitmentConfig,
) -> ProcessResult {
let total_supply = rpc_client.total_supply_with_commitment(commitment_config.clone())?;
let total_supply = rpc_client.total_supply_with_commitment(commitment_config)?;
Ok(format!("{} SOL", lamports_to_sol(total_supply)))
}

pub fn process_get_transaction_count(
rpc_client: &RpcClient,
commitment_config: CommitmentConfig,
) -> ProcessResult {
let transaction_count =
rpc_client.get_transaction_count_with_commitment(commitment_config.clone())?;
let transaction_count = rpc_client.get_transaction_count_with_commitment(commitment_config)?;
Ok(transaction_count.to_string())
}

Expand Down Expand Up @@ -952,10 +951,8 @@ pub fn process_ping(
Ok(signature) => {
let transaction_sent = Instant::now();
loop {
let signature_status = rpc_client.get_signature_status_with_commitment(
&signature,
commitment_config.clone(),
)?;
let signature_status = rpc_client
.get_signature_status_with_commitment(&signature, commitment_config)?;
let elapsed_time = Instant::now().duration_since(transaction_sent);
if let Some(transaction_status) = signature_status {
match transaction_status {
Expand Down
15 changes: 5 additions & 10 deletions cli/src/offline/blockhash_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,11 @@ impl Source {
Ok(res)
}
Self::NonceAccount(ref pubkey) => {
let res = nonce::get_account(rpc_client, pubkey)
.and_then(|ref a| nonce::data_from_account(a))
.and_then(|d| {
if d.blockhash == *blockhash {
Ok(Some(d.fee_calculator))
} else {
Ok(None)
}
})?;
Ok(res)
let res = nonce::get_account(rpc_client, pubkey)?;
let res = nonce::data_from_account(&res)?;
Ok(Some(res)
.filter(|d| d.blockhash == *blockhash)
.map(|d| d.fee_calculator))
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions client/src/rpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ impl RpcClient {
) -> ClientResult<u64> {
let now = Instant::now();
loop {
match self.get_balance_with_commitment(&pubkey, commitment_config.clone()) {
match self.get_balance_with_commitment(&pubkey, commitment_config) {
Ok(bal) => {
return Ok(bal.value);
}
Expand Down Expand Up @@ -699,8 +699,7 @@ impl RpcClient {
) -> Option<u64> {
const LAST: usize = 30;
for run in 0..LAST {
let balance_result =
self.poll_get_balance_with_commitment(pubkey, commitment_config.clone());
let balance_result = self.poll_get_balance_with_commitment(pubkey, commitment_config);
if expected_balance.is_none() {
return balance_result.ok();
}
Expand Down Expand Up @@ -734,7 +733,7 @@ impl RpcClient {
let now = Instant::now();
loop {
if let Ok(Some(_)) =
self.get_signature_status_with_commitment(&signature, commitment_config.clone())
self.get_signature_status_with_commitment(&signature, commitment_config)
{
break;
}
Expand Down
9 changes: 4 additions & 5 deletions client/src/rpc_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,12 @@ mod tests {
fn test_build_request_json() {
let test_request = RpcRequest::GetAccountInfo;
let addr = json!("deadbeefXjn8o3yroDHxUtKsZZgoy4GPkPPXfouKNHhx");
let request = test_request.build_request_json(1, json!([addr.clone()]));
let request = test_request.build_request_json(1, json!([addr]));
assert_eq!(request["method"], "getAccountInfo");
assert_eq!(request["params"], json!([addr]));

let test_request = RpcRequest::GetBalance;
let request = test_request.build_request_json(1, json!([addr.clone()]));
let request = test_request.build_request_json(1, json!([addr]));
assert_eq!(request["method"], "getBalance");

let test_request = RpcRequest::GetEpochInfo;
Expand Down Expand Up @@ -186,13 +186,12 @@ mod tests {

// Test request with CommitmentConfig and no params
let test_request = RpcRequest::GetRecentBlockhash;
let request = test_request.build_request_json(1, json!([commitment_config.clone()]));
let request = test_request.build_request_json(1, json!([commitment_config]));
assert_eq!(request["params"], json!([commitment_config.clone()]));

// Test request with CommitmentConfig and params
let test_request = RpcRequest::GetBalance;
let request =
test_request.build_request_json(1, json!([addr.clone(), commitment_config.clone()]));
let request = test_request.build_request_json(1, json!([addr, commitment_config]));
assert_eq!(request["params"], json!([addr, commitment_config]));
}
}
9 changes: 4 additions & 5 deletions core/benches/banking_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ fn make_accounts_txs(txes: usize, mint_keypair: &Keypair, hash: Hash) -> Vec<Tra
fn make_programs_txs(txes: usize, hash: Hash) -> Vec<Transaction> {
let progs = 4;
(0..txes)
.into_iter()
.map(|_| {
let mut instructions = vec![];
let from_key = Keypair::new();
Expand Down Expand Up @@ -181,7 +180,7 @@ fn bench_banking(bencher: &mut Bencher, tx_type: TransactionType) {
assert!(r.is_ok(), "sanity parallel execution");
}
bank.clear_signatures();
let verified: Vec<_> = to_packets_chunked(&transactions.clone(), PACKETS_PER_BATCH);
let verified: Vec<_> = to_packets_chunked(&transactions, PACKETS_PER_BATCH);
let ledger_path = get_tmp_ledger_path!();
{
let blockstore = Arc::new(
Expand All @@ -207,7 +206,7 @@ fn bench_banking(bencher: &mut Bencher, tx_type: TransactionType) {
// If it is dropped before poh_service, then poh_service will error when
// calling send() on the channel.
let signal_receiver = Arc::new(signal_receiver);
let signal_receiver2 = signal_receiver.clone();
let signal_receiver2 = signal_receiver;
bencher.iter(move || {
let now = Instant::now();
let mut sent = 0;
Expand Down Expand Up @@ -262,7 +261,7 @@ fn simulate_process_entries(
mint_keypair: &Keypair,
mut tx_vector: Vec<Transaction>,
genesis_config: &GenesisConfig,
keypairs: &Vec<Keypair>,
keypairs: &[Keypair],
initial_lamports: u64,
num_accounts: usize,
) {
Expand All @@ -288,7 +287,7 @@ fn simulate_process_entries(
hash: next_hash(&bank.last_blockhash(), 1, &tx_vector),
transactions: tx_vector,
};
process_entries(&bank, &vec![entry], randomize_txs, None).unwrap();
process_entries(&bank, &[entry], randomize_txs, None).unwrap();
}

fn bench_process_entries(randomize_txs: bool, bencher: &mut Bencher) {
Expand Down
2 changes: 0 additions & 2 deletions core/benches/blockstore.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#![feature(test)]
use rand;

extern crate solana_ledger;
extern crate test;

Expand Down
4 changes: 2 additions & 2 deletions core/benches/cluster_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ fn broadcast_shreds_bench(bencher: &mut Bencher) {
solana_logger::setup();
let leader_pubkey = Pubkey::new_rand();
let leader_info = Node::new_localhost_with_pubkey(&leader_pubkey);
let cluster_info = ClusterInfo::new_with_invalid_keypair(leader_info.info.clone());
let cluster_info = ClusterInfo::new_with_invalid_keypair(leader_info.info);
let socket = UdpSocket::bind("0.0.0.0:0").unwrap();

const NUM_SHREDS: usize = 32;
Expand All @@ -37,7 +37,7 @@ fn broadcast_shreds_bench(bencher: &mut Bencher) {
}
let stakes = Arc::new(stakes);
let cluster_info = Arc::new(cluster_info);
let (peers, peers_and_stakes) = get_broadcast_peers(&cluster_info, Some(stakes.clone()));
let (peers, peers_and_stakes) = get_broadcast_peers(&cluster_info, Some(stakes));
let shreds = Arc::new(shreds);
let last_datapoint = Arc::new(AtomicU64::new(0));
bencher.iter(move || {
Expand Down
6 changes: 2 additions & 4 deletions core/benches/poh_verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,21 @@ const NUM_ENTRIES: usize = 800;
fn bench_poh_verify_ticks(bencher: &mut Bencher) {
let zero = Hash::default();
let mut cur_hash = hash(&zero.as_ref());
let start = *&cur_hash;

let mut ticks: Vec<Entry> = Vec::with_capacity(NUM_ENTRIES);
for _ in 0..NUM_ENTRIES {
ticks.push(next_entry_mut(&mut cur_hash, NUM_HASHES, vec![]));
}

bencher.iter(|| {
ticks.verify(&start);
ticks.verify(&cur_hash);
})
}

#[bench]
fn bench_poh_verify_transaction_entries(bencher: &mut Bencher) {
let zero = Hash::default();
let mut cur_hash = hash(&zero.as_ref());
let start = *&cur_hash;

let keypair1 = Keypair::new();
let pubkey1 = keypair1.pubkey();
Expand All @@ -42,6 +40,6 @@ fn bench_poh_verify_transaction_entries(bencher: &mut Bencher) {
}

bencher.iter(|| {
ticks.verify(&start);
ticks.verify(&cur_hash);
})
}
6 changes: 4 additions & 2 deletions core/benches/retransmit_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ fn bench_retransmitter(bencher: &mut Bencher) {
let tx = test_tx();
const NUM_PACKETS: usize = 50;
let chunk_size = NUM_PACKETS / (4 * NUM_THREADS);
let batches = to_packets_chunked(&vec![tx; NUM_PACKETS], chunk_size);
let batches = to_packets_chunked(
&std::iter::repeat(tx).take(NUM_PACKETS).collect::<Vec<_>>(),
chunk_size,
);
info!("batches: {}", batches.len());

let retransmitter_handles = retransmitter(
Expand All @@ -80,7 +83,6 @@ fn bench_retransmitter(bencher: &mut Bencher) {
bencher.iter(move || {
let peer_sockets1 = peer_sockets.clone();
let handles: Vec<_> = (0..NUM_PEERS)
.into_iter()
.map(|p| {
let peer_sockets2 = peer_sockets1.clone();
let total2 = total.clone();
Expand Down
6 changes: 2 additions & 4 deletions core/benches/sigverify_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,14 @@ fn bench_sigverify_stage(bencher: &mut Bencher) {
let from_keypair = Keypair::new();
let to_keypair = Keypair::new();
let txs: Vec<_> = (0..len)
.into_iter()
.map(|_| {
let amount = thread_rng().gen();
let tx = system_transaction::transfer(
system_transaction::transfer(
&from_keypair,
&to_keypair.pubkey(),
amount,
Hash::default(),
);
tx
)
})
.collect();
to_packets_chunked(&txs, chunk_size)
Expand Down
29 changes: 14 additions & 15 deletions core/src/cluster_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ impl ClusterInfo {

pub fn update_contact_info<F>(&self, modify: F)
where
F: FnOnce(&mut ContactInfo) -> (),
F: FnOnce(&mut ContactInfo),
{
let my_id = self.id();
modify(&mut self.my_contact_info.write().unwrap());
Expand Down Expand Up @@ -1917,19 +1917,18 @@ impl ClusterInfo {
.into_iter()
.filter_map(|(from, prune_set)| {
inc_new_counter_debug!("cluster_info-push_message-prunes", prune_set.len());
me.lookup_contact_info(&from, |ci| ci.clone())
.and_then(|ci| {
let mut prune_msg = PruneData {
pubkey: self_id,
prunes: prune_set.into_iter().collect(),
signature: Signature::default(),
destination: from,
wallclock: timestamp(),
};
prune_msg.sign(&me.keypair);
let rsp = Protocol::PruneMessage(self_id, prune_msg);
Some((ci.gossip, rsp))
})
me.lookup_contact_info(&from, |ci| ci.clone()).map(|ci| {
let mut prune_msg = PruneData {
pubkey: self_id,
prunes: prune_set.into_iter().collect(),
signature: Signature::default(),
destination: from,
wallclock: timestamp(),
};
prune_msg.sign(&me.keypair);
let rsp = Protocol::PruneMessage(self_id, prune_msg);
(ci.gossip, rsp)
})
})
.collect();
if rsp.is_empty() {
Expand Down Expand Up @@ -2932,7 +2931,7 @@ mod tests {
assert_eq!(slots.len(), 1);
assert!(since.is_some());

let (slots, since2) = cluster_info.get_epoch_slots_since(since.clone());
let (slots, since2) = cluster_info.get_epoch_slots_since(since);
assert!(slots.is_empty());
assert_eq!(since2, since);
}
Expand Down
Loading

0 comments on commit e23340d

Please sign in to comment.