Minimize mempool lock, sync txo spender index only when and if needed
What the developer wrote
Authored by sstone
Minimize mempool lock, sync txo spender index only when and if needed
We sync txospenderindex after we've checked the mempool for spending transaction, and only if search is not limited to the mempool and no
spending transactions have been found for some of the provided outpoints.
This should minimize the chance of having a block containing a spending transaction that is no longer in the mempool but has not been indexed yet.
What changed, and why it matters
This commit tightens the timing of when Bitcoin Core's RPC call 'gettxspendingprevout' consults the optional on-disk 'txo spender index'. Previously, the code would wait for that index to finish syncing before even looking at the mempool. Now it searches the mempool first, releases the mempool lock, and only then waits for the index if it actually needs to. The goal is to reduce a small window where a block has just arrived, the spending transaction left the mempool, but the index hasn't recorded it yet, so the RPC could incorrectly report no spender. It is a robustness improvement, not a fix for a clear exploit.
Treat as a normal correctness/robustness patch. Reviewers should verify that BlockUntilSyncedToCurrentChain is now invoked in all code paths where the index is actually queried, and that the mempool lock is not held during the index sync. No urgent security deployment is indicated by the diff alone.
Security signals we found
Race condition between mempool eviction and txo-spender-index sync
Reduced lock contention on mempool.cs
RPC result correctness for spent-outpoint lookups
No explicit vulnerability or exploit mechanism in diff
Evidence from the diff
The change refactors gettxspendingprevout in src/rpc/mempool.cpp. It removes the upfront BlockUntilSyncedToCurrentChain() call on g_txospenderindex, instead checking the mempool under mempool.cs first and recording which prevouts are missing. Only if the caller did not request mempool_only and at least one prevout was missing from the mempool does it call BlockUntilSyncedToCurrentChain() and then query the index. The lock scope is narrowed to just the mempool scan. A functional test is added to verify that mempool_only=true does not return confirmed spenders.
Changed components
src/rpc/mempool.cppRPC gettxspendingprevoutCTxMemPool::GetConflictTxg_txospenderindexInspect captured patch +44 / −23
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 68d6d4d5..2fec90b2 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -31,6 +31,7 @@
#include <util/time.h>
#include <util/vector.h>
+#include <map>
#include <string_view>
#include <utility>
@@ -826,11 +827,15 @@ static RPCHelpMan gettxspendingprevout()
{"return_spending_tx", UniValueType(UniValue::VBOOL)},
}, /*fAllowNull=*/true, /*fStrict=*/true);
- const bool txospenderindex_ready{g_txospenderindex && g_txospenderindex->BlockUntilSyncedToCurrentChain()};
- const bool mempool_only{options.exists("mempool_only") ? options["mempool_only"].get_bool() : !txospenderindex_ready};
+ const bool mempool_only{options.exists("mempool_only") ? options["mempool_only"].get_bool() : !g_txospenderindex};
const bool return_spending_tx{options.exists("return_spending_tx") ? options["return_spending_tx"].get_bool() : false};
- std::vector<COutPoint> prevouts;
+ struct Entry {
+ const COutPoint prevout;
+ const UniValue& input;
+ UniValue output;
+ };
+ std::vector<Entry> prevouts;
prevouts.reserve(output_params.size());
for (unsigned int idx = 0; idx < output_params.size(); idx++) {
@@ -847,33 +852,45 @@ static RPCHelpMan gettxspendingprevout()
if (nOutput < 0) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout cannot be negative");
}
-
- prevouts.emplace_back(txid, nOutput);
+ prevouts.emplace_back(COutPoint{txid, uint32_t(nOutput)}, o, UniValue{});
}
- const CTxMemPool& mempool = EnsureAnyMemPool(request.context);
- LOCK(mempool.cs);
-
- UniValue result{UniValue::VARR};
-
- for (const COutPoint& prevout : prevouts) {
- UniValue o(UniValue::VOBJ);
- o.pushKV("txid", prevout.hash.ToString());
- o.pushKV("vout", prevout.n);
-
- const CTransaction* spendingTx = mempool.GetConflictTx(prevout);
- if (spendingTx != nullptr) {
- o.pushKV("spendingtxid", spendingTx->GetHash().ToString());
- if (return_spending_tx) {
- o.pushKV("spendingtx", EncodeHexTx(*spendingTx));
+ // search the mempool first
+ bool missing_from_mempool{false};
+ {
+ const CTxMemPool& mempool = EnsureAnyMemPool(request.context);
+ LOCK(mempool.cs);
+ for (auto& entry : prevouts) {
+ const CTransaction* spendingTx = mempool.GetConflictTx(entry.prevout);
+ if (spendingTx != nullptr) {
+ UniValue o{entry.input};
+ o.pushKV("spendingtxid", spendingTx->GetHash().ToString());
+ if (return_spending_tx) {
+ o.pushKV("spendingtx", EncodeHexTx(*spendingTx));
+ }
+ entry.output = std::move(o);
+ } else {
+ missing_from_mempool = true;
}
- } else if (mempool_only) {
+ }
+ }
+ // if search is not limited to the mempool and no spender was found for an outpoint, search the txospenderindex
+ // we call g_txospenderindex->BlockUntilSyncedToCurrentChain() only if g_txospenderindex is going to be used
+ UniValue result{UniValue::VARR};
+ bool txospenderindex_ready{mempool_only || !missing_from_mempool || (g_txospenderindex && g_txospenderindex->BlockUntilSyncedToCurrentChain())};
+ for (auto& entry : prevouts) {
+ if (!entry.output.isNull()) {
+ result.push_back(std::move(entry.output));
+ continue;
+ }
+ UniValue o{entry.input};
+ if (mempool_only) {
// do nothing, caller has selected to only query the mempool
} else if (!txospenderindex_ready) {
- throw JSONRPCError(RPC_MISC_ERROR, strprintf("No spending tx for the outpoint %s:%d in mempool, and txospenderindex is unavailable.", prevout.hash.GetHex(), prevout.n));
+ throw JSONRPCError(RPC_MISC_ERROR, strprintf("No spending tx for the outpoint %s:%d in mempool, and txospenderindex is unavailable.", entry.prevout.hash.GetHex(), entry.prevout.n));
} else {
// no spending tx in mempool, query txospender index
- const auto spender{g_txospenderindex->FindSpender(prevout)};
+ const auto spender{g_txospenderindex->FindSpender(entry.prevout)};
if (!spender) {
throw JSONRPCError(RPC_MISC_ERROR, spender.error());
}
diff --git a/test/functional/rpc_gettxspendingprevout.py b/test/functional/rpc_gettxspendingprevout.py
index 42efba37..05697dd8 100755
--- a/test/functional/rpc_gettxspendingprevout.py
+++ b/test/functional/rpc_gettxspendingprevout.py
@@ -116,6 +116,10 @@ class GetTxSpendingPrevoutTest(BitcoinTestFramework):
result = self.nodes[2].gettxspendingprevout([{ 'txid' : confirmed_utxo['txid'], 'vout' : 0}, {'txid' : txidA, 'vout' : 1} ], return_spending_tx=True)
assert_equal(result, [ {'txid' : confirmed_utxo['txid'], 'vout' : 0}, {'txid' : txidA, 'vout' : 1}])
+ # spending transaction is not found if we only search the mempool
+ result = self.nodes[0].gettxspendingprevout([ {'txid' : confirmed_utxo['txid'], 'vout' : 0}, {'txid' : txidA, 'vout' : 1} ], return_spending_tx=True, mempool_only=True)
+ assert_equal(result, [ {'txid' : confirmed_utxo['txid'], 'vout' : 0}, {'txid' : txidA, 'vout' : 1}])
+
self.log.info("Check that our txospenderindex is updated when a reorg replaces a spending transaction")
confirmed_utxo = self.wallet.get_utxo(mark_as_spent = False)
tx1 = create_tx(utxos_to_spend=[confirmed_utxo], num_outputs=1)
Why this scored 25/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.