diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index d04c036841..5790d51a82 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -18,7 +18,7 @@ static void AddTx(const CTransaction& tx, const CAmount& nFee, CTxMemPool& pool) unsigned int sigOpCost = 4; LockPoints lp; pool.addUnchecked(tx.GetHash(), CTxMemPoolEntry( - MakeTransactionRef(tx), nFee, nTime, dPriority, nHeight, pool.HasNoInputsOf(tx), + MakeTransactionRef(tx), nFee, nTime, dPriority, nHeight, tx.GetValueOut(), spendsCoinbase, sigOpCost, lp)); } diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 52d0e5c5e5..ac2d7edaec 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -327,13 +327,6 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo if (!fCurrentEstimate) return; - if (!entry.WasClearAtEntry()) { - // This transaction depends on other transactions in the mempool to - // be included in a block before it will be able to be included, so - // we shouldn't include it in our calculations - return; - } - // Feerates are stored and reported as BTC-per-kb: CFeeRate feeRate(entry.GetFee(), entry.GetTxSize()); @@ -343,10 +336,8 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry& entry) { - if (!entry.WasClearAtEntry()) { - // This transaction depended on other transactions in the mempool to - // be included in a block before it was able to be included, so - // we shouldn't include it in our calculations + if (!removeTx(entry.GetTx().GetHash())) { + // This transaction wasn't being tracked for fee estimation return; } @@ -378,14 +369,18 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, // transaction fees." return; } - nBestSeenHeight = nBlockHeight; // Only want to be updating estimates when our blockchain is synced, // otherwise we'll miscalculate how many blocks its taking to get included. if (!fCurrentEstimate) return; - // Clear the current block state + // Must update nBestSeenHeight in sync with ClearCurrent so that + // calls to removeTx (via processBlockTx) correctly calculate age + // of unconfirmed txs to remove from tracking. + nBestSeenHeight = nBlockHeight; + + // Clear the current block state and update unconfirmed circular buffer feeStats.ClearCurrent(nBlockHeight); // Repopulate the current block states diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 37f0de354e..91f549fe48 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -120,7 +120,6 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) { CTxMemPool pool(CFeeRate(0)); TestMemPoolEntryHelper entry; - entry.hadNoDependencies = true; /* 3rd highest fee */ CMutableTransaction tx1 = CMutableTransaction(); @@ -323,7 +322,6 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) { CTxMemPool pool(CFeeRate(0)); TestMemPoolEntryHelper entry; - entry.hadNoDependencies = true; /* 3rd highest fee */ CMutableTransaction tx1 = CMutableTransaction(); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 672cd11428..f0eaab2217 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -147,12 +147,11 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction &tx, CT } CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransaction &txn, CTxMemPool *pool) { - bool hasNoDependencies = pool ? pool->HasNoInputsOf(txn) : hadNoDependencies; // Hack to assume either its completely dependent on other mempool txs or not at all - CAmount inChainValue = hasNoDependencies ? txn.GetValueOut() : 0; + CAmount inChainValue = pool && pool->HasNoInputsOf(txn) ? txn.GetValueOut() : 0; return CTxMemPoolEntry(MakeTransactionRef(txn), nFee, nTime, dPriority, nHeight, - hasNoDependencies, inChainValue, spendsCoinbase, sigOpCost, lp); + inChainValue, spendsCoinbase, sigOpCost, lp); } void Shutdown(void* parg) diff --git a/src/test/test_bitcoin.h b/src/test/test_bitcoin.h index 0fe77437e4..5ef6fa764f 100644 --- a/src/test/test_bitcoin.h +++ b/src/test/test_bitcoin.h @@ -70,14 +70,13 @@ struct TestMemPoolEntryHelper int64_t nTime; double dPriority; unsigned int nHeight; - bool hadNoDependencies; bool spendsCoinbase; unsigned int sigOpCost; LockPoints lp; TestMemPoolEntryHelper() : nFee(0), nTime(0), dPriority(0.0), nHeight(1), - hadNoDependencies(false), spendsCoinbase(false), sigOpCost(4) { } + spendsCoinbase(false), sigOpCost(4) { } CTxMemPoolEntry FromTx(const CMutableTransaction &tx, CTxMemPool *pool = NULL); CTxMemPoolEntry FromTx(const CTransaction &tx, CTxMemPool *pool = NULL); @@ -87,7 +86,6 @@ struct TestMemPoolEntryHelper TestMemPoolEntryHelper &Time(int64_t _time) { nTime = _time; return *this; } TestMemPoolEntryHelper &Priority(double _priority) { dPriority = _priority; return *this; } TestMemPoolEntryHelper &Height(unsigned int _height) { nHeight = _height; return *this; } - TestMemPoolEntryHelper &HadNoDependencies(bool _hnd) { hadNoDependencies = _hnd; return *this; } TestMemPoolEntryHelper &SpendsCoinbase(bool _flag) { spendsCoinbase = _flag; return *this; } TestMemPoolEntryHelper &SigOpsCost(unsigned int _sigopsCost) { sigOpCost = _sigopsCost; return *this; } }; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 4ccbcadefc..57300bb51d 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -22,10 +22,10 @@ using namespace std; CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, int64_t _nTime, double _entryPriority, unsigned int _entryHeight, - bool poolHasNoInputsOf, CAmount _inChainInputValue, + CAmount _inChainInputValue, bool _spendsCoinbase, int64_t _sigOpsCost, LockPoints lp): tx(_tx), nFee(_nFee), nTime(_nTime), entryPriority(_entryPriority), entryHeight(_entryHeight), - hadNoDependencies(poolHasNoInputsOf), inChainInputValue(_inChainInputValue), + inChainInputValue(_inChainInputValue), spendsCoinbase(_spendsCoinbase), sigOpCost(_sigOpsCost), lockPoints(lp) { nTxWeight = GetTransactionWeight(*tx); @@ -604,6 +604,8 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne if (i != mapTx.end()) entries.push_back(*i); } + // Before the txs in the new block have been removed from the mempool, update policy estimates + minerPolicyEstimator->processBlock(nBlockHeight, entries, fCurrentEstimate); for (const auto& tx : vtx) { txiter it = mapTx.find(tx->GetHash()); @@ -615,8 +617,6 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne removeConflicts(*tx); ClearPrioritisation(tx->GetHash()); } - // After the txs in the new block have been removed from the mempool, update policy estimates - minerPolicyEstimator->processBlock(nBlockHeight, entries, fCurrentEstimate); lastRollingFeeUpdate = GetTime(); blockSinceLastRollingFeeBump = true; } diff --git a/src/txmempool.h b/src/txmempool.h index 7c68053f26..5c8cf7af1b 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -88,7 +88,6 @@ private: 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 - 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 bool spendsCoinbase; //!< keep track of transactions that spend a coinbase int64_t sigOpCost; //!< Total sigop cost @@ -113,7 +112,7 @@ private: public: CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, int64_t _nTime, double _entryPriority, unsigned int _entryHeight, - bool poolHasNoInputsOf, CAmount _inChainInputValue, bool spendsCoinbase, + CAmount _inChainInputValue, bool spendsCoinbase, int64_t nSigOpsCost, LockPoints lp); CTxMemPoolEntry(const CTxMemPoolEntry& other); @@ -130,7 +129,6 @@ public: size_t GetTxWeight() const { return nTxWeight; } int64_t GetTime() const { return nTime; } unsigned int GetHeight() const { return entryHeight; } - bool WasClearAtEntry() const { return hadNoDependencies; } int64_t GetSigOpCost() const { return sigOpCost; } int64_t GetModifiedFee() const { return nFee + feeDelta; } size_t DynamicMemoryUsage() const { return nUsageSize; } diff --git a/src/validation.cpp b/src/validation.cpp index 2783a313f7..a68763e489 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -693,7 +693,6 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C } CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, dPriority, chainActive.Height(), - !IsInitialBlockDownload() && pool.HasNoInputsOf(tx), inChainInputValue, fSpendsCoinbase, nSigOpsCost, lp); unsigned int nSize = entry.GetTxSize(); @@ -943,7 +942,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C pool.RemoveStaged(allConflicting, false); // Store transaction in memory - pool.addUnchecked(hash, entry, setAncestors, !IsInitialBlockDownload()); + pool.addUnchecked(hash, entry, setAncestors, !IsInitialBlockDownload() && pool.HasNoInputsOf(tx)); // trim mempool and check if tx was trimmed if (!fOverrideMempoolLimit) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9676832560..0b13c2ba74 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2563,7 +2563,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, false, 0, false, 0, lp); + CTxMemPoolEntry entry(wtxNew.tx, 0, 0, 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;