Give separate reject reasons to each TRUC check

Author: luke-jr

src/policy/truc_policy.cpp

@@ -56,6 +56,7 @@ struct ParentInfo {
 };
 
 std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t vsize,
+                                           const std::string& reason_prefix, std::string& out_reason,
                                            const Package& package,
                                            const CTxMemPool::setEntries& mempool_ancestors)
 {
@@ -69,11 +70,13 @@ std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t
     if (ptx->version == TRUC_VERSION) {
         // SingleTRUCChecks should have checked this already.
         if (!Assume(vsize <= TRUC_MAX_VSIZE)) {
+            out_reason = reason_prefix + "vsize-toobig";
             return strprintf("version=3 tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
                              ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, TRUC_MAX_VSIZE);
         }
 
         if (mempool_ancestors.size() + in_package_parents.size() + 1 > TRUC_ANCESTOR_LIMIT) {
+            out_reason = reason_prefix + "ancestors-toomany";
             return strprintf("tx %s (wtxid=%s) would have too many ancestors",
                              ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString());
         }
@@ -82,6 +85,7 @@ std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t
         if (has_parent) {
             // A TRUC child cannot be too large.
             if (vsize > TRUC_CHILD_MAX_VSIZE) {
+                out_reason = reason_prefix + "child-toobig";
                 return strprintf("version=3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
                                  ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
                                  vsize, TRUC_CHILD_MAX_VSIZE);
@@ -107,6 +111,7 @@ std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t
 
             // If there is a parent, it must have the right version.
             if (parent_info.m_version != TRUC_VERSION) {
+                out_reason = reason_prefix + "spends-nontruc";
                 return strprintf("version=3 tx %s (wtxid=%s) cannot spend from non-version=3 tx %s (wtxid=%s)",
                                  ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
                                  parent_info.m_txid.ToString(), parent_info.m_wtxid.ToString());
@@ -121,20 +126,23 @@ std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t
                     // sibling is to-be-replaced (done in SingleTRUCChecks) because these transactions
                     // are within the same package.
                     if (input.prevout.hash == parent_info.m_txid) {
+                        out_reason = reason_prefix + "sibling-known";
                         return strprintf("tx %s (wtxid=%s) would exceed descendant count limit",
                                          parent_info.m_txid.ToString(),
                                          parent_info.m_wtxid.ToString());
                     }
 
                     // This tx can't have both a parent and an in-package child.
                     if (input.prevout.hash == ptx->GetHash()) {
+                        out_reason = reason_prefix + "parent-and-child-both";
                         return strprintf("tx %s (wtxid=%s) would have too many ancestors",
                                          package_tx->GetHash().ToString(), package_tx->GetWitnessHash().ToString());
                     }
                 }
             }
 
             if (parent_info.m_has_mempool_descendant) {
+                out_reason = reason_prefix + "descendant-toomany";
                 return strprintf("tx %s (wtxid=%s) would exceed descendant count limit",
                                 parent_info.m_txid.ToString(), parent_info.m_wtxid.ToString());
             }
@@ -143,13 +151,15 @@ std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t
         // Non-TRUC transactions cannot have TRUC parents.
         for (auto it : mempool_ancestors) {
             if (it->GetTx().version == TRUC_VERSION) {
+                out_reason = reason_prefix + "spent-by-nontruc";
                 return strprintf("non-version=3 tx %s (wtxid=%s) cannot spend from version=3 tx %s (wtxid=%s)",
                                  ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
                                  it->GetSharedTx()->GetHash().ToString(), it->GetSharedTx()->GetWitnessHash().ToString());
             }
         }
         for (const auto& index: in_package_parents) {
             if (package.at(index)->version == TRUC_VERSION) {
+                out_reason = reason_prefix + "spent-by-nontruc";
                 return strprintf("non-version=3 tx %s (wtxid=%s) cannot spend from version=3 tx %s (wtxid=%s)",
                                  ptx->GetHash().ToString(),
                                  ptx->GetWitnessHash().ToString(),
@@ -162,18 +172,21 @@ std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t
 }
 
 std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTransactionRef& ptx,
+                                          const std::string& reason_prefix, std::string& out_reason,
                                           const CTxMemPool::setEntries& mempool_ancestors,
                                           const std::set<Txid>& direct_conflicts,
                                           int64_t vsize)
 {
     // Check TRUC and non-TRUC inheritance.
     for (const auto& entry : mempool_ancestors) {
         if (ptx->version != TRUC_VERSION && entry->GetTx().version == TRUC_VERSION) {
+            out_reason = reason_prefix + "spent-by-nontruc";
             return std::make_pair(strprintf("non-version=3 tx %s (wtxid=%s) cannot spend from version=3 tx %s (wtxid=%s)",
                              ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
                              entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()),
                 nullptr);
         } else if (ptx->version == TRUC_VERSION && entry->GetTx().version != TRUC_VERSION) {
+            out_reason = reason_prefix + "spends-nontruc";
             return std::make_pair(strprintf("version=3 tx %s (wtxid=%s) cannot spend from non-version=3 tx %s (wtxid=%s)",
                              ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
                              entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()),
@@ -189,13 +202,15 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
     if (ptx->version != TRUC_VERSION) return std::nullopt;
 
     if (vsize > TRUC_MAX_VSIZE) {
+        out_reason = reason_prefix + "vsize-toobig";
         return std::make_pair(strprintf("version=3 tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
                          ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, TRUC_MAX_VSIZE),
             nullptr);
     }
 
     // Check that TRUC_ANCESTOR_LIMIT would not be violated.
     if (mempool_ancestors.size() + 1 > TRUC_ANCESTOR_LIMIT) {
+        out_reason = reason_prefix + "ancestors-toomany";
         return std::make_pair(strprintf("tx %s (wtxid=%s) would have too many ancestors",
                          ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString()),
             nullptr);
@@ -205,6 +220,7 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
     if (mempool_ancestors.size() > 0) {
         // If this transaction spends TRUC parents, it cannot be too large.
         if (vsize > TRUC_CHILD_MAX_VSIZE) {
+            out_reason = reason_prefix + "child-toobig";
             return std::make_pair(strprintf("version=3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
                              ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, TRUC_CHILD_MAX_VSIZE),
                 nullptr);
@@ -233,6 +249,7 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
 
             // Return the sibling if its eviction can be considered. Provide the "descendant count
             // limit" string either way, as the caller may decide not to do sibling eviction.
+            out_reason = reason_prefix + "descendants-toomany";
             return std::make_pair(strprintf("tx %u (wtxid=%s) would exceed descendant count limit",
                                             parent_entry->GetSharedTx()->GetHash().ToString(),
                                             parent_entry->GetSharedTx()->GetWitnessHash().ToString()),

src/policy/truc_policy.h

@@ -62,6 +62,7 @@ static_assert(TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE <= DEFAULT_DESCENDANT_SIZE_L
  *   applicable.
  */
 std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTransactionRef& ptx,
+                                          const std::string& reason_prefix, std::string& out_reason,
                                           const CTxMemPool::setEntries& mempool_ancestors,
                                           const std::set<Txid>& direct_conflicts,
                                           int64_t vsize);
@@ -88,6 +89,7 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
  * @returns debug string if an error occurs, std::nullopt otherwise.
  * */
 std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t vsize,
+                                           const std::string& reason_prefix, std::string& out_reason,
                                            const Package& package,
                                            const CTxMemPool::setEntries& mempool_ancestors);
 

src/test/txvalidation_tests.cpp

@@ -20,6 +20,18 @@
 
 BOOST_AUTO_TEST_SUITE(txvalidation_tests)
 
+std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTransactionRef& ptx, const CTxMemPool::setEntries& mempool_ancestors, const std::set<Txid>& direct_conflicts, int64_t vsize)
+{
+    std::string dummy;
+    return SingleTRUCChecks(ptx, dummy, dummy, mempool_ancestors, direct_conflicts, vsize);
+}
+
+std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t vsize, const Package& package, const CTxMemPool::setEntries& mempool_ancestors)
+{
+    std::string dummy;
+    return PackageTRUCChecks(ptx, vsize, dummy, dummy, package, mempool_ancestors);
+}
+
 /**
  * Ensure that the mempool won't accept coinbase transactions.
  */

src/validation.cpp

@@ -1063,7 +1063,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     // Even though just checking direct mempool parents for inheritance would be sufficient, we
     // check using the full ancestor set here because it's more convenient to use what we have
     // already calculated.
-    if (const auto err{SingleTRUCChecks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
+    if (const auto err{SingleTRUCChecks(ws.m_ptx, "truc-", reason, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
         // Single transaction contexts only.
         if (args.m_allow_sibling_eviction && err->second != nullptr) {
             // We should only be considering where replacement is considered valid as well.
@@ -1082,7 +1082,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
             // (which is normally done in PreChecks). However, the only way a TRUC transaction can
             // have a non-TRUC and non-BIP125 descendant is due to a reorg.
         } else {
-            return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "TRUC-violation", err->first);
+            return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, reason, err->first);
         }
     }
 
@@ -1588,9 +1588,10 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
 
     // At this point we have all in-mempool ancestors, and we know every transaction's vsize.
     // Run the TRUC checks on the package.
+    std::string reason;
     for (Workspace& ws : workspaces) {
-        if (auto err{PackageTRUCChecks(ws.m_ptx, ws.m_vsize, txns, ws.m_ancestors)}) {
-            package_state.Invalid(PackageValidationResult::PCKG_POLICY, "TRUC-violation", err.value());
+        if (auto err{PackageTRUCChecks(ws.m_ptx, ws.m_vsize, "truc-", reason, txns, ws.m_ancestors)}) {
+            package_state.Invalid(PackageValidationResult::PCKG_POLICY, reason, err.value());
             return PackageMempoolAcceptResult(package_state, {});
         }
     }