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

[MPC] Phase2 MPC #73

Merged
merged 46 commits into from
Oct 24, 2019
Merged

[MPC] Phase2 MPC #73

merged 46 commits into from
Oct 24, 2019

Conversation

dtebbs
Copy link
Contributor

@dtebbs dtebbs commented Sep 10, 2019

Functions and cli utils to perform the phase2 MPC.
Some things that are still WIP / need discussion:

  • sources of randomness
  • read/write compressed points
  • hashing to G2
  • robust file operations

(depends on #68)

@dtebbs dtebbs force-pushed the mpc-keypair-file branch 2 times, most recently from 53aceb9 to ecf0bda Compare September 11, 2019 09:48
@dtebbs dtebbs changed the title Mpc phase2 WIP: Mpc phase2 Sep 11, 2019
@dtebbs dtebbs force-pushed the mpc-phase2 branch 3 times, most recently from e344171 to 89817d2 Compare September 11, 2019 17:39
@dtebbs dtebbs changed the title WIP: Mpc phase2 WIP: [MPC] Phase2 MPC (depends on #68) Sep 11, 2019
@dtebbs dtebbs force-pushed the mpc-phase2 branch 11 times, most recently from 98c0df8 to 7fe08a6 Compare September 13, 2019 10:10
@dtebbs dtebbs added CRS/SRS MPC Task related to the Multi-Party Computation protocol/code labels Sep 13, 2019
@dtebbs dtebbs force-pushed the mpc-keypair-file branch 2 times, most recently from b01d4db to ffa9c39 Compare September 16, 2019 17:15
@dtebbs dtebbs force-pushed the mpc-phase2 branch 3 times, most recently from 80ddcec to fe1d473 Compare September 18, 2019 12:09
@dtebbs dtebbs changed the title WIP: [MPC] Phase2 MPC (depends on #68) [MPC] Phase2 MPC (depends on #68) Sep 19, 2019
@dtebbs dtebbs changed the base branch from mpc-keypair-file to mpc September 19, 2019 08:11
src/mpc/cli/mpc_common.cpp Outdated Show resolved Hide resolved
src/snarks/groth16/mpc/phase2.cpp Show resolved Hide resolved
src/test/powersoftau_test.cpp Outdated Show resolved Hide resolved
src/snarks/groth16/mpc/hash_utils.hpp Outdated Show resolved Hide resolved
src/snarks/groth16/mpc/hash_utils.hpp Outdated Show resolved Hide resolved
src/snarks/groth16/mpc/hash_utils.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@AntoineRondelet AntoineRondelet left a comment

Choose a reason for hiding this comment

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

Please add some comments to clarify some of the choices you took in the chacha files, or refer to the corresponding sections of the implementation you followed

src/snarks/groth16/mpc/chacha_rng.hpp Outdated Show resolved Hide resolved
src/snarks/groth16/mpc/chacha_rng.hpp Outdated Show resolved Hide resolved
src/snarks/groth16/mpc/chacha_rng.cpp Outdated Show resolved Hide resolved
src/snarks/groth16/mpc/chacha_rng.cpp Show resolved Hide resolved
@dtebbs
Copy link
Contributor Author

dtebbs commented Oct 24, 2019

template<>

Let's either add a comment if necessary rather than using an empty template. Templatized code aims to be written in a tcc file anyway.

(@AntoineRondelet for some reason, github is not letting me reply directly, so I've had to move this to a new comment).

I've added some comments to explain these functions and why they are here. Just in case it's not clear, this code will only work for the case where ppT == libff::alt_bn128_pp, and the empty template here is to specify that this is a concrete function for the case ppT == libff::alt_bn128_pp, rather than generic templated code.

Since this is a concrete function, if it was in a .tcc file it would likely cause link errors due to the symbol being defined multiple times. If we turned it into fully generic code with template<typename ppT> then it would not compile for any other ppT, and it would make it difficult to provide implementations for other curves in the future.

Let me know if the comments are not clear, or you think we should take a different approach.

@AntoineRondelet
Copy link
Contributor

AntoineRondelet commented Oct 24, 2019

template<>

Let's either add a comment if necessary rather than using an empty template. Templatized code aims to be written in a tcc file anyway.

(@AntoineRondelet for some reason, github is not letting me reply directly, so I've had to move this to a new comment).

I've added some comments to explain these functions and why they are here. Just in case it's not clear, this code will only work for the case where ppT == libff::alt_bn128_pp, and the empty template here is to specify that this is a concrete function for the case ppT == libff::alt_bn128_pp, rather than generic templated code.

Since this is a concrete function, if it was in a .tcc file it would likely cause link errors due to the symbol being defined multiple times. If we turned it into fully generic code with template<typename ppT> then it would not compile for any other ppT, and it would make it difficult to provide implementations for other curves in the future.

Let me know if the comments are not clear, or you think we should take a different approach.

Yes. What I was saying was basically: if this is not generic code, then let's remove the template<> line, and add a comment saying "this code will only work for the case where ppT == libff::alt_bn128_pp"

[EDIT] I thought the compiler would not complain if we removed template<> as the type was specified, but apparently it does, so let's keep the template<> here then

@AntoineRondelet
Copy link
Contributor

LGTM

@AntoineRondelet AntoineRondelet merged commit 2f8673b into mpc Oct 24, 2019
@AntoineRondelet AntoineRondelet deleted the mpc-phase2 branch November 13, 2019 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CRS/SRS MPC Task related to the Multi-Party Computation protocol/code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants