Page MenuHomePhabricator

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

Authored by PiRK on Feb 28 2025, 15:25.

Details

Summary

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().

Initialize globals with kernel::Context's life 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".

kernel: SanityChecks: Return an error struct
This reduces libbitcoinkernel's coupling with ui_interface and translation.

This is a backport of core#25065

Test Plan

ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Feb 28 2025, 15:25
Fabien requested changes to this revision.Mon, Mar 3, 09:29
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/init/common.cpp
49 ↗(On Diff #52850)

This check disappeared.

It's possible it's no longer needed, but please check if that's the case and remove it in its own diff.

src/kernel/checks.cpp
13 ↗(On Diff #52850)

Any reason util::Result is not used ?

This revision now requires changes to proceed.Mon, Mar 3, 09:29
src/init/common.cpp
49 ↗(On Diff #52850)

nice catch
TODO core#25233

src/kernel/checks.cpp
13 ↗(On Diff #52850)

good catch, we backported core#25977 before this PR, so we should apply the former

depend on D17735 to address one of the review items

The other item will be done in a follow-up by backporting core#25308 with core#25977 in mind

This revision is now accepted and ready to land.Mon, Mar 3, 14:31