kvazar
/
kevacoin
mirror of https://github.com/kvazar-network/kevacoin.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Fix mempool limiting for PrioritiseTransaction 

Redo the feerate index to be based on mining score, rather than fee.

Update mempool_packages.py to test prioritisetransaction's effect on
package scores.
0.13
Suhas Daftuar 9 years ago committed by Suhas Daftuar
parent
aeedd8a53b
commit
eb306664e7
4 changed files with 78 additions and 46 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 24
      qa/rpc-tests/mempool_packages.py
  2. 4
      src/rpcblockchain.cpp
  3. 53
      src/txmempool.cpp
  4. 43
      src/txmempool.h

24
qa/rpc-tests/mempool_packages.py
Unescape View File

@ -64,17 +64,41 @@ class MempoolPackagesTest(BitcoinTestFramework):
for x in reversed(chain): for x in reversed(chain):
assert_equal(mempool[x]['descendantcount'], descendant_count) assert_equal(mempool[x]['descendantcount'], descendant_count)
descendant_fees += mempool[x]['fee'] descendant_fees += mempool[x]['fee']
assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee'])
assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees) assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees)
descendant_size += mempool[x]['size'] descendant_size += mempool[x]['size']
assert_equal(mempool[x]['descendantsize'], descendant_size) assert_equal(mempool[x]['descendantsize'], descendant_size)
descendant_count += 1 descendant_count += 1
# Check that descendant modified fees includes fee deltas from
# prioritisetransaction
self.nodes[0].prioritisetransaction(chain[-1], 0, 1000)
mempool = self.nodes[0].getrawmempool(True)
descendant_fees = 0
for x in reversed(chain):
descendant_fees += mempool[x]['fee']
assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees+1000)
# Adding one more transaction on to the chain should fail. # Adding one more transaction on to the chain should fail.
try: try:
self.chain_transaction(self.nodes[0], txid, vout, value, fee, 1) self.chain_transaction(self.nodes[0], txid, vout, value, fee, 1)
except JSONRPCException as e: except JSONRPCException as e:
print "too-long-ancestor-chain successfully rejected" print "too-long-ancestor-chain successfully rejected"
# Check that prioritising a tx before it's added to the mempool works
self.nodes[0].generate(1)
self.nodes[0].prioritisetransaction(chain[-1], 0, 2000)
self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash())
mempool = self.nodes[0].getrawmempool(True)
descendant_fees = 0
for x in reversed(chain):
descendant_fees += mempool[x]['fee']
if (x == chain[-1]):
assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee']+satoshi_round(0.00002))
assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees+2000)
# TODO: check that node1's mempool is as expected # TODO: check that node1's mempool is as expected
# TODO: test ancestor size limits # TODO: test ancestor size limits

4
src/rpcblockchain.cpp
Unescape View File

@ -197,7 +197,7 @@ UniValue mempoolToJSON(bool fVerbose = false)
info.push_back(Pair("currentpriority", e.GetPriority(chainActive.Height()))); info.push_back(Pair("currentpriority", e.GetPriority(chainActive.Height())));
info.push_back(Pair("descendantcount", e.GetCountWithDescendants())); info.push_back(Pair("descendantcount", e.GetCountWithDescendants()));
info.push_back(Pair("descendantsize", e.GetSizeWithDescendants())); info.push_back(Pair("descendantsize", e.GetSizeWithDescendants()));
info.push_back(Pair("descendantfees", e.GetFeesWithDescendants())); info.push_back(Pair("descendantfees", e.GetModFeesWithDescendants()));
const CTransaction& tx = e.GetTx(); const CTransaction& tx = e.GetTx();
set<string> setDepends; set<string> setDepends;
BOOST_FOREACH(const CTxIn& txin, tx.vin) BOOST_FOREACH(const CTxIn& txin, tx.vin)
@ -255,7 +255,7 @@ UniValue getrawmempool(const UniValue& params, bool fHelp)
" \"currentpriority\" : n, (numeric) transaction priority now\n" " \"currentpriority\" : n, (numeric) transaction priority now\n"
" \"descendantcount\" : n, (numeric) number of in-mempool descendant transactions (including this one)\n" " \"descendantcount\" : n, (numeric) number of in-mempool descendant transactions (including this one)\n"
" \"descendantsize\" : n, (numeric) size of in-mempool descendants (including this one)\n" " \"descendantsize\" : n, (numeric) size of in-mempool descendants (including this one)\n"
" \"descendantfees\" : n, (numeric) fees of in-mempool descendants (including this one)\n" " \"descendantfees\" : n, (numeric) modified fees (see above) of in-mempool descendants (including this one)\n"
" \"depends\" : [ (array) unconfirmed transactions used as inputs for this transaction\n" " \"depends\" : [ (array) unconfirmed transactions used as inputs for this transaction\n"
" \"transactionid\", (string) parent transaction id\n" " \"transactionid\", (string) parent transaction id\n"
" ... ]\n" " ... ]\n"

