See responses inline. On Tue, Aug 18, 2015 at 6:31 AM, Tamas Blummer <ta...@bitsofproof.com> wrote: > Thanks a lot Cory for following through the test case and producing a patch. > > I confirm that libconsensus is now running stable within the Bits of Proof > stack, > in-line with test cases we use to verify the java implementation of the > script engine, > that are BTW borrowed from Bitcoin Core. > > The performance of libconsensus is surprisingly close to the java one. > Validating a 2-of-2 a multi-sig transaction runs at 1021 ops/sec with java > and 1135 ops/sec > in libconsensus. This is on a 2.2GH i7 laptop (4 hyper threading cores used > by 8 threads). > Another nice demonstration why one should not trade in advances > of languages for the last decades for a marginal gain of performance with > C/C++, > I assume thereby that Bouncy Castle’ EC lib s not superior to OpenSSL's.
A few points there. First, Core is switching to libsecp256k1 for several reasons, and one of them is speed. I seem to recall it being up to 8x faster than OpenSSL. Also, it can depend heavily on compiler switches and optimization levels. For example, In playing with my test-case for hitting the OpenSSL race issue, I managed to get a ~100% speedup by simply using -O3 and lto. > > I disagree that the problem was rare in the real-world, it should affect any > modern > implementation that validates transactions parallel in multiple threads. > Well I'd say you're a bit biased in this case ;) It's only those using ancient (0.98 or 1.00) versions of OpenSSL who are affected, or those with OPENSSL_BN_ASM_MONT support disabled or missing. Note that official releases of libbitcoinconsensus are compiled against a much newer version and shouldn't have any issues. The earlier patches for locking callbacks should be unnecessary. > Aborting also does not make the problem less severe in my opinion. Well it's not a good thing by any means, but it's certainly better than incorrect results! In any undefined/error condition for the consensus library, aborting is the right thing to do. If we can't explain how we've reached a certain "unreachable" condition as is the case here, the only reasonable recourse is to shut down. Otherwise we risk network forks, DOS, etc. > Therefore hope the pull will be included into Core with next release. > It will likely be unnecessary for the next release, but I do think it's worth backporting to the 0.10 and 0.9 series. > I can’t assign a timeline to “near future" secp256k1 integration. Can you? I believe the libsecp256k1 guys are generally happy with the lib these days, but I'll avoid guessing at a timeline. We can discuss that on the PR for this fix, which I'll do today. Regards, Cory _______________________________________________ bitcoin-dev mailing list bitcoin-dev@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
