Loopring / protocols

A zkRollup DEX & Payment Protocol
https://loopring.org
332 stars 122 forks source link

[Protocol3] Test the optimized libsnark implementation from SECBIT #291

Closed kongliangzhong closed 5 years ago

kongliangzhong commented 5 years ago

SECBIC optimized libsnark with a new paring library, we can try to use their implementation and measure there is any performance boost (~20% expected)

Brechtpd commented 5 years ago

I don't think we can use this (yet). It seems like we need to use MCL_BN128 for the better performance, but this curve setup doesn't look the same as ALT_BN128 (different parameters/calculations). We need all parameters (not just the curve parameters a and b) to be exactly the same as described in EIP-196 and EIP-197, otherwise we can't use the precompile (if we can't use the precompile the gas cost is too high to verify the proof). For example, we can't use BN128 because it differs from ALT_BN128 by just a generator.

However, maybe some optimizations can also be done on ALT_BN128, I don't know, but it doesn't look like we can just switch to MCL_BN128 as it is.

I have some problems compiling it with ethsnarks when using MCL_BN128 so I'm not sure of this yet, but I'd like to hear the thoughts of SECBIT on this.

With Istanbul we may be able to use any curve (still very uncertain, and maybe just some extra curves), but if we can use any curve we'll probably want to switch to a curve that allows for recursive SNARKs (MNT).

huyuguang commented 5 years ago

I don't think we can use this (yet). It seems like we need to use MCL_BN128 for the better performance, but this curve setup doesn't look the same as ALT_BN128 (different parameters/calculations). We need all parameters (not just the curve parameters a and b) to be exactly the same as described in EIP-196 and EIP-197, otherwise we can't use the precompile (if we can't use the precompile the gas cost is too high to verify the proof). For example, we can't use BN128 because it differs from ALT_BN128 by just a generator.

However, maybe some optimizations can also be done on ALT_BN128, I don't know, but it doesn't look like we can just switch to MCL_BN128 as it is.

I have some problems compiling it with ethsnarks when using MCL_BN128 so I'm not sure of this yet, but I'd like to hear the thoughts of SECBIT on this.

With Istanbul we may be able to use any curve (still very uncertain, and maybe just some extra curves), but if we can use any curve we'll probably want to switch to a curve that allows for recursive SNARKs (MNT).

It because of the mcl_bn128 use the curve param same with bn128 but not alt_bn128. The curve is the same but some parameters are different.

    alt_bn128_G2::G2_one() = alt_bn128_G2(alt_bn128_Fq2(alt_bn128_Fq("10857046999023057135944570762232829481370756359578518086990519993285655852781"),
                                                alt_bn128_Fq("11559732032986387107991004021392285783925812861821192530917403151452391805634")),
                                    alt_bn128_Fq2(alt_bn128_Fq("8495653923123431417604973247489272438418190587263600148770280649306958101930"),
                                                alt_bn128_Fq("4082367875863433681332203403145435568316851327593401208105741076214120093531")),
                                    alt_bn128_Fq2::one());
mcl_bn128_G2::G2_one().pt.x = Fp2(Fp("15267802884793550383558706039165621050290089775961208824303765753922461897946"),
                                        Fp("9034493566019742339402378670461897774509967669562610788113215988055021632533"));
    mcl_bn128_G2::G2_one().pt.y = Fp2(Fp("644888581738283025171396578091639672120333224302184904896215738366765861164"),
                                        Fp("20532875081203448695448744255224543661959516361327385779878476709582931298750"));
    mcl_bn128_G2::G2_one().pt.z = Fp2(Fp(1), Fp(0));

Let me add a compile switch.

huyuguang commented 5 years ago

Please check this branch: https://github.com/huyuguang/libsnark/tree/ae132066657cccec5883059fc31296cbcaf158e3

Now the generator of the mcl_bn128 G2 is the same as alt_bn128 (the original value is the same as bn128).

2019/07/25 Add a compile switch to support compabible with ethsnark. Check the mcl_bn128_init.cpp, macro MCL_COMPATIBLE_BN128. (The original mcl_bn128 used bn128 curve parameters, and the ethsnark uses alt_bn128. Most of the parameters are same, the only difference item is the generator of the G2. )

Note: since the ethsnark use uint256 to store a Fr and two Fr to store a G1, so not use Montgomery and point compression to make.

Use the following cmd instead: cmake -DCMAKE_INSTALL_PREFIX=../../install -DMULTICORE=ON -DUSE_PT_COMPRESSION=OFF -DMONTGOMERY_OUTPUT=OFF -DBINARY_OUTPUT=OFF -DWITH_PROCPS=OFF -DWITH_SUPERCOP=OFF -DCURVE=MCL_BN128 -DUSE_ASM=ON ..

The proof format will be human-readable and easy to convert to the format which matches the ethsnark requirement.

The following tests pass: 1, Generate vk&pk through mcl_bn128 and load through alt_bn128. 2, Generate proof throught mcl_bn128 and verify through alt_bn128. 3, For the testing circuit, the prove time relation between alt_bn128:bn128:mcl_bn128 is 30:18:14.

Brechtpd commented 5 years ago

Thank you @huyuguang! Will try to get it up and running soon.

dong77 commented 5 years ago

I changed my mind not to recommend using this library as it is not proactively maintained as a priority of SECLIB.