53
src/txmempool.cpp
Unescape View File

@ -33,7 +33,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee,
nCountWithDescendants = 1; nCountWithDescendants = 1;
nSizeWithDescendants = nTxSize; nSizeWithDescendants = nTxSize;
nFeesWithDescendants = nFee; nModFeesWithDescendants = nFee;
CAmount nValueIn = tx.GetValueOut()+nFee; CAmount nValueIn = tx.GetValueOut()+nFee;
assert(inChainInputValue <= nValueIn); assert(inChainInputValue <= nValueIn);
@ -57,6 +57,7 @@ CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const
void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta) void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta)
{ {
nModFeesWithDescendants += newFeeDelta - feeDelta;
feeDelta = newFeeDelta; feeDelta = newFeeDelta;
} }
@ -114,7 +115,7 @@ bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit
BOOST_FOREACH(txiter cit, setAllDescendants) { BOOST_FOREACH(txiter cit, setAllDescendants) {
if (!setExclude.count(cit->GetTx().GetHash())) { if (!setExclude.count(cit->GetTx().GetHash())) {
modifySize += cit->GetTxSize(); modifySize += cit->GetTxSize();
modifyFee += cit->GetFee(); modifyFee += cit->GetModifiedFee();
modifyCount++; modifyCount++;
cachedDescendants[updateIt].insert(cit); cachedDescendants[updateIt].insert(cit);
} }
@ -244,7 +245,7 @@ void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors
} }
const int64_t updateCount = (add ? 1 : -1); const int64_t updateCount = (add ? 1 : -1);
const int64_t updateSize = updateCount * it->GetTxSize(); const int64_t updateSize = updateCount * it->GetTxSize();
const CAmount updateFee = updateCount * it->GetFee(); const CAmount updateFee = updateCount * it->GetModifiedFee();
BOOST_FOREACH(txiter ancestorIt, setAncestors) { BOOST_FOREACH(txiter ancestorIt, setAncestors) {
mapTx.modify(ancestorIt, update_descendant_state(updateSize, updateFee, updateCount)); mapTx.modify(ancestorIt, update_descendant_state(updateSize, updateFee, updateCount));
} }
@ -304,7 +305,7 @@ void CTxMemPoolEntry::SetDirty()
{ {
nCountWithDescendants = 0; nCountWithDescendants = 0;
nSizeWithDescendants = nTxSize; nSizeWithDescendants = nTxSize;
nFeesWithDescendants = nFee; nModFeesWithDescendants = GetModifiedFee();
} }
void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount) void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount)
@ -312,8 +313,7 @@ void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t
if (!IsDirty()) { if (!IsDirty()) {
nSizeWithDescendants += modifySize; nSizeWithDescendants += modifySize;
assert(int64_t(nSizeWithDescendants) > 0); assert(int64_t(nSizeWithDescendants) > 0);
nFeesWithDescendants += modifyFee; nModFeesWithDescendants += modifyFee;
assert(nFeesWithDescendants >= 0);
nCountWithDescendants += modifyCount; nCountWithDescendants += modifyCount;
assert(int64_t(nCountWithDescendants) > 0); assert(int64_t(nCountWithDescendants) > 0);
} }
@ -372,6 +372,17 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
indexed_transaction_set::iterator newit = mapTx.insert(entry).first; indexed_transaction_set::iterator newit = mapTx.insert(entry).first;
mapLinks.insert(make_pair(newit, TxLinks())); mapLinks.insert(make_pair(newit, TxLinks()));
// Update transaction for any feeDelta created by PrioritiseTransaction
// TODO: refactor so that the fee delta is calculated before inserting
// into mapTx.
std::map<uint256, std::pair<double, CAmount> >::const_iterator pos = mapDeltas.find(hash);
if (pos != mapDeltas.end()) {
const std::pair<double, CAmount> &deltas = pos->second;
if (deltas.second) {
mapTx.modify(newit, update_fee_delta(deltas.second));
}
}
// Update cachedInnerUsage to include contained transaction's usage. // Update cachedInnerUsage to include contained transaction's usage.
// (When we update the entry for in-mempool parents, memory usage will be // (When we update the entry for in-mempool parents, memory usage will be
// further updated.) // further updated.)
@ -399,15 +410,6 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
} }
UpdateAncestorsOf(true, newit, setAncestors); UpdateAncestorsOf(true, newit, setAncestors);
// Update transaction's score for any feeDelta created by PrioritiseTransaction
std::map<uint256, std::pair<double, CAmount> >::const_iterator pos = mapDeltas.find(hash);
if (pos != mapDeltas.end()) {
const std::pair<double, CAmount> &deltas = pos->second;
if (deltas.second) {
mapTx.modify(newit, update_fee_delta(deltas.second));
}
}
nTransactionsUpdated++; nTransactionsUpdated++;
totalTxSize += entry.GetTxSize(); totalTxSize += entry.GetTxSize();
minerPolicyEstimator->processTransaction(entry, fCurrentEstimate); minerPolicyEstimator->processTransaction(entry, fCurrentEstimate);
@ -644,27 +646,24 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
CTxMemPool::setEntries setChildrenCheck; CTxMemPool::setEntries setChildrenCheck;
std::map<COutPoint, CInPoint>::const_iterator iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0)); std::map<COutPoint, CInPoint>::const_iterator iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0));
int64_t childSizes = 0; int64_t childSizes = 0;
CAmount childFees = 0; CAmount childModFee = 0;
for (; iter != mapNextTx.end() && iter->first.hash == it->GetTx().GetHash(); ++iter) { for (; iter != mapNextTx.end() && iter->first.hash == it->GetTx().GetHash(); ++iter) {
txiter childit = mapTx.find(iter->second.ptx->GetHash()); txiter childit = mapTx.find(iter->second.ptx->GetHash());
assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions
if (setChildrenCheck.insert(childit).second) { if (setChildrenCheck.insert(childit).second) {
childSizes += childit->GetTxSize(); childSizes += childit->GetTxSize();
childFees += childit->GetFee(); childModFee += childit->GetModifiedFee();
} }
} }
assert(setChildrenCheck == GetMemPoolChildren(it)); assert(setChildrenCheck == GetMemPoolChildren(it));
// Also check to make sure size/fees is greater than sum with immediate children. // Also check to make sure size is greater than sum with immediate children.
// just a sanity check, not definitive that this calc is correct... // just a sanity check, not definitive that this calc is correct...
// also check that the size is less than the size of the entire mempool.
if (!it->IsDirty()) { if (!it->IsDirty()) {
assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize()); assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize());
assert(it->GetFeesWithDescendants() >= childFees + it->GetFee());
} else { } else {
assert(it->GetSizeWithDescendants() == it->GetTxSize()); assert(it->GetSizeWithDescendants() == it->GetTxSize());
assert(it->GetFeesWithDescendants() == it->GetFee()); assert(it->GetModFeesWithDescendants() == it->GetModifiedFee());
} }
assert(it->GetFeesWithDescendants() >= 0);
if (fDependsWait) if (fDependsWait)
waitingOnDependants.push_back(&(*it)); waitingOnDependants.push_back(&(*it));
@ -788,6 +787,14 @@ void CTxMemPool::PrioritiseTransaction(const uint256 hash, const string strHash,
txiter it = mapTx.find(hash); txiter it = mapTx.find(hash);
if (it != mapTx.end()) { if (it != mapTx.end()) {
mapTx.modify(it, update_fee_delta(deltas.second)); mapTx.modify(it, update_fee_delta(deltas.second));
// Now update all ancestors' modified fees with descendants
setEntries setAncestors;
uint64_t nNoLimit = std::numeric_limits<uint64_t>::max();
std::string dummy;
CalculateMemPoolAncestors(*it, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false);
BOOST_FOREACH(txiter ancestorIt, setAncestors) {
mapTx.modify(ancestorIt, update_descendant_state(0, nFeeDelta, 0));
}
} }
} }
LogPrintf("PrioritiseTransaction: %s priority += %f, fee += %d\n", strHash, dPriorityDelta, FormatMoney(nFeeDelta)); LogPrintf("PrioritiseTransaction: %s priority += %f, fee += %d\n", strHash, dPriorityDelta, FormatMoney(nFeeDelta));
@ -956,7 +963,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<uint256>* pvNoSpendsRe
// "minimum reasonable fee rate" (ie some value under which we consider txn // "minimum reasonable fee rate" (ie some value under which we consider txn
// to have 0 fee). This way, we don't allow txn to enter mempool with feerate // to have 0 fee). This way, we don't allow txn to enter mempool with feerate
// equal to txn which were removed with no block in between. // equal to txn which were removed with no block in between.
CFeeRate removed(it->GetFeesWithDescendants(), it->GetSizeWithDescendants()); CFeeRate removed(it->GetModFeesWithDescendants(), it->GetSizeWithDescendants());
removed += minReasonableRelayFee; removed += minReasonableRelayFee;
trackPackageRemoved(removed); trackPackageRemoved(removed);
maxFeeRateRemoved = std::max(maxFeeRateRemoved, removed); maxFeeRateRemoved = std::max(maxFeeRateRemoved, removed);

