HarryR / ethsnarks

A toolkit for viable zk-SNARKS on Ethereum, Web, Mobile and Desktop
GNU Lesser General Public License v3.0
240 stars 57 forks source link

fixed_base_mul_zcash fixes #86

Closed fleupold closed 5 years ago

fleupold commented 5 years ago

The function was not working correctly when a) The scalar span more than two base points (>372 bits) b) The scalar span one triplet beyond any number of basepoint (e.g. 62*3 + 3 = 189 bits)

This change fixes both issues and adds unit test for b). It makes the existing unit test for a) pass.

The problem for a) was an off by one error in the for loop connecting MontgomeryAdders to the PointConverters (as of the second segment we were converting the wrong MG adder). We were also creating one too many MGAdder (the 63rd triplet was added to the sum of 0..62). The result was not used, however we created unnecessary constraints for it. The probelm for b) was that the 0th window in each segment relies to be added to the first (i % CHUNK_SIZE == 1 case). However, what if there is no next window? Then we don't connect the 0th window to anything and just ignore it. Therefore PH with 189 bits would effectively ignore the top 3 bits.

In general, we should probably create a testsuite with multiple examples for all scalar length up to a certain limit (maybe 768) to make sure we have covered all edge cases. ZCash does this.