public inbox for bitcoindev@googlegroups.com
 help / color / mirror / Atom feed
From: Cory Fields <lists@coryfields.com>
To: Tamas Blummer <tamas@bitsofproof.com>
Cc: bitcoin-dev@lists.linuxfoundation.org
Subject: Re: [bitcoin-dev] libconsensus assertion fails if used in multiple threads
Date: Tue, 18 Aug 2015 13:50:23 -0400	[thread overview]
Message-ID: <CAApLimhj8yMcfEFBWBHjos6eqC8m2=brk6j-o=L2og5BmPp59g@mail.gmail.com> (raw)
In-Reply-To: <CAApLimhbZj44OE0HStooCtzW33=jba=eEjf89H=0uJATxAvu5g@mail.gmail.com>

Pull request submitted: https://github.com/bitcoin/bitcoin/pull/6571

Regards,
Cory

On Tue, Aug 18, 2015 at 1:25 PM, Cory Fields <lists@coryfields.com> wrote:
> See responses inline.
>
> On Tue, Aug 18, 2015 at 6:31 AM, Tamas Blummer <tamas@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


  reply	other threads:[~2015-08-18 17:50 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-14  9:59 [bitcoin-dev] Adjusted difficulty depending on relative blocksize Jakob Rönnbäck
2015-08-14 13:32 ` Angel Leon
2015-08-14 14:19   ` Jakob Rönnbäck
2015-08-14 16:37     ` [bitcoin-dev] libconsensus assertion fails if used in multiple threads Tamas Blummer
2015-08-14 21:10       ` Cory Fields
2015-08-18  5:03         ` Cory Fields
2015-08-18 10:31           ` Tamas Blummer
2015-08-18 17:25             ` Cory Fields
2015-08-18 17:50               ` Cory Fields [this message]
2015-08-18 21:40             ` Eric Voskuil
2015-08-14 14:20 ` [bitcoin-dev] Adjusted difficulty depending on relative blocksize Anthony Towns
     [not found]   ` <A6B32C22-4006-434E-9B89-D7C99B5743A8@me.com>
2015-08-14 14:48     ` Jakob Rönnbäck
2015-08-14 15:00       ` Anthony Towns
2015-08-14 15:03       ` Adam Back
2015-08-14 15:14         ` Jakob Rönnbäck
2015-09-09  3:27           ` Tom Harding
2015-09-09 18:59             ` Warren Togami Jr.
2015-09-09 19:53               ` Tom Harding
2015-08-14 22:12         ` Tom Harding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAApLimhj8yMcfEFBWBHjos6eqC8m2=brk6j-o=L2og5BmPp59g@mail.gmail.com' \
    --to=lists@coryfields.com \
    --cc=bitcoin-dev@lists.linuxfoundation.org \
    --cc=tamas@bitsofproof.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox