From 186e61b20aa8e9fc359e41c14df698ea7ab72bd8 Mon Sep 17 00:00:00 2001 From: TomZ Date: Wed, 17 Feb 2016 11:07:59 +0000 Subject: [PATCH] Remove the code that accepts ReplaceByFee --- src/init.cpp | 10 --- src/main.cpp | 208 ++---------------------------------------------- src/main.h | 3 - src/txmempool.h | 10 +-- 4 files changed, 10 insertions(+), 221 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index c5e60343f33..d77853a322a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -489,7 +489,6 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-bytespersigop", strprintf(_("Minimum bytes per sigop in transactions we relay and mine (default: %u)"), DEFAULT_BYTES_PER_SIGOP)); strUsage += HelpMessageOpt("-datacarrier", strprintf(_("Relay and mine data carrier transactions (default: %u)"), DEFAULT_ACCEPT_DATACARRIER)); strUsage += HelpMessageOpt("-datacarriersize", strprintf(_("Maximum size of data in data carrier transactions we relay and mine (default: %u)"), MAX_OP_RETURN_RELAY)); - strUsage += HelpMessageOpt("-mempoolreplacement", strprintf(_("Enable transaction replacement in the memory pool (default: %u)"), DEFAULT_ENABLE_REPLACEMENT)); strUsage += HelpMessageGroup(_("Block creation options:")); strUsage += HelpMessageOpt("-blockminsize=", strprintf(_("Set minimum block size in bytes (default: %u)"), DEFAULT_BLOCK_MIN_SIZE)); @@ -1028,15 +1027,6 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) if (GetBoolArg("-peerbloomfilters", true)) nLocalServices |= NODE_BLOOM; - fEnableReplacement = GetBoolArg("-mempoolreplacement", DEFAULT_ENABLE_REPLACEMENT); - if ((!fEnableReplacement) && mapArgs.count("-mempoolreplacement")) { - // Minimal effort at forwards compatibility - std::string strReplacementModeList = GetArg("-mempoolreplacement", ""); // default is impossible - std::vector vstrReplacementModes; - boost::split(vstrReplacementModes, strReplacementModeList, boost::is_any_of(",")); - fEnableReplacement = (std::find(vstrReplacementModes.begin(), vstrReplacementModes.end(), "fee") != vstrReplacementModes.end()); - } - // ********************************************************* Step 4: application initialization: dir lock, daemonize, pidfile, debug log // Initialize elliptic curve code diff --git a/src/main.cpp b/src/main.cpp index 0eb5b58765f..326cadedaa2 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -77,7 +77,6 @@ bool fCheckpointsEnabled = DEFAULT_CHECKPOINTS_ENABLED; size_t nCoinCacheUsage = 5000 * 300; uint64_t nPruneTarget = 0; bool fAlerts = DEFAULT_ALERTS; -bool fEnableReplacement = DEFAULT_ENABLE_REPLACEMENT; /** Fees smaller than this (in satoshi) are considered zero fee (for relaying, mining and transaction creation) */ CFeeRate minRelayTxFee = CFeeRate(DEFAULT_MIN_RELAY_TX_FEE); @@ -852,46 +851,15 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C if (pool.exists(hash)) return state.Invalid(false, REJECT_ALREADY_KNOWN, "txn-already-in-mempool"); - // Check for conflicts with in-memory transactions - set setConflicts; { LOCK(pool.cs); // protect pool.mapNextTx - BOOST_FOREACH(const CTxIn &txin, tx.vin) + for (unsigned int i = 0; i < tx.vin.size(); i++) { - if (pool.mapNextTx.count(txin.prevout)) + COutPoint outpoint = tx.vin[i].prevout; + if (pool.mapNextTx.count(outpoint)) { - const CTransaction *ptxConflicting = pool.mapNextTx[txin.prevout].ptx; - if (!setConflicts.count(ptxConflicting->GetHash())) - { - // Allow opt-out of transaction replacement by setting - // nSequence >= maxint-1 on all inputs. - // - // maxint-1 is picked to still allow use of nLockTime by - // non-replacable transactions. All inputs rather than just one - // is for the sake of multi-party protocols, where we don't - // want a single party to be able to disable replacement. - // - // The opt-out ignores descendants as anyone relying on - // first-seen mempool behavior should be checking all - // unconfirmed ancestors anyway; doing otherwise is hopelessly - // insecure. - bool fReplacementOptOut = true; - if (fEnableReplacement) - { - BOOST_FOREACH(const CTxIn &txin, ptxConflicting->vin) - { - if (txin.nSequence < std::numeric_limits::max()-1) - { - fReplacementOptOut = false; - break; - } - } - } - if (fReplacementOptOut) - return state.Invalid(false, REJECT_CONFLICT, "txn-mempool-conflict"); - - setConflicts.insert(ptxConflicting->GetHash()); - } + // Disable replacement feature for now + return state.Invalid(false, REJECT_CONFLICT, "txn-mempool-conflict"); } } } @@ -1027,160 +995,6 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C return state.DoS(0, false, REJECT_NONSTANDARD, "too-long-mempool-chain", false, errString); } - // A transaction that spends outputs that would be replaced by it is invalid. Now - // that we have the set of all ancestors we can detect this - // pathological case by making sure setConflicts and setAncestors don't - // intersect. - BOOST_FOREACH(CTxMemPool::txiter ancestorIt, setAncestors) - { - const uint256 &hashAncestor = ancestorIt->GetTx().GetHash(); - if (setConflicts.count(hashAncestor)) - { - return state.DoS(10, error("AcceptToMemoryPool: %s spends conflicting transaction %s", - hash.ToString(), - hashAncestor.ToString()), - REJECT_INVALID, "bad-txns-spends-conflicting-tx"); - } - } - - // Check if it's economically rational to mine this transaction rather - // than the ones it replaces. - CAmount nConflictingFees = 0; - size_t nConflictingSize = 0; - uint64_t nConflictingCount = 0; - CTxMemPool::setEntries allConflicting; - - // If we don't hold the lock allConflicting might be incomplete; the - // subsequent RemoveStaged() and addUnchecked() calls don't guarantee - // mempool consistency for us. - LOCK(pool.cs); - if (setConflicts.size()) - { - CFeeRate newFeeRate(nModifiedFees, nSize); - set setConflictsParents; - const int maxDescendantsToVisit = 100; - CTxMemPool::setEntries setIterConflicting; - BOOST_FOREACH(const uint256 &hashConflicting, setConflicts) - { - CTxMemPool::txiter mi = pool.mapTx.find(hashConflicting); - if (mi == pool.mapTx.end()) - continue; - - // Save these to avoid repeated lookups - setIterConflicting.insert(mi); - - // If this entry is "dirty", then we don't have descendant - // state for this transaction, which means we probably have - // lots of in-mempool descendants. - // Don't allow replacements of dirty transactions, to ensure - // that we don't spend too much time walking descendants. - // This should be rare. - if (mi->IsDirty()) { - return state.DoS(0, - error("AcceptToMemoryPool: rejecting replacement %s; cannot replace tx %s with untracked descendants", - hash.ToString(), - mi->GetTx().GetHash().ToString()), - REJECT_NONSTANDARD, "too many potential replacements"); - } - - // Don't allow the replacement to reduce the feerate of the - // mempool. - // - // We usually don't want to accept replacements with lower - // feerates than what they replaced as that would lower the - // feerate of the next block. Requiring that the feerate always - // be increased is also an easy-to-reason about way to prevent - // DoS attacks via replacements. - // - // The mining code doesn't (currently) take children into - // account (CPFP) so we only consider the feerates of - // transactions being directly replaced, not their indirect - // descendants. While that does mean high feerate children are - // ignored when deciding whether or not to replace, we do - // require the replacement to pay more overall fees too, - // mitigating most cases. - CFeeRate oldFeeRate(mi->GetModifiedFee(), mi->GetTxSize()); - if (newFeeRate <= oldFeeRate) - { - return state.DoS(0, - error("AcceptToMemoryPool: rejecting replacement %s; new feerate %s <= old feerate %s", - hash.ToString(), - newFeeRate.ToString(), - oldFeeRate.ToString()), - REJECT_INSUFFICIENTFEE, "insufficient fee"); - } - - BOOST_FOREACH(const CTxIn &txin, mi->GetTx().vin) - { - setConflictsParents.insert(txin.prevout.hash); - } - - nConflictingCount += mi->GetCountWithDescendants(); - } - // This potentially overestimates the number of actual descendants - // but we just want to be conservative to avoid doing too much - // work. - if (nConflictingCount <= maxDescendantsToVisit) { - // If not too many to replace, then calculate the set of - // transactions that would have to be evicted - BOOST_FOREACH(CTxMemPool::txiter it, setIterConflicting) { - pool.CalculateDescendants(it, allConflicting); - } - BOOST_FOREACH(CTxMemPool::txiter it, allConflicting) { - nConflictingFees += it->GetModifiedFee(); - nConflictingSize += it->GetTxSize(); - } - } else { - return state.DoS(0, - error("AcceptToMemoryPool: rejecting replacement %s; too many potential replacements (%d > %d)\n", - hash.ToString(), - nConflictingCount, - maxDescendantsToVisit), - REJECT_NONSTANDARD, "too many potential replacements"); - } - - for (unsigned int j = 0; j < tx.vin.size(); j++) - { - // We don't want to accept replacements that require low - // feerate junk to be mined first. Ideally we'd keep track of - // the ancestor feerates and make the decision based on that, - // but for now requiring all new inputs to be confirmed works. - if (!setConflictsParents.count(tx.vin[j].prevout.hash)) - { - // Rather than check the UTXO set - potentially expensive - - // it's cheaper to just check if the new input refers to a - // tx that's in the mempool. - if (pool.mapTx.find(tx.vin[j].prevout.hash) != pool.mapTx.end()) - return state.DoS(0, error("AcceptToMemoryPool: replacement %s adds unconfirmed input, idx %d", - hash.ToString(), j), - REJECT_NONSTANDARD, "replacement-adds-unconfirmed"); - } - } - - // The replacement must pay greater fees than the transactions it - // replaces - if we did the bandwidth used by those conflicting - // transactions would not be paid for. - if (nModifiedFees < nConflictingFees) - { - return state.DoS(0, error("AcceptToMemoryPool: rejecting replacement %s, less fees than conflicting txs; %s < %s", - hash.ToString(), FormatMoney(nModifiedFees), FormatMoney(nConflictingFees)), - REJECT_INSUFFICIENTFEE, "insufficient fee"); - } - - // Finally in addition to paying more fees than the conflicts the - // new transaction must pay for its own bandwidth. - CAmount nDeltaFees = nModifiedFees - nConflictingFees; - if (nDeltaFees < ::minRelayTxFee.GetFee(nSize)) - { - return state.DoS(0, - error("AcceptToMemoryPool: rejecting replacement %s, not enough additional fees to relay; %s < %s", - hash.ToString(), - FormatMoney(nDeltaFees), - FormatMoney(::minRelayTxFee.GetFee(nSize))), - REJECT_INSUFFICIENTFEE, "insufficient fee"); - } - } - // Check against previous transactions // This is done last to help prevent CPU exhaustion denial-of-service attacks. ValidationCostTracker costTracker(MAX_BLOCK_SIGOPS, MAX_BLOCK_SIGHASH); @@ -1214,18 +1028,6 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); } - - // Remove conflicting transactions from the mempool - BOOST_FOREACH(const CTxMemPool::txiter it, allConflicting) - { - LogPrint("mempool", "replacing tx %s with %s for %s BTC additional fees, %d delta bytes\n", - it->GetTx().GetHash().ToString(), - hash.ToString(), - FormatMoney(nModifiedFees - nConflictingFees), - (int)nSize - (int)nConflictingSize); - } - pool.RemoveStaged(allConflicting); - // Store transaction in memory pool.addUnchecked(hash, entry, setAncestors, !IsInitialBlockDownload()); diff --git a/src/main.h b/src/main.h index 0ca3042e8f1..b44b1d5dcde 100644 --- a/src/main.h +++ b/src/main.h @@ -110,8 +110,6 @@ static const bool DEFAULT_TXINDEX = false; static const unsigned int DEFAULT_BANSCORE_THRESHOLD = 100; static const bool DEFAULT_TESTSAFEMODE = false; -/** Default for -mempoolreplacement */ -static const bool DEFAULT_ENABLE_REPLACEMENT = false; /** Maximum number of headers to announce when relaying blocks with headers message.*/ static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8; @@ -144,7 +142,6 @@ extern bool fCheckpointsEnabled; extern size_t nCoinCacheUsage; extern CFeeRate minRelayTxFee; extern bool fAlerts; -extern bool fEnableReplacement; /** Best header we've seen so far (used for getheaders queries' starting points). */ extern CBlockIndex *pindexBestHeader; diff --git a/src/txmempool.h b/src/txmempool.h index 386cb26d256..0eccd6fa728 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -471,11 +471,6 @@ public: */ bool CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents = true); - /** Populate setDescendants with all in-mempool descendants of hash. - * Assumes that setDescendants includes all in-mempool descendants of anything - * already in it. */ - void CalculateDescendants(txiter it, setEntries &setDescendants); - /** The minimum fee to get into the mempool, which may itself not be enough * for larger-sized transactions. * The minReasonableRelayFee constructor arg is used to bound the time it @@ -565,6 +560,11 @@ private: /** Sever link between specified transaction and direct children. */ void UpdateChildrenForRemoval(txiter entry); + /** Populate setDescendants with all in-mempool descendants of hash. + * Assumes that setDescendants includes all in-mempool descendants of anything + * already in it. */ + void CalculateDescendants(txiter it, setEntries &setDescendants); + /** Before calling removeUnchecked for a given transaction, * UpdateForRemoveFromMempool must be called on the entire (dependent) set * of transactions being removed at the same time. We use each