#35671 mining: add TxCollection to bandwidth-efficiently validate external block templates

ReviewabilityReviewsAgreementSize
#35671 Sjors mining: add TxCollection to bandwidth-efficiently validate external block templatesReady0Strong491 + 447 tests
  • Let external mining coordinators validate block templates without keeping duplicate mempools
  • Reduces memory and bandwidth overhead for tools like Stratum v2 Job Declarator Servers
  • Ready for review
  • Substantive support with no open objections
  • Strong support because it avoids duplicate storage and continuous mirror sync (enirox001)
  • Verified by running the new tests and the mining test suite (jeanpablojp)
  • Concept approval without stated reasons (ismaelsadeeq, pablomartin4btc)
  • 491 lines added or modified outside tests
  • 447 lines added or modified in tests
  • 58 lines removed in total
  • 17 files, 7 commits
Mining#3 of 17
P2 · new feature
  • P2 because it lets mining coordinators reconstruct and validate block templates efficiently
  • Removes the need for external tools to maintain out-of-process mempool mirrors
  • Directly unblocks Stratum v2 reference tooling integration
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Adds a TxCollection interface to Bitcoin Core's mining subsystem and Cap'n Proto IPC interface to allow external clients to reconstruct and validate block templates from an ordered list of witness transaction IDs (WTXIDs). The interface identifies which transactions are absent from the node's mempool, accepts missing transaction payloads from the caller, and assembles candidate block templates validated against the active chain tip via TestBlockValidity. It also refactors coinbase generation in BlockAssembler into a standalone helper to support both node-generated dummy coinbases and custom caller-provided coinbases.

Problem: External mining coordinators that validate and approve miner-selected block templates (such as Stratum v2 Job Declarator Servers) currently maintain duplicate out-of-process mempool mirrors. Keeping those mirrors synchronized requires repeatedly fetching entire block templates or pulling missing transactions over IPC, creating significant memory and bandwidth overhead.

Evidence of importance:

  • PR description notes that Stratum v2 Job Declarator Servers currently must maintain 'its own mempool mirror, which it populates by repeatedly calling getBlock() and waitNext() and then requesting any missing transactions from the miner', which this PR removes.
  • The new interface is already integrated and demonstrated in Stratum v2 reference tooling via stratum-mining/sv2-apps PR #599.

Scope: Adds node::TxCollection and implements its IPC bindings; extracts BlockAssembler::CreateCoinbaseTx() from miner.cpp without changing existing block template creation behavior.

Risk of a mistake: medium (validation, rpc_api, other). Flaws in block template assembly or validation could cause mining coordinators to approve invalid candidate blocks or crash IPC server processes.

Changed since the previous assessment: Rebased onto recent changes (such as the merged block template manager); core architecture and features remain unchanged.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • reviewer: review of the PR and feedback on whether anything remains missing for the draft spec 2026-10-15

Agreement

Strong

substantive support, no open objection (enirox001, jeanpablojp)

