From f04f4f0b98ece999eaaed19d678cd375fc6f69ab Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Tue, 4 Feb 2020 22:53:24 -0800 Subject: [PATCH] Apply clippy fixes --- clippy.toml | 1 + zebra-chain/src/block.rs | 1 - zebra-chain/src/transaction.rs | 2 ++ zebra-chain/src/transaction/hash.rs | 2 -- zebra-chain/src/transaction/joinsplit.rs | 4 ++-- zebra-chain/src/transaction/serialize.rs | 8 +++---- zebra-chain/src/transaction/shielded_data.rs | 12 +++++----- zebra-chain/src/types.rs | 1 - zebra-network/src/address_book.rs | 24 +++++++++---------- zebra-network/src/constants.rs | 2 +- zebra-network/src/network.rs | 2 +- zebra-network/src/peer/client.rs | 2 +- zebra-network/src/peer/connection.rs | 7 +++--- zebra-network/src/peer/error.rs | 4 ++-- zebra-network/src/peer/handshake.rs | 13 +++++----- zebra-network/src/peer_set/candidate_set.rs | 3 +-- zebra-network/src/peer_set/set.rs | 1 + zebra-network/src/protocol/external/codec.rs | 9 ++++--- .../src/protocol/external/message.rs | 6 +++-- zebra-network/src/protocol/external/types.rs | 3 +-- zebrad/src/commands.rs | 2 +- zebrad/src/commands/connect.rs | 8 +++---- zebrad/src/commands/seed.rs | 10 ++++---- 23 files changed, 61 insertions(+), 66 deletions(-) create mode 100644 clippy.toml diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 000000000..dc9158b32 --- /dev/null +++ b/clippy.toml @@ -0,0 +1 @@ +type-complexity-threshold = 999999 diff --git a/zebra-chain/src/block.rs b/zebra-chain/src/block.rs index b00936585..a614bb37a 100644 --- a/zebra-chain/src/block.rs +++ b/zebra-chain/src/block.rs @@ -7,7 +7,6 @@ mod tests; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use chrono::{DateTime, TimeZone, Utc}; -use hex; use std::{fmt, io}; #[cfg(test)] diff --git a/zebra-chain/src/transaction.rs b/zebra-chain/src/transaction.rs index 651e684db..6148a7f4c 100644 --- a/zebra-chain/src/transaction.rs +++ b/zebra-chain/src/transaction.rs @@ -31,6 +31,8 @@ use crate::types::{BlockHeight, LockTime}; /// internally by different enum variants. Because we checkpoint on Sapling /// activation, we do not parse any pre-Sapling transaction types. #[derive(Clone, Debug, PartialEq, Eq)] +// XXX consider boxing the Optional fields of V4 txs +#[allow(clippy::large_enum_variant)] pub enum Transaction { /// A fully transparent transaction (`version = 1`). V1 { diff --git a/zebra-chain/src/transaction/hash.rs b/zebra-chain/src/transaction/hash.rs index bbba046e6..af353921c 100644 --- a/zebra-chain/src/transaction/hash.rs +++ b/zebra-chain/src/transaction/hash.rs @@ -1,7 +1,5 @@ use std::fmt; -use hex; - #[cfg(test)] use proptest_derive::Arbitrary; diff --git a/zebra-chain/src/transaction/joinsplit.rs b/zebra-chain/src/transaction/joinsplit.rs index fd1c7fea1..9c3f741b2 100644 --- a/zebra-chain/src/transaction/joinsplit.rs +++ b/zebra-chain/src/transaction/joinsplit.rs @@ -105,8 +105,8 @@ impl Arbitrary for JoinSplitData

