From 5945819717eb842df28cd9291a226d0505cb49d0 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 17 Nov 2015 16:00:19 -0500 Subject: [PATCH 1/3] Remove default arguments for CTxMemPoolEntry() --- src/txmempool.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/txmempool.h b/src/txmempool.h index 7f43120f7..e1ecad360 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -78,7 +78,7 @@ private: public: CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, - int64_t _nTime, double _dPriority, unsigned int _nHeight, bool poolHasNoInputsOf = false); + int64_t _nTime, double _dPriority, unsigned int _nHeight, bool poolHasNoInputsOf); CTxMemPoolEntry(const CTxMemPoolEntry& other); const CTransaction& GetTx() const { return this->tx; } From 71f1d9fd4ae2c2fc90d5487bdf2096f9eb5898d9 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 30 Jun 2015 11:14:24 -0400 Subject: [PATCH 2/3] Modify variable names for entry height and priority --- src/txmempool.cpp | 10 +++++----- src/txmempool.h | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index ec7971c2f..ea3aad34a 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -19,9 +19,9 @@ using namespace std; CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, - int64_t _nTime, double _dPriority, - unsigned int _nHeight, bool poolHasNoInputsOf): - tx(_tx), nFee(_nFee), nTime(_nTime), dPriority(_dPriority), nHeight(_nHeight), + int64_t _nTime, double _entryPriority, + unsigned int _entryHeight, bool poolHasNoInputsOf): + tx(_tx), nFee(_nFee), nTime(_nTime), entryPriority(_entryPriority), entryHeight(_entryHeight), hadNoDependencies(poolHasNoInputsOf) { nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); @@ -42,8 +42,8 @@ double CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const { CAmount nValueIn = tx.GetValueOut()+nFee; - double deltaPriority = ((double)(currentHeight-nHeight)*nValueIn)/nModSize; - double dResult = dPriority + deltaPriority; + double deltaPriority = ((double)(currentHeight-entryHeight)*nValueIn)/nModSize; + double dResult = entryPriority + deltaPriority; return dResult; } diff --git a/src/txmempool.h b/src/txmempool.h index e1ecad360..e189e2e46 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -63,8 +63,8 @@ private: 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 dPriority; //! Priority when entering the mempool - unsigned int nHeight; //! Chain height when entering the mempool + double entryPriority; //! Priority when entering the mempool + unsigned int entryHeight; //! Chain height when entering the mempool bool hadNoDependencies; //! Not dependent on any other txs when it entered the mempool // Information about descendants of this transaction that are in the @@ -78,7 +78,7 @@ private: public: CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, - int64_t _nTime, double _dPriority, unsigned int _nHeight, bool poolHasNoInputsOf); + int64_t _nTime, double _entryPriority, unsigned int _entryHeight, bool poolHasNoInputsOf); CTxMemPoolEntry(const CTxMemPoolEntry& other); const CTransaction& GetTx() const { return this->tx; } @@ -86,7 +86,7 @@ public: const CAmount& GetFee() const { return nFee; } size_t GetTxSize() const { return nTxSize; } int64_t GetTime() const { return nTime; } - unsigned int GetHeight() const { return nHeight; } + unsigned int GetHeight() const { return entryHeight; } bool WasClearAtEntry() const { return hadNoDependencies; } size_t DynamicMemoryUsage() const { return nUsageSize; } From c0353064ddf71ad103bd19f6e7c10ff8e240ac46 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 13 Nov 2015 10:05:21 -0500 Subject: [PATCH 3/3] Change GetPriority calculation. Compute the value of inputs that already are in the chain at time of mempool entry and only increase priority due to aging for those inputs. This effectively changes the CTxMemPoolEntry's GetPriority calculation from an upper bound to a lower bound. --- src/coins.cpp | 6 ++++-- src/coins.h | 8 ++++++-- src/main.cpp | 7 ++++--- src/test/policyestimator_tests.cpp | 2 +- src/test/test_bitcoin.cpp | 9 +++++++-- src/txmempool.cpp | 13 ++++++++----- src/txmempool.h | 8 +++++++- 7 files changed, 37 insertions(+), 16 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index f0ea5c045..723e11470 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -243,8 +243,9 @@ bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const return true; } -double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight) const +double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight, CAmount &inChainInputValue) const { + inChainInputValue = 0; if (tx.IsCoinBase()) return 0.0; double dResult = 0.0; @@ -253,8 +254,9 @@ double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight) const const CCoins* coins = AccessCoins(txin.prevout.hash); assert(coins); if (!coins->IsAvailable(txin.prevout.n)) continue; - if (coins->nHeight < nHeight) { + if (coins->nHeight <= nHeight) { dResult += coins->vout[txin.prevout.n].nValue * (nHeight-coins->nHeight); + inChainInputValue += coins->vout[txin.prevout.n].nValue; } } return tx.ComputePriority(dResult); diff --git a/src/coins.h b/src/coins.h index 99b25de45..d17442210 100644 --- a/src/coins.h +++ b/src/coins.h @@ -456,8 +456,12 @@ 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 - double GetPriority(const CTransaction &tx, int nHeight) 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; diff --git a/src/main.cpp b/src/main.cpp index 33bd2e0ce..55b051734 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -950,9 +950,10 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa CAmount nValueOut = tx.GetValueOut(); CAmount nFees = nValueIn-nValueOut; - double dPriority = view.GetPriority(tx, chainActive.Height()); + CAmount inChainInputValue; + double dPriority = view.GetPriority(tx, chainActive.Height(), inChainInputValue); - CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainActive.Height(), pool.HasNoInputsOf(tx)); + CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainActive.Height(), pool.HasNoInputsOf(tx), inChainInputValue); unsigned int nSize = entry.GetTxSize(); // Don't accept it if it can't get into a block @@ -964,7 +965,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa CAmount mempoolRejectFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nSize); if (mempoolRejectFee > 0 && nFees < mempoolRejectFee) { return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", false, strprintf("%d < %d", nFees, mempoolRejectFee)); - } else if (GetBoolArg("-relaypriority", DEFAULT_RELAYPRIORITY) && nFees < ::minRelayTxFee.GetFee(nSize) && !AllowFree(view.GetPriority(tx, chainActive.Height() + 1))) { + } else if (GetBoolArg("-relaypriority", DEFAULT_RELAYPRIORITY) && nFees < ::minRelayTxFee.GetFee(nSize) && !AllowFree(entry.GetPriority(chainActive.Height() + 1))) { // Require that free transactions have sufficient priority to be mined in the next block. return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "insufficient priority"); } diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 1315146f1..644c3da21 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -196,7 +196,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) // Test that if the mempool is limited, estimateSmartFee won't return a value below the mempool min fee // and that estimateSmartPriority returns essentially an infinite value - mpool.addUnchecked(tx.GetHash(), CTxMemPoolEntry(tx, feeV[0][5], GetTime(), priV[1][5], blocknum, mpool.HasNoInputsOf(tx))); + mpool.addUnchecked(tx.GetHash(), entry.Fee(feeV[0][5]).Time(GetTime()).Priority(priV[1][5]).Height(blocknum).FromTx(tx, &mpool)); // evict that transaction which should set a mempool min fee of minRelayTxFee + feeV[0][5] mpool.TrimToSize(1); BOOST_CHECK(mpool.GetMinFee(1).GetFeePerK() > feeV[0][5]); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 2fe190f88..351870014 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -144,8 +144,13 @@ TestChain100Setup::~TestChain100Setup() CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(CMutableTransaction &tx, CTxMemPool *pool) { - return CTxMemPoolEntry(tx, nFee, nTime, dPriority, nHeight, - pool ? pool->HasNoInputsOf(tx) : hadNoDependencies); + CTransaction txn(tx); + bool hasNoDependencies = pool ? pool->HasNoInputsOf(tx) : hadNoDependencies; + // Hack to assume either its completely dependent on other mempool txs or not at all + CAmount inChainValue = hasNoDependencies ? txn.GetValueOut() : 0; + + return CTxMemPoolEntry(txn, nFee, nTime, dPriority, nHeight, + hasNoDependencies, inChainValue); } void Shutdown(void* parg) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index ea3aad34a..6d1df0b3d 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -19,10 +19,10 @@ using namespace std; CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, - int64_t _nTime, double _entryPriority, - unsigned int _entryHeight, bool poolHasNoInputsOf): + int64_t _nTime, double _entryPriority, unsigned int _entryHeight, + bool poolHasNoInputsOf, CAmount _inChainInputValue): tx(_tx), nFee(_nFee), nTime(_nTime), entryPriority(_entryPriority), entryHeight(_entryHeight), - hadNoDependencies(poolHasNoInputsOf) + hadNoDependencies(poolHasNoInputsOf), inChainInputValue(_inChainInputValue) { nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); nModSize = tx.CalculateModifiedSize(nTxSize); @@ -31,6 +31,8 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, nCountWithDescendants = 1; nSizeWithDescendants = nTxSize; nFeesWithDescendants = nFee; + CAmount nValueIn = tx.GetValueOut()+nFee; + assert(inChainInputValue <= nValueIn); } CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other) @@ -41,9 +43,10 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other) double CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const { - CAmount nValueIn = tx.GetValueOut()+nFee; - double deltaPriority = ((double)(currentHeight-entryHeight)*nValueIn)/nModSize; + 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; } diff --git a/src/txmempool.h b/src/txmempool.h index e189e2e46..c470bbe28 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -66,6 +66,7 @@ private: double entryPriority; //! Priority when entering the mempool unsigned int entryHeight; //! Chain height when entering the mempool bool hadNoDependencies; //! Not dependent on any other txs when it entered the mempool + CAmount inChainInputValue; //! Sum of all txin values that are already in blockchain // Information about descendants of this transaction that are in the // mempool; if we remove this transaction we must remove all of these @@ -78,10 +79,15 @@ private: public: CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, - int64_t _nTime, double _entryPriority, unsigned int _entryHeight, bool poolHasNoInputsOf); + int64_t _nTime, double _entryPriority, unsigned int _entryHeight, + bool poolHasNoInputsOf, CAmount _inChainInputValue); CTxMemPoolEntry(const CTxMemPoolEntry& other); const CTransaction& GetTx() 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 { return nTxSize; }