From dba6b24bde655bccc87a7b69ea9f53b2a4a58e13 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Sat, 18 Feb 2023 13:44:06 +0100 Subject: [PATCH] chore: clippy fixes and make benches compile again (#1449) --- crates/consensus/src/validation.rs | 2 +- crates/net/common/src/bandwidth_meter.rs | 6 +-- crates/net/eth-wire/src/p2pstream.rs | 13 +++--- crates/primitives/src/block.rs | 4 +- crates/primitives/src/forkid.rs | 4 +- crates/staged-sync/src/utils/init.rs | 6 +-- crates/stages/benches/criterion.rs | 3 +- .../stages/benches/setup/account_hashing.rs | 2 +- crates/stages/src/stages/finish.rs | 2 +- crates/storage/db/benches/criterion.rs | 40 ++++++++++++------- crates/storage/db/benches/hash_keys.rs | 3 +- crates/storage/db/benches/utils.rs | 3 +- .../storage/db/src/implementation/mdbx/mod.rs | 36 +++++++---------- .../storage/db/src/tables/models/accounts.rs | 2 +- .../storage/libmdbx-rs/benches/transaction.rs | 2 +- .../provider/src/providers/state/chain.rs | 1 + 16 files changed, 65 insertions(+), 64 deletions(-) diff --git a/crates/consensus/src/validation.rs b/crates/consensus/src/validation.rs index 3cb7d03cc..b81172a6c 100644 --- a/crates/consensus/src/validation.rs +++ b/crates/consensus/src/validation.rs @@ -648,7 +648,7 @@ mod tests { let chain_spec = ChainSpecBuilder::mainnet().shanghai_activated().build(); let header = Header { - base_fee_per_gas: Some(1337u64.into()), + base_fee_per_gas: Some(1337u64), withdrawals_root: Some(proofs::calculate_withdrawals_root(&[])), ..Default::default() } diff --git a/crates/net/common/src/bandwidth_meter.rs b/crates/net/common/src/bandwidth_meter.rs index 349c9ccd8..b2c52a4f3 100644 --- a/crates/net/common/src/bandwidth_meter.rs +++ b/crates/net/common/src/bandwidth_meter.rs @@ -182,10 +182,10 @@ mod tests { let mut buf = [0u8; 4]; client.write_all(b"ping").await.unwrap(); - server.read(&mut buf).await.unwrap(); + server.read_exact(&mut buf).await.unwrap(); server.write_all(b"pong").await.unwrap(); - client.read(&mut buf).await.unwrap(); + client.read_exact(&mut buf).await.unwrap(); } fn assert_bandwidth_counts( @@ -236,7 +236,7 @@ mod tests { let mut buf = [0u8; 4]; - metered_server_stream.read(&mut buf).await.unwrap(); + metered_server_stream.read_exact(&mut buf).await.unwrap(); assert_eq!(metered_server_stream.meter.total_inbound(), client_meter.total_outbound()); }); diff --git a/crates/net/eth-wire/src/p2pstream.rs b/crates/net/eth-wire/src/p2pstream.rs index 6b5adc695..e7e92389a 100644 --- a/crates/net/eth-wire/src/p2pstream.rs +++ b/crates/net/eth-wire/src/p2pstream.rs @@ -906,16 +906,15 @@ mod tests { let unauthed_stream = UnauthedP2PStream::new(stream); match unauthed_stream.handshake(server_hello.clone()).await { - Ok((_, hello)) => panic!( - "expected handshake to fail, instead got a successful Hello: {:?}", - hello - ), + Ok((_, hello)) => { + panic!("expected handshake to fail, instead got a successful Hello: {hello:?}") + } Err(P2PStreamError::MismatchedProtocolVersion { expected, got }) => { assert_eq!(expected, server_hello.protocol_version as u8); assert_ne!(expected, got); } Err(other_err) => { - panic!("expected mismatched protocol version error, got {:?}", other_err) + panic!("expected mismatched protocol version error, got {other_err:?}") } } }); @@ -931,14 +930,14 @@ mod tests { let unauthed_stream = UnauthedP2PStream::new(sink); match unauthed_stream.handshake(client_hello.clone()).await { Ok((_, hello)) => { - panic!("expected handshake to fail, instead got a successful Hello: {:?}", hello) + panic!("expected handshake to fail, instead got a successful Hello: {hello:?}") } Err(P2PStreamError::MismatchedProtocolVersion { expected, got }) => { assert_eq!(expected, client_hello.protocol_version as u8); assert_ne!(expected, got); } Err(other_err) => { - panic!("expected mismatched protocol version error, got {:?}", other_err) + panic!("expected mismatched protocol version error, got {other_err:?}") } } diff --git a/crates/primitives/src/block.rs b/crates/primitives/src/block.rs index 19fc72d36..a7e700f1c 100644 --- a/crates/primitives/src/block.rs +++ b/crates/primitives/src/block.rs @@ -501,7 +501,7 @@ mod test { /// Serde tests #[test] fn serde_blockid_tags() { - let block_ids = [Latest, Finalized, Safe, Pending].map(|id| BlockId::from(id)); + let block_ids = [Latest, Finalized, Safe, Pending].map(BlockId::from); for block_id in &block_ids { let serialized = serde_json::to_string(&block_id).unwrap(); let deserialized: BlockId = serde_json::from_str(&serialized).unwrap(); @@ -539,7 +539,7 @@ mod test { let hash = H256::from_str("0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3") .unwrap(); - assert_eq!(BlockId::from(hash.clone()), block_id); + assert_eq!(BlockId::from(hash), block_id); let serialized = serde_json::to_string(&BlockId::from(hash)).unwrap(); assert_eq!("{\"blockHash\":\"0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3\"}", serialized) } diff --git a/crates/primitives/src/forkid.rs b/crates/primitives/src/forkid.rs index b175330a8..a737e8430 100644 --- a/crates/primitives/src/forkid.rs +++ b/crates/primitives/src/forkid.rs @@ -561,8 +561,8 @@ mod tests { let b1 = 1_150_000; let b2 = 1_920_000; - let h0 = ForkId { hash: ForkHash(hex!("fc64ec04")), next: b1.into() }; - let h1 = ForkId { hash: ForkHash(hex!("97c2c34c")), next: b2.into() }; + let h0 = ForkId { hash: ForkHash(hex!("fc64ec04")), next: b1 }; + let h1 = ForkId { hash: ForkHash(hex!("97c2c34c")), next: b2 }; let h2 = ForkId { hash: ForkHash(hex!("91d1f948")), next: 0 }; let mut fork_filter = ForkFilter::new( diff --git a/crates/staged-sync/src/utils/init.rs b/crates/staged-sync/src/utils/init.rs index f11514063..5551ee7d9 100644 --- a/crates/staged-sync/src/utils/init.rs +++ b/crates/staged-sync/src/utils/init.rs @@ -74,7 +74,7 @@ mod tests { #[test] fn success_init_genesis_mainnet() { let db = create_test_rw_db(); - let genesis_hash = init_genesis(db.clone(), MAINNET.clone()).unwrap(); + let genesis_hash = init_genesis(db, MAINNET.clone()).unwrap(); // actual, expected assert_eq!(genesis_hash, MAINNET_GENESIS); @@ -83,7 +83,7 @@ mod tests { #[test] fn success_init_genesis_goerli() { let db = create_test_rw_db(); - let genesis_hash = init_genesis(db.clone(), GOERLI.clone()).unwrap(); + let genesis_hash = init_genesis(db, GOERLI.clone()).unwrap(); // actual, expected assert_eq!(genesis_hash, GOERLI_GENESIS); @@ -92,7 +92,7 @@ mod tests { #[test] fn success_init_genesis_sepolia() { let db = create_test_rw_db(); - let genesis_hash = init_genesis(db.clone(), SEPOLIA.clone()).unwrap(); + let genesis_hash = init_genesis(db, SEPOLIA.clone()).unwrap(); // actual, expected assert_eq!(genesis_hash, SEPOLIA_GENESIS); diff --git a/crates/stages/benches/criterion.rs b/crates/stages/benches/criterion.rs index d9fcb100e..3fa21ee6f 100644 --- a/crates/stages/benches/criterion.rs +++ b/crates/stages/benches/criterion.rs @@ -40,8 +40,7 @@ fn senders(c: &mut Criterion) { for batch in [1000usize, 10_000, 100_000, 250_000] { let num_blocks = 10_000; - let mut stage = SenderRecoveryStage::default(); - stage.commit_threshold = num_blocks; + let stage = SenderRecoveryStage { commit_threshold: num_blocks, ..Default::default() }; let label = format!("SendersRecovery-batch-{batch}"); measure_stage(&mut group, stage, num_blocks, label); } diff --git a/crates/stages/benches/setup/account_hashing.rs b/crates/stages/benches/setup/account_hashing.rs index ce6f152a8..ec408c358 100644 --- a/crates/stages/benches/setup/account_hashing.rs +++ b/crates/stages/benches/setup/account_hashing.rs @@ -30,7 +30,7 @@ pub fn prepare_account_hashing( (path, AccountHashingStage::default(), stage_range) } -fn find_stage_range(db: &PathBuf) -> (ExecInput, UnwindInput) { +fn find_stage_range(db: &Path) -> (ExecInput, UnwindInput) { let mut stage_range = None; TestTransaction::new(db) .tx diff --git a/crates/stages/src/stages/finish.rs b/crates/stages/src/stages/finish.rs index fd3df6498..952bb7f8a 100644 --- a/crates/stages/src/stages/finish.rs +++ b/crates/stages/src/stages/finish.rs @@ -110,7 +110,7 @@ mod tests { fn current_status(&self) -> Head { let status_lock = self.status.try_lock().expect("competing for status lock"); let status = status_lock.as_ref().expect("no status receiver set").borrow(); - status.clone() + *status } } diff --git a/crates/storage/db/benches/criterion.rs b/crates/storage/db/benches/criterion.rs index 9e6947f38..c0f73d160 100644 --- a/crates/storage/db/benches/criterion.rs +++ b/crates/storage/db/benches/criterion.rs @@ -4,7 +4,10 @@ use criterion::{ black_box, criterion_group, criterion_main, measurement::WallTime, BenchmarkGroup, Criterion, }; use pprof::criterion::{Output, PProfProfiler}; -use reth_db::cursor::{DbDupCursorRO, DbDupCursorRW}; +use reth_db::{ + cursor::{DbCursorRO, DbCursorRW, DbDupCursorRO, DbDupCursorRW}, + tables::*, +}; use std::time::Instant; criterion_group! { @@ -64,11 +67,12 @@ where b.iter_with_setup( || input.clone(), |input| { - black_box({ + { for (k, _, _, _) in input { k.encode(); } - }); + }; + black_box(()); }, ) }); @@ -77,11 +81,12 @@ where b.iter_with_setup( || input.clone(), |input| { - black_box({ + { for (_, k, _, _) in input { let _ = ::Key::decode(k); } - }); + }; + black_box(()); }, ) }); @@ -90,11 +95,12 @@ where b.iter_with_setup( || input.clone(), |input| { - black_box({ + { for (_, _, v, _) in input { v.compress(); } - }); + }; + black_box(()); }, ) }); @@ -103,11 +109,12 @@ where b.iter_with_setup( || input.clone(), |input| { - black_box({ + { for (_, _, _, v) in input { let _ = ::Value::decompress(v); } - }); + }; + black_box(()); }, ) }); @@ -177,13 +184,14 @@ where // Create TX let tx = db.tx().expect("tx"); - black_box({ + { let mut cursor = tx.cursor_read::().expect("cursor"); let walker = cursor.walk(Some(input.first().unwrap().0.clone())).unwrap(); for element in walker { element.unwrap(); } - }); + }; + black_box(()); }) }); @@ -194,12 +202,13 @@ where // Create TX let tx = db.tx().expect("tx"); - black_box({ + { for index in RANDOM_INDEXES { let mut cursor = tx.cursor_read::().expect("cursor"); cursor.seek_exact(input.get(index).unwrap().0.clone()).unwrap(); } - }); + }; + black_box(()); }) }); } @@ -267,13 +276,14 @@ where // Create TX let tx = db.tx().expect("tx"); - black_box({ + { let mut cursor = tx.cursor_dup_read::().expect("cursor"); let walker = cursor.walk_dup(None, Some(T::SubKey::default())).unwrap(); for element in walker { element.unwrap(); } - }); + }; + black_box(()); }) }); diff --git a/crates/storage/db/benches/hash_keys.rs b/crates/storage/db/benches/hash_keys.rs index cad28e49f..b7cb59297 100644 --- a/crates/storage/db/benches/hash_keys.rs +++ b/crates/storage/db/benches/hash_keys.rs @@ -11,8 +11,9 @@ use proptest::{ test_runner::TestRunner, }; use reth_db::{ - cursor::{DbDupCursorRO, DbDupCursorRW}, + cursor::{DbCursorRW, DbDupCursorRO, DbDupCursorRW}, mdbx::Env, + TxHashNumber, }; use std::{collections::HashSet, time::Instant}; use test_fuzz::runtime::num_traits::Zero; diff --git a/crates/storage/db/benches/utils.rs b/crates/storage/db/benches/utils.rs index d82eb55ae..08d88a9ba 100644 --- a/crates/storage/db/benches/utils.rs +++ b/crates/storage/db/benches/utils.rs @@ -1,9 +1,7 @@ use reth_db::{ - cursor::{DbCursorRO, DbCursorRW}, database::Database, mdbx::{test_utils::create_test_db_with_path, EnvKind, WriteMap}, table::*, - tables::*, transaction::{DbTx, DbTxMut}, }; use std::path::Path; @@ -44,6 +42,7 @@ where } /// Sets up a clear database at `bench_db_path`. +#[allow(clippy::ptr_arg)] fn set_up_db( bench_db_path: &Path, pair: &Vec<(::Key, bytes::Bytes, ::Value, bytes::Bytes)>, diff --git a/crates/storage/db/src/implementation/mdbx/mod.rs b/crates/storage/db/src/implementation/mdbx/mod.rs index c252ce7bd..8b9189c69 100644 --- a/crates/storage/db/src/implementation/mdbx/mod.rs +++ b/crates/storage/db/src/implementation/mdbx/mod.rs @@ -210,7 +210,7 @@ mod tests { assert!(first.is_some(), "First should be our put"); // Walk - let walk = cursor.walk(Some(key.into())).unwrap(); + let walk = cursor.walk(Some(key)).unwrap(); let first = walk.into_iter().next().unwrap().unwrap(); assert_eq!(first.1, value, "First next should be put value"); } @@ -636,18 +636,15 @@ mod tests { // PUT (0,0) let value00 = StorageEntry::default(); - env.update(|tx| tx.put::(key, value00.clone()).expect(ERROR_PUT)) - .unwrap(); + env.update(|tx| tx.put::(key, value00).expect(ERROR_PUT)).unwrap(); // PUT (2,2) let value22 = StorageEntry { key: H256::from_low_u64_be(2), value: U256::from(2) }; - env.update(|tx| tx.put::(key, value22.clone()).expect(ERROR_PUT)) - .unwrap(); + env.update(|tx| tx.put::(key, value22).expect(ERROR_PUT)).unwrap(); // PUT (1,1) let value11 = StorageEntry { key: H256::from_low_u64_be(1), value: U256::from(1) }; - env.update(|tx| tx.put::(key, value11.clone()).expect(ERROR_PUT)) - .unwrap(); + env.update(|tx| tx.put::(key, value11).expect(ERROR_PUT)).unwrap(); // Iterate with cursor { @@ -656,7 +653,7 @@ mod tests { // Notice that value11 and value22 have been ordered in the DB. assert!(Some(value00) == cursor.next_dup_val().unwrap()); - assert!(Some(value11.clone()) == cursor.next_dup_val().unwrap()); + assert!(Some(value11) == cursor.next_dup_val().unwrap()); assert!(Some(value22) == cursor.next_dup_val().unwrap()); } @@ -685,18 +682,15 @@ mod tests { // PUT key1 (0,0) let value00 = StorageEntry::default(); - env.update(|tx| tx.put::(key1, value00.clone()).expect(ERROR_PUT)) - .unwrap(); + env.update(|tx| tx.put::(key1, value00).expect(ERROR_PUT)).unwrap(); // PUT key1 (1,1) let value11 = StorageEntry { key: H256::from_low_u64_be(1), value: U256::from(1) }; - env.update(|tx| tx.put::(key1, value11.clone()).expect(ERROR_PUT)) - .unwrap(); + env.update(|tx| tx.put::(key1, value11).expect(ERROR_PUT)).unwrap(); // PUT key2 (2,2) let value22 = StorageEntry { key: H256::from_low_u64_be(2), value: U256::from(2) }; - env.update(|tx| tx.put::(key2, value22.clone()).expect(ERROR_PUT)) - .unwrap(); + env.update(|tx| tx.put::(key2, value22).expect(ERROR_PUT)).unwrap(); // Iterate with walk_dup { @@ -705,8 +699,8 @@ mod tests { let mut walker = cursor.walk_dup(None, None).unwrap(); // Notice that value11 and value22 have been ordered in the DB. - assert_eq!(Some(Ok((key1, value00.clone()))), walker.next()); - assert_eq!(Some(Ok((key1, value11.clone()))), walker.next()); + assert_eq!(Some(Ok((key1, value00))), walker.next()); + assert_eq!(Some(Ok((key1, value11))), walker.next()); // NOTE: Dup cursor does NOT iterates on all values but only on duplicated values of the // same key. assert_eq!(Ok(Some(value22.clone())), walker.next()); assert_eq!(None, walker.next()); @@ -732,13 +726,11 @@ mod tests { // PUT key1 (0,1) let value01 = StorageEntry { key: H256::from_low_u64_be(0), value: U256::from(1) }; - env.update(|tx| tx.put::(key1, value01.clone()).expect(ERROR_PUT)) - .unwrap(); + env.update(|tx| tx.put::(key1, value01).expect(ERROR_PUT)).unwrap(); // PUT key1 (0,0) let value00 = StorageEntry::default(); - env.update(|tx| tx.put::(key1, value00.clone()).expect(ERROR_PUT)) - .unwrap(); + env.update(|tx| tx.put::(key1, value00).expect(ERROR_PUT)).unwrap(); // Iterate with walk { @@ -748,7 +740,7 @@ mod tests { let mut walker = cursor.walk(Some(first.0)).unwrap(); // NOTE: Both values are present - assert_eq!(Some(Ok((key1, value00.clone()))), walker.next()); + assert_eq!(Some(Ok((key1, value00))), walker.next()); assert_eq!(Some(Ok((key1, value01))), walker.next()); assert_eq!(None, walker.next()); } @@ -759,7 +751,7 @@ mod tests { let mut cursor = tx.cursor_dup_read::().unwrap(); // NOTE: There are two values with same SubKey but only first one is shown - assert_eq!(Ok(Some(value00.clone())), cursor.seek_by_key_subkey(key1, value00.key)); + assert_eq!(Ok(Some(value00)), cursor.seek_by_key_subkey(key1, value00.key)); } } diff --git a/crates/storage/db/src/tables/models/accounts.rs b/crates/storage/db/src/tables/models/accounts.rs index 3783cf3ac..07633818b 100644 --- a/crates/storage/db/src/tables/models/accounts.rs +++ b/crates/storage/db/src/tables/models/accounts.rs @@ -118,7 +118,7 @@ mod test { bytes[..8].copy_from_slice(&num.to_be_bytes()); bytes[8..].copy_from_slice(&hash.0); - let encoded = Encode::encode(key.clone()); + let encoded = Encode::encode(key); assert_eq!(encoded, bytes); let decoded: TransitionIdAddress = Decode::decode(encoded.to_vec()).unwrap(); diff --git a/crates/storage/libmdbx-rs/benches/transaction.rs b/crates/storage/libmdbx-rs/benches/transaction.rs index 27f27839f..f049916a4 100644 --- a/crates/storage/libmdbx-rs/benches/transaction.rs +++ b/crates/storage/libmdbx-rs/benches/transaction.rs @@ -117,7 +117,7 @@ fn bench_put_rand_raw(c: &mut Criterion) { criterion_group! { name = benches; - config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None))); + config = Criterion::default().with_profiler(pprof::criterion::PProfProfiler::new(100, pprof::criterion::Output::Flamegraph(None))); targets = bench_get_rand, bench_get_rand_raw, bench_put_rand, bench_put_rand_raw } criterion_main!(benches); diff --git a/crates/storage/provider/src/providers/state/chain.rs b/crates/storage/provider/src/providers/state/chain.rs index a5e247a29..567a9f827 100644 --- a/crates/storage/provider/src/providers/state/chain.rs +++ b/crates/storage/provider/src/providers/state/chain.rs @@ -44,6 +44,7 @@ mod tests { fn assert_state_provider() {} #[allow(unused)] + #[allow(clippy::extra_unused_lifetimes)] fn assert_chain_state_provider<'txn>() { assert_state_provider::>(); }