{ ) .prop_map(|(first, rest, pub_key_bytes, sig_bytes)| { return Self { - first: first, - rest: rest, + first, + rest, pub_key: ed25519_zebra::PublicKeyBytes::from(pub_key_bytes), sig: ed25519_zebra::Signature::from({ let mut b = [0u8; 64]; diff --git a/zebra-chain/src/transaction/serialize.rs b/zebra-chain/src/transaction/serialize.rs index 802ebb97e..12e1d5b1c 100644 --- a/zebra-chain/src/transaction/serialize.rs +++ b/zebra-chain/src/transaction/serialize.rs @@ -12,8 +12,8 @@ use crate::types::Script; use super::*; -const OVERWINTER_VERSION_GROUP_ID: u32 = 0x03C48270; -const SAPLING_VERSION_GROUP_ID: u32 = 0x892F2085; +const OVERWINTER_VERSION_GROUP_ID: u32 = 0x03C4_8270; +const SAPLING_VERSION_GROUP_ID: u32 = 0x892F_2085; impl ZcashSerialize for OutPoint { fn zcash_serialize(&self, mut writer: W) -> Result<(), SerializationError> { @@ -373,14 +373,14 @@ impl ZcashDeserialize for Transaction { let joinsplit_data = OptV4JSD::zcash_deserialize(&mut reader)?; use futures::future::Either::*; - let shielded_data = if shielded_spends.len() > 0 { + let shielded_data = if !shielded_spends.is_empty() { Some(ShieldedData { first: Left(shielded_spends.remove(0)), rest_spends: shielded_spends, rest_outputs: shielded_outputs, binding_sig: reader.read_64_bytes()?.into(), }) - } else if shielded_outputs.len() > 0 { + } else if !shielded_outputs.is_empty() { Some(ShieldedData { first: Right(shielded_outputs.remove(0)), rest_spends: shielded_spends, diff --git a/zebra-chain/src/transaction/shielded_data.rs b/zebra-chain/src/transaction/shielded_data.rs index 0d1cfa334..54f9350c9 100644 --- a/zebra-chain/src/transaction/shielded_data.rs +++ b/zebra-chain/src/transaction/shielded_data.rs @@ -53,8 +53,8 @@ impl Arbitrary for SpendDescription { .prop_map( |(cv_bytes, anchor, nullifier_bytes, rpk_bytes, proof, sig_bytes)| { return Self { + anchor, cv: cv_bytes, - anchor: anchor, nullifier: nullifier_bytes, rk: redjubjub::PublicKeyBytes::from(rpk_bytes), zkproof: proof, @@ -186,14 +186,14 @@ impl Arbitrary for ShieldedData { vec(any::(), 0..10), vec(any::(), 64), ) - .prop_map(|(first, rest_spends, rest_outputs, sig)| { + .prop_map(|(first, rest_spends, rest_outputs, sig_bytes)| { return Self { - first: first, - rest_spends: rest_spends, - rest_outputs: rest_outputs, + first, + rest_spends, + rest_outputs, binding_sig: redjubjub::Signature::from({ let mut b = [0u8; 64]; - b.copy_from_slice(sig.as_slice()); + b.copy_from_slice(sig_bytes.as_slice()); b }), }; diff --git a/zebra-chain/src/types.rs b/zebra-chain/src/types.rs index 9c3523014..185e54e98 100644 --- a/zebra-chain/src/types.rs +++ b/zebra-chain/src/types.rs @@ -7,7 +7,6 @@ use std::{ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use chrono::{DateTime, TimeZone, Utc}; -use hex; #[cfg(test)] use proptest_derive::Arbitrary; diff --git a/zebra-network/src/address_book.rs b/zebra-network/src/address_book.rs index 811f5d9ad..f4ed72271 100644 --- a/zebra-network/src/address_book.rs +++ b/zebra-network/src/address_book.rs @@ -24,6 +24,7 @@ pub struct AddressBook { span: Span, } +#[allow(clippy::len_without_is_empty)] impl AddressBook { /// Construct an `AddressBook` with the given [`tracing::Span`]. pub fn new(span: Span) -> AddressBook { @@ -76,17 +77,14 @@ impl AddressBook { #[cfg(test)] self.assert_consistency(); - match self.get_by_addr(new.addr) { - Some(prev) => { - if prev.last_seen > new.last_seen { - return; - } else { - self.by_time - .take(&prev) - .expect("cannot have by_addr entry without by_time entry"); - } + if let Some(prev) = self.get_by_addr(new.addr) { + if prev.last_seen > new.last_seen { + return; + } else { + self.by_time + .take(&prev) + .expect("cannot have by_addr entry without by_time entry"); } - None => {} } self.by_time.insert(new); self.by_addr.insert(new.addr, (new.last_seen, new.services)); @@ -115,7 +113,7 @@ impl AddressBook { pub fn is_potentially_connected(&self, addr: &SocketAddr) -> bool { let _guard = self.span.enter(); match self.by_addr.get(addr) { - None => return false, + None => false, Some((ref last_seen, _)) => last_seen > &AddressBook::cutoff_time(), } } @@ -192,9 +190,9 @@ impl<'a> Iterator for Drain<'a> { fn next(&mut self) -> Option { let next_item = if self.newest_first { - self.book.by_time.iter().next()?.clone() + *self.book.by_time.iter().next()? } else { - self.book.by_time.iter().rev().next()?.clone() + *self.book.by_time.iter().rev().next()? }; self.book.by_time.remove(&next_item); self.book diff --git a/zebra-network/src/constants.rs b/zebra-network/src/constants.rs index 7a54b8a26..6d2ebd953 100644 --- a/zebra-network/src/constants.rs +++ b/zebra-network/src/constants.rs @@ -32,7 +32,7 @@ pub const HEARTBEAT_INTERVAL: Duration = Duration::from_secs(60); pub const TIMESTAMP_TRUNCATION_SECONDS: i64 = 30 * 60; /// The User-Agent string provided by the node. -pub const USER_AGENT: &'static str = "🦓Zebra v2.0.0-alpha.0🦓"; +pub const USER_AGENT: &str = "🦓Zebra v2.0.0-alpha.0🦓"; /// The Zcash network protocol version used on mainnet. pub const CURRENT_VERSION: Version = Version(170_007); diff --git a/zebra-network/src/network.rs b/zebra-network/src/network.rs index d5cfdc478..ffeb98bcd 100644 --- a/zebra-network/src/network.rs +++ b/zebra-network/src/network.rs @@ -11,7 +11,7 @@ pub enum Network { impl Network { /// Get the magic value associated to this `Network`. - pub fn magic(&self) -> Magic { + pub fn magic(self) -> Magic { match self { Network::Mainnet => magics::MAINNET, Network::Testnet => magics::TESTNET, diff --git a/zebra-network/src/peer/client.rs b/zebra-network/src/peer/client.rs index 941aaf581..f9b1e87e2 100644 --- a/zebra-network/src/peer/client.rs +++ b/zebra-network/src/peer/client.rs @@ -39,7 +39,7 @@ impl Service for Client { Pin> + Send + 'static>>; fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { - if let Err(_) = ready!(self.server_tx.poll_ready(cx)) { + if ready!(self.server_tx.poll_ready(cx)).is_err() { Poll::Ready(Err(self .error_slot .try_get_error() diff --git a/zebra-network/src/peer/connection.rs b/zebra-network/src/peer/connection.rs index a4d410ed1..baf4da968 100644 --- a/zebra-network/src/peer/connection.rs +++ b/zebra-network/src/peer/connection.rs @@ -347,9 +347,8 @@ where } }; - match req { - Some(req) => self.drive_peer_request(req).await, - None => {} + if let Some(req) = req { + self.drive_peer_request(req).await } } @@ -362,7 +361,7 @@ where trace!(?req); use tower::{load_shed::error::Overloaded, ServiceExt}; - if let Err(_) = self.svc.ready().await { + if self.svc.ready().await.is_err() { // Treat all service readiness errors as Overloaded self.fail_with(PeerError::Overloaded); } diff --git a/zebra-network/src/peer/error.rs b/zebra-network/src/peer/error.rs index 592e16978..734da9597 100644 --- a/zebra-network/src/peer/error.rs +++ b/zebra-network/src/peer/error.rs @@ -63,7 +63,7 @@ impl ErrorSlot { .lock() .expect("error mutex should be unpoisoned") .as_ref() - .map(|e| e.clone()) + .cloned() } } @@ -72,7 +72,7 @@ impl ErrorSlot { pub enum HandshakeError { /// The remote peer sent an unexpected message during the handshake. #[error("The remote peer sent an unexpected message: {0:?}")] - UnexpectedMessage(crate::protocol::external::Message), + UnexpectedMessage(Box), /// The peer connector detected handshake nonce reuse, possibly indicating self-connection. #[error("Detected nonce reuse, possible self-connection")] NonceReuse, diff --git a/zebra-network/src/peer/handshake.rs b/zebra-network/src/peer/handshake.rs index 30b70d214..20a09bb84 100644 --- a/zebra-network/src/peer/handshake.rs +++ b/zebra-network/src/peer/handshake.rs @@ -102,7 +102,7 @@ where let internal_service = self.internal_service.clone(); let timestamp_collector = self.timestamp_collector.clone(); let user_agent = self.config.user_agent.clone(); - let network = self.config.network.clone(); + let network = self.config.network; let fut = async move { info!("connecting to remote peer"); @@ -149,17 +149,18 @@ where { (nonce, services) } else { - return Err(HandshakeError::UnexpectedMessage(remote_msg)); + return Err(HandshakeError::UnexpectedMessage(Box::new(remote_msg))); }; // Check for nonce reuse, indicating self-connection. - if { + let nonce_reuse = { let mut locked_nonces = nonces.lock().expect("mutex should be unpoisoned"); let nonce_reuse = locked_nonces.contains(&remote_nonce); // Regardless of whether we observed nonce reuse, clean up the nonce set. locked_nonces.remove(&local_nonce); nonce_reuse - } { + }; + if nonce_reuse { return Err(HandshakeError::NonceReuse); } @@ -172,7 +173,7 @@ where if let Message::Verack = remote_msg { debug!("got verack from remote peer"); } else { - return Err(HandshakeError::UnexpectedMessage(remote_msg)); + return Err(HandshakeError::UnexpectedMessage(Box::new(remote_msg))); } // XXX here is where we would set the version to the minimum of the @@ -208,7 +209,7 @@ where .then(move |msg| { let mut timestamp_collector = timestamp_collector.clone(); async move { - if let Ok(_) = msg { + if msg.is_ok() { use futures::sink::SinkExt; let _ = timestamp_collector .send(MetaAddr { diff --git a/zebra-network/src/peer_set/candidate_set.rs b/zebra-network/src/peer_set/candidate_set.rs index d3dad5867..a7a297444 100644 --- a/zebra-network/src/peer_set/candidate_set.rs +++ b/zebra-network/src/peer_set/candidate_set.rs @@ -152,8 +152,7 @@ where .drain_oldest() .chain(self.gossiped.drain_newest()) .chain(self.failed.drain_oldest()) - .filter(|meta| !guard.is_potentially_connected(&meta.addr)) - .next() + .find(|meta| !guard.is_potentially_connected(&meta.addr)) } pub fn report_failed(&mut self, mut addr: MetaAddr) { diff --git a/zebra-network/src/peer_set/set.rs b/zebra-network/src/peer_set/set.rs index 6db759e68..1dbf2d439 100644 --- a/zebra-network/src/peer_set/set.rs +++ b/zebra-network/src/peer_set/set.rs @@ -220,6 +220,7 @@ where type Future = Pin> + Send + 'static>>; + #[allow(clippy::cognitive_complexity)] fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { // Process peer discovery updates. let _ = self.poll_discover(cx)?; diff --git a/zebra-network/src/protocol/external/codec.rs b/zebra-network/src/protocol/external/codec.rs index d0217df50..1cbb2bc44 100644 --- a/zebra-network/src/protocol/external/codec.rs +++ b/zebra-network/src/protocol/external/codec.rs @@ -293,6 +293,7 @@ impl Decoder for Codec { type Item = Message; type Error = Error; + #[allow(clippy::cognitive_complexity)] fn decode(&mut self, src: &mut BytesMut) -> Result, Self::Error> { use Error::Parse; match self.state { @@ -467,7 +468,7 @@ impl Codec { fn read_block(&self, mut reader: R) -> Result { Ok(Message::Block { version: Version(reader.read_u32::()?), - block: Block::zcash_deserialize(&mut reader)?, + block: Box::new(Block::zcash_deserialize(&mut reader)?), }) } @@ -511,7 +512,7 @@ impl Codec { fn read_tx(&self, mut reader: R) -> Result { Ok(Message::Tx { version: Version(reader.read_u32::()?), - transaction: Transaction::zcash_deserialize(&mut reader)?, + transaction: Box::new(Transaction::zcash_deserialize(&mut reader)?), }) } @@ -546,9 +547,7 @@ impl Codec { let mut bytes = Vec::new(); // Maximum size of data is 520 bytes. - let mut handle = reader.take(520); - - handle.read(&mut bytes)?; + reader.take(520).read_exact(&mut bytes)?; Ok(Message::FilterAdd { data: bytes }) } diff --git a/zebra-network/src/protocol/external/message.rs b/zebra-network/src/protocol/external/message.rs index c5f831e7a..12b72be1a 100644 --- a/zebra-network/src/protocol/external/message.rs +++ b/zebra-network/src/protocol/external/message.rs @@ -134,10 +134,11 @@ pub enum Message { /// [Bitcoin reference](https://en.bitcoin.it/wiki/Protocol_documentation#block) Block { /// Transaction data format version (note, this is signed). + // XXX does this get folded into the Block struct? version: Version, /// The block itself. - block: Block, + block: Box, }, /// A `getblocks` message. @@ -255,10 +256,11 @@ pub enum Message { // `tx_witnesses` aren't either, as they go if `flag` goes. Tx { /// Transaction data format version (note, this is signed). + // XXX do we still need this with the transaction data handling? version: Version, /// The `Transaction` type itself. - transaction: Transaction, + transaction: Box, }, /// A `mempool` message. diff --git a/zebra-network/src/protocol/external/types.rs b/zebra-network/src/protocol/external/types.rs index 0839b8de1..487974970 100644 --- a/zebra-network/src/protocol/external/types.rs +++ b/zebra-network/src/protocol/external/types.rs @@ -1,4 +1,3 @@ -use hex; use std::fmt; #[cfg(test)] @@ -29,7 +28,7 @@ bitflags! { /// NODE_NETWORK means that the node is a full node capable of serving /// blocks, as opposed to a light client that makes network requests but /// does not provide network services. - const NODE_NETWORK = (1 << 0); + const NODE_NETWORK = 1; } } diff --git a/zebrad/src/commands.rs b/zebrad/src/commands.rs index e3101f047..635461de3 100644 --- a/zebrad/src/commands.rs +++ b/zebrad/src/commands.rs @@ -57,7 +57,7 @@ impl Configurable for ZebradCmd { let if_exists = |f: PathBuf| if f.exists() { Some(f) } else { None }; - filename.and_then(|f| if_exists(f)) + filename.and_then(if_exists) } /// Apply changes to the config after it's been loaded, e.g. overriding diff --git a/zebrad/src/commands/connect.rs b/zebrad/src/commands/connect.rs index f80fdb381..038f6011c 100644 --- a/zebrad/src/commands/connect.rs +++ b/zebrad/src/commands/connect.rs @@ -49,11 +49,9 @@ impl ConnectCmd { use tower::{buffer::Buffer, service_fn, Service, ServiceExt}; let node = Buffer::new( - service_fn(|req| { - async move { - info!(?req); - Ok::(Response::Ok) - } + service_fn(|req| async move { + info!(?req); + Ok::(Response::Ok) }), 1, ); diff --git a/zebrad/src/commands/seed.rs b/zebrad/src/commands/seed.rs index 1c12cbeb5..fc5210daa 100644 --- a/zebrad/src/commands/seed.rs +++ b/zebrad/src/commands/seed.rs @@ -41,20 +41,20 @@ impl Service for SeedService { #[instrument(skip(self, _cx))] fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { match self.state { - SeederState::Ready(_) => return Poll::Ready(Ok(())), + SeederState::Ready(_) => Poll::Ready(Ok(())), SeederState::AwaitingAddressBook(ref mut rx) => match rx.try_recv() { Err(e) => { error!("oneshot sender dropped, failing service: {:?}", e); - return Poll::Ready(Err(e.into())); + Poll::Ready(Err(e.into())) } Ok(None) => { trace!("awaiting address book, service is unready"); - return Poll::Pending; + Poll::Pending } Ok(Some(address_book)) => { debug!("received address_book via oneshot, service becomes ready"); self.state = SeederState::Ready(address_book); - return Poll::Ready(Ok(())); + Poll::Ready(Ok(())) } }, } @@ -95,7 +95,7 @@ impl Service for SeedService { Ok(Response::Ok) } }; - return Box::pin(futures::future::ready(response)); + Box::pin(futures::future::ready(response)) } }