Merge pull request #11 from datachainlab/fix-lint-warnings

Fix lint warnings

Signed-off-by: Jun Kimura <[email protected]>

.github/workflows/no-std.yaml

@@ -21,11 +21,10 @@ jobs:
     name: Check no_std panic conflict
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions-rs/toolchain@v1
+      - uses: actions/checkout@v4
+      - uses: actions-rust-lang/setup-rust-toolchain@v1
         with:
           toolchain: stable
-          override: true
       - run: |
           cd ci/no-std-check
           make check-panic-conflict
@@ -34,12 +33,11 @@ jobs:
     name: Check no_std substrate support
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions-rs/toolchain@v1
+      - uses: actions/checkout@v4
+      - uses: actions-rust-lang/setup-rust-toolchain@v1
         with:
           toolchain: nightly
           target: wasm32-unknown-unknown
-          override: true
       - run: |
           cd ci/no-std-check
           make check-substrate

.github/workflows/test.yml

@@ -20,10 +20,9 @@ jobs:
   test:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions-rs/toolchain@v1
-        with:
-          toolchain: stable
-      - run: make lint
+      - uses: actions/checkout@v4
+      - uses: actions-rust-lang/setup-rust-toolchain@v1
+      - uses: Swatinem/rust-cache@v2
       - run: cargo test
-      - run: cargo build --bin ethlc
+      - run: cargo build
+      - run: make lint-tools lint

Makefile

@@ -1,3 +1,16 @@
+######## Lint ########
+
+.PHONY: lint-tools
+lint-tools:
+	rustup component add rustfmt clippy
+	cargo install cargo-machete
+
+.PHONY: fmt
+fmt:
+	@cargo fmt --all $(CARGO_FMT_OPT)
+
 .PHONY: lint
 lint:
-	@cargo fmt --all -- --check
+	@$(MAKE) CARGO_FMT_OPT=--check fmt
+	@cargo clippy --locked --tests $(CARGO_TARGET) -- -D warnings
+	@cargo machete

crates/consensus/src/bellatrix.rs

