aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorPeter Todd <pete@petertodd.org>2015-10-29 22:55:48 -0400
committerPeter Todd <pete@petertodd.org>2015-11-10 14:14:07 -0500
commitfc8c19a07c20ab63f6a69f7494f486204d8f2b7a (patch)
treed238f7f8bc0690e2c5f5f813f97c83632f2c3f72 /src
parent0137e6fafd08788879193c1155883364237869f1 (diff)
Prevent low feerate txs from (directly) replacing high feerate txs
Previously all conflicting transactions were evaluated as a whole to determine if the feerate was being increased. This meant that low feerate children pulled the feerate down, potentially allowing a high transaction with a high feerate to be replaced by one with a lower feerate.
Diffstat (limited to 'src')
-rw-r--r--src/main.cpp62
1 files changed, 38 insertions, 24 deletions
diff --git a/src/main.cpp b/src/main.cpp
index 274a336ee2..10d661b2a8 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -1008,23 +1008,51 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
{
LOCK(pool.cs);
- // For efficiency we simply sum up the pre-calculated
- // fees/size-with-descendants values from the mempool package
- // tracking; this does mean the pathological case of diamond tx
- // graphs will be overcounted.
+ CFeeRate newFeeRate(nFees, nSize);
BOOST_FOREACH(const uint256 hashConflicting, setConflicts)
{
CTxMemPool::txiter mi = pool.mapTx.find(hashConflicting);
if (mi == pool.mapTx.end())
continue;
+
+ // 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->GetFee(), 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");
+ }
+
+ // For efficiency we simply sum up the pre-calculated
+ // fees/size-with-descendants values from the mempool package
+ // tracking; this does mean the pathological case of diamond tx
+ // graphs will be overcounted.
nConflictingFees += mi->GetFeesWithDescendants();
nConflictingSize += mi->GetSizeWithDescendants();
}
- // First of all we can't allow a replacement unless it pays greater
- // fees than the transactions it conflicts with - if we did the
- // bandwidth used by those conflicting transactions would not be
- // paid for
+ // 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 (nFees < nConflictingFees)
{
return state.DoS(0, error("AcceptToMemoryPool: rejecting replacement %s, less fees than conflicting txs; %s < %s",
@@ -1032,8 +1060,8 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
REJECT_INSUFFICIENTFEE, "insufficient fee");
}
- // Secondly in addition to paying more fees than the conflicts the
- // new transaction must additionally pay for its own bandwidth.
+ // Finally in addition to paying more fees than the conflicts the
+ // new transaction must pay for its own bandwidth.
CAmount nDeltaFees = nFees - nConflictingFees;
if (nDeltaFees < ::minRelayTxFee.GetFee(nSize))
{
@@ -1044,20 +1072,6 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
FormatMoney(::minRelayTxFee.GetFee(nSize))),
REJECT_INSUFFICIENTFEE, "insufficient fee");
}
-
- // Finally replace only if we end up with a larger fees-per-kb than
- // the replacements.
- CFeeRate oldFeeRate(nConflictingFees, nConflictingSize);
- CFeeRate newFeeRate(nFees, nSize);
- 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");
- }
}
// Check against previous transactions