Change most assert_except into assert. #11

ebfull · 2017-02-09T22:19:30Z

This PR changes most assert_except into assert in the interest of avoiding exceptions inside of OpenMP parallel regions.

The remaining:

src/algebra/curves/alt_bn128/alt_bn128_pairing.cpp
14:#include "common/assert_except.hpp"
348:    assert_except(Q1.Z == alt_bn128_Fq2::one());
350:    assert_except(Q2.Z == alt_bn128_Fq2::one());

src/algebra/fields/fp.tcc
18:#include "common/assert_except.hpp"
728:        assert_except(0);

src/algebra/fields/fp2.tcc
169:        assert_except(0);

src/common/assert_except.hpp
6:inline void assert_except(bool condition) {

The one in the pairing is there because libsnark's pairing function doesn't appear to work properly for zero inputs (scipr-lab#62) and we don't want a remote DoS (zcash/zcash#437), and the two in fp/fp2 are for sqrt which is not used at all inside of parallel code.

This PR also removes the evaluation domains that we don't use.

ebfull · 2017-02-09T22:20:40Z

src/algebra/fields/bigint.tcc

@@ -187,7 +186,7 @@ inline void bigint<n>::div_qr(bigint<n-d+1>& quotient, bigint<d>& remainder,
                              const bigint<n>& dividend, const bigint<d>& divisor)
 {
    static_assert(n >= d, "dividend must not be smaller than divisor for bigint::div_qr");
-    assert_except(divisor.data[d-1] != 0);
+    assert(divisor.data[d-1] != 0);


(The divisor for our purposes is always the modulus.)

radix42 · 2017-02-09T22:27:41Z

This looks good to me, I've previously tested a version of libsnark with all assert_except instances changed to assert, I'll track this change downstream once its merged

str4d · 2017-02-09T23:10:09Z

LGTM, although I can only check that the assert_except -> assert was done correctly.

This PR also removes the evaluation domains that we don't use.

I'd personally prefer that this was split into a separate commit.

daira

ACK

daira · 2017-02-09T23:28:38Z

src/algebra/curves/alt_bn128/alt_bn128_g1.cpp

@@ -257,7 +256,7 @@ alt_bn128_G1 alt_bn128_G1::add(const alt_bn128_G1 &other) const
 alt_bn128_G1 alt_bn128_G1::mixed_add(const alt_bn128_G1 &other) const
 {
 #ifdef DEBUG
-    assert_except(other.is_special());
+    assert(other.is_special());


We don't compile with DEBUG, I think (based on cursory inspection of depends/packages/libsnark.mk.) But why is this in an #ifdef DEBUG conditional anyway?

daira · 2017-02-09T23:32:41Z

src/algebra/fields/bigint.tcc

@@ -224,7 +223,7 @@ inline bool bigint<n>::operator>(const bigint<n>& other) const
 template<mp_size_t n>
 bigint<n>& bigint<n>::randomize()
 {
-    assert_except(GMP_NUMB_BITS == sizeof(mp_limb_t) * 8);
+    assert(GMP_NUMB_BITS == sizeof(mp_limb_t) * 8);


I think this could be a static assert. Does not block.

daira · 2017-02-10T00:26:13Z

Oops, I didn't notice @str4d's #11 (comment) before merging.

Update libsnark. This updates libsnark to adopt zcash/libsnark#11. Do not merge this until zcash/libsnark#11 is merged. Closes #2043

Change most assert_except into assert.

9ada3f8

ebfull commented Feb 9, 2017

View reviewed changes

ebfull mentioned this pull request Feb 9, 2017

Update libsnark. zcash/zcash#2080

Merged

daira approved these changes Feb 9, 2017

View reviewed changes

daira merged commit 9ada3f8 into master Feb 10, 2017

zkbot added a commit to zcash/zcash that referenced this pull request Feb 10, 2017

Auto merge of #2080 - ebfull:libsnark-update, r=bitcartel

44788f9

Update libsnark. This updates libsnark to adopt zcash/libsnark#11. Do not merge this until zcash/libsnark#11 is merged. Closes #2043

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change most assert_except into assert. #11

Change most assert_except into assert. #11

ebfull commented Feb 9, 2017

ebfull Feb 9, 2017

radix42 commented Feb 9, 2017

str4d commented Feb 9, 2017

daira left a comment

daira Feb 9, 2017

daira Feb 9, 2017

daira commented Feb 10, 2017

Change most assert_except into assert. #11

Change most assert_except into assert. #11

Conversation

ebfull commented Feb 9, 2017

ebfull Feb 9, 2017

Choose a reason for hiding this comment

radix42 commented Feb 9, 2017

str4d commented Feb 9, 2017

daira left a comment

Choose a reason for hiding this comment

daira Feb 9, 2017

Choose a reason for hiding this comment

daira Feb 9, 2017

Choose a reason for hiding this comment

daira commented Feb 10, 2017