Merge bitcoin/bitcoin#31096: Package validation: accept packages of size 1

32fc59796f rpc: Allow single transaction through submitpackage (glozow)

Pull request description:

  There's no particular reason to restrict single transaction submissions with submitpackage. This change relaxes the RPC checks as enables the `AcceptPackage` flow to accept packages of a single transaction.

  Resolves #31085

ACKs for top commit:
  naumenkogs:
    ACK 32fc59796f
  achow101:
    ACK 32fc59796f
  glozow:
    ACK 32fc59796f

Tree-SHA512: ffed353bfdca610ffcfd53b40b76da05ffc26df6bac4b0421492e067bede930380e03399d2e2d1d17f0e88fb91cd8eb376e3aabebbabcc724590bf068d09807c
This commit is contained in:
Ava Chow
2024-12-03 17:46:23 -05:00
5 changed files with 155 additions and 54 deletions

View File

@@ -1685,10 +1685,14 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptSubPackage(const std::vector<CTr
PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, ATMPArgs& args)
{
Assert(!package.empty());
AssertLockHeld(cs_main);
// Used if returning a PackageMempoolAcceptResult directly from this function.
PackageValidationState package_state_quit_early;
// There are two topologies we are able to handle through this function:
// (1) A single transaction
// (2) A child-with-unconfirmed-parents package.
// Check that the package is well-formed. If it isn't, we won't try to validate any of the
// transactions and thus won't return any MempoolAcceptResults, just a package-wide error.
@@ -1697,48 +1701,50 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
return PackageMempoolAcceptResult(package_state_quit_early, {});
}
// All transactions in the package must be a parent of the last transaction. This is just an
// opportunity for us to fail fast on a context-free check without taking the mempool lock.
if (!IsChildWithParents(package)) {
package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-parents");
return PackageMempoolAcceptResult(package_state_quit_early, {});
}
// IsChildWithParents() guarantees the package is > 1 transactions.
assert(package.size() > 1);
// The package must be 1 child with all of its unconfirmed parents. The package is expected to
// be sorted, so the last transaction is the child.
const auto& child = package.back();
std::unordered_set<uint256, SaltedTxidHasher> unconfirmed_parent_txids;
std::transform(package.cbegin(), package.cend() - 1,
std::inserter(unconfirmed_parent_txids, unconfirmed_parent_txids.end()),
[](const auto& tx) { return tx->GetHash(); });
// All child inputs must refer to a preceding package transaction or a confirmed UTXO. The only
// way to verify this is to look up the child's inputs in our current coins view (not including
// mempool), and enforce that all parents not present in the package be available at chain tip.
// Since this check can bring new coins into the coins cache, keep track of these coins and
// uncache them if we don't end up submitting this package to the mempool.
const CCoinsViewCache& coins_tip_cache = m_active_chainstate.CoinsTip();
for (const auto& input : child->vin) {
if (!coins_tip_cache.HaveCoinInCache(input.prevout)) {
args.m_coins_to_uncache.push_back(input.prevout);
if (package.size() > 1) {
// All transactions in the package must be a parent of the last transaction. This is just an
// opportunity for us to fail fast on a context-free check without taking the mempool lock.
if (!IsChildWithParents(package)) {
package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-parents");
return PackageMempoolAcceptResult(package_state_quit_early, {});
}
// IsChildWithParents() guarantees the package is > 1 transactions.
assert(package.size() > 1);
// The package must be 1 child with all of its unconfirmed parents. The package is expected to
// be sorted, so the last transaction is the child.
const auto& child = package.back();
std::unordered_set<uint256, SaltedTxidHasher> unconfirmed_parent_txids;
std::transform(package.cbegin(), package.cend() - 1,
std::inserter(unconfirmed_parent_txids, unconfirmed_parent_txids.end()),
[](const auto& tx) { return tx->GetHash(); });
// All child inputs must refer to a preceding package transaction or a confirmed UTXO. The only
// way to verify this is to look up the child's inputs in our current coins view (not including
// mempool), and enforce that all parents not present in the package be available at chain tip.
// Since this check can bring new coins into the coins cache, keep track of these coins and
// uncache them if we don't end up submitting this package to the mempool.
const CCoinsViewCache& coins_tip_cache = m_active_chainstate.CoinsTip();
for (const auto& input : child->vin) {
if (!coins_tip_cache.HaveCoinInCache(input.prevout)) {
args.m_coins_to_uncache.push_back(input.prevout);
}
}
// Using the MemPoolAccept m_view cache allows us to look up these same coins faster later.
// This should be connecting directly to CoinsTip, not to m_viewmempool, because we specifically
// require inputs to be confirmed if they aren't in the package.
m_view.SetBackend(m_active_chainstate.CoinsTip());
const auto package_or_confirmed = [this, &unconfirmed_parent_txids](const auto& input) {
return unconfirmed_parent_txids.count(input.prevout.hash) > 0 || m_view.HaveCoin(input.prevout);
};
if (!std::all_of(child->vin.cbegin(), child->vin.cend(), package_or_confirmed)) {
package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-unconfirmed-parents");
return PackageMempoolAcceptResult(package_state_quit_early, {});
}
// Protect against bugs where we pull more inputs from disk that miss being added to
// coins_to_uncache. The backend will be connected again when needed in PreChecks.
m_view.SetBackend(m_dummy);
}
// Using the MemPoolAccept m_view cache allows us to look up these same coins faster later.
// This should be connecting directly to CoinsTip, not to m_viewmempool, because we specifically
// require inputs to be confirmed if they aren't in the package.
m_view.SetBackend(m_active_chainstate.CoinsTip());
const auto package_or_confirmed = [this, &unconfirmed_parent_txids](const auto& input) {
return unconfirmed_parent_txids.count(input.prevout.hash) > 0 || m_view.HaveCoin(input.prevout);
};
if (!std::all_of(child->vin.cbegin(), child->vin.cend(), package_or_confirmed)) {
package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-unconfirmed-parents");
return PackageMempoolAcceptResult(package_state_quit_early, {});
}
// Protect against bugs where we pull more inputs from disk that miss being added to
// coins_to_uncache. The backend will be connected again when needed in PreChecks.
m_view.SetBackend(m_dummy);
LOCK(m_pool.cs);
// Stores results from which we will create the returned PackageMempoolAcceptResult.
@@ -1748,6 +1754,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
// this transaction. "Nonfinal" because if a transaction fails by itself but succeeds later
// (i.e. when evaluated with a fee-bumping child), the result in this map may be discarded.
std::map<uint256, MempoolAcceptResult> individual_results_nonfinal;
// Tracks whether we think package submission could result in successful entry to the mempool
bool quit_early{false};
std::vector<CTransactionRef> txns_package_eval;
for (const auto& tx : package) {
@@ -1789,8 +1796,9 @@ 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_RECONSIDERABLE &&
single_res.m_state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) {
} else if (package.size() == 1 || // If there is only one transaction, no need to retry it "as a package"
(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
// consensus rule, the result will not change when it is submitted as part of a