Return-Path: Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 546DD8A5 for ; 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 ; Tue, 18 Aug 2015 17:50:25 +0000 (UTC) Received: by lagz9 with SMTP id z9so104433563lag.3 for ; 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: References: <09C8843E-8379-404D-8357-05BDB1F749C1@me.com> <499C1F46-5EB8-4846-86B6-0B3F2E02D972@bitsofproof.com> <68E206FF-4ABD-4547-BF73-8661A7C2F08B@bitsofproof.com> Date: Tue, 18 Aug 2015 13:50:23 -0400 Message-ID: From: Cory Fields To: Tamas Blummer 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 wrote: > See responses inline. > > On Tue, Aug 18, 2015 at 6:31 AM, Tamas Blummer 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