[kernel 2c/n] Introduce `kernel::Context`, encapsulate global init/teardown #25065

dongcarl · 2022-05-04T22:41:12Z

The full init/common.cpp is dependent on things like ArgsManager (which we wish to remove from libbitcoinkernel in the future) and sanity checks. These aren't necessary for libbitcoinkernel so we only extract the portion that is necessary (namely init::{Set,Unset}Globals().

maflcko · 2022-05-05T06:06:24Z

src/Makefile.am

@@ -870,7 +872,7 @@ libbitcoinkernel_la_SOURCES = \
  index/base.cpp \
  index/blockfilterindex.cpp \
  index/coinstatsindex.cpp \
-  init/common.cpp \
+  kernel/init/common.cpp \


Not sure if we want a kernel folder for code that is shared between node and kernel. An alternative would be to name it init/kernel.cpp or init/globals.cpp?

Otherwise it will be difficult to decide whether to put kernel code that is used elsewhere as well in kernel or in the "normal" hierarchy? cc @ryanofsky

I think the main thing here is to have a name that clearly indicates that this contains "only the common initialization code that the kernel needs" so that people don't accidentally add non-kernel code to it (it would result in a linker error when building bitcoin-chainstate).

Other PRs in the project (e.g. #24410) does this as well, since "extracting only what the kernel needs" is the main goal of this phase of the project. I like using the kernel/ prefix since it makes the indication mentioned in the previous paragraph clear, and doesn't require inventing a new name.

In this particular case though, perhaps kernel/init.cpp would work.

One question is: do we keep the two functions moved in the init:: namespace or the kernel:: namespace.

In the longer run, I think eventually (very far off when libbitcoinkernel is very minimal) we could put everything that links into libbitcoinkernel into the src/kernel/ folder.

So is the goal to eventually split validation into kernel/validation.cpp and node/validation.cpp ? What about util functions that are included in the kernel?

So is the goal to eventually split validation into kernel/validation.cpp and node/validation.cpp ?

Yes, far off into the future we will likely do that (although I think in reality node/validation.cpp will likely be either: 1. a very very short file, or 2. deleted because what would be left belongs better in a differently named file).

What about util functions that are included in the kernel?

I've discussed this with Ryan in #24352, and we came to the conclusion that we will move all the utils that libbitcoinkernel depends on to libbitcoin_util, now that's just w/re the libraries, we could do either one of two things for the hierarchy:

src/kernel/ and src/util/, or

src/kernel/ and src/kernel/util/

Either is fine with me. See here for more discussion: #24352 (comment)

Hmm, so to recall the goals of kernel are:

Provide a self-contained thing for other projects to use.

Make it clear what code is consensus related and what code isn't.

(Correct me if I am wrong)

Putting all utils into the kernel util library helps neither goal. Though, at the same time anything used by kernel isn't strictly consensus code. For example logging is used, but it is not consensus critical. Still, anything not used by kernel can't be consensus code, so I am wondering if there should be two util libs? If yes, then it would make sense to have both src/util and src/kernel/util. If not, then I don't care what is done.

so I am wondering if there should be two util libs? If yes, then it would make sense to have both src/util and src/kernel/util. If not, then I don't care what is done.

I think that the point of #24352 (comment) is to establish exactly what you're saying. In effect:

common will be the non-kernel "util lib" (i.e. src/util), and

util will be the kernel "util lib" (i.e. src/kernel/util).

I think that the point of #24352 (comment) is to establish exactly what you're saying. In effect:

SGTM

For example logging is used

Yes. Though, for the eventual libbitcoin kernel we need to find some way to do logging that doesn't make the client application depend on bitcoind's logging subsystem (say, a hook / notification registration). Out of scope here, of course.

src/init/common.h

DrahtBot · 2022-05-05T14:14:29Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#21763 (test: Run AppInitSanityChecks before all tests by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

dongcarl · 2022-05-05T20:15:31Z

Pushed d5876d1 -> 13e6551

Adjusted according to [kernel 2a/n] Split hashing/index GetUTXOStats codepaths, decouple from coinstatsindex #24410 (comment)

dongcarl · 2022-05-05T20:20:12Z

Pushed 13e6551 -> 708e271

Addressed: [kernel 2c/n] Introduce kernel::Context, encapsulate global init/teardown #25065 (comment)

dongcarl · 2022-05-05T22:10:27Z

Pushed 708e271 -> e4c3552

Fixed incorrect header #ifdef: https://github.com/bitcoin/bitcoin/runs/6313105188

DrahtBot · 2022-05-17T02:34:18Z

Guix builds

File	commit `225e5b5` (master)	commit `a35aa55` (master and this pull)
SHA256SUMS.part	`1b58b80ca65983b3...`	`697fe8221c822555...`
*-aarch64-linux-gnu-debug.tar.gz	`26b50f702207b949...`	`05f6440c047876da...`
*-aarch64-linux-gnu.tar.gz	`93156ddbf46f81df...`	`67a244cd13c03670...`
*-arm-linux-gnueabihf-debug.tar.gz	`448908d67e456427...`	`cb9335c39864cd59...`
*-arm-linux-gnueabihf.tar.gz	`595189752c32bf97...`	`edfeeb3c6586e579...`
*-arm64-apple-darwin-unsigned.dmg	`8fe9a774204a88c7...`	`0efc857243f4834c...`
*-arm64-apple-darwin-unsigned.tar.gz	`293f411c2dc51edb...`	`3def6f714f5c9ac2...`
*-arm64-apple-darwin.tar.gz	`abca1d81d437a08a...`	`c7ef8d7b1ffa0ff3...`
*-powerpc64-linux-gnu-debug.tar.gz	`1744beebac78a5d6...`	`2ed359d1c868c668...`
*-powerpc64-linux-gnu.tar.gz	`ec4d4712136dc767...`	`6334b422a3a3e628...`
*-powerpc64le-linux-gnu-debug.tar.gz	`ac1890b8743df6a5...`	`f63b456b18ad54e0...`
*-powerpc64le-linux-gnu.tar.gz	`b0ebc5e4120bf000...`	`bd14c28c65a5966e...`
*-riscv64-linux-gnu-debug.tar.gz	`702be3dc495a5a10...`	`434bec65cbf9eeed...`
*-riscv64-linux-gnu.tar.gz	`ad1394d672c7b965...`	`b95c9bec21a376cb...`
*-win64-debug.zip	`5464743530003d4d...`	`1ed6bffb08973b9b...`
*-win64-setup-unsigned.exe	`955c755466eb2b07...`	`6cac9eed002e77df...`
*-win64-unsigned.tar.gz	`0126640c2d061f14...`	`2f217181be03e5e8...`
*-win64.zip	`5b07889e2dce9103...`	`1589a390a05ca335...`
*-x86_64-apple-darwin-unsigned.dmg	`c5526437a0193c3b...`	`061d5c7851f79fd4...`
*-x86_64-apple-darwin-unsigned.tar.gz	`e1d81f18176fab2e...`	`2283101f6da8291c...`
*-x86_64-apple-darwin.tar.gz	`d34e5f7fe086c51f...`	`3b0e96d379306eda...`
*-x86_64-linux-gnu-debug.tar.gz	`acb6561a5919c80f...`	`e83da3071ae21cad...`
*-x86_64-linux-gnu.tar.gz	`00bb67a5b4e29ca3...`	`e947650852f0918d...`
*.tar.gz	`ab4e54f5d920eb45...`	`6167eae7a1103d2c...`
guix_build.log	`68d9a71f1973680b...`	`e17425caa820c218...`
guix_build.log.diff		`a2f6c50b1586c11b...`

dongcarl · 2022-05-20T21:31:34Z

Pushed e4c3552 -> e6f8606

Rebased over master after merge of [kernel 2b/n] Add ChainstateManager::m_adjusted_time_callback #25064

dongcarl · 2022-05-24T14:16:56Z

Push e6f8606 -> 6d93c44

Rebased over master after merge of [kernel 2a/n] Split hashing/index GetUTXOStats codepaths, decouple from coinstatsindex #24410

theuni

Concept ACK and looks good from a light review.

No comment on the naming/structure, what was discussed above SGTM. I suspect this will change around a few times before finally settling anyway.

The split here seems pretty arbitrary... I could imagine a few different approaches to chopping up common.cpp and (without digging too much) this isn't the most obvious one to me. But if this keeps things moving, sure 👍.

src/init/common.h

src/kernel/init/common.cpp

ryanofsky · 2022-05-25T00:43:24Z

Few suggestions (feel free to ignore):

Would squash 3 commits to 1 commit. There's like 7 lines of code moving in this PR. I don't think moving it in 3 parts instead of 1 part helps anyone!
Would rename src/kernel/init/common.h,cpp to src/kernel/init.h,cpp
Would rename namespace init to namespace kernel (namespace name not matching directory name is like a sign pointing in wrong direction, probably worse than not having the sign)
Maybe for followup, instead of kernel::SetGlobals kernel::UnsetGlobals call these kernel::Init kernel::Destroy or Load/Unload or similar. If you wanted to be really fancy you could call these kernel::Context::Context() and kernel::Context::~Context() but no need to go crazy with another context struct yet.

ryanofsky · 2022-05-25T12:34:38Z

To explain suggestions in #25065 (comment) a little, I think the libbitcoin_kernel external interface should not use global variables. libbitcoin_kernel is supposed to be a library, and having used many libraries, I'd claim that libraries which use global variables or other implicit singletons are a pain to develop and test with.

I understand bitcoin code uses global variables, and you don't want to make big changes to existing code to get rid of them. This is fine. But we shouldn't add kernel functions which require the implementation use to global variables, and make them difficult to get rid of in the future.

You can easily create a clean API that does not require other code to use global variables by defining a kernel::Context struct that owns whatever state is needed to run kernel functions, and passing Context* to functions in src/kernel/ meant to be called externally. Internally (especially at first), Context* won't be accessible to a lot of existing code, so global variables should still be used. They just need to point to the context or to specific members of the context instead of owning the state directly. Not much existing code should have to change. The only code that might have to change is the lowest-level code directly referencing global variables that were moved. Only some of this code would have to change, and no code above it would have to change.

At a higher level, the kernel::Context struct could just be understood as a thing which lets libbitcoin_kernel code remember state between calls, and interact with the outside world without relying on globals. So it can ask where is the data dir? How do I log this message? How do I generate a random number? What time is it? And so on

theuni · 2022-05-25T14:27:14Z

I understand bitcoin code uses global variables, and you don't want to make big changes to existing code to get rid of them. This is fine. But we shouldn't add kernel functions which require the implementation use to global variables, and make them difficult to get rid of in the future.

Totally agree with this. I think I was pretty much complaining about the same thing here: #25065 (comment)

I expect lots of weird in-between-ness at this stage. But I was generally hoping that for the most part, the stuff that gets moved into the library would be relatively clean and not in need of much more untangling. In that regard, this PR seems a bit regressive.

At a higher level, the kernel::Context struct could just be understood as a thing which lets libbitcoin_kernel code remember state between calls, and interact with the outside world without relying on globals. So it can ask where is the data dir? How do I log this message? How do I generate a random number? What time is it? And so on

100% agree with this as well. I've been expecting a new Context struct to come into play and at this point it's a bit weird that we don't have one. This PR may be a good opportunity to introduce it.

theuni

I took one last pass through, looking good.

Left a comment about a messy function, not the end of the world if it doesn't get addressed here.

I do think the const ref issues (there may be others) should be addressed though so that it's clear moving forward what's intended to be read-only.

src/init.cpp

src/kernel/checks.h

src/node/interfaces.cpp

vasild

ACK fa2ee7b

yes it should eventually be in namespace kernel

Ok, why not get it right from the first time, since it is trivial to do so:

--- i/src/kernel/chainstatemanager_opts.h
+++ w/src/kernel/chainstatemanager_opts.h
@@ -7,17 +7,20 @@
 
 #include <cstdint>
 #include <functional>
 
 class CChainParams;
 
+namespace kernel {
+
 /**
  * An options struct for `ChainstateManager`, more ergonomically referred to as
  * `ChainstateManager::Options` due to the using-declaration in
  * `ChainstateManager`.
  */
 struct ChainstateManagerOpts {
     const CChainParams& chainparams;
     const std::function<int64_t()> adjusted_time_callback{nullptr};
 };
+} // namespace kernel
 
 #endif // BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
--- i/src/validation.h
+++ w/src/validation.h
@@ -854,13 +854,13 @@ private:
         const CBlockHeader& block,
         BlockValidationState& state,
         CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     friend CChainState;
 
 public:
-    using Options = ChainstateManagerOpts;
+    using Options = kernel::ChainstateManagerOpts;
 
     explicit ChainstateManager(const Options& opts)
         : m_chainparams{opts.chainparams},
           m_adjusted_time_callback{Assert(opts.adjusted_time_callback)} {};
 
     const CChainParams& GetParams() const { return m_chainparams; }

...instead of explicitly calling init::{Set,Unset}Globals. Cool thing about this is that in both the testing and bitcoin-chainstate codepaths, we no longer need to explicitly unset globals. The kernel::Context goes out of scope and the globals are unset "automatically". Also construct kernel::Context outside of AppInitSanityChecks()

dongcarl · 2022-06-02T15:49:11Z

Pushed fa2ee7b -> a8cdeca

Addressed

dongcarl · 2022-06-02T15:51:57Z

@vasild

Ok, why not get it right from the first time, since it is trivial to do so:

Yeah sorry I should have done that in the first place. But it's unrelated to the current PR right now so we'll just do it later. I've added it as a TODO to the umbrella issue!

src/init.cpp

This reduces libbitcoinkernel's coupling with ui_interface and translation.

dongcarl · 2022-06-02T16:24:40Z

Pushed a8cdeca -> d87784a

Added comment about missing default case (thanks Jeremy!)

theuni

ACK d87784a

There are a few good outstanding suggestions, but this work is such a continuum that I think it's fine to address them in subsequent PRs if that's easier for @dongcarl.

src/test/util/setup_common.h

vasild

ACK d87784a

…late …

maflcko

Left some nits and a test setup question.

maflcko · 2022-06-06T13:29:25Z

src/bitcoin-chainstate.cpp

@@ -49,7 +52,11 @@ int main(int argc, char* argv[])
    SelectParams(CBaseChainParams::MAIN);
    const CChainParams& chainparams = Params();

-    init::SetGlobals(); // ECC_Start, etc.
+    kernel::Context kernel_context{};
+    // We can't use a goto here, but we can use an assert since none of the


unrelated: Probably a good time to get rid of goto instead of adding comments on why goto can't be used here. 😅

src/kernel/context.cpp

maflcko · 2022-06-06T13:43:04Z

src/test/util/setup_common.cpp

@@ -146,7 +146,6 @@ BasicTestingSetup::~BasicTestingSetup()
    LogInstance().DisconnectTestLogger();
    fs::remove_all(m_path_root);
    gArgs.ClearArgs();
-    init::UnsetGlobals();


fed085a: Looks like m_node.kernel isn't reset and left "dangling" until the end of ~BasicTestingSetup, which only happens to be here? Seems both brittle and inconsistent with the shutdown sequence in bitcoind, no?

src/kernel/checks.cpp

src/kernel/context.h

These checks were added in #4339, (see also #4081), to test our back-compat stubs, however, those stubs no-longer exist (#22930), meaning that these checks are now just testing some specific standard library behaviour, without a particular rationale, or reason, compared to any other standard library functions we use. There has also been some discussion about the sanity checks in the context of the libbitcoinkernel refactoring, see bitcoin/bitcoin#25065 (comment). Removing the checks removes the need to worry about atleast the glibcxx checks. Also remove the list of check from the doc in init.h, because it is incomplete, and anyone who wants to know what checks are included can look at the function.

It's partial due to this missing changes: ``` https://github.com/bitcoin/bitcoin/pull/25233/files#diff-4cb884d03ebb901069e4ee5de5d02538c40dd9b39919c615d8eaa9d364bbbd77L632 ``` cc61bc2 compat: remove glibcxx sanity checks (fanquake) Pull request description: These checks were added in dashpay#4339, (see also dashpay#4081), to test our back-compat stubs, however, those stubs no-longer exist (bitcoin#22930), meaning that these checks are now just testing some specific standard library behaviour, without a particular rationale, or reason, compared to any other standard library functionlity we use. There has also been some discussion about our sanity checks in the context of the libbitcoinkernel refactoring, see bitcoin#25065 (comment). Removing the checks removes the need to worry about atleast the glibcxx checks. Also remove the list of checks from the doc in `init.h`, because it is incomplete, and anyone who wants to know what checks are included can look at the function. Guix Build (arm64): ```bash e18a81e25b4707cbe113fb4d3ba2459013c1178e7cecfe446e4f14ee5ecd2ce8 guix-build-cc61bc2e19b1/output/arm-linux-gnueabihf/SHA256SUMS.part 9928cc38b79f827018cba0bdde98666b31806afcc79dd95a00acb8e153c36eec guix-build-cc61bc2e19b1/output/arm-linux-gnueabihf/bitcoin-cc61bc2e19b1-arm-linux-gnueabihf-debug.tar.gz ebf4635ba4688899ae62e4bb17ebb2afb25c538c4a8068ef515920fd4e43754e guix-build-cc61bc2e19b1/output/arm-linux-gnueabihf/bitcoin-cc61bc2e19b1-arm-linux-gnueabihf.tar.gz 74c7e35b47c6d101fda7205f144d37150329b4c360db09d37b8c1437f3390898 guix-build-cc61bc2e19b1/output/arm64-apple-darwin/SHA256SUMS.part 6e12643b17be9326f1d873dfe51a52c082671540792877af624b42ca9f6e1791 guix-build-cc61bc2e19b1/output/arm64-apple-darwin/bitcoin-cc61bc2e19b1-arm64-apple-darwin-unsigned.dmg 1d86d0416c7a50afd7bd8d850f416b7c7277464ccc95e4dae53b5b59415fc83d guix-build-cc61bc2e19b1/output/arm64-apple-darwin/bitcoin-cc61bc2e19b1-arm64-apple-darwin-unsigned.tar.gz 84070843f23839e7191ad3a667eb63c45f668eb95afbfb3fcdfb8363320f67d4 guix-build-cc61bc2e19b1/output/arm64-apple-darwin/bitcoin-cc61bc2e19b1-arm64-apple-darwin.tar.gz bf6ccd7b8c40476b1dc52b491757313ea3e96c43a01c8aabaf39f94dc1837329 guix-build-cc61bc2e19b1/output/dist-archive/bitcoin-cc61bc2e19b1.tar.gz 25e7e1ff7d8de38632abf9926343c8ba556209f3d03109c92864ffe72813a05f guix-build-cc61bc2e19b1/output/powerpc64-linux-gnu/SHA256SUMS.part d0398de83841607b1bf921d4553b30ad5e2d70d0570e96a2eaaf2762e1103c79 guix-build-cc61bc2e19b1/output/powerpc64-linux-gnu/bitcoin-cc61bc2e19b1-powerpc64-linux-gnu-debug.tar.gz f09cdc2ac2a2bb644f4749f3d74b5210ddb531594c33d127a907f0223e7793e5 guix-build-cc61bc2e19b1/output/powerpc64-linux-gnu/bitcoin-cc61bc2e19b1-powerpc64-linux-gnu.tar.gz ef36a68ef4e5ee9b311df40062cd2296f897e7b1550e39e0643601cd7d469010 guix-build-cc61bc2e19b1/output/powerpc64le-linux-gnu/SHA256SUMS.part 937b600a2b86304ccc5b6c71a7eaf8aa5e2020592724cef6a933a1955995480b guix-build-cc61bc2e19b1/output/powerpc64le-linux-gnu/bitcoin-cc61bc2e19b1-powerpc64le-linux-gnu-debug.tar.gz eca4eec41e71fdf7a7b0fa4065afa49c47d3b9541ed2cb4d083ad4a0de102e37 guix-build-cc61bc2e19b1/output/powerpc64le-linux-gnu/bitcoin-cc61bc2e19b1-powerpc64le-linux-gnu.tar.gz 981c0968c19905925a599cff357ec259c1e806bdb7691c7b52039be450bdad7c guix-build-cc61bc2e19b1/output/riscv64-linux-gnu/SHA256SUMS.part 89c709967f9a157256281fbf682aad246f2eaad9c2f1797c2787253cbabe12f8 guix-build-cc61bc2e19b1/output/riscv64-linux-gnu/bitcoin-cc61bc2e19b1-riscv64-linux-gnu-debug.tar.gz 454cd830dd382e176f5a23041fc33f93937668245481b0dd29fc04882d9528eb guix-build-cc61bc2e19b1/output/riscv64-linux-gnu/bitcoin-cc61bc2e19b1-riscv64-linux-gnu.tar.gz e0812c2dc492e5c5f06e3685d19da8fb29ed38d3b32821d293ef01cb4fefbd79 guix-build-cc61bc2e19b1/output/x86_64-apple-darwin/SHA256SUMS.part 0e7d4241d8ac882a8091fa00a7813db87a3e5afec59627e45b6c910cfdd4a7b0 guix-build-cc61bc2e19b1/output/x86_64-apple-darwin/bitcoin-cc61bc2e19b1-x86_64-apple-darwin-unsigned.dmg 3faaca046cbb2642445a7dd1f389ed7bf94a65de8372441c36d5cb79c030ce31 guix-build-cc61bc2e19b1/output/x86_64-apple-darwin/bitcoin-cc61bc2e19b1-x86_64-apple-darwin-unsigned.tar.gz 73080f032a42db679baf0d09619671ac5b9d85be84a68bdd6b6709eb0e6465bd guix-build-cc61bc2e19b1/output/x86_64-apple-darwin/bitcoin-cc61bc2e19b1-x86_64-apple-darwin.tar.gz 07b6e1b6291404bca1044df4a45b6958b882ffb88c143ba98f1959960a394897 guix-build-cc61bc2e19b1/output/x86_64-linux-gnu/SHA256SUMS.part 16b455f62398f4aa0d3821abb1cceb8151e31c2664e3f974a764a5b8702b50f3 guix-build-cc61bc2e19b1/output/x86_64-linux-gnu/bitcoin-cc61bc2e19b1-x86_64-linux-gnu-debug.tar.gz 3c1a3a6a343f17b83f3b3d47e9426eccd2d0bcc7f824cd958fcf2cf06cdc3276 guix-build-cc61bc2e19b1/output/x86_64-linux-gnu/bitcoin-cc61bc2e19b1-x86_64-linux-gnu.tar.gz f05afa688ea7211b0049555385fb2acc26986e24d8d00893389160e07037e693 guix-build-cc61bc2e19b1/output/x86_64-w64-mingw32/SHA256SUMS.part 8bcbae67dd0746c42e1e7c7db67725a69289b08e9aa97b873d443d0aa355615d guix-build-cc61bc2e19b1/output/x86_64-w64-mingw32/bitcoin-cc61bc2e19b1-win64-debug.zip efa45e3b76e5ae08a8392d58e741325df572d92c7dd69b65d876cdcda541d2fc guix-build-cc61bc2e19b1/output/x86_64-w64-mingw32/bitcoin-cc61bc2e19b1-win64-setup-unsigned.exe 3a8c2461ca826138c3017d06279a79b4c6bee2a507ad362aa6e424f76678596c guix-build-cc61bc2e19b1/output/x86_64-w64-mingw32/bitcoin-cc61bc2e19b1-win64-unsigned.tar.gz e56ae4f609d4e6a3ca5917a4bb763c91012ece2d236d6b62a666358791e43525 guix-build-cc61bc2e19b1/output/x86_64-w64-mingw32/bitcoin-cc61bc2e19b1-win64.zip ``` ACKs for top commit: MarcoFalke: lgtm ACK cc61bc2 laanwj: Concept and code review ACK cc61bc2 Tree-SHA512: 3da6aba44eef3f864fcbe897db1faa964923756e68c6a713e444b5d01c6d3542c3d7ca26678760e81a7a9e3cd40bd90622d0a7b697c27166817ba4f1023661ef

… shutdown order consistent c1144f0 tests: Reset node context members on ~BasicTestingSetup (TheCharlatan) 9759af1 shutdown: Destroy kernel last (TheCharlatan) Pull request description: The destruction/resetting of node context members in the tests should roughly follow the behavior of the `Shutdown` function in `init.cpp`. This was originally requested by MarcoFalke in this [comment](bitcoin/bitcoin#25065 (comment)) in response to the [original pull request](bitcoin/bitcoin#25065) introducing the `kernel::Context`. ACKs for top commit: maflcko: ACK c1144f0 🗣 achow101: ACK c1144f0 ryanofsky: Code review ACK c1144f0. No code changes since last review, just updated commits and descriptions Tree-SHA512: 819bb85ff82a5c6c60e429674d5684f3692fe9062500d00a87b361cc59e6bda145be21b5a4466dee6791faed910cbde4d26baab325bf6daa1813af13a63588ff

DrahtBot added Build system Utils/log/libs labels May 4, 2022

maflcko approved these changes May 5, 2022

View reviewed changes

dongcarl force-pushed the 2022-03-libbitcoinkernel-initcommon branch from d5876d1 to 13e6551 Compare May 5, 2022 20:14

dongcarl force-pushed the 2022-03-libbitcoinkernel-initcommon branch from 13e6551 to 708e271 Compare May 5, 2022 20:19

dongcarl force-pushed the 2022-03-libbitcoinkernel-initcommon branch from 708e271 to e4c3552 Compare May 5, 2022 21:06

DrahtBot mentioned this pull request May 5, 2022

[kernel 2a/n] Split hashing/index GetUTXOStats codepaths, decouple from coinstatsindex #24410

Merged

1 task

maflcko added the DrahtBot Guix build requested label May 13, 2022

dongcarl mentioned this pull request May 13, 2022

The libbitcoinkernel Project #24303

Closed

14 tasks

DrahtBot removed the DrahtBot Guix build requested label May 17, 2022

maflcko mentioned this pull request May 18, 2022

[kernel 2b/n] Add ChainstateManager::m_adjusted_time_callback #25064

Merged

DrahtBot added the Needs rebase label May 20, 2022

dongcarl force-pushed the 2022-03-libbitcoinkernel-initcommon branch from e4c3552 to e6f8606 Compare May 20, 2022 21:31

DrahtBot added Needs rebase and removed Needs rebase labels May 20, 2022

dongcarl force-pushed the 2022-03-libbitcoinkernel-initcommon branch from e6f8606 to 6d93c44 Compare May 24, 2022 14:16

theuni reviewed May 24, 2022

View reviewed changes

src/init/common.h Outdated Show resolved Hide resolved

src/init/common.h Show resolved Hide resolved

src/kernel/init/common.cpp Outdated Show resolved Hide resolved

DrahtBot removed the Needs rebase label May 24, 2022

dongcarl force-pushed the 2022-03-libbitcoinkernel-initcommon branch 2 times, most recently from e959bfd to 9bc9309 Compare May 25, 2022 19:04

DrahtBot removed the Needs rebase label May 31, 2022

theuni reviewed Jun 1, 2022

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

src/kernel/checks.h Outdated Show resolved Hide resolved

src/node/interfaces.cpp Outdated Show resolved Hide resolved

vasild approved these changes Jun 2, 2022

View reviewed changes

dongcarl added 2 commits June 2, 2022 11:40

Move init::SanityCheck to kernel::SanityCheck

265d639

dongcarl force-pushed the 2022-03-libbitcoinkernel-initcommon branch from fa2ee7b to a8cdeca Compare June 2, 2022 15:44

JeremyRubin reviewed Jun 2, 2022

View reviewed changes

src/init.cpp Show resolved Hide resolved

kernel: SanityChecks: Return an error struct

d87784a

This reduces libbitcoinkernel's coupling with ui_interface and translation.

dongcarl force-pushed the 2022-03-libbitcoinkernel-initcommon branch from a8cdeca to d87784a Compare June 2, 2022 16:23

theuni approved these changes Jun 2, 2022

View reviewed changes

src/test/util/setup_common.h Show resolved Hide resolved

vasild approved these changes Jun 3, 2022

View reviewed changes

fanquake merged commit aac9c25 into bitcoin:master Jun 4, 2022

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 5, 2022

Merge bitcoin#25065: [kernel 2c/n] Introduce kernel::Context, encapsu…

ceee697

…late …

maflcko reviewed Jun 6, 2022

View reviewed changes

src/kernel/context.h Show resolved Hide resolved

maflcko mentioned this pull request Jun 8, 2022

refactor: Reduce number of LoadChainstate parameters and return values #25308

Merged

theuni mentioned this pull request Sep 14, 2022

build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set #25972

Merged

dongcarl mentioned this pull request Sep 26, 2022

refactor / kernel: Move non-gArgs chainparams functionality to kernel #26177

Merged

TheCharlatan mentioned this pull request May 6, 2023

Bitcoin Kernel Library Project Tracking #27587

Open

2 tasks

This was referenced Aug 5, 2023

shutdown: Destroy kernel last, make test shutdown order consistent #28224

Merged

kernel: Run sanity checks on context construction #28228

Closed

bitcoin locked and limited conversation to collaborators Jan 17, 2024

[kernel 2c/n] Introduce kernel::Context, encapsulate global init/teardown #25065

[kernel 2c/n] Introduce kernel::Context, encapsulate global init/teardown #25065

Uh oh!

Conversation

dongcarl commented May 4, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongcarl May 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DrahtBot commented May 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

dongcarl commented May 5, 2022

Uh oh!

dongcarl commented May 5, 2022

Uh oh!

dongcarl commented May 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented May 17, 2022

Guix builds

Uh oh!

dongcarl commented May 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongcarl commented May 24, 2022

Uh oh!

theuni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanofsky commented May 25, 2022

Uh oh!

ryanofsky commented May 25, 2022

Uh oh!

theuni commented May 25, 2022

Uh oh!

theuni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

dongcarl commented Jun 2, 2022

Uh oh!

dongcarl commented Jun 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dongcarl commented Jun 2, 2022

Uh oh!

theuni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

[kernel 2c/n] Introduce `kernel::Context`, encapsulate global init/teardown #25065

[kernel 2c/n] Introduce `kernel::Context`, encapsulate global init/teardown #25065

dongcarl May 5, 2022 •

edited

Loading

DrahtBot commented May 5, 2022 •

edited

Loading

dongcarl commented May 5, 2022 •

edited

Loading

dongcarl commented May 20, 2022 •

edited

Loading

dongcarl commented Jun 2, 2022 •

edited

Loading