kernel: Remove util/system from kernel library, interface_ui from validation. #27636

TheCharlatan · 2023-05-12T08:03:05Z

This pull request is part of the libbitcoinkernel project #27587 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel".

It removes the kernel library's dependency on util/system and interface_ui. util/system contains networking and shell-related code that should not be part of the kernel library. The following pull requests prepared util/system for this final step: #27419 #27254 #27238.

interface_ui defines functions for a more general node interface and has a dependency on boost/signals2. After applying the patches from this pull request, the kernel's reliance on boost is down to boost::multiindex.

The approach implemented here introduces some indirection, which makes the code a bit harder to read. Any suggestions for improving or reworking this pull request to make it more concise, or even reworking it into a more proper interface, are appreciated.

DrahtBot · 2023-05-12T08:03:08Z

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	stickies-v, MarcoFalke, hebasto
Stale ACK	ryanofsky

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#27607 (index: improve initialization and pruning violation check by furszy)
#27596 (assumeutxo (2) by jamesob)
#27460 (rpc: Add importmempool RPC by MarcoFalke)
#27331 (refactor: extract CCheckQueue's data handling into a separate container "Bag" by martinus)
#26762 (refactor: Make CCheckQueue RAII-styled by hebasto)
#19461 (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
#19460 (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)

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.

ryanofsky

Approach ACK. I think adding the level of indirection here is right approach, but I left some suggestions below which I think would make the implementation less awkward and more future-proof

src/kernel/chainstatemanager_opts.h

src/node/blockstorage.cpp

src/node/chainstatemanager_notifications.h

TheCharlatan · 2023-05-13T17:24:47Z

Thank you for the suggestions @ryanofsky!

Updated 0b159b9 -> 63e915a (chainstateRmKernelUiInterface_0 -> chainstateRmKernelUiInterface_1, compare).

Re-worked the commits as suggested by @ryanofsky's comment by changing the approach from passing around lambdas to using polymorphisms of a virtual class.
Addressed @ryanofsky's comment, renaming chainstatemanager_notifications to validation_notifications.
Re-worked the last commit as suggested by @ryanofsky's comment, by moving away from wrapping InitError to implementing a FatalError method, replacing the AbortNode function in shutdown. This is also a similar approach to the existing FatalErrorf method in index/base.cpp
Added a commit renaming FatalError to FatalErrorf in index/base.cpp to better distinguish it from the new FatalError function and not let the format string linter trip up on it.
Made the ChainstateManagerOpts notification member a shared_ptr in order to allow the option struct to be safely re-used for multiple ChainstateManagers

hebasto · 2023-05-14T10:53:01Z

Concept ACK.

hebasto

Approach ACK 63e915a.

The approach implemented here introduces some indirection, which makes the code a bit harder to read.

As for me, it does not look like a problem at all. Good function naming works well :D

In commit 7355679:

--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4659,7 +4659,7 @@ void Chainstate::LoadExternalBlockFile(
                     }
                 }
 
-                NotifyHeaderTip(*this);
+                ::NotifyHeaderTip(*this);
 
                 if (!blocks_with_unknown_parent) continue;

Also mind considering the IWYU output regarding src/node/validation_notifications.{h,cpp}?

src/init.cpp

TheCharlatan · 2023-05-14T13:40:02Z

Re #27636 (review)

In commit 7355679:

--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4659,7 +4659,7 @@ void Chainstate::LoadExternalBlockFile(
                     }
                 }
 
-                NotifyHeaderTip(*this);
+                ::NotifyHeaderTip(*this);
 
                 if (!blocks_with_unknown_parent) continue;

I added the global scope resolution where it was required, because of the naming conflict with the ChainstateManager method. These here are methods of Chainstate and don't have a naming conflict. Is adding them good practice for consistency's sake?

hebasto · 2023-05-14T14:52:10Z

src/kernel/validation_notifications_interface.h

