1 files changed, 32 insertions, 17 deletions

diff --git a/src/policy/v3_policy.cpp b/src/policy/v3_policy.cpp
index 158881aeb9..3c3942d707 100644
--- a/src/policy/v3_policy.cpp
+++ b/src/policy/v3_policy.cpp
@@ -81,9 +81,9 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
                                  vsize, V3_CHILD_MAX_VSIZE);
             }
 
+            // Exactly 1 parent exists, either in mempool or package. Find it.
             const auto parent_info = [&] {
                 if (mempool_ancestors.size() > 0) {
-                    // There's a parent in the mempool.
                     auto& mempool_parent = *mempool_ancestors.begin();
                     Assume(mempool_parent->GetCountWithDescendants() == 1);
                     return ParentInfo{mempool_parent->GetTx().GetHash(),
@@ -91,7 +91,6 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
                                       mempool_parent->GetTx().nVersion,
                                       /*has_mempool_descendant=*/mempool_parent->GetCountWithDescendants() > 1};
                 } else {
-                    // Ancestor must be in the package. Find it.
                     auto& parent_index = in_package_parents.front();
                     auto& package_parent = package.at(parent_index);
                     return ParentInfo{package_parent->GetHash(),
@@ -131,10 +130,11 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
             }
 
             // It shouldn't be possible to have any mempool siblings at this point. SingleV3Checks
-            // catches mempool siblings. Also, if the package consists of connected transactions,
+            // catches mempool siblings and sibling eviction is not extended to packages. Also, if the package consists of connected transactions,
             // any tx having a mempool ancestor would mean the package exceeds ancestor limits.
             if (!Assume(!parent_info.m_has_mempool_descendant)) {
-                return strprintf("tx %u would exceed descendant count limit", parent_info.m_wtxid.ToString());
+                return strprintf("tx %s (wtxid=%s) would exceed descendant count limit",
+                                parent_info.m_txid.ToString(), parent_info.m_wtxid.ToString());
             }
         }
     } else {
@@ -159,7 +159,7 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
     return std::nullopt;
 }
 
-std::optional<std::string> SingleV3Checks(const CTransactionRef& ptx,
+std::optional<std::pair<std::string, CTransactionRef>> SingleV3Checks(const CTransactionRef& ptx,
                                           const CTxMemPool::setEntries& mempool_ancestors,
                                           const std::set<Txid>& direct_conflicts,
                                           int64_t vsize)
@@ -167,13 +167,15 @@ std::optional<std::string> SingleV3Checks(const CTransactionRef& ptx,
     // Check v3 and non-v3 inheritance.
     for (const auto& entry : mempool_ancestors) {
         if (ptx->nVersion != 3 && entry->GetTx().nVersion == 3) {
-            return strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
+            return std::make_pair(strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
                              ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
-                             entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString());
+                             entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()),
+                nullptr);
         } else if (ptx->nVersion == 3 && entry->GetTx().nVersion != 3) {
-            return strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
+            return std::make_pair(strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
                              ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
-                             entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString());
+                             entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()),
+                nullptr);
         }
     }
 
@@ -184,18 +186,20 @@ std::optional<std::string> SingleV3Checks(const CTransactionRef& ptx,
     // The rest of the rules only apply to transactions with nVersion=3.
     if (ptx->nVersion != 3) return std::nullopt;
 
-    // Check that V3_ANCESTOR_LIMIT would not be violated, including both in-package and in-mempool.
+    // Check that V3_ANCESTOR_LIMIT would not be violated.
     if (mempool_ancestors.size() + 1 > V3_ANCESTOR_LIMIT) {
-        return strprintf("tx %s (wtxid=%s) would have too many ancestors",
-                         ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString());
+        return std::make_pair(strprintf("tx %s (wtxid=%s) would have too many ancestors",
+                         ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString()),
+            nullptr);
     }
 
     // Remaining checks only pertain to transactions with unconfirmed ancestors.
     if (mempool_ancestors.size() > 0) {
         // If this transaction spends V3 parents, it cannot be too large.
         if (vsize > V3_CHILD_MAX_VSIZE) {
-            return strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
-                             ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, V3_CHILD_MAX_VSIZE);
+            return std::make_pair(strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
+                             ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, V3_CHILD_MAX_VSIZE),
+                nullptr);
         }
 
         // Check the descendant counts of in-mempool ancestors.
@@ -211,9 +215,20 @@ std::optional<std::string> SingleV3Checks(const CTransactionRef& ptx,
             std::any_of(children.cbegin(), children.cend(),
                 [&direct_conflicts](const CTxMemPoolEntry& child){return direct_conflicts.count(child.GetTx().GetHash()) > 0;});
         if (parent_entry->GetCountWithDescendants() + 1 > V3_DESCENDANT_LIMIT && !child_will_be_replaced) {
-            return strprintf("tx %u (wtxid=%s) would exceed descendant count limit",
-                             parent_entry->GetSharedTx()->GetHash().ToString(),
-                             parent_entry->GetSharedTx()->GetWitnessHash().ToString());
+            // Allow sibling eviction for v3 transaction: if another child already exists, even if
+            // we don't conflict inputs with it, consider evicting it under RBF rules. We rely on v3 rules
+            // only permitting 1 descendant, as otherwise we would need to have logic for deciding
+            // which descendant to evict. Skip if this isn't true, e.g. if the transaction has
+            // multiple children or the sibling also has descendants due to a reorg.
+            const bool consider_sibling_eviction{parent_entry->GetCountWithDescendants() == 2 &&
+                children.begin()->get().GetCountWithAncestors() == 2};
+
+            // 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.
+            return std::make_pair(strprintf("tx %u (wtxid=%s) would exceed descendant count limit",
+                                            parent_entry->GetSharedTx()->GetHash().ToString(),
+                                            parent_entry->GetSharedTx()->GetWitnessHash().ToString()),
+                                  consider_sibling_eviction ?  children.begin()->get().GetSharedTx() : nullptr);
         }
     }
     return std::nullopt;