CI: Clippy, linting, check rustdoc, etc (#104)

* Clippy, linting, check rustdoc, etc * Ignore frost-redjubjub in the workspace for now * Make rustfmt happy * Clippy * Appease Clippy * Comment out bench for now
2022-08-03 23:04:37 -04:00 · 2022-08-03 23:04:37 -04:00 · 866fdefb20
parent c551cd56eb
commit 866fdefb20
7 changed files with 183 additions and 85 deletions
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@ -1,20 +1,116 @@
 name: CI

-on: [push]
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+    branches:
+      - main

 jobs:
+
  test_nightly:
    name: test on nightly
    runs-on: ubuntu-latest
+
    steps:
-    - uses: actions/checkout@v3
-    # Because we use nightly features for building docs,
-    # using --all-features will fail without nightly toolchain.
-    - uses: actions-rs/toolchain@v1.0.7
-      with:
-        toolchain: nightly
-        override: true
-    - uses: actions-rs/cargo@v1.0.3
-      with:
-        command: test
-        args: --all-features
+      - uses: actions/checkout@v3.0.2
+      # Because we use nightly features for building docs,
+      # using --all-features will fail without nightly toolchain.
+      - uses: actions-rs/toolchain@v1.0.7
+        with:
+          toolchain: nightly
+          override: true
+      - uses: actions-rs/cargo@v1.0.3
+        with:
+          command: test
+          args: --all-features
+
+  clippy:
+    name: Clippy
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v3.0.2
+        with:
+          persist-credentials: false
+
+      - name: Check workflow permissions
+        id: check_permissions
+        uses: scherermichael-oss/action-has-permission@1.0.6
+        with:
+          required-permission: write
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Run clippy action to produce annotations
+        uses: actions-rs/clippy-check@v1.0.7
+        if: ${{ steps.check_permissions.outputs.has-permission }}
+        with:
+          # GitHub displays the clippy job and its results as separate entries
+          name: Clippy (stable) Results
+          token: ${{ secrets.GITHUB_TOKEN }}
+          args: --all-features --all-targets -- -D warnings
+
+      - uses: actions-rs/toolchain@v1.0.1
+        if: ${{ !steps.check_permissions.outputs.has-permission }}
+        with:
+          toolchain: stable
+          override: true
+
+      - name: Run clippy manually without annotations
+        if: ${{ !steps.check_permissions.outputs.has-permission }}
+        run: cargo clippy --all-features --all-targets -- -D warnings
+
+  fmt:
+    name: Rustfmt
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v3.0.2
+        with:
+          persist-credentials: false
+
+      - uses: actions-rs/toolchain@v1.0.6
+        with:
+          toolchain: stable
+          components: rustfmt
+          override: true
+
+      - uses: Swatinem/rust-cache@v1
+
+      - uses: actions-rs/cargo@v1.0.3
+        with:
+          command: fmt
+          args: --all -- --check
+
+  docs:
+    name: Check Rust doc
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v3.0.2
+        with:
+          persist-credentials: false
+
+      - uses: actions-rs/toolchain@v1.0.6
+        with:
+          toolchain: stable
+          profile: minimal
+          override: true
+
+      - uses: actions-rs/cargo@v1.0.3
+        with:
+          command: doc
+          args: --no-deps --document-private-items --all-features
+
+  actionlint:
+    runs-on: ubuntu-latest
+    continue-on-error: true
+    steps:
+      - uses: actions/checkout@v3.0.2
+      - uses: reviewdog/action-actionlint@v1.26.0
+        with:
+          level: warning
+          fail_on_error: false
--- a/Cargo.toml
+++ b/Cargo.toml
@ -2,7 +2,7 @@
 resolver = "2"
 members = [
    "frost-core",
-    "frost-redjubjub",
+    #"frost-redjubjub",
    "frost-ristretto255",
    "frost-p256",
 ]
--- a/frost-core/src/scalar_mul.rs
+++ b/frost-core/src/scalar_mul.rs
@ -51,14 +51,14 @@ where
            // Construct a buffer of bits of the scalar, starting at bit `pos`
            let u64_idx = pos / 64;
            let bit_idx = pos % 64;
-            let bit_buf: u64;
-            if bit_idx < 64 - w {
+
+            let bit_buf: u64 = if bit_idx < 64 - w {
                // This window's bits are contained in a single u64
-                bit_buf = x_u64[u64_idx] >> bit_idx;
+                x_u64[u64_idx] >> bit_idx
            } else {
                // Combine the current u64's bits with the bits from the next u64
-                bit_buf = (x_u64[u64_idx] >> bit_idx) | (x_u64[1 + u64_idx] << (64 - bit_idx));
-            }
+                (x_u64[u64_idx] >> bit_idx) | (x_u64[1 + u64_idx] << (64 - bit_idx))
+            };

            // Add the carry into the current window
            let window = carry + (bit_buf & window_mask);
@ -130,6 +130,7 @@ impl<C> VartimeMultiscalarMul<C> for Element<C>
 where
    C: Ciphersuite,
 {
+    #[allow(clippy::comparison_chain)]
    fn optional_multiscalar_mul<I, J>(scalars: I, elements: J) -> Option<Element<C>>
    where
        I: IntoIterator,
--- a/frost-core/tests/common/ciphersuite.rs
+++ b/frost-core/tests/common/ciphersuite.rs
@ -64,7 +64,7 @@ impl Field for RistrettoScalarField {
    }
 }

-#[derive(Clone, Copy, PartialEq)]
+#[derive(Clone, Copy, PartialEq, Eq)]
 pub struct RistrettoGroup;

 impl Group for RistrettoGroup {
@ -103,7 +103,7 @@ impl Group for RistrettoGroup {
 /// [spec]: https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-05.html#section-6.2-1
 const CONTEXT_STRING: &str = "FROST-RISTRETTO255-SHA512-v5";

-#[derive(Clone, Copy, PartialEq)]
+#[derive(Clone, Copy, PartialEq, Eq)]
 pub struct Ristretto255Sha512;

 impl Ciphersuite for Ristretto255Sha512 {
--- a/frost-p256/src/lib.rs
+++ b/frost-p256/src/lib.rs
@ -75,7 +75,7 @@ impl Field for P256ScalarField {
    }
 }

-#[derive(Clone, Copy, PartialEq)]
+#[derive(Clone, Copy, PartialEq, Eq)]
 /// An implementation of the FROST P-256 ciphersuite group.
 pub struct P256Group;

@ -139,7 +139,7 @@ impl Group for P256Group {
 /// [spec]: https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-05.html#section-6.4-1
 const CONTEXT_STRING: &str = "FROST-P256-SHA256-v5";

-#[derive(Clone, Copy, PartialEq)]
+#[derive(Clone, Copy, PartialEq, Eq)]
 /// An implementation of the FROST ciphersuite FROST(P-256, SHA-256).
 pub struct P256Sha256;

@ -226,7 +226,7 @@ pub mod keys {

 ///
 pub mod round1 {
-    use frost_core::frost::keys::{Secret};
+    use frost_core::frost::keys::Secret;

    use super::*;
    ///
--- a/frost-ristretto255/benches/bench.rs
+++ b/frost-ristretto255/benches/bench.rs
@ -1,71 +1,72 @@
-use std::convert::TryFrom;
+// use std::convert::TryFrom;

-use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Throughput};
-use rand::thread_rng;
+// use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Throughput};
+// use rand::thread_rng;

-use frost_ristretto255::*;
+// use frost_ristretto255::*;

-struct Item {
-    vk_bytes: VerificationKeyBytes,
-    sig: Signature,
-}
+// struct Item {
+//     vk: VerifyingKey,
+//     sig: Signature,
+// }

-fn sigs_with_distinct_keys() -> impl Iterator<Item = Item> {
-    std::iter::repeat_with(|| {
-        let msg = b"Bench";
-        let sk = SigningKey::new(thread_rng());
-        let vk_bytes = VerificationKey::from(&sk).into();
-        let sig = sk.sign(thread_rng(), &msg[..]);
-        Item { vk_bytes, sig }
-    })
-}
+// fn sigs_with_distinct_keys() -> impl Iterator<Item = Item> {
+//     std::iter::repeat_with(|| {
+//         let msg = b"Bench";
+//         let sk = SigningKey::new(thread_rng());
+//         let vk = VerifyingKey::from(&sk).into();
+//         let sig = sk.sign(thread_rng(), &msg[..]);
+//         Item { vk, sig }
+//     })
+// }

-fn bench_batch_verify(c: &mut Criterion) {
-    let mut group = c.benchmark_group("Batch Verification");
-    for &n in [8usize, 16, 24, 32, 40, 48, 56, 64].iter() {
-        group.throughput(Throughput::Elements(n as u64));
+// fn bench_batch_verify(c: &mut Criterion) {
+//     let mut group = c.benchmark_group("Batch Verification");
+//     for &n in [8usize, 16, 24, 32, 40, 48, 56, 64].iter() {
+//         group.throughput(Throughput::Elements(n as u64));

-        let sigs = sigs_with_distinct_keys().take(n).collect::<Vec<_>>();
+//         let sigs = sigs_with_distinct_keys().take(n).collect::<Vec<_>>();

-        group.bench_with_input(
-            BenchmarkId::new("Unbatched verification", n),
-            &sigs,
-            |b, sigs| {
-                b.iter(|| {
-                    for item in sigs.iter() {
-                        let msg = b"Bench";
+//         group.bench_with_input(
+//             BenchmarkId::new("Unbatched verification", n),
+//             &sigs,
+//             |b, sigs| {
+//                 b.iter(|| {
+//                     for item in sigs.iter() {
+//                         let msg = b"Bench";

-                        let Item { vk_bytes, sig } = item;
-                        {
-                            let _ = VerificationKey::try_from(*vk_bytes)
-                                .and_then(|vk| vk.verify(msg, sig));
-                        }
-                    }
-                })
-            },
-        );
+//                         let Item { vk, sig } = item;
+//                         {
+//                             vk.verify(msg, sig);
+//                         }
+//                     }
+//                 })
+//             },
+//         );

-        group.bench_with_input(
-            BenchmarkId::new("Batched verification", n),
-            &sigs,
-            |b, sigs| {
-                b.iter(|| {
-                    let mut batch = batch::Verifier::new();
-                    for item in sigs.iter() {
-                        let msg = b"Bench";
+//         group.bench_with_input(
+//             BenchmarkId::new("Batched verification", n),
+//             &sigs,
+//             |b, sigs| {
+//                 b.iter(|| {
+//                     let mut batch = batch::Verifier::new();
+//                     for item in sigs.iter() {
+//                         let msg = b"Bench";

-                        let Item { vk_bytes, sig } = item;
-                        {
-                            batch.queue((*vk_bytes, *sig, msg));
-                        }
-                    }
-                    batch.verify(thread_rng())
-                })
-            },
-        );
-    }
-    group.finish();
-}
+//                         let Item { vk, sig } = item;
+//                         {
+//                             batch.queue((*vk, *sig, msg));
+//                         }
+//                     }
+//                     batch.verify(thread_rng())
+//                 })
+//             },
+//         );
+//     }
+//     group.finish();
+// }

-criterion_group!(benches, bench_batch_verify);
-criterion_main!(benches);
+// criterion_group!(benches, bench_batch_verify);
+// criterion_main!(benches);
+
+fn main() {}
--- a/frost-ristretto255/src/lib.rs
+++ b/frost-ristretto255/src/lib.rs
@ -72,7 +72,7 @@ impl Field for RistrettoScalarField {
    }
 }

-#[derive(Clone, Copy, PartialEq)]
+#[derive(Clone, Copy, PartialEq, Eq)]
 /// An implementation of the FROST ciphersuite group.
 pub struct RistrettoGroup;

@ -112,7 +112,7 @@ impl Group for RistrettoGroup {
 /// [spec]: https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-05.html#section-6.2-1
 const CONTEXT_STRING: &str = "FROST-RISTRETTO255-SHA512-v5";

-#[derive(Clone, Copy, PartialEq)]
+#[derive(Clone, Copy, PartialEq, Eq)]
 /// An implementation of the FROST ciphersuite Ristretto255-SHA512.
 pub struct Ristretto255Sha512;