Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support no-std builds #4

Merged
merged 5 commits into from
Sep 23, 2021
Merged

Support no-std builds #4

merged 5 commits into from
Sep 23, 2021

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Mar 3, 2021

No description provided.

ebfull
ebfull previously requested changes Mar 4, 2021
src/arithmetic/curves.rs Outdated Show resolved Hide resolved
src/fields/fq.rs Outdated Show resolved Hide resolved
Base automatically changed from pasta_curves-crate to main March 4, 2021 22:40
@str4d str4d marked this pull request as draft June 4, 2021 18:45
@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2021

Codecov Report

Merging #4 (2b35011) into main (adf66ae) will decrease coverage by 0.96%.
The diff coverage is 17.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main       #4      +/-   ##
==========================================
- Coverage   66.50%   65.53%   -0.97%     
==========================================
  Files          10       10              
  Lines        1421     1445      +24     
==========================================
+ Hits          945      947       +2     
- Misses        476      498      +22     
Impacted Files Coverage Δ
src/arithmetic/curves.rs 0.00% <ø> (ø)
src/arithmetic/fields.rs 80.95% <ø> (ø)
src/curves.rs 37.95% <0.00%> (ø)
src/lib.rs 100.00% <ø> (ø)
src/pallas.rs 100.00% <ø> (ø)
src/vesta.rs 100.00% <ø> (ø)
src/fields/fp.rs 73.24% <18.36%> (-2.09%) ⬇️
src/fields/fq.rs 72.72% <18.36%> (-2.08%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update adf66ae...2b35011. Read the comment docs.

The `FieldExt` trait was originally the only trait implemented in this
crate. When we added `ff` support, we reworked `FieldExt` to be an
extension trait on top of `ff::PrimeField`, but left the existing impls
in `FieldExt`. This resulted in some circular dependencies that prevent
us from making `FieldExt` conditional (e.g. for no-std support).

This commit removes the cycles like so:

- `ff::PrimeField::{from_repr, to_repr}` were implemented as calls to
  `FieldExt::{from_bytes, to_bytes}`. The field encoding/decoding logic
  is moved into the `ff::PrimeField` trait impl, and `FieldExt` now
  calls into `ff::PrimeField`.

- `ff::Field::sqrt` was implemented in terms of `FieldExt::sqrt_alt`.
  Given that the latter is a trivial wrapper around the `SqrtTables`
  implementation, we duplicate the call to eliminate the cycle.

- `ff::Field::random` used `FieldExt::from_bytes_wide`, which wraps
  either `Fp::from_u512` or `Fq::from_u512`. We now use these internal
  methods directly.
@str4d str4d force-pushed the no-std branch 3 times, most recently from 3397293 to abcf70a Compare September 20, 2021 17:48
We re-introduce the Tonelli-Shank square root algoritm that was removed
in zcash/halo2#120, to use in no-std mode (the table-based impl requires
allocations, and also uses 29kiB of memory which is a problem for
constrained environments that typically need no-std).
@str4d str4d marked this pull request as ready for review September 20, 2021 18:19
@str4d str4d requested a review from ebfull September 20, 2021 18:28
@str4d str4d added this to the Core Sprint 2021-36 milestone Sep 20, 2021
src/arithmetic/fields.rs Outdated Show resolved Hide resolved
src/fields/fp.rs Outdated Show resolved Hide resolved
src/fields/fq.rs Outdated Show resolved Hide resolved
Co-authored-by: Daira Hopwood <daira@jacaranda.org>
@str4d str4d mentioned this pull request Sep 21, 2021
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

utACK

@str4d str4d dismissed ebfull’s stale review September 23, 2021 13:38

Addressed comments.

@str4d str4d merged commit 275dad2 into main Sep 23, 2021
@str4d str4d deleted the no-std branch September 23, 2021 13:46
@str4d str4d mentioned this pull request Dec 7, 2021
huitseeker pushed a commit to huitseeker/pasta_curves that referenced this pull request Apr 15, 2023
It doesn't affect the serialized output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants