Merge bitcoindevkit/rust-electrum-client#139: chore: set rust edition to 2021, fix clippy, add ci fmt and clippy checks

0dd51408a3 chore(ci): add fmt and clippy checks (Steve Myers)
e7af332684 chore: fix clippy errors and set msrv to 1.63 (Steve Myers)
84f4f609b9 chore: set rust edition to 2021 (Steve Myers)

Pull request description:

  A little house keeping to avoid the warning about missing the edition. After setting the rust edition to 2021 I had to fix clippy warnings.  I also added CI fmt and clippy checks.

ACKs for top commit:
  ValuedMammal:
    ACK 0dd51408a3
  oleonardolima:
    ACK 0dd51408a3

Tree-SHA512: ee37c3ef06e217f76dcb622cdbb53e9b5c94163b789ca584037d5a8bb4bdef13c6cf251b555ac0e0ede309d7436295f1ede4e2ac6c0dc141c9bf2e3d6ecf9238

.github/workflows/cont_integration.yml

@@ -3,12 +3,11 @@ on: [push, pull_request]
 name: CI
 
 jobs:
-  test-fmt:
+  test:
     name: Test
     runs-on: ubuntu-20.04
     env:
       TEST_ELECTRUM_SERVER: electrum.blockstream.info:50001
-      #TEST_ELECTRUM_SERVER: bitcoin.aranguren.org:50001
     strategy:
       matrix:
         rust:
@@ -16,7 +15,7 @@ jobs:
           - 1.63.0 # MSRV
     steps:
       - name: Checkout
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
       - name: Cache
         uses: actions/cache@v2
         with:
@@ -25,14 +24,10 @@ jobs:
             ~/.cargo/git
             target
           key: ${{ runner.os }}-cargo-${{ github.job }}-${{ hashFiles('**/Cargo.toml','**/Cargo.lock') }}
-      - name: Install rustup
-        run: curl https://sh.rustup.rs -sSf | sh -s -- -y
-      - name: Set default toolchain
-        run: $HOME/.cargo/bin/rustup default ${{ matrix.rust }}
-      - name: Set profile
-        run: $HOME/.cargo/bin/rustup set profile minimal
-      - name: Fmt
-        run: cargo fmt -- --check --verbose
+      - name: Install rust
+        uses: dtolnay/rust-toolchain@stable
+        with:
+          toolchain: ${{ matrix.rust }}
       - name: Test
         run: cargo test --verbose --all-features
       - name: Setup iptables for the timeout test
@@ -46,3 +41,32 @@ jobs:
       - run: cargo check --verbose --no-default-features --features=proxy,use-openssl
       - run: cargo check --verbose --no-default-features --features=proxy,use-rustls
       - run: cargo check --verbose --no-default-features --features=proxy,use-rustls-ring
+
+  fmt:
+    name: Rust fmt
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+      - name: Install Rust toolchain
+        uses: dtolnay/rust-toolchain@stable
+        with:
+          toolchain: stable
+          components: rustfmt
+      - name: Check fmt
+        run: cargo fmt --all -- --config format_code_in_doc_comments=true --check
+
+  clippy_check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@stable
+        with:
+          toolchain: 1.78.0
+          components: clippy
+      - name: Rust Cache
+        uses: Swatinem/rust-cache@v2.2.1
+      - uses: actions-rs/clippy-check@v1
+        with:
+          token: ${{ secrets.GITHUB_TOKEN }}
+          args: --all-features --all-targets -- -D warnings

Cargo.toml

@@ -10,6 +10,7 @@ description = "Bitcoin Electrum client library. Supports plaintext, TLS and Onio
 keywords = ["bitcoin", "electrum"]
 readme = "README.md"
 rust-version = "1.63.0"
+edition = "2021"
 
 # loosely based on https://github.com/evgeniy-scherbina/rust-electrumx-client
 

clippy.toml

@@ -0,0 +1 @@
+msrv="1.63.0"

src/api.rs

