Subject: Re: [bitcoin-dev] libconsensus and bitcoin development process

From: Jeff Garzik <[email-masked]>
Date: Tue, 29 Sep 2015 09:04:09 -0400

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 <[email-masked]>
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 <<site:
> 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 <<site:lists.linuxfoundation.org
> 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 <
> [email-masked]> wrote:
>
>> On Tue, Sep 15, 2015 at 6:10 AM, Jeff Garzik via bitcoin-dev
>> <[email-masked]> 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
>> [email-masked]
>> 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 <http://www.litmocracy.com> and Meme Racing
> <http://www.memeracing.net> (in alpha).
> I'm the webmaster for The Voluntaryist <http://www.voluntaryist.com>
> which now accepts Bitcoin.
> I also code for The Dollar Vigilante <http://dollarvigilante.com/>.
> "He ought to find it more profitable to play by the rules" - Satoshi
> Nakamoto
>