diff --git a/src/init.cpp b/src/init.cpp index adc1dacc757..6a6e7a925b2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1206,8 +1206,10 @@ bool AppInitLockDirectories() bool AppInitInterfaces(NodeContext& node) { - node.chain = node.init->makeChain(); - node.mining = node.init->makeMining(); + node.chain = interfaces::MakeChain(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; } @@ -1304,16 +1306,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 +1412,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 +1485,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/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/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); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index ca1700495e6..6b4e85a59e4 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -68,7 +68,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()