@@ -6,8 +6,8 @@ use std::convert::TryInto;
 use bitcoin::consensus::encode::{deserialize, serialize};
 use bitcoin::{block, Script, Transaction, Txid};
 
-use batch::Batch;
-use types::*;
+use crate::batch::Batch;
+use crate::types::*;
 
 /// API calls exposed by an Electrum client
 pub trait ElectrumApi {

src/batch.rs

@@ -4,7 +4,7 @@
 
 use bitcoin::{Script, Txid};
 
-use types::{Call, Param, ToElectrumScriptHash};
+use crate::types::{Call, Param, ToElectrumScriptHash};
 
 /// Helper structure that caches all the requests before they are actually sent to the server.
 ///
@@ -16,6 +16,7 @@ use types::{Call, Param, ToElectrumScriptHash};
 /// [`Client`](../client/struct.Client.html), like
 /// [`batch_script_get_balance`](../client/struct.Client.html#method.batch_script_get_balance) to ask the
 /// server for the balance of multiple scripts with a single request.
+#[derive(Default)]
 pub struct Batch {
     calls: Vec<Call>,
 }
@@ -107,9 +108,3 @@ impl<'a> std::iter::Iterator for BatchIter<'a> {
         val
     }
 }
-
-impl std::default::Default for Batch {
-    fn default() -> Self {
-        Batch { calls: Vec::new() }
-    }
-}

src/client.rs

@@ -6,12 +6,12 @@ use log::{info, warn};
 
 use bitcoin::{Script, Txid};
 
-use api::ElectrumApi;
-use batch::Batch;
-use config::Config;
-use raw_client::*;
+use crate::api::ElectrumApi;
+use crate::batch::Batch;
+use crate::config::Config;
+use crate::raw_client::*;
+use crate::types::*;
 use std::convert::TryFrom;
-use types::*;
 
 /// Generalized Electrum client that supports multiple backends. This wraps
 /// [`RawClient`](client/struct.RawClient.html) and provides a more user-friendly
