The libbitcoinkernel Project #24303

issue dongcarl openend this issue on February 9, 2022
  1. dongcarl commented at 8:45 pm on February 9, 2022: contributor

    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:

    1. Reusing existing code …which allows us to be continually integrated with Bitcoin Core and benefit from our extensive test suite
    2. 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

    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?

    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:

    1. netaddress.cpp
    2. Parts of timedata.cpp
    3. Parts of init/common.cpp
    4. ArgsManager (this one’s a doozy)
    5. index/*.cpp
    6. shutdown.cpp
    7. logging.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:

    1. 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.
    2. 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 :smile:).

    Action Items

    1. If you have any questions, please post them below!
    2. If you plan on reviewing libbitcoinkernel or are a maintainer:
      1. Please make sure you’ve read the “Getting libbitcoinkernel Through Review” section above.
      2. Please let me know if there’s anything else I can do to help streamline the review process.
    3. 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!
  2. JeremyRubin commented at 8:54 pm on February 9, 2022: contributor

    One thing that I’d like to see better / more clearly documented is which components have which requirements and what changes as a result of libbitcoinkernel.

    E.g., I recently learned that script/interpreter.* is not allowed to link against threading primitives so the code can be used in ??? environments.

    Does libbitcoinkernel have similar restrictions on what sorts of things are includable / not includable as a result (other than our own code, like platform stuff)?

  3. dongcarl commented at 9:10 pm on February 9, 2022: contributor

    @JeremyRubin

    One thing that I’d like to see better / more clearly documented is which components have which requirements and what changes as a result of libbitcoinkernel.

    Hmm, I’m not sure what you mean…

    E.g., I recently learned that script/interpreter.* is not allowed to link against threading primitives so the code can be used in ??? environments.

    Was this a public conversation? Could you link me?

  4. JeremyRubin commented at 11:12 pm on February 9, 2022: contributor

    Yeah this came up when I was implementing lazy caching for CTV using std::call_once that linking pthread from interpreter was a nono https://gnusha.org/bitcoin-core-dev/2022-01-20.log.

    I guess what I mean is it would be good to have a more clear cut boundary for libbitcoinkernel both up the stack on API boundaries but also “down the stack” in terms of what libbitcoinkernel can provide v.s. what the host environment should. Maybe this is a malformed question, feel free to chat me and we can flesh out if it’s meaningful.

  5. maflcko commented at 7:25 pm on February 10, 2022: member

    (ot: I couldn’t find “nono” in the chat. Mind quoting the exact phrase? I think for your specific implementation std::call_once would have been more complicated than the std::optional approach you later took.)

    Concept ACK on libbitcoinkernel.

  6. theuni commented at 7:51 pm on February 10, 2022: member

    Strongest possible Concept ACK. IMO we should’ve done this years ago.

    Kudos to @dongcarl for taking it on. I’ll help in every way I can!

  7. Sjors commented at 7:58 pm on February 10, 2022: member

    Bindings to other languages (first C, then others) should be introduced.

    Nice! That might be handy for iOs too; so far I’ve no luck with this, see #12557. It seems much easier to include a C project than C++, maybe because Objective C inherits from the former. Bonus points for Swift bindings. An iOs node would need p2p stuff too.

  8. michaelfolkson commented at 8:47 pm on February 10, 2022: contributor

    Concept ACK

    As others have said even if there are occasional consensus leaks from a libbitcoinkernel keeping consensus code siloed for the most part would be great for risk mitigation and review rigor.

    Some initial discussions in today’s IRC meeting.

  9. JeremyRubin commented at 8:49 pm on February 10, 2022: contributor

    @MarcoFalke the approach I ended up taking is substantially more complicated, perhaps you haven’t fully reviewed it (callers must pass in a lambda that contains an interior synchonicity primitive or otherwise guarnatee single threaded executiuon), whereas the call_once approach was correct for all users).

    The convo spanned a few days, that was just the start, my bad (I searched my local IRC for a date ref). On +1 day https://gnusha.org/bitcoin-core-dev/2022-01-21.log

     010:23 < jeremyrubin> hey cfields can you have a look at [#21702](/bitcoin-bitcoin/21702/) build failure?
 110:23 <@gribble> [#21702](/bitcoin-bitcoin/21702/) | Implement BIP-119 Validation (CheckTemplateVerify) by JeremyRubin · Pull Request [#21702](/bitcoin-bitcoin/21702/) · bitcoin/bitcoin · GitHub
 210:23 < jeremyrubin> https://stackoverflow.com/questions/34924083/mingw32-make-error-error-once-flag-in-namespace-std-does-not-name-a-type
 310:23 < jeremyrubin> https://github.com/RavenProject/Ravencoin/issues/460
 410:24 < jeremyrubin> sort of weird b/c we use once_flag and call_once in other places in the code, so it looks like we're somehow linking with the wrong options?
 510:26 < jeremyrubin> looks like laanwj might know whats going on here based on [#8653](/bitcoin-bitcoin/8653/)
 610:27 < sipa> [@stick](/bitcoin-bitcoin/contributor/stick/): https://www.96boards.org/product/developerbox/
 710:30 < jeremyrubin> specific errors are here https://gist.github.com/JeremyRubin/b3fc2ba910a8e5b6b807df3746e949d9
 810:30 < jeremyrubin> impl_pthread not getting linked
 911:09 < laanwj> jeremyrubin: that's a 5 year old issue, i'm sure it's no longer relevant? we've been using the posix variant since forever
1011:10 < laanwj> jeremyrubin: IIRC libbitcoin_consensus isn't linked against pthread intentionally, it's not supposed to be threaded
1111:11 < laanwj> because it could be used from non-threaded C code, or code using some completely different threading model
1211:25 < laanwj> i'd agree "using C++ synchronization primitives" isn't threading in itself, but apparently it does cause linking to one
1311:26 < jeremyrubin> laanwj: ok that makes sense I guess, but it's problematic slightly since the scriptchecks generated are designed to be threadsafe so i wanted to ensure that i had a low cost way to do cache on first use... but it sounds like that's troublesome no matter what I do (mutex, atomic, etc)
1411:28 < laanwj> right-even atomic isn't guaranteed, on some platforms it can be done with instructions, but on others it does need support from the OS through a threading library
1511:29 < jeremyrubin> i thought c++ requires atomics OR proof no threads are used otherwise things like static init are broken?
1611:29 < laanwj> any caching would have to be done at a higher level
1711:29 < jeremyrubin> hmm that seems like a boundary violation
1811:29 < jeremyrubin> this caching is required for validation to be correct
1911:29 < laanwj> not in the consensus code itself, i think it makes sense from the perspective that libbitcoin_consensus is supposed to be stateless too
2011:30 < laanwj> wait, no, consensus never should need caching between validations to be correct
2111:30 < jeremyrubin> PrecomputedData is required for correctness
2211:30 < jeremyrubin> where correctness includes runtime

    But this seems largely off topic for here other than having a clearer deliniation of what can and cannot be linked within kernel consensus code after this change, because I am unhappy with libbitcoin_consensus preventing the use of std::call_once in a straightforward manner, so thinking about similar issues or expectations would be good!

  10. dongcarl commented at 8:58 pm on February 10, 2022: contributor

    @JeremyRubin

    I guess what I mean is it would be good to have a more clear cut boundary for libbitcoinkernel both up the stack on API boundaries…

    Right, I think we leave the API boundary up the stack until stage 2, so that we “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)”

    but also “down the stack” in terms of what libbitcoinkernel can provide v.s. what the host environment should. Maybe this is a malformed question, feel free to chat me and we can flesh out if it’s meaningful.

    Ah I see, yeah I think the libbitcoinkernel case is very different from the libbitcoinconsensus case, since “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”. Right now if you build #24304 with --enable-shared --enable-experimental-util-chainstate you will find that the library relies on only the “expect” set of runtime libraries:

    0$ ldd ./src/.libs/libbitcoinkernel.so
1        linux-vdso.so.1 (0x00007ffc387c6000)
2        libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007faaecf6c000)
3        libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0x00007faaecd56000)
4        libm.so.6 => /usr/lib/libm.so.6 (0x00007faaecc12000)
5        libgcc_s.so.1 => /usr/lib/libgcc_s.so.1 (0x00007faaecbf7000)
6        libc.so.6 => /usr/lib/libc.so.6 (0x00007faaeca2b000)
7        /usr/lib64/ld-linux-x86-64.so.2 (0x00007faaedc83000)

    Now, a good decision to ponder upon is whether or not we should be embedding libraries such as leveldb, crc32c, and boost or whether we should require them as a runtime dependency. Currently, it is embedded: https://github.com/bitcoin/bitcoin/pull/24304/commits/1d9faa8fc7187c61a45764ea2e4c919f02b891d7#diff-4cb884d03ebb901069e4ee5de5d02538c40dd9b39919c615d8eaa9d364bbbd77R826

  11. dongcarl commented at 9:01 pm on February 10, 2022: contributor

    @Sjors

    Nice! That might be handy for iOs too; so far I’ve no luck with this, see #12557. It seems much easier to include a C project than C++, maybe because Objective C inherits from the former. Bonus points for Swift bindings. An iOs node would need p2p stuff too.

    Yup, most languages have FFI support of some kind to C, so once we have a C interface everything else will be easy!

  12. dongcarl commented at 9:02 pm on February 10, 2022: contributor

    @theuni

    Strongest possible Concept ACK. IMO we should’ve done this years ago.

    Kudos to @dongcarl for taking it on. I’ll help in every way I can!

    Thank you for your help behind the scenes so far and thanks in advance for your help in the future! 😄

  13. ryanofsky commented at 3:51 pm on February 11, 2022: contributor
    I don’t understand why there isn’t a clearer answer about what’s in the kernel and what’s not in the kernel. If there isn’t a clear line that can be drawn, I think it would be best to just treat libbitcoin_kernel as an evolving whitelist of API’s that can be called externally, and not move a lot of code around or significantly change source code organization. Otherwise, I would like to know more know more about the statement ‘Most of the changes to be made are “move only”’ because it is not clear to me what code would be moving or where it would be moving to.
  14. dongcarl commented at 7:10 pm on February 13, 2022: contributor

    @ryanofsky

    I don’t understand why there isn’t a clearer answer about what’s in the kernel and what’s not in the kernel.

    What’s included in libbitcoinknernel will always be “everything that is required for the consensus engine to function”.

    As of #24322, we’re only capturing the dependencies as they are right now, which unfortunately includes things like index/*.cpp, netaddress.cpp, etc.

    Throughout stage 1, we will decouple (or in some cases, abstract away) these unnecessary modules from the consensus engine, which will allow us to remove them from libbitcoinkernel.

    Right now in my exploration branch, here is the list of files that are still necessary to be linked in:

     0libbitcoinkernel_la_SOURCES = \
 1  arith_uint256.cpp \
 2  chain.cpp \
 3  kernel/chainparamsbase.cpp \
 4  chainparams.cpp \
 5  clientversion.cpp \
 6  coins.cpp \
 7  compat/glibcxx_sanity.cpp \
 8  compressor.cpp \
 9  consensus/merkle.cpp \
10  consensus/tx_check.cpp \
11  consensus/tx_verify.cpp \
12  core_read.cpp \
13  dbwrapper.cpp \
14  deploymentinfo.cpp \
15  deploymentstatus.cpp \
16  flatfile.cpp \
17  fs.cpp \
18  hash.cpp \
19  kernel/init/common.cpp \
20  key.cpp \
21  kernel/logging.cpp \
22  node/blockstorage.cpp \
23  node/chainstate.cpp \
24  kernel/node/coinstats.cpp \
25  node/ui_interface.cpp \
26  policy/feerate.cpp \
27  policy/fees.cpp \
28  policy/packages.cpp \
29  policy/policy.cpp \
30  policy/rbf.cpp \
31  policy/settings.cpp \
32  pow.cpp \
33  primitives/block.cpp \
34  primitives/transaction.cpp \
35  pubkey.cpp \
36  random.cpp \
37  randomenv.cpp \
38  scheduler.cpp \
39  script/interpreter.cpp \
40  script/script.cpp \
41  script/script_error.cpp \
42  script/sigcache.cpp \
43  script/standard.cpp \
44  kernel/shutdown.cpp \
45  signet.cpp \
46  support/cleanse.cpp \
47  support/lockedpool.cpp \
48  sync.cpp \
49  kernel/timedata.cpp \
50  txdb.cpp \
51  txmempool.cpp \
52  uint256.cpp \
53  util/bytevectorhash.cpp \
54  util/getuniquepath.cpp \
55  util/hasher.cpp \
56  util/moneystr.cpp \
57  util/rbf.cpp \
58  util/serfloat.cpp \
59  util/settings.cpp \
60  util/strencodings.cpp \
61  util/syscall_sandbox.cpp \
62  util/system.cpp \
63  util/thread.cpp \
64  util/threadnames.cpp \
65  util/time.cpp \
66  validation.cpp \
67  validationinterface.cpp \
68  versionbits.cpp \
69  warnings.cpp

    A few modules that we decouple/abstract from:

    1. netaddress.cpp
    2. Parts of timedata.cpp
    3. Parts of init/common.cpp
    4. ArgsManager (this one’s a doozy)
    5. index/*.cpp
    6. shutdown.cpp
    7. logging.cpp

    Of course this is still not minimal, but we can make progress on the decoupling a step at a time, and each step is worthwhile because it prevents future re-coupling.

    If there isn’t a clear line that can be drawn, I think it would be best to just treat libbitcoin_kernel as an evolving whitelist of API’s that can be called externally, and not move a lot of code around or significantly change source code organization.

    Do you mean that instead of (say) splitting off the consensus-used parts of init/common.cpp to kernel/init/common.cpp, we just explicitly mark their visibility as default or something?

    I can totally see how that would work, however, I think there’s a lot to be gained from a clean split: the functions not existing at all in libbitcoinkernel instead of being hidden.

    In any case, I’d love to avoid significant source code organization changes if I can achieve the same goals, and I know you’ve thought a lot about it, so let’s talk more!

    Otherwise, I would like to know more know more about the statement ‘Most of the changes to be made are “move only”’ because it is not clear to me what code would be moving or where it would be moving to.

    Oh of course! Here’s an example: https://github.com/dongcarl/bitcoin/commit/d7cf3f0fc2af24e6d3d38513fb01c2b9d9d48105

    In the first commit there, we split the parts of timedata.cpp not dependent on netaddress.cpp and asmap.cpp into kernel/timedata.cpp, which is “move-only” and allows us to eliminate netaddress.cpp and asmap.cpp from our _SOURCES.

    Let me know if you have any other questions or if there’s a strictly better/easier to do this that I’m missing!

  15. dongcarl commented at 7:21 pm on February 14, 2022: contributor
    Minor edit to main description: replace “style or style-adjacent comments/reviews” with “comments/reviews not pertinent to the main thrust of PRs”
  16. ryanofsky commented at 7:21 pm on February 14, 2022: contributor

    Thanks for posting #24332. Is #24332 your “exploration branch” or is that available somewhere else? A lot of things just seem vague to me and I think it would be useful to be concrete.

    In general, the approach here seems a little different than I what would expect. I would expect most of the work of making the “consensus engine” usable as the library to involve getting rid of globals more than moving code around. Like I’d expect step one to be introducing some kind of context for the consensus code to be able to function. Something like a src/kernel/context.h file with a struct like:

     0namespace kernel {
 1struct Context {
 2  std::function<void(const std::string&)> log_fn;
 3  std::function<std::string(const char*)> translate_fn;
 4  std::optional<int64_t> adjusted_time;
 5  std::unique_ptr<RNGState> rng_state;
 6  Consensus::Params params;
 7  std::unique_ptr<ChainstateManager> chainman;
 8  std::unique_ptr<CTxMemPool> mempool;
 9  ...
10};
11} // namespace kernel

    meant to replace globals. Getting rid of globals would let you use the kernel library to write a application simulating multiple nodes. Or let you use the library to write an application that uses consensus functionality for something else, and still has unit tests that run in parallel (not forced to run serially because the kernel library has shared global state.)

    Moving code around and changing source code organization could be related to this, but doesn’t have to be. I do think if you do want to change source code organization, you should have a clear idea of how the source code should be organized. Right now we already have:

    • src/node/ and libbitcoin_node.a
    • src/consensus/ and libbitcoin_consensus.a

    And you are proposing to add:

    • src/kernel/ and libbitcoin_kernel.a

    The current split between node and consensus makes sense to me. Node is for code we consider internal and unstable and don’t want to be reused externally. Consensus is for code that is stable and we think is useful to expose. But maybe we should rename node to kernel or rename consensus to kernel? Or maybe we should actually go ahead and keep node and consensus and add kernel as a third middle layer. This could make sense if there is a clear idea of what belongs in this middle layer. Right now the kernel library sounds like a “consensus engine” or “consensus support” library for things that somehow have to do with consensus, but aren’t in consensus library.

    The idea of excluding anything in the consensus library that uses threads does not seem very tenable to me, when a lot of performant code people will want to write is likely to involve parallelization. It might make more sense if the dividing line had something to do with persistent storage. Like maybe libbitcoin_consensus would define abstract mempool and UTXO database interfaces, and libbitcoin_kernel would provide a boost::multi_index implementation of the mempool interface, and a leveldb implementation of the UTXO database interface.

    re: #24303 (comment)

    I don’t understand why there isn’t a clearer answer about what’s in the kernel and what’s not in the kernel.

    What’s included in libbitcoinknernel will always be “everything that is required for the consensus engine to function”.

    This doesn’t really answer the question, just turns it into “what things are in the consensus engine?” or “why isn’t libbitcoin_consensus.a the consensus engine?

    As of #24322, we’re only capturing the dependencies as they are right now, which unfortunately includes things like index/*.cpp, netaddress.cpp, etc.

    Throughout stage 1, we will decouple (or in some cases, abstract away) these unnecessary modules from the consensus engine, which will allow us to remove them from libbitcoinkernel.

    Right now in my exploration branch, here is the list of files that are still necessary to be linked in:

    …

    A few modules that we decouple/abstract from:

    …

    This all seems good, but probably the list of things to decouple should be almost as long as the original list, and there should be not very much code to move to src/kernel/ if we are keeping src/node/ and src/consensus/.

    Do you mean that instead of (say) splitting off the consensus-used parts of init/common.cpp to kernel/init/common.cpp, we just explicitly mark their visibility as default or something?

    Right, everything’s already public.

    Oh of course! Here’s an example: dongcarl@d7cf3f0

    In the first commit there, we split the parts of timedata.cpp not dependent on netaddress.cpp and asmap.cpp into kernel/timedata.cpp, which is “move-only” and allows us to eliminate netaddress.cpp and asmap.cpp from our _SOURCES.

    Let me know if you have any other questions or if there’s a strictly better/easier to do this that I’m missing!

    Can you help me understand why this is an improvement? It seems like there could be benefits for understanding and testing and code reuse getting rid of the g_timeoffset_mutex and nTimeOffset globals and moving them into a context. But this PR just seems to be moving code from one location to another for some benefit I am unable to discern. Maybe there is some insight about the build system I’m missing, but the logic behind having:

    0src/kernel/timedata.cpp
1src/kernel/timedata.h
2src/timedata.cpp
3src/timedata.h

    eludes me and I think I don’t know what the end state is.

  17. ryanofsky commented at 9:15 pm on February 14, 2022: contributor

    eludes me and I think I don’t know what the end state is.

    I figured this out a little better now and made some concrete suggestions in #24332 (comment)

  18. ryanofsky commented at 2:24 pm on February 15, 2022: contributor

    re: #24332 (comment)

    • I still don’t understand where the boundary between libbitcoin_kernel and libbitcoin_consensus is or why these are two different libraries

    To follow up on this, I talked to Carl and he convinced me with a single word (“utreexo”) that deciding what goes into libbitcoinconsensus is not an easy task, so we need some other library that we can change more flexibly if we are going to expose new functions to outside applications. libbitcoin_node exposes too much and was never meant to be a public interface, so introducing libbitcoin_kernel makes sense, and while a good deal of libbitcoin_node code will move to libbitcoin_kernel, it’s not immediately important to figure out where the boundary is between them (though it would be nice).

    re: #24303 (comment)

    I’d expect step one to be introducing some kind of context for the consensus code to be able to function

    On the globals vs context struct thing there’s a concrete suggestion and discussion in #24332 (comment). I think it is important to create a context struct, because without one new code is forced to use global variables, and it is harder to get rid of globals gradualy without big blunt instrument changes. The libbitcoinkernel branch https://github.com/dongcarl/bitcoin/commits/2022-01-v8-on-new-kirby makes reasonable choices about what to deglobalize and what not to (ArgsManager yes, LogInstance no), but in places where it’s easy to add channels to avoid referencing globals more places I do think we should do that.

  19. dongcarl commented at 7:46 pm on February 16, 2022: contributor

    After talking to Ryan, I’m am convinced that adding a kernel::Context struct is worthwhile and will make things easier in the long run as it’ll at least be a place where people (who may be working on other things that may belong in kernel) can put their long-lived vars instead of littering more globals all over the place.

    Logistically, we don’t have to de-globalize everything at once (:fearful: logger), but just having a place is better than not.

  20. ajtowns commented at 1:49 pm on February 17, 2022: contributor

    Concept/approach ACK

    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.

    Have you considered naming it something like “libbitcoin-chainstate” (matching the new executable name)? “kernel”, “util”, “common” and “node” are all very generic names; would be nice to be more specific if it’s at all possible.

    Having the library focus on managing chain state seems plausible to me: it’s distinct from “node” which also encompasses mining and p2p and rpc access, and distinct from the current libconsensus which doesn’t keep state. To me it would make sense for “chain state” to include raw block storage, utxo info for the current tip, the mempool (transactions validated against the current chain state), and perhaps indexes (coinstats, txindex) since they also track the chain…

  21. dongcarl commented at 9:33 pm on February 22, 2022: contributor

    Have you considered naming it something like “libbitcoin-chainstate” (matching the new executable name)? “kernel”, “util”, “common” and “node” are all very generic names; would be nice to be more specific if it’s at all possible.

    Ooooof it might be a little late for a name change at this stage since I’ve been talking about it with so many people under the libbitcoinkernel name, but your point is well-taken and I will try to use more specific names in the future!

  22. maflcko referenced this in commit 619f8a27ad on Mar 3, 2022
  23. ariard commented at 10:16 pm on March 5, 2022: member

    Concept/approach ACK

    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.

    Once we have a mature libbitcoinkernel, I can see two types of new Bitcoin applications leveraging the possibilities of a consensus engine, potentially not requiring as much work than yet-another full-node implementation : a) “hybrid clients”, a node running as a resources-optimized lightclient in normal time, fallbacking to block validation in case of long-fork detection or any other network anomalies since the last hardcoded assume-utxo b) “validating second-layer signer”, a signer module releasing transactions signatures in reaction to a chain height or confirmed counterparties transactions, only if the events happen on the most-work valid chain. The consensus engine would run on the trusted zone of the embedded device. [0]

    If there is interest to develop those applications, they might be sources of feedback to a libbitcoinkernel API, at a velocity faster than a full-node. Also for b), it might be a good use-case to also abstract the raw block storage and utxo info to generic interface as they likely won’t fit for embedded resources.

    and perhaps indexes (coinstats, txindex) since they also track the chain

    I think indexes are more server oriented features than validation. A full-node implementation embedding libbitcoinkernel do not need them to stay in consensus with a Bitcoin Core node… Of course, it could be argued that’s the same with the mempool, though in practice lack of performance benefits brought to you by transaction validation caching might prevent you to stay effectively at chain tip.

    [0] https://gitlab.com/lightning-signer/validating-lightning-signer

  24. maflcko referenced this in commit 5e49b2a252 on Mar 7, 2022
  25. fanquake referenced this in commit dd17c42a16 on Apr 28, 2022
  26. laanwj referenced this in commit 7008087548 on May 24, 2022
  27. fanquake referenced this in commit a7a36590f5 on Jun 15, 2022
  28. laanwj referenced this in commit 489b587669 on Jun 16, 2022
  29. laanwj referenced this in commit a4e066af85 on Jun 22, 2022
  30. maflcko referenced this in commit e4e201dfd9 on Jun 29, 2022
  31. maflcko referenced this in commit 02ede4f1fd on Jul 14, 2022
  32. glozow referenced this in commit 821f5c824f on Jul 18, 2022
  33. glozow referenced this in commit 7312effe6a on Aug 4, 2022
  34. fanquake referenced this in commit b175bdb9b2 on Mar 14, 2023
  35. fanquake referenced this in commit e695d8536e on Mar 16, 2023
  36. fanquake referenced this in commit 369d4c03b7 on Apr 3, 2023
  37. fanquake referenced this in commit 669af32632 on Apr 21, 2023
  38. fanquake referenced this in commit fc06881f13 on May 9, 2023
  39. maflcko commented at 2:46 pm on May 10, 2023: member
    Let’s continue discussion in #27587?
  40. maflcko closed this on May 10, 2023
  41. bitcoin locked this on May 9, 2024


dongcarl JeremyRubin maflcko theuni Sjors michaelfolkson ryanofsky ajtowns ariard

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 12:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me