summaryrefslogtreecommitdiff
path: root/a3/7692d8b3ac3c4ff7c8dba28614a6e46041268d
blob: a012c01eda81c5fff1366b5473ef41620d57d90d (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
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 92BBB89D
	for <bitcoin-dev@lists.linuxfoundation.org>;
	Tue, 18 Aug 2015 17:25:43 +0000 (UTC)
X-Greylist: whitelisted by SQLgrey-1.7.6
Received: from mail-la0-f48.google.com (mail-la0-f48.google.com
	[209.85.215.48])
	by smtp1.linuxfoundation.org (Postfix) with ESMTPS id B9723157
	for <bitcoin-dev@lists.linuxfoundation.org>;
	Tue, 18 Aug 2015 17:25:42 +0000 (UTC)
Received: by lalv9 with SMTP id v9so103958025lal.0
	for <bitcoin-dev@lists.linuxfoundation.org>;
	Tue, 18 Aug 2015 10:25:40 -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=XIqnArof7B64dswG8x1ti474LpREx8gjS2XNuHaKmXg=;
	b=MAzT76y6LYWoQp30ZNymzZNwztN5GQ8758KAUmfevLYA0E1lvU6KViW4xxF6W7XYSi
	fqMkpMOUGiqzzurF+aXLO89djpG/XTFF96zC+HS65RK1Pn2hK/jeIlrk+gUoOXRsYifS
	pdUZdm9oJ45Hm7pBQID9A0p8CltuaLxdzYwssOvIAr4jLBi5+l9iuw7Psae+N4EnLvrn
	p9XNaKxY/22ANYSJMAfE7ScxsEtC+oFa6Zcb2uLVVcWzbzT1TOl/BcjbcNBEcZls+ayH
	u2+iGo/EFtqdlM1JPDYU/Dn+QVIZ3ejkRzOyM1NwR4m7JLgT7cptBEAPyD+Wj98pUZyW
	tOYA==
X-Gm-Message-State: ALoCoQmcGTtCOYyflqkXaYboqskB7/P/1BdjXC2aIsz0bsgMzBALFz74AtPmsYGaAyMFoZNa8f4v
MIME-Version: 1.0
X-Received: by 10.152.30.73 with SMTP id q9mr6037502lah.31.1439918739441; Tue,
	18 Aug 2015 10:25:39 -0700 (PDT)
Received: by 10.114.83.200 with HTTP; Tue, 18 Aug 2015 10:25:39 -0700 (PDT)
In-Reply-To: <68E206FF-4ABD-4547-BF73-8661A7C2F08B@bitsofproof.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>
Date: Tue, 18 Aug 2015 13:25:39 -0400
Message-ID: <CAApLimhbZj44OE0HStooCtzW33=jba=eEjf89H=0uJATxAvu5g@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:25:43 -0000

See responses inline.

On Tue, Aug 18, 2015 at 6:31 AM, Tamas Blummer <tamas@bitsofproof.com> wrot=
e:
> Thanks a lot Cory for following through the test case and producing a pat=
ch.
>
> I confirm that libconsensus is now running stable within the Bits of Proo=
f
> 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 ja=
va
> and 1135 ops/sec
> in libconsensus. This is on a 2.2GH i7 laptop (4 hyper threading cores us=
ed
> 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=E2=80=99 EC lib s not superior to Ope=
nSSL'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 inte=
gration. 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