search

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

Pre-release sanity checks for C++ code #49

Open HarryR opened 5 years ago

HarryR commented 5 years ago

It would be good to catch obscure errors ahead of major releases which may not be caught by unit tests. I suggest an additional set of tests, which run all of the C++ unit tests, under the following set of tools:

memcheck has an option to create a page per memory allocation, which while expensive to check can also accurately detect bounds overflows.

I also added the cxx-lint target to the Makefile which uses cppcheck to find common errors in the source code.

Other options available are:

I think that it's worth running the whole project through this set of tools at least twice, once as an interim mid-way through development, and once before major releases, so that we have as many opportunities as possible to catch and fix subtle or previously unrealised bugs.

However, none of these tools can help without thorough tests which verify the critical paths and their edge cases.

drewstone commented 5 years ago

Perhaps along different lines, would this be useful? https://www.bazel.build/

HarryR commented 5 years ago

I'm not sure if Bazel would be useful, CMake does a good job and I've never used Bazel before.

However, last time I said that I was using automake/autotools and was like 'lol CMake...' So maybe I should give it a try?

What I'm really trying to get at is, if you look at the ZCash audit by NCC Group - aside from the insight from an experienced auditor - they run a number of tools to automatically detect potential faults and undefined behaviour - the aim of this ticket is to, in the event of paying for an audit, to have 99% of the low hanging fruit dealt with already - so anything that comes back from an audit is more likely to be signal than noise.