From mboxrd@z Thu Jan 1 00:00:00 1970 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 ) 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 ; 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: From: Gregory Maxwell To: Peter Todd 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 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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Dec 2014 17:39:04 -0000 On Mon, Dec 15, 2014 at 12:47 PM, Peter Todd 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.