fix: compilation, docs, CI & clippy

.github/workflows/lints.yml

@@ -0,0 +1,72 @@
+name: Lints
+
+# We only run these lints on trial-merges of PRs to reduce noise.
+on:
+  merge_group:
+  pull_request:
+    types: [synchronize, opened, reopened, ready_for_review]
+  push:
+    branches:
+      - main
+
+jobs:
+  skip_check:
+    runs-on: ubuntu-latest
+    outputs:
+      should_skip: ${{ steps.skip_check.outputs.should_skip }}
+    steps:
+      - id: skip_check
+        uses: fkirc/skip-duplicate-actions@v5
+        with:
+          cancel_others: 'true'
+          concurrent_skipping: 'same_content_newer'
+
+  lints:
+    needs: [skip_check]
+    if: |
+      github.event.pull_request.draft == false &&
+      (github.event.action == 'ready_for_review' || needs.skip_check.outputs.should_skip != 'true')
+
+    name: Various lints
+    timeout-minutes: 30
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          components: rustfmt, clippy
+          override: false
+      - name: Cargo cache
+        uses: actions/cache@v3
+        with:
+          path: |
+            ~/.cargo/bin/
+            ~/.cargo/registry/index/
+            ~/.cargo/registry/cache/
+            ~/.cargo/git/db/
+            target/
+          key: lint-${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
+      - name: Cargo build
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+      - name: Check code format
+        uses: actions-rs/cargo@v1
+        with:
+          command: fmt
+          args: --all -- --check
+
+      - name: Run clippy
+        uses: actions-rs/cargo@v1
+        with:
+          command: clippy
+          args: --all-features --all-targets -- -D warnings
+
+      # Ensure intra-documentation links all resolve correctly
+      # Requires #![deny(intra_doc_link_resolution_failure)] in crates.
+      - name: Check intra-doc links
+        uses: actions-rs/cargo@v1
+        with:
+          command: doc
+          args: --no-deps --all --document-private-items

.github/workflows/test.yml

@@ -0,0 +1,60 @@
+name: CI checks
+
+on:
+  merge_group:
+  pull_request:
+  push:
+    branches:
+      - main
+
+jobs:
+  test:
+    name: Test on ${{ matrix.os }} with ${{ matrix.feature_set }} features
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        feature_set: [basic, all]
+        os: [ubuntu-latest, windows-latest, macOS-latest]
+        include:
+          - feature_set: all
+            features: test-dev-graph,circuit-params
+
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions-rs/toolchain@v1
+        with:
+          override: false
+      - name: Run tests
+        uses: actions-rs/cargo@v1
+        with:
+          command: test
+          args: --verbose --release --workspace --no-default-features --features "${{ matrix.features }}"
+
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions-rs/toolchain@v1
+        with:
+          override: false
+      - name: Add target
+        run: rustup target add ${{ matrix.target }}
+      - name: cargo build
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+          args: --no-default-features --target ${{ matrix.target }}
+
+  bitrot:
+    name: Bitrot check
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions-rs/toolchain@v1
+        with:
+          override: false
+      # Build benchmarks to prevent bitrot
+      - name: Build benchmarks
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+          args: --benches --examples --all-features

Cargo.toml

@@ -1,5 +1,5 @@
 [package]
-name = "poseidon"
+name = "halo2_poseidon"
 version = "0.2.0"
 authors = [
     "Sean Bowe <[email protected]>",
@@ -59,7 +59,6 @@ circuit-params = ["halo2_proofs/circuit-params"]
 test-dependencies = ["proptest"]
 unstable = []
 
-
 [[bench]]
 name = "poseidon"
 harness = false

README.md

@@ -2,7 +2,7 @@
 
 [Poseidon hash function](https://eprint.iacr.org/2019/458.pdf) gadget for Halo2.
 
-Requires Rust 1.56.1+.
+Requires Rust 1.72.0+.
 
 ## Documentation
 

benches/poseidon.rs

@@ -20,7 +20,7 @@ use halo2_proofs::{
 };
 use halo2curves::pasta::{pallas, vesta, EqAffine, Fp};
 
-use halo2_gadgets::poseidon::{
+use halo2_poseidon::poseidon::{
     primitives::{self as poseidon, generate_constants, ConstantLength, Mds, Spec},
     Hash, Pow5Chip, Pow5Config,
 };

benches/rm_primitives.rs

@@ -1,17 +1,14 @@
-use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion};
+use criterion::{criterion_group, criterion_main, Criterion};
 use ff::Field;
-use halo2_gadgets::{
-    poseidon::primitives::{self as poseidon, ConstantLength, P128Pow5T3},
-    sinsemilla::primitives as sinsemilla,
-};
+use halo2_poseidon::poseidon::primitives::{self as poseidon, ConstantLength, P128Pow5T3};
 
 use halo2curves::pasta::pallas;
 #[cfg(unix)]
 use pprof::criterion::{Output, PProfProfiler};
-use rand::{rngs::OsRng, Rng};
+use rand::rngs::OsRng;
 
 fn bench_primitives(c: &mut Criterion) {
-    let mut rng = OsRng;
+    let rng = OsRng;
 
     {
         let mut group = c.benchmark_group("Poseidon");
@@ -24,37 +21,6 @@ fn bench_primitives(c: &mut Criterion) {
             })
         });
     }
-
-    {
-        let mut group = c.benchmark_group("Sinsemilla");
-
-        let hasher = sinsemilla::HashDomain::new("hasher");
-        let committer = sinsemilla::CommitDomain::new("committer");
-        let bits: Vec<bool> = (0..1086).map(|_| rng.gen()).collect();
-        let r = pallas::Scalar::random(rng);
-
-        // Benchmark the input sizes we use in Orchard:
-        // - 510 bits for Commit^ivk
-        // - 520 bits for MerkleCRH
-        // - 1086 bits for NoteCommit
-        for size in [510, 520, 1086] {
-            group.bench_function(BenchmarkId::new("hash-to-point", size), |b| {
-                b.iter(|| hasher.hash_to_point(bits[..size].iter().cloned()))
-            });
-
-            group.bench_function(BenchmarkId::new("hash", size), |b| {
-                b.iter(|| hasher.hash(bits[..size].iter().cloned()))
-            });
-
-            group.bench_function(BenchmarkId::new("commit", size), |b| {
-                b.iter(|| committer.commit(bits[..size].iter().cloned(), &r))
-            });
-
-            group.bench_function(BenchmarkId::new("short-commit", size), |b| {
-                b.iter(|| committer.commit(bits[..size].iter().cloned(), &r))
-            });
-        }
-    }
 }
 
 #[cfg(unix)]

