Skip to content

Commit

Permalink
[validation] add TxValidationResult::TX_RECONSIDERABLE, TX_UNKNOWN
Browse files Browse the repository at this point in the history
With package validation rules, transactions that fail individually may
sometimes be eligible for reconsideration if submitted as part of a
(different) package. For now, that includes trasactions that failed for
being too low feerate.  Add a new TxValidationResult type to distinguish
these failures from others.  In the next commits, we will abort package
validation if a tx fails for any other reason. In the future, we will
also decide whether to cache failures in recent_rejects based on this
result (we won't want to reject a package containing a transaction that
was rejected previously for being low feerate).

Package validation also sometimes elects to skip some transactions when
it knows the package will not be submitted in order to quit sooner. Add
a result to specify this situation; we also don't want to cache these
as rejections.
  • Loading branch information
glozow committed Nov 6, 2023
1 parent 5c786a0 commit 3979f1a
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 4 deletions.
2 changes: 2 additions & 0 deletions src/consensus/validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ enum class TxValidationResult {
TX_CONFLICT,
TX_MEMPOOL_POLICY, //!< violated mempool's fee/size/descendant/RBF/etc limits
TX_NO_MEMPOOL, //!< this node does not have a mempool so can't validate the transaction
TX_RECONSIDERABLE, //!< fails some policy, but might be acceptable if submitted in a (different) package
TX_UNKNOWN, //!< transaction was not validated because package failed
};

/** A "reason" why a block was invalid, suitable for determining whether the
Expand Down
2 changes: 2 additions & 0 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1820,6 +1820,8 @@ bool PeerManagerImpl::MaybePunishNodeForTx(NodeId nodeid, const TxValidationStat
case TxValidationResult::TX_CONFLICT:
case TxValidationResult::TX_MEMPOOL_POLICY:
case TxValidationResult::TX_NO_MEMPOOL:
case TxValidationResult::TX_RECONSIDERABLE:
case TxValidationResult::TX_UNKNOWN:
break;
}
return false;
Expand Down
17 changes: 13 additions & 4 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -670,11 +670,11 @@ class MemPoolAccept
AssertLockHeld(m_pool.cs);
CAmount mempoolRejectFee = m_pool.GetMinFee().GetFee(package_size);
if (mempoolRejectFee > 0 && package_fee < mempoolRejectFee) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee));
return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee));
}

if (package_fee < m_pool.m_min_relay_feerate.GetFee(package_size)) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met",
return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "min relay fee not met",
strprintf("%d < %d", package_fee, m_pool.m_min_relay_feerate.GetFee(package_size)));
}
return true;
Expand Down Expand Up @@ -867,6 +867,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// method of ensuring the tx remains bumped. For example, the fee-bumping child could disappear
// due to a replacement.
if (!bypass_limits && ws.m_modified_fees < m_pool.m_min_relay_feerate.GetFee(ws.m_vsize)) {
// 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.
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met",
strprintf("%d < %d", ws.m_modified_fees, m_pool.m_min_relay_feerate.GetFee(ws.m_vsize)));
}
Expand Down Expand Up @@ -981,6 +983,9 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws)
// 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)}) {
// 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);
}

Expand All @@ -1002,6 +1007,9 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws)
}
if (const auto err_string{PaysForRBF(ws.m_conflicting_fees, ws.m_modified_fees, ws.m_vsize,
m_pool.m_incremental_relay_feerate, hash)}) {
// 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 true;
Expand Down Expand Up @@ -1139,7 +1147,8 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws)
if (!args.m_package_submission && !bypass_limits) {
LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip());
if (!m_pool.exists(GenTxid::Txid(hash)))
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool full");
// The tx no longer meets our (new) mempool minimum feerate but could be reconsidered in a package.
return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "mempool full");
}
return true;
}
Expand Down Expand Up @@ -1510,7 +1519,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
// in package validation, because its fees should only be "used" once.
assert(m_pool.exists(GenTxid::Wtxid(wtxid)));
results_final.emplace(wtxid, single_res);
} else if (single_res.m_state.GetResult() != TxValidationResult::TX_MEMPOOL_POLICY &&
} else if (single_res.m_state.GetResult() != TxValidationResult::TX_RECONSIDERABLE &&
single_res.m_state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) {
// Package validation policy only differs from individual policy in its evaluation
// of feerate. For example, if a transaction fails here due to violation of a
Expand Down

0 comments on commit 3979f1a

Please sign in to comment.