Cory Fields [ARCHIVE] on Nostr: đź“… Original date posted:2015-08-18 đź“ť Original message:See responses inline. On ...
đź“… Original date posted:2015-08-18
đź“ť Original message:See responses inline.
On Tue, Aug 18, 2015 at 6:31 AM, Tamas Blummer <tamas at 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
đź“ť Original message:See responses inline.
On Tue, Aug 18, 2015 at 6:31 AM, Tamas Blummer <tamas at 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