mining: add getTransactions(ByWitnessID) IPC methods

Sjors commented at 7:13 pm on December 5, 2025: member

For Stratum v2 custom job declaration to be bandwidth efficient, the pool can request¹ only the transactions that it doesn’t know about.

The spec doesn’t specify how this is achieved, but one method is to call the getrawtransaction RPC on each transaction id listed in DeclareMiningJob (or a subset if the pool software maintains a cache). Using RPC is inefficient, made worse by the need to make multiple calls. It also doesn’t support queuing by witness id (yet, see #34013).

This PR introduces two new IPC methods:

getTransactionsById(): takes a list of Txid’s
getTransactionsByWitnessID(): : takes a list of Wtxid’s

Both return a list of serialised transactions. An empty element is returned for transactions that were not found.

The Txid variant, just like its RPC counterpart, takes advantage of -txindex if enabled. The Wtxid variant can’t do that unless we add a -witnesstxindex, but at least sv2 has no use for that.

The RPC additionally allows providing a block_hash hint. I haven’t implemented that here because I don’t have a need for it. It can always be added later.

I thought about having a single (or overloaded) getTransactions() that works with both Txid and Wtxid, but I prefer that clients are intentional about which one they want.

A unit and functional test cover the new functionality.

Sv2 probably only needs getTransactionsByWitnessID(), but it’s easy enough to just add both.

To rest with Rust use:

https://github.com/2140-dev/bitcoin-capnp-types/pull/11

there’s two reasons the pool requests these transactions: to approve the template and to broadcast the block if a solution is found (the miner will also broadcast via their template provider). See also https://github.com/stratum-mining/sv2-spec/issues/170 ↩︎

DrahtBot added the label Mining on Dec 5, 2025

DrahtBot commented at 7:13 pm on December 5, 2025: 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/34020.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34644 (mining: add submitBlock to IPC Mining interface by w0xlt)
#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 CI failed on Dec 5, 2025

DrahtBot commented at 8:32 pm on December 5, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/19973417058/job/57283535788 LLM reason (✨ experimental): Lint failure: a subtree directory was touched without performing a proper subtree merge.

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.

ryanofsky commented at 1:01 am on December 8, 2025: contributor

figure out Treat std::shared_ptr nullptr as empty Data bitcoin-core/libmultiprocess#235 (this PR is draft pending that, subtree linter failure is expected)

This turned out to be an interesting problem, because in general it’s not safe to treat empty Data fields the same as unset values. For example with std::optional<std::string> or std::string* we would want to interpret empty Data fields as empty strings not null values. Even for serializable types it’s not really safe to treat empty Data the same as null since empty Data could be a valid serialization for some objects. For CTransaction objects though it is safe.

Another annoying thing is that if the goal is to be able to serialize vector<CTransactionRef> as List(Data) and allow the vector to contain null values, the most obvious way to do it would be to map null CTransactionRef values to null data values. But annoyingly although the Cap’n Proto wire format can represent null Data values in a list and has APIs to set null Data values and has APIs to distinguish between null and empty Data values in non-list contexts, it doesn’t currently provide any way to distinguish between null and empty Data values when the Data value is in a List.

So your workaround of interpreting empty Data values as null CTransactionRef is probably the best approach for now. I put together an implementation that should be more robust in b14778374b0694f7255f774cb657c7049cb4256e (tag). It limits the workaround to CTransactionRef data fields only and avoids making any assumptions in type-pointer.h about the type of pointers. It also includes a unit test that makes sure null values can be passed correctly.

Sjors force-pushed on Dec 8, 2025

Sjors commented at 8:44 am on December 8, 2025: member

@ryanofsky thanks, I swapped in your commit (subtree linter is still expected to fail).

So your workaround of interpreting empty Data values as null CTransactionRef is probably the best approach for now.

One reason I have some confidence in this approach is that the Python functional tests are able to read the result. Although it interprets the empty fields as b'' rather than None, that shouldn’t be an issue for client developers. This is captured by your note:

0//! [*] The Cap'n Proto wire format actually does distinguish between null and
1//! empty Data values inside Lists, and the C++ API does allow distinguishing
2//! between null and empty Data values in other contexts, just not the List
3//! context, so this limitation could be removed in the future.

If this is fixed one day and we correctly encode null on the wire, it would technically be a breaking API change, but trivial to deal with for clients. E.g. the functional test would check for both b'' and None.

b14778374b0694f7255f774cb657c7049cb4256e looks good to me, can you make a libmultiprocess PR, and another PR here that bumps the subtree to use it and add the tests? I’ll rebase on that.

DrahtBot added the label Needs rebase on Dec 16, 2025

Sjors commented at 12:06 pm on December 19, 2025: member

Needs rebase after #34003, but I’ll do that after there’s a multiprocess PR.

in src/ipc/capnp/mining.capnp:22 in 0ce5aba23c

18@@ -19,6 +19,8 @@ interface Mining $Proxy.wrap("interfaces::Mining") {
19     waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64) -> (result: Common.BlockRef);
20     createNewBlock @4 (options: BlockCreateOptions) -> (result: BlockTemplate);
21     checkBlock @5 (block: Data, options: BlockCheckOptions) -> (reason: Text, debug: Text, result: Bool);
22+    getTransactions @6 (context :Proxy.Context, txids: List(Data)) -> (result: List(Data));

plebhash commented at 1:43 pm on February 13, 2026:

0    getTransactionsByTxID [@6](/bitcoin-bitcoin/contributor/6/) (context :Proxy.Context, txids: List(Data)) -> (result: List(Data));

Sjors referenced this in commit fb2622ff5e on Feb 20, 2026

Sjors referenced this in commit e8bcca3be6 on Feb 20, 2026

Sjors referenced this in commit 423a789eb4 on Feb 20, 2026

Sjors force-pushed on Feb 20, 2026

Sjors commented at 10:37 am on February 20, 2026: member

I opened https://github.com/bitcoin-core/libmultiprocess/pull/242 and this PR now pulls that in.

Also rebased and renamed getTransactions to getTransactionsByTxID.

Oh oops, #34568 got merged right before I pushed this. Will rebase again.

Sjors force-pushed on Feb 20, 2026

DrahtBot removed the label Needs rebase on Feb 20, 2026

Sjors force-pushed on Feb 20, 2026

Sjors commented at 11:46 am on February 20, 2026: member

p2p_opportunistic_1p1c.py failure seems spurious: https://github.com/bitcoin/bitcoin/actions/runs/22222027229/job/64279933038?pr=34020

Sjors referenced this in commit 413f915979 on Feb 21, 2026

Sjors force-pushed on Feb 21, 2026

Sjors commented at 10:59 am on February 23, 2026: member

Test failure is expected: the subtree update pulled in https://github.com/bitcoin-core/libmultiprocess/pull/240 without the corresponding changes from #34422, so I’ll rebase once that lands.

DrahtBot added the label Needs rebase on Feb 24, 2026

Sjors referenced this in commit 2d3cc775e6 on Feb 27, 2026

Sjors referenced this in commit 97d877053b on Mar 4, 2026

Sjors force-pushed on Mar 5, 2026

Sjors commented at 0:39 am on March 5, 2026: member

Rebased after #34422 and using the latest https://github.com/bitcoin-core/libmultiprocess/pull/242. I added a separate test for run_transaction_lookup_test instead of modifying the existing tests in test/functional/interface_ipc_mining.py.

DrahtBot removed the label CI failed on Mar 5, 2026

DrahtBot removed the label Needs rebase on Mar 5, 2026

Sjors commented at 3:22 pm on March 5, 2026: member

I opened https://github.com/2140-dev/bitcoin-capnp-types/pull/11 for easier Rust testing.

Sjors referenced this in commit aa804580ee on Mar 5, 2026

Sjors referenced this in commit df8ed54aa5 on Mar 5, 2026

Sjors referenced this in commit d7d22ec00f on Mar 5, 2026

Sjors referenced this in commit ebe2996baa on Mar 6, 2026

ryanofsky referenced this in commit 8a5e3ae6ed on Mar 9, 2026

Sjors referenced this in commit 9a8d8614a0 on Mar 10, 2026

Sjors referenced this in commit 99ff226379 on Mar 11, 2026

mempool: add lookup by witness hash

Add a simple test for both the Txid and Wtxid variants of GetTransaction().

7768467455

Squashed 'src/ipc/libmultiprocess/' changes from 1868a84451..22bec918c9

22bec918c9 Merge bitcoin-core/libmultiprocess#247: type-map: Work around LLVM 22 "out of bounds index" error
8a5e3ae6ed Merge bitcoin-core/libmultiprocess#242: proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values
e8d3524691 Merge bitcoin-core/libmultiprocess#246: doc: Bump version 8 > 9
97d877053b proxy-types: add CustomHasField hook for nullable decode paths
8c2f10252c refactor: add missing includes to mp/type-data.h
b1638aceb4 doc: Bump version 8 > 9
f61af48721 type-map: Work around LLVM 22 "out of bounds index" error

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 22bec918c97d32660c4694c3a8b5af4cdbb88481

9a92945173

Merge commit '9a929451734a93ded114b06176a83db26eda300e' into tmp/pr34020-rewrite c059e6557e

ipc: Serialize null CTransactionRef as empty Data

Co-authored-by: Russell Yanofsky <russ@yanofsky.org>

f47e6556f0

mining: add getTransactionsByTxID() IPC method fee26555ba

mining: add getTransactionsByWitnessID() IPC method 12dac5f4e7

Sjors force-pushed on Mar 11, 2026

Sjors commented at 2:51 pm on March 11, 2026: member

https://github.com/bitcoin-core/libmultiprocess/pull/242 landed, so this PR now simply updates the libmultiprocess subtree. We’ll probably have another regular subtree update for 31.x bug fixes (see #34804), so I’ll wait for that, rebase, drop the subtree update here, and then mark ready for review.

mining: add getTransactions(ByWitnessID) IPC methods #34020

Code Coverage & Benchmarks

Reviews

Conflicts