+    virtual void notifyBlockTip(SynchronizationState state, CBlockIndex* index) = 0;
+    virtual void notifyHeaderTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync) = 0;
+    virtual void showProgress(const std::string& title, int nProgress, bool resume_possible) = 0;
+    virtual void doWarning(const bilingual_str& warning) = 0;
+    virtual bool fatalError(const std::string& strMessage, bilingual_str user_message = {}) = 0;


Should these methods be non-const?

Yes, as far as I am aware, const virtual methods are bad practice. See: #25337 (comment).

src/kernel/validation_notifications_interface.h

TheCharlatan · 2023-05-14T15:35:43Z

Thank you for the review @hebasto,

Updated 63e915a -> 97b0e28 (chainstateRmKernelUiInterface_1 -> chainstateRmKernelUiInterface_2, compare).

Addressed @hebasto comment, adding EOL.
Addressed @hebasto comment, correcting initialization warning in the first commit.
Addressed @hebasto comment, correcting IWYU in last commit.
Slightly improved commit messages (s/option/options/).

hebasto

Approach ACK 97b0e28.

In d84952c commit message:
s/"to FatalError"/"to FatalErrorf"/

Maybe make it a scripted-diff?

hebasto · 2023-05-14T16:53:02Z

nit: https://api.cirrus-ci.com/v1/task/4542005133443072/logs/ci.log:

node/validation_notifications.h should add these lines:
enum class SynchronizationState;

node/validation_notifications.cpp should add these lines:
enum class SynchronizationState;

node/validation_notifications.cpp should remove these lines:
- #include <kernel/validation_notifications_interface.h>

stickies-v

Code Review ACK 7d3b350

I'm still familiarising myself with the libbitcoinkernel project so for now I've mostly focused on the code being correct, readable, ... and not so much on architecture (which is arguably the most important thing here) etc. All the changes made sense, and are very well structured and readable.

I've left a few nits (+here) mostly regarding includes, all of them can be safely ignored and definitely shouldn't stand in the way of this PR making progress.