rust-toolchain

@@ -0,0 +1 @@
+1.72.0

src/lib.rs

@@ -1,10 +1,8 @@
 //! This crate provides the poseidon gadget for use with `halo2_proofs`.
 //! This gadget has been stracted from zcash's halo2_gadgets:
-//! https://github.com/zcash/halo2/tree/main/halo2_gadgets
+//! <https://github.com/zcash/halo2/tree/main/halo2_gadgets>
 
 #![cfg_attr(docsrs, feature(doc_cfg))]
-// Temporary until we have more of the crate implemented.
-#![allow(dead_code)]
 // Catch documentation errors caused by code changes.
 #![deny(rustdoc::broken_intra_doc_links)]
 #![deny(missing_debug_implementations)]

src/poseidon.rs

@@ -288,7 +288,7 @@ impl<
             .enumerate()
         {
             self.sponge
-                .absorb(layouter.namespace(|| format!("absorb_{}", i)), value)?;
+                .absorb(layouter.namespace(|| format!("absorb_{i}")), value)?;
         }
         self.sponge
             .finish_absorbing(layouter.namespace(|| "finish absorbing"))?

src/poseidon/pow5.rs

@@ -32,6 +32,7 @@ pub struct Pow5Config<F: Field, const WIDTH: usize, const RATE: usize> {
     alpha: [u64; 4],
     round_constants: Vec<[F; WIDTH]>,
     m_reg: Mds<F, WIDTH>,
+    #[allow(dead_code)]
     m_inv: Mds<F, WIDTH>,
 }
 
