Skip to content

ff: More trait refactoring#228

Merged
str4d merged 7 commits into
zcash:masterfrom
str4d:ff-more-trait-refactoring
May 12, 2020
Merged

ff: More trait refactoring#228
str4d merged 7 commits into
zcash:masterfrom
str4d:ff-more-trait-refactoring

Conversation

@str4d
Copy link
Copy Markdown
Contributor

@str4d str4d commented May 2, 2020

Addresses trait-related comments from @ebfull on #227.

str4d added 2 commits May 2, 2020 16:07
The sqrt() function is now part of the Field trait. ff_derive returns an
error on fields for which it does not support generating a square root
function.

Note that Fq6 and Fq12 in pairing::bls12_381 leave the function
unimplemented. They will be dropped once the migration to the bls12_381
crate is complete. The equivalent structs in that crate are not exposed.
It is only used internally in the bls12_381 crate, and field extensions
aren't exposed anywhere in the Zcash stack.
@str4d str4d force-pushed the ff-more-trait-refactoring branch from be2f02d to 3e7a2e4 Compare May 2, 2020 04:08
@codecov
Copy link
Copy Markdown

codecov Bot commented May 2, 2020

Codecov Report

Merging #228 into master will decrease coverage by 0.01%.
The diff coverage is 78.70%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #228      +/-   ##
==========================================
- Coverage   65.25%   65.24%   -0.02%     
==========================================
  Files         106      106              
  Lines       15057    14977      -80     
==========================================
- Hits         9825     9771      -54     
+ Misses       5232     5206      -26     
Impacted Files Coverage Δ
bellman/src/domain.rs 63.58% <0.00%> (ø)
bellman/src/gadgets/multieq.rs 50.00% <ø> (ø)
bellman/src/groth16/generator.rs 89.94% <ø> (+1.77%) ⬆️
bellman/src/groth16/tests/dummy_engine.rs 7.10% <0.00%> (-0.38%) ⬇️
bellman/src/groth16/tests/mod.rs 74.44% <ø> (-0.56%) ⬇️
ff/ff_derive/src/lib.rs 15.38% <0.00%> (-4.23%) ⬇️
group/src/lib.rs 11.76% <ø> (ø)
pairing/src/bls12_381/mod.rs 62.03% <ø> (+0.53%) ⬆️
pairing/src/lib.rs 75.00% <ø> (ø)
zcash_primitives/src/jubjub/mod.rs 50.28% <ø> (ø)
... and 62 more

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 b02cf3b...c597db5. Read the comment docs.

str4d added 5 commits May 2, 2020 18:54
This enables generic code to reliably operate on the bits of an encoded
field element, by converting them to and from a known (little)
endianness.

The BitAnd and Shr bounds on PrimeField are now removed, as users can
perform these operations themselves as needed.
ff_derive still implements Ord and PartialOrd for the fields it
implements, because pairing::bls12_381 internally assumes that those are
implemented. Once we delete that implementation, we will remove the Ord
and PartialOrd implementations from ff_derive.
The only places we don't use constant u64 limbs, we use PrimeField::char
instead (except in a single test where we use a field element).
Now that PrimeField::ReprEndianness exists, users can obtain a
known-endianness representation from the output of PrimeField::char
(which is a PrimeField::Repr, and should return a representation with
the same endianness as PrimeField::into_repr).
@str4d str4d force-pushed the ff-more-trait-refactoring branch from 24e9d68 to 9114c36 Compare May 2, 2020 06:56
@str4d str4d mentioned this pull request May 3, 2020
7 tasks
@str4d str4d added this to the Core Sprint 2020-17 milestone May 4, 2020
Comment thread ff/src/lib.rs
/// specific endianness. This is useful when you need to act on the bit representation
/// of an element generically, as the native binary representation of a prime field is
/// field-dependent.
pub trait Endianness: ByteOrder {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This could be renamed ByteOrderExt if we workshop its API to be more widely useful, but it can only be implemented for byteorder::{BigEndian, LittleEndian} because byteorder::ByteOrder is sealed, so the only effect this has on end users is the import they use to access the APIs.

@str4d str4d merged commit 3727077 into zcash:master May 12, 2020
@str4d str4d deleted the ff-more-trait-refactoring branch May 12, 2020 21:18
greg0x pushed a commit to valargroup/librustzcash that referenced this pull request Mar 12, 2026
Add Slack PR notifier for upstream Zcash contributions
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.

3 participants