feat(ci): fix make clippy script and clippy CI job (#220)

* feat(ci): Fix make clippy script and clippy CI job

* fix: Install toolchain required for clippy

* fix: Fix clippy lints

* chore(clippy): fix lints and improve clippy script

* chore(clippy): build risc0 guest to compile driver and host

* chore(clippy): add env vars

* chore(clippy): fix lints

* chore(clippy): do not specify projects for risc0 command

* chore(clippy): install tools before running clippy

* chore(ci): remove unused file

* refactor(clippy): use build script instead of separate clippy script

* refactor(clippy): move install script to separate step in job

Author: petarvujovic98

Diff for: .github/workflows/ci-build-test-reusable.yml

@@ -1,56 +1,56 @@
 name: CI Build and Test - Reusable
 
 on:
-    workflow_call:
-      inputs:
-        version_name:
-          type: string
-          required: true
-        version_toolchain:
-          type: string
-          required: true
+  workflow_call:
+    inputs:
+      version_name:
+        type: string
+        required: true
+      version_toolchain:
+        type: string
+        required: true
 
 env:
   CARGO_TERM_COLOR: always
   GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
 jobs:
-    build-test:
-      name: Build and test
-      runs-on: [taiko-runner]
-      timeout-minutes: 120
-
-      env:
-        TARGET: ${{ inputs.version_name}}
-        CI: 1
-        MOCK: 1
-  
-      steps:
-        - uses: actions/checkout@v4
-          with:
-            submodules: recursive
-  
-        - uses: actions-rs/toolchain@v1
-          with:
-              toolchain: ${{ inputs.version_toolchain }}
-              profile: minimal
-  
-        - name: Install cargo-binstall
-          uses: cargo-bins/[email protected]
-  
-        - name: Setup sccache
-          if: ${{ inputs.version_name }} == risc0
-          uses: risc0/risc0/.github/actions/[email protected]
-  
-        - name: Install ${{ inputs.version_name }}
-          run: make install
-  
-        - name: Build ${{ inputs.version_name }} prover
-          run: make build
-  
-        - name: Test ${{ inputs.version_name }} prover
-          run: make test
-          
-        - name: Build with tracer
-          if: ${{ inputs.version_name }} == native
-          run: cargo build -F tracer
+  build-test:
+    name: Build and test
+    runs-on: [taiko-runner]
+    timeout-minutes: 120
+
+    env:
+      TARGET: ${{ inputs.version_name }}
+      CI: 1
+      MOCK: 1
+
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          submodules: recursive
+
+      - uses: actions-rs/toolchain@v1
+        with:
+          toolchain: ${{ inputs.version_toolchain }}
+          profile: minimal
+
+      - name: Install cargo-binstall
+        uses: cargo-bins/[email protected]
+
+      - name: Setup sccache
+        if: ${{ inputs.version_name }} == risc0
+        uses: risc0/risc0/.github/actions/[email protected]
+
+      - name: Install ${{ inputs.version_name }}
+        run: make install
+
+      - name: Build ${{ inputs.version_name }} prover
+        run: make build
+
+      - name: Test ${{ inputs.version_name }} prover
+        run: make test
+
+      - name: Build with tracer
+        if: ${{ inputs.version_name }} == native
+        run: cargo build -F tracer

Diff for: .github/workflows/ci-lint.yml

@@ -1,8 +1,8 @@
 name: CI - Lint
 
 on:
-    pull_request:
-      types: [opened, reopened, edited, synchronize]
+  pull_request:
+    types: [opened, reopened, edited, synchronize]
 
 env:
   CARGO_TERM_COLOR: always
@@ -17,15 +17,11 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
-      - uses: risc0/risc0/.github/actions/[email protected]
+      - name: Run install script for all targets
+        run: make install
 
-      - uses: risc0/risc0/.github/actions/[email protected]
-
-      - uses: risc0/clippy-action@main
-        with:
-          reporter: 'github-pr-check'
-          fail_on_error: true
-          clippy_flags: --workspace --all-targets --all-features -- -D warnings
+      - name: Run clippy check for all targets
+        run: make clippy
 
   fmt:
     name: fmt
@@ -35,6 +31,5 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
-      - uses: risc0/risc0/.github/actions/[email protected]
-
-      - run: make fmt
+      - name: Run format script for all targets
+        run: make fmt

Diff for: core/src/lib.rs

@@ -169,7 +169,7 @@ pub fn merge(a: &mut Value, b: &Value) {
                 merge(a.entry(k).or_insert(Value::Null), v);
             }
         }
