From ae7327b8322d36d00047e92fe699371185de1c68 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 14 Feb 2017 16:54:46 -0500 Subject: [PATCH 1/7] Make feeEstimator its own global instance of CBlockPolicyEstimator --- src/test/policyestimator_tests.cpp | 3 ++- src/txmempool.cpp | 17 +++++------------ src/txmempool.h | 3 +-- src/validation.cpp | 3 ++- src/validation.h | 2 ++ 5 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index bc2f49ef3..93abd1830 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -16,7 +16,8 @@ BOOST_FIXTURE_TEST_SUITE(policyestimator_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) { - CTxMemPool mpool; + CBlockPolicyEstimator feeEst; + CTxMemPool mpool(&feeEst); TestMemPoolEntryHelper entry; CAmount basefee(2000); CAmount deltaFee(100); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 0794a3902..3dc7c11f2 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -333,8 +333,8 @@ void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, assert(int(nSigOpCostWithAncestors) >= 0); } -CTxMemPool::CTxMemPool() : - nTransactionsUpdated(0) +CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator) : + nTransactionsUpdated(0), minerPolicyEstimator(estimator) { _clear(); //lock free clear @@ -342,13 +342,6 @@ CTxMemPool::CTxMemPool() : // accepting transactions becomes O(N^2) where N is the number // of transactions in the pool nCheckFrequency = 0; - - minerPolicyEstimator = new CBlockPolicyEstimator(); -} - -CTxMemPool::~CTxMemPool() -{ - delete minerPolicyEstimator; } void CTxMemPool::pruneSpent(const uint256 &hashTx, CCoins &coins) @@ -427,7 +420,7 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, nTransactionsUpdated++; totalTxSize += entry.GetTxSize(); - minerPolicyEstimator->processTransaction(entry, validFeeEstimate); + if (minerPolicyEstimator) {minerPolicyEstimator->processTransaction(entry, validFeeEstimate);} vTxHashes.emplace_back(tx.GetWitnessHash(), newit); newit->vTxHashesIdx = vTxHashes.size() - 1; @@ -457,7 +450,7 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) mapLinks.erase(it); mapTx.erase(it); nTransactionsUpdated++; - minerPolicyEstimator->removeTx(hash); + if (minerPolicyEstimator) {minerPolicyEstimator->removeTx(hash);} } // Calculates descendants of entry that are not already in setDescendants, and adds to @@ -591,7 +584,7 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne entries.push_back(&*i); } // Before the txs in the new block have been removed from the mempool, update policy estimates - minerPolicyEstimator->processBlock(nBlockHeight, entries); + if (minerPolicyEstimator) {minerPolicyEstimator->processBlock(nBlockHeight, entries);} for (const auto& tx : vtx) { txiter it = mapTx.find(tx->GetHash()); diff --git a/src/txmempool.h b/src/txmempool.h index 422278951..8ec2b0090 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -496,8 +496,7 @@ public: /** Create a new CTxMemPool. */ - CTxMemPool(); - ~CTxMemPool(); + CTxMemPool(CBlockPolicyEstimator* estimator = nullptr); /** * If sanity-checking is turned on, check makes sure the pool is diff --git a/src/validation.cpp b/src/validation.cpp index 35b957a45..95b94d6b0 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -80,7 +80,8 @@ uint256 hashAssumeValid; CFeeRate minRelayTxFee = CFeeRate(DEFAULT_MIN_RELAY_TX_FEE); CAmount maxTxFee = DEFAULT_TRANSACTION_MAXFEE; -CTxMemPool mempool; +CBlockPolicyEstimator feeEstimator; +CTxMemPool mempool(&feeEstimator); static void CheckBlockIndex(const Consensus::Params& consensusParams); diff --git a/src/validation.h b/src/validation.h index 4aa10cbb0..075a836a9 100644 --- a/src/validation.h +++ b/src/validation.h @@ -39,6 +39,7 @@ class CChainParams; class CInv; class CConnman; class CScriptCheck; +class CBlockPolicyEstimator; class CTxMemPool; class CValidationInterface; class CValidationState; @@ -152,6 +153,7 @@ struct BlockHasher extern CScript COINBASE_FLAGS; extern CCriticalSection cs_main; +extern CBlockPolicyEstimator feeEstimator; extern CTxMemPool mempool; typedef boost::unordered_map BlockMap; extern BlockMap mapBlockIndex; From f6187d6e393b5ca587604b678f91496d50149a20 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Wed, 15 Feb 2017 09:16:51 -0500 Subject: [PATCH 2/7] Make processBlockTx private. --- src/policy/fees.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/policy/fees.h b/src/policy/fees.h index dd01c90c4..7abd1d43a 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -209,9 +209,6 @@ public: void processBlock(unsigned int nBlockHeight, std::vector& entries); - /** Process a transaction confirmed in a block*/ - bool processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry); - /** Process a transaction accepted to the mempool*/ void processTransaction(const CTxMemPoolEntry& entry, bool validFeeEstimate); @@ -251,6 +248,10 @@ private: unsigned int trackedTxs; unsigned int untrackedTxs; + + /** Process a transaction confirmed in a block*/ + bool processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry); + }; class FeeFilterRounder From dbb9e3699b8e835fd72a5db2c22927d828484c32 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Wed, 15 Feb 2017 09:24:11 -0500 Subject: [PATCH 3/7] Give CBlockPolicyEstimator it's own lock --- src/policy/fees.cpp | 32 ++++++++++++++++++++++---------- src/policy/fees.h | 3 +++ src/txmempool.cpp | 4 ---- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 38e07dc34..05f2a20e9 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -290,6 +290,7 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe // of no harm to try to remove them again. bool CBlockPolicyEstimator::removeTx(uint256 hash) { + LOCK(cs_feeEstimator); std::map::iterator pos = mapMemPoolTxs.find(hash); if (pos != mapMemPoolTxs.end()) { feeStats.removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex); @@ -315,6 +316,7 @@ CBlockPolicyEstimator::CBlockPolicyEstimator() void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, bool validFeeEstimate) { + LOCK(cs_feeEstimator); unsigned int txHeight = entry.GetHeight(); uint256 hash = entry.GetTx().GetHash(); if (mapMemPoolTxs.count(hash)) { @@ -374,6 +376,7 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, std::vector& entries) { + LOCK(cs_feeEstimator); if (nBlockHeight <= nBestSeenHeight) { // Ignore side chains and re-orgs; assuming they are random // they don't affect the estimate. @@ -410,6 +413,7 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget) { + LOCK(cs_feeEstimator); // Return failure if trying to analyze a target we're not tracking // It's not possible to get reasonable estimates for confTarget of 1 if (confTarget <= 1 || (unsigned int)confTarget > feeStats.GetMaxConfirms()) @@ -427,18 +431,24 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun { if (answerFoundAtTarget) *answerFoundAtTarget = confTarget; - // Return failure if trying to analyze a target we're not tracking - if (confTarget <= 0 || (unsigned int)confTarget > feeStats.GetMaxConfirms()) - return CFeeRate(0); - - // It's not possible to get reasonable estimates for confTarget of 1 - if (confTarget == 1) - confTarget = 2; double median = -1; - while (median < 0 && (unsigned int)confTarget <= feeStats.GetMaxConfirms()) { - median = feeStats.EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); - } + + { + LOCK(cs_feeEstimator); + + // Return failure if trying to analyze a target we're not tracking + if (confTarget <= 0 || (unsigned int)confTarget > feeStats.GetMaxConfirms()) + return CFeeRate(0); + + // It's not possible to get reasonable estimates for confTarget of 1 + if (confTarget == 1) + confTarget = 2; + + while (median < 0 && (unsigned int)confTarget <= feeStats.GetMaxConfirms()) { + median = feeStats.EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); + } + } // Must unlock cs_feeEstimator before taking mempool locks if (answerFoundAtTarget) *answerFoundAtTarget = confTarget - 1; @@ -456,12 +466,14 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun void CBlockPolicyEstimator::Write(CAutoFile& fileout) { + LOCK(cs_feeEstimator); fileout << nBestSeenHeight; feeStats.Write(fileout); } void CBlockPolicyEstimator::Read(CAutoFile& filein, int nFileVersion) { + LOCK(cs_feeEstimator); int nFileBestSeenHeight; filein >> nFileBestSeenHeight; feeStats.Read(filein); diff --git a/src/policy/fees.h b/src/policy/fees.h index 7abd1d43a..e11a6582a 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -8,6 +8,7 @@ #include "amount.h" #include "uint256.h" #include "random.h" +#include "sync.h" #include #include @@ -249,6 +250,8 @@ private: unsigned int trackedTxs; unsigned int untrackedTxs; + mutable CCriticalSection cs_feeEstimator; + /** Process a transaction confirmed in a block*/ bool processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 3dc7c11f2..7fac48d3c 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -845,12 +845,10 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const CFeeRate CTxMemPool::estimateFee(int nBlocks) const { - LOCK(cs); return minerPolicyEstimator->estimateFee(nBlocks); } CFeeRate CTxMemPool::estimateSmartFee(int nBlocks, int *answerFoundAtBlocks) const { - LOCK(cs); return minerPolicyEstimator->estimateSmartFee(nBlocks, answerFoundAtBlocks, *this); } @@ -858,7 +856,6 @@ bool CTxMemPool::WriteFeeEstimates(CAutoFile& fileout) const { try { - LOCK(cs); fileout << 139900; // version required to read: 0.13.99 or later fileout << CLIENT_VERSION; // version that wrote the file minerPolicyEstimator->Write(fileout); @@ -878,7 +875,6 @@ CTxMemPool::ReadFeeEstimates(CAutoFile& filein) filein >> nVersionRequired >> nVersionThatWrote; if (nVersionRequired > CLIENT_VERSION) return error("CTxMemPool::ReadFeeEstimates(): up-version (%d) fee estimate file", nVersionRequired); - LOCK(cs); minerPolicyEstimator->Read(filein, nVersionThatWrote); } catch (const std::exception&) { From 14e10aa842b8583f9648accd5d151dbdf342b9dc Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Wed, 15 Feb 2017 15:23:34 -0500 Subject: [PATCH 4/7] Call estimate(Smart)Fee directly from CBlockPolicyEstimator --- src/policy/fees.cpp | 6 ++--- src/policy/fees.h | 8 +++--- src/qt/coincontroldialog.cpp | 5 ++-- src/qt/sendcoinsdialog.cpp | 3 ++- src/rpc/mining.cpp | 5 ++-- src/test/policyestimator_tests.cpp | 42 +++++++++++++++--------------- src/txmempool.cpp | 9 ------- src/txmempool.h | 9 ------- src/wallet/feebumper.cpp | 5 ++-- src/wallet/rpcwallet.cpp | 1 + src/wallet/wallet.cpp | 11 ++++---- src/wallet/wallet.h | 5 ++-- 12 files changed, 49 insertions(+), 60 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 05f2a20e9..66da753a8 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -77,7 +77,7 @@ void TxConfirmStats::UpdateMovingAverages() // returns -1 on error conditions double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, double successBreakPoint, bool requireGreater, - unsigned int nBlockHeight) + unsigned int nBlockHeight) const { // Counters for a bucket (or range of buckets) double nConf = 0; // Number of tx's confirmed within the confTarget @@ -411,7 +411,7 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, untrackedTxs = 0; } -CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget) +CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget) const { LOCK(cs_feeEstimator); // Return failure if trying to analyze a target we're not tracking @@ -427,7 +427,7 @@ CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget) return CFeeRate(median); } -CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool) +CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool) const { if (answerFoundAtTarget) *answerFoundAtTarget = confTarget; diff --git a/src/policy/fees.h b/src/policy/fees.h index e11a6582a..2299144b9 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -150,10 +150,10 @@ public: * @param nBlockHeight the current block height */ double EstimateMedianVal(int confTarget, double sufficientTxVal, - double minSuccess, bool requireGreater, unsigned int nBlockHeight); + double minSuccess, bool requireGreater, unsigned int nBlockHeight) const; /** Return the max number of confirms we're tracking */ - unsigned int GetMaxConfirms() { return confAvg.size(); } + unsigned int GetMaxConfirms() const { return confAvg.size(); } /** Write state of estimation data to a file*/ void Write(CAutoFile& fileout); @@ -217,13 +217,13 @@ public: bool removeTx(uint256 hash); /** Return a feerate estimate */ - CFeeRate estimateFee(int confTarget); + CFeeRate estimateFee(int confTarget) const; /** Estimate feerate needed to get be included in a block within * confTarget blocks. If no answer can be given at confTarget, return an * estimate at the lowest target where one can be given. */ - CFeeRate estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool); + CFeeRate estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool) const; /** Write estimation data to a file */ void Write(CAutoFile& fileout); diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 1d19c6575..38ad6e9aa 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -15,6 +15,7 @@ #include "wallet/coincontrol.h" #include "init.h" +#include "policy/fees.h" #include "policy/policy.h" #include "validation.h" // For mempool #include "wallet/wallet.h" @@ -512,7 +513,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) nBytes -= 34; // Fee - nPayFee = CWallet::GetMinimumFee(nBytes, nTxConfirmTarget, mempool); + nPayFee = CWallet::GetMinimumFee(nBytes, nTxConfirmTarget, ::mempool, ::feeEstimator); if (nPayFee > 0 && coinControl->nMinimumTotalFee > nPayFee) nPayFee = coinControl->nMinimumTotalFee; @@ -592,7 +593,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) if (payTxFee.GetFeePerK() > 0) dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), payTxFee.GetFeePerK()) / 1000; else { - dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), mempool.estimateSmartFee(nTxConfirmTarget).GetFeePerK()) / 1000; + dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), ::feeEstimator.estimateSmartFee(nTxConfirmTarget, NULL, ::mempool).GetFeePerK()) / 1000; } QString toolTip4 = tr("Can vary +/- %1 satoshi(s) per input.").arg(dFeeVary); diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index ed7eab03f..5d58a6a11 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -21,6 +21,7 @@ #include "validation.h" // mempool and minRelayTxFee #include "ui_interface.h" #include "txmempool.h" +#include "policy/fees.h" #include "wallet/wallet.h" #include @@ -660,7 +661,7 @@ void SendCoinsDialog::updateSmartFeeLabel() int nBlocksToConfirm = ui->sliderSmartFee->maximum() - ui->sliderSmartFee->value() + 2; int estimateFoundAtBlocks = nBlocksToConfirm; - CFeeRate feeRate = mempool.estimateSmartFee(nBlocksToConfirm, &estimateFoundAtBlocks); + CFeeRate feeRate = ::feeEstimator.estimateSmartFee(nBlocksToConfirm, &estimateFoundAtBlocks, ::mempool); if (feeRate <= CFeeRate(0)) // not enough data => minfee { ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 7e5f0d608..4aad267b8 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -15,6 +15,7 @@ #include "validation.h" #include "miner.h" #include "net.h" +#include "policy/fees.h" #include "pow.h" #include "rpc/blockchain.h" #include "rpc/server.h" @@ -819,7 +820,7 @@ UniValue estimatefee(const JSONRPCRequest& request) if (nBlocks < 1) nBlocks = 1; - CFeeRate feeRate = mempool.estimateFee(nBlocks); + CFeeRate feeRate = ::feeEstimator.estimateFee(nBlocks); if (feeRate == CFeeRate(0)) return -1.0; @@ -857,7 +858,7 @@ UniValue estimatesmartfee(const JSONRPCRequest& request) UniValue result(UniValue::VOBJ); int answerFound; - CFeeRate feeRate = mempool.estimateSmartFee(nBlocks, &answerFound); + CFeeRate feeRate = ::feeEstimator.estimateSmartFee(nBlocks, &answerFound, ::mempool); result.push_back(Pair("feerate", feeRate == CFeeRate(0) ? -1.0 : ValueFromAmount(feeRate.GetFeePerK()))); result.push_back(Pair("blocks", answerFound)); return result; diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 93abd1830..ed6782ea3 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -79,16 +79,16 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) // At this point we should need to combine 5 buckets to get enough data points // So estimateFee(1,2,3) should fail and estimateFee(4) should return somewhere around // 8*baserate. estimateFee(4) %'s are 100,100,100,100,90 = average 98% - BOOST_CHECK(mpool.estimateFee(1) == CFeeRate(0)); - BOOST_CHECK(mpool.estimateFee(2) == CFeeRate(0)); - BOOST_CHECK(mpool.estimateFee(3) == CFeeRate(0)); - BOOST_CHECK(mpool.estimateFee(4).GetFeePerK() < 8*baseRate.GetFeePerK() + deltaFee); - BOOST_CHECK(mpool.estimateFee(4).GetFeePerK() > 8*baseRate.GetFeePerK() - deltaFee); + BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0)); + BOOST_CHECK(feeEst.estimateFee(2) == CFeeRate(0)); + BOOST_CHECK(feeEst.estimateFee(3) == CFeeRate(0)); + BOOST_CHECK(feeEst.estimateFee(4).GetFeePerK() < 8*baseRate.GetFeePerK() + deltaFee); + BOOST_CHECK(feeEst.estimateFee(4).GetFeePerK() > 8*baseRate.GetFeePerK() - deltaFee); int answerFound; - BOOST_CHECK(mpool.estimateSmartFee(1, &answerFound) == mpool.estimateFee(4) && answerFound == 4); - BOOST_CHECK(mpool.estimateSmartFee(3, &answerFound) == mpool.estimateFee(4) && answerFound == 4); - BOOST_CHECK(mpool.estimateSmartFee(4, &answerFound) == mpool.estimateFee(4) && answerFound == 4); - BOOST_CHECK(mpool.estimateSmartFee(8, &answerFound) == mpool.estimateFee(8) && answerFound == 8); + BOOST_CHECK(feeEst.estimateSmartFee(1, &answerFound, mpool) == feeEst.estimateFee(4) && answerFound == 4); + BOOST_CHECK(feeEst.estimateSmartFee(3, &answerFound, mpool) == feeEst.estimateFee(4) && answerFound == 4); + BOOST_CHECK(feeEst.estimateSmartFee(4, &answerFound, mpool) == feeEst.estimateFee(4) && answerFound == 4); + BOOST_CHECK(feeEst.estimateSmartFee(8, &answerFound, mpool) == feeEst.estimateFee(8) && answerFound == 8); } } @@ -100,7 +100,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) // Second highest feerate has 100% chance of being included by 2 blocks, // so estimateFee(2) should return 9*baseRate etc... for (int i = 1; i < 10;i++) { - origFeeEst.push_back(mpool.estimateFee(i).GetFeePerK()); + origFeeEst.push_back(feeEst.estimateFee(i).GetFeePerK()); if (i > 2) { // Fee estimates should be monotonically decreasing BOOST_CHECK(origFeeEst[i-1] <= origFeeEst[i-2]); } @@ -119,10 +119,10 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) while (blocknum < 250) mpool.removeForBlock(block, ++blocknum); - BOOST_CHECK(mpool.estimateFee(1) == CFeeRate(0)); + BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0)); for (int i = 2; i < 10;i++) { - BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() < origFeeEst[i-1] + deltaFee); - BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee); + BOOST_CHECK(feeEst.estimateFee(i).GetFeePerK() < origFeeEst[i-1] + deltaFee); + BOOST_CHECK(feeEst.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee); } @@ -142,8 +142,8 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) int answerFound; for (int i = 1; i < 10;i++) { - BOOST_CHECK(mpool.estimateFee(i) == CFeeRate(0) || mpool.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee); - BOOST_CHECK(mpool.estimateSmartFee(i, &answerFound).GetFeePerK() > origFeeEst[answerFound-1] - deltaFee); + BOOST_CHECK(feeEst.estimateFee(i) == CFeeRate(0) || feeEst.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee); + BOOST_CHECK(feeEst.estimateSmartFee(i, &answerFound, mpool).GetFeePerK() > origFeeEst[answerFound-1] - deltaFee); } // Mine all those transactions @@ -158,9 +158,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) } mpool.removeForBlock(block, 265); block.clear(); - BOOST_CHECK(mpool.estimateFee(1) == CFeeRate(0)); + BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0)); for (int i = 2; i < 10;i++) { - BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee); + BOOST_CHECK(feeEst.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee); } // Mine 200 more blocks where everything is mined every block @@ -180,9 +180,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) mpool.removeForBlock(block, ++blocknum); block.clear(); } - BOOST_CHECK(mpool.estimateFee(1) == CFeeRate(0)); + BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0)); for (int i = 2; i < 10; i++) { - BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() < origFeeEst[i-1] - deltaFee); + BOOST_CHECK(feeEst.estimateFee(i).GetFeePerK() < origFeeEst[i-1] - deltaFee); } // Test that if the mempool is limited, estimateSmartFee won't return a value below the mempool min fee @@ -191,8 +191,8 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) mpool.TrimToSize(1); BOOST_CHECK(mpool.GetMinFee(1).GetFeePerK() > feeV[5]); for (int i = 1; i < 10; i++) { - BOOST_CHECK(mpool.estimateSmartFee(i).GetFeePerK() >= mpool.estimateFee(i).GetFeePerK()); - BOOST_CHECK(mpool.estimateSmartFee(i).GetFeePerK() >= mpool.GetMinFee(1).GetFeePerK()); + BOOST_CHECK(feeEst.estimateSmartFee(i, NULL, mpool).GetFeePerK() >= feeEst.estimateFee(i).GetFeePerK()); + BOOST_CHECK(feeEst.estimateSmartFee(i, NULL, mpool).GetFeePerK() >= mpool.GetMinFee(1).GetFeePerK()); } } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 7fac48d3c..fa0584149 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -843,15 +843,6 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const return GetInfo(i); } -CFeeRate CTxMemPool::estimateFee(int nBlocks) const -{ - return minerPolicyEstimator->estimateFee(nBlocks); -} -CFeeRate CTxMemPool::estimateSmartFee(int nBlocks, int *answerFoundAtBlocks) const -{ - return minerPolicyEstimator->estimateSmartFee(nBlocks, answerFoundAtBlocks, *this); -} - bool CTxMemPool::WriteFeeEstimates(CAutoFile& fileout) const { diff --git a/src/txmempool.h b/src/txmempool.h index 8ec2b0090..3aa5b46c4 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -617,15 +617,6 @@ public: TxMempoolInfo info(const uint256& hash) const; std::vector infoAll() const; - /** Estimate fee rate needed to get into the next nBlocks - * If no answer can be given at nBlocks, return an estimate - * at the lowest number of blocks where one can be given - */ - CFeeRate estimateSmartFee(int nBlocks, int *answerFoundAtBlocks = NULL) const; - - /** Estimate fee rate needed to get into the next nBlocks */ - CFeeRate estimateFee(int nBlocks) const; - /** Write/Read estimates to disk */ bool WriteFeeEstimates(CAutoFile& fileout) const; bool ReadFeeEstimates(CAutoFile& filein); diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index fe3871a91..67792dad7 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -5,6 +5,7 @@ #include "consensus/validation.h" #include "wallet/feebumper.h" #include "wallet/wallet.h" +#include "policy/fees.h" #include "policy/policy.h" #include "policy/rbf.h" #include "validation.h" //for mempool access @@ -159,11 +160,11 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConf } else { // if user specified a confirm target then don't consider any global payTxFee if (specifiedConfirmTarget) { - nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool, CAmount(0)); + nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool, ::feeEstimator, CAmount(0)); } // otherwise use the regular wallet logic to select payTxFee or default confirm target else { - nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool); + nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool, ::feeEstimator); } nNewFeeRate = CFeeRate(nNewFee, maxNewTxSize); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 2cc3072c1..43b71712c 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -11,6 +11,7 @@ #include "init.h" #include "validation.h" #include "net.h" +#include "policy/fees.h" #include "policy/policy.h" #include "policy/rbf.h" #include "rpc/server.h" diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 99fcb21f6..9defcb54c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -16,6 +16,7 @@ #include "keystore.h" #include "validation.h" #include "net.h" +#include "policy/fees.h" #include "policy/policy.h" #include "policy/rbf.h" #include "primitives/block.h" @@ -2575,7 +2576,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT if (coinControl && coinControl->nConfirmTarget > 0) currentConfirmationTarget = coinControl->nConfirmTarget; - CAmount nFeeNeeded = GetMinimumFee(nBytes, currentConfirmationTarget, mempool); + CAmount nFeeNeeded = GetMinimumFee(nBytes, currentConfirmationTarget, ::mempool, ::feeEstimator); if (coinControl && nFeeNeeded > 0 && coinControl->nMinimumTotalFee > nFeeNeeded) { nFeeNeeded = coinControl->nMinimumTotalFee; } @@ -2749,19 +2750,19 @@ CAmount CWallet::GetRequiredFee(unsigned int nTxBytes) return std::max(minTxFee.GetFee(nTxBytes), ::minRelayTxFee.GetFee(nTxBytes)); } -CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool) +CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator) { // payTxFee is the user-set global for desired feerate - return GetMinimumFee(nTxBytes, nConfirmTarget, pool, payTxFee.GetFee(nTxBytes)); + return GetMinimumFee(nTxBytes, nConfirmTarget, pool, estimator, payTxFee.GetFee(nTxBytes)); } -CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, CAmount targetFee) +CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, CAmount targetFee) { CAmount nFeeNeeded = targetFee; // User didn't set: use -txconfirmtarget to estimate... if (nFeeNeeded == 0) { int estimateFoundTarget = nConfirmTarget; - nFeeNeeded = pool.estimateSmartFee(nConfirmTarget, &estimateFoundTarget).GetFee(nTxBytes); + nFeeNeeded = estimator.estimateSmartFee(nConfirmTarget, &estimateFoundTarget, pool).GetFee(nTxBytes); // ... unless we don't have enough mempool data for estimatefee, then use fallbackFee if (nFeeNeeded == 0) nFeeNeeded = fallbackFee.GetFee(nTxBytes); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index c714ddd09..c8869a4c0 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -75,6 +75,7 @@ class CReserveKey; class CScript; class CScheduler; class CTxMemPool; +class CBlockPolicyEstimator; class CWalletTx; /** (client) version numbers for particular wallet features */ @@ -890,12 +891,12 @@ public: * Estimate the minimum fee considering user set parameters * and the required fee */ - static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool); + static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator); /** * Estimate the minimum fee considering required fee and targetFee or if 0 * then fee estimation for nConfirmTarget */ - static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, CAmount targetFee); + static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, CAmount targetFee); /** * Return the minimum required fee taking into account the * floating relay fee and user set minimum transaction fee From 5ba81e54e0390ec0be7dbc8ebea0c35933442a8a Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Wed, 15 Feb 2017 15:48:48 -0500 Subject: [PATCH 5/7] Read and Write fee estimate file directly from CBlockPolicyEstimator --- src/init.cpp | 5 +++-- src/policy/fees.cpp | 44 ++++++++++++++++++++++++++++++++------------ src/policy/fees.h | 6 +++--- src/txmempool.cpp | 34 ---------------------------------- src/txmempool.h | 4 ---- 5 files changed, 38 insertions(+), 55 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 1e7e388a5..f06c9e110 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -25,6 +25,7 @@ #include "netbase.h" #include "net.h" #include "net_processing.h" +#include "policy/fees.h" #include "policy/policy.h" #include "rpc/server.h" #include "rpc/register.h" @@ -215,7 +216,7 @@ void Shutdown() fs::path est_path = GetDataDir() / FEE_ESTIMATES_FILENAME; CAutoFile est_fileout(fsbridge::fopen(est_path, "wb"), SER_DISK, CLIENT_VERSION); if (!est_fileout.IsNull()) - mempool.WriteFeeEstimates(est_fileout); + ::feeEstimator.Write(est_fileout); else LogPrintf("%s: Failed to write fee estimates to %s\n", __func__, est_path.string()); fFeeEstimatesInitialized = false; @@ -1550,7 +1551,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) CAutoFile est_filein(fsbridge::fopen(est_path, "rb"), SER_DISK, CLIENT_VERSION); // Allowed to fail as this file IS missing on first startup. if (!est_filein.IsNull()) - mempool.ReadFeeEstimates(est_filein); + ::feeEstimator.Read(est_filein); fFeeEstimatesInitialized = true; // ********************************************************* Step 8: load wallet diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 66da753a8..eed71089e 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -7,6 +7,7 @@ #include "policy/policy.h" #include "amount.h" +#include "clientversion.h" #include "primitives/transaction.h" #include "random.h" #include "streams.h" @@ -173,7 +174,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, return median; } -void TxConfirmStats::Write(CAutoFile& fileout) +void TxConfirmStats::Write(CAutoFile& fileout) const { fileout << decay; fileout << buckets; @@ -464,21 +465,40 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun return CFeeRate(median); } -void CBlockPolicyEstimator::Write(CAutoFile& fileout) +bool CBlockPolicyEstimator::Write(CAutoFile& fileout) const { - LOCK(cs_feeEstimator); - fileout << nBestSeenHeight; - feeStats.Write(fileout); + try { + LOCK(cs_feeEstimator); + fileout << 139900; // version required to read: 0.13.99 or later + fileout << CLIENT_VERSION; // version that wrote the file + fileout << nBestSeenHeight; + feeStats.Write(fileout); + } + catch (const std::exception&) { + LogPrintf("CBlockPolicyEstimator::Write(): unable to read policy estimator data (non-fatal)\n"); + return false; + } + return true; } -void CBlockPolicyEstimator::Read(CAutoFile& filein, int nFileVersion) +bool CBlockPolicyEstimator::Read(CAutoFile& filein) { - LOCK(cs_feeEstimator); - int nFileBestSeenHeight; - filein >> nFileBestSeenHeight; - feeStats.Read(filein); - nBestSeenHeight = nFileBestSeenHeight; - // if nVersionThatWrote < 139900 then another TxConfirmStats (for priority) follows but can be ignored. + try { + LOCK(cs_feeEstimator); + int nVersionRequired, nVersionThatWrote, nFileBestSeenHeight; + filein >> nVersionRequired >> nVersionThatWrote; + if (nVersionRequired > CLIENT_VERSION) + return error("CBlockPolicyEstimator::Read(): up-version (%d) fee estimate file", nVersionRequired); + filein >> nFileBestSeenHeight; + feeStats.Read(filein); + nBestSeenHeight = nFileBestSeenHeight; + // if nVersionThatWrote < 139900 then another TxConfirmStats (for priority) follows but can be ignored. + } + catch (const std::exception&) { + LogPrintf("CBlockPolicyEstimator::Read(): unable to read policy estimator data (non-fatal)\n"); + return false; + } + return true; } FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee) diff --git a/src/policy/fees.h b/src/policy/fees.h index 2299144b9..d11de81ee 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -156,7 +156,7 @@ public: unsigned int GetMaxConfirms() const { return confAvg.size(); } /** Write state of estimation data to a file*/ - void Write(CAutoFile& fileout); + void Write(CAutoFile& fileout) const; /** * Read saved state of estimation data from a file and replace all internal data structures and @@ -226,10 +226,10 @@ public: CFeeRate estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool) const; /** Write estimation data to a file */ - void Write(CAutoFile& fileout); + bool Write(CAutoFile& fileout) const; /** Read estimation data from a file */ - void Read(CAutoFile& filein, int nFileVersion); + bool Read(CAutoFile& filein); private: CFeeRate minTrackedFee; //!< Passed to constructor to avoid dependency on main diff --git a/src/txmempool.cpp b/src/txmempool.cpp index fa0584149..ac842da6b 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -5,7 +5,6 @@ #include "txmempool.h" -#include "clientversion.h" #include "consensus/consensus.h" #include "consensus/validation.h" #include "validation.h" @@ -16,7 +15,6 @@ #include "util.h" #include "utilmoneystr.h" #include "utiltime.h" -#include "version.h" CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, int64_t _nTime, unsigned int _entryHeight, @@ -843,38 +841,6 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const return GetInfo(i); } -bool -CTxMemPool::WriteFeeEstimates(CAutoFile& fileout) const -{ - try { - fileout << 139900; // version required to read: 0.13.99 or later - fileout << CLIENT_VERSION; // version that wrote the file - minerPolicyEstimator->Write(fileout); - } - catch (const std::exception&) { - LogPrintf("CTxMemPool::WriteFeeEstimates(): unable to write policy estimator data (non-fatal)\n"); - return false; - } - return true; -} - -bool -CTxMemPool::ReadFeeEstimates(CAutoFile& filein) -{ - try { - int nVersionRequired, nVersionThatWrote; - filein >> nVersionRequired >> nVersionThatWrote; - if (nVersionRequired > CLIENT_VERSION) - return error("CTxMemPool::ReadFeeEstimates(): up-version (%d) fee estimate file", nVersionRequired); - minerPolicyEstimator->Read(filein, nVersionThatWrote); - } - catch (const std::exception&) { - LogPrintf("CTxMemPool::ReadFeeEstimates(): unable to read policy estimator data (non-fatal)\n"); - return false; - } - return true; -} - void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta) { { diff --git a/src/txmempool.h b/src/txmempool.h index 3aa5b46c4..92c4d9f9d 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -617,10 +617,6 @@ public: TxMempoolInfo info(const uint256& hash) const; std::vector infoAll() const; - /** Write/Read estimates to disk */ - bool WriteFeeEstimates(CAutoFile& fileout) const; - bool ReadFeeEstimates(CAutoFile& filein); - size_t DynamicMemoryUsage() const; boost::signals2::signal NotifyEntryAdded; From 2332f19bef025c22fab5a96a0cd2d52d22489aa2 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 16 Feb 2017 16:23:15 -0500 Subject: [PATCH 6/7] Initialize TxConfirmStats in constructor and change to storing as a pointer. --- src/policy/fees.cpp | 35 ++++++++++++++++++++--------------- src/policy/fees.h | 7 ++++--- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index eed71089e..f926c0961 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -14,8 +14,8 @@ #include "txmempool.h" #include "util.h" -void TxConfirmStats::Initialize(std::vector& defaultBuckets, - unsigned int maxConfirms, double _decay) +TxConfirmStats::TxConfirmStats(const std::vector& defaultBuckets, + unsigned int maxConfirms, double _decay) { decay = _decay; for (unsigned int i = 0; i < defaultBuckets.size(); i++) { @@ -294,7 +294,7 @@ bool CBlockPolicyEstimator::removeTx(uint256 hash) LOCK(cs_feeEstimator); std::map::iterator pos = mapMemPoolTxs.find(hash); if (pos != mapMemPoolTxs.end()) { - feeStats.removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex); + feeStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex); mapMemPoolTxs.erase(hash); return true; } else { @@ -312,7 +312,12 @@ CBlockPolicyEstimator::CBlockPolicyEstimator() vfeelist.push_back(bucketBoundary); } vfeelist.push_back(INF_FEERATE); - feeStats.Initialize(vfeelist, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); + feeStats = new TxConfirmStats(vfeelist, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); +} + +CBlockPolicyEstimator::~CBlockPolicyEstimator() +{ + delete feeStats; } void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, bool validFeeEstimate) @@ -346,7 +351,7 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo CFeeRate feeRate(entry.GetFee(), entry.GetTxSize()); mapMemPoolTxs[hash].blockHeight = txHeight; - mapMemPoolTxs[hash].bucketIndex = feeStats.NewTx(txHeight, (double)feeRate.GetFeePerK()); + mapMemPoolTxs[hash].bucketIndex = feeStats->NewTx(txHeight, (double)feeRate.GetFeePerK()); } bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry) @@ -370,7 +375,7 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM // Feerates are stored and reported as BTC-per-kb: CFeeRate feeRate(entry->GetFee(), entry->GetTxSize()); - feeStats.Record(blocksToConfirm, (double)feeRate.GetFeePerK()); + feeStats->Record(blocksToConfirm, (double)feeRate.GetFeePerK()); return true; } @@ -393,7 +398,7 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, nBestSeenHeight = nBlockHeight; // Clear the current block state and update unconfirmed circular buffer - feeStats.ClearCurrent(nBlockHeight); + feeStats->ClearCurrent(nBlockHeight); unsigned int countedTxs = 0; // Repopulate the current block states @@ -403,7 +408,7 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, } // Update all exponential averages with the current block state - feeStats.UpdateMovingAverages(); + feeStats->UpdateMovingAverages(); LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy after updating estimates for %u of %u txs in block, since last block %u of %u tracked, new mempool map size %u\n", countedTxs, entries.size(), trackedTxs, trackedTxs + untrackedTxs, mapMemPoolTxs.size()); @@ -417,10 +422,10 @@ CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget) const LOCK(cs_feeEstimator); // Return failure if trying to analyze a target we're not tracking // It's not possible to get reasonable estimates for confTarget of 1 - if (confTarget <= 1 || (unsigned int)confTarget > feeStats.GetMaxConfirms()) + if (confTarget <= 1 || (unsigned int)confTarget > feeStats->GetMaxConfirms()) return CFeeRate(0); - double median = feeStats.EstimateMedianVal(confTarget, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); + double median = feeStats->EstimateMedianVal(confTarget, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); if (median < 0) return CFeeRate(0); @@ -439,15 +444,15 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun LOCK(cs_feeEstimator); // Return failure if trying to analyze a target we're not tracking - if (confTarget <= 0 || (unsigned int)confTarget > feeStats.GetMaxConfirms()) + if (confTarget <= 0 || (unsigned int)confTarget > feeStats->GetMaxConfirms()) return CFeeRate(0); // It's not possible to get reasonable estimates for confTarget of 1 if (confTarget == 1) confTarget = 2; - while (median < 0 && (unsigned int)confTarget <= feeStats.GetMaxConfirms()) { - median = feeStats.EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); + while (median < 0 && (unsigned int)confTarget <= feeStats->GetMaxConfirms()) { + median = feeStats->EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); } } // Must unlock cs_feeEstimator before taking mempool locks @@ -472,7 +477,7 @@ bool CBlockPolicyEstimator::Write(CAutoFile& fileout) const fileout << 139900; // version required to read: 0.13.99 or later fileout << CLIENT_VERSION; // version that wrote the file fileout << nBestSeenHeight; - feeStats.Write(fileout); + feeStats->Write(fileout); } catch (const std::exception&) { LogPrintf("CBlockPolicyEstimator::Write(): unable to read policy estimator data (non-fatal)\n"); @@ -490,7 +495,7 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) if (nVersionRequired > CLIENT_VERSION) return error("CBlockPolicyEstimator::Read(): up-version (%d) fee estimate file", nVersionRequired); filein >> nFileBestSeenHeight; - feeStats.Read(filein); + feeStats->Read(filein); nBestSeenHeight = nFileBestSeenHeight; // if nVersionThatWrote < 139900 then another TxConfirmStats (for priority) follows but can be ignored. } diff --git a/src/policy/fees.h b/src/policy/fees.h index d11de81ee..1a9eb0051 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -108,13 +108,13 @@ private: public: /** - * Initialize the data structures. This is called by BlockPolicyEstimator's + * Create new TxConfirmStats. This is called by BlockPolicyEstimator's * constructor with default values. * @param defaultBuckets contains the upper limits for the bucket boundaries * @param maxConfirms max number of confirms to track * @param decay how much to decay the historical moving average per block */ - void Initialize(std::vector& defaultBuckets, unsigned int maxConfirms, double decay); + TxConfirmStats(const std::vector& defaultBuckets, unsigned int maxConfirms, double decay); /** Clear the state of the curBlock variables to start counting for the new block */ void ClearCurrent(unsigned int nBlockHeight); @@ -205,6 +205,7 @@ class CBlockPolicyEstimator public: /** Create new BlockPolicyEstimator and initialize stats tracking classes with default values */ CBlockPolicyEstimator(); + ~CBlockPolicyEstimator(); /** Process all the transactions that have been included in a block */ void processBlock(unsigned int nBlockHeight, @@ -245,7 +246,7 @@ private: std::map mapMemPoolTxs; /** Classes to track historical data on transaction confirmations */ - TxConfirmStats feeStats; + TxConfirmStats* feeStats; unsigned int trackedTxs; unsigned int untrackedTxs; From 68af6514987d9d7bfcd67caa9394edda6ab5ef2c Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 16 Feb 2017 17:27:20 -0500 Subject: [PATCH 7/7] MOVEONLY: move TxConfirmStats to cpp --- src/policy/fees.cpp | 106 +++++++++++++++++++++++++++++++++++++++++++ src/policy/fees.h | 108 +------------------------------------------- 2 files changed, 107 insertions(+), 107 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index f926c0961..f3f7f8378 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -14,6 +14,112 @@ #include "txmempool.h" #include "util.h" +/** + * We will instantiate an instance of this class to track transactions that were + * included in a block. We will lump transactions into a bucket according to their + * approximate feerate and then track how long it took for those txs to be included in a block + * + * The tracking of unconfirmed (mempool) transactions is completely independent of the + * historical tracking of transactions that have been confirmed in a block. + */ +class TxConfirmStats +{ +private: + //Define the buckets we will group transactions into + std::vector buckets; // The upper-bound of the range for the bucket (inclusive) + std::map bucketMap; // Map of bucket upper-bound to index into all vectors by bucket + + // For each bucket X: + // Count the total # of txs in each bucket + // Track the historical moving average of this total over blocks + std::vector txCtAvg; + // and calculate the total for the current block to update the moving average + std::vector curBlockTxCt; + + // Count the total # of txs confirmed within Y blocks in each bucket + // Track the historical moving average of theses totals over blocks + std::vector > confAvg; // confAvg[Y][X] + // and calculate the totals for the current block to update the moving averages + std::vector > curBlockConf; // curBlockConf[Y][X] + + // Sum the total feerate of all tx's in each bucket + // Track the historical moving average of this total over blocks + std::vector avg; + // and calculate the total for the current block to update the moving average + std::vector curBlockVal; + + // Combine the conf counts with tx counts to calculate the confirmation % for each Y,X + // Combine the total value with the tx counts to calculate the avg feerate per bucket + + double decay; + + // Mempool counts of outstanding transactions + // For each bucket X, track the number of transactions in the mempool + // that are unconfirmed for each possible confirmation value Y + std::vector > unconfTxs; //unconfTxs[Y][X] + // transactions still unconfirmed after MAX_CONFIRMS for each bucket + std::vector oldUnconfTxs; + +public: + /** + * Create new TxConfirmStats. This is called by BlockPolicyEstimator's + * constructor with default values. + * @param defaultBuckets contains the upper limits for the bucket boundaries + * @param maxConfirms max number of confirms to track + * @param decay how much to decay the historical moving average per block + */ + TxConfirmStats(const std::vector& defaultBuckets, unsigned int maxConfirms, double decay); + + /** Clear the state of the curBlock variables to start counting for the new block */ + void ClearCurrent(unsigned int nBlockHeight); + + /** + * Record a new transaction data point in the current block stats + * @param blocksToConfirm the number of blocks it took this transaction to confirm + * @param val the feerate of the transaction + * @warning blocksToConfirm is 1-based and has to be >= 1 + */ + void Record(int blocksToConfirm, double val); + + /** Record a new transaction entering the mempool*/ + unsigned int NewTx(unsigned int nBlockHeight, double val); + + /** Remove a transaction from mempool tracking stats*/ + void removeTx(unsigned int entryHeight, unsigned int nBestSeenHeight, + unsigned int bucketIndex); + + /** Update our estimates by decaying our historical moving average and updating + with the data gathered from the current block */ + void UpdateMovingAverages(); + + /** + * Calculate a feerate estimate. Find the lowest value bucket (or range of buckets + * to make sure we have enough data points) whose transactions still have sufficient likelihood + * of being confirmed within the target number of confirmations + * @param confTarget target number of confirmations + * @param sufficientTxVal required average number of transactions per block in a bucket range + * @param minSuccess the success probability we require + * @param requireGreater return the lowest feerate such that all higher values pass minSuccess OR + * return the highest feerate such that all lower values fail minSuccess + * @param nBlockHeight the current block height + */ + double EstimateMedianVal(int confTarget, double sufficientTxVal, + double minSuccess, bool requireGreater, unsigned int nBlockHeight) const; + + /** Return the max number of confirms we're tracking */ + unsigned int GetMaxConfirms() const { return confAvg.size(); } + + /** Write state of estimation data to a file*/ + void Write(CAutoFile& fileout) const; + + /** + * Read saved state of estimation data from a file and replace all internal data structures and + * variables with this state. + */ + void Read(CAutoFile& filein); +}; + + TxConfirmStats::TxConfirmStats(const std::vector& defaultBuckets, unsigned int maxConfirms, double _decay) { diff --git a/src/policy/fees.h b/src/policy/fees.h index 1a9eb0051..34f07c727 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -18,6 +18,7 @@ class CAutoFile; class CFeeRate; class CTxMemPoolEntry; class CTxMemPool; +class TxConfirmStats; /** \class CBlockPolicyEstimator * The BlockPolicyEstimator is used for estimating the feerate needed @@ -60,113 +61,6 @@ class CTxMemPool; * they've been outstanding. */ -/** - * We will instantiate an instance of this class to track transactions that were - * included in a block. We will lump transactions into a bucket according to their - * approximate feerate and then track how long it took for those txs to be included in a block - * - * The tracking of unconfirmed (mempool) transactions is completely independent of the - * historical tracking of transactions that have been confirmed in a block. - */ -class TxConfirmStats -{ -private: - //Define the buckets we will group transactions into - std::vector buckets; // The upper-bound of the range for the bucket (inclusive) - std::map bucketMap; // Map of bucket upper-bound to index into all vectors by bucket - - // For each bucket X: - // Count the total # of txs in each bucket - // Track the historical moving average of this total over blocks - std::vector txCtAvg; - // and calculate the total for the current block to update the moving average - std::vector curBlockTxCt; - - // Count the total # of txs confirmed within Y blocks in each bucket - // Track the historical moving average of theses totals over blocks - std::vector > confAvg; // confAvg[Y][X] - // and calculate the totals for the current block to update the moving averages - std::vector > curBlockConf; // curBlockConf[Y][X] - - // Sum the total feerate of all tx's in each bucket - // Track the historical moving average of this total over blocks - std::vector avg; - // and calculate the total for the current block to update the moving average - std::vector curBlockVal; - - // Combine the conf counts with tx counts to calculate the confirmation % for each Y,X - // Combine the total value with the tx counts to calculate the avg feerate per bucket - - double decay; - - // Mempool counts of outstanding transactions - // For each bucket X, track the number of transactions in the mempool - // that are unconfirmed for each possible confirmation value Y - std::vector > unconfTxs; //unconfTxs[Y][X] - // transactions still unconfirmed after MAX_CONFIRMS for each bucket - std::vector oldUnconfTxs; - -public: - /** - * Create new TxConfirmStats. This is called by BlockPolicyEstimator's - * constructor with default values. - * @param defaultBuckets contains the upper limits for the bucket boundaries - * @param maxConfirms max number of confirms to track - * @param decay how much to decay the historical moving average per block - */ - TxConfirmStats(const std::vector& defaultBuckets, unsigned int maxConfirms, double decay); - - /** Clear the state of the curBlock variables to start counting for the new block */ - void ClearCurrent(unsigned int nBlockHeight); - - /** - * Record a new transaction data point in the current block stats - * @param blocksToConfirm the number of blocks it took this transaction to confirm - * @param val the feerate of the transaction - * @warning blocksToConfirm is 1-based and has to be >= 1 - */ - void Record(int blocksToConfirm, double val); - - /** Record a new transaction entering the mempool*/ - unsigned int NewTx(unsigned int nBlockHeight, double val); - - /** Remove a transaction from mempool tracking stats*/ - void removeTx(unsigned int entryHeight, unsigned int nBestSeenHeight, - unsigned int bucketIndex); - - /** Update our estimates by decaying our historical moving average and updating - with the data gathered from the current block */ - void UpdateMovingAverages(); - - /** - * Calculate a feerate estimate. Find the lowest value bucket (or range of buckets - * to make sure we have enough data points) whose transactions still have sufficient likelihood - * of being confirmed within the target number of confirmations - * @param confTarget target number of confirmations - * @param sufficientTxVal required average number of transactions per block in a bucket range - * @param minSuccess the success probability we require - * @param requireGreater return the lowest feerate such that all higher values pass minSuccess OR - * return the highest feerate such that all lower values fail minSuccess - * @param nBlockHeight the current block height - */ - double EstimateMedianVal(int confTarget, double sufficientTxVal, - double minSuccess, bool requireGreater, unsigned int nBlockHeight) const; - - /** Return the max number of confirms we're tracking */ - unsigned int GetMaxConfirms() const { return confAvg.size(); } - - /** Write state of estimation data to a file*/ - void Write(CAutoFile& fileout) const; - - /** - * Read saved state of estimation data from a file and replace all internal data structures and - * variables with this state. - */ - void Read(CAutoFile& filein); -}; - - - /** Track confirm delays up to 25 blocks, can't estimate beyond that */ static const unsigned int MAX_BLOCK_CONFIRMS = 25;