Skip to content

Implement BatchNormalize for NonIdentity - #1896

Merged
tarcieri merged 8 commits into
RustCrypto:masterfrom
khonsulabs:non-identity-batch-normalize
Jun 13, 2025
Merged

Implement BatchNormalize for NonIdentity#1896
tarcieri merged 8 commits into
RustCrypto:masterfrom
khonsulabs:non-identity-batch-normalize

Conversation

@daxpedda

@daxpeddadaxpedda commented Jun 8, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

As discussed in #1889. I will add some tests in elliptic-curves as well.

Resolves#1889.
Companion PR: RustCrypto/elliptic-curves#1248.

@daxpeddadaxpedda mentioned this pull request Jun 8, 2025
Closed
tarcieri
tarcieri reviewed Jun 11, 2025
View reviewed changes
Comment threadelliptic-curve/src/point/non_identity.rs Outdated
tarcieri
tarcieri reviewed Jun 11, 2025
View reviewed changes
Comment threadelliptic-curve/src/point/non_identity.rs Outdated
@daxpedda
daxpeddaforce-pushed the non-identity-batch-normalize branch from 1c8e9df to e2c3a8fCompareJune 12, 2025 09:08
@daxpeddadaxpedda mentioned this pull request Jun 12, 2025
Closed
daxpedda
daxpedda commented Jun 12, 2025
View reviewed changes
Comment threadelliptic-curve/src/point/non_identity.rs Outdated
@daxpedda

daxpedda commented Jun 12, 2025•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

I completely removed any usage of mem::transmute() and mem::transmute_copy() and opted for safer alternatives.

One notably change: instead of mem::transmute_copy() for fixed arrays, I used array::map(). This seems to be optimized to a memcpy by compilers (Godbolt). Additionally this gets rid of one unsafe call.

@daxpedda
daxpedda requested a review from tarcieriJune 12, 2025 09:21
tarcieri
tarcieri reviewed Jun 12, 2025
View reviewed changes
Comment threadelliptic-curve/src/lib.rs
daxpedda
daxpedda commented Jun 12, 2025
View reviewed changes
Comment threadelliptic-curve/src/point/non_identity.rs
@daxpedda
daxpedda requested a review from tarcieriJune 12, 2025 16:39
@daxpedda
daxpeddaforce-pushed the non-identity-batch-normalize branch 2 times, most recently from 225884b to e3e5f8fCompareJune 12, 2025 16:41
@daxpedda
daxpeddaforce-pushed the non-identity-batch-normalize branch from e3e5f8f to 0a351c7CompareJune 12, 2025 21:29
tarcieri
tarcieri reviewed Jun 12, 2025
View reviewed changes
Comment threadelliptic-curve/src/point.rs
tarcieri
tarcieri reviewed Jun 12, 2025
View reviewed changes
Comment threadelliptic-curve/src/point/non_identity.rs Outdated
@daxpedda
daxpedda requested a review from tarcieriJune 13, 2025 00:12
@daxpedda
daxpeddaforce-pushed the non-identity-batch-normalize branch from 21d93b8 to ced7993CompareJune 13, 2025 00:14
@daxpedda
daxpeddaforce-pushed the non-identity-batch-normalize branch from ced7993 to 6edc705CompareJune 13, 2025 00:14
tarcieri
tarcieri reviewed Jun 13, 2025
View reviewed changes
Comment threadelliptic-curve/src/point/non_identity.rs Outdated
@tarcieri

tarcieri commented Jun 13, 2025

Copy link
Copy Markdown
Member

@daxpedda can you also add cargo careful and cargo miri to CI, similar to these?

https://github.com/RustCrypto/crypto-bigint/blob/dff9eb8/.github/workflows/crypto-bigint.yml#L98-L105

https://github.com/RustCrypto/crypto-bigint/blob/dff9eb8/.github/workflows/crypto-bigint.yml#L133-L147

It would also be good to find a way to exercise this code with tests somewhere in this repo, possibly leveraging MockCurve.

tarcieri
tarcieri approved these changes Jun 13, 2025
View reviewed changes

@tarcieritarcieri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now. I can handle the CI changes as a followup.

@daxpedda

daxpedda commented Jun 13, 2025

Copy link
Copy Markdown
ContributorAuthor

Looks good now. I can handle the CI changes as a followup.

Already on it. Trying to add a test right now.

tarcieri
tarcieri reviewed Jun 13, 2025
View reviewed changes
Comment threadelliptic-curve/src/point/non_identity.rs

#[allow(unsafe_code)]
// SAFETY: `NonIdentity` is `repr(transparent)`.
let points: &[P; N] = unsafe { &*points.as_ptr().cast() };

@tarcieritarcieriJun 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious if you could extract an AsRef impl here, but perhaps I can experiment with that myself

@daxpeddadaxpeddaJun 13, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently not, because array is a foreign type. Same applies to slices.

I guess we could add it as a method?

