summaryrefslogtreecommitdiff
path: root/00/f9cb2e04545a7abbb26045c81a0199bbbe6ab5
blob: d3b05c1568bb8801dacffdc12a4f782d0d1af0fd (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
144
145
146
Received: from sog-mx-3.v43.ch3.sourceforge.com ([172.29.43.193]
	helo=mx.sourceforge.net)
	by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.76)
	(envelope-from <gmaxwell@gmail.com>) id 1Y0Zbs-0007f0-7C
	for bitcoin-development@lists.sourceforge.net;
	Mon, 15 Dec 2014 17:39:04 +0000
Received-SPF: pass (sog-mx-3.v43.ch3.sourceforge.com: domain of gmail.com
	designates 209.85.213.173 as permitted sender)
	client-ip=209.85.213.173; envelope-from=gmaxwell@gmail.com;
	helo=mail-ig0-f173.google.com; 
Received: from mail-ig0-f173.google.com ([209.85.213.173])
	by sog-mx-3.v43.ch3.sourceforge.com with esmtps (TLSv1:RC4-SHA:128)
	(Exim 4.76) id 1Y0Zbr-0007ll-AF
	for bitcoin-development@lists.sourceforge.net;
	Mon, 15 Dec 2014 17:39:04 +0000
Received: by mail-ig0-f173.google.com with SMTP id r2so5386234igi.12
	for <bitcoin-development@lists.sourceforge.net>;
	Mon, 15 Dec 2014 09:38:58 -0800 (PST)
MIME-Version: 1.0
X-Received: by 10.42.68.203 with SMTP id y11mr27964051ici.62.1418665137965;
	Mon, 15 Dec 2014 09:38:57 -0800 (PST)
Received: by 10.107.16.30 with HTTP; Mon, 15 Dec 2014 09:38:57 -0800 (PST)
In-Reply-To: <20141215124730.GA8321@savin.petertodd.org>
References: <20141215124730.GA8321@savin.petertodd.org>
Date: Mon, 15 Dec 2014 17:38:57 +0000
Message-ID: <CAAS2fgQuLfU=QmK4oyYtEQxiLByLqie9GKDyMs2_2-KZP0qrWA@mail.gmail.com>
From: Gregory Maxwell <gmaxwell@gmail.com>
To: Peter Todd <pete@petertodd.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
X-Spam-Score: -1.6 (-)
X-Spam-Report: Spam Filtering performed by mx.sourceforge.net.
	See http://spamassassin.org/tag/ for more details.
	-1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for
	sender-domain
	0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider
	(gmaxwell[at]gmail.com)
	-0.0 SPF_PASS               SPF: sender matches SPF record
	-0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from
	author's domain
	0.1 DKIM_SIGNED            Message has a DKIM or DK signature,
	not necessarily valid
	-0.1 DKIM_VALID Message has at least one valid DKIM or DK signature
X-Headers-End: 1Y0Zbr-0007ll-AF
Cc: Bitcoin Dev <bitcoin-development@lists.sourceforge.net>
Subject: Re: [Bitcoin-development] Recent EvalScript() changes mean
 CHECKLOCKTIMEVERIFY can't be merged
X-BeenThere: bitcoin-development@lists.sourceforge.net
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: <bitcoin-development.lists.sourceforge.net>
List-Unsubscribe: <https://lists.sourceforge.net/lists/listinfo/bitcoin-development>,
	<mailto:bitcoin-development-request@lists.sourceforge.net?subject=unsubscribe>
List-Archive: <http://sourceforge.net/mailarchive/forum.php?forum_name=bitcoin-development>
List-Post: <mailto:bitcoin-development@lists.sourceforge.net>
List-Help: <mailto:bitcoin-development-request@lists.sourceforge.net?subject=help>
List-Subscribe: <https://lists.sourceforge.net/lists/listinfo/bitcoin-development>,
	<mailto:bitcoin-development-request@lists.sourceforge.net?subject=subscribe>
X-List-Received-Date: Mon, 15 Dec 2014 17:39:04 -0000

On Mon, Dec 15, 2014 at 12:47 PM, Peter Todd <pete@petertodd.org> wrote:
[snip]
> Pull-req #4890=C2=B2, specifically commit c7829ea7, changed the

This change was authored more than three months ago and merged more
than two months ago.
[And also, AFAICT, prior to you authoring BIP65]

I didn't participate in that pull-req, though I saw it... it had five
other contributors working on it and I try to have minimal opinions on
code organization and formatting.

But the idea sounded (and still sounds) reasonable to me.  Of course,
anything could still be backed out if it turned out to be ill-advised
(even post 0.10, as I think now we've had months of testing with this
code in place and removing it may be more risky)... but your comments
here are really not timely.
Everyone has limited resources, which is understandable, but the
concerns you are here are ones that didn't involve looking at the code
to raise, and would have been better process wise raised earlier.

> We need to fix this if CHECKLOCKTIMEVERIFY is to be merged.

I don't see why you conclude this. Rather than violating the layering
by re-parsing the transaction as the lower level, just make this data
additional information that is needed available.
Yes, does mean that rebasing an altcoin that made modifications here
will take more effort and understanding of the code than a purely
mechanical change.

> Secondly, that this change was made, and the manner in which is was made,=
 is I
> think indicative of a development process that has been taking significan=
t
> risks with regard to refactoring the consensus critical codebase.

I don't agree. The character of this change is fairly narrow. We have
moderately good test coverage here, and there were five participants
on the PR.

> While it would be nice to have a library encapsulating the consensus code=
, this
> shouldn't come at the cost of safety, especially when the actual users of=
 that

This is all true stuff, but the fact of it doesn't follow that any
particular change was especially risky.

Beyond the general 'things were changed in a way that made rebasing
an-altcoin take more work' do you have a specific concern here?

Other than travling back in time three months and doing something
differently, do you have any suggestions to ameliorate that concern?
E.g. are their additional tests we don't already have that you think
would increase your confidence with respect to specific safety
concerns?

> A much safer approach would be to keep the code changes required for a
> consensus library to only simple movements of code for this release, acce=
pt
> that the interface to that library won't be ideal, and wait until we have
> feedback from multiple opensource projects with publicly evaluatable code=
 on
> where to go next with the API.

There won't be any public users of the library until there can
actually _be_ a library.

PR4890's primary objective was disentangling the script validation
from the node state introduced by the the signature caching changes a
couple years ago, making it possible to build the consensus components
without application specific threading logic... and makes it possible
to have a plain script evaluator call without having to replicate all
of bitcoind's threading, signature cache, etc. logic.  Without a
change like this you can't invoke the script engine without having a
much larger chunk of bitcoind running.

0.10 is a major release, not a maintenance release. It's specifically
in major releases that we make changes which are not purely code
motion and narrow bugfixes (Though many of the changes in 0.10 were
nicely factored into verify pure code motion changes from behavioral
changes). There are many very important, even critical, behavioural
changes in 0.10.  That these changes have their own risks are part of
why they aren't in 0.9.x.