Page MenuHomePhabricator
Files F12944998

D16573.id49008.diff
No OneTemporary
Actions

D16573.id49008.diff
View Options

diff --git a/src/consensus/validation.h b/src/consensus/validation.h
--- a/src/consensus/validation.h
+++ b/src/consensus/validation.h
@@ -44,7 +44,7 @@
TX_NO_MEMPOOL,
//! fails some policy, but might be acceptable if submitted in a (different)
//! package
- TX_RECONSIDERABLE,
+ TX_PACKAGE_RECONSIDERABLE,
//! transaction was not validated because package failed
TX_UNKNOWN,
};
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -904,8 +904,9 @@
* before, e.g. is an orphan, to avoid
* adding duplicate entries.
*
- * Updates m_txrequest, m_recent_rejects, m_recent_rejects_reconsiderable,
- * m_orphanage and vExtraTxnForCompact.
+ * Updates m_txrequest, m_recent_rejects,
+ * m_recent_rejects_package_reconsiderable, m_orphanage and
+ * vExtraTxnForCompact.
*/
void ProcessInvalidTx(NodeId nodeid, const CTransactionRef &tx,
const TxValidationState &result,
@@ -1240,10 +1241,11 @@
* - mempool
* - orphanage
* - m_recent_rejects
- * - m_recent_rejects_reconsiderable (if include_reconsiderable = true)
+ * - m_recent_rejects_package_reconsiderable (if
+ * include_reconsiderable = true)
* - m_recent_confirmed_transactions
* Also responsible for resetting m_recent_rejects and
- * m_recent_rejects_reconsiderable if the chain tip has changed.
+ * m_recent_rejects_package_reconsiderable if the chain tip has changed.
* */
bool AlreadyHaveTx(const TxId &txid, bool include_reconsiderable)
EXCLUSIVE_LOCKS_REQUIRED(cs_main,
@@ -1273,7 +1275,7 @@
/**
* Block hash of chain tip the last time we reset m_recent_rejects and
- * m_recent_rejects_reconsiderable.
+ * m_recent_rejects_package_reconsiderable.
* FIXME: should be of BlockHash type
*/
uint256 hashRecentRejectsChainTip GUARDED_BY(cs_main);
@@ -1289,9 +1291,10 @@
* e.g. all of our peers have larger mempools and thus lower minimum
* feerates than us.
*
- * When a transaction's error is TxValidationResult::TX_RECONSIDERABLE (in a
- * package or by itself), add its txid to this filter. When a package fails
- * for any reason, add the combined hash to this filter.
+ * When a transaction's error is
+ * TxValidationResult::TX_PACKAGE_RECONSIDERABLE (in a package or by
+ * itself), add its txid to this filter. When a package fails for any
+ * reason, add the combined hash to this filter.
*
* Upon receiving an announcement for a transaction, if it exists in this
* filter, do not download the txdata. When considering packages, if it
@@ -1302,8 +1305,8 @@
* Parameters are picked to be the same as m_recent_rejects, with the same
* rationale.
*/
- CRollingBloomFilter m_recent_rejects_reconsiderable GUARDED_BY(::cs_main){
- 120'000, 0.000'001};
+ CRollingBloomFilter m_recent_rejects_package_reconsiderable
+ GUARDED_BY(::cs_main){120'000, 0.000'001};
/**
* Filter for transactions that have been recently confirmed.
@@ -2631,7 +2634,7 @@
case TxValidationResult::TX_CHILD_BEFORE_PARENT:
case TxValidationResult::TX_MEMPOOL_POLICY:
case TxValidationResult::TX_NO_MEMPOOL:
- case TxValidationResult::TX_RECONSIDERABLE:
+ case TxValidationResult::TX_PACKAGE_RECONSIDERABLE:
case TxValidationResult::TX_UNKNOWN:
break;
}
@@ -2957,7 +2960,7 @@
hashRecentRejectsChainTip =
m_chainman.ActiveChain().Tip()->GetBlockHash();
m_recent_rejects.reset();
- m_recent_rejects_reconsiderable.reset();
+ m_recent_rejects_package_reconsiderable.reset();
}
if (m_orphanage.HaveTx(txid)) {
@@ -2965,7 +2968,7 @@
}
if (include_reconsiderable &&
- m_recent_rejects_reconsiderable.contains(txid)) {
+ m_recent_rejects_package_reconsiderable.contains(txid)) {
return true;
}
@@ -4099,12 +4102,12 @@
return;
}
- if (state.GetResult() == TxValidationResult::TX_RECONSIDERABLE) {
- // If the result is TX_RECONSIDERABLE, add it to
- // m_recent_rejects_reconsiderable because we should not download or
- // submit this transaction by itself again, but may submit it as part
- // of a package later.
- m_recent_rejects_reconsiderable.insert(ptx->GetId());
+ if (state.GetResult() == TxValidationResult::TX_PACKAGE_RECONSIDERABLE) {
+ // If the result is TX_PACKAGE_RECONSIDERABLE, add it to
+ // m_recent_rejects_package_reconsiderable because we should not
+ // download or submit this transaction by itself again, but may submit
+ // it as part of a package later.
+ m_recent_rejects_package_reconsiderable.insert(ptx->GetId());
} else {
m_recent_rejects.insert(ptx->GetId());
}
@@ -4159,7 +4162,7 @@
const auto &senders = package_to_validate.m_senders;
if (package_result.m_state.IsInvalid()) {
- m_recent_rejects_reconsiderable.insert(GetPackageHash(package));
+ m_recent_rejects_package_reconsiderable.insert(GetPackageHash(package));
}
// We currently only expect to process 1-parent-1-child packages. Remove if
// this changes.
@@ -4187,7 +4190,7 @@
case MempoolAcceptResult::ResultType::INVALID: {
// Don't add to vExtraTxnForCompact, as these transactions
// should have already been added there when added to the
- // orphanage or rejected for TX_RECONSIDERABLE.
+ // orphanage or rejected for TX_PACKAGE_RECONSIDERABLE.
// This should be updated if package submission is ever used
// for transactions that haven't already been validated
// before.
@@ -4216,7 +4219,7 @@
const auto &parent_txid{ptx->GetId()};
- Assume(m_recent_rejects_reconsiderable.contains(parent_txid));
+ Assume(m_recent_rejects_package_reconsiderable.contains(parent_txid));
// Prefer children from this peer. This helps prevent censorship attempts in
// which an attacker sends lots of fake children for the parent, and we
@@ -4228,7 +4231,7 @@
// These children should be sorted from newest to oldest.
for (const auto &child : cpfp_candidates_same_peer) {
Package maybe_cpfp_package{ptx, child};
- if (!m_recent_rejects_reconsiderable.contains(
+ if (!m_recent_rejects_package_reconsiderable.contains(
GetPackageHash(maybe_cpfp_package))) {
return PeerManagerImpl::PackageToValidate{ptx, child, nodeid,
nodeid};
@@ -4258,11 +4261,11 @@
for (const auto index : tx_indices) {
// If we already tried a package and failed for any reason, the combined
- // hash was cached in m_recent_rejects_reconsiderable.
+ // hash was cached in m_recent_rejects_package_reconsiderable.
const auto [child_tx, child_sender] =
cpfp_candidates_different_peer.at(index);
Package maybe_cpfp_package{ptx, child_tx};
- if (!m_recent_rejects_reconsiderable.contains(
+ if (!m_recent_rejects_package_reconsiderable.contains(
GetPackageHash(maybe_cpfp_package))) {
return PeerManagerImpl::PackageToValidate{ptx, child_tx, nodeid,
child_sender};
@@ -5593,10 +5596,10 @@
}
}
- if (m_recent_rejects_reconsiderable.contains(txid)) {
+ if (m_recent_rejects_package_reconsiderable.contains(txid)) {
// When a transaction is already in
- // m_recent_rejects_reconsiderable, we shouldn't submit it by
- // itself again. However, look for a matching child in the
+ // m_recent_rejects_package_reconsiderable, we shouldn't submit
+ // it by itself again. However, look for a matching child in the
// orphanage, as it is possible that they succeed as a package.
LogPrint(BCLog::TXPACKAGES,
"found tx %s in reconsiderable rejects, looking for "
@@ -5662,9 +5665,9 @@
unique_parents.end());
// Distinguish between parents in m_recent_rejects and
- // m_recent_rejects_reconsiderable. We can tolerate having up to 1
- // parent in m_recent_rejects_reconsiderable since we submit 1p1c
- // packages. However, fail immediately if any are in
+ // m_recent_rejects_package_reconsiderable. We can tolerate having
+ // up to 1 parent in m_recent_rejects_package_reconsiderable since
+ // we submit 1p1c packages. However, fail immediately if any are in
// m_recent_rejects.
std::optional<TxId> rejected_parent_reconsiderable;
for (const TxId &parent_txid : unique_parents) {
@@ -5673,9 +5676,11 @@
break;
}
- if (m_recent_rejects_reconsiderable.contains(parent_txid) &&
+ if (m_recent_rejects_package_reconsiderable.contains(
+ parent_txid) &&
!m_mempool.exists(parent_txid)) {
- // More than 1 parent in m_recent_rejects_reconsiderable:
+ // More than 1 parent in
+ // m_recent_rejects_package_reconsiderable:
// 1p1c will not be sufficient to accept this package, so
// just give up here.
if (rejected_parent_reconsiderable.has_value()) {
@@ -5691,9 +5696,9 @@
for (const TxId &parent_txid : unique_parents) {
// FIXME: MSG_TX should use a TxHash, not a TxId.
AddKnownTx(*peer, parent_txid);
- // Exclude m_recent_rejects_reconsiderable: the missing
- // parent may have been previously rejected for being too
- // low feerate. This orphan might CPFP it.
+ // Exclude m_recent_rejects_package_reconsiderable: the
+ // missing parent may have been previously rejected for
+ // being too low feerate. This orphan might CPFP it.
if (!AlreadyHaveTx(parent_txid,
/*include_reconsiderable=*/false)) {
AddTxAnnouncement(pfrom, parent_txid, current_time);
@@ -5728,10 +5733,11 @@
ProcessInvalidTx(pfrom.GetId(), ptx, state,
/*maybe_add_extra_compact_tx=*/true);
}
- // When a transaction fails for TX_RECONSIDERABLE, look for a matching
- // child in the orphanage, as it is possible that they succeed as a
- // package.
- if (state.GetResult() == TxValidationResult::TX_RECONSIDERABLE) {
+ // When a transaction fails for TX_PACKAGE_RECONSIDERABLE, look for a
+ // matching child in the orphanage, as it is possible that they succeed
+ // as a package.
+ if (state.GetResult() ==
+ TxValidationResult::TX_PACKAGE_RECONSIDERABLE) {
LogPrint(BCLog::TXPACKAGES,
"tx %s failed but reconsiderable, looking for child in "
"orphanage\n",
@@ -8623,9 +8629,9 @@
entry.second.ToString(), entry.first);
}
for (const TxId &txid : requestable) {
- // Exclude m_recent_rejects_reconsiderable: we may be requesting a
- // missing parent that was previously rejected for being too low
- // feerate.
+ // Exclude m_recent_rejects_package_reconsiderable: we may be
+ // requesting a missing parent that was previously rejected for
+ // being too low feerate.
if (!AlreadyHaveTx(txid, /*include_reconsiderable=*/false)) {
addGetDataAndMaybeFlush(MSG_TX, txid);
m_txrequest.RequestedData(
diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp
--- a/src/test/txpackage_tests.cpp
+++ b/src/test/txpackage_tests.cpp
@@ -860,7 +860,7 @@
BOOST_CHECK_EQUAL(
submit_package_too_low.m_tx_results.at(tx_parent_cheap->GetId())
.m_state.GetResult(),
- TxValidationResult::TX_RECONSIDERABLE);
+ TxValidationResult::TX_PACKAGE_RECONSIDERABLE);
BOOST_CHECK(
submit_package_too_low.m_tx_results.at(tx_parent_cheap->GetId())
.m_effective_feerate.value() ==
@@ -870,7 +870,7 @@
BOOST_CHECK_EQUAL(
submit_package_too_low.m_tx_results.at(tx_child_cheap->GetId())
.m_state.GetResult(),
- TxValidationResult::TX_RECONSIDERABLE);
+ TxValidationResult::TX_PACKAGE_RECONSIDERABLE);
BOOST_CHECK(
submit_package_too_low.m_tx_results.at(tx_child_cheap->GetId())
.m_effective_feerate.value() ==
diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp
--- a/src/test/util/txmempool.cpp
+++ b/src/test/util/txmempool.cpp
@@ -59,12 +59,12 @@
}
// m_effective_feerate and m_txids_fee_calculations should exist iff the
- // result was valid or if the failure was TX_RECONSIDERABLE
+ // result was valid or if the failure was TX_PACKAGE_RECONSIDERABLE
const bool valid_or_reconsiderable{
atmp_result.m_result_type ==
MempoolAcceptResult::ResultType::VALID ||
atmp_result.m_state.GetResult() ==
- TxValidationResult::TX_RECONSIDERABLE};
+ TxValidationResult::TX_PACKAGE_RECONSIDERABLE};
if (atmp_result.m_effective_feerate.has_value() !=
valid_or_reconsiderable) {
return strprintf("tx %s result should %shave m_effective_feerate",
diff --git a/src/validation.h b/src/validation.h
--- a/src/validation.h
+++ b/src/validation.h
@@ -185,19 +185,19 @@
* package).
* Here are the expected fields and properties of a result depending on its
* ResultType, applicable to results returned from package evaluation:
- *+--------------------------+-----------+---------------------------------+----------------+
- *| Field or property | VALID | INVALID | MEMPOOL_ENTRY |
- *| | |---------------------------------| |
- *| | | TX_RECONSIDERABLE | Other | |
- *+--------------------------+-----------+-------------------+-------------+----------------+
- *| txid in mempool? | yes | no | no* | yes |
- *| m_state | IsValid() | IsInvalid() | IsInvalid() | IsValid() |
- *| m_replaced_transactions | yes | no | no | no |
- *| m_vsize | yes | no | no | yes |
- *| m_base_fees | yes | no | no | yes |
- *| m_effective_feerate | yes | yes | no | no |
- *| m_txids_fee_calculations | yes | yes | no | no |
- *+--------------------------+-----------+-------------------+-------------+----------------+
+ *+--------------------------+-----------+-------------------------------------------+---------------+
+ *| Field or property | VALID | INVALID | MEMPOOL_ENTRY |
+ *| | |-------------------------------------------| |
+ *| | | TX_PACKAGE_RECONSIDERABLE | Other | |
+ *+--------------------------+-----------+---------------------------+---------------+---------------+
+ *| txid in mempool? | yes | no | no* | yes |
+ *| m_state | IsValid() | IsInvalid() | IsInvalid() | IsValid() |
+ *| m_replaced_transactions | yes | no | no | no |
+ *| m_vsize | yes | no | no | yes |
+ *| m_base_fees | yes | no | no | yes |
+ *| m_effective_feerate | yes | yes | no | no |
+ *| m_txids_fee_calculations | yes | yes | no | no |
+ *+--------------------------+-----------+---------------------------+---------------+---------------+
* (*) Individual transaction acceptance doesn't return MEMPOOL_ENTRY. It
* returns INVALID, with the error txn-already-in-mempool. In this case, the
* txid may be in the mempool for a TX_CONFLICT.
diff --git a/src/validation.cpp b/src/validation.cpp
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -547,7 +547,7 @@
if (mempoolRejectFee > Amount::zero() &&
package_fee < mempoolRejectFee) {
return state.Invalid(
- TxValidationResult::TX_RECONSIDERABLE,
+ TxValidationResult::TX_PACKAGE_RECONSIDERABLE,
"mempool min fee not met",
strprintf("%d < %d", package_fee, mempoolRejectFee));
}
@@ -556,7 +556,8 @@
// policy upgrade.
if (package_fee < m_pool.m_min_relay_feerate.GetFee(package_size)) {
return state.Invalid(
- TxValidationResult::TX_RECONSIDERABLE, "min relay fee not met",
+ TxValidationResult::TX_PACKAGE_RECONSIDERABLE,
+ "min relay fee not met",
strprintf("%d < %d", package_fee,
m_pool.m_min_relay_feerate.GetFee(package_size)));
}
@@ -745,8 +746,8 @@
ws.m_modified_fees <
m_pool.m_min_relay_feerate.GetFee(ws.m_ptx->GetTotalSize())) {
// 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.
+ // TX_MEMPOOL_POLICY, not TX_PACKAGE_RECONSIDERABLE, because it cannot
+ // be bypassed using package validation.
return state.Invalid(
TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met",
strprintf("%d < %d", ws.m_modified_fees,
@@ -829,7 +830,7 @@
if (!m_pool.exists(txid)) {
// The tx no longer meets our (new) mempool minimum feerate but
// could be reconsidered in a package.
- return state.Invalid(TxValidationResult::TX_RECONSIDERABLE,
+ return state.Invalid(TxValidationResult::TX_PACKAGE_RECONSIDERABLE,
"mempool full");
}
}
@@ -952,7 +953,8 @@
// verification unless those checks pass, to mitigate CPU exhaustion
// denial-of-service attacks.
if (!PreChecks(args, ws)) {
- if (ws.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE) {
+ if (ws.m_state.GetResult() ==
+ TxValidationResult::TX_PACKAGE_RECONSIDERABLE) {
// Failed for fee reasons. Provide the effective feerate and which
// tx was included.
return MempoolAcceptResult::FeeFailure(
@@ -999,7 +1001,8 @@
// The only possible failure reason is fee-related (mempool full).
// Failed for fee reasons. Provide the effective feerate and which txns
// were included.
- Assume(ws.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE);
+ Assume(ws.m_state.GetResult() ==
+ TxValidationResult::TX_PACKAGE_RECONSIDERABLE);
return MempoolAcceptResult::FeeFailure(
ws.m_state, CFeeRate(ws.m_modified_fees, ws.m_vsize), single_txid);
}
@@ -1314,7 +1317,7 @@
assert(m_pool.exists(txid));
results_final.emplace(txid, single_res);
} else if (single_res.m_state.GetResult() !=
- TxValidationResult::TX_RECONSIDERABLE &&
+ TxValidationResult::TX_PACKAGE_RECONSIDERABLE &&
single_res.m_state.GetResult() !=
TxValidationResult::TX_MISSING_INPUTS) {
// Package validation policy only differs from individual policy

File Metadata

Mime Type
text/plain
Expires
Thu, Feb 6, 16:58 (20 h, 46 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5082734
Default Alt Text
D16573.id49008.diff (20 KB)

Event Timeline