Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Avoiding forks and workarounds for known fixes for zkcrypto libraries #53

Closed
dignifiedquire opened this issue Jan 16, 2019 · 25 comments
Closed

Comments

@dignifiedquire
Copy link
Contributor

Currently we rely on some functionality that is not in the released/merged repos of the zkcrypto libraries.

This is a tracking issue, such that we can work on getting these resolved and upstreamed if possible, to avoid maintaining a bunch of forks of these.

@dignifiedquire
Copy link
Contributor Author

cc @nicola @porcuquine

@sternhenri sternhenri transferred this issue from another repository Jan 21, 2019
@burdges
Copy link

burdges commented Mar 20, 2019

We've almost exactly the same concerns for polkadot. I'm currently using the branch https://github.com/burdges/pairing/tree/derives_borrows_etc0 which mostly adds some obvious functionality.

We need hashing-to-the curve that works with their newer crate layout, ala ff, group, etc. I started porting both my commits and the hashing-to-the-curve pull over to zcash's ff, group, etc. variant in https://github.com/burdges/pairing/tree/derives_borrows_etc0 but wondered off so now maybe outdated.

We should clearly focus on doing one fork that everyone likes because up-streaming changes sounds unlikely. That said, we might reduce headaches by doing parallel traits for hashing-to algorithms. I'm never quite settled on any particular design when I started thinking about it months ago. I do however think an external hashing-to trait setup might help address Sean's vague concerns about there being multiple ways to do hashing-to-the-curve.

I think @mmaker mentioned working on a new approach with some ENS student, so maybe he'll weigh in.

cc @demimarie-parity

@burdges
Copy link

burdges commented Mar 20, 2019

As for rand, I'm worried that cryptography folks dislike complexity and churn in their dependencies and especially random number dependencies, so heroic updates might take ages to up-stream.

Instead, we might bridge the rand 0.3/0.4 vs 0.6 fork by keeping the diffs small, even though that means ugly hacks like a local Rand trait.

That said, I think the ZCash Foundation recently hired Henry de Valance, whose dalek ecosystem has kept up with the rand crate, so maybe..

@burdges
Copy link

burdges commented Mar 20, 2019

Among the crates directly dependent on the pairing crate, I think only POA Network's threshold_crypto crate does not belong to ZCash, and it depends directly upon the ZCash version without hashing-to-the-curve: https://github.com/poanetwork/threshold_crypto/blob/master/Cargo.toml

Anyone know if any of the forks have created separate crates yet? https://github.com/zkcrypto/pairing/network/members

@dignifiedquire
Copy link
Contributor Author

fyi I recently ported the hashing to curve work onto the latest master of the pairing crate (post ff): https://github.com/filecoin-project/pairing/tree/hashing-master

@burdges
Copy link

burdges commented Mar 20, 2019

Very cool. I'll check out what else I want on your branch.

In the longer run, I suppose this comment roughly explains lots about the bls12_381 situation: zkcrypto/jubjub#19 (comment)

@nicola nicola mentioned this issue Mar 21, 2019
@burdges
Copy link

burdges commented Mar 27, 2019

You just directly ported the Fouque-Tibouchi implementation by @mmaker right? Any idea if you caught the commits that Sean Bowe added to https://github.com/mmaker/pairing/commits/master perhaps?

I'm tempted to fix zkcrypto/pairing#101 too but doing so makes merging hard.

@dignifiedquire
Copy link
Contributor Author

dignifiedquire commented Mar 27, 2019 via email

@dignifiedquire
Copy link
Contributor Author

I am happy to fix that issue on our fork, if you want to PR sth. I remember needing to work around the issue in the bls-signature impl.

@dignifiedquire
Copy link
Contributor Author

It seems that the current work is now happening on https://github.com/zcash/librustzcash

@burdges
Copy link

burdges commented Apr 15, 2019

Yup. I pushed a bls crate with fairly flexible aggregation at https://github.com/w3f/bls btw.

@burdges
Copy link

burdges commented Apr 17, 2019

I think zkcrypto/pairing#102 is the start of improving the hashing-to-the-curve

@burdges
Copy link

burdges commented Apr 23, 2019

Just noticed https://eprint.iacr.org/2019/403.pdf but the introuctiond makes it sounds much like Michele Orru's existing implementation.

@afck
Copy link

afck commented Apr 26, 2019

I know that at least the POA and MaidSafe projects are also very interested in collaborating on a pairing fork.

If you're open to it, I'd move my serde PR to filecoin-project/pairing, too.
It would also make sense to enable GitHub issues there. And I guess we'd eventually want to publish it (under a different name—ec-pairing?) on crates.io?

@burdges
Copy link

burdges commented Apr 26, 2019

Sure. We want to merge everything back into zcash's crates eventually, but a coherent fork likely simplifies this, and no reason to rush them while they're doing their big refactor.

As an aside, I think https://github.com/zkcrypto/bls12_381 is the start of making the zcash crates constant-time, so maybe Sean would accept pulls there if anyone has the urge to help, not sure.

@dignifiedquire
Copy link
Contributor Author

If you're open to it, I'd move my serde PR to filecoin-project/pairing, too.

Happy to take a look

but a coherent fork likely simplifies this, and no reason to rush them while they're doing their big refactor.

fully agreed

@afck
Copy link

afck commented Apr 30, 2019

I created a PR for serde: filecoin-project/paired#2

How do you all feel about operator overloading? (See zkcrypto/pairing#85.)

@burdges
Copy link

burdges commented Apr 30, 2019

I think Sean Bowe felt uncomfortable with operator overloading previously, but his experimental bls12-381 crate for constant-time code uses operator overloading for field arithmetic, so maybe his opinions changed. I'd favor being cautious here and sticking with whatever he does.

@str4d
Copy link

str4d commented May 1, 2019

Re: the rand crate update, I plan to do this in pairing (or rather, across all our crates via the librustzcash workspace) in the next month or so. I'm hoping we might even be able to reduce the dependency tree further down from the main rand crate to just what we need to target a CSPRNG or OsRng, but I'm not sure whether that's doable yet in 0.6, or whether we'd need to wait for 0.7 to do that.

More generally, you're welcome to make PRs on the librustzcash workspace if you want to propose changes while the refactor is ongoing, with the proviso that you may need to rebase your PRs to account for said refactor.

@burdges
Copy link

burdges commented Jun 22, 2019

There is an attempt at https://github.com/pairingwg/bls_standard/issues to standardize BLS signatures with the two most pertinent discussions being w3f/bls#5 and zkcrypto/pairing#56 (comment)

@daira
Copy link

daira commented Aug 5, 2019

@burdges wrote:

We should clearly focus on doing one fork that everyone likes because up-streaming changes sounds unlikely.

We'd be happy to take these kinds of changes (subject to our normal code review/quality requirements of course); the obstacle at the moment is that we're in the middle of a big refactoring, which has taken longer than we expected. After that, please do try to upstream your changes.

@daira
Copy link

daira commented Aug 5, 2019

Note that updating rand has already been done in our refactoring (zcash/librustzcash#91). Please feel free to take stuff from that PR in order to reduce the work needed for the eventual merge.

@burdges
Copy link

burdges commented Nov 17, 2019

I've noticed folks asking for faster G2 deserialization in several places recently: zkcrypto/pairing#102

@nicola
Copy link
Contributor

nicola commented May 5, 2020

any update on this?

@nicola nicola closed this as completed Jan 21, 2021
@str4d
Copy link

str4d commented May 21, 2021

We did complete our big refactor last year, and moved the trait crates back into their zkcrypto homes. I do want to pick this issue back up this year.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants