Extend Operators

[gxavier38]

I'm submitting a pull request with my work on PySNARK from the past few weeks.
I've mainly been extending operators, adding tests, and reformatting code according to PEP8.
I also implemented the Poseidon hash in PySNARK for my own use. Seems useful to have it in this repo.

There are some things I need help with. Firstly, I'm not entirely sure I have all the constraints implemented correctly. I might be missing a few, which might cause security issues.
Secondly, I added a CircleCI configuration file to for automatic testing in case it's useful for you. However, the qaptools and libsnark tests fail because I didn't set them up correctly. Since I haven't used those before, I don't exactly know how to set them up for CircleCI.

Sorry for the huge pull request. The changes were all interconnected so it didn't make sense to submit them individually.

Let me know which changes you'd like to have and which you don't.

Thanks!

Changes

Operators

• Implemented unimplemented operators
• Extended existing operators to work for more data types

For example:

• LinComb.__pow__ by LinComb,
• LinComb.__mod__ by integer that is not a power of 2,
• LinComb.__mod__ by LinComb,
• LinCombFxp.__divmod__, etc.

See tests/ folder for examples

Booleans

• Added LinCombBool for boolean values with logical operators as discussed in Fix some operators in pysnark.runtime #18
• Changed LinComb operators (e.g. __and__, __or__, __xor__) to be bitwise operations
• Made if_then_else use LinCombBool

Code Quality

• Added tests for LinComb, LinCombFxp, and LinCombBool
• Added CircleCI script

Extensions

• Added Poseidon hash gadget

To Do

• Some LinCombFxp operators still aren't implemented, e.g. __pow__ by a LinCombFxp
• Check constraints. Some constraints may be missing, which might affect security
• Fix CircleCI script for libsnark and qaptools
• Update examples to work with new changes

Resolves #23 and resolves #25

[meilof]

Would be nice if this would be somehow configurable

[gxavier38]

What would be a good way to handle it? Another global variable? An extra parameter?

What if we update each multiplicand with multiplicand.value = multiplicand.value % backend.get_modulus()? Then, the maximum exponent would be limited only by bitlength. That would make the output LinComb's value and __repr__ wrong, but would increase performance and shouldn't affect the actual circuit.

It might also be worth thinking about how to handle LinComb.value when it overflows. Should we leave the value as is or wrap around? If we were to wrap the value around (e.g. in __init__), it might easily solve these issues.

[meilof]

Yes, doing multiplicand.value = multiplicand.value % backend.get_modulus() definitely makes sense. Of course other.to_bits still requires a bit length, but I guess we can just use the default bit length and if the user wants to have a lower bit length (because of efficiency) he can just temporarily change it.

[meilof]

And indeed, it is a good point about the overflows. Originally the code used to just always reduce modulo backend.get_modulus(). But later my idea was to keep stuff completely independent from the modulus wherever possible. Because who knows, one day there may be backend that just works over the integers and doesn't use a modulus at all. In fact, the only place where get_modulus is now used is in the hash code.

Maybe instead of using backend.get_modulus(), the backends should instead support a backend.reduce(val) function that performs modular reduction...

[gxavier38]

Should I just reduce using backend.get_modulus() for now?

[meilof]

Yes, let's just keep it like that at least for pow. It doesn't make much sense to do a pow that has an overflow, so in practice it shouldn't matter.

[gxavier38]

I made the change, but oblivious pow now costs 7 * bitlength + 1 constraints. Is that acceptable?

[meilof]

Thanks a lot for your work! I will go through it over the next days but it may take some time..

[gxavier38]

Thanks for your help! There's no rush.
I'll respond to your comments as they come.

[meilof]

This should work. It will const bit_length constraints due to the right-shift (at least I assume (1<<resolution) is implemented with a right shift in runtime.py? otherwise it is more costly). The previous code did two assert_positive's of size equal to the resolution. So it seems that depending the resolution and the bit length either of the two could be more efficient.. There would also be differences in the sizes of the inputs that they would accept.. I would have to think a bit more about which version would be preferable in which cases.

[gxavier38]

Good point. LinComb.__rshift__ uses a division right now instead of to_bits. I'll have to change that and change the fixed point code to use >> (1 << resolution).

Could you explain the constraints you had before? Are they still required here?

diff = (1<<LinCombFxp.res)*ret.lc-self.lc*other.lc
(diff + (1<<LinCombFxp.res)).assert_positive(LinCombFxp.res+1)
((1<<LinCombFxp.res) - diff).assert_positive(LinCombFxp.res+1)

[gxavier38]

Actually, I did some digging and it turns out that LinCombFxp.__mul__ costs only 1 constraint, even with the division. I traced it to LinComb.__truediv__ costing 0 constraints to divide by a public integer, and (1 << resolution) being a public integer.

Could you check that LinComb.__truediv__ is correct?

[meilof]

LinComb.truediv as it is now implements proper division, meaning it only works if the division does not have a remainder. So you can divide 8 by 4 (and so do a right-shift by two), but not 9 because this division has remainder one. So you cannot use truediv to implement a right-shift, it should be floordiv. I think for a right-shift there is really no other way than doing a bit decomposition one way or the other

[gxavier38]

I see.
I'll implement it using floordiv for now. Using a direct rshift with bit decomposition is cheaper, but we easily run into trouble because of to_bits since the numbers are often larger than 2 ** bitlength.

[meilof]

I finally went through the pull request in full and merged it into master. Thanks a lot for your work! I also added your name to the acknowledgements section in the README, I hope that is OK?

I still found a few small things where I think efficiency could be improved, so hopefully I or you can still fix them in the future. And hopefully I can find time to do some sanity checks to see if everything does what we expect. For future reference, here's some notes I made:

• some of the LinCombBool operations can be implemented more efficiently
  since we know that it stores bits, e.g., a<=b <=> not(a=1 and b=0)
• implement __pow__ using square-and-multiply
• runtime __mod__, __floordiv__ with a power of two can be more efficient