@@ -148,7 +148,6 @@ impl Client {
     /// If no prefix is specified, then `tcp://` is assumed.
     ///
     /// See [Client::from_config] for more configuration options
-    ///
     pub fn new(url: &str) -> Result<Self, Error> {
         Self::from_config(url, Config::default())
     }
@@ -353,7 +352,7 @@ mod tests {
     fn more_failed_attempts_than_retries_means_exhausted() {
         let exhausted = retries_exhausted(10, 5);
 
-        assert_eq!(exhausted, true)
+        assert!(exhausted)
     }
 
     #[test]
@@ -362,21 +361,21 @@ mod tests {
 
         let exhausted = retries_exhausted(failed_attempts, u8::MAX);
 
-        assert_eq!(exhausted, true)
+        assert!(exhausted)
     }
 
     #[test]
     fn less_failed_attempts_means_not_exhausted() {
         let exhausted = retries_exhausted(2, 5);
 
-        assert_eq!(exhausted, false)
+        assert!(!exhausted)
     }
 
     #[test]
     fn attempts_equals_retries_means_not_exhausted_yet() {
         let exhausted = retries_exhausted(2, 2);
 
-        assert_eq!(exhausted, false)
+        assert!(!exhausted)
     }
 
     #[test]
@@ -408,7 +407,7 @@ mod tests {
             sender.send(()).unwrap();
 
             for _stream in listener.incoming() {
-                loop {}
+                std::thread::sleep(Duration::from_secs(60))
             }
         });
 

src/raw_client.rs

@@ -39,11 +39,11 @@ use rustls::{
 #[cfg(any(feature = "default", feature = "proxy"))]
 use crate::socks::{Socks5Stream, TargetAddr, ToTargetAddr};
 
-use stream::ClonableStream;
+use crate::stream::ClonableStream;
 
-use api::ElectrumApi;
-use batch::Batch;
-use types::*;
+use crate::api::ElectrumApi;
+use crate::batch::Batch;
+use crate::types::*;
 
 macro_rules! impl_batch_call {
     ( $self:expr, $data:expr, $call:ident ) => {{
@@ -83,7 +83,7 @@ pub trait ToSocketAddrsDomain: ToSocketAddrs {
 
 impl ToSocketAddrsDomain for &str {
     fn domain(&self) -> Option<&str> {
-        self.splitn(2, ':').next()
+        self.split(':').next()
     }
 }
 
@@ -298,7 +298,7 @@ impl RawClient<ElectrumSslStream> {
     not(feature = "use-openssl")
 ))]
 mod danger {
-    use raw_client::ServerName;
+    use crate::raw_client::ServerName;
     use rustls::client::danger::ServerCertVerified;
     use rustls::pki_types::CertificateDer;
     use rustls::pki_types::UnixTime;
@@ -406,11 +406,11 @@ impl RawClient<ElectrumSslStream> {
             socket_addr.domain().ok_or(Error::MissingDomain)?;
 
             let store = webpki_roots::TLS_SERVER_ROOTS
-                .into_iter()
+                .iter()
                 .map(|t| TrustAnchor {
                     subject: Der::from_slice(t.subject),
                     subject_public_key_info: Der::from_slice(t.spki),
-                    name_constraints: t.name_constraints.map(|nc| Der::from_slice(nc)),
+                    name_constraints: t.name_constraints.map(Der::from_slice),
                 })
                 .collect::<RootCertStore>();
 
@@ -605,7 +605,7 @@ impl<S: Read + Write> RawClient<S> {
                             // No id, that's probably a notification.
                             let mut resp = resp;
 
-                            if let Some(ref method) = resp["method"].take().as_str() {
+                            if let Some(method) = resp["method"].take().as_str() {
                                 self.handle_notification(method, resp["params"].take())?;
                             } else {
                                 warn!("Unexpected response: {:?}", resp);
@@ -722,7 +722,7 @@ impl<S: Read + Write> RawClient<S> {
     ) -> Result<serde_json::Value, Error> {
         let req = Request::new_id(
             self.last_id.fetch_add(1, Ordering::SeqCst),
-            &method_name,
+            method_name,
             params,
         );
         let result = self.call(req)?;
@@ -763,7 +763,7 @@ impl<T: Read + Write> ElectrumApi for RawClient<T> {
         for (method, params) in batch.iter() {
             let req = Request::new_id(
                 self.last_id.fetch_add(1, Ordering::SeqCst),
-                &method,
+                method,
                 params.to_vec(),
             );
             missing_responses.insert(req.id);
@@ -804,7 +804,7 @@ impl<T: Read + Write> ElectrumApi for RawClient<T> {
             };
         }
 
-        Ok(answers.into_iter().map(|(_, r)| r).collect())
+        Ok(answers.into_values().collect())
     }
 
     fn block_headers_subscribe_raw(&self) -> Result<RawHeaderNotification, Error> {
@@ -1128,7 +1128,7 @@ mod test {
     use crate::utils;
 
     use super::RawClient;
-    use api::ElectrumApi;
+    use crate::api::ElectrumApi;
 
     fn get_test_server() -> String {
         std::env::var("TEST_ELECTRUM_SERVER").unwrap_or("electrum.blockstream.info:50001".into())
@@ -1426,7 +1426,7 @@ mod test {
 
     #[test]
     fn test_raw_call() {
-        use types::Param;
+        use crate::types::Param;
 
         let client = RawClient::new(get_test_server(), None).unwrap();
 

src/socks/v4.rs

@@ -109,7 +109,7 @@ impl Socks4Stream {
                 let _ = packet.write_u32::<BigEndian>(Ipv4Addr::new(0, 0, 0, 1).into());
                 let _ = packet.write_all(userid.as_bytes());
                 let _ = packet.write_u8(0);
-                let _ = packet.extend(host.as_bytes());
+                packet.extend(host.as_bytes());
                 let _ = packet.write_u8(0);
             }
         }
@@ -117,10 +117,7 @@ impl Socks4Stream {
         socket.write_all(&packet)?;
         let proxy_addr = read_response(&mut socket)?;
 
-        Ok(Socks4Stream {
-            socket: socket,
-            proxy_addr: proxy_addr,
-        })
+        Ok(Socks4Stream { socket, proxy_addr })
     }
 
     /// Returns the proxy-side address of the connection between the proxy and

src/socks/v5.rs

@@ -110,7 +110,7 @@ fn write_addr(mut packet: &mut [u8], target: &TargetAddr) -> io::Result<usize> {
         }
         TargetAddr::Domain(ref domain, port) => {
             packet.write_u8(3).unwrap();
-            if domain.len() > u8::max_value() as usize {
+            if domain.len() > u8::MAX as usize {
                 return Err(io::Error::new(
                     io::ErrorKind::InvalidInput,
                     "domain name too long",
@@ -144,11 +144,7 @@ impl<'a> Authentication<'a> {
     }
 
     fn is_no_auth(&self) -> bool {
-        if let Authentication::None = *self {
-            true
-        } else {
-            false
-        }
+        matches!(*self, Authentication::None)
     }
 }
 
@@ -257,10 +253,7 @@ impl Socks5Stream {
 
         let proxy_addr = read_response(&mut socket)?;
 
-        Ok(Socks5Stream {
-            socket: socket,
-            proxy_addr: proxy_addr,
-        })
+        Ok(Socks5Stream { socket, proxy_addr })
     }
 
     fn password_authentication(
@@ -268,13 +261,13 @@ impl Socks5Stream {
         username: &str,
         password: &str,
     ) -> io::Result<()> {
-        if username.len() < 1 || username.len() > 255 {
+        if username.is_empty() || username.len() > 255 {
             return Err(io::Error::new(
                 io::ErrorKind::InvalidInput,
                 "invalid username",
             ));
         };
-        if password.len() < 1 || password.len() > 255 {
+        if password.is_empty() || password.len() > 255 {
             return Err(io::Error::new(
                 io::ErrorKind::InvalidInput,
                 "invalid password",
@@ -474,10 +467,7 @@ impl Socks5Datagram {
         let socket = UdpSocket::bind(addr)?;
         socket.connect(&stream.proxy_addr)?;
 
-        Ok(Socks5Datagram {
-            socket: socket,
-            stream: stream,
-        })
+        Ok(Socks5Datagram { socket, stream })
     }
 
     /// Like `UdpSocket::send_to`.
@@ -526,11 +516,7 @@ impl Socks5Datagram {
         let addr = read_addr(&mut header)?;
 
         unsafe {
-            ptr::copy(
-                buf.as_ptr(),
-                buf.as_mut_ptr().offset(header.len() as isize),
-                overflow,
-            );
+            ptr::copy(buf.as_ptr(), buf.as_mut_ptr().add(header.len()), overflow);
         }
         buf[..header.len()].copy_from_slice(header);
 

src/types.rs

@@ -87,7 +87,7 @@ impl From<[u8; 32]> for Hex32Bytes {
 }
 
 impl Hex32Bytes {
-    pub(crate) fn to_hex(&self) -> String {
+    pub(crate) fn to_hex(self) -> String {
         self.0.to_lower_hex_string()
     }
 }

src/utils.rs

@@ -1,10 +1,10 @@
 //! Utilities helping to handle Electrum-related data.
 
+use crate::types::GetMerkleRes;
 use bitcoin::hash_types::TxMerkleNode;
 use bitcoin::hashes::sha256d::Hash as Sha256d;
 use bitcoin::hashes::{Hash, HashEngine};
 use bitcoin::Txid;
-use types::GetMerkleRes;
 
 /// Verifies a Merkle inclusion proof as retrieved via [`transaction_get_merkle`] for a transaction with the
 /// given `txid` and `merkle_root` as included in the [`BlockHeader`].