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.
This commit is contained in:
Peter Todd 2015-10-29 22:55:48 -04:00
parent 0137e6fafd
commit fc8c19a07c
2 changed files with 39 additions and 25 deletions

View File

@ -219,7 +219,7 @@ class Test_ReplaceByFee(unittest.TestCase):
self.proxy.getrawtransaction(tx.GetHash())
def test_replacement_feeperkb(self):
"""Replacement requires overall fee-per-KB to be higher"""
"""Replacement requires fee-per-KB to be higher"""
tx0_outpoint = self.make_txout(1.1*COIN)
tx1a = CTransaction([CTxIn(tx0_outpoint, nSequence=0)],

View File

@ -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