Description
Project Board: https://github.com/bitcoin/bitcoin/projects/18
This is the main tracking issue for the libbitcoinkernel project.
The libbitcoinkernel project is a new attempt at extracting out our consensus engine. The kernel part of the name highlights one of the key functional differences from libbitcoinconsensus and in fact, most libraries: it is a stateful library that can spawn threads, do caching, do I/O, and many other things which one may not normally expect from a library.
This statefulness is necessary for libbitcoinkernel's decidedly incremental approach to extracting our consensus engine. This approach favors:
- Reusing existing code
...which allows us to be continually integrated with Bitcoin Core and benefit from our extensive test suite - Incremental decoupling instead of building from scratch
...which allows us to avoid having to prematurely optimizing for a "perfect" boundary or API (tends to be highly subjective, non-obvious, may lead to unproductive bike-shedding before we've even done anything meaningful)
I believe that the work of extracting out our consensus engine into a library and making the API ergonomic is likely to be a multi-release project involving multiple contributors. The incremental approach takes this into account, and respects the sheer size of work (both in writing code and getting it through review) that needs to be undertaken.
PRs
Please see the Project Board: https://github.com/bitcoin/bitcoin/projects/18
Project-wide TODOs
- Remove
blockfilter.cppandindex/blockfilterindex.cppfrom{bitcoin_chainstate,libbitcoinkernel_la}_SOURCESafter merge of Improve Indices on pruned nodes via prune blockers #21726 - Place ChainstateManageOpts under
kernel::namespace: [kernel 2c/n] Introducekernel::Context, encapsulate global init/teardown #25065 (review) - Various followups for [kernel 2c/n] Introduce
kernel::Context, encapsulate global init/teardown #25065- Marco's post-merge review comments: [kernel 2c/n] Introduce
kernel::Context, encapsulate global init/teardown #25065 (review) - Cory's
kernel::Contextcleanup suggestions: [kernel 2c/n] Introducekernel::Context, encapsulate global init/teardown #25065 (comment)
- Marco's post-merge review comments: [kernel 2c/n] Introduce
- Ryan's post-merge review comment: [kernel 2e/n] miner: Make
mempooloptional, stop constructing temporary empty mempools #25223 (comment) - Various followups for [kernel 3a/n] Decouple
CTxMemPoolfromArgsManager#25290- Perform gArgs checking in
ApplyArgsManOptions: [kernel 3a/n] DecoupleCTxMemPoolfromArgsManager#25290 (comment) - Perform
CTxMemPool::Optionschecking inCTxMemPoolconstructor: [kernel 3a/n] DecoupleCTxMemPoolfromArgsManager#25290 (comment) - Change
CalculateMemPoolAncestorsto take inCTxMemPool::Limitsinstead: [kernel 3a/n] DecoupleCTxMemPoolfromArgsManager#25290 (comment)
- Perform gArgs checking in
- Various followups for [kernel 3b/n] Decouple
{Dump,Load}MempoolfromArgsManager#25487- Improve
DumpMempool's mutex: [kernel 3b/n] Decouple{Dump,Load}MempoolfromArgsManager#25487 (comment) - Change
LoadMempool's semantics: [kernel 3b/n] Decouple{Dump,Load}MempoolfromArgsManager#25487 (comment) - Make LoadMempool less foot-gunny: [kernel 3b/n] Decouple
{Dump,Load}MempoolfromArgsManager#25487 (comment), c4249c1
- Improve
The Game Plan
Stage 1: Extracting out a usable libbitcoinkernel.{so,dylib,dll}
Step 1: Introduce internal bitcoin-chainstate and libbitcoinkernel
a.k.a. What .cpp files do we need?
libbitcoinkernel.{so,dylib,dll}bitcoin-chainstate and libbitcoinkernel.cpp files do we need?This bitcoin-chainstate executable uses our consensus engine and its build system code will reveal the minimal set of files we need to link in to use our consensus engine as-is. It is important to note that this list of files will serve as a guiding "North Star" for this first stage of the plan: as we decouple more and more modules of the codebase from our consensus engine, this list will grow shorter and shorter.
This list of files (the _SOURCES in Automake speak) then serves as the basis for a libbitcoinkernel, which bitcoin-chainstate will be linked against.
Key Result: Any further coupling of our consensus engine with non-consensus modules will result in linker errors, preventing this project from becoming a sisyphean task of battling coupling regressions.
Step 2: Decouple most non-consensus code from libbitcoinkernel
a.k.a. Prune the unnecessary .cpp files!
There are many modules which do not logically belong in libbitcoinkernel (e.g. index/*.cpp, netaddress.cpp), but are nevertheless necessary to be included in its _SOURCES for bitcoin-chainstate to link correctly. This is because Bitcoin Core's existing codebase is full of unnecessary dependencies/couplings that need to be untangled/decoupled/broken up.
This step is where we do the decoupling for:
netaddress.cpp- Parts of
timedata.cpp - Parts of
init/common.cpp ArgsManager(this one's a doozy)index/*.cppshutdown.cpplogging.cpp
Developer Note: We do not decouple the mempool yet because most users of libbitcoinkernel may want to have an embedded mempool with Bitcoin Core's policies and we can decouple it later.
Step 3: Introduce an external bitcoin-chainstate
a.k.a. What .h files do we need?
Before this step, bitcoin-chainstate has been an internal executable managed by our build system with access to all files and headers. In this step, we add an external bitcoin-chainstate with a separate build system to reveal the minimal set of headers we need to ship in order to make the libbitcoinkernel library usable.
Step 4: Decouple most non-consensus headers from libbitcoinkernel
a.k.a. Prune the unnecessary .h files!
Similar to Step 2, there are lots of small decoupling of the header dependency tree here. A notable piece of this step is to remove leveldb includes from our headers to avoid needing to re-ship leveldb headers.
Stage 2: Polishing the API / Continual De-coupling
At this point, we have a usable libbitcoinkernel that is somewhat minimally linked. However, it has a very idiosyncratic, Bitcoin Core-specific C++ interface. The goal of this stage is to incrementally make the libbitcoinkernel API more ergonomic for users outside of Bitcoin Core. Bindings to other languages (first C, then others) should be introduced.
Personally, I have near-zero experience with library API design and langauge bindings, so I think it would be wise for other contributors to lead this stage. Ideally, they would be able to work with users looking to integrate with libbitcoinkernel who can give an accurate account of the API ergonomics from the library user's point of view.
Getting libbitcoinkernel Through Review
Most of the changes to be made are "move only", but there are a lot of these "move only" changes to be made. Of course comments/reviews regarding correctness are always more than welcome, but I want very much to avoid losing momentum on this project because of style or style-adjacent comments/reviews.
I propose the following ground rules to make this process more streamlined for all parties involved and a few things that I can do to help:
- Any outstanding comments/reviews not pertinent to the main thrust of PRs should not delay/block the merging of the core functionality of PRs. I will make sure to open a separate issue tracking all the leftover comments/reviews so that they won't be missed and can be addressed one by one.
- Whenever the PR reaches a stage where there are only leftover comments/reviews left, I will make a comment saying so. This might make it easier for maintainers to determine roughly where the PR is at in its lifecycle (ofc don't trust me, verify 😄).
Action Items
- If you have any questions, please post them below!
- If you plan on reviewing
libbitcoinkernelor are a maintainer:- Please make sure you've read the "Getting
libbitcoinkernelThrough Review" section above. - Please let me know if there's anything else I can do to help streamline the review process.
- Please make sure you've read the "Getting
- If you would like to take the lead on "Stage 2: Polishing the API / Continual De-coupling", please leave a comment below, I'd love to talk!
Metadata
Metadata
Assignees
Labels
Type
Projects
Status