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.

Sign up for GitHub

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

Jump to bottom

Update to hash-to-curve draft 16, with some API adjustments #90

Merged
merged 6 commits into from
Jul 21, 2024
Merged

Update to hash-to-curve draft 16, with some API adjustments #90

merged 6 commits into from
Jul 21, 2024

Conversation

andrewwhitehead
Copy link
Contributor

  • Adds a Message trait for use by expand_message rather than requiring the message to be a single [u8] (more flexibility for larger message values and no-alloc situations)
  • Allows expand_message to be used with security levels other than k=128
  • Removes lifetime parameters and the separate InitExpandMessage trait
  • Moves unit tests to tests/ for compilation performance

@str4d str4d mentioned this pull request Dec 6, 2022
Open
daira
daira reviewed Jan 12, 2023
View reviewed changes
tests/hash_to_curve_g1.rs Outdated Show resolved Hide resolved
@str4d
Copy link
Member

str4d commented Feb 27, 2023 •
edited
Loading

Hey! This PR has been close to the top of our to-review list for months now, but just as it keeps getting close to "hey, we can start working on this now!" some other emergency pushes it back down. Part of the problem is our desire to review the delta from draft 12 to draft 16 and confirm there are no backwards-incompatible changes, but also this PR is very big and requires significant uninterrupted time to review (even though most of it appears to only be test moves, it does add to the review complexity).

I strongly want to get this in, and will continue to try and do so, but we do not have time to review it before the next crate release.

@str4d
Copy link
Member

str4d commented Feb 27, 2023

On the above point, I think it would be significantly easier to review this PR if it was split into two separate PRs: a move-only PR that moves the tests to tests/, and then this PR changing the API.

This was referenced Feb 27, 2023
Merged
Closed
vmx added a commit to filecoin-project/ref-fvm that referenced this pull request May 24, 2024
@vmx
fix: remove the pairing feature from fvm_shared
8a367f9 
The `pairing` feature from the `fvm_shared` crate isn't used. It causes
problems, as it forces the `subtle` dependency to v2.4.1, although the
rest is happy to have v2.5.0.

Here is a detailed dependency graph and issue outline:

`fvm_shared` depends on `bls-signatures`.
In `bls-signatures` we depend on an old version (v0.11) of `hkdf`.
That version depends on `hmac` v0.11, which depends on `crypto-mac` v0.11.
`crypto-mac` v0.11.0 depends on `subtle` v2. That is fine, it would
automatically select v2.5.0.
The problem is that `crypto-mac` v0.11.1 pins `subtle` to exactly v2.4,
therefore v2.5.0 won't be selected.

The obvious thing is to upgrade in`bls-signatures` the version of `hkdf`
to the latest v0.12.
That would make it possible to use `subtle` v2.5.0.
The problem is that such an upgrade is not easily possible.
`hkdf` v0.12 depends on a newer version v0.10 of the `sha2` crate.
Updating that breaks the `bls12_381` crate.
The reason is the current version v0.8.0 of `bls12_381` depends on an old
version v0.9 of the `digest` crate.

The obvious thing is to upgrade in `bls12_381` the version of `digest` to
v0.10.
That would make it possible to get `hkdf` v0.12 built.
But such an upgrade is and open issue at
zkcrypto/bls12_381#102, which mentions that it's
blocked on zkcrypto/bls12_381#90.
That pull request is about updating do the hash-to-curve draft v16, currently
it's using v12.
We use that code path in `bls-signatures`, else we wouldn't enable the
`experimental` feature of `bls12_381`.
So it's even not clear if we'd want such a change to v16.
@vmx vmx mentioned this pull request May 24, 2024
Merged
vmx added a commit to filecoin-project/ref-fvm that referenced this pull request May 24, 2024
@vmx
fix: remove the pairing feature from fvm_shared
fe2665c 
The `pairing` feature from the `fvm_shared` crate isn't used. It causes
problems, as it forces the `subtle` dependency to v2.4.1, although the
rest is happy to have v2.5.0.

