highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-05-15 06:11:09 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#29306: policy: enable sibling eviction for v3 transactions

Browse Source 
1342a31f3ab61d964b9a24825bee24f53ba4e1cc [functional test] sibling eviction (glozow)
5fbab378597126eb1d0c2b2addb0859f79e508f4 [unit test] sibling not returned from SingleV3Checks if 1p2c or 3gen (glozow)
170306728aa23a4d5fcc383ddabd97f66fed5119 [policy] sibling eviction for v3 transactions (glozow)
b5d15f764fed0b30c9113429163700dea907c2b1 [refactor] return pair from SingleV3Checks (glozow)

Pull request description:

  When we receive a v3 transaction that would bust a mempool transaction's descendant limit, instead of rejecting the new tx, consider replacing the other descendant if it is much higher feerate (using existing RBF criteria to assess that it's more incentive compatible and to avoid DoS).

  Delving post with more background and motivation: https://delvingbitcoin.org/t/sibling-eviction-for-v3-transactions/472

ACKs for top commit:
  sdaftuar:
    ACK 1342a31f3ab61d964b9a24825bee24f53ba4e1cc
  achow101:
    ACK 1342a31f3ab61d964b9a24825bee24f53ba4e1cc
  instagibbs:
    ACK 1342a31f3a

Tree-SHA512: dd957d49e51db78758f566c49bddc579b72478e371275c592d3d5ba097d20de47a6c81952045021b99d82a787f5b799baf16dd0ee0e6de90ba12e21e275352be
This commit is contained in:
Ava Chow 2024-03-12 12:06:45 -04:00
commit 12dae637a4
No known key found for this signature in database
GPG Key ID: 17565732E08E5E41
5 changed files with 314 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
src/policy/v3_policy.cpp
View File

