Implement new RBF logic for cluster mempool

With a total ordering on mempool transactions, we are now able to calculate a
transaction's mining score at all times. Use this to improve the RBF logic:

- we no longer enforce a "no new unconfirmed parents" rule

- we now require that the mempool's feerate diagram must improve in order
  to accept a replacement

- the topology restrictions for conflicts in the package rbf setting have been
  eliminated

Revert the temporary change to mempool_ephemeral_dust.py that were previously
made due to RBF validation checks being reordered.

Co-authored-by: Gregory Sanders <gsanders87@gmail.com>, glozow <gloriajzhao@gmail.com>
This commit is contained in:
Suhas Daftuar
2023-09-21 13:19:32 -04:00
parent ff8f115dec
commit 216e693729
11 changed files with 182 additions and 758 deletions

View File

@@ -1070,21 +1070,6 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws)
TxValidationState& state = ws.m_state;
CFeeRate newFeeRate(ws.m_modified_fees, ws.m_vsize);
// Enforce Rule #6. The replacement transaction must have a higher feerate than its direct conflicts.
// - The motivation for this check is to ensure that the replacement transaction is preferable for
// block-inclusion, compared to what would be removed from the mempool.
// - This logic predates ancestor feerate-based transaction selection, which is why it doesn't
// consider feerates of descendants.
// - Note: Ancestor feerate-based transaction selection has made this comparison insufficient to
// guarantee that this is incentive-compatible for miners, because it is possible for a
// descendant transaction of a direct conflict to pay a higher feerate than the transaction that
// might replace them, under these rules.
if (const auto err_string{PaysMoreThanConflicts(ws.m_iters_conflicting, newFeeRate, hash)}) {
// This fee-related failure is TX_RECONSIDERABLE because validating in a package may change
// the result.
return state.Invalid(TxValidationResult::TX_RECONSIDERABLE,
strprintf("insufficient fee%s", ws.m_sibling_eviction ? " (including sibling eviction)" : ""), *err_string);
}
CTxMemPool::setEntries all_conflicts;
@@ -1094,20 +1079,13 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws)
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, all_conflicts)}) {
// Sibling eviction is only done for TRUC transactions, which cannot have multiple ancestors.
Assume(!ws.m_sibling_eviction);
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY,
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.
for (CTxMemPool::txiter it : all_conflicts) {
m_subpackage.m_conflicting_fees += it->GetModifiedFee();
m_subpackage.m_conflicting_size += it->GetTxSize();
}
if (const auto err_string{PaysForRBF(m_subpackage.m_conflicting_fees, ws.m_modified_fees, ws.m_vsize,
m_pool.m_opts.incremental_relay_feerate, hash)}) {
// Result may change in a package context
@@ -1119,6 +1097,19 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws)
for (auto it : all_conflicts) {
m_subpackage.m_changeset->StageRemoval(it);
}
// Run cluster size limit checks and fail if we exceed them.
if (!m_subpackage.m_changeset->CheckMemPoolPolicyLimits()) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-large-cluster", "");
}
if (const auto err_string{ImprovesFeerateDiagram(*m_subpackage.m_changeset)}) {
// We checked above for the cluster size limits being respected, so a
// failure here can only be due to an insufficient fee.
Assume(err_string->first == DiagramCheckError::FAILURE);
return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "replacement-failed", err_string->second);
}
return true;
}
@@ -1205,9 +1196,14 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
strprintf("package feerate %s <= parent feerate is %s", package_feerate.ToString(), parent_feerate.ToString()));
}
// Run cluster size limit checks and fail if we exceed them.
if (!m_subpackage.m_changeset->CheckMemPoolPolicyLimits()) {
return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "too-large-cluster", "");
}
// Check if it's economically rational to mine this package rather than the ones it replaces.
// This takes the place of ReplacementChecks()'s PaysMoreThanConflicts() in the package RBF setting.
if (const auto err_tup{ImprovesFeerateDiagram(*m_subpackage.m_changeset)}) {
Assume(err_tup->first == DiagramCheckError::FAILURE);
return package_state.Invalid(PackageValidationResult::PCKG_POLICY,
"package RBF failed: " + err_tup.value().second, "");
}