highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-01-21 15:50:07 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#22084: package testmempoolaccept followups

Browse Source 
ee862d6efb MOVEONLY: context-free package policies (glozow)
5cac95cd15 disallow_mempool_conflicts -> allow_bip125_replacement and check earlier (glozow)
e8ecc621be [refactor] comment/naming improvements (glozow)
7d91442461 [rpc] reserve space in txns (glozow)
6c5f19d9c4 [package] static_assert max package size >= max tx size (glozow)

Pull request description:

  various followups from #20833

ACKs for top commit:
  jnewbery:
    utACK ee862d6efb
  ariard:
    Code Review ACK ee862d6

Tree-SHA512: 96ecb41f7bbced84d4253070f5274b7267607bfe4033e2bb0d2f55ec778cc41e811130b6321131e0418b5835894e510a4be8a0f822bc9d68d9224418359ac837
This commit is contained in:
fanquake
2021-06-10 18:52:52 +08:00
parent e87fbee402 ee862d6efb
commit ef8f2966ac
10 changed files with 120 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

87
src/validation.cpp
View File

@@ -472,8 +472,10 @@ public:
*/
std::vector<COutPoint>& m_coins_to_uncache;
const bool m_test_accept;
/** Disable BIP125 RBFing; disallow all conflicts with mempool transactions. */
const bool disallow_mempool_conflicts;
/** Whether we allow transactions to replace mempool transactions by BIP125 rules. If false,
* any transaction spending the same inputs as a transaction in the mempool is considered
* a conflict. */
const bool m_allow_bip125_replacement{true};
};
// Single transaction acceptance
@@ -482,7 +484,7 @@ public:
/**
* Multiple transaction acceptance. Transactions may or may not be interdependent,
* but must not conflict with each other. Parents must come before children if any
* dependencies exist, otherwise a TX_MISSING_INPUTS error will be returned.
* dependencies exist.
*/
PackageMempoolAcceptResult AcceptMultipleTransactions(const std::vector<CTransactionRef>& txns, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
@@ -619,6 +621,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
{
const CTransaction* ptxConflicting = m_pool.GetConflictTx(txin.prevout);
if (ptxConflicting) {
if (!args.m_allow_bip125_replacement) {
// Transaction conflicts with a mempool tx, but we're not allowing replacements.
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "bip125-replacement-disallowed");
}
if (!setConflicts.count(ptxConflicting->GetHash()))
{
// Allow opt-out of transaction replacement by setting
@@ -645,7 +651,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
break;
}
}
if (fReplacementOptOut || args.disallow_mempool_conflicts) {
if (fReplacementOptOut) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict");
}
@@ -1080,65 +1086,15 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
{
AssertLockHeld(cs_main);
// These context-free package limits can be done before taking the mempool lock.
PackageValidationState package_state;
const unsigned int package_count = txns.size();
if (!CheckPackage(txns, package_state)) return PackageMempoolAcceptResult(package_state, {});
// These context-free package limits can be checked before taking the mempool lock.
if (package_count > MAX_PACKAGE_COUNT) {
package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-too-many-transactions");
return PackageMempoolAcceptResult(package_state, {});
}
const int64_t total_size = std::accumulate(txns.cbegin(), txns.cend(), 0,
[](int64_t sum, const auto& tx) { return sum + GetVirtualTransactionSize(*tx); });
// If the package only contains 1 tx, it's better to report the policy violation on individual tx size.
if (package_count > 1 && total_size > MAX_PACKAGE_SIZE * 1000) {
package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-too-large");
return PackageMempoolAcceptResult(package_state, {});
}
// Construct workspaces and check package policies.
std::vector<Workspace> workspaces{};
workspaces.reserve(package_count);
{
std::unordered_set<uint256, SaltedTxidHasher> later_txids;
std::transform(txns.cbegin(), txns.cend(), std::inserter(later_txids, later_txids.end()),
[](const auto& tx) { return tx->GetHash(); });
// Require the package to be sorted in order of dependency, i.e. parents appear before children.
// An unsorted package will fail anyway on missing-inputs, but it's better to quit earlier and
// fail on something less ambiguous (missing-inputs could also be an orphan or trying to
// spend nonexistent coins).
for (const auto& tx : txns) {
for (const auto& input : tx->vin) {
if (later_txids.find(input.prevout.hash) != later_txids.end()) {
// The parent is a subsequent transaction in the package.
package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-sorted");
return PackageMempoolAcceptResult(package_state, {});
}
}
later_txids.erase(tx->GetHash());
workspaces.emplace_back(Workspace(tx));
}
}
workspaces.reserve(txns.size());
std::transform(txns.cbegin(), txns.cend(), std::back_inserter(workspaces),
[](const auto& tx) { return Workspace(tx); });
std::map<const uint256, const MempoolAcceptResult> results;
{
// Don't allow any conflicting transactions, i.e. spending the same inputs, in a package.
std::unordered_set<COutPoint, SaltedOutpointHasher> inputs_seen;
for (const auto& tx : txns) {
for (const auto& input : tx->vin) {
if (inputs_seen.find(input.prevout) != inputs_seen.end()) {
// This input is also present in another tx in the package.
package_state.Invalid(PackageValidationResult::PCKG_POLICY, "conflict-in-package");
return PackageMempoolAcceptResult(package_state, {});
}
}
// Batch-add all the inputs for a tx at a time. If we added them 1 at a time, we could
// catch duplicate inputs within a single tx. This is a more severe, consensus error,
// and we want to report that from CheckTransaction instead.
std::transform(tx->vin.cbegin(), tx->vin.cend(), std::inserter(inputs_seen, inputs_seen.end()),
[](const auto& input) { return input.prevout; });
}
}
LOCK(m_pool.cs);
@@ -1151,10 +1107,10 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
return PackageMempoolAcceptResult(package_state, std::move(results));
}
// Make the coins created by this transaction available for subsequent transactions in the
// package to spend. Since we already checked conflicts in the package and RBFs are
// impossible, we don't need to track the coins spent. Note that this logic will need to be
// updated if RBFs in packages are allowed in the future.
assert(args.disallow_mempool_conflicts);
// package to spend. Since we already checked conflicts in the package and we don't allow
// replacements, we don't need to track the coins spent. Note that this logic will need to be
// updated if package replace-by-fee is allowed in the future.
assert(!args.m_allow_bip125_replacement);
m_viewmempool.PackageAddTransaction(ws.m_ptx);
}
@@ -1188,7 +1144,7 @@ static MempoolAcceptResult AcceptToMemoryPoolWithTime(const CChainParams& chainp
{
std::vector<COutPoint> coins_to_uncache;
MemPoolAccept::ATMPArgs args { chainparams, nAcceptTime, bypass_limits, coins_to_uncache,
test_accept, /* disallow_mempool_conflicts */ false };
test_accept, /* m_allow_bip125_replacement */ true };
assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate));
const MempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args);
@@ -1225,12 +1181,11 @@ PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTx
std::vector<COutPoint> coins_to_uncache;
const CChainParams& chainparams = Params();
MemPoolAccept::ATMPArgs args { chainparams, GetTime(), /* bypass_limits */ false, coins_to_uncache,
test_accept, /* disallow_mempool_conflicts */ true };
test_accept, /* m_allow_bip125_replacement */ false };
assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate));
const PackageMempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptMultipleTransactions(package, args);
// Uncache coins pertaining to transactions that were not submitted to the mempool.
// Ensure the cache is still within its size limits.
for (const COutPoint& hashTx : coins_to_uncache) {
active_chainstate.CoinsTip().Uncache(hashTx);
}