43
src/txmempool.h
Unescape View File

@ -44,12 +44,12 @@ class CTxMemPool;
* ("descendant" transactions). * ("descendant" transactions).
* *
* When a new entry is added to the mempool, we update the descendant state * When a new entry is added to the mempool, we update the descendant state
* (nCountWithDescendants, nSizeWithDescendants, and nFeesWithDescendants) for * (nCountWithDescendants, nSizeWithDescendants, and nModFeesWithDescendants) for
* all ancestors of the newly added transaction. * all ancestors of the newly added transaction.
* *
* If updating the descendant state is skipped, we can mark the entry as * If updating the descendant state is skipped, we can mark the entry as
* "dirty", and set nSizeWithDescendants/nFeesWithDescendants to equal nTxSize/ * "dirty", and set nSizeWithDescendants/nModFeesWithDescendants to equal nTxSize/
* nTxFee. (This can potentially happen during a reorg, where we limit the * nFee+feeDelta. (This can potentially happen during a reorg, where we limit the
* amount of work we're willing to do to avoid consuming too much CPU.) * amount of work we're willing to do to avoid consuming too much CPU.)
* *
*/ */
@ -74,11 +74,11 @@ private:
// Information about descendants of this transaction that are in the // Information about descendants of this transaction that are in the
// mempool; if we remove this transaction we must remove all of these // mempool; if we remove this transaction we must remove all of these
// descendants as well. if nCountWithDescendants is 0, treat this entry as // descendants as well. if nCountWithDescendants is 0, treat this entry as
// dirty, and nSizeWithDescendants and nFeesWithDescendants will not be // dirty, and nSizeWithDescendants and nModFeesWithDescendants will not be
// correct. // correct.
uint64_t nCountWithDescendants; //! number of descendant transactions uint64_t nCountWithDescendants; //! number of descendant transactions
uint64_t nSizeWithDescendants; //! ... and size uint64_t nSizeWithDescendants; //! ... and size
CAmount nFeesWithDescendants; //! ... and total fees (all including us) CAmount nModFeesWithDescendants; //! ... and total fees (all including us)
public: public:
CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee,
@ -104,7 +104,8 @@ public:
// Adjusts the descendant state, if this entry is not dirty. // Adjusts the descendant state, if this entry is not dirty.
void UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount); void UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount);
// Updates the fee delta used for mining priority score // Updates the fee delta used for mining priority score, and the
// modified fees with descendants.
void UpdateFeeDelta(int64_t feeDelta); void UpdateFeeDelta(int64_t feeDelta);
/** We can set the entry to be dirty if doing the full calculation of in- /** We can set the entry to be dirty if doing the full calculation of in-
@ -116,7 +117,7 @@ public:
uint64_t GetCountWithDescendants() const { return nCountWithDescendants; } uint64_t GetCountWithDescendants() const { return nCountWithDescendants; }
uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; } uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; }
CAmount GetFeesWithDescendants() const { return nFeesWithDescendants; } CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; }
bool GetSpendsCoinbase() const { return spendsCoinbase; } bool GetSpendsCoinbase() const { return spendsCoinbase; }
}; };
@ -163,27 +164,27 @@ struct mempoolentry_txid
} }
}; };
/** \class CompareTxMemPoolEntryByFee /** \class CompareTxMemPoolEntryByDescendantScore
* *
* Sort an entry by max(feerate of entry's tx, feerate with all descendants). * Sort an entry by max(score/size of entry's tx, score/size with all descendants).
*/ */
class CompareTxMemPoolEntryByFee class CompareTxMemPoolEntryByDescendantScore
{ {
public: public:
bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b)
{ {
bool fUseADescendants = UseDescendantFeeRate(a); bool fUseADescendants = UseDescendantScore(a);
bool fUseBDescendants = UseDescendantFeeRate(b); bool fUseBDescendants = UseDescendantScore(b);
double aFees = fUseADescendants ? a.GetFeesWithDescendants() : a.GetFee(); double aModFee = fUseADescendants ? a.GetModFeesWithDescendants() : a.GetModifiedFee();
double aSize = fUseADescendants ? a.GetSizeWithDescendants() : a.GetTxSize(); double aSize = fUseADescendants ? a.GetSizeWithDescendants() : a.GetTxSize();
double bFees = fUseBDescendants ? b.GetFeesWithDescendants() : b.GetFee(); double bModFee = fUseBDescendants ? b.GetModFeesWithDescendants() : b.GetModifiedFee();
double bSize = fUseBDescendants ? b.GetSizeWithDescendants() : b.GetTxSize(); double bSize = fUseBDescendants ? b.GetSizeWithDescendants() : b.GetTxSize();
// Avoid division by rewriting (a/b > c/d) as (a*d > c*b). // Avoid division by rewriting (a/b > c/d) as (a*d > c*b).
double f1 = aFees * bSize; double f1 = aModFee * bSize;
double f2 = aSize * bFees; double f2 = aSize * bModFee;
if (f1 == f2) { if (f1 == f2) {
return a.GetTime() >= b.GetTime(); return a.GetTime() >= b.GetTime();
@ -191,11 +192,11 @@ public:
return f1 < f2; return f1 < f2;
} }
// Calculate which feerate to use for an entry (avoiding division). // Calculate which score to use for an entry (avoiding division).
bool UseDescendantFeeRate(const CTxMemPoolEntry &a) bool UseDescendantScore(const CTxMemPoolEntry &a)
{ {
double f1 = (double)a.GetFee() * a.GetSizeWithDescendants(); double f1 = (double)a.GetModifiedFee() * a.GetSizeWithDescendants();
double f2 = (double)a.GetFeesWithDescendants() * a.GetTxSize(); double f2 = (double)a.GetModFeesWithDescendants() * a.GetTxSize();
return f2 > f1; return f2 > f1;
} }
}; };
@ -350,7 +351,7 @@ public:
// sorted by fee rate // sorted by fee rate
boost::multi_index::ordered_non_unique< boost::multi_index::ordered_non_unique<
boost::multi_index::identity<CTxMemPoolEntry>, boost::multi_index::identity<CTxMemPoolEntry>,
CompareTxMemPoolEntryByFee CompareTxMemPoolEntryByDescendantScore
>, >,
// sorted by entry time // sorted by entry time
boost::multi_index::ordered_non_unique< boost::multi_index::ordered_non_unique<

Write Preview
Loading…
Cancel
Save