[WIP] Use --cfg attributes to select backends

Crate features are intended to be additive, whereas only 1-of-N possible backends can be selected. Features can also be activated by transitive dependencies, which leads to a problem of different dependences selecting conflicting backends. Using `--cfg` instead moves all backend selection control to the toplevel executable. This commit switches to the following RUSTFLAGS to enable backends: - `--cfg curve25519_dalek_backend="fiat"`: uses `fiat-crypto` - `--cfg curve25519_dalek_backend="simd"`: uses nightly-only SIMD
dalek-cryptography · Dec 9, 2022 · a4270c4 · a4270c4
1 parent 0b72bb5
commit a4270c4
Show file tree

Hide file tree

Showing 11 changed files with 40 additions and 36 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -27,7 +27,9 @@ jobs:
       - run: rustup target add ${{ matrix.target }}
       - run: ${{ matrix.deps }}
       - run: cargo test --target ${{ matrix.target }}
-      - run: cargo test --target ${{ matrix.target }} --features fiat_backend
+      - env:
+          RUSTFLAGS: '--cfg curve25519_dalek_backend="fiat" -D warnings'
+        run: cargo test --target ${{ matrix.target }}
 
   build-simd:
     name: Build simd backend (nightly)
@@ -37,11 +39,11 @@ jobs:
     - uses: dtolnay/rust-toolchain@nightly
     # Build with AVX2 features, then with AVX512 features
     - env:
-        RUSTFLAGS: "-C target_feature=+avx2"
-      run: cargo build --target x86_64-unknown-linux-gnu --features simd_backend
+        RUSTFLAGS: '--cfg curve25519_dalek_backend="simd" -C target_feature=+avx2 -D warnings'
+      run: cargo build --target x86_64-unknown-linux-gnu
     - env:
-        RUSTFLAGS: "-C target_feature=+avx512ifma"
-      run: cargo build --target x86_64-unknown-linux-gnu --features simd_backend
+        RUSTFLAGS: '--cfg curve25519_dalek_backend="simd" -C target_feature=+avx512ifma -D warnings'
+      run: cargo build --target x86_64-unknown-linux-gnu
 
   test-defaults-serde:
     name: Test default feature selection and serde
@@ -78,11 +80,11 @@ jobs:
       with:
         components: clippy
     - env:
-        RUSTFLAGS: "-C target_feature=+avx2"
-      run: cargo clippy --target x86_64-unknown-linux-gnu --features simd_backend -- -D warnings
+        RUSTFLAGS: '--cfg curve25519_dalek_backend="simd" -C target_feature=+avx2 -D warnings'
+      run: cargo clippy --target x86_64-unknown-linux-gnu
     - env:
-        RUSTFLAGS: "-C target_feature=+avx512ifma"
-      run: cargo clippy --target x86_64-unknown-linux-gnu --features simd_backend -- -D warnings
+        RUSTFLAGS: '--cfg curve25519_dalek_backend="simd" -C target_feature=+avx512ifma -D warnings'
+      run: cargo clippy --target x86_64-unknown-linux-gnu
 
   rustfmt:
     name: Check formatting
@@ -102,11 +104,11 @@ jobs:
     # First run `cargo +nightly -Z minimal-verisons check` in order to get a
     # Cargo.lock with the oldest possible deps
     - uses: dtolnay/rust-toolchain@nightly
-    - run: cargo -Z minimal-versions check --no-default-features --features fiat_backend,serde
+    - run: cargo -Z minimal-versions check --no-default-features --features serde
     # Now check that `cargo build` works with respect to the oldest possible
     # deps and the stated MSRV
     - uses: dtolnay/[email protected]
-    - run: cargo build --no-default-features --features fiat_backend,serde
+    - run: cargo build --no-default-features --features serde
 
   bench:
     name: Check that benchmarks compile
@@ -115,8 +117,12 @@ jobs:
     - uses: actions/checkout@v3
     - uses: dtolnay/rust-toolchain@stable
     - name: Build u32 bench