~~general nit: could we stick to camelcase naming for new functions, e.g. Notifications::headerTip() -> Notification::HeaderTip()?~~ (edit: TheCharlatan pointed me to this section in the developer notes which I wasn't aware of: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#internal-interface-naming-style)

src/bitcoin-chainstate.cpp

src/kernel/notifications_interface.h

src/node/kernel_notifications.h

src/node/blockstorage.cpp

src/test/fuzz/system.cpp

TheCharlatan · 2023-05-25T14:18:42Z

Re #27636 (review)

Thank you for the review!

all of them can be safely ignored and definitely shouldn't stand in the way of this PR making progress.

I'll fix these if I have to push again, otherwise will address them in follow ups.

maflcko · 2023-05-25T14:22:05Z

re-ACK 7d3b350 (no change) 🎋

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e (no change) 🎋
KyefeMll325W9RWk14PPm6bu59qHDnXgwGbMBKjFGB9wmlCtLMlA81b/7AQmrnMQk3tKDPrwtNU3/dBtq+dvDg==

hebasto

re-ACK 7d3b350, only last two commits dropped since my recent review.

…interfa…

@ryanofsky

…ion code. 6eb33bd kernel: Add fatalError method to notifications (TheCharlatan) 7320db9 kernel: Add flushError method to notifications (TheCharlatan) 3fa9094 scripted-diff: Rename FatalError to FatalErrorf (TheCharlatan) edb55e2 kernel: Pass interrupt reference to chainman (TheCharlatan) e2d680a util: Add SignalInterrupt class and use in shutdown.cpp (TheCharlatan) Pull request description: Get rid of all `ShutdownRequested` calls in validation code by introducing an interrupt object that applications can use to cancel long-running kernel operations. Replace all `AbortNode` calls in validation code with new fatal error and flush error notifications so kernel applications can be notified about failures and choose how to handle them. --- This pull request is part of the `libbitcoinkernel` project #27587 https://github.com/orgs/bitcoin/projects/3 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". The pull request mostly allows dropping the kernel dependency on shutdown.cpp. The only dependency left after this is a `StartShutdown` call which will be removed in followup PR #27711. This PR also drops the last reference to the `uiInterface` global in kernel code. The process of moving the `uiInterface` out of the kernel was started in #27636. This pull request contains a subset of patches originally proposed in #27711. It will be part of a series of changes required to make handling of interrupts (or in other words the current shutdown procedure) in the kernel library more transparent and less reliable on global mutable state. The set of patches contained here was originally proposed by @ryanofsky [here](#27711 (comment)). ACKs for top commit: achow101: light ACK 6eb33bd hebasto: ACK 6eb33bd, I have reviewed the code and it looks OK. ryanofsky: Code review ACK 6eb33bd. No changes since last review other than rebase. Tree-SHA512: 7d2d05fa4805428a09466d43c11ae32946cbb25aa5e741b1eec9cd142e4de4bb311e13ebf1bb125ae490c9d08274f2d56c93314e10f3d69e7fec7445e504987c

The following changes considered: - bitcoin/bitcoin#27419 - bitcoin/bitcoin#27491 - bitcoin/bitcoin#27576 - bitcoin/bitcoin#27636

Pull request description: Sync with the main repo up to the latest bitcoin/bitcoin@ab42b2e, which includes the recent changes in the CI. There is no downloadable artifacts support for now. It will be done in a separated PR(s). Additionally: - The code was adjusted to reflect changes from [PR27419](bitcoin/bitcoin#27419), [PR27491](bitcoin/bitcoin#27491), [PR27576](bitcoin/bitcoin#27576) and [PR27636](bitcoin/bitcoin#27636). - Fixed `modernize-use-default-member-init` clang-tidy warnings. - The ARM task has been temporarily disabled until the issue with the depends cache is resolved. Guix builds: ``` e92b8c4c3298165edb1a0e85ee516d52c81af1269405dcbc6520e63069de2363 guix-build-b3261144c892/output/aarch64-linux-gnu/SHA256SUMS.part 939c6c002490d5649bdbfabacd20cd2270b41b20b7b3a254c9fcd5780209900d guix-build-b3261144c892/output/aarch64-linux-gnu/bitcoin-b3261144c892-aarch64-linux-gnu-debug.tar.gz b3c1383fb394997378997bdd2933965cf4ecc694143b4703108ff6ecb946696c guix-build-b3261144c892/output/aarch64-linux-gnu/bitcoin-b3261144c892-aarch64-linux-gnu.tar.gz f43fedf3af666d35e83b84e63cfe19f315f74f01296982f47d8c159385c3b03c guix-build-b3261144c892/output/arm-linux-gnueabihf/SHA256SUMS.part 73b89b0487e8eee474a6c9c96ae0e7ad635cccc332fc062eb5d4ff5555356c3e guix-build-b3261144c892/output/arm-linux-gnueabihf/bitcoin-b3261144c892-arm-linux-gnueabihf-debug.tar.gz b4518dd9396f316de8d7de5181b8b5d1083e0afa9081625c37117472d2559380 guix-build-b3261144c892/output/arm-linux-gnueabihf/bitcoin-b3261144c892-arm-linux-gnueabihf.tar.gz 0213e754408e2a032cef61a946354656f5b5f755f85aeac1ce4b37f1d22528e6 guix-build-b3261144c892/output/arm64-apple-darwin/SHA256SUMS.part 11bc1be1f53dad337565f3c556dd69abc2d702a31e661359daad6ff89225c794 guix-build-b3261144c892/output/arm64-apple-darwin/bitcoin-b3261144c892-arm64-apple-darwin-unsigned.dmg 558d8e805420c7a348759df6f559ca349953646aa28840efafe5a3d245ea917f guix-build-b3261144c892/output/arm64-apple-darwin/bitcoin-b3261144c892-arm64-apple-darwin-unsigned.tar.gz e679ce3f1c80aff11a5eab8890efbd0d396a851875fbd6f93f32eef5cdf06813 guix-build-b3261144c892/output/arm64-apple-darwin/bitcoin-b3261144c892-arm64-apple-darwin.tar.gz 0cb346390dc6620593b1af5b6669ddc3c1a8d2219a51b1697747c5ab24069c27 guix-build-b3261144c892/output/dist-archive/bitcoin-b3261144c892.tar.gz ac8bd2d58d9d0ebe2da1c8efa2d57bd97c3ef2b2590c758edbc4919808c528c5 guix-build-b3261144c892/output/powerpc64-linux-gnu/SHA256SUMS.part cdf8252fa8aca6da61ff6926de5c7e2e6560ab046049c84c26ba44823f83236a guix-build-b3261144c892/output/powerpc64-linux-gnu/bitcoin-b3261144c892-powerpc64-linux-gnu-debug.tar.gz 3b8b5f53d365b5bf962ecd7def9f06b6f13af0e5c9ef69c6d028f1ed772459be guix-build-b3261144c892/output/powerpc64-linux-gnu/bitcoin-b3261144c892-powerpc64-linux-gnu.tar.gz b44e688d233dcb46a7d6d0b1d97979335d3cc559d16190cc5cd647add79298d2 guix-build-b3261144c892/output/powerpc64le-linux-gnu/SHA256SUMS.part ae5c19afefd523cdc171a3f9aa9f707870fd99749c01c01166086619dfd95ece guix-build-b3261144c892/output/powerpc64le-linux-gnu/bitcoin-b3261144c892-powerpc64le-linux-gnu-debug.tar.gz bb581b1444fa1686f8889248af13d1859f2915091cd640bc522185d5ad83e13d guix-build-b3261144c892/output/powerpc64le-linux-gnu/bitcoin-b3261144c892-powerpc64le-linux-gnu.tar.gz bdca0a3c19b5a9a5c72b2b43b07050678d960009d3fa80cf7e0689d508346974 guix-build-b3261144c892/output/riscv64-linux-gnu/SHA256SUMS.part b0b9c91abe2ad0b5ab3b0bfd10c90133d8d75b50aef0a6a98ac2c2ae4219eaa8 guix-build-b3261144c892/output/riscv64-linux-gnu/bitcoin-b3261144c892-riscv64-linux-gnu-debug.tar.gz fcce0ea00f1d9df136dd677cbc468183faa92bd4bfcd4a77cd1c70f1b894b5f0 guix-build-b3261144c892/output/riscv64-linux-gnu/bitcoin-b3261144c892-riscv64-linux-gnu.tar.gz 7be84969950bb9570522be5a37551c01698cd3fb65eca3988fc9bd6867460552 guix-build-b3261144c892/output/x86_64-apple-darwin/SHA256SUMS.part 25203f50aa6a344ad1c6c4a44a48082440bb0af9bf38f0d60506569f216d1672 guix-build-b3261144c892/output/x86_64-apple-darwin/bitcoin-b3261144c892-x86_64-apple-darwin-unsigned.dmg 16c5baaf6d00ed43b0611c86c2d4555d500b3896daa1daac6a567bc2611c39f6 guix-build-b3261144c892/output/x86_64-apple-darwin/bitcoin-b3261144c892-x86_64-apple-darwin-unsigned.tar.gz 86662f39c29b013b576e6555ecb6cbbc98eaa08532a541e22a7ed6b1baf87209 guix-build-b3261144c892/output/x86_64-apple-darwin/bitcoin-b3261144c892-x86_64-apple-darwin.tar.gz fbbc0ad2376431fdc5b214fd63f24a6da907d87f6f11e0833def50c0d45772cd guix-build-b3261144c892/output/x86_64-linux-gnu/SHA256SUMS.part cba8d700f746a6063809570e45d6dc3d5e60ad5f1a28e0f41f8beed8b546a7b1 guix-build-b3261144c892/output/x86_64-linux-gnu/bitcoin-b3261144c892-x86_64-linux-gnu-debug.tar.gz 0a32985a1e26e13ce883a85e4a92cc68bf51ce096f2f6d74ea499a9fa662d7d0 guix-build-b3261144c892/output/x86_64-linux-gnu/bitcoin-b3261144c892-x86_64-linux-gnu.tar.gz 0bd4cc64cd6ad733cdef87cd74d5034e79dd250b72795cebf9c2c63500509457 guix-build-b3261144c892/output/x86_64-w64-mingw32/SHA256SUMS.part 6ed8f2e6c6cf1992d156672707cd2c254754051f88223dd052a9cd9078d84789 guix-build-b3261144c892/output/x86_64-w64-mingw32/bitcoin-b3261144c892-win64-debug.zip 1ea6d7660652e20b2b1529e406be1f606745d35f6a179b006335a19a19aa9a5b guix-build-b3261144c892/output/x86_64-w64-mingw32/bitcoin-b3261144c892-win64-setup-unsigned.exe 41b0f8cbac614e8c555921de60b25a73a75e6bed025de98ca40d3db48c5db6b1 guix-build-b3261144c892/output/x86_64-w64-mingw32/bitcoin-b3261144c892-win64-unsigned.tar.gz 5c68d711782e76f9e4be93b5468c505f022b72ca299532b200e58fe1e51343b1 guix-build-b3261144c892/output/x86_64-w64-mingw32/bitcoin-b3261144c892-win64.zip ``` Top commit has no ACKs. Tree-SHA512: dd18cfb2cfd6fd45b35bef8a0397bccc0752ce946b304bae986006ff09a9a183d6222b0f607e4dd3373992814ae0e61d5ba63cb54fef9a288152edef3d7ea81d

…e time (#5693) ## Issue being fixed or feature implemented Some headers include other heavy headers, such as `logging.h`, `tinyformat.h`, `iostream`. These headers are heavy and increase compilation time on scale of whole project drastically because can be used in many other headers. ## What was done? Moved many heavy includes from headers to cpp files to optimize compilation time. In some places added forward declarations if it is reasonable. As side effect removed 2 circular dependencies: ``` "llmq/debug -> llmq/dkgsessionhandler -> llmq/debug" "llmq/debug -> llmq/dkgsessionhandler -> llmq/dkgsession -> llmq/debug" ``` ## How Has This Been Tested? Run build 2 times before refactoring and after refactoring: `make clean && sleep 10s; time make -j18` Before refactoring: ``` real 5m37,826s user 77m12,075s sys 6m20,547s real 5m32,626s user 76m51,143s sys 6m24,511s ``` After refactoring: ``` real 5m18,509s user 73m32,133s sys 6m21,590s real 5m14,466s user 73m20,942s sys 6m17,868s ``` ~5% of improvement for compilation time. That's not huge, but that's worth to get merged There're several more refactorings TODO but better to do them later by backports: - bitcoin#27636 - bitcoin#26286 - bitcoin#27238 - and maybe this one: bitcoin#28200 ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone

Summary: This commit is part of the libbitcoinkernel project and seeks to remove the ChainstateManager's and, more generally, the kernel library's dependency on interface_ui with options methods in this and the following few commits. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. Define a new kernel notification class with virtual methods for notifying about internal kernel events. Create a new file in the node library for defining a function creating the default set of notification methods such that these do not need to be re-defined all over the codebase. As a first step, add a `blockTip` method, wrapping `uiInterface.NotifyBlockTip`. This is a backport of [[bitcoin/bitcoin#27636 | core#27636]] bitcoin/bitcoin@447761c Test Plan: With bitcoin-chainstate: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D16204

Summary: This commit is part of the libbitcoinkernel project and seeks to remove the ChainstateManager's and, more generally, the kernel library's dependency on interface_ui with options methods in this and the following few commits. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. This is a partial backport of [[bitcoin/bitcoin#27636 | core#27636]] bitcoin/bitcoin@84d7145 Depends on D16204 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D16205

Summary: This commit is part of the libbitcoinkernel project and seeks to remove the ChainstateManager's and, more generally, the kernel library's dependency on interface_ui with options methods in this and the following few commits. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. This is a partial backport of [[bitcoin/bitcoin#27636 | core#27636]] bitcoin/bitcoin@4452707 Depends on D16205 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D16206

Summary: This commit is part of the libbitcoinkernel project and seeks to remove the ChainstateManager's and, more generally, the kernel library's dependency on interface_ui with options methods in this and the following few commits. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. The AlertNotify function is moved out of the validation.cpp file, which removes its dependency on interface_ui as well as util/system This is a partial backport of [[bitcoin/bitcoin#27636 | core#27636]] bitcoin/bitcoin@f871c69 Depends on D16206 Notes: - I removed the util/system include from validation in D16151, but it was actually needed for `void runCommand(const std::string &strCommand)`. After this diff, we actually no longer need it in validation.cpp - we don't use AlertNotify for the same thing, so there are minor differences: we never backported [[bitcoin/bitcoin#10464 | core#10464]], we don't use a bilingual_str, we don't call SetMiscWarning so we don't need warning.h in kernel_notification.cpp Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D16207

Summary: With the previous move of AlertNotify out of the validation file, and thus out of the kernel library, ScheduleBatchPriority is the last remaining function used by the kernel library from util/system. Move it to its own file, such that util/system can be moved out of the util library in the following few commits. Moving util/system out of the kernel library removes further networking as well as shell related code from it. This is a partial backport of [[bitcoin/bitcoin#27636 | core#27636]] bitcoin/bitcoin@9ec5da3 Depends on D16207 Test Plan: With bitcoin-chainstate `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D16208

Summary: It makes sense to keep util::insert and util::AnyPtr functions in util/, not common/ as these are header-only template functions with no dependencies, and there's no particular reason they shouldn't be used by kernel code even if they aren't used right now. This is a partial backport of [[bitcoin/bitcoin#27636 | core#27636]] bitcoin/bitcoin@44de325 Depends on D16210 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D16213

Summary: It makes sense to keep util::insert and util::AnyPtr functions in util/, not common/ as these are header-only template functions with no dependencies, and there's no particular reason they shouldn't be used by kernel code even if they aren't used right now. This is a partial backport of [[bitcoin/bitcoin#27636 | core#27636]] bitcoin/bitcoin@7eee356 Depends on D16213 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D16214

Summary: Since the kernel library no longer depends on the system file, move it to the common library instead in accordance to the diagram in doc/design/libraries.md. This is a move-only commit. This concludes backport of [[bitcoin/bitcoin#27636 | core#27636]] bitcoin/bitcoin@7d3b350 Depends on D16216 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D16217

DrahtBot added the Validation label May 12, 2023

fanquake requested review from ryanofsky and theuni May 12, 2023 08:55

ryanofsky approved these changes May 12, 2023

View reviewed changes

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

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

src/node/chainstatemanager_notifications.h Outdated Show resolved Hide resolved

Aminkavoos approved these changes May 12, 2023

View reviewed changes

TheCharlatan force-pushed the chainstateRmKernelUiInterface branch from 0b159b9 to 63e915a Compare May 13, 2023 17:22

This was referenced May 14, 2023

rpc: Add importmempool RPC #27460

Merged

index: initial sync speedup, parallelize process #26966

Open

Add fee_est tool for debugging fee estimation code #10443

Closed

hebasto reviewed May 14, 2023

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

hebasto reviewed May 14, 2023

View reviewed changes

TheCharlatan force-pushed the chainstateRmKernelUiInterface branch from 63e915a to 97b0e28 Compare May 14, 2023 15:35

hebasto reviewed May 14, 2023

View reviewed changes

TheCharlatan force-pushed the chainstateRmKernelUiInterface branch from 97b0e28 to 2f9c2d2 Compare May 14, 2023 17:57

TheCharlatan changed the title ~~kernel: Remove interface_ui, util/system from kernel library~~ kernel: Remove util/system from kernel library, interface_ui from validation. May 24, 2023

stickies-v reviewed May 24, 2023

View reviewed changes

src/bitcoin-chainstate.cpp Show resolved Hide resolved

src/kernel/notifications_interface.h Show resolved Hide resolved

src/node/kernel_notifications.h Show resolved Hide resolved

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

src/test/fuzz/system.cpp Show resolved Hide resolved

DrahtBot requested review from hebasto and maflcko May 24, 2023 15:56

maflcko requested a review from theuni May 25, 2023 14:20

DrahtBot removed the request for review from maflcko May 25, 2023 14:22

TheCharlatan mentioned this pull request May 25, 2023

Bitcoin Kernel Library Project Tracking #27587

Open

2 tasks

hebasto approved these changes May 29, 2023

View reviewed changes

fanquake merged commit 9564f98 into bitcoin:master May 30, 2023

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 30, 2023

Merge bitcoin#27636: kernel: Remove util/system from kernel library, …

46edd31

…interfa…

TheCharlatan mentioned this pull request Jun 12, 2023

kernel: Rm ShutdownRequested and AbortNode from validation code. #27861

Merged

hebasto added a commit to hebasto/gui-qml that referenced this pull request Aug 29, 2023

Adjust code according to changes in the main repository

592da16

The following changes considered: - bitcoin/bitcoin#27419 - bitcoin/bitcoin#27491 - bitcoin/bitcoin#27576 - bitcoin/bitcoin#27636

hebasto added a commit to hebasto/gui-qml that referenced this pull request Aug 29, 2023

Adjust code according to changes in the main repository

c77e8b5

The following changes considered: - bitcoin/bitcoin#27419 - bitcoin/bitcoin#27491 - bitcoin/bitcoin#27576 - bitcoin/bitcoin#27636

hebasto added a commit to hebasto/gui-qml that referenced this pull request Aug 29, 2023

Adjust code according to changes in the main repository

8b02563

The following changes considered: - bitcoin/bitcoin#27419 - bitcoin/bitcoin#27491 - bitcoin/bitcoin#27576 - bitcoin/bitcoin#27636

hebasto mentioned this pull request Aug 30, 2023

Sync with the main repo bitcoin-core/gui-qml#359

Merged

knst mentioned this pull request Nov 13, 2023

refactor: re-order headers and forward declarations to improve compile time dashpay/dash#5693

Merged

5 tasks

bitcoin locked and limited conversation to collaborators May 29, 2024

kernel: Remove util/system from kernel library, interface_ui from validation. #27636

kernel: Remove util/system from kernel library, interface_ui from validation. #27636

Uh oh!

Conversation

TheCharlatan commented May 12, 2023

Uh oh!

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

Uh oh!

Reviews

Conflicts

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TheCharlatan commented May 13, 2023

Uh oh!

hebasto commented May 14, 2023

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheCharlatan commented May 14, 2023

Uh oh!

hebasto May 14, 2023

Choose a reason for hiding this comment

Uh oh!

TheCharlatan May 14, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheCharlatan commented May 14, 2023

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto commented May 14, 2023

Uh oh!

stickies-v left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TheCharlatan commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented May 25, 2023

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DrahtBot commented May 12, 2023 •

edited

Loading

stickies-v left a comment •

edited

Loading

TheCharlatan commented May 25, 2023 •

edited

Loading