mining: add block template manager #35675

ismaelsadeeq commented at 11:05 AM on July 7, 2026: member

This PR introduces node::BlockTemplateManager and moves block template creation, submission, and mining wait helpers behind it.

Motivation

Instead of keeping template-related state and helper functions spread across NodeContext, miner.cpp, the mining interface, RPC, and tests, the manager now owns the node's init-time mining options and exposes methods needed by callers.

This also prevents some redundant copies previously done when using the mining interface to create a block template and then retrieve the template data.

This keeps the IPC Mining interface focused on IPC-facing mining objects, while RPC and tests use the block template manager directly.

Changes

NodeContext no longer stores BlockCreateOptions directly.
BlockTemplateManager stores the parsed init-time mining options and applies them to unset per-call options before creating templates.
Block submission through the mining interface is routed through BlockTemplateManager::SubmitBlock(), preserving the existing BlockChecked state-capture behavior.
Tip lookup, tip waiting, cooldown, and waitNext() template creation helpers are moved from miner helper functions into BlockTemplateManager.
In-process RPC and tests create raw CBlockTemplate objects directly through BlockTemplateManager, avoiding cached BlockTemplateImpl objects that hold NodeContext references during shutdown.
A fuzz target is added for BlockTemplateManager::CreateNewTemplate() with fuzzed mempool contents and mining options.

Note: This change is intended to be a pure refactor that preserves behavior.

DrahtBot added the label Mining on Jul 7, 2026

DrahtBot commented at 11:06 AM on July 7, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35675.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	Sjors, pablomartin4btc

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35671 (mining: add TxCollection to bandwidth-efficiently validate external block templates by Sjors)
#35646 (RFC: Separate out runtime errors from BlockValidationState using util::Expected by yuvicc)
#35581 (node: add block template manager and track waitNext fee inflow by ismaelsadeeq)
#35482 (fuzz: exercise the transaction-handling path in process_message(s) by HowHsu)
#35474 (node: move index ownership to NodeContext by w0xlt)
#34672 (mining: add reason/debug to submitSolution and unify with submitBlock by w0xlt)
#34565 (refactor: extract BlockDownloadManager from PeerManagerImpl by w0xlt)
#34075 (fees: Introduce Mempool Based Fee Estimation to reduce overestimation by ismaelsadeeq)
#32468 (rpc: generateblock to allow multiple outputs by polespinasa)
#31117 (miner: Reorg Testnet4 minimum difficulty blocks by fjahr)

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.

ismaelsadeeq commented at 11:06 AM on July 7, 2026: member

CC @Sjors @pablomartin4btc @w0xlt as discussed in #35581

Sjors commented at 3:49 PM on July 7, 2026: member

Concept ACK

I updated https://github.com/Sjors/bitcoin/pull/120 to build on this branch.

Will review later.

Sjors commented at 4:47 PM on July 7, 2026: member

Please rebase, so I can make #33922 cleanly based on this (it conflicts with #35129 and #34020).

node: introduce block template manager

Add BlockTemplateManager as an encapsulation
that wraps BlockAssembler::CreateNewBlock().

Add a simple unit test that verifies that block
template can be created via the block template manager.

1aa695a6f0

node: move mining_args to block template manager

Pass the parsed mining args to BlockTemplateManager at construction and
expose them via GetInitBlockCreateOptions(), so the manager owns the
init-time block create options instead of NodeContext.

610d5d704c

miner: move SubmitBlock into BlockTemplateManager

Move SubmitBlockStateCatcher and SubmitBlock from miner.cpp into
BlockTemplateManager as a member function. This groups block submission
with block creation in the same class. The function uses m_chainman
directly instead of taking it as a parameter.

00e3b67172

node: move tip and wait helpers into BlockTemplateManager

Move mining tip lookup and block-template waiting helpers into
BlockTemplateManager so the manager owns the template waiting flow,
including the notification wait helpers used by waitNext.

e2c0b97f9f

interfaces: create block template via block template manager

Route the Mining interface's createNewBlock() through
BlockTemplateManager::CreateNewTemplate() instead of constructing a
BlockAssembler directly.

5071074631

rpc, test: build block templates via BlockTemplateManager

Use BlockTemplateManager directly for in-process RPC and test block
template creation instead of going through the IPC Mining interface.

This keeps RPC-created templates untracked and prevents process-static RPC
caches from owning BlockTemplateImpl objects whose destructor reaches back
into NodeContext during shutdown.

231fa5ad4f

test: add block template manager fuzz test

Add a fuzz target that creates block templates through
BlockTemplateManager under fuzzed mining options and asserts a template
is always produced.

149baad220

ismaelsadeeq force-pushed on Jul 8, 2026

ismaelsadeeq commented at 11:37 AM on July 8, 2026: member

Please rebase, so I can make #33922 cleanly based on this (it conflicts with #35129 and #34020).

Done.

Sjors commented at 2:11 PM on July 8, 2026: member

Commit 231fa5ad4f71e652c3c7a52a88f540d47dad29b7 rpc, test: build block templates via BlockTemplateManager is rather large. Maybe have one commit per RPC method, or at least do the critical getblocktemplate in a separate commit.

After this change, does node.mining still do anything?

Consider adding the new files to FILES_WITH_ENFORCED_IWYU since their ancestors were enforced too.

