Return-Path: <cory@atlastechnologiesinc.com>
Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org
	[172.17.192.35])
	by mail.linuxfoundation.org (Postfix) with ESMTPS id 546DD8A5
	for <bitcoin-dev@lists.linuxfoundation.org>;
	Tue, 18 Aug 2015 17:50:26 +0000 (UTC)
X-Greylist: whitelisted by SQLgrey-1.7.6
Received: from mail-la0-f43.google.com (mail-la0-f43.google.com
	[209.85.215.43])
	by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 7BB6A271
	for <bitcoin-dev@lists.linuxfoundation.org>;
	Tue, 18 Aug 2015 17:50:25 +0000 (UTC)
Received: by lagz9 with SMTP id z9so104433563lag.3
	for <bitcoin-dev@lists.linuxfoundation.org>;
	Tue, 18 Aug 2015 10:50:24 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
	d=1e100.net; s=20130820;
	h=x-gm-message-state:mime-version:in-reply-to:references:date
	:message-id:subject:from:to:cc:content-type
	:content-transfer-encoding;
	bh=/6u8JndlabGDXDN/piYMf4qE0uZVCCIMe+/TbzNM/Rk=;
	b=bKs1DjvOO+rYKn9lI+2ADsmQr1ocvNQMILCJ24+gFrr42iBsSovhezwz2qGNEWUOT5
	2NCLptnw4NmsMJA5sbbpzIznrcsJjM/zT8O2zCV0JpYDlKIQplI9wkIRc9GEmhbI8w0x
	APGuHCRv4rTgCYTbi4JTdIu/zpijFlF+JdTdiSITtde48JLRnqCf3XlEHHWCWF+jEEup
	2vAQFW/f0LOrc3u5pCswNIUfooSeM3TpJHdPuSztCBQsC8GID5LA7BaqbME1kL/PS6ze
	tS5For3fVqY2tyG/HHRHeJky1C5qfXPF186t+Bs/RA3Rkn3sDY+2AwAGXhIOcXNJDprW
	biOg==
X-Gm-Message-State: ALoCoQkGQL6HK0ZN4QDoD8KhqhWgDxPjBkiY8f7ytFg/tdGv7fMXUP2U7RbGcmcL3iEddagVAZL7
MIME-Version: 1.0
X-Received: by 10.112.129.167 with SMTP id nx7mr7315756lbb.27.1439920223881;
	Tue, 18 Aug 2015 10:50:23 -0700 (PDT)
Received: by 10.114.83.200 with HTTP; Tue, 18 Aug 2015 10:50:23 -0700 (PDT)
In-Reply-To: <CAApLimhbZj44OE0HStooCtzW33=jba=eEjf89H=0uJATxAvu5g@mail.gmail.com>
References: <09C8843E-8379-404D-8357-05BDB1F749C1@me.com>
	<CADZB0_YvvDDq4XzfvQeeWJ2oZxPukP0oXYSrEeC3gy9_Fk0ZuA@mail.gmail.com>
	<D018B1B0-B613-4C05-84BB-02CE6E2FEA3E@me.com>
	<499C1F46-5EB8-4846-86B6-0B3F2E02D972@bitsofproof.com>
	<CAApLimiQFaUPgQTLiCKvEb+PQ7pOGg-JvfMfYGzq_X7SfmWLHQ@mail.gmail.com>
	<CAApLimgAHB4Bgqp9WG1LiGegUPeKxuppTgizheN4-jwxiuAUag@mail.gmail.com>
	<68E206FF-4ABD-4547-BF73-8661A7C2F08B@bitsofproof.com>
	<CAApLimhbZj44OE0HStooCtzW33=jba=eEjf89H=0uJATxAvu5g@mail.gmail.com>
Date: Tue, 18 Aug 2015 13:50:23 -0400
Message-ID: <CAApLimhj8yMcfEFBWBHjos6eqC8m2=brk6j-o=L2og5BmPp59g@mail.gmail.com>
From: Cory Fields <lists@coryfields.com>
To: Tamas Blummer <tamas@bitsofproof.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW
	autolearn=ham version=3.3.1
X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on
	smtp1.linux-foundation.org
Cc: bitcoin-dev@lists.linuxfoundation.org
Subject: Re: [bitcoin-dev] libconsensus assertion fails if used in multiple
	threads
X-BeenThere: bitcoin-dev@lists.linuxfoundation.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: Bitcoin Development Discussion <bitcoin-dev.lists.linuxfoundation.org>
List-Unsubscribe: <https://lists.linuxfoundation.org/mailman/options/bitcoin-dev>,
	<mailto:bitcoin-dev-request@lists.linuxfoundation.org?subject=unsubscribe>
List-Archive: <http://lists.linuxfoundation.org/pipermail/bitcoin-dev/>
List-Post: <mailto:bitcoin-dev@lists.linuxfoundation.org>
List-Help: <mailto:bitcoin-dev-request@lists.linuxfoundation.org?subject=help>
List-Subscribe: <https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev>,
	<mailto:bitcoin-dev-request@lists.linuxfoundation.org?subject=subscribe>
X-List-Received-Date: Tue, 18 Aug 2015 17:50:26 -0000

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> wr=
ote:
>> Thanks a lot Cory for following through the test case and producing a pa=
tch.
>>
>> I confirm that libconsensus is now running stable within the Bits of Pro=
of
>> 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 j=
ava
>> and 1135 ops/sec
>> in libconsensus. This is on a 2.2GH i7 laptop (4 hyper threading cores u=
sed
>> 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 wit=
h
>> C/C++,
>> I assume thereby that Bouncy Castle=E2=80=99 EC lib s not superior to Op=
enSSL'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=E2=80=99t assign a timeline to =E2=80=9Cnear future" secp256k1 int=
egration. 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