Update replace-by-fee logic to use fee deltas
This commit is contained in:
parent
eb306664e7
commit
9ef2a25603
2 changed files with 89 additions and 9 deletions
|
@ -63,8 +63,14 @@ def make_utxo(node, amount, confirmed=True, scriptPubKey=CScript([1])):
|
|||
|
||||
# If requested, ensure txouts are confirmed.
|
||||
if confirmed:
|
||||
while len(node.getrawmempool()):
|
||||
mempool_size = len(node.getrawmempool())
|
||||
while mempool_size > 0:
|
||||
node.generate(1)
|
||||
new_size = len(node.getrawmempool())
|
||||
# Error out if we have something stuck in the mempool, as this
|
||||
# would likely be a bug.
|
||||
assert(new_size < mempool_size)
|
||||
mempool_size = new_size
|
||||
|
||||
return COutPoint(int(txid, 16), 0)
|
||||
|
||||
|
@ -72,7 +78,7 @@ class ReplaceByFeeTest(BitcoinTestFramework):
|
|||
|
||||
def setup_network(self):
|
||||
self.nodes = []
|
||||
self.nodes.append(start_node(0, self.options.tmpdir, ["-maxorphantx=1000",
|
||||
self.nodes.append(start_node(0, self.options.tmpdir, ["-maxorphantx=1000", "-debug",
|
||||
"-relaypriority=0", "-whitelist=127.0.0.1",
|
||||
"-limitancestorcount=50",
|
||||
"-limitancestorsize=101",
|
||||
|
@ -108,6 +114,9 @@ class ReplaceByFeeTest(BitcoinTestFramework):
|
|||
print "Running test opt-in..."
|
||||
self.test_opt_in()
|
||||
|
||||
print "Running test prioritised transactions..."
|
||||
self.test_prioritised_transactions()
|
||||
|
||||
print "Passed\n"
|
||||
|
||||
def test_simple_doublespend(self):
|
||||
|
@ -513,5 +522,72 @@ class ReplaceByFeeTest(BitcoinTestFramework):
|
|||
# but make sure it is accepted anyway
|
||||
self.nodes[0].sendrawtransaction(tx3c_hex, True)
|
||||
|
||||
def test_prioritised_transactions(self):
|
||||
# Ensure that fee deltas used via prioritisetransaction are
|
||||
# correctly used by replacement logic
|
||||
|
||||
# 1. Check that feeperkb uses modified fees
|
||||
tx0_outpoint = make_utxo(self.nodes[0], 1.1*COIN)
|
||||
|
||||
tx1a = CTransaction()
|
||||
tx1a.vin = [CTxIn(tx0_outpoint, nSequence=0)]
|
||||
tx1a.vout = [CTxOut(1*COIN, CScript([b'a']))]
|
||||
tx1a_hex = txToHex(tx1a)
|
||||
tx1a_txid = self.nodes[0].sendrawtransaction(tx1a_hex, True)
|
||||
|
||||
# Higher fee, but the actual fee per KB is much lower.
|
||||
tx1b = CTransaction()
|
||||
tx1b.vin = [CTxIn(tx0_outpoint, nSequence=0)]
|
||||
tx1b.vout = [CTxOut(0.001*COIN, CScript([b'a'*740000]))]
|
||||
tx1b_hex = txToHex(tx1b)
|
||||
|
||||
# Verify tx1b cannot replace tx1a.
|
||||
try:
|
||||
tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, True)
|
||||
except JSONRPCException as exp:
|
||||
assert_equal(exp.error['code'], -26)
|
||||
else:
|
||||
assert(False)
|
||||
|
||||
# Use prioritisetransaction to set tx1a's fee to 0.
|
||||
self.nodes[0].prioritisetransaction(tx1a_txid, 0, int(-0.1*COIN))
|
||||
|
||||
# Now tx1b should be able to replace tx1a
|
||||
tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, True)
|
||||
|
||||
assert(tx1b_txid in self.nodes[0].getrawmempool())
|
||||
|
||||
# 2. Check that absolute fee checks use modified fee.
|
||||
tx1_outpoint = make_utxo(self.nodes[0], 1.1*COIN)
|
||||
|
||||
tx2a = CTransaction()
|
||||
tx2a.vin = [CTxIn(tx1_outpoint, nSequence=0)]
|
||||
tx2a.vout = [CTxOut(1*COIN, CScript([b'a']))]
|
||||
tx2a_hex = txToHex(tx2a)
|
||||
tx2a_txid = self.nodes[0].sendrawtransaction(tx2a_hex, True)
|
||||
|
||||
# Lower fee, but we'll prioritise it
|
||||
tx2b = CTransaction()
|
||||
tx2b.vin = [CTxIn(tx1_outpoint, nSequence=0)]
|
||||
tx2b.vout = [CTxOut(1.01*COIN, CScript([b'a']))]
|
||||
tx2b.rehash()
|
||||
tx2b_hex = txToHex(tx2b)
|
||||
|
||||
# Verify tx2b cannot replace tx2a.
|
||||
try:
|
||||
tx2b_txid = self.nodes[0].sendrawtransaction(tx2b_hex, True)
|
||||
except JSONRPCException as exp:
|
||||
assert_equal(exp.error['code'], -26)
|
||||
else:
|
||||
assert(False)
|
||||
|
||||
# Now prioritise tx2b to have a higher modified fee
|
||||
self.nodes[0].prioritisetransaction(tx2b.hash, 0, int(0.1*COIN))
|
||||
|
||||
# tx2b should now be accepted
|
||||
tx2b_txid = self.nodes[0].sendrawtransaction(tx2b_hex, True)
|
||||
|
||||
assert(tx2b_txid in self.nodes[0].getrawmempool())
|
||||
|
||||
if __name__ == '__main__':
|
||||
ReplaceByFeeTest().main()
|
||||
|
|
18
src/main.cpp
18
src/main.cpp
|
@ -1061,13 +1061,17 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
|
|||
uint64_t nConflictingCount = 0;
|
||||
CTxMemPool::setEntries allConflicting;
|
||||
|
||||
CAmount nModifiedFees = nFees;
|
||||
double nPriorityDummy = 0;
|
||||
pool.ApplyDeltas(hash, nPriorityDummy, nModifiedFees);
|
||||
|
||||
// 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(nFees, nSize);
|
||||
CFeeRate newFeeRate(nModifiedFees, nSize);
|
||||
set<uint256> setConflictsParents;
|
||||
const int maxDescendantsToVisit = 100;
|
||||
CTxMemPool::setEntries setIterConflicting;
|
||||
|
@ -1110,7 +1114,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
|
|||
// 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());
|
||||
CFeeRate oldFeeRate(mi->GetModifiedFee(), mi->GetTxSize());
|
||||
if (newFeeRate <= oldFeeRate)
|
||||
{
|
||||
return state.DoS(0,
|
||||
|
@ -1138,7 +1142,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
|
|||
pool.CalculateDescendants(it, allConflicting);
|
||||
}
|
||||
BOOST_FOREACH(CTxMemPool::txiter it, allConflicting) {
|
||||
nConflictingFees += it->GetFee();
|
||||
nConflictingFees += it->GetModifiedFee();
|
||||
nConflictingSize += it->GetTxSize();
|
||||
}
|
||||
} else {
|
||||
|
@ -1171,16 +1175,16 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
|
|||
// 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)
|
||||
if (nModifiedFees < nConflictingFees)
|
||||
{
|
||||
return state.DoS(0, error("AcceptToMemoryPool: rejecting replacement %s, less fees than conflicting txs; %s < %s",
|
||||
hash.ToString(), FormatMoney(nFees), FormatMoney(nConflictingFees)),
|
||||
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 = nFees - nConflictingFees;
|
||||
CAmount nDeltaFees = nModifiedFees - nConflictingFees;
|
||||
if (nDeltaFees < ::minRelayTxFee.GetFee(nSize))
|
||||
{
|
||||
return state.DoS(0,
|
||||
|
@ -1218,7 +1222,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
|
|||
LogPrint("mempool", "replacing tx %s with %s for %s BTC additional fees, %d delta bytes\n",
|
||||
it->GetTx().GetHash().ToString(),
|
||||
hash.ToString(),
|
||||
FormatMoney(nFees - nConflictingFees),
|
||||
FormatMoney(nModifiedFees - nConflictingFees),
|
||||
(int)nSize - (int)nConflictingSize);
|
||||
}
|
||||
pool.RemoveStaged(allConflicting);
|
||||
|
|
Loading…
Reference in a new issue