From a6c9ba866acc0ede4f2939146246908139dd1ddc Mon Sep 17 00:00:00 2001
From: Tony Arcieri <bascule@gmail.com>
Date: Thu, 3 Sep 2020 20:31:47 -0700
Subject: [PATCH] k256: add `expose-field` feature; fix/CI benches

The benchmarks want direct access to `FieldElement` for benchmarking
purposes. This is a legitimate use case, but breaks encapsulation in
that we'd like to generally prevent exposure of `FieldElement` except
for "hazmat"-style use cases.

This is a well-recognized problem with Rust in general:
https://github.com/rust-lang/cargo/issues/2911

This commit adds a semi-hidden `expose-field` feature in order to fix
the benchmark. Perhaps it will be abused, but the alternative is not
having a benchmark, which seems bad too.

It also adds a CI build step to ensure the benchmarks compile to prevent
this sort of regression in the future.
---
 .github/workflows/k256.yml |  1 +
 k256/Cargo.toml            |  4 +++-
 k256/bench/bench.rs        | 15 +++++++--------
 k256/src/arithmetic.rs     |  2 +-
 k256/src/lib.rs            |  3 +++
 5 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/.github/workflows/k256.yml b/.github/workflows/k256.yml
index 889657c46..d1c669c1a 100644
--- a/.github/workflows/k256.yml
+++ b/.github/workflows/k256.yml
@@ -62,3 +62,4 @@ jobs:
     - run: cargo test --all-features
     - run: cargo test --features field-montgomery,rand
     - run: cargo test --features force-32-bit,rand
+    - run: cargo build --all-features --benches
diff --git a/k256/Cargo.toml b/k256/Cargo.toml
index 7143470de..b26619486 100644
--- a/k256/Cargo.toml
+++ b/k256/Cargo.toml
@@ -39,6 +39,7 @@ digest = ["elliptic-curve/digest", "ecdsa-core/digest"]
 ecdh = ["elliptic-curve/ecdh", "rand", "zeroize"]
 ecdsa = ["arithmetic", "digest", "ecdsa-core/rand", "ecdsa-core/sign", "ecdsa-core/verify", "rand", "zeroize"]
 endomorphism-mul = []
+expose-field = ["arithmetic"]
 field-montgomery = []
 force-32-bit = []
 keccak256 = ["digest", "sha3"]
@@ -50,10 +51,11 @@ std = ["elliptic-curve/std"]
 zeroize = ["elliptic-curve/zeroize"]
 
 [package.metadata.docs.rs]
-all-features = true
+features = ["ecdh", "ecdsa", "sha256", "keccak256"]
 rustdoc-args = ["--cfg", "docsrs"]
 
 [[bench]]
 name = "bench"
 path = "bench/bench.rs"
 harness = false
+required-features = ["expose-field"]
diff --git a/k256/bench/bench.rs b/k256/bench/bench.rs
index f5eb41494..26ae867da 100644
--- a/k256/bench/bench.rs
+++ b/k256/bench/bench.rs
@@ -1,8 +1,9 @@
-use core::convert::TryInto;
+//! secp256k1 benchmarks
+
 use criterion::measurement::Measurement;
 use criterion::{criterion_group, criterion_main, BenchmarkGroup, Criterion};
-
 use k256::{elliptic_curve::FromBytes, FieldElement, ProjectivePoint, Scalar};
+use hex_literal::hex;
 
 fn test_scalar_x() -> Scalar {
     Scalar::from_bytes(
@@ -30,10 +31,8 @@ fn test_scalar_y() -> Scalar {
 
 fn bench_point_mul<'a, M: Measurement>(group: &mut BenchmarkGroup<'a, M>) {
     let p = ProjectivePoint::generator();
-    let ms = "AA5E28D6A97A2479A65527F7290311A3624D4CC0FA1578598EE3C2613BF99522";
-    let m = hex::decode(&ms).unwrap();
-    let s = Scalar::from_bytes(m[..].try_into().unwrap()).unwrap();
-
+    let m = hex!("AA5E28D6A97A2479A65527F7290311A3624D4CC0FA1578598EE3C2613BF99522");
+    let s = Scalar::from_bytes(&m.into()).unwrap();
     group.bench_function("point-scalar mul", |b| b.iter(|| &p * &s));
 }
 
@@ -86,7 +85,7 @@ fn test_field_element_x() -> FieldElement {
         0xbb, 0x48, 0x8a, 0xef, 0x41, 0x6a, 0x41, 0xd7, 0x68, 0x0d, 0x1c, 0xf0, 0x1d, 0x70, 0xf5,
         0x9b, 0x60, 0xd7, 0xf5, 0xf7, 0x7e, 0x30, 0xe7, 0x8b, 0x8b, 0xf9, 0xd2, 0xd8, 0x82, 0xf1,
         0x56, 0xa6,
-    ])
+    ].into())
     .unwrap()
 }
 
@@ -95,7 +94,7 @@ fn test_field_element_y() -> FieldElement {
         0x67, 0xe2, 0xf6, 0x80, 0x71, 0xed, 0x82, 0x81, 0xe8, 0xae, 0xd6, 0xbc, 0xf1, 0xc5, 0x20,
         0x7c, 0x5e, 0x63, 0x37, 0x22, 0xd9, 0x20, 0xaf, 0xd6, 0xae, 0x22, 0xd0, 0x6e, 0xeb, 0x80,
         0x35, 0xe3,
-    ])
+    ].into())
     .unwrap()
 }
 
diff --git a/k256/src/arithmetic.rs b/k256/src/arithmetic.rs
index 545b42648..46687316b 100644
--- a/k256/src/arithmetic.rs
+++ b/k256/src/arithmetic.rs
@@ -9,7 +9,7 @@ pub(crate) mod scalar;
 #[cfg(test)]
 mod dev;
 
-pub(crate) use field::FieldElement;
+pub use field::FieldElement;
 
 use crate::Secp256k1;
 use affine::AffinePoint;
diff --git a/k256/src/lib.rs b/k256/src/lib.rs
index 6dcc1f142..505566868 100644
--- a/k256/src/lib.rs
+++ b/k256/src/lib.rs
@@ -68,6 +68,9 @@ pub use arithmetic::{
     scalar::{NonZeroScalar, Scalar},
 };
 
+#[cfg(feature = "expose-field")]
+pub use arithmetic::FieldElement;
+
 use elliptic_curve::consts::U32;
 
 #[cfg(feature = "oid")]