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

C++ cleanups, field-only merkle tree, LongsightF removal #44

Closed HarryR closed 5 years ago

HarryR commented 5 years ago

This makes the following changes:

The proving key for the Miximus circuit is now down to ~550kb and runs very quickly, and components are tested in both Python and Solidity. However the following items remain:

HarryR commented 5 years ago

Interesting warnings from clang build:

/Users/travis/build/HarryR/ethsnarks/src/mod/miximus.cpp:77:15:warning:moving a temporary object prevents copy elision [-Wpessimizing-move]
        m_IVs(std::move(merkle_tree_IVs(in_pb)))
              ^
/Users/travis/build/HarryR/ethsnarks/src/mod/miximus.cpp:77:15:note:remove std::move call here
        m_IVs(std::move(merkle_tree_IVs(in_pb)))
              ^~~~~~~~~~                      ~
1 warning generated.

I had problems with the Miximus circuit during proving where it would access an invalid variable (e.g. offset in the hundreds of trillions range). I think I 'fixed' it using std::move and an rvalue.

However, another problem is that I turned some compiler warnings off in the GCC build which made the compilation output cleaner (all the unused variable warnings from libff).

But, the good news is the Python & C++ implementation of miximus work together well, so just need to cleanup these last warnings.

However, now the Mac build is erroring with the following message in the middle of the tests, without indicating which test it was running:

make: *** [cxx-tests] Abort trap: 6

I will have to double-check the Mac build locally.