@ -130,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 {
@ -158,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)
@ -166,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);
}
}
@ -185,16 +188,18 @@ std::optional<std::string> SingleV3Checks(const CTransactionRef& ptx,
// 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.
@ -210,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;

10
src/policy/v3_policy.h
View File

@ -48,9 +48,15 @@ static_assert(V3_CHILD_MAX_VSIZE + MAX_STANDARD_TX_WEIGHT / WITNESS_SCALE_FACTOR
* count of in-mempool ancestors.
* @param[in] vsize The sigop-adjusted virtual size of ptx.
*
* @returns debug string if an error occurs, std::nullopt otherwise.
* @returns 3 possibilities:
* - std::nullopt if all v3 checks were applied successfully
* - debug string + pointer to a mempool sibling if this transaction would be the second child in a
* 1-parent-1-child cluster; the caller may consider evicting the specified sibling or return an
* error with the debug string.
* - debug string + nullptr if this transaction violates some v3 rule and sibling eviction is not
* applicable.
*/
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);

90
src/test/txvalidation_tests.cpp
View File

@ -115,7 +115,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
const auto expected_error_str{strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
tx_v2_from_v3->GetHash().ToString(), tx_v2_from_v3->GetWitnessHash().ToString(),
mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())};
BOOST_CHECK(*SingleV3Checks(tx_v2_from_v3, *ancestors_v2_from_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v3)) == expected_error_str);
auto result_v2_from_v3{SingleV3Checks(tx_v2_from_v3, *ancestors_v2_from_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v3))};
BOOST_CHECK_EQUAL(result_v2_from_v3->first, expected_error_str);
BOOST_CHECK_EQUAL(result_v2_from_v3->second, nullptr);
Package package_v3_v2{mempool_tx_v3, tx_v2_from_v3};
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), package_v3_v2, empty_ancestors), expected_error_str);
@ -130,8 +132,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
const auto expected_error_str_2{strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
tx_v2_from_v2_and_v3->GetHash().ToString(), tx_v2_from_v2_and_v3->GetWitnessHash().ToString(),
mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())};
BOOST_CHECK(*SingleV3Checks(tx_v2_from_v2_and_v3, *ancestors_v2_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3))
== expected_error_str_2);
auto result_v2_from_both{SingleV3Checks(tx_v2_from_v2_and_v3, *ancestors_v2_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3))};
BOOST_CHECK_EQUAL(result_v2_from_both->first, expected_error_str_2);
BOOST_CHECK_EQUAL(result_v2_from_both->second, nullptr);
Package package_v3_v2_v2{mempool_tx_v3, mempool_tx_v2, tx_v2_from_v2_and_v3};
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v2_from_v2_and_v3, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3), package_v3_v2_v2, empty_ancestors), expected_error_str_2);
@ -147,7 +150,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
const auto expected_error_str{strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
tx_v3_from_v2->GetHash().ToString(), tx_v3_from_v2->GetWitnessHash().ToString(),
mempool_tx_v2->GetHash().ToString(), mempool_tx_v2->GetWitnessHash().ToString())};
BOOST_CHECK(*SingleV3Checks(tx_v3_from_v2, *ancestors_v3_from_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2)) == expected_error_str);
auto result_v3_from_v2{SingleV3Checks(tx_v3_from_v2, *ancestors_v3_from_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2))};
BOOST_CHECK_EQUAL(result_v3_from_v2->first, expected_error_str);
BOOST_CHECK_EQUAL(result_v3_from_v2->second, nullptr);
Package package_v2_v3{mempool_tx_v2, tx_v3_from_v2};
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_from_v2, GetVirtualTransactionSize(*tx_v3_from_v2), package_v2_v3, empty_ancestors), expected_error_str);
@ -162,8 +167,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
const auto expected_error_str_2{strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
tx_v3_from_v2_and_v3->GetHash().ToString(), tx_v3_from_v2_and_v3->GetWitnessHash().ToString(),
mempool_tx_v2->GetHash().ToString(), mempool_tx_v2->GetWitnessHash().ToString())};
BOOST_CHECK(*SingleV3Checks(tx_v3_from_v2_and_v3, *ancestors_v3_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3))
== expected_error_str_2);
auto result_v3_from_both{SingleV3Checks(tx_v3_from_v2_and_v3, *ancestors_v3_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3))};
BOOST_CHECK_EQUAL(result_v3_from_both->first, expected_error_str_2);
BOOST_CHECK_EQUAL(result_v3_from_both->second, nullptr);
// tx_v3_from_v2_and_v3 also violates V3_ANCESTOR_LIMIT.
const auto expected_error_str_3{strprintf("tx %s (wtxid=%s) would have too many ancestors",
@ -215,8 +221,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
BOOST_CHECK_EQUAL(ancestors->size(), 3);
const auto expected_error_str{strprintf("tx %s (wtxid=%s) would have too many ancestors",
tx_v3_multi_parent->GetHash().ToString(), tx_v3_multi_parent->GetWitnessHash().ToString())};
BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_multi_parent, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_parent)),
expected_error_str);
auto result{SingleV3Checks(tx_v3_multi_parent, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_parent))};
BOOST_CHECK_EQUAL(result->first, expected_error_str);
BOOST_CHECK_EQUAL(result->second, nullptr);
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_multi_parent, GetVirtualTransactionSize(*tx_v3_multi_parent), package_multi_parents, empty_ancestors),
expected_error_str);
@ -239,8 +246,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_multi_gen), m_limits)};
const auto expected_error_str{strprintf("tx %s (wtxid=%s) would have too many ancestors",
tx_v3_multi_gen->GetHash().ToString(), tx_v3_multi_gen->GetWitnessHash().ToString())};
BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_multi_gen, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_gen)),
expected_error_str);
auto result{SingleV3Checks(tx_v3_multi_gen, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_gen))};
BOOST_CHECK_EQUAL(result->first, expected_error_str);
BOOST_CHECK_EQUAL(result->second, nullptr);
// Middle tx is what triggers a failure for the grandchild:
BOOST_CHECK_EQUAL(*PackageV3Checks(middle_tx, GetVirtualTransactionSize(*middle_tx), package_multi_gen, empty_ancestors), expected_error_str);
@ -256,8 +264,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child_big), m_limits)};
const auto expected_error_str{strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
tx_v3_child_big->GetHash().ToString(), tx_v3_child_big->GetWitnessHash().ToString(), vsize, V3_CHILD_MAX_VSIZE)};
BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_child_big, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child_big)),
expected_error_str);
auto result{SingleV3Checks(tx_v3_child_big, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child_big))};
BOOST_CHECK_EQUAL(result->first, expected_error_str);
BOOST_CHECK_EQUAL(result->second, nullptr);
Package package_child_big{mempool_tx_v3, tx_v3_child_big};
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_child_big, GetVirtualTransactionSize(*tx_v3_child_big), package_child_big, empty_ancestors),
@ -298,9 +307,10 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
const auto expected_error_str{strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
tx_many_sigops->GetHash().ToString(), tx_many_sigops->GetWitnessHash().ToString(),
total_sigops * DEFAULT_BYTES_PER_SIGOP / WITNESS_SCALE_FACTOR, V3_CHILD_MAX_VSIZE)};
BOOST_CHECK_EQUAL(*SingleV3Checks(tx_many_sigops, *ancestors, empty_conflicts_set,
GetVirtualTransactionSize(*tx_many_sigops, /*nSigOpCost=*/total_sigops, /*bytes_per_sigop=*/ DEFAULT_BYTES_PER_SIGOP)),
expected_error_str);
auto result{SingleV3Checks(tx_many_sigops, *ancestors, empty_conflicts_set,
GetVirtualTransactionSize(*tx_many_sigops, /*nSigOpCost=*/total_sigops, /*bytes_per_sigop=*/ DEFAULT_BYTES_PER_SIGOP))};
BOOST_CHECK_EQUAL(result->first, expected_error_str);
BOOST_CHECK_EQUAL(result->second, nullptr);
Package package_child_sigops{mempool_tx_v3, tx_many_sigops};
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_many_sigops, total_sigops * DEFAULT_BYTES_PER_SIGOP / WITNESS_SCALE_FACTOR, package_child_sigops, empty_ancestors),
@ -319,22 +329,58 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
BOOST_CHECK(PackageV3Checks(tx_mempool_v3_child, GetVirtualTransactionSize(*tx_mempool_v3_child), package_v3_1p1c, empty_ancestors) == std::nullopt);
}
// A v3 transaction cannot have more than 1 descendant.
// Configuration where tx has multiple direct children.
// A v3 transaction cannot have more than 1 descendant. Sibling is returned when exactly 1 exists.
{
auto tx_v3_child2 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 1}}, /*version=*/3);
auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child2), m_limits)};
// Configuration where parent already has 1 other child in mempool
auto ancestors_1sibling{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child2), m_limits)};
const auto expected_error_str{strprintf("tx %s (wtxid=%s) would exceed descendant count limit",
mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())};
BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_child2, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child2)),
expected_error_str);
// If replacing the child, make sure there is no double-counting.
BOOST_CHECK(SingleV3Checks(tx_v3_child2, *ancestors, {tx_mempool_v3_child->GetHash()}, GetVirtualTransactionSize(*tx_v3_child2))
auto result_with_sibling_eviction{SingleV3Checks(tx_v3_child2, *ancestors_1sibling, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child2))};
BOOST_CHECK_EQUAL(result_with_sibling_eviction->first, expected_error_str);
// The other mempool child is returned to allow for sibling eviction.
BOOST_CHECK_EQUAL(result_with_sibling_eviction->second, tx_mempool_v3_child);
// If directly replacing the child, make sure there is no double-counting.
BOOST_CHECK(SingleV3Checks(tx_v3_child2, *ancestors_1sibling, {tx_mempool_v3_child->GetHash()}, GetVirtualTransactionSize(*tx_v3_child2))
== std::nullopt);
Package package_v3_1p2c{mempool_tx_v3, tx_mempool_v3_child, tx_v3_child2};
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_child2, GetVirtualTransactionSize(*tx_v3_child2), package_v3_1p2c, empty_ancestors),
expected_error_str);
// Configuration where parent already has 2 other children in mempool (no sibling eviction allowed). This may happen as the result of a reorg.
pool.addUnchecked(entry.FromTx(tx_v3_child2));
auto tx_v3_child3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 24}}, /*version=*/3);
auto entry_mempool_parent = pool.GetIter(mempool_tx_v3->GetHash().ToUint256()).value();
BOOST_CHECK_EQUAL(entry_mempool_parent->GetCountWithDescendants(), 3);
auto ancestors_2siblings{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child3), m_limits)};
auto result_2children{SingleV3Checks(tx_v3_child3, *ancestors_2siblings, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child3))};
BOOST_CHECK_EQUAL(result_2children->first, expected_error_str);
// The other mempool child is not returned because sibling eviction is not allowed.
BOOST_CHECK_EQUAL(result_2children->second, nullptr);
}
// Sibling eviction: parent already has 1 other child, which also has its own child (no sibling eviction allowed). This may happen as the result of a reorg.
{
auto tx_mempool_grandparent = make_tx(random_outpoints(1), /*version=*/3);
auto tx_mempool_sibling = make_tx({COutPoint{tx_mempool_grandparent->GetHash(), 0}}, /*version=*/3);
auto tx_mempool_nibling = make_tx({COutPoint{tx_mempool_sibling->GetHash(), 0}}, /*version=*/3);
auto tx_to_submit = make_tx({COutPoint{tx_mempool_grandparent->GetHash(), 1}}, /*version=*/3);
pool.addUnchecked(entry.FromTx(tx_mempool_grandparent));
pool.addUnchecked(entry.FromTx(tx_mempool_sibling));
pool.addUnchecked(entry.FromTx(tx_mempool_nibling));
auto ancestors_3gen{pool.CalculateMemPoolAncestors(entry.FromTx(tx_to_submit), m_limits)};
const auto expected_error_str{strprintf("tx %s (wtxid=%s) would exceed descendant count limit",
tx_mempool_grandparent->GetHash().ToString(), tx_mempool_grandparent->GetWitnessHash().ToString())};
auto result_3gen{SingleV3Checks(tx_to_submit, *ancestors_3gen, empty_conflicts_set, GetVirtualTransactionSize(*tx_to_submit))};
BOOST_CHECK_EQUAL(result_3gen->first, expected_error_str);
// The other mempool child is not returned because sibling eviction is not allowed.
BOOST_CHECK_EQUAL(result_3gen->second, nullptr);
}
// Configuration where tx has multiple generations of descendants is not tested because that is