-      run: env RUSTFLAGS="--cfg curve25519_dalek_bits=\"32\"" cargo build --benches
+      env:
+        RUSTFLAGS: '--cfg curve25519_dalek_bits="32" -D warnings'
+      run: cargo build --benches
     - name: Build u64 bench
-      run: env RUSTFLAGS="--cfg curve25519_dalek_bits=\"64\"" cargo build --benches
+      env:
+        RUSTFLAGS: '--cfg curve25519_dalek_bits="64" -D warnings'
+      run: cargo build --benches
     - name: Build default (host native) bench
       run: cargo build --benches
diff --git a/Cargo.toml b/Cargo.toml
@@ -49,18 +49,17 @@ rand_core = { version = "0.6", default-features = false, optional = true }
 digest = { version = "0.10", default-features = false, optional = true }
 subtle = { version = "^2.2.1", default-features = false }
 serde = { version = "1.0", default-features = false, optional = true, features = ["derive"] }
+zeroize = { version = "1", default-features = false }
+
+[target.'cfg(curve25519_dalek_backend = "fiat")'.dependencies]
+fiat-crypto = "0.1.6"
+
 # The original packed_simd package was orphaned, see
 # https://github.com/rust-lang/packed_simd/issues/303#issuecomment-701361161
-packed_simd = { version = "0.3.4", package = "packed_simd_2", features = ["into_bits"], optional = true }
-zeroize = { version = "1", default-features = false }
-fiat-crypto = { version = "0.1.6", optional = true}
+[target.'cfg(curve25519_dalek_backend = "simd")'.dependencies]
+packed_simd = { version = "0.3.4", package = "packed_simd_2", features = ["into_bits"] }
 
 [features]
 nightly = ["subtle/nightly"]
 default = ["alloc"]
 alloc = ["zeroize/alloc"]
-
-# fiat-crypto backend with formally-verified field arithmetic
-fiat_backend = ["fiat-crypto"]
-# The SIMD backend uses parallel formulas, using either AVX2 or AVX512-IFMA.
-simd_backend = ["nightly", "packed_simd"]
diff --git a/src/backend/mod.rs b/src/backend/mod.rs
@@ -36,5 +36,5 @@
 
 pub mod serial;
 
-#[cfg(any(feature = "simd_backend", docsrs))]
+#[cfg(any(curve25519_dalek_backend = "simd", docsrs))]
 pub mod vector;
diff --git a/src/backend/serial/mod.rs b/src/backend/serial/mod.rs
@@ -21,8 +21,7 @@
 use cfg_if::cfg_if;
 
 cfg_if! {
-
-    if #[cfg(feature = "fiat_backend")] {
+    if #[cfg(curve25519_dalek_backend = "fiat")] {
 
         #[cfg(curve25519_dalek_bits = "32")]
         pub mod fiat_u32;
@@ -44,7 +43,7 @@ cfg_if! {
 pub mod curve_models;
 
 #[cfg(not(all(
-    feature = "simd_backend",
+    curve25519_dalek_backend = "simd",
     any(target_feature = "avx2", target_feature = "avx512ifma")
 )))]
 pub mod scalar_mul;
diff --git a/src/backend/vector/mod.rs b/src/backend/vector/mod.rs
@@ -12,7 +12,7 @@
 #![doc = include_str!("../../../docs/parallel-formulas.md")]
 
 #[cfg(not(any(target_feature = "avx2", target_feature = "avx512ifma", docsrs)))]
-compile_error!("simd_backend selected without target_feature=+avx2 or +avx512ifma");
+compile_error!("'simd' backend selected without target_feature=+avx2 or +avx512ifma");
 
 #[cfg(any(
     all(target_feature = "avx2", not(target_feature = "avx512ifma")),

