Require mempool lock to be held when invoking TRUC checks
What the developer wrote
Authored by Suhas Daftuar
Require mempool lock to be held when invoking TRUC checks
What changed, and why it matters
This commit changes two Bitcoin Core functions that enforce special rules for a new type of transaction called TRUC. Previously, one function locked the memory pool (mempool) itself. Now both functions require their callers to already hold the mempool lock. This is a defensive coding change that makes it explicit who is responsible for thread safety, but it does not by itself fix a known exploitable bug.
Review all call sites of SingleTRUCChecks and PackageTRUCChecks to confirm they already acquire pool.cs before calling these functions, and run the project's thread-safety/static-analysis tooling to verify the annotations are satisfied. No emergency deployment is indicated by this commit alone.
Security signals we found
Change in concurrency/locking contract for mempool-critical validation functions
Addition of EXCLUSIVE_LOCKS_REQUIRED annotations for static thread-safety enforcement
Removal of internal lock acquisition in favor of caller-held lock assertions
Evidence from the diff
The patch replaces an internal LOCK(pool.cs) in SingleTRUCChecks with AssertLockHeld(pool.cs) and adds the same assertion plus EXCLUSIVE_LOCKS_REQUIRED(pool.cs) annotations in the header for both SingleTRUCChecks and PackageTRUCChecks. The effect is to shift the locking contract from callee-locked to caller-locked, which can prevent subtle double-lock or lock-order issues and make the code easier to reason about under Clang’s thread-safety analysis. The diff alone does not show a specific vulnerability being closed.
Changed components
src/policy/truc_policy.cppsrc/policy/truc_policy.hInspect captured patch +4 / −3
diff --git a/src/policy/truc_policy.cpp b/src/policy/truc_policy.cpp
index c5ba866a..bc5142c7 100644
--- a/src/policy/truc_policy.cpp
+++ b/src/policy/truc_policy.cpp
@@ -59,6 +59,7 @@ std::optional<std::string> PackageTRUCChecks(const CTxMemPool& pool, const CTran
const Package& package,
const std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef>& mempool_parents)
{
+ AssertLockHeld(pool.cs);
// This function is specialized for these limits, and must be reimplemented if they ever change.
static_assert(TRUC_ANCESTOR_LIMIT == 2);
static_assert(TRUC_DESCENDANT_LIMIT == 2);
@@ -173,7 +174,7 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
const std::set<Txid>& direct_conflicts,
int64_t vsize)
{
- LOCK(pool.cs);
+ AssertLockHeld(pool.cs);
// Check TRUC and non-TRUC inheritance.
for (const auto& entry_ref : mempool_parents) {
const auto& entry = &entry_ref.get();
diff --git a/src/policy/truc_policy.h b/src/policy/truc_policy.h
index 8b2ec097..9eec8d2f 100644
--- a/src/policy/truc_policy.h
+++ b/src/policy/truc_policy.h
@@ -66,7 +66,7 @@ static_assert(TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE <= DEFAULT_CLUSTER_SIZE_LIMI
std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx,
const std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef>& mempool_parents,
const std::set<Txid>& direct_conflicts,
- int64_t vsize);
+ int64_t vsize) EXCLUSIVE_LOCKS_REQUIRED(pool.cs);
/** Must be called for every transaction that is submitted within a package, even if not TRUC.
*
@@ -91,6 +91,6 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
* */
std::optional<std::string> PackageTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx, int64_t vsize,
const Package& package,
- const std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef>& mempool_parents);
+ const std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef>& mempool_parents) EXCLUSIVE_LOCKS_REQUIRED(pool.cs);
#endif // BITCOIN_POLICY_TRUC_POLICY_H
Why this scored 35/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.