On Mon, Dec 15, 2014 at 12:47 PM, Peter Todd <p...@petertodd.org> wrote: [snip] > Pull-req #4890², 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 significant > 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, accept > 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. ------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk _______________________________________________ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
