Page MenuHomePhabricator

[chronik] get consensus params via chainman
ClosedPublic

Authored by PiRK on May 6 2024, 12:27.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCfdaa22f51a9b: [chronik] get consensus params via chainman
Summary

The ChainstateManager has access to the config, so no need to pass it to ChronikBridge when it has already a NodeContext. This removes one GetConfig() call and is a step towards unglobalizing the Config.

Test Plan
cmake .. -GNinja -DBUILD_BITCOIN_CHRONIK=ON
ninja all check-all

Event Timeline

PiRK requested review of this revision.May 6 2024, 12:27
Fabien added inline comments.
chronik/chronik-cpp/chronik_bridge.cpp
199 ↗(On Diff #47632)

We should have an assert that the pointer is not null. It's not done anywhere else so maybe there is one at an upper level ?

chronik/chronik-cpp/chronik_bridge.cpp
199 ↗(On Diff #47632)

I will check if there is one. The rest of the chronik code seems to assume that chainman is not nullptr.

add an Assert(node.chainman) to ensure the chainman is not nullptr, as the ChronikBridge code heavily depends on that.
Note that in the current state of the codebase this is already done in init.cpp before calling chronik::Start(config, node, fReindexChronik), but it doesn't hurt to check again for all case where ChronikBridge is instantiated directly (unit tests)

This revision is now accepted and ready to land.May 6 2024, 13:56
This revision was landed with ongoing or failed builds.May 6 2024, 14:13
This revision was automatically updated to reflect the committed changes.