There seemed to be some agreement on IRC - after a bit of haranguing by myself :) -- that large refactors should (a) occur over a small window of time and (b) have a written plan beforehand. On Tue, Sep 22, 2015 at 7:49 PM, Dave Scotese wrote: > If I'm reading this situation correctly, Jeff is basically pointing out > that developers need more links (hooks, rungs, handholds, data points, > whatever you want to call them) so that they can see all the things his > email insinuated are missing (a plan, order, sense, etc.). He didn't say > these things were missing, but that it kind of feels like it from the > 10,000 foot view. > > If you use Google to search the list, as in < lists.linuxfoundation.org libconsensus plan>> you DO NOT get the page > Jorge gave. He wrote that page, so he had a good idea what to search for > to find it again. I just want to recommend that when you describe the work > you're doing on bitcoin, imagine several different ways people might try to > find this description in the future and make them work. In other words, > Jorge could have put "A plan for abstracting out libconsensus" in the email > where he wrote "Here are some things that need to happen first..." > > Likewise, if Jeff had searched for < libconsensus plan>> (maybe he did, but he didn't list any results), he may > have found enough clues to see Jorge's overall plan. The "site:" keyword > on Google fascinated me when I discovered it, so I let it inspire this > email :-) > > Maybe someone can explain this if I have it wrong: A few people are able > to pull code into Bitcoin/bitcoin. Isn't is possible that those few people > can agree to merge in a lot of refactor-hell PRs for those making the > requests, but postpone them to that one-week-per-month that someone > suggested? The idea of letting that "hell" come in (predictable) waves is > excellent and I was hoping to see some agreement. But I don't know who > those few are, so even if they all wrote "Yeah, we'll do that," I wouldn't > recognize that I got what I wanted. > > notplato > > On Tue, Sep 22, 2015 at 11:12 AM, Jorge Timón < > bitcoin-dev@lists.linuxfoundation.org> wrote: > >> On Tue, Sep 15, 2015 at 6:10 AM, Jeff Garzik via bitcoin-dev >> wrote: >> > [collating a private mail and a github issue comment, moving it to a >> > better forum] >> > >> > On libconsensus >> > --------------- >> > In general there exists the reasonable goal to move consensus state >> > and code to a specific, separate lib. >> > >> > To someone not closely reviewing the seemingly endless stream of >> > libconsensus refactoring PRs, the 10,000 foot view is that there is a >> > rather random stream of refactors that proceed in fits and starts >> > without apparent plan or end other than a one sentence "isolate >> > consensus state and code" summary. >> > >> > I am hoping that >> > * There is some plan >> > * We will not see a five year stream of random consensus code movement >> > patches causing lots of downstream developer headaches. >> > >> > I read every code change in every pull request that comes into >> > github/bitcoin/bitcoin with three exceptions: >> > * consensus code movement changes - too big, too chaotic, too >> > frequent, too unfocused, laziness guarantees others will inevitably >> > ACK it without me. >> > * some non-code changes (docs) >> > * ignore 80% of the Qt changes >> > >> > As with any sort of refactoring, they are easy to prove correct, easy >> > to reason, and therefore quick and easy to ACK and merge. >> > >> > Refactors however have a very real negative impact. >> > bitcoin/bitcoin.git is not only the source tree in the universe. >> > Software engineers at home, at startups, and at major companies are >> > maintaining branches of their own. >> > >> > It is very very easy to fall into a trap where a project is merging >> > lots of cosmetic changes and not seeing the downstream ripple effects. >> > Several people complained to me at the conference about all the code >> > movement changes breaking their own work, causing them to stay on >> > older versions of bitcoin due to the effort required to rebase to each >> > new release version - and I share those complaints. >> > >> > Complex code changes with longer development cycles than simple code >> > movement patches keep breaking. It is very frustrating, and causes >> > folks to get trapped between a rock and a hard place: >> > - Trying to push non-trivial changes upstream is difficult, for normal >> > and reasonable reasons (big important changes need review etc.). >> > - Maintaining non-trivial changes out of tree is also painful, for the >> > aforementioned reasons. >> > >> > Reasonable work languishes in constant-rebase hell, and incentivizes >> > against keeping up with the latest tree. >> > >> > >> > Aside from the refactor, libconsensus appears to be engineering in the >> > dark. Where is any sort of plan? I have low standards - a photo of a >> > whiteboard or youtube clip will do. >> >> Just because you don't understand the changes proposed it doesn't mean >> that they are random. >> I may have done a poor job in communicating "my plan for libconsensus" >> but I have tried many times and in many ways. >> #bitcoin-dev logs show that I have not worked "in the dark" at all, on >> the contrary, I've been very tenacious when asking for review and >> opinions, to the point that several people (at least @laanwj and >> @theuni have complained about their github inboxes being full of >> "spam"). >> This is a relatively recent thread where I describe my plan: >> >> http://lists.linuxfoundation.org/pipermail/bitcoin-dev/2015-July/009568.html >> Not my first attempt on this list. >> >> It is very frustrating that everybody seems to agree that separating >> libconsensus is a priority to maximize the number of people that can >> safely contribute to the project, but at the same time, nobody thinks >> that reviewing the necessary refactors to do so is a priority. >> I tried creating big PRs for people to see "the big picture" #5946 but >> those were too many commits and nobody wanted to read it. Gavin asked >> for an API. >> So I tried a smaller step: exposing just VerifyHeader in libconsensus >> and leave VerifyTx and VerifyBlock for later #5995 >> Again, this was "too big" and "a moving target". In the meantime I >> always had smaller one-little-step PRs that were part of a longer >> branch: >> >> ** [8/8] MERGED Consensus >> - [X] Consensus: Decouple pow from chainparams #5812 [consensuspow] >> - [X] MOVEONLY: Move constants and globals to consensus.h #5696 >> [consensus_policy0] >> - [X] Chainparams: Refactor: Decouple IsSuperMajority from Params() >> #5968 [params_consensus] >> - [X] Remove redundant getter CChainParams::SubsidyHalvingInterval() >> #5996 [params_subsidy] >> - [X] Separate CValidationState from main #5669 [consensus] >> - [X] Consensus: Decouple ContextualCheckBlockHeader from checkpoints >> #5975 [consensus_checkpoints] >> - [X] Separate Consensus::CheckTxInputs and GetSpendHeight in >> CheckInputs #6061 [consensus_inputs] >> - [X] Bugfix: Don't check the genesis block header before accepting it >> #6299 [5975-quick-fix] >> ** [5/5] DELETED >> *** DELETED Refactor: Create CCoinsViewEfficient interface for >> CCoinsViewCache #5747 [coins] >> *** DELETED Chainparams: Explicit Consensus::Params arg in consensus >> functions #6024 [params_consensus2] >> *** DELETED MOVEONLY: Move most of consensus functions (pre-block) >> #6051 [consensus_moveonly] (depends on consensus-blocksize-0.12.99) >> *** DELETED Consensus: Refactor: Separate CheckFinalTx from >> main::IsFinalTx #6063 [consensus_finaltx] >> *** DELETED Consensus: Refactor: Turn CBlockIndex::GetMedianTimePast >> into independent function #6009 [consensus_mediantime] >> *** DELETED Consensus: Adapt declarations of most obviously consensus >> functions #6591 [consensus-params-0.12.99] >> *** DELETED Consensus: Move blocksize and related parameters to >> consensusparams ...without removing consensus/consensus.h [#6526 >> alternative] #6625 [consensus-blocksize-0.12.99] >> >> After a while I stop rebasing the longer branches and just maintained >> a few small consensus-related PRs at a time. >> >> Now I consolidated 3 of them in >> >> *** REVIEW Optimizations: Consensus: In AcceptToMemoryPool, >> ConnectBlock, and CreateNewBlock #6445 [consensus-txinputs-0.12.99] >> >> with the hope that it would be merged relatively fast. >> After that it will be much simpler to start talking about potential C >> APIs for VerifyHeader, VerifyTx and VerifyBlock; as well as separating >> the library to a subtree. >> >> I'm more than happy to answer any questions anyone may have about any >> of the PRs or commits, until everybody interested is convinced that >> there's nothing random in the proposed changes. >> I'm also more than happy to get advice on how to better communicate my >> plans and structure my PRs. >> _______________________________________________ >> bitcoin-dev mailing list >> bitcoin-dev@lists.linuxfoundation.org >> https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev >> > > > > -- > I like to provide some work at no charge to prove my value. Do you need a > techie? > I own Litmocracy and Meme Racing > (in alpha). > I'm the webmaster for The Voluntaryist > which now accepts Bitcoin. > I also code for The Dollar Vigilante . > "He ought to find it more profitable to play by the rules" - Satoshi > Nakamoto >