@@ -317,7 +317,7 @@ pub fn gen_execution_payload_proof<
     branch.copy_from_slice(
         tree.proof(&[BLOCK_BODY_EXECUTION_PAYLOAD_LEAF_INDEX])
             .proof_hashes()
-            .into_iter()
+            .iter()
             .map(|h| H256::from_slice(h))
             .collect::<Vec<H256>>()
             .as_slice(),
@@ -357,7 +357,7 @@ pub fn gen_execution_payload_field_proof<
     branch.copy_from_slice(
         tree.proof(&[leaf_index])
             .proof_hashes()
-            .into_iter()
+            .iter()
             .map(|h| H256::from_slice(h))
             .collect::<Vec<H256>>()
             .as_slice(),

crates/consensus/src/bls.rs

@@ -80,9 +80,7 @@ pub struct PublicKeyBytesDef(
 
 impl From<PublicKeyBytesDef> for PublicKeyBytes {
     fn from(value: PublicKeyBytesDef) -> Self {
-        Self(Vector::<u8, PUBLIC_KEY_BYTES_LEN>::from_iter(
-            value.0.into_iter(),
-        ))
+        Self(Vector::<u8, PUBLIC_KEY_BYTES_LEN>::from_iter(value.0))
     }
 }
 
@@ -186,9 +184,7 @@ pub struct SignatureBytesDef(
 
 impl From<SignatureBytesDef> for SignatureBytes {
     fn from(value: SignatureBytesDef) -> Self {
-        Self(Vector::<u8, SIGNATURE_BYTES_LEN>::from_iter(
-            value.0.into_iter(),
-        ))
+        Self(Vector::<u8, SIGNATURE_BYTES_LEN>::from_iter(value.0))
     }
 }
 

crates/consensus/src/capella.rs

@@ -360,7 +360,7 @@ pub fn gen_execution_payload_proof<
     branch.copy_from_slice(
         tree.proof(&[BLOCK_BODY_EXECUTION_PAYLOAD_LEAF_INDEX])
             .proof_hashes()
-            .into_iter()
+            .iter()
             .map(|h| H256::from_slice(h))
             .collect::<Vec<H256>>()
             .as_slice(),
@@ -400,7 +400,7 @@ pub fn gen_execution_payload_field_proof<
     branch.copy_from_slice(
         tree.proof(&[leaf_index])
             .proof_hashes()
-            .into_iter()
+            .iter()
             .map(|h| H256::from_slice(h))
             .collect::<Vec<H256>>()
             .as_slice(),

crates/consensus/src/compute.rs

@@ -52,10 +52,10 @@ pub fn compute_fork_data_root(
     current_version: Version,
     genesis_validators_root: Root,
 ) -> Result<Root, Error> {
-    Ok(hash_tree_root(ForkData {
+    hash_tree_root(ForkData {
         current_version,
         genesis_validators_root,
-    })?)
+    })
 }
 
 /// https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#compute_domain
@@ -77,10 +77,10 @@ pub fn compute_domain<C: ChainContext>(
 
 /// https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#compute_signing_root
 pub fn compute_signing_root(header: BeaconBlockHeader, domain: Domain) -> Result<Root, Error> {
-    Ok(hash_tree_root(SigningData {
+    hash_tree_root(SigningData {
         object_root: hash_tree_root(header)?,
         domain,
-    })?)
+    })
 }
 
 /// hash_tree_root returns the hash tree root of the object

crates/consensus/src/deneb.rs

@@ -377,7 +377,7 @@ pub fn gen_execution_payload_proof<
         H256(tree.root().unwrap()),
         tree.proof(&[9])
             .proof_hashes()
-            .into_iter()
+            .iter()
             .map(|h| H256::from_slice(h))
             .collect(),
     ))
@@ -431,7 +431,7 @@ pub fn gen_execution_payload_field_proof<
     branch.copy_from_slice(
         tree.proof(&[leaf_index])
             .proof_hashes()
-            .into_iter()
+            .iter()
             .map(|h| H256::from_slice(h))
             .collect::<Vec<H256>>()
             .as_slice(),

crates/consensus/src/lib.rs

@@ -1,4 +1,5 @@
 #![cfg_attr(all(not(test), not(feature = "std")), no_std)]
+#![allow(clippy::result_large_err)]
 extern crate alloc;
 
 #[allow(unused_imports)]

crates/consensus/src/merkle.rs

@@ -23,7 +23,7 @@ pub fn is_valid_merkle_branch(
         ));
     }
     let index = 2u64.pow(depth) + leaf_index;
-    let mut value = leaf.clone();
+    let mut value = leaf;
     for (i, b) in branch.iter().enumerate() {
         if let Some(v) = 2u64.checked_pow(i as u32) {
             if index / v % 2 == 1 {

crates/light-client-cli/Cargo.toml

@@ -18,10 +18,6 @@ dirs = "4.0"
 env_logger = { version = "0.10.0" }
 tokio = { version = "1.24.1", default-features = false, features = ["rt-multi-thread", "macros"] }
 
-ssz-rs = { git = "https://github.com/bluele/ssz_rs", branch = "serde-no-std", default-features = false, features = ["serde"] }
-ssz-rs-derive = { git = "https://github.com/bluele/ssz_rs", branch = "serde-no-std", default-features = false }
-rs_merkle = { git = "https://github.com/antouhou/rs-merkle", rev = "8ffa623ce70a3659ae73619397b813887cd8d1c9" }
-
 ethereum-consensus = { path = "../consensus" }
 ethereum-light-client-verifier = { path = "../light-client-verifier" }
 lodestar-rpc = { path = "../lodestar-rpc" }

crates/light-client-cli/bin/main.rs

@@ -6,5 +6,5 @@ async fn main() -> anyhow::Result<()> {
     env_logger::init_from_env(
         env_logger::Env::default().filter_or(env_logger::DEFAULT_FILTER_ENV, "info"),
     );
-    Ok(Cli::parse().run().await?)
+    Cli::parse().run().await
 }

crates/light-client-cli/src/chain.rs

@@ -5,6 +5,7 @@ use ethereum_consensus::{
 };
 use ethereum_light_client_verifier::updates::deneb::LightClientBootstrapInfo;
 use lodestar_rpc::client::RPCClient;
+use std::str::FromStr;
 
 type Result<T> = core::result::Result<T, Error>;
 
@@ -60,24 +61,28 @@ pub enum Network {
 }
 
 impl Network {
-    pub fn from_str(s: &str) -> Result<Self> {
+    pub fn config(&self) -> Config {
+        match self {
+            Network::Minimal => config::minimal::get_config(),
+            Network::Mainnet => config::mainnet::get_config(),
+            Network::Goerli => config::goerli::get_config(),
+            Network::Sepolia => config::sepolia::get_config(),
+        }
+    }
+}
+
+impl FromStr for Network {
+    type Err = Error;
+
+    fn from_str(s: &str) -> Result<Self> {
         match s.to_lowercase().as_str() {
             "minimal" => Ok(Network::Minimal),
             "mainnet" => Ok(Network::Mainnet),
             "goerli" => Ok(Network::Goerli),
             "sepolia" => Ok(Network::Sepolia),
             s => Err(Error::Other {
-                description: format!("unknown network: {}", s).into(),
+                description: format!("unknown network: {}", s),
             }),
         }
     }
-
-    pub fn config(&self) -> Config {
-        match self {
-            Network::Minimal => config::minimal::get_config(),
-            Network::Mainnet => config::mainnet::get_config(),
-            Network::Goerli => config::goerli::get_config(),
-            Network::Sepolia => config::sepolia::get_config(),
-        }
-    }
 }

crates/light-client-cli/src/cli.rs

@@ -1,6 +1,6 @@
 use anyhow::Result;
 use clap::Parser;
-use std::path::PathBuf;
+use std::{path::PathBuf, str::FromStr};
 
 use crate::{
     chain::Network,

crates/light-client-cli/src/client.rs

@@ -77,7 +77,7 @@ impl<
     }
 
     pub async fn init_with_bootstrap(&self, trusted_block_root: Option<H256>) -> Result<()> {
-        let bootstrap = self.chain.get_bootstrap(trusted_block_root.clone()).await?;
+        let bootstrap = self.chain.get_bootstrap(trusted_block_root).await?;
 
         self.verifier
             .validate_boostrap(&bootstrap, trusted_block_root)?;
@@ -257,7 +257,7 @@ impl<
                 info!("updates: finalized header not found");
                 return Ok(None);
             }
-            Err(e) => return Err(e.into()),
+            Err(e) => return Err(e),
         };
 
         info!(
@@ -307,7 +307,7 @@ impl<
     fn build_verification_context(&self) -> impl ChainContext + ConsensusVerificationContext {
         LightClientContext::new_with_config(
             self.ctx.config.clone(),
-            self.genesis_validators_root.clone(),
+            self.genesis_validators_root,
             self.genesis_time,
             self.trust_level.clone(),
             SystemTime::now()
@@ -375,8 +375,8 @@ impl PartialEq<Updated> for Target {
 impl PartialOrd<Updated> for Target {
     fn partial_cmp(&self, other: &Updated) -> Option<core::cmp::Ordering> {
         match self {
-            Target::Slot(v) => v.partial_cmp(&other.0.into()),
-            Target::BlockNumber(v) => v.partial_cmp(&other.1.into()),
+            Target::Slot(v) => v.partial_cmp(&other.0),
+            Target::BlockNumber(v) => v.partial_cmp(&other.1),
             Target::None => Some(core::cmp::Ordering::Less),
             Target::Infinity => Some(core::cmp::Ordering::Greater),
         }

crates/light-client-cli/src/context.rs

@@ -6,6 +6,7 @@ use ethereum_consensus::config::Config;
 use ethereum_consensus::context::ChainContext;
 use ethereum_consensus::deneb::LightClientBootstrap;
 use log::*;
+use std::str::FromStr;
 
 #[derive(Debug)]
 pub struct Context<

crates/light-client-cli/src/lib.rs

@@ -1,3 +1,4 @@
+#![allow(clippy::result_large_err)]
 pub mod chain;
 pub mod cli;
 pub mod client;

crates/light-client-cli/src/state.rs

@@ -75,7 +75,7 @@ pub struct ExecutionUpdateInfo {
 
 impl ExecutionUpdate for ExecutionUpdateInfo {
     fn state_root(&self) -> H256 {
-        self.state_root.clone()
+        self.state_root
     }
 
     fn state_root_branch(&self) -> Vec<H256> {

crates/light-client-verifier/Cargo.toml

@@ -6,7 +6,6 @@ edition = "2021"
 [dependencies]
 serde = { version = "1.0", default-features = false, features = ["alloc", "derive"] }
 displaydoc = { version = "0.2", default-features = false }
-log = { version = "0.4.17", default-features = false }
 
 ethereum-consensus = { path = "../consensus", default-features = false }
 trie-db = { version= "0.24.0", default-features = false }

crates/light-client-verifier/src/consensus.rs

@@ -54,7 +54,7 @@ impl<
             &bootstrap.current_sync_committee_branch(),
             CURRENT_SYNC_COMMITTEE_DEPTH as u32,
             CURRENT_SYNC_COMMITTEE_SUBTREE_INDEX,
-            bootstrap.beacon_header().state_root.clone(),
+            bootstrap.beacon_header().state_root,
         )
         .map_err(Error::InvalidCurrentSyncCommitteeMerkleBranch)?;
         Ok(())
@@ -112,7 +112,7 @@ impl<
             &update.state_root_branch(),
             EXECUTION_PAYLOAD_TREE_DEPTH as u32,
             EXECUTION_PAYLOAD_STATE_ROOT_LEAF_INDEX as u64,
-            trusted_execution_root.clone(),
+            trusted_execution_root,
         )
         .map_err(Error::InvalidExecutionStateRootMerkleBranch)?;
 
@@ -282,6 +282,7 @@ pub fn verify_sync_committee_attestation<
 }
 
 /// validate_light_client_update validates a light client update
+///
 /// NOTE: we can skip the validation of the attested header's execution payload here because we do not reference it in the light client protocol
 pub fn validate_light_client_update<
     const SYNC_COMMITTEE_SIZE: usize,
@@ -339,7 +340,7 @@ pub fn validate_light_client_update<
         &consensus_update.finalized_beacon_header_branch(),
         FINALIZED_ROOT_DEPTH as u32,
         FINALIZED_ROOT_SUBTREE_INDEX,
-        consensus_update.attested_beacon_header().state_root.clone(),
+        consensus_update.attested_beacon_header().state_root,
     )
     .map_err(Error::InvalidFinalizedBeaconHeaderMerkleBranch)?;
 
@@ -369,22 +370,19 @@ pub fn validate_light_client_update<
                 return Err(Error::InconsistentNextSyncCommittee(
                     store_next_sync_committee.aggregate_pubkey.clone(),
                     update_next_sync_committee.aggregate_pubkey.clone(),
-                )
-                .into());
+                ));
             }
         }
         is_valid_merkle_branch(
             hash_tree_root(update_next_sync_committee.clone())?,
             &consensus_update.next_sync_committee_branch().unwrap(),
             NEXT_SYNC_COMMITTEE_DEPTH as u32,
             NEXT_SYNC_COMMITTEE_SUBTREE_INDEX,
-            consensus_update.attested_beacon_header().state_root.clone(),
+            consensus_update.attested_beacon_header().state_root,
         )
         .map_err(Error::InvalidNextSyncCommitteeMerkleBranch)?;
-    } else {
-        if let Some(branch) = consensus_update.next_sync_committee_branch() {
-            return Err(Error::NonEmptyNextSyncCommittee(branch.to_vec()));
-        }
+    } else if let Some(branch) = consensus_update.next_sync_committee_branch() {
+        return Err(Error::NonEmptyNextSyncCommittee(branch.to_vec()));
     }
 
     Ok(())