Audit report AR-28

Reported privately in an independent security review, 2026-08-31.

Report date
2026-08-31
Kind
API
Status on master
Present
Proposed issue state
Open

Summary

The Mining interface's transaction lookups by txid and wtxid promise one nullable result per requested id, but return an empty list when the node has no mempool, which breaks callers that match results by position. The report is accurate and calls it API contract hardening. A running node always has a mempool, so only contexts built without one are affected, and the same fix is already proposed publicly in bitcoin#36132.

The report

getTransactionsByTxID() and getTransactionsByWitnessID() in src/node/interfaces.cpp are documented to return one entry per requested id, null for ids not found. Their missing-mempool branch returns an empty vector instead, so a caller mapping results back to requests by index gets the wrong count.

The report says the state it demonstrates belongs to a direct in-process caller with no mempool, not to a fully initialized node or an established external IPC path; that the wrong result count breaks positional mapping but causes no crash, funds loss, consensus or availability effect; and that ordinary production reach is uncertain.

The reproduction is a Boost unit test, transaction_lookups_without_mempool, added to src/test/miner_tests.cpp. It builds a mining interface over a node context with no mempool and checks the result count and null entries for both id types.

Proposed change

Size the result from the request before the mempool check, and fill the same positions on the normal path:

     std::vector<CTransactionRef> getTransactionsByTxID(const std::vector<Txid>& txids) override
     {
-        if (!m_node.mempool) return {};
-
-        std::vector<CTransactionRef> results;
-        results.reserve(txids.size());
+        std::vector<CTransactionRef> results(txids.size());
+        if (!m_node.mempool) {
+            Assume(results.size() == txids.size());
+            return results;
+        }
         LOCK(m_node.mempool->cs);
-        for (const auto& txid : txids) {
-            results.emplace_back(m_node.mempool->get(txid));
+        for (size_t i{0}; i < txids.size(); ++i) {
+            results[i] = m_node.mempool->get(txids[i]);
         }

getTransactionsByWitnessID gets the same change. The report defers to bitcoin#36132, which addresses the same contract, as the place for further work.

Assessment

Present on Bitcoin Core master: both methods start with if (!m_node.mempool) return {};. bitcoin-node always creates a mempool, so an IPC client talking to a running node never sees this; only in-process callers with a partial node context (for example tests) do. No stability effect; outside the survey's scope. The fix is correct and small.

Status

Present on master. The same change is in review publicly in bitcoin#36132 (open); the private report was closed in its favor on 2026-10-11.

Reported privately in an independent security review on 2026-08-31.