Thread notes: c:6181436925 was edited by the author to indicate that the PR description has been updated to take v32 for granted and link to the draft spec.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
ismaelsadeeq (member)question, approachwhether collectTxs() can return missing transaction positions directly to reduce an IPC round tripanswered · author explained collectTxs is a constructor returning an interface and pipelining can chain requestsansweredyes2026-07-07: 'This could be collapsed to the response "JDS calls TxCollection::collectTxs() (this PR) with that Wtxid list" to reduce the round trip?'
Status from: 2026-07-07: Sjors: 'collectTxs() is the constructor so it needs to return an interface. I don't think it can return both the TxCollection in'
ViniciusCestarii (contributor)objection, correctnessorder-dependent addition in addMissingTxs could leave partially applied state if an invalid transaction aborts the loop earlyfixed · fix pushed · addressed in rebasechangedyes2026-07-08: 'AddMissingTxs is order-dependent, a bad entry aborts the loop at that point, so [valid, invalid] keeps valid but [invalid, valid] doesn't. Is this intentional? Correct me if I am wrong but validate-th'
Status from: 2026-07-23: Sjors: 'Addressed @ViniciusCestarii's inline comments https://github.com/bitcoin/bitcoin/pull/35671#pullrequestreview-4655098156'
ViniciusCestarii (contributor)suggestion, interfacesplit into two Cap'n Proto interfaces (ExternalBlockTemplate and BlockTemplate) so disallowed methods fail at compile-time rather than throwing at runtimeaccepted · reviewer stated they are fine either way after author raised concerns about client duplicationansweredyes2026-07-09: 'would it be worth splitting into two capnp interfaces instead of throwing on methods: an ExternalBlockTemplate with just getBlockHeader/getBlock/getCoinbaseMerklePath/submitSolution and BlockTemplate '
Status from: 2026-07-23: ViniciusCestarii: 'The split just removes what's currently unclear to a client consuming the interface (methods that appear callable but ar'
gmaxwell (contributor)objection, safetynon-differential indices allow repeated requests for the same transaction, risking buffer overflow or out-of-memory errorsanswered · author explained Mining IPC client trust model and local host assumptionansweredyes2026-07-14: 'compact blocks uses a differential index not primarily for efficiency... but so that the wire format is safe by construction against an attack that applies the same large transaction over and over aga'
Status from: 2026-07-15: Sjors: 'the Mining IPC client is expected to run on the same machine, so unknownTxPos() and addMissingTxs() are not designed for'
pablomartin4btc (member)suggestion, safetycheck the size of wtxids before looping using MAX_BLOCK_WEIGHT / MIN_TRANSACTION_WEIGHTfixed · fix pushed · author added the limit check in miner.cppansweredyes2026-07-24: 'Should the size of `wtxids` be checked before the loop (e.g. perhaps using existing constants — `MAX_BLOCK_WEIGHT / MIN_TRANSACTION_WEIGHT`)?'
Status from: 2026-07-24: Sjors: 'Added (similar check as in merkleblock.cpp), along with a test.'
pablomartin4btc (member)suggestion, maintenanceavoid locking m_mutex in TxCollection constructor since the object is not yet sharedfixed · fix pushed · author dropped the mutex lock in the constructoransweredyes2026-07-24: 'nit: does the `m_mutex` need to be locked in the constructor? — the object isn't shared until after construction returns, so no concurrent access is possible yet.'
Status from: 2026-07-24: Sjors: 'Dropped'
enirox001 (contributor)suggestion, safetycheck for duplicate wtxids in the constructor before locking the mempoolfixed · fix pushed · author added duplicate wtxid check prior to mempool lockansweredyes2026-08-12: 'I think the constructor could check for duplicates before locking the mempool? As the duplicate is entirely visible from the input.'
Status from: 2026-08-20: Sjors: 'Done, checking duplicates first without a lock now.'
enirox001 (contributor)suggestion, maintenancecall reserve on m_transactions before acquiring the mempool lockfixed · fix pushed · author implemented reserve prior to lockansweredyes2026-08-12: 'since the collection size is known, we could reserve it before acquiring the mempool lock to avoid the reallocating and rehashing while the mempool is locked'
Status from: 2026-08-20: Sjors: 'Done'
enirox001 (contributor)suggestion, correctnessadd a test verifying that TxCollection retains transactions when they leave the mempoolfixed · fix pushed · author incorporated suggested snapshot testansweredyes2026-08-12: 'I think an improvement to this test would be adding a test to verify the mempool snapshot behavior. such that it tests that after the collection is constructed with a transaction in the mempool, if th'
Status from: 2026-08-20: Sjors: 'Taken'
enirox001 (contributor)suggestion, maintenancedocument that all non-coinbase transactions must already be present in block.vtx before calling the coinbase helperfixed · fix pushed · author added doc commentansweredyes2026-08-12: 'The current documentation here does not mention that all the transactions must be present before calling this so that this does not happen.'
Status from: 2026-08-20: Sjors: 'Taken.'
enirox001 (contributor)suggestion, maintenancecalculate height locally as a const in the coinbase helper instead of modifying BlockAssembler memberfixed · fix pushed · author adopted local calculation and made helper constansweredyes2026-08-12: 'I think a cleaner approach to this helper would be to have it calculate its own height... const int height{pIndexPrev.nHeight + 1}'
Status from: 2026-08-20: Sjors: 'Taken, and made it a const member function.'
enirox001 (contributor)suggestion, maintenancelimit m_mutex scope in MakeTemplate to checking completeness and copying transactions into a local vectorfixed · fix pushed · author narrowed lock scopeansweredyes2026-08-12: 'Could we limit its scope to checking completeness and copying the ordered m_transactions into a local vector? The references remain valid after the lock is released, so block construction and TestBloc'
Status from: 2026-08-20: Sjors: 'Done'
enirox001 (contributor)question, interfacewhy interruptWait is permitted on external templates when waitNext throwsfixed · fix pushed · author acknowledged oversight and disabled interruptWait as wellansweredyes2026-08-12: 'waitNext is disabled for external templates, but interruptWait is still allowed? Is there a reason for this?'
Status from: 2026-08-20: Sjors: 'Good catch, I probably missed this in a rebase (before I opened the PR).'
enirox001 (contributor)objection, correctnessaddMissingTxs checks transaction count rather than weight, permitting oversized transaction payloads to be stored before block validationanswered · author declined duplicating consensus checks done by TestBlockValidityansweredyes2026-08-12: 'The check here is against the size, but it does not check the weight. So even though it can reject a million tiny transactions, it can accept 16666 large ones... This would be eventually rejected as o'
Status from: 2026-08-20: Sjors: 'I don't think we should take over too much work that TestBlockValidity() does for us. Duplicating consensus checks could'
jeanpablojp (contributor)question, interfacewhy makeTemplate() uses stale-prevblk for ancestor prevhash when TestBlockValidity and checkBlock use inconclusive-not-best-prevblkfixed · fix pushed · author switched to inconclusive-not-best-prevblkansweredyes2026-09-06: 'makeTemplate() reports an ancestor prevhash as `stale-prevblk`, but `Mining::checkBlock()` reports that same case as `inconclusive-not-best-prevblk`... Was that difference deliberate?'
Status from: 2026-09-24: Sjors: 'Also addressed @jeanpablojp's feedback: switched to `inconclusive-not-best-prevblk`'
jeanpablojp (contributor)suggestion, correctnessadd a test case for makeTemplate rejecting a coinbase with a mismatched witness commitmentfixed · fix pushed · author added test for mismatched witness commitmentansweredyes2026-09-06: 'The amount case is covered, the witness commitment one isn't, and that's the one that pins which transactions the coinbase commits to. I flipped a bit of the commitment and it gives bad-witness-merkle'
Status from: 2026-09-24: Sjors: 'Added such a test.'
average-gary (none)suggestion, interfaceexpose the block's fee total, either by enabling BlockTemplate::getTxFees() on externally generated templates or by adding a fees field to checkBlockanswered · author noted PR 36525 introduces getTotalFees() and can be used here if merged first, or handled in a followupansweredyes2026-10-08: 'the one value the TP then has to report that this interface doesn't expose is the block's fee total.'
Status from: 2026-10-13: Sjors: 'I might open a separate PR to refactor our mining code so that `BlockTemplate::getTxFees()` can work with external templ'
plebhash (none)question, approachclarification on the role and motivation of TxCollection given that the mempool mirror is already being removed via getTransactionsByWitnessID in v32answered · author confirmed the PR description was updated to assume v32 and referenced the draft specansweredyes2026-10-14: 'mempool mirror is going away via `getTransactionsByWitnessID` on v32 (https://github.com/stratum-mining/sv2-apps/pull/911), and my understanding is that `TxCollection` will only further optimize thing'
Status from: 2026-10-15: Sjors: 'Indeed that comment is stale. I ~'ll~ updated the description to take v32 for granted, and also to reference the draft spec'

Support:

  • ismaelsadeeq (member) (Concept ACK, 2026-07-07): no reason given [not substantive]
  • pablomartin4btc (member) (Concept ACK, 2026-07-24): no reason given [not substantive]
  • enirox001 (contributor) (Concept ACK, 2026-08-11): no reason given: Concept ACK, intending to benchmark this approach against the previous one [not substantive]
  • enirox001 (contributor) (Concept ACK, 2026-08-12) [read]: Reusing Bitcoin Core's mempool avoids duplicating transaction storage and continuous mirror synchronization, transfers only transactions the node is missing, and lets external templates b
  • jeanpablojp (contributor) (Concept ACK, 2026-09-06) [both]: Ran the new tests and the mining suite, and checked the coinbase commit against its parent.

Participants:

  • ismaelsadeeq (member, support, 1 statements 2026-07-07 to 2026-07-07): Concept ACK; asked about collapsing IPC steps to reduce round trips
  • ViniciusCestarii (contributor, objection, 7 statements 2026-07-08 to 2026-07-23): raised concern on order-dependent addMissingTxs handling, suggested interface split, and demonstrated pipelining
  • gmaxwell (contributor, objection, 1 statements 2026-07-14 to 2026-07-14): noted non-differential indices risk buffer overflows or OOM; author explained IPC trust model
  • pablomartin4btc (member, support, 4 statements 2026-07-24 to 2026-07-24): Concept ACK; suggested input size checking on wtxids and locking cleanup
  • enirox001 (contributor, objection, 12 statements 2026-08-11 to 2026-08-12): Concept ACK; raised concern on lack of weight check in addMissingTxs; suggested locking and test improvements
  • jeanpablojp (contributor, support, 4 statements 2026-09-06 to 2026-09-06): Concept ACK after testing; asked about reject reasons and suggested test and documentation improvements
  • average-gary (none, neutral, 1 statements 2026-10-08 to 2026-10-08): suggested exposing block fee total via getTxFees() or checkBlock
  • plebhash (none, question, 1 statements 2026-10-14 to 2026-10-14): asked about the relationship between TxCollection and getTransactionsByWitnessID removing the mempool mirror in v32

Dependencies

Enables:

  • https://github.com/stratum-mining/sv2-apps/pull/599

Files

448 lines under test/bench/ci.

  • src/test/tx_collection_tests.cpp +221/-0
  • test/functional/interface_ipc_mining_tx_collection.py +196/-0
  • src/node/tx_collection.cpp +191/-0
  • src/node/miner.cpp +60/-48
  • src/interfaces/mining.h +93/-6
  • src/node/tx_collection.h +59/-0
  • src/node/interfaces.cpp +52/-3
  • test/functional/test_framework/ipc_util.py +26/-0
  • src/node/miner.h +15/-0
  • src/ipc/capnp/mining.capnp +8/-0
  • src/node/block_template_manager.cpp +6/-0
  • src/node/block_template_manager.h +6/-0
  • ci/test/03_test_script.sh +1/-1
  • src/CMakeLists.txt +1/-0
  • src/test/CMakeLists.txt +1/-0
  • test/functional/test_framework/messages.py +1/-0
  • test/functional/test_runner.py +1/-0

Card

The short text the ranking pass compares across PRs.

Adds a TxCollection mining IPC interface that allows external services, like Stratum v2 Job Declarator Servers (JDS), to assemble and validate candidate block templates from a list of WTXIDs without maintaining duplicate mempool mirrors. External template coordinators currently synchronize duplicate mempools by repeatedly polling complete templates over IPC, incurring substantial bandwidth and memory overhead. The interface resolves WTXIDs against the local mempool, ingests missing raw transactions, and validates the assembled block against the chain tip using TestBlockValidity with an optional caller-supplied coinbase. The change enables downstream Stratum v2 JDS support demonstrated in sv2-apps PR #599.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-22T01:14, confidence high, input hash c58a1ecacf45d10a
thread read through 2026-10-15 (63 statements), head de11d8ec01
labels: Mining

Categories

Mining

P2 · new feature

Provides a dedicated interface for external mining coordinators like Stratum v2 Job Declarator Servers to reconstruct and validate block templates without maintaining costly out-of-process mempool mirrors. It directly unblocks external mining tooling integration (sv2-apps #599) and expands the Mining interface.

Membership: Carries the Mining label and adds TxCollection to the node's mining interface and BlockAssembler for external block template validation.

Factors: security/stability 0, bug 0, performance 1, user value 2, leverage 2