From 359e8a03d1667dca3e8375695131b8b5e6c54f0a Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 20 Jan 2017 09:24:35 -0500 Subject: [PATCH] [cleanup] Remove coin age priority completely. Remove GetPriority and ComputePriority. Remove internal machinery for tracking priority in CTxMemPoolEntry. --- src/bench/mempool_eviction.cpp | 5 ++--- src/coins.cpp | 19 ------------------- src/coins.h | 7 ------- src/primitives/transaction.cpp | 26 -------------------------- src/primitives/transaction.h | 6 ------ src/test/mempool_tests.cpp | 1 - src/test/test_bitcoin.cpp | 4 ++-- src/txmempool.cpp | 19 ++----------------- src/txmempool.h | 12 ++---------- src/validation.cpp | 7 ++----- src/wallet/wallet.cpp | 2 +- 11 files changed, 11 insertions(+), 97 deletions(-) diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index 5790d51a82..31a392ae7c 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -12,14 +12,13 @@ static void AddTx(const CTransaction& tx, const CAmount& nFee, CTxMemPool& pool) { int64_t nTime = 0; - double dPriority = 10.0; unsigned int nHeight = 1; bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; pool.addUnchecked(tx.GetHash(), CTxMemPoolEntry( - MakeTransactionRef(tx), nFee, nTime, dPriority, nHeight, - tx.GetValueOut(), spendsCoinbase, sigOpCost, lp)); + MakeTransactionRef(tx), nFee, nTime, nHeight, + spendsCoinbase, sigOpCost, lp)); } // Right now this is only testing eviction performance in an extremely small diff --git a/src/coins.cpp b/src/coins.cpp index 4d0e4bc0ad..b2e33abf33 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -295,25 +295,6 @@ bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const return true; } -double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight, CAmount &inChainInputValue) const -{ - inChainInputValue = 0; - if (tx.IsCoinBase()) - return 0.0; - double dResult = 0.0; - BOOST_FOREACH(const CTxIn& txin, tx.vin) - { - const CCoins* coins = AccessCoins(txin.prevout.hash); - assert(coins); - if (!coins->IsAvailable(txin.prevout.n)) continue; - if (coins->nHeight <= nHeight) { - dResult += (double)(coins->vout[txin.prevout.n].nValue) * (nHeight-coins->nHeight); - inChainInputValue += coins->vout[txin.prevout.n].nValue; - } - } - return tx.ComputePriority(dResult); -} - CCoinsModifier::CCoinsModifier(CCoinsViewCache& cache_, CCoinsMap::iterator it_, size_t usage) : cache(cache_), it(it_), cachedCoinUsage(usage) { assert(!cache.hasModifier); cache.hasModifier = true; diff --git a/src/coins.h b/src/coins.h index 902cb57f69..d921f5c2a5 100644 --- a/src/coins.h +++ b/src/coins.h @@ -460,13 +460,6 @@ public: //! Check whether all prevouts of the transaction are present in the UTXO set represented by this view bool HaveInputs(const CTransaction& tx) const; - /** - * Return priority of tx at height nHeight. Also calculate the sum of the values of the inputs - * that are already in the chain. These are the inputs that will age and increase priority as - * new blocks are added to the chain. - */ - double GetPriority(const CTransaction &tx, int nHeight, CAmount &inChainInputValue) const; - const CTxOut &GetOutputFor(const CTxIn& input) const; friend class CCoinsModifier; diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 790bc71d14..364a70adcd 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -89,32 +89,6 @@ CAmount CTransaction::GetValueOut() const return nValueOut; } -double CTransaction::ComputePriority(double dPriorityInputs, unsigned int nTxSize) const -{ - nTxSize = CalculateModifiedSize(nTxSize); - if (nTxSize == 0) return 0.0; - - return dPriorityInputs / nTxSize; -} - -unsigned int CTransaction::CalculateModifiedSize(unsigned int nTxSize) const -{ - // In order to avoid disincentivizing cleaning up the UTXO set we don't count - // the constant overhead for each txin and up to 110 bytes of scriptSig (which - // is enough to cover a compressed pubkey p2sh redemption) for priority. - // Providing any more cleanup incentive than making additional inputs free would - // risk encouraging people to create junk outputs to redeem later. - if (nTxSize == 0) - nTxSize = (GetTransactionWeight(*this) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR; - for (std::vector::const_iterator it(vin.begin()); it != vin.end(); ++it) - { - unsigned int offset = 41U + std::min(110U, (unsigned int)it->scriptSig.size()); - if (nTxSize > offset) - nTxSize -= offset; - } - return nTxSize; -} - unsigned int CTransaction::GetTotalSize() const { return ::GetSerializeSize(*this, SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index af2986a41b..d413e8b087 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -361,12 +361,6 @@ public: // GetValueIn() is a method on CCoinsViewCache, because // inputs must be known to compute value in. - // Compute priority, given priority of inputs and (optionally) tx size - double ComputePriority(double dPriorityInputs, unsigned int nTxSize=0) const; - - // Compute modified tx size for priority calculation (optionally given tx size) - unsigned int CalculateModifiedSize(unsigned int nTxSize=0) const; - /** * Get the total transaction size in bytes, including witness data. * "Total Size" defined in BIP141 and BIP144. diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index cdef1f5aa9..a3f706d9af 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -407,7 +407,6 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) /* set the fee to just below tx2's feerate when including ancestor */ CAmount fee = (20000/tx2Size)*(tx7Size + tx6Size) - 1; - //CTxMemPoolEntry entry7(tx7, fee, 2, 10.0, 1, true); pool.addUnchecked(tx7.GetHash(), entry.Fee(fee).FromTx(tx7)); BOOST_CHECK_EQUAL(pool.size(), 7); sortedOrder.insert(sortedOrder.begin()+1, tx7.GetHash().ToString()); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index e6d34de1af..fc37474362 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -147,8 +147,8 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction &tx) { } CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransaction &txn) { - return CTxMemPoolEntry(MakeTransactionRef(txn), nFee, nTime, 0.0, nHeight, - 0, spendsCoinbase, sigOpCost, lp); + return CTxMemPoolEntry(MakeTransactionRef(txn), nFee, nTime, nHeight, + spendsCoinbase, sigOpCost, lp); } void Shutdown(void* parg) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 58a71ad95e..79dd0fb5f9 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -19,22 +19,17 @@ #include "version.h" CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, - int64_t _nTime, double _entryPriority, unsigned int _entryHeight, - CAmount _inChainInputValue, + int64_t _nTime, unsigned int _entryHeight, bool _spendsCoinbase, int64_t _sigOpsCost, LockPoints lp): - tx(_tx), nFee(_nFee), nTime(_nTime), entryPriority(_entryPriority), entryHeight(_entryHeight), - inChainInputValue(_inChainInputValue), + tx(_tx), nFee(_nFee), nTime(_nTime), entryHeight(_entryHeight), spendsCoinbase(_spendsCoinbase), sigOpCost(_sigOpsCost), lockPoints(lp) { nTxWeight = GetTransactionWeight(*tx); - nModSize = tx->CalculateModifiedSize(GetTxSize()); nUsageSize = RecursiveDynamicUsage(*tx) + memusage::DynamicUsage(tx); nCountWithDescendants = 1; nSizeWithDescendants = GetTxSize(); nModFeesWithDescendants = nFee; - CAmount nValueIn = tx->GetValueOut()+nFee; - assert(inChainInputValue <= nValueIn); feeDelta = 0; @@ -49,16 +44,6 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other) *this = other; } -double -CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const -{ - double deltaPriority = ((double)(currentHeight-entryHeight)*inChainInputValue)/nModSize; - double dResult = entryPriority + deltaPriority; - if (dResult < 0) // This should only happen if it was called with a height below entry height - dResult = 0; - return dResult; -} - void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta) { nModFeesWithDescendants += newFeeDelta - feeDelta; diff --git a/src/txmempool.h b/src/txmempool.h index 6b2e680305..5d82e3016c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -73,12 +73,9 @@ private: CTransactionRef tx; CAmount nFee; //!< Cached to avoid expensive parent-transaction lookups size_t nTxWeight; //!< ... and avoid recomputing tx weight (also used for GetTxSize()) - size_t nModSize; //!< ... and modified size for priority size_t nUsageSize; //!< ... and total memory usage int64_t nTime; //!< Local time when entering the mempool - double entryPriority; //!< Priority when entering the mempool unsigned int entryHeight; //!< Chain height when entering the mempool - CAmount inChainInputValue; //!< Sum of all txin values that are already in blockchain bool spendsCoinbase; //!< keep track of transactions that spend a coinbase int64_t sigOpCost; //!< Total sigop cost int64_t feeDelta; //!< Used for determining the priority of the transaction for mining in a block @@ -101,19 +98,14 @@ private: public: CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, - int64_t _nTime, double _entryPriority, unsigned int _entryHeight, - CAmount _inChainInputValue, bool spendsCoinbase, + int64_t _nTime, unsigned int _entryHeight, + bool spendsCoinbase, int64_t nSigOpsCost, LockPoints lp); CTxMemPoolEntry(const CTxMemPoolEntry& other); const CTransaction& GetTx() const { return *this->tx; } CTransactionRef GetSharedTx() const { return this->tx; } - /** - * Fast calculation of lower bound of current priority as update - * from entry priority. Only inputs that were originally in-chain will age. - */ - double GetPriority(unsigned int currentHeight) const; const CAmount& GetFee() const { return nFee; } size_t GetTxSize() const; size_t GetTxWeight() const { return nTxWeight; } diff --git a/src/validation.cpp b/src/validation.cpp index 1f57468bc6..4a67dead32 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -722,9 +722,6 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C CAmount nModifiedFees = nFees; pool.ApplyDelta(hash, nModifiedFees); - CAmount inChainInputValue; - double dPriority = view.GetPriority(tx, chainActive.Height(), inChainInputValue); - // Keep track of transactions that spend a coinbase, which we re-scan // during reorgs to ensure COINBASE_MATURITY is still met. bool fSpendsCoinbase = false; @@ -736,8 +733,8 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C } } - CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, dPriority, chainActive.Height(), - inChainInputValue, fSpendsCoinbase, nSigOpsCost, lp); + CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, chainActive.Height(), + fSpendsCoinbase, nSigOpsCost, lp); unsigned int nSize = entry.GetTxSize(); // Check that the transaction doesn't have an excessive number of diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ba16cdf267..12117abd17 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2699,7 +2699,7 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt if (GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits LockPoints lp; - CTxMemPoolEntry entry(wtxNew.tx, 0, 0, 0, 0, 0, false, 0, lp); + CTxMemPoolEntry entry(wtxNew.tx, 0, 0, 0, false, 0, lp); CTxMemPool::setEntries setAncestors; size_t nLimitAncestors = GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); size_t nLimitAncestorSize = GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000;