Recent EvalScript() changes mean CHECKLOCKTIMEVERIFY can't be merged [combined summary]



Individual post summaries: Click here to read the original discussion on the bitcoin-dev mailing list

Published on: 2014-12-15T21:57:27+00:00


Summary:

In a series of email conversations among Bitcoin core developers, concerns were raised about the process of code movement and refactoring in the modularization of Bitcoin's codebase. The current method of continuous movement and refactoring was criticized for complicating the review process and making it difficult to track changes. Jeff Garzik suggested that if the majority of code movement was done upfront, followed by simplifications and data structure work, further patches would be easier to review and apply with less impact on unrelated code. However, Cory Fields argued that minor code changes are often needed at the micro level to accommodate useful code movement, making it challenging to handle these changes separately. Another concern was raised by Peter Todd regarding the development process of the consensus critical codebase. He pointed out that the volume of changes made for the v0.10 release posed risks, especially with regard to significant consensus critical code changes. Todd highlighted pull-req #4890 as an example, which had no ACKs and only two untested utACKS. He recommended not upgrading to v0.10 due to these issues and suggested limiting the changes required for the consensus library to simple movements of code.The importance of isolating the consensus code into a separate library was emphasized, as it would make it easier to enforce change control and track consensus changes. This goal was seen as crucial for the Bitcoin Core project. Additionally, concerns were raised about the safety of changes made to the consensus critical codebase. Commit c7829ea7 was discussed, which disentangled script validation from the node state introduced by signature caching changes. Todd argued that the change was narrow and reasonable, with multiple participants and good test coverage.Garzik stressed the significance of modular code and logical organization of smaller files, particularly for consensus critical code. He compared the process to the Linux kernel, where breaking up code into smaller files was the first step towards organization and easier review. He suggested that refactoring should come in a second wave after a stable release, and the initial step should be relocating blocks of code. The Bitcoin Core project faced criticism for the frequency and speed of refactorings taking place. Concerns were raised about small edge case bugs creeping in and causing unforeseen problems. It was suggested that relocating blocks of code into smaller files should be the first step towards more modular code, with refactoring coming later. API changes were emphasized as requiring careful consideration due to their impact on compatibility.In a separate development, it was found that a significant design change made merging the CHECKLOCKTIMEVERIFY patch to master impossible. This change indicated risks taken in the refactoring of the consensus critical codebase. The lack of ACKs and untested utACKS for pull-req #4890 further raised concerns about the safety of these changes. Todd recommended not upgrading to v0.10 due to these issues and proposed limiting code changes for the consensus library to simple movements of code.Overall, the email conversations highlighted the challenges and concerns surrounding code movement and refactoring in Bitcoin's modularization process. The importance of organized and reviewer-friendly code, the need for careful consideration of changes, and the prioritization of safety were key themes discussed among the developers.


Updated on: 2023-08-01T10:58:48.450428+00:00