@@ -286,7 +287,7 @@ impl<
                 let mut state = Vec::with_capacity(WIDTH);
                 let mut load_state_word = |i: usize, value: F| -> Result<_, Error> {
                     let var = region.assign_advice_from_constant(
-                        || format!("state_{}", i),
+                        || format!("state_{i}"),
                         config.state[i],
                         0,
                         value,
@@ -325,7 +326,7 @@ impl<
                     initial_state[i]
                         .0
                         .copy_advice(
-                            || format!("load state_{}", i),
+                            || format!("load state_{i}"),
                             &mut region,
                             config.state[i],
                             0,
@@ -341,7 +342,7 @@ impl<
                     let constraint_var = match input.0[i].clone() {
                         Some(PaddedWord::Message(word)) => word,
                         Some(PaddedWord::Padding(padding_value)) => region.assign_fixed(
-                            || format!("load pad_{}", i),
+                            || format!("load pad_{i}"),
                             config.rc_b[i],
                             1,
                             || Value::known(padding_value),
@@ -350,7 +351,7 @@ impl<
                     };
                     constraint_var
                         .copy_advice(
-                            || format!("load input_{}", i),
+                            || format!("load input_{i}"),
                             &mut region,
                             config.state[i],
                             1,
@@ -369,12 +370,7 @@ impl<
                             // The capacity element is never altered by the input.
                             .unwrap_or_else(|| Value::known(F::ZERO));
                     region
-                        .assign_advice(
-                            || format!("load output_{}", i),
-                            config.state[i],
-                            2,
-                            || value,
-                        )
+                        .assign_advice(|| format!("load output_{i}"), config.state[i], 2, || value)
                         .map(StateWord)
                 };
 
@@ -474,7 +470,7 @@ impl<F: Field, const WIDTH: usize> Pow5State<F, WIDTH> {
             });
 
             region.assign_advice(
-                || format!("round_{} partial_sbox", round),
+                || format!("round_{round} partial_sbox"),
                 config.partial_sbox,
                 offset,
                 || r.as_ref().map(|r| r[0]),
@@ -536,7 +532,7 @@ impl<F: Field, const WIDTH: usize> Pow5State<F, WIDTH> {
         let load_state_word = |i: usize| {
             initial_state[i]
                 .0
-                .copy_advice(|| format!("load state_{}", i), region, config.state[i], 0)
+                .copy_advice(|| format!("load state_{i}"), region, config.state[i], 0)
                 .map(StateWord)
         };
 
@@ -558,7 +554,7 @@ impl<F: Field, const WIDTH: usize> Pow5State<F, WIDTH> {
         // Load the round constants.
         let mut load_round_constant = |i: usize| {
             region.assign_fixed(
-                || format!("round_{} rc_{}", round, i),
+                || format!("round_{round} rc_{i}"),
                 config.rc_a[i],
                 offset,
                 || Value::known(config.round_constants[round][i]),
@@ -574,7 +570,7 @@ impl<F: Field, const WIDTH: usize> Pow5State<F, WIDTH> {
         let next_state_word = |i: usize| {
             let value = next_state[i];
             let var = region.assign_advice(
-                || format!("round_{} state_{}", next_round, i),
+                || format!("round_{next_round} state_{i}"),
                 config.state[i],
                 offset + 1,
                 || value,

src/poseidon/primitives.rs

@@ -310,7 +310,7 @@ impl<F: PrimeField, const RATE: usize, const L: usize> Domain<F, RATE> for Const
     type Padding = iter::Take<iter::Repeat<F>>;
 
     fn name() -> String {
-        format!("ConstantLength<{}>", L)
+        format!("ConstantLength<{L}>")
     }
 
     fn initial_capacity_element() -> F {

src/poseidon/primitives/p128pow5t3.rs

@@ -66,6 +66,7 @@ impl Spec<Fq, 3, 2> for P128Pow5T3 {
 
 #[cfg(test)]
 mod tests {
+    #![allow(dead_code)]
     use super::{
         super::{fp, fq},
         Fp, Fq,

src/utilities.rs

@@ -337,6 +337,7 @@ mod tests {
 
     #[allow(clippy::assign_op_pattern)]
     #[allow(clippy::ptr_offset_with_cast)]
+    #[allow(clippy::single_range_in_vec_init)]
     #[test]
     fn test_bitrange_subset() {
         let rng = OsRng;

src/utilities/cond_swap.rs

@@ -56,13 +56,6 @@ pub struct CondSwapConfig {
     swap: Column<Advice>,
 }
 
-#[cfg(test)]
-impl CondSwapConfig {
-    pub(crate) fn a(&self) -> Column<Advice> {
-        self.a
-    }
-}
-
 impl<F: Field> UtilitiesInstructions<F> for CondSwapChip<F> {
     type Var = AssignedCell<F, F>;
 }