From c8e332cb33594cc307cdf89b6036a0e95c238cd8 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 24 Feb 2026 08:31:38 -0500 Subject: [PATCH 1/3] init refactor: Remove node.init accesss in AppInitInterfaces There's no change in behavior. This is just a refactoring to avoid a minor layer violation in init code. The node.init object is intended to return interface pointers for code outside the node (like wallet and gui code), not used by node itself to initialize its internal state. (Motivation for this change is to introduce a MakeMining wait_loaded option in an upcoming commit that can only be used internally and not set by external clients.) --- src/init.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index adc1dacc757..bdff6863be0 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1206,8 +1206,8 @@ bool AppInitLockDirectories() bool AppInitInterfaces(NodeContext& node) { - node.chain = node.init->makeChain(); - node.mining = node.init->makeMining(); + node.chain = interfaces::MakeChain(node); + node.mining = interfaces::MakeMining(node); return true; } From a7cabf92e4de83c87f6b9274ddd2fb70712d29f8 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 24 Feb 2026 10:15:14 -0500 Subject: [PATCH 2/3] init refactor: Only initialize node.notifications one time Instead of having the InitAndLoadChainstate function delete and create the KernelNotifications object each time it is called (it can be called twice when reindexing) to clear cached state, create it just one time and add a setChainstateLoaded() method to manage state as it is loaded and unloaded. This refactoring should make sense by itself to be more explicit about how KernelNotifications state is cleared, but it's also needed to make outside code accessing KernelNotifications state (currently just mining code) safe during node startup and shutdown so the KernelNofications mutex can be used for synchronization and does not get recreated itself. --- src/init.cpp | 14 +++++++++----- src/node/kernel_notifications.cpp | 4 ++-- src/node/kernel_notifications.h | 22 ++++++++++++++++++++-- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index bdff6863be0..9d7c76a872f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1304,16 +1304,12 @@ static ChainstateLoadResult InitAndLoadChainstate( const ArgsManager& args) { // This function may be called twice, so any dirty state must be reset. - node.notifications.reset(); // Drop state, such as a cached tip block + node.notifications->setChainstateLoaded(false); // Drop state, such as a cached tip block node.mempool.reset(); node.chainman.reset(); // Drop state, such as an initialized m_block_tree_db const CChainParams& chainparams = Params(); - Assert(!node.notifications); // Was reset above - node.notifications = std::make_unique(Assert(node.shutdown_request), node.exit_status, *Assert(node.warnings)); - ReadNotificationArgs(args, *node.notifications); - CTxMemPool::Options mempool_opts{ .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0, .signals = node.validation_signals.get(), @@ -1414,6 +1410,7 @@ static ChainstateLoadResult InitAndLoadChainstate( std::tie(status, error) = catch_exceptions([&] { return VerifyLoadedChainstate(chainman, options); }); if (status == node::ChainstateLoadStatus::SUCCESS) { LogInfo("Block index and chainstate loaded"); + node.notifications->setChainstateLoaded(true); } } return {status, error}; @@ -1486,6 +1483,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.validation_signals = std::make_unique(std::make_unique(scheduler)); auto& validation_signals = *node.validation_signals; + // Create KernelNotifications object. Important to do this early before + // calling ipc->listenAddress() below so makeMining and other IPC methods + // can use this. + assert(!node.notifications); + node.notifications = std::make_unique(Assert(node.shutdown_request), node.exit_status, *Assert(node.warnings)); + ReadNotificationArgs(args, *node.notifications); + // Create client interfaces for wallets that are supposed to be loaded // according to -wallet and -disablewallet options. This only constructs // the interfaces, it doesn't load wallet data. Wallets actually get loaded diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp index f207b49a96b..ab0e5ccb69e 100644 --- a/src/node/kernel_notifications.cpp +++ b/src/node/kernel_notifications.cpp @@ -53,7 +53,7 @@ kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state { LOCK(m_tip_block_mutex); Assume(index.GetBlockHash() != uint256::ZERO); - m_tip_block = index.GetBlockHash(); + m_state.tip_block = index.GetBlockHash(); m_tip_block_cv.notify_all(); } @@ -103,7 +103,7 @@ void KernelNotifications::fatalError(const bilingual_str& message) std::optional KernelNotifications::TipBlock() { AssertLockHeld(m_tip_block_mutex); - return m_tip_block; + return m_state.tip_block; }; diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h index e9f1e8f1889..b90248bf0ab 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -29,6 +29,18 @@ namespace node { class Warnings; static constexpr int DEFAULT_STOPATHEIGHT{0}; +//! State tracked by the KernelNotifications interface meant to be used by +//! mining code, index code, RPCs, and other code sitting above the validation +//! layer. +//! +//! Currently just tracks the chain tip, but could be used to hold other +//! information in the future, like the last flushed block, pruning +//! information, etc. +struct KernelState { + bool chainstate_loaded{false}; + std::optional tip_block; +}; + class KernelNotifications : public kernel::Notifications { public: @@ -49,6 +61,13 @@ public: void fatalError(const bilingual_str& message) override; + void setChainstateLoaded(bool chainstate_loaded) EXCLUSIVE_LOCKS_REQUIRED(!m_tip_block_mutex) { + LOCK(m_tip_block_mutex); + if (!chainstate_loaded) m_state = {}; + m_state.chainstate_loaded = chainstate_loaded; + m_tip_block_cv.notify_all(); + } + //! Block height after which blockTip notification will return Interrupted{}, if >0. int m_stop_at_height{DEFAULT_STOPATHEIGHT}; //! Useful for tests, can be set to false to avoid shutdown on fatal error. @@ -56,6 +75,7 @@ public: Mutex m_tip_block_mutex; std::condition_variable m_tip_block_cv GUARDED_BY(m_tip_block_mutex); + KernelState m_state GUARDED_BY(m_tip_block_mutex); //! The block for which the last blockTip notification was received. //! It's first set when the tip is connected during node initialization. //! Might be unset during an early shutdown. @@ -65,8 +85,6 @@ private: const std::function& m_shutdown_request; std::atomic& m_exit_status; node::Warnings& m_warnings; - - std::optional m_tip_block GUARDED_BY(m_tip_block_mutex); }; void ReadNotificationArgs(const ArgsManager& args, KernelNotifications& notifications); From bbc8f1e0a7e5739f15b2e646a4ace338083309a3 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 24 Feb 2026 08:31:38 -0500 Subject: [PATCH 3/3] ipc mining: Prevent ``Assertion `m_node.chainman' failed`` errors on early startup This fixes ``Assertion `m_node.chainman' failed`` errors first reported https://github.com/bitcoin/bitcoin/issues/33994#issuecomment-3602551596 when IPC mining methods are called before ChainstateManager is loaded. The fix works by making the `Init.makeMining` method block until chainstate data is loaded. --- src/init.cpp | 4 +++- src/interfaces/mining.h | 6 ++++- src/node/interfaces.cpp | 14 +++++++++++- src/test/miner_tests.cpp | 2 +- src/test/testnet4_miner_tests.cpp | 2 +- test/functional/interface_ipc_mining.py | 29 +++++++++++++++++++++++++ 6 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 9d7c76a872f..6a6e7a925b2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1207,7 +1207,9 @@ bool AppInitLockDirectories() bool AppInitInterfaces(NodeContext& node) { node.chain = interfaces::MakeChain(node); - node.mining = interfaces::MakeMining(node); + // Specify wait_loaded=false so internal mining interface can be initialized + // on early startup and does not need to be tied to chainstate loading. + node.mining = interfaces::MakeMining(node, /*wait_loaded=*/false); return true; } diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 72cc0bcb67b..f4c42e204a7 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -160,7 +160,11 @@ public: }; //! Return implementation of Mining interface. -std::unique_ptr MakeMining(node::NodeContext& node); +//! +//! @param[in] wait_loaded waits for chainstate data to be loaded before +//! returning. Used to prevent external clients from +//! being able to crash the node during startup. +std::unique_ptr MakeMining(node::NodeContext& node, bool wait_loaded=true); } // namespace interfaces diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index afcaeb20c3e..6c61b2105d9 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -1017,5 +1017,17 @@ public: namespace interfaces { std::unique_ptr MakeNode(node::NodeContext& context) { return std::make_unique(context); } std::unique_ptr MakeChain(node::NodeContext& context) { return std::make_unique(context); } -std::unique_ptr MakeMining(node::NodeContext& context) { return std::make_unique(context); } +std::unique_ptr MakeMining(node::NodeContext& context, bool wait_loaded) +{ + if (wait_loaded) { + node::KernelNotifications& kernel_notifications(*Assert(context.notifications)); + util::SignalInterrupt& interrupt(*Assert(context.shutdown_signal)); + WAIT_LOCK(kernel_notifications.m_tip_block_mutex, lock); + kernel_notifications.m_tip_block_cv.wait(lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) { + return kernel_notifications.m_state.chainstate_loaded || interrupt; + }); + if (interrupt) return nullptr; + } + return std::make_unique(context); +} } // namespace interfaces diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 7f05e4e9d81..0ae8e538cb4 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -67,7 +67,7 @@ struct MinerTestingSetup : public TestingSetup { } std::unique_ptr MakeMining() { - return interfaces::MakeMining(m_node); + return interfaces::MakeMining(m_node, /*wait_loaded=*/false); } }; } // namespace miner_tests diff --git a/src/test/testnet4_miner_tests.cpp b/src/test/testnet4_miner_tests.cpp index 614d2fd63ac..d0aeebe0f21 100644 --- a/src/test/testnet4_miner_tests.cpp +++ b/src/test/testnet4_miner_tests.cpp @@ -22,7 +22,7 @@ namespace testnet4_miner_tests { struct Testnet4MinerTestingSetup : public Testnet4Setup { std::unique_ptr MakeMining() { - return interfaces::MakeMining(m_node); + return interfaces::MakeMining(m_node, /*wait_loaded=*/false); } }; } // namespace testnet4_miner_tests diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index 5095583a0e2..b741beeb5b0 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -147,6 +147,34 @@ class IPCMiningTest(BitcoinTestFramework): asyncio.run(capnp.run(async_routine())) + def run_early_startup_test(self): + """Make sure mining.createNewBlock safely returns on early startup as + soon as mining interface is available """ + self.log.info("Running Mining interface early startup test") + + node = self.nodes[0] + self.stop_node(node.index) + node.start() + + async def async_routine(): + while True: + try: + ctx, mining = await self.make_mining_ctx() + break + except (ConnectionRefusedError, FileNotFoundError): + # Poll quickly to connect as soon as socket becomes + # available but without using a lot of CPU + await asyncio.sleep(0.005) + + opts = self.capnp_modules['mining'].BlockCreateOptions() + await mining.createNewBlock(ctx, opts) + + asyncio.run(capnp.run(async_routine())) + + # Reconnect nodes so next tests are happy + node.wait_for_rpc_connection() + self.connect_nodes(1, 0) + def run_block_template_test(self): """Test BlockTemplate interface methods.""" self.log.info("Running BlockTemplate interface test") @@ -374,6 +402,7 @@ class IPCMiningTest(BitcoinTestFramework): self.miniwallet = MiniWallet(self.nodes[0]) self.default_block_create_options = self.capnp_modules['mining'].BlockCreateOptions() self.run_mining_interface_test() + self.run_early_startup_test() self.run_block_template_test() self.run_coinbase_and_submission_test() self.run_ipc_option_override_test()