{"type":"rich","version":"1.0","author_name":"npub1lhe3qfx2q5m7mq5d39waepf9lzhsy0cdey66svn63fyk6rt6n7ps7zg7ed","author_url":"https://nostr.ae/npub1lhe3qfx2q5m7mq5d39waepf9lzhsy0cdey66svn63fyk6rt6n7ps7zg7ed","provider_name":"njump","provider_url":"https://nostr.ae","html":"📅 Original date posted:2014-12-15\n📝 Original message:This is a pretty good example about refactoring discipline as well as\npremature/over optimisation.\n\nWe all want to see more modular code, but the first steps should just be to\nrelocate blocks of code so everything is more logically organised in\nsmaller files (especially for consensus critical code). Refactoring should\ncome in a second wave preferably after a stable release. Refactoring should\nbe in the pure sense, optimising code with absolutely no change in\nbehaviour.\n\nWhen it comes to actual API changes, I think we need to be a lot more\ncareful and should be considered feature requests and get a lot more\nscrutiny as we are essentially breaking backwards compatibility. #4890 was\npretty much merged with no discussion or thought yet other really simple\nand uncontroversial PRs remain unmerged for months. A key question in the\ncase of EvalScript() would have been, \"why are we passing txTo and nIn\nhere, and are there any future use cases that might require them? Why\nshould this be removed from the API and the entire method signature\nchanged?\". BC breaks always need strong justification.\n\nSo I've expressed my concern a few times about the speed and frequency of\nrefactoring and also the way it's being done. I am not alone, as others not\ndirectly connected with the Bitcoin Core project have also expressed\nconcerns about the number of refactorings \"for the sake of refactoring\",\nespecially of consensus critical code. Careful as we may be, we know from\nhistory that small edge case bugs can creep in very easily and cause a lot\nof unforeseen problems.\n\nBtcDrak\n\n\nOn Mon, Dec 15, 2014 at 12:47 PM, Peter Todd \u003cpete at petertodd.org\u003e wrote:\n\u003e\n\u003e BtcDrak was working on rebasing my CHECKLOCKTIMEVERIFY¹ patch to master a\n\u003e few\n\u003e days ago and found a fairly large design change that makes merging it\n\u003e currently\n\u003e impossible. Pull-req #4890², specifically commit c7829ea7, changed the\n\u003e EvalScript() function to take an abstract SignatureChecker object,\n\u003e removing the\n\u003e txTo and nIn arguments that used to contain the transaction the script was\n\u003e in\n\u003e and the txin # respectively. CHECKLOCKTIMEVERIFY needs txTo to obtain the\n\u003e nLockTime field of the transaction, and it needs nIn to obtain the\n\u003e nSequence of\n\u003e the txin.\n\u003e\n\u003e We need to fix this if CHECKLOCKTIMEVERIFY is to be merged.\n\u003e\n\u003e Secondly, that this change was made, and the manner in which is was made,\n\u003e is I\n\u003e think indicative of a development process that has been taking significant\n\u003e risks with regard to refactoring the consensus critical codebase. I know I\n\u003e personally have had a hard time keeping up with the very large volume of\n\u003e code\n\u003e being moved and changed for the v0.10 release, and I know BtcDrak - who is\n\u003e keeping Viacoin up to date with v0.10 - has also had a hard time giving the\n\u003e changes reasonable review. The #4890 pull-req in question had no ACKs at\n\u003e all,\n\u003e and only two untested utACKS, which I find worrying for something that made\n\u003e significant consensus critical code changes.\n\u003e\n\u003e While it would be nice to have a library encapsulating the consensus code,\n\u003e this\n\u003e shouldn't come at the cost of safety, especially when the actual users of\n\u003e that\n\u003e library or their needs is still uncertain. This is after all a\n\u003e multi-billion\n\u003e project where a simple fork will cost miners alone tens of thousands of\n\u003e dollars\n\u003e an hour; easily much more if it results in users being defrauded. That's\n\u003e also\n\u003e not taking into account the significant negative PR impact and loss of\n\u003e trust. I\n\u003e personally would recommend *not* upgrading to v0.10 due to these issues.\n\u003e\n\u003e A much safer approach would be to keep the code changes required for a\n\u003e consensus library to only simple movements of code for this release, accept\n\u003e that the interface to that library won't be ideal, and wait until we have\n\u003e feedback from multiple opensource projects with publicly evaluatable code\n\u003e on\n\u003e where to go next with the API.\n\u003e\n\u003e 1) https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki\n\u003e 2) https://github.com/bitcoin/bitcoin/pull/4890\n\u003e\n\u003e --\n\u003e 'peter'[:-1]@petertodd.org\n\u003e 00000000000000001b18a596ecadd07c0e49620fb71b16f9e41131df9fc52fa6\n\u003e\n\u003e\n\u003e ------------------------------------------------------------------------------\n\u003e Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server\n\u003e from Actuate! Instantly Supercharge Your Business Reports and Dashboards\n\u003e with Interactivity, Sharing, Native Excel Exports, App Integration \u0026 more\n\u003e Get technology previously reserved for billion-dollar corporations, FREE\n\u003e\n\u003e http://pubads.g.doubleclick.net/gampad/clk?id=164703151\u0026iu=/4140/ostg.clktrk\n\u003e _______________________________________________\n\u003e Bitcoin-development mailing list\n\u003e Bitcoin-development at lists.sourceforge.net\n\u003e https://lists.sourceforge.net/lists/listinfo/bitcoin-development\n\u003e\n\u003e\n-------------- next part --------------\nAn HTML attachment was scrubbed...\nURL: \u003chttp://lists.linuxfoundation.org/pipermail/bitcoin-dev/attachments/20141215/d2a843f5/attachment.html\u003e"}