Re: [bitcoin-dev] Libconsensus separated repository (was Bitcoin Core and hard forks)

["Jorge Timón" <jtimon@jtimon.cc>]

On Thu, Aug 20, 2015 at 9:14 AM, Tamas Blummer <tamas@bitsofproof.com> wrote:
> Jorge,
>
> separating script engine into libconsensus was very helpful, since wrapped
> the piece of consensus
> that would least likely to be captured exactly with an implementation from
> scratch. Thank you for your
> effort there.  Bits of Proof now uses its own or alternatively libconsensus
> for full validation.

It wasn't just me: I didn't had the idea of creating a libconsensus
with a C API (thank Matt Corallo for that), I didn't removed all the
undesired dependencies or prepared the building part (thank Cory
Fields) and also thank at least Wladimir and Pieter who also
contributed in some ways I don't remember.
And of course also thank all the reviewers that made the PR merges possible.

I'm really happy to hear that libconsensus is being used, thank you
for your effort there too.

> I am sceptical however that a “full” consensus lib extracted from satoshi’s
> code is worth trying.
> Not because it was impossible, but because the result would not be higher
> quality, if measured on agreement
> with satoshi, than other re-implementations. It would actually be lower
> quality because of the antique tool set.

I don't understand what you mean by "quality" in this context. One of
the goals is to have as little dependencies as possible (so "more
modern tool sets" may not be suitable for libconsensus). libsecp256k1
will keep on being a dependency (highly optimized C code) and that's
about it.
Ideally I would like to slowly move libconsensus from C++ to C too,
but it seems other people would prefer to move to C++11 instead.

> The rules outside script engine are simpler, therefore much easier to
> capture exactly. They are however
> scattered around in the spaghetti and are often just a single if statement,
> also repeated elsewhere.

Yes, they are simpler and thus there's less risks of consensus fork
bugs, but it still exists.
It is true that the consensus code is currently spread all around
(specially in main.cpp), but completing libconsensus would solve that.
Lastly, since for consensus rules "the code is the specification", it
is unfortunate that the specification is coupled with a concrete
implementation (Bitcoin Core) and we should fix that.

> You would either have to very extensively refactor the code, that unlikely
> goes through as a PR, or
> do what me and others did. Read satoshi code and rewrite the same. You have
> a slight advantage of copy-paste small fragments, but I doubt the consensus
> relevant advantage of that.

But the goal is not reimplementing the consensus rules but rather
extract them from Bitcoin Core so that nobody needs to re-implement
them again.
It is not only exposing it but also separating it from Bitcoin Core so
that they can be changed without having to also change/take into
account non-consensus Bitcoin Core specific things.
A single PR would certainly be unacceptable, I was making many little
more acceptable ones (some of them already merged):

* [1/9] Consensus
** MERGED or DELETED
*** MERGED Consensus: Decouple pow from chainparams #5812 [consensuspow]
*** MERGED MOVEONLY: Move constants and globals to consensus.h #5696
[consensus_policy0]
*** DELETED Refactor: Create CCoinsViewEfficient interface for
CCoinsViewCache #5747 [coins]
*** MERGED Chainparams: Refactor: Decouple IsSuperMajority from
Params() #5968 [params_consensus]
*** MERGED Remove redundant getter
CChainParams::SubsidyHalvingInterval() #5996 [params_subsidy]
*** MERGED Separate CValidationState from main #5669 [consensus]
*** DELETED Consensus: Refactor: Separate CheckFinalTx from
main::IsFinalTx #6063 [consensus_finaltx]
*** MERGED Consensus: Decouple ContextualCheckBlockHeader from
checkpoints #5975 [consensus_checkpoints]
*** MERGED Separate Consensus::CheckTxInputs and GetSpendHeight in
CheckInputs #6061 [consensus_inputs]
*** MERGED Bugfix: Don't check the genesis block header before
accepting it #6299 [5975-quick-fix]
** REBASE Chainparams: Explicit Consensus::Params arg in consensus
functions #6024 [params_consensus2]
** REBASE Optimizations: Consensus: In AcceptToMemoryPool,
ConnectBlock, and CreateNewBlock #6445 [consensus-txinputs-0.12.99]
** REBASE MOVEONLY: Move most of consensus functions (pre-block) #6051
[consensus_moveonly]
** REBASE Consensus: Refactor: Turn CBlockIndex::GetMedianTimePast
into independent function #6009 [consensus_mediantime]
** DEPENDENT Consensus: Refactor: Consensus version of
CheckBlockHeader() #6035 [consensus_checkblockheader]
** DEPENDENT Consensus: Consensus version of pow functions [consensus_pow2]
** DEPENDENT API: Expose bitcoinconsensus_verify_header() in
libconsensus #5995 [consensus_header]
** DEPENDENT API: Expose bitcoinconsensus_verify_block() in
libconsensus #5946 [consensus_tip]