51
src/validation.cpp
View File

@ -589,12 +589,14 @@ private:
// of checking a given transaction.
struct Workspace {
explicit Workspace(const CTransactionRef& ptx) : m_ptx(ptx), m_hash(ptx->GetHash()) {}
/** Txids of mempool transactions that this transaction directly conflicts with. */
/** Txids of mempool transactions that this transaction directly conflicts with or may
* replace via sibling eviction. */
std::set<Txid> m_conflicts;
/** Iterators to mempool entries that this transaction directly conflicts with. */
/** Iterators to mempool entries that this transaction directly conflicts with or may
* replace via sibling eviction. */
CTxMemPool::setEntries m_iters_conflicting;
/** Iterators to all mempool entries that would be replaced by this transaction, including
* those it directly conflicts with and their descendants. */
* m_conflicts and their descendants. */
CTxMemPool::setEntries m_all_conflicting;
/** All mempool ancestors of this transaction. */
CTxMemPool::setEntries m_ancestors;
@ -602,9 +604,12 @@ private:
* inserted into the mempool until Finalize(). */
std::unique_ptr<CTxMemPoolEntry> m_entry;
/** Pointers to the transactions that have been removed from the mempool and replaced by
* this transaction, used to return to the MemPoolAccept caller. Only populated if
* this transaction (everything in m_all_conflicting), used to return to the MemPoolAccept caller. Only populated if
* validation is successful and the original transactions are removed. */
std::list<CTransactionRef> m_replaced_transactions;
/** Whether RBF-related data structures (m_conflicts, m_iters_conflicting, m_all_conflicting,
* m_replaced_transactions) include a sibling in addition to txns with conflicting inputs. */
bool m_sibling_eviction{false};
/** Virtual size of the transaction as used by the mempool, calculated using serialized size
* of the transaction and sigops. */
@ -694,7 +699,8 @@ private:
Chainstate& m_active_chainstate;
/** Whether the transaction(s) would replace any mempool transactions. If so, RBF rules apply. */
/** Whether the transaction(s) would replace any mempool transactions and/or evict any siblings.
* If so, RBF rules apply. */
bool m_rbf{false};
};
@ -958,8 +964,27 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
}
ws.m_ancestors = *ancestors;
if (const auto err_string{SingleV3Checks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "v3-rule-violation", *err_string);
// 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{SingleV3Checks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
// Disabled within package validation.
if (err->second != nullptr && args.m_allow_replacement) {
// Potential sibling eviction. Add the sibling to our list of mempool conflicts to be
// included in RBF checks.
ws.m_conflicts.insert(err->second->GetHash());
// Adding the sibling to m_iters_conflicting here means that it doesn't count towards
// RBF Carve Out above. This is correct, since removing to-be-replaced transactions from
// the descendant count is done separately in SingleV3Checks for v3 transactions.
ws.m_iters_conflicting.insert(m_pool.GetIter(err->second->GetHash()).value());
ws.m_sibling_eviction = true;
// The sibling will be treated as part of the to-be-replaced set in ReplacementChecks.
// Note that we are not checking whether it opts in to replaceability via BIP125 or v3
// (which is normally done in PreChecks). However, the only way a v3 transaction can
// have a non-v3 and non-BIP125 descendant is due to a reorg.
} else {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "v3-rule-violation", err->first);
}
}
// A transaction that spends outputs that would be replaced by it is invalid. Now
@ -999,18 +1024,21 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws)
// Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not
// TX_RECONSIDERABLE, because it cannot be bypassed using package validation.
// This must be changed if package RBF is enabled.
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string);
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY,
strprintf("insufficient fee%s", ws.m_sibling_eviction ? " (including sibling eviction)" : ""), *err_string);
}
// Calculate all conflicting entries and enforce Rule #5.
if (const auto err_string{GetEntriesForConflicts(tx, m_pool, ws.m_iters_conflicting, ws.m_all_conflicting)}) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY,
"too many potential replacements", *err_string);
strprintf("too many potential replacements%s", ws.m_sibling_eviction ? " (including sibling eviction)" : ""), *err_string);
}
// Enforce Rule #2.
if (const auto err_string{HasNoNewUnconfirmed(tx, m_pool, ws.m_iters_conflicting)}) {
// Sibling eviction is only done for v3 transactions, which cannot have multiple ancestors.
Assume(!ws.m_sibling_eviction);
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY,
"replacement-adds-unconfirmed", *err_string);
strprintf("replacement-adds-unconfirmed%s", ws.m_sibling_eviction ? " (including sibling eviction)" : ""), *err_string);
}
// Check if it's economically rational to mine this transaction rather than the ones it
// replaces and pays for its own relay fees. Enforce Rules #3 and #4.
@ -1023,7 +1051,8 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws)
// Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not
// TX_RECONSIDERABLE, because it cannot be bypassed using package validation.
// This must be changed if package RBF is enabled.
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string);
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY,
strprintf("insufficient fee%s", ws.m_sibling_eviction ? " (including sibling eviction)" : ""), *err_string);
}
return true;
}