@daxpedda
daxpedda requested a review from tarcieriJune 13, 2025 18:46
@tarcieri
tarcieri merged commit f24c2ae into RustCrypto:masterJun 13, 2025
tarcieri pushed a commit to RustCrypto/elliptic-curves that referenced this pull request Jun 13, 2025
@daxpedda
Updateelliptic-curve(#1257)
b9103a6
* Update to changes in `BatchNormalize` (RustCrypto/traits#1896)
* Update to hash2curve changes (RustCrypto/traits#1901)
@tarcieritarcieri mentioned this pull request Jun 23, 2026
Merged
tarcieri added a commit that referenced this pull request Jun 23, 2026
@tarcieri
elliptic-curve v0.14.0 (#2466)
ab8ee23
## Added
- Implement `PartialEq + Eq` for `NonIdentity` and `NonZeroScalar` (#1834)
- Implement `Zeroize` for `NonIdentity` (#1832)
- `NonIdentity::mul_by_generator()` (#1833)
- Implement `Mul<&NonZeroScalar>` for `NonIdentity` (#1852)
- Implement `Mul<NonIdentity>` for `NonZeroScalar` (#1855)
- Expose `AffineCoordinates::y` (#1891)
- Scalar macros originall from `primeorder` (#1894)
- Implement `BatchNormalize` for `NonIdentity` (#1896)
- Re-export `group::Curve` as `CurveGroup` (#1902)
- `NonIdentity`/`NonZeroScalar` casting methods (#1903)
- `AffineCoordinates::from_coordinates` (#1996)
- `getrandom` feature (#2085)
- `ctutils` traits to `arithmetic` bounds (#2166)
- `Retrieve` bound for `C::Scalar` (#2169)
- `crypto_common::Generate` support (#2173, #2208)
- Implement `crypto_common::TryKeyInit` for `SecretKey<C>` (#2174)
- `dev::bench_projective!` macro (#2177)
- Provide `Sec1Point::from/to_sec1_bytes` (#2221)
- Implement `From<SecretKey<C>>` for `PublicKey<C>` (#2247)
- `SecretKey::diffie_hellman` (#2248)
- `LinearCombination::lincomb_vartime` method (#2286)
- `ops::MulVartime` trait and bound `Scalar` (#2379)
- `ops::MulByGeneratorVartime` trait ([#2381])
- `SecretKey::from_pem` (#2387)
- `SecretKey::from_der` (#2408)
- `hazmat` module with `FieldArithmetic` trait (#2458)
- `Double::double_in_place` (#2464)
## Changed
- Migrate from `generic-array` to `hybrid-array` (#1462)
- Rename `LinearCombinationExt` => `LinearCombination`; replacing old
trait (#1501)
- Edition changed to 2024 and MSRV bumped to 1.85 (#1759)
- Make `SecretKey::new` fallible (#1804)
- Replace `ops::Invert` trait with `crypto_bigint::Invert` (#1839)
- Rename `SecretKey::new` => `::from_scalar` (#1893)
- Replace `Reduce` trait with `crypto_bigint::Reduce` (#1949)
- Bump `serdect` dependency to v0.4 (#1978)
- Use `crypto_bigint::Odd` to represent `Curve::ORDER` (#2006)
- Bound `Curve::Uint` on `Unsigned` (#2007)
- Rename `ScalarPrimitive` => `ScalarValue` (#2008)
- Accept mixed-case hex-encoded strings in `FromStr` impl for
`ScalarValue` (#2037)
- Deprecate `SecretKey::random` (#2086)
- Move `MockCurve` to `dev::mock_curve` (#2176)
- Bump `rand_core` to v0.10 (#2250)
- Rename `EncodedPoint` => `Sec1Point` (#2264)
- Bump `crypto-bigint` to v0.7 (#2330)
- Bump `digest` to v0.11 (#2331)
- Bump `sec1` to v0.8 (#2339)
- Bump `hkdf` dependency to v0.13 (#2349)
- Use `*Vartime` as a suffix in names (#2378)
- Bump `pkcs8` to v0.11 (#2397)
- Bump `ff` and `group` to v0.14 (#2430, #2431)
- Simplify `BatchInvert` trait (#2455)
- Replace `FieldBytesEncoding` trait with `C::FIELD_ENDIANNESS` (#2457)
- Move `Double` to `ops` module (#2465)
## Removed
- `hazmat` feature (#1599)
- `hash2curve` and `oprf` modules: moved to same-name crates (#1929)
- PKCS#8 blanket impls for SEC1 private key traits (#1930)
- `ShrAssign` bound on `Scalar`s (#1938)
- JWK support: migrated to `jose-jwk` crate (#1963)
- `weierstrass` module (#2005)
- `bits` feature (#2417)
## Fixed
- Include curve OID in SEC1 private keys (#1707, #1933)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tarcieritarcieriAwaiting requested review from tarcieri

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement BatchNormalize for NonIdentity

2 participants

@daxpedda@tarcieri