Here is a detailed dependency graph and issue outline:

`fvm_shared` depends on `bls-signatures`.
In `bls-signatures` we depend on an old version (v0.11) of `hkdf`.
That version depends on `hmac` v0.11, which depends on `crypto-mac` v0.11.
`crypto-mac` v0.11.0 depends on `subtle` v2. That is fine, it would
automatically select v2.5.0.
The problem is that `crypto-mac` v0.11.1 pins `subtle` to exactly v2.4,
therefore v2.5.0 won't be selected.

The obvious thing is to upgrade in`bls-signatures` the version of `hkdf`
to the latest v0.12.
That would make it possible to use `subtle` v2.5.0.
The problem is that such an upgrade is not easily possible.
`hkdf` v0.12 depends on a newer version v0.10 of the `sha2` crate.
Updating that breaks the `bls12_381` crate.
The reason is the current version v0.8.0 of `bls12_381` depends on an old
version v0.9 of the `digest` crate.

The obvious thing is to upgrade in `bls12_381` the version of `digest` to
v0.10.
That would make it possible to get `hkdf` v0.12 built.
But such an upgrade is and open issue at
zkcrypto/bls12_381#102, which mentions that it's
blocked on zkcrypto/bls12_381#90.
That pull request is about updating do the hash-to-curve draft v16, currently
it's using v12.
We use that code path in `bls-signatures`, else we wouldn't enable the
`experimental` feature of `bls12_381`.
So it's even not clear if we'd want such a change to v16.
Stebalien pushed a commit to filecoin-project/ref-fvm that referenced this pull request May 24, 2024
@vmx
fix: remove the pairing feature from fvm_shared (#2009)
206a772 
The `pairing` feature from the `fvm_shared` crate isn't used. It causes
problems, as it forces the `subtle` dependency to v2.4.1, although the
rest is happy to have v2.5.0.

Here is a detailed dependency graph and issue outline:

`fvm_shared` depends on `bls-signatures`.
In `bls-signatures` we depend on an old version (v0.11) of `hkdf`.
That version depends on `hmac` v0.11, which depends on `crypto-mac` v0.11.
`crypto-mac` v0.11.0 depends on `subtle` v2. That is fine, it would
automatically select v2.5.0.
The problem is that `crypto-mac` v0.11.1 pins `subtle` to exactly v2.4,
therefore v2.5.0 won't be selected.

The obvious thing is to upgrade in`bls-signatures` the version of `hkdf`
to the latest v0.12.
That would make it possible to use `subtle` v2.5.0.
The problem is that such an upgrade is not easily possible.
`hkdf` v0.12 depends on a newer version v0.10 of the `sha2` crate.
Updating that breaks the `bls12_381` crate.
The reason is the current version v0.8.0 of `bls12_381` depends on an old
version v0.9 of the `digest` crate.

The obvious thing is to upgrade in `bls12_381` the version of `digest` to
v0.10.
That would make it possible to get `hkdf` v0.12 built.
But such an upgrade is and open issue at
zkcrypto/bls12_381#102, which mentions that it's
blocked on zkcrypto/bls12_381#90.
That pull request is about updating do the hash-to-curve draft v16, currently
it's using v12.
We use that code path in `bls-signatures`, else we wouldn't enable the
`experimental` feature of `bls12_381`.
So it's even not clear if we'd want such a change to v16.
rjan90 pushed a commit to filecoin-project/ref-fvm that referenced this pull request Jun 12, 2024
@vmx @rjan90
fix: remove the pairing feature from fvm_shared (#2009)
c5e6576 
The `pairing` feature from the `fvm_shared` crate isn't used. It causes
problems, as it forces the `subtle` dependency to v2.4.1, although the
rest is happy to have v2.5.0.

Here is a detailed dependency graph and issue outline:

`fvm_shared` depends on `bls-signatures`.
In `bls-signatures` we depend on an old version (v0.11) of `hkdf`.
That version depends on `hmac` v0.11, which depends on `crypto-mac` v0.11.
`crypto-mac` v0.11.0 depends on `subtle` v2. That is fine, it would
automatically select v2.5.0.
The problem is that `crypto-mac` v0.11.1 pins `subtle` to exactly v2.4,
therefore v2.5.0 won't be selected.

The obvious thing is to upgrade in`bls-signatures` the version of `hkdf`
to the latest v0.12.
That would make it possible to use `subtle` v2.5.0.
The problem is that such an upgrade is not easily possible.
`hkdf` v0.12 depends on a newer version v0.10 of the `sha2` crate.
Updating that breaks the `bls12_381` crate.
The reason is the current version v0.8.0 of `bls12_381` depends on an old
version v0.9 of the `digest` crate.

The obvious thing is to upgrade in `bls12_381` the version of `digest` to
v0.10.
That would make it possible to get `hkdf` v0.12 built.
But such an upgrade is and open issue at
zkcrypto/bls12_381#102, which mentions that it's
blocked on zkcrypto/bls12_381#90.
That pull request is about updating do the hash-to-curve draft v16, currently
it's using v12.
We use that code path in `bls-signatures`, else we wouldn't enable the
`experimental` feature of `bls12_381`.
So it's even not clear if we'd want such a change to v16.
rjan90 pushed a commit to filecoin-project/ref-fvm that referenced this pull request Jun 12, 2024
@vmx @rjan90
fix: remove the pairing feature from fvm_shared (#2009)
cdadf15 
The `pairing` feature from the `fvm_shared` crate isn't used. It causes
problems, as it forces the `subtle` dependency to v2.4.1, although the
rest is happy to have v2.5.0.

Here is a detailed dependency graph and issue outline:

`fvm_shared` depends on `bls-signatures`.
In `bls-signatures` we depend on an old version (v0.11) of `hkdf`.
That version depends on `hmac` v0.11, which depends on `crypto-mac` v0.11.
`crypto-mac` v0.11.0 depends on `subtle` v2. That is fine, it would
automatically select v2.5.0.
The problem is that `crypto-mac` v0.11.1 pins `subtle` to exactly v2.4,
therefore v2.5.0 won't be selected.

The obvious thing is to upgrade in`bls-signatures` the version of `hkdf`
to the latest v0.12.
That would make it possible to use `subtle` v2.5.0.
The problem is that such an upgrade is not easily possible.
`hkdf` v0.12 depends on a newer version v0.10 of the `sha2` crate.
Updating that breaks the `bls12_381` crate.
The reason is the current version v0.8.0 of `bls12_381` depends on an old
version v0.9 of the `digest` crate.

The obvious thing is to upgrade in `bls12_381` the version of `digest` to
v0.10.
That would make it possible to get `hkdf` v0.12 built.
But such an upgrade is and open issue at
zkcrypto/bls12_381#102, which mentions that it's
blocked on zkcrypto/bls12_381#90.
That pull request is about updating do the hash-to-curve draft v16, currently
it's using v12.
We use that code path in `bls-signatures`, else we wouldn't enable the
`experimental` feature of `bls12_381`.
So it's even not clear if we'd want such a change to v16.
Stebalien pushed a commit to filecoin-project/ref-fvm that referenced this pull request Jun 13, 2024
@rjan90 @vmx
chore: release fvm v3.10.0 (#2016)
3c5da7e 
* Update `filecoin-proofs-api` to v18

Update `filecoin-proofs-api` to v18

* Bump to 3.10.0

Bump to 3.10.0

* Update cargo.lock and changelog

Update cargo.lock and changelog

* fix: remove the pairing feature from fvm_shared (#2009)

The `pairing` feature from the `fvm_shared` crate isn't used. It causes
problems, as it forces the `subtle` dependency to v2.4.1, although the
rest is happy to have v2.5.0.

Here is a detailed dependency graph and issue outline:

`fvm_shared` depends on `bls-signatures`.
In `bls-signatures` we depend on an old version (v0.11) of `hkdf`.
That version depends on `hmac` v0.11, which depends on `crypto-mac` v0.11.
`crypto-mac` v0.11.0 depends on `subtle` v2. That is fine, it would
automatically select v2.5.0.
The problem is that `crypto-mac` v0.11.1 pins `subtle` to exactly v2.4,
therefore v2.5.0 won't be selected.

The obvious thing is to upgrade in`bls-signatures` the version of `hkdf`
to the latest v0.12.
That would make it possible to use `subtle` v2.5.0.
The problem is that such an upgrade is not easily possible.
`hkdf` v0.12 depends on a newer version v0.10 of the `sha2` crate.
Updating that breaks the `bls12_381` crate.
The reason is the current version v0.8.0 of `bls12_381` depends on an old
version v0.9 of the `digest` crate.

The obvious thing is to upgrade in `bls12_381` the version of `digest` to
v0.10.
That would make it possible to get `hkdf` v0.12 built.
But such an upgrade is and open issue at
zkcrypto/bls12_381#102, which mentions that it's
blocked on zkcrypto/bls12_381#90.
That pull request is about updating do the hash-to-curve draft v16, currently
it's using v12.
We use that code path in `bls-signatures`, else we wouldn't enable the
`experimental` feature of `bls12_381`.
So it's even not clear if we'd want such a change to v16.

* Update Changelog.md with #2009 backport

Update Changelog.md with #2009 backport

* Update fvm_shared to 3.10.0

Update fvm_shared to 3.10.0 in `testing/integration/Cargo.toml`

* Update fvm_shared to v3.10.0

Update fvm_shared to v3.10.0

* Update cargo.lock

Update cargo.lock by running `cargo check --all`

* Update `shared/CHANGELOG.md`

Update `shared/CHANGELOG.md`

---------

Co-authored-by: Volker Mische <volker.mische@gmail.com>
Stebalien pushed a commit to filecoin-project/ref-fvm that referenced this pull request Jun 13, 2024
@rjan90 @vmx
chore: release fvm v2.8.0 (#2017)
cefb3d1 
* Update filecoin-proofs-api to v18

Update filecoin-proofs-api to v18

* Bump to 2.8.0

Bump to 2.8.0

* Update cargo.lock and changelog

Update cargo.lock and changelog

* fix: remove the pairing feature from fvm_shared (#2009)

The `pairing` feature from the `fvm_shared` crate isn't used. It causes
problems, as it forces the `subtle` dependency to v2.4.1, although the
rest is happy to have v2.5.0.

Here is a detailed dependency graph and issue outline:

`fvm_shared` depends on `bls-signatures`.
In `bls-signatures` we depend on an old version (v0.11) of `hkdf`.
That version depends on `hmac` v0.11, which depends on `crypto-mac` v0.11.
`crypto-mac` v0.11.0 depends on `subtle` v2. That is fine, it would
automatically select v2.5.0.
The problem is that `crypto-mac` v0.11.1 pins `subtle` to exactly v2.4,
therefore v2.5.0 won't be selected.

The obvious thing is to upgrade in`bls-signatures` the version of `hkdf`
to the latest v0.12.
That would make it possible to use `subtle` v2.5.0.
The problem is that such an upgrade is not easily possible.
`hkdf` v0.12 depends on a newer version v0.10 of the `sha2` crate.
Updating that breaks the `bls12_381` crate.
The reason is the current version v0.8.0 of `bls12_381` depends on an old
version v0.9 of the `digest` crate.

The obvious thing is to upgrade in `bls12_381` the version of `digest` to
v0.10.
That would make it possible to get `hkdf` v0.12 built.
But such an upgrade is and open issue at
zkcrypto/bls12_381#102, which mentions that it's
blocked on zkcrypto/bls12_381#90.
That pull request is about updating do the hash-to-curve draft v16, currently
it's using v12.
We use that code path in `bls-signatures`, else we wouldn't enable the
`experimental` feature of `bls12_381`.
So it's even not clear if we'd want such a change to v16.

* Update cargo.lock and Changelog.md

Update cargo.locl and Changelog.md

* Update fvm_shared, cargo.lock and changelog

Update fvm_shared, cargo.lock and changelog

---------

Co-authored-by: Volker Mische <volker.mische@gmail.com>
@str4d
Copy link
Member

str4d commented Jul 21, 2024

The PR has not been split yet by the author, so I am going to take over this PR and do so myself. I'll do the move in a separate PR, and then rebase this PR on top of that.

str4d added a commit that referenced this pull request Jul 21, 2024
@str4d
Move hash-to-curve tests to tests/ for compilation performance
5ffba28 
Extracted from #90.
@str4d str4d mentioned this pull request Jul 21, 2024
Merged
str4d added a commit that referenced this pull request Jul 21, 2024
@str4d
Move hash-to-curve tests to tests/ for compilation performance
6d98c36 
Extracted from #90.
@str4d
Copy link
Member

str4d commented Jul 21, 2024

Rebased on main to remove the tests/ move from this PR (and gain access to updated CI).

@str4d
Copy link
Member

str4d commented Jul 21, 2024

Force-pushed to fix rustfmt issue.

@str4d
Copy link
Member

str4d commented Jul 21, 2024

Force-pushed to fix benchmarks that were broken by the second commit.

str4d
str4d requested changes Jul 21, 2024
View reviewed changes
Copy link
Member

@str4d str4d left a comment

Choose a reason for hiding this comment

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

Reviewed 7c6f4bc, except for changes to the tests.

src/hash_to_curve/expand_msg.rs Outdated Show resolved Hide resolved
src/hash_to_curve/expand_msg.rs Show resolved Hide resolved
src/hash_to_curve/expand_msg.rs Show resolved Hide resolved
src/hash_to_curve/expand_msg.rs Show resolved Hide resolved
tests/hash_to_curve_g1.rs Outdated Show resolved Hide resolved
@andrewwhitehead @str4d
update expand_message API
4b4e7f8 
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
@andrewwhitehead @str4d
only implement Message for iterators
db71a01 
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
@str4d
Copy link
Member

str4d commented Jul 21, 2024

Force-pushed to unswap the test orders, and unwrap test vectors.

str4d
str4d requested changes Jul 21, 2024
View reviewed changes
Copy link
Member

@str4d str4d left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Reviewed db71a01. I have confirmed that the only functional changes in the draft update (that affect this PR) are additional assertions.

Just so others looking at this repo have a sense of time, this PR took me over half of my Sunday to update and review.

str4d added 4 commits July 21, 2024 21:26
@str4d
Document L generic of ExpandMessage::init_expand
eb3f0a9
@str4d
Add missing len_in_bytes check to ExpandMsgXmd::init_expand
cf264e6
@str4d
Remove pub qualifiers from non-pub ExpandMsgDst
9a81247
@str4d
Require ExpandMsgXmd<H> has H::OutputSize < 256 instead of trunca…
09466e0 
…ting

The draft (and the final RFC) do not explicitly give an upper limit on
the hash output size, but section 5.3.3 says that `expand_message_xmd`
requires that DST is at most 255 bytes, and when given a longer domain
separation tag, to use the output of `H` directly as the DST. To avoid
any ambiguities, we bound the set of `H` we support.
str4d
str4d approved these changes Jul 21, 2024
View reviewed changes
Copy link
Member

@str4d str4d left a comment •
edited
Loading

Choose a reason for hiding this comment

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

ACK 09466e0. I'm going to open a follow-up PR that migrates to the now-released RFC 9380 and removes the experimental feature flag.

@str4d str4d merged commit 2874b5a into zkcrypto:main Jul 21, 2024
12 of 13 checks passed
@str4d str4d mentioned this pull request Jul 21, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daira daira daira left review comments

@str4d str4d str4d approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrewwhitehead @str4d @daira