diff --git a/src/constants.rs b/src/constants.rs
@@ -37,7 +37,7 @@ use crate::ristretto::RistrettoPoint;
 use crate::scalar::Scalar;
 
 cfg_if! {
-    if #[cfg(feature = "fiat_backend")] {
+    if #[cfg(curve25519_dalek_backend = "fiat")] {
         #[cfg(curve25519_dalek_bits = "32")]
         pub use crate::backend::serial::fiat_u32::constants::*;
         #[cfg(curve25519_dalek_bits = "64")]
@@ -149,7 +149,7 @@ mod test {
 
     /// Test that d = -121665/121666
     #[test]
-    #[cfg(all(curve25519_dalek_bits = "32", not(feature = "fiat_backend")))]
+    #[cfg(all(curve25519_dalek_bits = "32", not(curve25519_dalek_backend = "fiat")))]
     fn test_d_vs_ratio() {
         use crate::backend::serial::u32::field::FieldElement2625;
         let a = -&FieldElement2625([121665, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
@@ -162,7 +162,7 @@ mod test {
 
     /// Test that d = -121665/121666
     #[test]
-    #[cfg(all(curve25519_dalek_bits = "64", not(feature = "fiat_backend")))]
+    #[cfg(all(curve25519_dalek_bits = "64", not(curve25519_dalek_backend = "fiat")))]
     fn test_d_vs_ratio() {
         use crate::backend::serial::u64::field::FieldElement51;
         let a = -&FieldElement51([121665, 0, 0, 0, 0]);

diff --git a/src/edwards.rs b/src/edwards.rs
@@ -139,12 +139,12 @@ use crate::traits::MultiscalarMul;
 use crate::traits::{VartimeMultiscalarMul, VartimePrecomputedMultiscalarMul};
 
 #[cfg(not(all(
-    feature = "simd_backend",
+    curve25519_dalek_backend = "simd",
     any(target_feature = "avx2", target_feature = "avx512ifma")
 )))]
 use crate::backend::serial::scalar_mul;
 #[cfg(all(
-    feature = "simd_backend",
+    curve25519_dalek_backend = "simd",
     any(target_feature = "avx2", target_feature = "avx512ifma")
 ))]
 use crate::backend::vector::scalar_mul;

diff --git a/src/field.rs b/src/field.rs
@@ -36,7 +36,7 @@ use crate::backend;
 use crate::constants;
 
 cfg_if! {
-    if #[cfg(feature = "fiat_backend")] {
+    if #[cfg(curve25519_dalek_backend = "fiat")] {
         #[cfg(curve25519_dalek_bits = "32")]
         pub use backend::serial::fiat_u32::field::*;
         #[cfg(curve25519_dalek_bits = "64")]

diff --git a/src/lib.rs b/src/lib.rs
@@ -10,7 +10,7 @@
 // - Henry de Valence <[email protected]>
 
 #![no_std]
-#![cfg_attr(feature = "simd_backend", feature(stdsimd))]
+#![cfg_attr(curve25519_dalek_backend = "simd", feature(stdsimd))]
 #![cfg_attr(docsrs, feature(doc_auto_cfg, doc_cfg, doc_cfg_hide))]
 #![cfg_attr(docsrs, doc(cfg_hide(docsrs)))]
 //------------------------------------------------------------------------

diff --git a/src/ristretto.rs b/src/ristretto.rs
@@ -197,12 +197,12 @@ use crate::traits::Identity;
 use crate::traits::{MultiscalarMul, VartimeMultiscalarMul, VartimePrecomputedMultiscalarMul};
 
 #[cfg(not(all(
-    feature = "simd_backend",
+    curve25519_dalek_backend = "simd",
     any(target_feature = "avx2", target_feature = "avx512ifma")
 )))]
 use crate::backend::serial::scalar_mul;
 #[cfg(all(
-    feature = "simd_backend",
+    curve25519_dalek_backend = "simd",
     any(target_feature = "avx2", target_feature = "avx512ifma")
 ))]
 use crate::backend::vector::scalar_mul;

diff --git a/src/scalar.rs b/src/scalar.rs
@@ -170,7 +170,7 @@ use crate::backend;
 use crate::constants;
 
 cfg_if! {
-    if #[cfg(feature = "fiat_backend")] {
+    if #[cfg(curve25519_dalek_backend = "fiat")] {
         /// An `UnpackedScalar` represents an element of the field GF(l), optimized for speed.
         ///
         /// This is a type alias for one of the scalar types in the `backend`