Support other curves in mpc #223
Conversation
| @@ -8,7 +8,7 @@ | |||
| #include "libzeth/snarks/groth16/groth16_snark.hpp" | |||
|
|
|||
| #include <istream> | |||
| #include <libff/algebra/curves/alt_bn128/alt_bn128_pp.hpp> | |||
| // #include <libff/algebra/curves/alt_bn128/alt_bn128_pp.hpp> | |||
There was a problem hiding this comment.
Shall this comment be removed?
|
|
||
| using ppT = libzeth::ppT; | ||
| // Test data is specifically for alt_bn128 | ||
|
|
| @@ -71,33 +71,38 @@ srs_mpc_phase2_accumulator<ppT> dummy_initial_accumulator( | |||
|
|
|||
| TEST(MPCTests, HashToG2) | |||
| { | |||
| // Data here is specialised for alt_bn128. Call init_public_params on it, | |||
| // just in case `ppT` is set to a different curve. | |||
| using pp = libff::alt_bn128_pp; | |||
There was a problem hiding this comment.
Why not use ppT which is defined as using ppT = libff::alt_bn128_pp; at the top of the file? I mean, you called alt_bn_128::init_params() in the entry point exactly for the reason you mention in your comment, i.e. in case ppT is set to another curve, no?
As such, if this test only works for alt_bn128, I'd remove pp all together, and use alt_bn_128 here, and rely on the init_params done in main. This is confusing otherwise since we have ppT, pp and alt_bn128_pp in this test file..
There was a problem hiding this comment.
Thanks. ppT is supposed to represent the pairing used to run the tests, except those that are specialized for a specific pairing. So ppT is the default (eventually it would be nice to parameterize all these tests by ppT so we can run over all pairings - there should be no reason to restrict to the default - but I didn't plan to address that here), the second call to init_params in main is just in case ppT is not set to alt_bn_128.
The pp is kind of independent - it's just local to the function for convenience. If we expand everything to libff::alt_bn128_pp everywhere its many extra lines of code for no extra clarity.
Hope it's clear in the new version.
| // Data here is specialised for alt_bn128. Call init_public_params on it, | ||
| // just in case `ppT` is set to a different curve. | ||
| using pp = libff::alt_bn128_pp; | ||
| pp::init_public_params(); |
There was a problem hiding this comment.
This is not necessary as you already init the params of ppT and of alt_bn_128 in the entry point
There was a problem hiding this comment.
(covered by the above)
dtebbs
force-pushed
the
mpc-other-curves
branch
2 times, most recently
from
5465cd3
to
3ab5170
Compare
dtebbs
changed the title
| mpc_hash_t hash; | ||
| const std::string seed = hex_to_bytes( | ||
| "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" | ||
| "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"); | ||
| memcpy(hash, seed.data(), sizeof(mpc_hash_t)); | ||
|
|
||
| Fr expect_fr; | ||
| libff::alt_bn128_Fr expect_fr; |
There was a problem hiding this comment.
Why not use libff::Fr<pp>?
Support for other curves in groth16 mpc.