Skip to content

Commit

Permalink
k256: add expose-field feature; fix/CI benches (#161)
Browse files Browse the repository at this point in the history
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:
rust-lang/cargo#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.
  • Loading branch information
tarcieri authored Sep 4, 2020
1 parent 0eb8f6f commit 089aab4
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 18 deletions.
1 change: 1 addition & 0 deletions .github/workflows/k256.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 3 additions & 1 deletion k256/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -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"]
37 changes: 21 additions & 16 deletions k256/bench/bench.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use core::convert::TryInto;
//! secp256k1 benchmarks
use criterion::measurement::Measurement;
use criterion::{criterion_group, criterion_main, BenchmarkGroup, Criterion};

use hex_literal::hex;
use k256::{elliptic_curve::FromBytes, FieldElement, ProjectivePoint, Scalar};

fn test_scalar_x() -> Scalar {
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -82,20 +81,26 @@ fn bench_scalar(c: &mut Criterion) {
}

fn test_field_element_x() -> FieldElement {
FieldElement::from_bytes(&[
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,
])
FieldElement::from_bytes(
&[
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()
}

fn test_field_element_y() -> FieldElement {
FieldElement::from_bytes(&[
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,
])
FieldElement::from_bytes(
&[
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()
}

Expand Down
2 changes: 1 addition & 1 deletion k256/src/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions k256/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down

0 comments on commit 089aab4

Please sign in to comment.