183
test/functional/mempool_accept_v3.py
View File

@ -15,10 +15,13 @@ from test_framework.util import (
assert_raises_rpc_error,
)
from test_framework.wallet import (
COIN,
DEFAULT_FEE,
MiniWallet,
)
MAX_REPLACEMENT_CANDIDATES = 100
def cleanup(extra_args=None):
def decorator(func):
def wrapper(self):
@ -290,8 +293,13 @@ class MempoolAcceptV3(BitcoinTestFramework):
self.check_mempool([tx_in_mempool["txid"]])
@cleanup(extra_args=["-acceptnonstdtxn=1"])
def test_mempool_sibling(self):
self.log.info("Test that v3 transaction cannot have mempool siblings")
def test_sibling_eviction_package(self):
"""
When a transaction has a mempool sibling, it may be eligible for sibling eviction.
However, this option is only available in single transaction acceptance. It doesn't work in
a multi-testmempoolaccept (where RBF is disabled) or when doing package CPFP.
"""
self.log.info("Test v3 sibling eviction in submitpackage and multi-testmempoolaccept")
node = self.nodes[0]
# Add a parent + child to mempool
tx_mempool_parent = self.wallet.send_self_transfer_multi(
@ -307,26 +315,57 @@ class MempoolAcceptV3(BitcoinTestFramework):
)
self.check_mempool([tx_mempool_parent["txid"], tx_mempool_sibling["txid"]])
tx_has_mempool_sibling = self.wallet.create_self_transfer(
tx_sibling_1 = self.wallet.create_self_transfer(
utxo_to_spend=tx_mempool_parent["new_utxos"][1],
version=3
version=3,
fee_rate=DEFAULT_FEE*100,
)
expected_error_mempool_sibling = f"v3-rule-violation, tx {tx_mempool_parent['txid']} (wtxid={tx_mempool_parent['wtxid']}) would exceed descendant count limit"
assert_raises_rpc_error(-26, expected_error_mempool_sibling, node.sendrawtransaction, tx_has_mempool_sibling["hex"])
tx_has_mempool_uncle = self.wallet.create_self_transfer(utxo_to_spend=tx_sibling_1["new_utxo"], version=3)
tx_has_mempool_uncle = self.wallet.create_self_transfer(utxo_to_spend=tx_has_mempool_sibling["new_utxo"], version=3)
tx_sibling_2 = self.wallet.create_self_transfer(
utxo_to_spend=tx_mempool_parent["new_utxos"][0],
version=3,
fee_rate=DEFAULT_FEE*200,
)
# Also fails with another non-related transaction via testmempoolaccept
tx_sibling_3 = self.wallet.create_self_transfer(
utxo_to_spend=tx_mempool_parent["new_utxos"][1],
version=3,
fee_rate=0,
)
tx_bumps_parent_with_sibling = self.wallet.create_self_transfer(
utxo_to_spend=tx_sibling_3["new_utxo"],
version=3,
fee_rate=DEFAULT_FEE*300,
)
# Fails with another non-related transaction via testmempoolaccept
tx_unrelated = self.wallet.create_self_transfer(version=3)
result_test_unrelated = node.testmempoolaccept([tx_has_mempool_sibling["hex"], tx_unrelated["hex"]])
result_test_unrelated = node.testmempoolaccept([tx_sibling_1["hex"], tx_unrelated["hex"]])
assert_equal(result_test_unrelated[0]["reject-reason"], "v3-rule-violation")
result_test_1p1c = node.testmempoolaccept([tx_has_mempool_sibling["hex"], tx_has_mempool_uncle["hex"]])
# Fails in a package via testmempoolaccept
result_test_1p1c = node.testmempoolaccept([tx_sibling_1["hex"], tx_has_mempool_uncle["hex"]])
assert_equal(result_test_1p1c[0]["reject-reason"], "v3-rule-violation")
# Also fails with a child via submitpackage
result_submitpackage = node.submitpackage([tx_has_mempool_sibling["hex"], tx_has_mempool_uncle["hex"]])
assert_equal(result_submitpackage["tx-results"][tx_has_mempool_sibling['wtxid']]['error'], expected_error_mempool_sibling)
# Allowed when tx is submitted in a package and evaluated individually.
# Note that the child failed since it would be the 3rd generation.
result_package_indiv = node.submitpackage([tx_sibling_1["hex"], tx_has_mempool_uncle["hex"]])
self.check_mempool([tx_mempool_parent["txid"], tx_sibling_1["txid"]])
expected_error_gen3 = f"v3-rule-violation, tx {tx_has_mempool_uncle['txid']} (wtxid={tx_has_mempool_uncle['wtxid']}) would have too many ancestors"
assert_equal(result_package_indiv["tx-results"][tx_has_mempool_uncle['wtxid']]['error'], expected_error_gen3)
# Allowed when tx is submitted in a package with in-mempool parent (which is deduplicated).
node.submitpackage([tx_mempool_parent["hex"], tx_sibling_2["hex"]])
self.check_mempool([tx_mempool_parent["txid"], tx_sibling_2["txid"]])
# Child cannot pay for sibling eviction for parent, as it violates v3 topology limits
result_package_cpfp = node.submitpackage([tx_sibling_3["hex"], tx_bumps_parent_with_sibling["hex"]])
self.check_mempool([tx_mempool_parent["txid"], tx_sibling_2["txid"]])
expected_error_cpfp = f"v3-rule-violation, tx {tx_mempool_parent['txid']} (wtxid={tx_mempool_parent['wtxid']}) would exceed descendant count limit"
assert_equal(result_package_cpfp["tx-results"][tx_sibling_3['wtxid']]['error'], expected_error_cpfp)
@cleanup(extra_args=["-datacarriersize=1000", "-acceptnonstdtxn=1"])
@ -429,11 +468,123 @@ class MempoolAcceptV3(BitcoinTestFramework):
self.check_mempool([ancestor_tx["txid"], child_1_conflict["txid"], child_2["txid"]])
assert_equal(node.getmempoolentry(ancestor_tx["txid"])["descendantcount"], 3)
@cleanup(extra_args=["-acceptnonstdtxn=1"])
def test_v3_sibling_eviction(self):
self.log.info("Test sibling eviction for v3")
node = self.nodes[0]
tx_v3_parent = self.wallet.send_self_transfer_multi(from_node=node, num_outputs=2, version=3)
# This is the sibling to replace
tx_v3_child_1 = self.wallet.send_self_transfer(
from_node=node, utxo_to_spend=tx_v3_parent["new_utxos"][0], fee_rate=DEFAULT_FEE * 2, version=3
)
assert tx_v3_child_1["txid"] in node.getrawmempool()
self.log.info("Test tx must be higher feerate than sibling to evict it")
tx_v3_child_2_rule6 = self.wallet.create_self_transfer(
utxo_to_spend=tx_v3_parent["new_utxos"][1], fee_rate=DEFAULT_FEE, version=3
)
rule6_str = f"insufficient fee (including sibling eviction), rejecting replacement {tx_v3_child_2_rule6['txid']}; new feerate"
assert_raises_rpc_error(-26, rule6_str, node.sendrawtransaction, tx_v3_child_2_rule6["hex"])
self.check_mempool([tx_v3_parent['txid'], tx_v3_child_1['txid']])
self.log.info("Test tx must meet absolute fee rules to evict sibling")
tx_v3_child_2_rule4 = self.wallet.create_self_transfer(
utxo_to_spend=tx_v3_parent["new_utxos"][1], fee_rate=2 * DEFAULT_FEE + Decimal("0.00000001"), version=3
)
rule4_str = f"insufficient fee (including sibling eviction), rejecting replacement {tx_v3_child_2_rule4['txid']}, not enough additional fees to relay"
assert_raises_rpc_error(-26, rule4_str, node.sendrawtransaction, tx_v3_child_2_rule4["hex"])
self.check_mempool([tx_v3_parent['txid'], tx_v3_child_1['txid']])
self.log.info("Test tx cannot cause more than 100 evictions including RBF and sibling eviction")
# First add 4 groups of 25 transactions.
utxos_for_conflict = []
txids_v2_100 = []
for _ in range(4):
confirmed_utxo = self.wallet.get_utxo(confirmed_only=True)
utxos_for_conflict.append(confirmed_utxo)
# 25 is within descendant limits
chain_length = int(MAX_REPLACEMENT_CANDIDATES / 4)
chain = self.wallet.create_self_transfer_chain(chain_length=chain_length, utxo_to_spend=confirmed_utxo)
for item in chain:
txids_v2_100.append(item["txid"])
node.sendrawtransaction(item["hex"])
self.check_mempool(txids_v2_100 + [tx_v3_parent["txid"], tx_v3_child_1["txid"]])
# Replacing 100 transactions is fine
tx_v3_replacement_only = self.wallet.create_self_transfer_multi(utxos_to_spend=utxos_for_conflict, fee_per_output=4000000)
# Override maxfeerate - it costs a lot to replace these 100 transactions.
assert node.testmempoolaccept([tx_v3_replacement_only["hex"]], maxfeerate=0)[0]["allowed"]
# Adding another one exceeds the limit.
utxos_for_conflict.append(tx_v3_parent["new_utxos"][1])
tx_v3_child_2_rule5 = self.wallet.create_self_transfer_multi(utxos_to_spend=utxos_for_conflict, fee_per_output=4000000, version=3)
rule5_str = f"too many potential replacements (including sibling eviction), rejecting replacement {tx_v3_child_2_rule5['txid']}; too many potential replacements (101 > 100)"
assert_raises_rpc_error(-26, rule5_str, node.sendrawtransaction, tx_v3_child_2_rule5["hex"])
self.check_mempool(txids_v2_100 + [tx_v3_parent["txid"], tx_v3_child_1["txid"]])
self.log.info("Test sibling eviction is successful if it meets all RBF rules")
tx_v3_child_2 = self.wallet.create_self_transfer(
utxo_to_spend=tx_v3_parent["new_utxos"][1], fee_rate=DEFAULT_FEE*10, version=3
)
node.sendrawtransaction(tx_v3_child_2["hex"])
self.check_mempool(txids_v2_100 + [tx_v3_parent["txid"], tx_v3_child_2["txid"]])
self.log.info("Test that it's possible to do a sibling eviction and RBF at the same time")
utxo_unrelated_conflict = self.wallet.get_utxo(confirmed_only=True)
tx_unrelated_replacee = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=utxo_unrelated_conflict)
assert tx_unrelated_replacee["txid"] in node.getrawmempool()
fee_to_beat_child2 = int(tx_v3_child_2["fee"] * COIN)
tx_v3_child_3 = self.wallet.create_self_transfer_multi(
utxos_to_spend=[tx_v3_parent["new_utxos"][0], utxo_unrelated_conflict], fee_per_output=fee_to_beat_child2*5, version=3
)
node.sendrawtransaction(tx_v3_child_3["hex"])
self.check_mempool(txids_v2_100 + [tx_v3_parent["txid"], tx_v3_child_3["txid"]])
@cleanup(extra_args=["-acceptnonstdtxn=1"])
def test_reorg_sibling_eviction_1p2c(self):
node = self.nodes[0]
self.log.info("Test that sibling eviction is not allowed when multiple siblings exist")
tx_with_multi_children = self.wallet.send_self_transfer_multi(from_node=node, num_outputs=3, version=3, confirmed_only=True)
self.check_mempool([tx_with_multi_children["txid"]])
block_to_disconnect = self.generate(node, 1)[0]
self.check_mempool([])
tx_with_sibling1 = self.wallet.send_self_transfer(from_node=node, version=3, utxo_to_spend=tx_with_multi_children["new_utxos"][0])
tx_with_sibling2 = self.wallet.send_self_transfer(from_node=node, version=3, utxo_to_spend=tx_with_multi_children["new_utxos"][1])
self.check_mempool([tx_with_sibling1["txid"], tx_with_sibling2["txid"]])
# Create a reorg, bringing tx_with_multi_children back into the mempool with a descendant count of 3.
node.invalidateblock(block_to_disconnect)
self.check_mempool([tx_with_multi_children["txid"], tx_with_sibling1["txid"], tx_with_sibling2["txid"]])
assert_equal(node.getmempoolentry(tx_with_multi_children["txid"])["descendantcount"], 3)
# Sibling eviction is not allowed because there are two siblings
tx_with_sibling3 = self.wallet.create_self_transfer(
version=3,
utxo_to_spend=tx_with_multi_children["new_utxos"][2],
fee_rate=DEFAULT_FEE*50
)
expected_error_2siblings = f"v3-rule-violation, tx {tx_with_multi_children['txid']} (wtxid={tx_with_multi_children['wtxid']}) would exceed descendant count limit"
assert_raises_rpc_error(-26, expected_error_2siblings, node.sendrawtransaction, tx_with_sibling3["hex"])
# However, an RBF (with conflicting inputs) is possible even if the resulting cluster size exceeds 2
tx_with_sibling3_rbf = self.wallet.send_self_transfer(
from_node=node,
version=3,
utxo_to_spend=tx_with_multi_children["new_utxos"][0],
fee_rate=DEFAULT_FEE*50
)
self.check_mempool([tx_with_multi_children["txid"], tx_with_sibling3_rbf["txid"], tx_with_sibling2["txid"]])
def run_test(self):
self.log.info("Generate blocks to create UTXOs")
node = self.nodes[0]
self.wallet = MiniWallet(node)
self.generate(self.wallet, 110)
self.generate(self.wallet, 120)
self.test_v3_acceptance()
self.test_v3_replacement()
self.test_v3_bip125()
@ -441,10 +592,12 @@ class MempoolAcceptV3(BitcoinTestFramework):
self.test_nondefault_package_limits()
self.test_v3_ancestors_package()
self.test_v3_ancestors_package_and_mempool()
self.test_mempool_sibling()
self.test_sibling_eviction_package()
self.test_v3_package_inheritance()
self.test_v3_in_testmempoolaccept()
self.test_reorg_2child_rbf()
self.test_v3_sibling_eviction()
self.test_reorg_sibling_eviction_1p2c()
if __name__ == "__main__":