-        (a, b) if !b.is_null() => *a = b.to_owned(),
+        (a, b) if !b.is_null() => b.clone_into(a),
         // If b is null, just keep a (which means do nothing).
         _ => {}
     }

Diff for: harness/core/src/assert.rs

@@ -60,6 +60,10 @@ impl AssertionLog {
         self.assertions.len()
     }
 
+    pub fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+
     pub fn display_failures(&self, start: usize, end: usize) {
         for i in start..end {
             if let Some(assertion) = self.assertions.get(i) {

Diff for: harness/macro/src/lib.rs

@@ -1,9 +1,39 @@
 extern crate proc_macro;
 use proc_macro::TokenStream;
 use quote::quote;
-use syn::{parse_macro_input, punctuated::Punctuated, Ident, Item, ItemFn, ItemMod, Path, Token};
+use syn::{parse_macro_input, Item, ItemFn, ItemMod};
+
+#[cfg(any(feature = "sp1", feature = "risc0"))]
+use syn::{punctuated::Punctuated, Ident, Path, Token};
+
+// Helper struct to parse input
+#[cfg(any(feature = "sp1", feature = "risc0"))]
+struct EntryArgs {
+    main_entry: Ident,
+    test_modules: Option<Punctuated<Path, Token![,]>>,
+}
+
+#[cfg(any(feature = "sp1", feature = "risc0"))]
+impl syn::parse::Parse for EntryArgs {
+    fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
+        let main_entry: Ident = input.parse()?;
+        let test_modules: Option<Punctuated<Path, Token![,]>> = if input.peek(Token![,]) {
+            input.parse::<Token![,]>()?; // Parse and consume the comma
+                                         // Now parse a list of module paths if they are present
+            Some(input.parse_terminated(Path::parse)?)
+        } else {
+            None
+        };
+
+        Ok(EntryArgs {
+            main_entry,
+            test_modules,
+        })
+    }
+}
 
 #[proc_macro]
+#[cfg(any(feature = "sp1", feature = "risc0"))]
 pub fn entrypoint(input: TokenStream) -> TokenStream {
     let input = parse_macro_input!(input as EntryArgs);
     let main_entry = input.main_entry;
@@ -70,41 +100,21 @@ pub fn entrypoint(input: TokenStream) -> TokenStream {
         }
     };
 
-    #[cfg(all(not(feature = "sp1"), not(feature = "risc0")))]
-    let output = quote! {
+    output.into()
+}
+
+#[proc_macro]
+#[cfg(not(any(feature = "sp1", feature = "risc0")))]
+pub fn entrypoint(_input: TokenStream) -> TokenStream {
+    quote! {
         mod generated_main {
             #[no_mangle]
             fn main() {
                 super::ENTRY()
             }
         }
-    };
-
-    output.into()
-}
-
-// Helper struct to parse input
-struct EntryArgs {
-    main_entry: Ident,
-    test_modules: Option<Punctuated<Path, Token![,]>>,
-}
-
-impl syn::parse::Parse for EntryArgs {
-    fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
-        let main_entry: Ident = input.parse()?;
-        let test_modules: Option<Punctuated<Path, Token![,]>> = if input.peek(Token![,]) {
-            input.parse::<Token![,]>()?; // Parse and consume the comma
-                                         // Now parse a list of module paths if they are present
-            Some(input.parse_terminated(Path::parse)?)
-        } else {
-            None
-        };
-
-        Ok(EntryArgs {
-            main_entry,
-            test_modules,
-        })
     }
+    .into()
 }
 
 #[proc_macro]

Diff for: lib/src/builder/finalize.rs

@@ -42,8 +42,11 @@ impl BlockFinalizeStrategy<MemDb> for MemDbBlockFinalizeStrategy {
     fn finalize(mut block_builder: BlockBuilder<MemDb>) -> Result<(AlloyConsensusHeader, MptNode)> {
         let db: MemDb = block_builder.db.take().expect("DB not initialized");
 
-        let mut account_touched = 0;
-        let mut storage_touched = 0;
+        #[cfg(feature = "sp1-cycle-tracker")]
+        {
+            let mut account_touched = 0;
+            let mut storage_touched = 0;
+        }
 
         // apply state updates
         let mut state_trie = mem::take(&mut block_builder.input.parent_state_trie);
@@ -62,7 +65,10 @@ impl BlockFinalizeStrategy<MemDb> for MemDbBlockFinalizeStrategy {
                 continue;
             }
 
-            account_touched += 1;
+            #[cfg(feature = "sp1-cycle-tracker")]
+            {
+                account_touched += 1;
+            }
 
             // otherwise, compute the updated storage root for that account
             let state_storage = &account.storage;
@@ -88,7 +94,10 @@ impl BlockFinalizeStrategy<MemDb> for MemDbBlockFinalizeStrategy {
                         storage_trie.insert_rlp(&storage_trie_index, *value)?;
                     }
 
-                    storage_touched += 1;
+                    #[cfg(feature = "sp1-cycle-tracker")]
+                    {
+                        storage_touched += 1;
+                    }
                 }
 
                 storage_trie.hash()

Diff for: lib/src/builder/initialize.rs

@@ -62,8 +62,14 @@ impl DbInitStrategy<MemDb> for MemDbInitStrategy {
             .map(|bytes| (keccak(&bytes).into(), bytes))
             .collect();
 
-        let mut account_touched = 0;
-        let mut storage_touched = 0;
+        #[cfg(all(
+            all(target_os = "zkvm", target_vendor = "succinct"),
+            feature = "sp1-cycle-tracker"
+        ))]
+        {
+            let mut account_touched = 0;
+            let mut storage_touched = 0;
+        }
 
         // Load account data into db
         let mut accounts = HashMap::with_capacity(block_builder.input.parent_storage.len());
