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

Sjors commented at 7:36 PM on July 6, 2026: member

The Stratum v2 spec defines a Job Declarator Server (JDS), whose job it is to approve custom miner templates. It currently maintains its own mempool mirror, which it populates by repeatedly calling getBlock() and waitNext() and then requesting any missing transactions from the miner (since #34020 it can use getTransactionsByWitnessID() to first ask the node).

The new TxCollection interface introduced by this PR removes the need for such a mirror mempool. We build the block template ourselves now, based on the requested transaction list and our own mempool.

The following flow is demonstrated in https://github.com/stratum-mining/sv2-apps/pull/599:

Miner generates a template, e.g. using our createNewBlock() IPC method
They send the list of Wtxids to the pool's JDS
JDS calls TxCollection::collectTxs() (this PR) with that Wtxid list
We check the list against our mempool and hold a CTransactionRef for each match
The JDS queries unknownTxPos() to learn what's missing in our mempool
The JDS requests the fully serialized missing transactions from the miner (using native Stratum v2 messages)
JDS calls addMissingTxs() which completes the collection
JDS calls makeTemplate() which reconstructs the block and validates it, optionally with the coinbase it intends to use (e.g. to check the reward amount); a node-generated dummy is used otherwise. The JDS needs the check to succeed, may relay the BIP-22 failure reason to the miner.
It returns a BlockTemplate which works as if we generated the template ourselves, but with methods like waitNext() disabled. The JDS can call submitSolution() (the sv2 spec allows for redundant block reconstruction by both the miner node and the JDS).

If this sounds very similar to compact block relay: it is! In fact, it would be easy to expand the above methods to take transaction short ids instead of Wtxid. That may be useful for the p2pool revival project, which needs a way to validate and relay weak blocks as compact blocks. The p2pool client software would need to (trivially) verify the PoW, but could use the TxCollection interface to reconstruct and verify the block content. That said, the SRI JDS implementation is much further along and likely to be the first consumer of this.

Commits:

ipc: add TxCollection scaffold - step 3 and 4
mining: add TxCollection unknownTxPos - step 5
mining: add TxCollection addMissingTxs - step 7
mining: add coinbase transaction helper - we need a dummy coinbase to verify the block, so extract a helper from BlockAssembler::CreateNewBlock()
mining: make TxCollection create a BlockTemplate - step 8 and 9
mining: restrict externally generated templates - step 9

Potential followups:

https://github.com/Sjors/bitcoin/pull/122
have addMissingTxs() (optionally) insert favorable transactions into our mempool
add a data structure to store transactions that don't meet our mempool threshold (so TxCollection can grab them, instead of needing another round trip)

Sjors commented at 7:37 PM on July 6, 2026: member

@plebhash it would be very useful to have a (very) rough JDS draft that uses this interface, to see if it actually gets rid of the mirror mempool.

DrahtBot commented at 7:37 PM on July 6, 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/35671.

Reviews

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

Type	Reviewers
Concept ACK	ismaelsadeeq, 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:

#35675 (mining: add block template manager by ismaelsadeeq)
#35569 (Encapsulation for CTransaction by purpleKarrot)
#35551 (test: add interface_gui.py to test bitcoin-qt startup by ryanofsky)
#33922 (mining: add getMemoryLoad() and track template non-mempool memory footprint by Sjors)

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.

DrahtBot added the label Mining on Jul 6, 2026

Sjors force-pushed on Jul 6, 2026

DrahtBot added the label CI failed on Jul 6, 2026

DrahtBot commented at 7:51 PM on July 6, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/28818181619/job/85462853775</sub> <sub>LLM reason (✨ experimental): CI failed because IWYU reported/required include fixes (modified files) and intentionally exited non-zero.</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot removed the label CI failed on Jul 6, 2026

ismaelsadeeq commented at 9:41 AM on July 7, 2026: member

Concept ACK

The JDS queries unknownTxPos() to learn what's missing in our mempool

This could be collapsed to the response "JDS calls TxCollection::collectTxs() (this PR) with that Wtxid list" to reduce the round trip?

Sjors force-pushed on Jul 7, 2026

Sjors commented at 10:49 AM on July 7, 2026: member

I added an optional coinbase argument to makeTemplate(). Based on my draft JDS implementation https://github.com/stratum-mining/sv2-apps/pull/599 this does appear useful, e.g. for checking the reward amount. We still generate a dummy if no coinbase transaction is provided. @ismaelsadeeq collectTxs() is the constructor so it needs to return an interface. I don't think it can return both the TxCollection interface and the list of missing transactions. But clients can chain requests (not sure if we've fully implemented the on the libmultiprocess side).

I can also imagine a (slight) benefit in clients being able to call unknownTxPos() multiple times, e.g. maybe in a future upgrade we could have the node request missing transactions from peers - not sure how useful that is though.

Sjors referenced this in commit 354f23abd0 on Jul 7, 2026

DrahtBot added the label Needs rebase on Jul 8, 2026

Sjors force-pushed on Jul 8, 2026

Sjors commented at 6:41 AM on July 8, 2026: member

Rebased after #34020:

first test commit no longer needed
bumped collectTxs() to @10

Sjors referenced this in commit 22bc165d93 on Jul 8, 2026

Sjors referenced this in commit 487f2e365b on Jul 8, 2026

Sjors referenced this in commit ddadd4bde7 on Jul 8, 2026

Sjors referenced this in commit f20d0ce98b on Jul 8, 2026

Sjors referenced this in commit 1d65d7bbf5 on Jul 8, 2026

Sjors referenced this in commit 31d3c510db on Jul 8, 2026

DrahtBot removed the label Needs rebase on Jul 8, 2026

in src/node/miner.cpp:96 in ee1c43e3ce outdated

  91 | +    LOCK(m_mutex);
  92 | +    for (const auto& tx : txs) {
  93 | +        if (!tx) throw std::runtime_error("unexpected null transaction");
  94 | +        const auto it{m_transactions.find(tx->GetWitnessHash())};
  95 | +        if (it == m_transactions.end()) {
  96 | +            throw std::runtime_error(strprintf("unexpected wtxid %s", tx->GetWitnessHash().ToString()));

ViniciusCestarii commented at 2:29 PM on July 8, 2026:

In "mining: add TxCollection addMissingTxs" ee1c43e3ceb21fdf38340801221e9fa42c8e6993

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.

ViniciusCestarii commented at 6:45 PM on July 9, 2026: contributor

But clients can chain requests (not sure if we've fully implemented the on the libmultiprocess side).

Capnp pipelining works for any client hitting the RPC socket, regardless of libmultiprocess, because it's a protocol-level feature the server doesn't need to implement. So the client (JDS) just needs to pipeline requests and it will work.

(The caveat only applies to Bitcoin Core's own libmultiprocess generated C++ client code, which currently blocks per call in clientInvoke and doesn't exploit pipelining internally. But since this interface is expected to be consumed by external clients this doens't apply)

in test/functional/interface_ipc_mining.py:385 in ee1c43e3ce

 380 |                  # The first transaction is already in node's mempool, but
 381 |                  # the child transaction only exists on the disconnected
 382 |                  # remote node.
 383 |                  assert_equal(await tx_collection_unknown_pos(tx_collection, ctx0), [1])
 384 |  
 385 | +                self.log.debug("Reject unexpected transactions in addMissingTxs(), without undoing earlier additions")

ViniciusCestarii commented at 7:27 PM on July 9, 2026:

In "mining: add TxCollection addMissingTxs" ee1c43e3ceb21fdf38340801221e9fa42c8e6993

nit: there could be a test case for "unexpected null transaction" when calling addMissingTxs with a null tx

Sjors commented at 6:32 PM on July 23, 2026:

Added.

in src/interfaces/mining.h:1 in 9bb89df02f outdated

ViniciusCestarii commented at 9:02 PM on July 9, 2026:

In "mining: add TxCollection to bandwidth-efficiently validate external block templates" 9bb89df02fc3dbcb870b36d4a339986089ea40e2

not blocking/design question: would it be worth splitting into two capnp interfaces instead of throwing on methods: an ExternalBlockTemplate with just getBlockHeader/getBlock/getCoinbaseMerklePath/submitSolution and BlockTemplate extends(ExternalBlockTemplate) adding getTxFees/getTxSigops/getCoinbaseTx/waitNext/interruptWait? Then TxCollection::makeTemplate() returns the narrower type and the disallowed methods aren't callable, rather than compiling and throwing on call.

I took the liberty to prototype this here: https://github.com/ViniciusCestarii/bitcoin/compare/9bb89df02fc3dbcb870b36d4a339986089ea40e2...pr-35671-new-interface

ViniciusCestarii commented at 9:06 PM on July 9, 2026:

In "ipc: add TxCollection scaffold" 19bae00aa17be0964e28529ecc348a94522204af

nit: commit states that it adds CollectedTxs class but there's no CollectedTxs class

Sjors commented at 6:31 PM on July 23, 2026:

Two interfaces might require a bunch of code duplication on the Rust client side though? Also several disabled methods could be made to work with external templates, if someone actually needs them.

Sjors commented at 6:32 PM on July 23, 2026:

Fixed commit message.

ViniciusCestarii commented at 7:54 PM on July 23, 2026:

On the duplication concern: extends() gives upcasting, so shared logic that only needs the common subset doesn't need duplicating. Where branching is needed, it's because the type system is now correctly rejecting a call that would've thrown at runtime anyway and that's the point of the split, not an added cost.

On the disabled methods: if they could be implemented for external templates later, they could just as well be added to the split interfaces later too. The split just removes what's currently unclear to a client consuming the interface (methods that appear callable but are guaranteed to throw).

Just my two cents. I'm fine either way

Sjors commented at 11:50 AM on July 10, 2026: member

@ViniciusCestarii wrote:

But since this interface is expected to be consumed by external clients this doens't apply

Interesting, so we don't need to change anything in libmultiprocess on the server (node) side?

I'll take your inline comments into account on the next rebase. As I mentioned in the PR description, the code is still a bit rough.

ViniciusCestarii commented at 7:48 PM on July 13, 2026: contributor

Interesting, so we don't need to change anything in libmultiprocess on the server (node) side?

Yes, pipelining is already possible: I have made this client demo that chains calls, including the new interface TxCollection, in a single round trip with only one .wait() at the end.

OBS: Only interface fields (and struct fields that transitively lead to one) can be actually pipelined. (in the example I only pipelined interfaces), any other value like int, bool, enum, text, data, list, etc cannot be pipelined.

gmaxwell commented at 10:03 PM on July 14, 2026: contributor

compact blocks uses a differential index not primarily for efficiency (though it does save quite a bit when there are many misses) but so that the wire format is safe by construction against an attack that applies the same large transaction over and over again (e.g. if index 5 is some giant txn, you request 5,5,5,5,5,5...) and overflows a buffer or OOMs an implementation explicitly constructs the block and without having to have expensive duplication checks.

Sjors commented at 8:59 AM on July 15, 2026: member

@gmaxwell the Mining IPC client[^0] 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.[^1]

Using absolute positions matches the Stratum v2 spec[^2], so there's probably no point in hardening it at the node level. But cc @TheBlueMatt as to why it didn't use a differential index?

If we later expand TxCollection to support compact blocks, we would probably add new unknownTxDifferentialPos() and addMissingTxsForCompactBlock() methods. Those should then match the p2p pattern you describe.

[^0]: for TxCollection it's the Job Declarator Server: https://github.com/stratum-mining/sv2-apps/tree/main/pool-apps/jd-server

[^1]: not having to deal with another network protocol and its DoS risks was one of the motivations for not supporting Stratum v2 p2p messages directly, see discussion that starts at: #29432#pullrequestreview-2132699185. That said, over time we could gradually harden the IPC to where the Stratum v2 client can be a very thin wrapper.

[^2]: ProvideMissingTransactions and ProvideMissingTransactions.Success

Sjors force-pushed on Jul 23, 2026

Sjors commented at 6:42 PM on July 23, 2026: member

Rebased after and made consistent with #34672. Addressed @ViniciusCestarii's inline comments #35671#pullrequestreview-4655098156. Ready for review.

Sjors marked this as ready for review on Jul 23, 2026

Sjors force-pushed on Jul 23, 2026

Sjors commented at 7:08 PM on July 23, 2026: member

Used a lambda to avoid MakeTemplateInternal, reduced intra-commit churn.

DrahtBot added the label CI failed on Jul 23, 2026

DrahtBot removed the label CI failed on Jul 23, 2026

in src/node/miner.cpp:68 in af9a1157f6 outdated

  63 | +      m_node(node)
  64 | +{
  65 | +    LOCK(m_mutex);
  66 | +    CTxMemPool& mempool{*Assert(m_node.mempool)};
  67 | +    LOCK(mempool.cs);
  68 | +    for (const auto& wtxid : m_wtxids) {

pablomartin4btc commented at 3:31 AM on July 24, 2026:

Should the size of wtxids be checked before the loop (e.g. perhaps using existing constants — MAX_BLOCK_WEIGHT / MIN_TRANSACTION_WEIGHT)?

Sjors commented at 11:24 AM on July 24, 2026:

Added (similar check as in merkleblock.cpp), along with a test.

I added a similar guard to addMissingTxs(), but for it to be actually useful, we'd need to modify libmultiprocess to enforce things like a maximum param size.

in src/node/miner.cpp:65 in af9a1157f6

  60 |  
  61 | +TxCollection::TxCollection(std::vector<Wtxid> wtxids, const NodeContext& node)
  62 | +    : m_wtxids(std::move(wtxids)),
  63 | +      m_node(node)
  64 | +{
  65 | +    LOCK(m_mutex);

pablomartin4btc commented at 3:36 AM on July 24, 2026:

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.

Sjors commented at 11:24 AM on July 24, 2026:

Dropped

in src/node/miner.cpp:153 in af9a1157f6

 148 | +        // transactions are added so the witness commitment is current.
 149 | +        block.vtx.emplace_back();
 150 | +
 151 | +        for (const auto& wtxid : m_wtxids) {
 152 | +            const auto it{m_transactions.find(wtxid)};
 153 | +            Assume(it != m_transactions.end());

pablomartin4btc commented at 3:39 AM on July 24, 2026:

nit: these invariants are guaranteed by the constructor and the any_of check above — I think Assert would be stronger than Assume here to catch future regressions.

Sjors commented at 11:25 AM on July 24, 2026:

More importantly it->second below would crash, so Assert is appropriate. Fixed.

pablomartin4btc commented at 3:49 AM on July 24, 2026: member

Concept ACK

Left a few comments.

ipc: add TxCollection scaffold

Add the TxCollection interface and the node::TxCollection class that
backs it. The constructor looks up each requested wtxid in the mempool
and keeps a reference to any transaction that is already present, so
later commits can report which requested transactions are still missing
and let the client fill them in.

02ed6eafc4

mining: add TxCollection unknownTxPos e23f21b5cb

mining: add TxCollection addMissingTxs

Now that the collection can be mutated after construction, guard the
collected transactions with a mutex: IPC clients may call TxCollection
methods concurrently from different threads.

f4d53d1636

mining: add coinbase transaction helper

Move the coinbase construction out of CreateNewBlock() into a reusable
BlockAssembler::CreateCoinbaseTx() helper, so a later commit can build a
coinbase when assembling a template from externally collected
transactions.

This does not change behavior. Review with --color-moved=dimmed-zebra.

02d8f43959

mining: make TxCollection create a BlockTemplate

Add TxCollection::makeTemplate(), which assembles the collected
transactions, in the requested order, into a block that builds on the
given prevhash, and validates it.

The block is validated by the same TestBlockValidity() call used by
checkBlock(), with the proof-of-work and merkle-root checks disabled.

A dummy coinbase is added only so the transactions can be validated as
part of a block. Its output pays no fees (fees=0), because the total fee
amount is not tracked here. Clients construct the real coinbase
themselves, which is why the next commit disables getCoinbaseTx() for
externally generated templates.

When the requested prevhash does not match the active tip, we mirror the
getblocktemplate proposal reasons: 'stale-prevblk' when prevhash is an
ancestor of the current tip, and 'inconclusive-not-best-prevblk' when it
is unknown or on a fork.

a4a3db4577

mining: restrict externally generated templates

Disable methods that are not expected to be used in the externally
provided template use case.

getTxFees() and getTxSigops() could be implemented, but it's not
worth doing since the external software that constructed the
template is expected to know this.

getCoinbaseTx() is disabled because the dummy coinbase is only added to
allow validating the collected transactions as a block. It pays no fees
(fees=0) and should not be used by clients, which construct the real
coinbase themselves.

d0c20776ab

Sjors force-pushed on Jul 24, 2026

Sjors commented at 11:25 AM on July 24, 2026: member

Addressed @pablomartin4btc's feedback. This PR grew interface_ipc_mining.py to over 1000 lines and the new tests add significant runtime. So I add interface_ipc_mining_tx_collection.py.

Sjors referenced this in commit e3f3e02ac5 on Jul 24, 2026

Sjors referenced this in commit 68eafcb0ca on Jul 24, 2026

DrahtBot added the label Needs rebase on Jul 29, 2026

DrahtBot commented at 8:45 PM on July 29, 2026: contributor

🐙 This pull request conflicts with the target branch and needs rebase.