in src/node/block_template_manager.h:18 in 1aa695a6f0

  13 | +class CTxMemPool;
  14 | +
  15 | +namespace node {
  16 | +struct CBlockTemplate;
  17 | +
  18 | +/** Creates block templates. */

Sjors commented at 3:23 PM on July 8, 2026:

In 1aa695a6f08a6824f5577ce8de3e5fe6578daa3b node: introduce block template manager: in this PR the block template manager doesn't really live up to its name. We should clarify what it does, e.g.:

Creates block templates, submits solved blocks, and provides tip-waiting helpers for mining code. Owns the init-time mining options. Currently stateless — templates it returns are owned by the caller; template tracking is added in followups (see #33758).

in src/test/fuzz/cmpctblock.cpp:116 in 1aa695a6f0

 112 | @@ -112,13 +113,16 @@ void ResetChainmanAndMempool(TestingSetup& setup)
 113 |      SetMockTime(Params().GenesisBlock().Time());
 114 |  
 115 |      bilingual_str error{};
 116 | -    setup.m_node.mempool.reset();
 117 | -    setup.m_node.mempool = std::make_unique<CTxMemPool>(MemPoolOptionsForTest(setup.m_node), error);
 118 | +    auto& node = setup.m_node;

Sjors commented at 3:28 PM on July 8, 2026:

In 1aa695a6f08a6824f5577ce8de3e5fe6578daa3b node: introduce block template manager: not sure if this alias adds much value, is does add churn.

in src/node/block_template_manager.h:24 in 610d5d704c

  20 | @@ -21,10 +21,15 @@ class BlockTemplateManager
  21 |  private:
  22 |      CTxMemPool& m_mempool;
  23 |      ChainstateManager& m_chainman;
  24 | +    const BlockCreateOptions m_init_block_create_options;

Sjors commented at 3:36 PM on July 8, 2026:

In 610d5d704c35c8aeda860640b599e9ffa5e99d92 node: move mining_args to block template manager: maybe call this m_block_create_args and then document that they're set during Init.

in src/node/block_template_manager.h:32 in 610d5d704c

  28 | -                                  ChainstateManager& chainman);
  29 | +                                  ChainstateManager& chainman,
  30 | +                                  BlockCreateOptions init_block_create_options = {});
  31 | +
  32 | +    /** @return a copy of the block create options set during node init. */
  33 | +    BlockCreateOptions GetInitBlockCreateOptions() const { return m_init_block_create_options; }

Sjors commented at 3:37 PM on July 8, 2026:

In 610d5d704c35c8aeda860640b599e9ffa5e99d92 node: move mining_args to block template manager: in line with my above comment, I'd call this BlockCreateArgs() (dropping Get makes the call sites more readable imo, the method is already marked const)

nit: const BlockCreateOptions&

Sjors commented at 4:15 PM on July 8, 2026: member

Reviewed up to 00e3b6717204937272f2d81074b429e7c13430a7 miner: move SubmitBlock into BlockTemplateManager. Be careful when rebasing after #34672.

in src/node/interfaces.cpp:945 in 149baad220

 949 |  
 950 |      const BlockCreateOptions m_create_options;
 951 |  
 952 |      const std::unique_ptr<CBlockTemplate> m_block_template;
 953 |  
 954 |      bool m_interrupt_wait{false};

pablomartin4btc commented at 4:55 PM on July 8, 2026:

nit: m_interrupt_wait is missing the comment that m_interrupt_mining in MinerImpl has:

    // Treat as if guarded by notifications().m_tip_block_mutex
    bool m_interrupt_wait{false};

I think both fields are used identically — written under the mutex via InterruptWait(), read inside lambda predicates holding the mutex. The missing comment on m_interrupt_wait seems inconsistent.

pablomartin4btc commented at 5:15 PM on July 8, 2026: member

Concept ACK

Thanks for taking the suggestion from the previous PR.

General first pass review at 149baad220.

This PR is not based on #34803. #35675 targets master directly. IsStale, TemplateSnapshot, m_template_snapshots, and any MempoolUpdated subscription are absent — this is a pure refactor. The fee inflow tracking from #35581 will follow as a separate PR once this (or #34803) merges.

Key differences from #35581:

WaitAndCreateNewBlock has no cheap staleness check — it rebuilds a full template to detect fee increases. The original comment (// The latter check is expensive so we only run it once per second.) is accurate again here.
No template_id / tracked-vs-untracked template distinction.
rpc/mining.cpp calls BlockTemplateManager::CreateNewTemplate() directly (returning CBlockTemplate). BlockTemplateImpl is only ever instantiated in node/interfaces.cpp (the IPC layer) — both on initial creation via MinerImpl::createNewBlock and on subsequent calls via waitNext — never from the RPC path. This removes the NodeContext-reference-during-shutdown concern that was present in #35581.

Re Sjors' question about node.mining: it appears to be unused after this PR. On master it was read via EnsureMining() in rpc/server_util.cpp, but that's now replaced by EnsureBlockTemplateManager(). The field could likely be removed.

I plan to follow up with a more detailed commit-by-commit review. Please let me know if any of my observations above are off.

Thanks for splitting this out from #35581 — the separation between the pure refactor and the fee-inflow tracking makes both easier to review.