From cc88a3b020909cd54a68e7a806c444ca23352829 Mon Sep 17 00:00:00 2001 From: Samuel Burnham <45365069+samuelburnham@users.noreply.github.com> Date: Thu, 25 Apr 2024 15:04:05 -0400 Subject: [PATCH] chore: Fix `zkvm` CI (#7) * ci: Update clippy, toolchain, dependabot (#5) * chore: Fix CI --- .cargo/config.toml | 54 +++++ .github/dependabot.yml | 32 +-- .github/workflows/ci.yml | 11 +- .github/workflows/lints-beta.yml | 24 --- .github/workflows/lints-stable.yml | 18 -- rust-toolchain.toml | 4 +- src/fp.rs | 4 +- src/fp2.rs | 36 ++-- src/g1.rs | 1 + src/g2.rs | 4 +- src/hash_to_curve/map_g1.rs | 12 +- src/hash_to_curve/map_g2.rs | 314 ++++++++++++----------------- 12 files changed, 240 insertions(+), 274 deletions(-) create mode 100644 .cargo/config.toml delete mode 100644 .github/workflows/lints-beta.yml delete mode 100644 .github/workflows/lints-stable.yml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 00000000..697e24ef --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,54 @@ +[alias] +# Collection of project wide clippy lints. This is done via an alias because +# clippy doesn't currently allow for specifiying project-wide lints in a +# configuration file. This is a similar workaround to the ones presented here: +# +xclippy = [ + "clippy", "--workspace", "--all-targets", "--all-features", "--", + "-Wclippy::all", + #"-Wclippy::cast_lossless", + #"-Wclippy::checked_conversions", + #"-Wclippy::dbg_macro", + #"-Wclippy::disallowed_methods", + #"-Wclippy::derive_partial_eq_without_eq", + #"-Wclippy::enum_glob_use", + #"-Wclippy::explicit_into_iter_loop", + #"-Wclippy::fallible_impl_from", + #"-Wclippy::filter_map_next", + #"-Wclippy::flat_map_option", + #"-Wclippy::from_iter_instead_of_collect", + #"-Wclippy::implicit_clone", + #"-Wclippy::inefficient_to_string", + #"-Wclippy::invalid_upcast_comparisons", + #"-Wclippy::large_stack_arrays", + #"-Wclippy::large_types_passed_by_value", + #"-Wclippy::macro_use_imports", + #"-Wclippy::manual_assert", + #"-Wclippy::manual_ok_or", + #"-Wclippy::map_flatten", + #"-Wclippy::map_unwrap_or", + #"-Wclippy::match_same_arms", + #"-Wclippy::match_wild_err_arm", + #"-Wclippy::missing_const_for_fn", + #"-Wclippy::needless_borrow", + #"-Wclippy::needless_continue", + #"-Wclippy::needless_for_each", + #"-Wclippy::needless_pass_by_value", + #"-Wclippy::option_option", + #"-Wclippy::same_functions_in_if_condition", + #"-Wclippy::single_match_else", + #"-Wclippy::trait_duplication_in_bounds", + #"-Wclippy::unnecessary_wraps", + #"-Wclippy::unnested_or_patterns", + #"-Wnonstandard_style", + #"-Wrust_2018_idioms", + #"-Wtrivial_numeric_casts", + #"-Wunused_lifetimes", + #"-Wunreachable_pub", + #"-Wtrivial_numeric_casts", + #"-Wunused_qualifications", + "-Aclippy::needless_borrows_for_generic_args", + "-Aclippy::needless_borrow", + "-Aclippy::clone_on_copy", + "-Aclippy::op_ref", +] diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 5c4156c6..7f4af65f 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,14 +1,22 @@ version: 2 updates: -- package-ecosystem: github-actions - directory: "/" - schedule: - interval: daily - timezone: Etc/UTC - open-pull-requests-limit: 10 - reviewers: - - str4d - assignees: - - str4d - labels: - - "A-CI" + - package-ecosystem: cargo + directory: / + pull-request-branch-name: + separator: "-" + schedule: + interval: weekly + groups: + rust-dependencies: + patterns: + - "*" + update-types: + - "minor" + - "patch" + open-pull-requests-limit: 5 + + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5df85c65..02f74d1c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,22 +9,19 @@ jobs: strategy: matrix: os: [ubuntu-latest, windows-latest, macOS-latest] - steps: - uses: actions/checkout@v3 - name: Run tests run: cargo test --verbose --release --features experimental,zeroize - no-std: - name: Check no-std target ${{ matrix.target }} + wasm: + name: Check wasm target ${{ matrix.target }} runs-on: ubuntu-latest strategy: matrix: target: - - thumbv6m-none-eabi - wasm32-unknown-unknown - wasm32-wasi - steps: - uses: actions/checkout@v3 - run: rustup target add ${{ matrix.target }} @@ -56,7 +53,7 @@ jobs: - name: Check intra-doc links run: cargo doc --document-private-items - fmt: + clippy: name: Rustfmt timeout-minutes: 30 runs-on: ubuntu-latest @@ -64,3 +61,5 @@ jobs: - uses: actions/checkout@v3 - name: Check formatting run: cargo fmt --all -- --check + - name: Clippy + run: cargo xclippy diff --git a/.github/workflows/lints-beta.yml b/.github/workflows/lints-beta.yml deleted file mode 100644 index 4440b58f..00000000 --- a/.github/workflows/lints-beta.yml +++ /dev/null @@ -1,24 +0,0 @@ -name: Beta lints - -# These lints are only informative, so we only run them directly on branches -# and not trial-merges of PRs, to reduce noise. -on: push - -jobs: - clippy-beta: - name: Clippy (beta) - timeout-minutes: 30 - runs-on: ubuntu-latest - continue-on-error: true - steps: - - uses: actions/checkout@v3 - - uses: dtolnay/rust-toolchain@beta - id: toolchain - - run: rustup override set ${{steps.toolchain.outputs.name}} - - name: Run Clippy (beta) - uses: actions-rs/clippy-check@v1 - continue-on-error: true - with: - name: Clippy (beta) - token: ${{ secrets.GITHUB_TOKEN }} - args: --all-features --all-targets -- -W clippy::all diff --git a/.github/workflows/lints-stable.yml b/.github/workflows/lints-stable.yml deleted file mode 100644 index 1e697ef6..00000000 --- a/.github/workflows/lints-stable.yml +++ /dev/null @@ -1,18 +0,0 @@ -name: Stable lints - -# We only run these lints on trial-merges of PRs to reduce noise. -on: pull_request - -jobs: - clippy: - name: Clippy (MSRV) - timeout-minutes: 30 - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - name: Run clippy - uses: actions-rs/clippy-check@v1 - with: - name: Clippy (MSRV) - token: ${{ secrets.GITHUB_TOKEN }} - args: --all-features --all-targets -- -D warnings diff --git a/rust-toolchain.toml b/rust-toolchain.toml index de43b230..02cb8fcb 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "1.56.0" -components = [ "clippy", "rustfmt" ] +channel = "stable" +profile = "default" diff --git a/src/fp.rs b/src/fp.rs index 5ad7883f..c11c117a 100644 --- a/src/fp.rs +++ b/src/fp.rs @@ -8,6 +8,7 @@ use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption}; use crate::util::{adc, mac, sbb}; +#[cfg(target_os = "zkvm")] extern "C" { fn syscall_bls12381_fp_add(p: *mut u32, q: *const u32); fn syscall_bls12381_fp_sub(p: *mut u32, q: *const u32); @@ -87,6 +88,7 @@ const MODULUS: [u64; 6] = [ const INV: u64 = 0x89f3_fffc_fffc_fffd; /// R_INV = (2^384)^(-1) mod p +#[cfg(target_os = "zkvm")] const R_INV: Fp = Fp([ 0xf4d38259380b4820, 0x7fe11274d898fafb, @@ -655,6 +657,7 @@ impl Fp { } } + #[cfg(target_os = "zkvm")] pub(crate) fn reduce_internal(&self) -> Fp { // Turn into canonical form by computing // (a.R) / R = a @@ -674,7 +677,6 @@ impl Fp { } } - /// Squares this element. #[inline] pub fn square(&self) -> Self { diff --git a/src/fp2.rs b/src/fp2.rs index 91a1cca6..ea72a324 100644 --- a/src/fp2.rs +++ b/src/fp2.rs @@ -7,6 +7,7 @@ use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption}; use crate::fp::Fp; +#[cfg(target_os = "zkvm")] extern "C" { fn syscall_bls12381_fp2_add(p: *mut u32, q: *const u32); fn syscall_bls12381_fp2_sub(p: *mut u32, q: *const u32); @@ -236,25 +237,22 @@ impl Fp2 { }; new_out } else { - let res = { - // F_{p^2} x F_{p^2} multiplication implemented with operand scanning (schoolbook) - // computes the result as: - // - // a·b = (a_0 b_0 + a_1 b_1 β) + (a_0 b_1 + a_1 b_0)i - // - // In BLS12-381's F_{p^2}, our β is -1, so the resulting F_{p^2} element is: - // - // c_0 = a_0 b_0 - a_1 b_1 - // c_1 = a_0 b_1 + a_1 b_0 - // - // Each of these is a "sum of products", which we can compute efficiently. - - Fp2 { - c0: Fp::sum_of_products([self.c0, -self.c1], [rhs.c0, rhs.c1]), - c1: Fp::sum_of_products([self.c0, self.c1], [rhs.c1, rhs.c0]), - } - }; - res + // F_{p^2} x F_{p^2} multiplication implemented with operand scanning (schoolbook) + // computes the result as: + // + // a·b = (a_0 b_0 + a_1 b_1 β) + (a_0 b_1 + a_1 b_0)i + // + // In BLS12-381's F_{p^2}, our β is -1, so the resulting F_{p^2} element is: + // + // c_0 = a_0 b_0 - a_1 b_1 + // c_1 = a_0 b_1 + a_1 b_0 + // + // Each of these is a "sum of products", which we can compute efficiently. + + Fp2 { + c0: Fp::sum_of_products([self.c0, -self.c1], [rhs.c0, rhs.c1]), + c1: Fp::sum_of_products([self.c0, self.c1], [rhs.c1, rhs.c0]), + } } } } diff --git a/src/g1.rs b/src/g1.rs index d33de95d..22f75917 100644 --- a/src/g1.rs +++ b/src/g1.rs @@ -17,6 +17,7 @@ use group::WnafGroup; use crate::fp::Fp; use crate::Scalar; +#[cfg(target_os = "zkvm")] extern "C" { fn syscall_bls12381_g1_decompress(p: &mut [u8; 96]); } diff --git a/src/g2.rs b/src/g2.rs index 2b4bca07..35af64be 100644 --- a/src/g2.rs +++ b/src/g2.rs @@ -648,8 +648,8 @@ impl_binops_multiplicative_mixed!(Scalar, G2Projective, G2Projective); #[inline(always)] fn mul_by_3b(x: Fp2) -> Fp2 { - let B3: Fp2 = Fp2::add(&Fp2::add(&B, &B), &B); // FIXME - x * B3 + let b3: Fp2 = Fp2::add(&Fp2::add(&B, &B), &B); // FIXME + x * b3 } impl G2Projective { diff --git a/src/hash_to_curve/map_g1.rs b/src/hash_to_curve/map_g1.rs index 0f2c94e6..b865f624 100644 --- a/src/hash_to_curve/map_g1.rs +++ b/src/hash_to_curve/map_g1.rs @@ -943,14 +943,14 @@ pub const P_M1_OVER2: Fp = Fp::from_raw_unchecked([ #[test] fn test_sgn0() { - assert_eq!(bool::from(Fp::zero().sgn0()), false); - assert_eq!(bool::from(Fp::one().sgn0()), true); - assert_eq!(bool::from((-Fp::one()).sgn0()), false); - assert_eq!(bool::from((-Fp::zero()).sgn0()), false); - assert_eq!(bool::from(P_M1_OVER2.sgn0()), true); + assert!(!bool::from(Fp::zero().sgn0())); + assert!(bool::from(Fp::one().sgn0())); + assert!(!bool::from((-Fp::one()).sgn0())); + assert!(!bool::from((-Fp::zero()).sgn0())); + assert!(bool::from(P_M1_OVER2.sgn0())); let p_p1_over2 = P_M1_OVER2 + Fp::one(); - assert_eq!(bool::from(p_p1_over2.sgn0()), false); + assert!(!bool::from(p_p1_over2.sgn0())); let neg_p_p1_over2 = { let mut tmp = p_p1_over2; diff --git a/src/hash_to_curve/map_g2.rs b/src/hash_to_curve/map_g2.rs index a0959dcd..1bebcb5d 100644 --- a/src/hash_to_curve/map_g2.rs +++ b/src/hash_to_curve/map_g2.rs @@ -711,190 +711,136 @@ fn test_hash_to_curve_10() { fn test_sgn0() { use super::map_g1::P_M1_OVER2; - assert_eq!(bool::from(Fp2::zero().sgn0()), false); - assert_eq!(bool::from(Fp2::one().sgn0()), true); - assert_eq!( - bool::from( - Fp2 { - c0: P_M1_OVER2, - c1: Fp::zero() - } - .sgn0() - ), - true - ); - assert_eq!( - bool::from( - Fp2 { - c0: P_M1_OVER2, - c1: Fp::one() - } - .sgn0() - ), - true - ); - assert_eq!( - bool::from( - Fp2 { - c0: Fp::zero(), - c1: P_M1_OVER2, - } - .sgn0() - ), - true - ); - assert_eq!( - bool::from( - Fp2 { - c0: Fp::one(), - c1: P_M1_OVER2, - } - .sgn0() - ), - true - ); + assert!(!bool::from(Fp2::zero().sgn0())); + assert!(bool::from(Fp2::one().sgn0())); + assert!(bool::from( + Fp2 { + c0: P_M1_OVER2, + c1: Fp::zero() + } + .sgn0() + ),); + assert!(bool::from( + Fp2 { + c0: P_M1_OVER2, + c1: Fp::one() + } + .sgn0() + ),); + assert!(bool::from( + Fp2 { + c0: Fp::zero(), + c1: P_M1_OVER2, + } + .sgn0() + ),); + assert!(bool::from( + Fp2 { + c0: Fp::one(), + c1: P_M1_OVER2, + } + .sgn0() + ),); let p_p1_over2 = P_M1_OVER2 + Fp::one(); - assert_eq!( - bool::from( - Fp2 { - c0: p_p1_over2, - c1: Fp::zero() - } - .sgn0() - ), - false - ); - assert_eq!( - bool::from( - Fp2 { - c0: p_p1_over2, - c1: Fp::one() - } - .sgn0() - ), - false - ); - assert_eq!( - bool::from( - Fp2 { - c0: Fp::zero(), - c1: p_p1_over2, - } - .sgn0() - ), - false - ); - assert_eq!( - bool::from( - Fp2 { - c0: Fp::one(), - c1: p_p1_over2, - } - .sgn0() - ), - true - ); - - assert_eq!( - bool::from( - Fp2 { - c0: P_M1_OVER2, - c1: -Fp::one() - } - .sgn0() - ), - true - ); - assert_eq!( - bool::from( - Fp2 { - c0: p_p1_over2, - c1: -Fp::one() - } - .sgn0() - ), - false - ); - assert_eq!( - bool::from( - Fp2 { - c0: Fp::zero(), - c1: -Fp::one() - } - .sgn0() - ), - false - ); - assert_eq!( - bool::from( - Fp2 { - c0: P_M1_OVER2, - c1: p_p1_over2 - } - .sgn0() - ), - true - ); - assert_eq!( - bool::from( - Fp2 { - c0: p_p1_over2, - c1: P_M1_OVER2 - } - .sgn0() - ), - false - ); - - assert_eq!( - bool::from( - Fp2 { - c0: -Fp::one(), - c1: P_M1_OVER2, - } - .sgn0() - ), - false - ); - assert_eq!( - bool::from( - Fp2 { - c0: -Fp::one(), - c1: p_p1_over2, - } - .sgn0() - ), - false - ); - assert_eq!( - bool::from( - Fp2 { - c0: -Fp::one(), - c1: Fp::zero(), - } - .sgn0() - ), - false - ); - assert_eq!( - bool::from( - Fp2 { - c0: p_p1_over2, - c1: P_M1_OVER2, - } - .sgn0() - ), - false - ); - assert_eq!( - bool::from( - Fp2 { - c0: P_M1_OVER2, - c1: p_p1_over2, - } - .sgn0() - ), - true - ); + assert!(!bool::from( + Fp2 { + c0: p_p1_over2, + c1: Fp::zero() + } + .sgn0() + ),); + assert!(!bool::from( + Fp2 { + c0: p_p1_over2, + c1: Fp::one() + } + .sgn0() + ),); + assert!(!bool::from( + Fp2 { + c0: Fp::zero(), + c1: p_p1_over2, + } + .sgn0() + ),); + assert!(bool::from( + Fp2 { + c0: Fp::one(), + c1: p_p1_over2, + } + .sgn0() + ),); + + assert!(bool::from( + Fp2 { + c0: P_M1_OVER2, + c1: -Fp::one() + } + .sgn0() + ),); + assert!(!bool::from( + Fp2 { + c0: p_p1_over2, + c1: -Fp::one() + } + .sgn0() + ),); + assert!(!bool::from( + Fp2 { + c0: Fp::zero(), + c1: -Fp::one() + } + .sgn0() + ),); + assert!(bool::from( + Fp2 { + c0: P_M1_OVER2, + c1: p_p1_over2 + } + .sgn0() + ),); + assert!(!bool::from( + Fp2 { + c0: p_p1_over2, + c1: P_M1_OVER2 + } + .sgn0() + ),); + + assert!(!bool::from( + Fp2 { + c0: -Fp::one(), + c1: P_M1_OVER2, + } + .sgn0() + ),); + assert!(!bool::from( + Fp2 { + c0: -Fp::one(), + c1: p_p1_over2, + } + .sgn0() + ),); + assert!(!bool::from( + Fp2 { + c0: -Fp::one(), + c1: Fp::zero(), + } + .sgn0() + ),); + assert!(!bool::from( + Fp2 { + c0: p_p1_over2, + c1: P_M1_OVER2, + } + .sgn0() + ),); + assert!(bool::from( + Fp2 { + c0: P_M1_OVER2, + c1: p_p1_over2, + } + .sgn0() + ),); }