@@ -85,7 +91,13 @@ impl DbInitStrategy<MemDb> for MemDbInitStrategy {
                     storage_trie.hash()
                 );
             }
-            account_touched += 1;
+            #[cfg(all(
+                all(target_os = "zkvm", target_vendor = "succinct"),
+                feature = "sp1-cycle-tracker"
+            ))]
+            {
+                account_touched += 1;
+            }
 
             // load the corresponding code
             let code_hash = state_account.code_hash;
@@ -106,7 +118,13 @@ impl DbInitStrategy<MemDb> for MemDbInitStrategy {
                     .get_rlp(&keccak(slot.to_be_bytes::<32>()))?
                     .unwrap_or_default();
                 storage.insert(slot, value);
-                storage_touched += 1;
+                #[cfg(all(
+                    all(target_os = "zkvm", target_vendor = "succinct"),
+                    feature = "sp1-cycle-tracker"
+                ))]
+                {
+                    storage_touched += 1;
+                }
             }
 
             let mem_account = DbAccount {
@@ -129,8 +147,8 @@ impl DbInitStrategy<MemDb> for MemDbInitStrategy {
             feature = "sp1-cycle-tracker"
         ))]
         {
-            println!("initialize_db Account touch {:?}", account_touched);
-            println!("initialize_db Storage touch {:?}", storage_touched);
+            println!("initialize_db Account touch {account_touched:?}");
+            println!("initialize_db Storage touch {storage_touched:?}");
         }
 
         // prepare block hash history

Diff for: lib/src/consts.rs

@@ -250,14 +250,14 @@ pub enum Network {
     TaikoMainnet,
 }
 
-impl ToString for Network {
-    fn to_string(&self) -> String {
-        match self {
-            Network::Ethereum => "ethereum".to_string(),
-            Network::Holesky => "holesky".to_string(),
-            Network::TaikoA7 => "taiko_a7".to_string(),
-            Network::TaikoMainnet => "taiko_mainnet".to_string(),
-        }
+impl std::fmt::Display for Network {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        f.write_str(match self {
+            Network::Ethereum => "ethereum",
+            Network::Holesky => "holesky",
+            Network::TaikoA7 => "taiko_a7",
+            Network::TaikoMainnet => "taiko_mainnet",
+        })
     }
 }
 

Diff for: lib/src/lib.rs

@@ -86,6 +86,7 @@ mod time {
 }
 
 pub struct CycleTracker {
+    #[allow(dead_code)]
     title: String,
 }
 

Diff for: lib/src/protocol_instance.rs

@@ -45,7 +45,7 @@ impl ProtocolInstance {
                 let aligned_data_size = (data_size + 3) / 4 * 4;
                 let layout = Layout::from_size_align(aligned_data_size, 4).unwrap();
                 // Allocate aligned memory
-                let raw_ptr = unsafe { alloc(layout) as *mut u8 };
+                let raw_ptr = unsafe { alloc(layout) };
                 if raw_ptr.is_null() {
                     panic!("Failed to allocate memory with aligned pointer");
                 }

Diff for: makefile

@@ -16,5 +16,4 @@ fmt:
 	@cargo fmt --all --check
 
 clippy:
-	@cargo +nightly-2024-04-18 check --features "sgx,sp1,risc0"
-	@cargo +nightly-2024-04-18 clippy --workspace --features "sgx,sp1,risc0" --all-targets -- -Dwarnings
+	CLIPPY=1 ./script/build.sh $(TARGET)