#35671 mining: add TxCollection to bandwidth-efficiently validate external block templates
https://github.com/bitcoin/bitcoin/pull/35671 · · +732/-57 in 9 files, 6 commits · labels: Mining
Goal
- Allow Stratum v2 services to validate miner-submitted block templates directly against the mempool
- Saves bandwidth and compute by avoiding duplicate mempool mirrors in mining infrastructure
This pull request introduces the `TxCollection` interface to Bitcoin Core's mining IPC API. It allows external clients (specifically Stratum v2 Job Declarator Servers) to supply a list of transaction witness IDs, query which transactions the node mempool is missing, provide only those missing transactions, and construct and validate an external block template using Core's existing validation logic.
Problem: External mining servers running Stratum v2 Job Declarator Protocols previously had to run their own mirror mempools by polling block templates and requesting missing transactions over P2P/IPC. This wasted bandwidth, duplicated mempool tracking logic, and risked divergence from the node's consensus validation.
Category: Mining (#2 of 13)
P2 · new feature
- P2 because it provides a key integration requirement for Stratum v2 template providers
- It unblocks downstream Job Declarator Servers from needing full mempool mirrors
- Reduces redundant transaction synchronization overhead across mining setups
Substantially advances Stratum v2 integration by allowing the Job Declarator Server to validate external templates against Core's mempool without keeping an in-memory mirror.
Membership: Touches src/node/miner.cpp, src/interfaces/mining.h, and modifies block template construction logic.
Factors: security/stability 1, bug 0, performance 1, user value 2, leverage 2
Reviewability: Ready
- Ready for review after multiple stabilizing review rounds
The code is clean, tests are passing, and outstanding reviewer comments are minor clarifications that do not invalidate review.
Author status: Addressing review actively through August 2026; silent for 28 days following minor follow-up feedback.
Open concerns:
- jeanpablojp asked whether the difference between `stale-prevblk` and `inconclusive-not-best-prevblk` error strings relative to `Mining::checkBlock()` was intentional, and suggested an extra test case for invalid witness commitment.
Resolved concerns:
- ViniciusCestarii suggested splitting into two interfaces rather than throwing on unsupported methods for external templates; concluded as non-blocking after author explanation.
- enirox001 noted mutex lock scope was wider than necessary in `MakeTemplate` and suggested reserving map size, both incorporated by author.
- pablomartin4btc suggested input bounds checking on `wtxids` vector size in the constructor, which author implemented.
Agreement: Strong
- Strong concept support for avoiding mirror mempools in Stratum v2 (ismaelsadeeq, pablomartin4btc)
- Confirmed architectural benefit and resolved code review feedback (enirox001)
- Tested functionality with minor open question on error return codes (jeanpablojp)
Strong concept consensus from mining contributors; all code review comments addressed.
Multiple contributors and maintainers supporting Stratum v2 integration have reviewed and endorsed the architectural shift to using Core's mempool rather than an external mirror.
- enirox001: 'Reusing Bitcoin Core's mempool avoids duplicating transaction storage and continuous mirror synchronization...'
- jeanpablojp: 'Concept ACK: Ran the new tests and the mining suite...'
- ViniciusCestarii: 'I took the liberty to prototype this... I'm fine either way'
Objections:
| Reviewer | Kind | Harm | Status | Blocking | Author replied | Quote |
|---|---|---|---|---|---|---|
| ViniciusCestarii | correctness | order-dependent addition in addMissingTxs could leave partially applied state if an invalid transaction aborts the loop early | resolved | no | yes | 2026-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-then-apply seems to be a safer approach.' Settled: 2026-07-23: 'Addressed @ViniciusCestarii's inline comments... Ready for review.' |
| gmaxwell | safety | non-differential indices allow an attacker to repeatedly request large transactions to overflow buffers or cause out-of-memory errors | resolved | no | yes | 2026-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 again... and overflows a buffer or OOMs an implementation explicitly constructs the block' Settled: 2026-07-15: 'the Mining IPC client is expected to run on the same machine, so unknownTxPos() and addMissingTxs() are not designed for bandwidth efficiency. We also (currently) more or less trust the IPC client. It needs to deal with any hardening against its clients itself.' |
| enirox001 | safety | addMissingTxs checks transaction count rather than weight, permitting oversized transaction payloads to be stored before block validation | resolved | no | yes | 2026-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 oversized in makeTemplate when TestBlockValidity is called, but that is a bit too late.' Settled: 2026-08-20: 'I don't think we should take over too much work that TestBlockValidity() does for us. Duplicating consensus checks could introduce new bugs.' |
Support:
- enirox001: Reusing Bitcoin Core's mempool avoids duplicating transaction storage and continuous mirror synchronization, transfers only missing transactions, and lets external templates be reconstructed using existing validation logic.
- ismaelsadeeq: Concept ACK for enabling efficient external template validation. [not substantive]
- pablomartin4btc: Concept ACK. [not substantive]
- jeanpablojp: Concept ACK after running the new tests and checking coinbase commit handling.
Participants: ismaelsadeeq (support), ViniciusCestarii (objection), gmaxwell (objection), pablomartin4btc (support), enirox001 (objection), jeanpablojp (support)
State derived from the lists: substantive support, no open objection (enirox001, jeanpablojp)
Review verdicts (DrahtBot): 0
- Concept ACK: ismaelsadeeq, pablomartin4btc, enirox001, jeanpablojp
Dependencies
Enables:
- Stratum v2 Job Declarator Server (sv2-apps #599)
Files
320 lines under test/bench/ci.
- test/functional/interface_ipc_mining_tx_collection.py +292/-0
- src/node/miner.cpp +215/-48
- src/interfaces/mining.h +85/-4
- src/node/interfaces.cpp +52/-4
- src/node/miner.h +52/-1
- test/functional/test_framework/ipc_util.py +26/-0
- src/ipc/capnp/mining.capnp +8/-0
- test/functional/test_framework/messages.py +1/-0
- test/functional/test_runner.py +1/-0
Card
PR 35671 adds a TxCollection interface to the mining API to allow external software like the Stratum v2 Job Declarator Server to validate externally proposed block templates against Bitcoin Core's mempool. Instead of transmitting full blocks or maintaining duplicate mirror mempools in external daemons, clients provide a list of wtxids and only transfer transactions missing from the node's mempool. It has strong Concept ACK support from mining developers, with previous rounds of review from enirox001, ViniciusCestarii, and pablomartin4btc addressed in pushes.