From c0a273f4c8bed50de9fc227d98d90dc5ff755515 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 23 Feb 2017 15:13:41 -0500 Subject: [PATCH 01/15] Change file format for fee estimates. Move buckets and bucketMap to be stored as part of overall serialization of estimator. Add some placeholder data so file format is only changed once. Maintain 3 different TxConfirmStats with potential for different decays and scales. --- src/policy/fees.cpp | 236 +++++++++++++++++++++++++++++--------------- src/policy/fees.h | 5 + 2 files changed, 160 insertions(+), 81 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index f3f7f8378e5..0f3f2c63d26 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -26,8 +26,8 @@ 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 + const std::vector& buckets; // The upper-bound of the range for the bucket (inclusive) + const 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 @@ -38,9 +38,11 @@ private: // 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] + 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] + std::vector> curBlockConf; // curBlockConf[Y][X] + + std::vector> failAvg; // future use // Sum the total feerate of all tx's in each bucket // Track the historical moving average of this total over blocks @@ -53,6 +55,8 @@ private: double decay; + unsigned int scale; + // 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 @@ -60,6 +64,8 @@ private: // transactions still unconfirmed after MAX_CONFIRMS for each bucket std::vector oldUnconfTxs; + void resizeInMemoryCounters(size_t newbuckets); + public: /** * Create new TxConfirmStats. This is called by BlockPolicyEstimator's @@ -68,7 +74,8 @@ public: * @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); + TxConfirmStats(const std::vector& defaultBuckets, const std::map& defaultBucketMap, + unsigned int maxConfirms, double decay); /** Clear the state of the curBlock variables to start counting for the new block */ void ClearCurrent(unsigned int nBlockHeight); @@ -116,32 +123,39 @@ public: * Read saved state of estimation data from a file and replace all internal data structures and * variables with this state. */ - void Read(CAutoFile& filein); + void Read(CAutoFile& filein, int nFileVersion, size_t numBuckets); }; TxConfirmStats::TxConfirmStats(const std::vector& defaultBuckets, + const std::map& defaultBucketMap, unsigned int maxConfirms, double _decay) + : buckets(defaultBuckets), bucketMap(defaultBucketMap) { decay = _decay; - for (unsigned int i = 0; i < defaultBuckets.size(); i++) { - buckets.push_back(defaultBuckets[i]); - bucketMap[defaultBuckets[i]] = i; - } + scale = 1; confAvg.resize(maxConfirms); - curBlockConf.resize(maxConfirms); - unconfTxs.resize(maxConfirms); for (unsigned int i = 0; i < maxConfirms; i++) { confAvg[i].resize(buckets.size()); - curBlockConf[i].resize(buckets.size()); - unconfTxs[i].resize(buckets.size()); } - oldUnconfTxs.resize(buckets.size()); - curBlockTxCt.resize(buckets.size()); txCtAvg.resize(buckets.size()); - curBlockVal.resize(buckets.size()); avg.resize(buckets.size()); + + resizeInMemoryCounters(buckets.size()); +} + +void TxConfirmStats::resizeInMemoryCounters(size_t newbuckets) { + curBlockConf.resize(GetMaxConfirms()); + // newbuckets must be passed in because the buckets referred to during Read have not been updated yet. + unconfTxs.resize(GetMaxConfirms()); + for (unsigned int i = 0; i < unconfTxs.size(); i++) { + curBlockConf[i].resize(newbuckets); + unconfTxs[i].resize(newbuckets); + } + oldUnconfTxs.resize(newbuckets); + curBlockTxCt.resize(newbuckets); + curBlockVal.resize(newbuckets); } // Zero out the data for the current block @@ -283,70 +297,55 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, void TxConfirmStats::Write(CAutoFile& fileout) const { fileout << decay; - fileout << buckets; + fileout << scale; fileout << avg; fileout << txCtAvg; fileout << confAvg; + fileout << failAvg; } -void TxConfirmStats::Read(CAutoFile& filein) +void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets) { - // Read data file into temporary variables and do some very basic sanity checking - std::vector fileBuckets; - std::vector fileAvg; - std::vector > fileConfAvg; - std::vector fileTxCtAvg; - double fileDecay; + // Read data file and do some very basic sanity checking + // buckets and bucketMap are not updated yet, so don't access them + // If there is a read failure, we'll just discard this entire object anyway size_t maxConfirms; - size_t numBuckets; - filein >> fileDecay; - if (fileDecay <= 0 || fileDecay >= 1) - throw std::runtime_error("Corrupt estimates file. Decay must be between 0 and 1 (non-inclusive)"); - filein >> fileBuckets; - numBuckets = fileBuckets.size(); - if (numBuckets <= 1 || numBuckets > 1000) - throw std::runtime_error("Corrupt estimates file. Must have between 2 and 1000 feerate buckets"); - filein >> fileAvg; - if (fileAvg.size() != numBuckets) - throw std::runtime_error("Corrupt estimates file. Mismatch in feerate average bucket count"); - filein >> fileTxCtAvg; - if (fileTxCtAvg.size() != numBuckets) - throw std::runtime_error("Corrupt estimates file. Mismatch in tx count bucket count"); - filein >> fileConfAvg; - maxConfirms = fileConfAvg.size(); - if (maxConfirms <= 0 || maxConfirms > 6 * 24 * 7) // one week - throw std::runtime_error("Corrupt estimates file. Must maintain estimates for between 1 and 1008 (one week) confirms"); - for (unsigned int i = 0; i < maxConfirms; i++) { - if (fileConfAvg[i].size() != numBuckets) - throw std::runtime_error("Corrupt estimates file. Mismatch in feerate conf average bucket count"); + // The current version will store the decay with each individual TxConfirmStats and also keep a scale factor + if (nFileVersion >= 149900) { + filein >> decay; + if (decay <= 0 || decay >= 1) { + throw std::runtime_error("Corrupt estimates file. Decay must be between 0 and 1 (non-inclusive)"); + } + filein >> scale; //Unused for now + } + + filein >> avg; + if (avg.size() != numBuckets) { + throw std::runtime_error("Corrupt estimates file. Mismatch in feerate average bucket count"); + } + filein >> txCtAvg; + if (txCtAvg.size() != numBuckets) { + throw std::runtime_error("Corrupt estimates file. Mismatch in tx count bucket count"); + } + filein >> confAvg; + maxConfirms = confAvg.size(); + if (maxConfirms <= 0 || maxConfirms > 6 * 24 * 7) { // one week + throw std::runtime_error("Corrupt estimates file. Must maintain estimates for between 1 and 1008 (one week) confirms"); + } + for (unsigned int i = 0; i < maxConfirms; i++) { + if (confAvg[i].size() != numBuckets) { + throw std::runtime_error("Corrupt estimates file. Mismatch in feerate conf average bucket count"); + } + } + + if (nFileVersion >= 149900) { + filein >> failAvg; } - // Now that we've processed the entire feerate estimate data file and not - // thrown any errors, we can copy it to our data structures - decay = fileDecay; - buckets = fileBuckets; - avg = fileAvg; - confAvg = fileConfAvg; - txCtAvg = fileTxCtAvg; - bucketMap.clear(); // Resize the current block variables which aren't stored in the data file // to match the number of confirms and buckets - curBlockConf.resize(maxConfirms); - for (unsigned int i = 0; i < maxConfirms; i++) { - curBlockConf[i].resize(buckets.size()); - } - curBlockTxCt.resize(buckets.size()); - curBlockVal.resize(buckets.size()); - - unconfTxs.resize(maxConfirms); - for (unsigned int i = 0; i < maxConfirms; i++) { - unconfTxs[i].resize(buckets.size()); - } - oldUnconfTxs.resize(buckets.size()); - - for (unsigned int i = 0; i < buckets.size(); i++) - bucketMap[buckets[i]] = i; + resizeInMemoryCounters(numBuckets); LogPrint(BCLog::ESTIMATEFEE, "Reading estimates: %u buckets counting confirms up to %u blocks\n", numBuckets, maxConfirms); @@ -413,17 +412,25 @@ CBlockPolicyEstimator::CBlockPolicyEstimator() { static_assert(MIN_BUCKET_FEERATE > 0, "Min feerate must be nonzero"); minTrackedFee = CFeeRate(MIN_BUCKET_FEERATE); - std::vector vfeelist; - for (double bucketBoundary = minTrackedFee.GetFeePerK(); bucketBoundary <= MAX_BUCKET_FEERATE; bucketBoundary *= FEE_SPACING) { - vfeelist.push_back(bucketBoundary); + size_t bucketIndex = 0; + for (double bucketBoundary = minTrackedFee.GetFeePerK(); bucketBoundary <= MAX_BUCKET_FEERATE; bucketBoundary *= FEE_SPACING, bucketIndex++) { + buckets.push_back(bucketBoundary); + bucketMap[bucketBoundary] = bucketIndex; } - vfeelist.push_back(INF_FEERATE); - feeStats = new TxConfirmStats(vfeelist, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); + buckets.push_back(INF_FEERATE); + bucketMap[INF_FEERATE] = bucketIndex; + assert(bucketMap.size() == buckets.size()); + + feeStats = new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); + shortStats = new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); + longStats = new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); } CBlockPolicyEstimator::~CBlockPolicyEstimator() { delete feeStats; + delete shortStats; + delete longStats; } void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, bool validFeeEstimate) @@ -580,10 +587,15 @@ bool CBlockPolicyEstimator::Write(CAutoFile& fileout) const { try { LOCK(cs_feeEstimator); - fileout << 139900; // version required to read: 0.13.99 or later + fileout << 149900; // version required to read: 0.14.99 or later fileout << CLIENT_VERSION; // version that wrote the file fileout << nBestSeenHeight; + unsigned int future1 = 0, future2 = 0; + fileout << future1 << future2; + fileout << buckets; feeStats->Write(fileout); + shortStats->Write(fileout); + longStats->Write(fileout); } catch (const std::exception&) { LogPrintf("CBlockPolicyEstimator::Write(): unable to read policy estimator data (non-fatal)\n"); @@ -596,17 +608,79 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) { try { LOCK(cs_feeEstimator); - int nVersionRequired, nVersionThatWrote, nFileBestSeenHeight; + int nVersionRequired, nVersionThatWrote; + unsigned int nFileBestSeenHeight; filein >> nVersionRequired >> nVersionThatWrote; if (nVersionRequired > CLIENT_VERSION) return error("CBlockPolicyEstimator::Read(): up-version (%d) fee estimate file", nVersionRequired); + + // Read fee estimates file into temporary variables so existing data + // structures aren't corrupted if there is an exception. filein >> nFileBestSeenHeight; - feeStats->Read(filein); - nBestSeenHeight = nFileBestSeenHeight; - // if nVersionThatWrote < 139900 then another TxConfirmStats (for priority) follows but can be ignored. + + if (nVersionThatWrote < 149900) { + // Read the old fee estimates file for temporary use, but then discard. Will start collecting data from scratch. + // decay is stored before buckets in old versions, so pre-read decay and pass into TxConfirmStats constructor + double tempDecay; + filein >> tempDecay; + if (tempDecay <= 0 || tempDecay >= 1) + throw std::runtime_error("Corrupt estimates file. Decay must be between 0 and 1 (non-inclusive)"); + + std::vector tempBuckets; + filein >> tempBuckets; + size_t tempNum = tempBuckets.size(); + if (tempNum <= 1 || tempNum > 1000) + throw std::runtime_error("Corrupt estimates file. Must have between 2 and 1000 feerate buckets"); + + std::map tempMap; + + std::unique_ptr tempFeeStats(new TxConfirmStats(tempBuckets, tempMap, MAX_BLOCK_CONFIRMS, tempDecay)); + tempFeeStats->Read(filein, nVersionThatWrote, tempNum); + // if nVersionThatWrote < 139900 then another TxConfirmStats (for priority) follows but can be ignored. + + tempMap.clear(); + for (unsigned int i = 0; i < tempBuckets.size(); i++) { + tempMap[tempBuckets[i]] = i; + } + } + else { // nVersionThatWrote >= 149900 + unsigned int future1, future2; + filein >> future1 >> future2; + + std::vector fileBuckets; + filein >> fileBuckets; + size_t numBuckets = fileBuckets.size(); + if (numBuckets <= 1 || numBuckets > 1000) + throw std::runtime_error("Corrupt estimates file. Must have between 2 and 1000 feerate buckets"); + + std::unique_ptr fileFeeStats(new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY)); + std::unique_ptr fileShortStats(new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY)); + std::unique_ptr fileLongStats(new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY)); + fileFeeStats->Read(filein, nVersionThatWrote, numBuckets); + fileShortStats->Read(filein, nVersionThatWrote, numBuckets); + fileLongStats->Read(filein, nVersionThatWrote, numBuckets); + + // Fee estimates file parsed correctly + // Copy buckets from file and refresh our bucketmap + buckets = fileBuckets; + bucketMap.clear(); + for (unsigned int i = 0; i < buckets.size(); i++) { + bucketMap[buckets[i]] = i; + } + + // Destroy old TxConfirmStats and point to new ones that already reference buckets and bucketMap + delete feeStats; + delete shortStats; + delete longStats; + feeStats = fileFeeStats.release(); + shortStats = fileShortStats.release(); + longStats = fileLongStats.release(); + + nBestSeenHeight = nFileBestSeenHeight; + } } - catch (const std::exception&) { - LogPrintf("CBlockPolicyEstimator::Read(): unable to read policy estimator data (non-fatal)\n"); + catch (const std::exception& e) { + LogPrintf("CBlockPolicyEstimator::Read(): unable to read policy estimator data (non-fatal): %s\n",e.what()); return false; } return true; diff --git a/src/policy/fees.h b/src/policy/fees.h index 34f07c72707..7aaba7d7d56 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -141,10 +141,15 @@ private: /** Classes to track historical data on transaction confirmations */ TxConfirmStats* feeStats; + TxConfirmStats* shortStats; + TxConfirmStats* longStats; unsigned int trackedTxs; unsigned int untrackedTxs; + 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 + mutable CCriticalSection cs_feeEstimator; /** Process a transaction confirmed in a block*/ From e5007bae35ce22036a816505038277d99c84e3f7 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 9 Mar 2017 14:02:00 -0500 Subject: [PATCH 02/15] Change parameters for fee estimation and estimates on all 3 time horizons. Make feerate buckets smaller (5% instead of 10%) and make the 3 different horizons have half lifes of 3 hours, 1 day and 1 week respectively. --- src/policy/fees.cpp | 37 ++++++++++----- src/policy/fees.h | 76 +++++++++++++++++++----------- src/test/policyestimator_tests.cpp | 27 ++++++----- 3 files changed, 89 insertions(+), 51 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 0f3f2c63d26..42059cf94f8 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -14,6 +14,8 @@ #include "txmempool.h" #include "util.h" +static constexpr double INF_FEERATE = 1e99; + /** * 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 @@ -400,6 +402,8 @@ bool CBlockPolicyEstimator::removeTx(uint256 hash) std::map::iterator pos = mapMemPoolTxs.find(hash); if (pos != mapMemPoolTxs.end()) { feeStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex); + shortStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex); + longStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex); mapMemPoolTxs.erase(hash); return true; } else { @@ -421,9 +425,9 @@ CBlockPolicyEstimator::CBlockPolicyEstimator() bucketMap[INF_FEERATE] = bucketIndex; assert(bucketMap.size() == buckets.size()); - feeStats = new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); - shortStats = new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); - longStats = new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); + feeStats = new TxConfirmStats(buckets, bucketMap, MED_BLOCK_CONFIRMS, MED_DECAY); + shortStats = new TxConfirmStats(buckets, bucketMap, SHORT_BLOCK_CONFIRMS, SHORT_DECAY); + longStats = new TxConfirmStats(buckets, bucketMap, LONG_BLOCK_CONFIRMS, LONG_DECAY); } CBlockPolicyEstimator::~CBlockPolicyEstimator() @@ -464,7 +468,12 @@ 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()); + unsigned int bucketIndex = feeStats->NewTx(txHeight, (double)feeRate.GetFeePerK()); + mapMemPoolTxs[hash].bucketIndex = bucketIndex; + unsigned int bucketIndex2 = shortStats->NewTx(txHeight, (double)feeRate.GetFeePerK()); + assert(bucketIndex == bucketIndex2); + unsigned int bucketIndex3 = longStats->NewTx(txHeight, (double)feeRate.GetFeePerK()); + assert(bucketIndex == bucketIndex3); } bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry) @@ -489,6 +498,8 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM CFeeRate feeRate(entry->GetFee(), entry->GetTxSize()); feeStats->Record(blocksToConfirm, (double)feeRate.GetFeePerK()); + shortStats->Record(blocksToConfirm, (double)feeRate.GetFeePerK()); + longStats->Record(blocksToConfirm, (double)feeRate.GetFeePerK()); return true; } @@ -512,6 +523,8 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, // Clear the current block state and update unconfirmed circular buffer feeStats->ClearCurrent(nBlockHeight); + shortStats->ClearCurrent(nBlockHeight); + longStats->ClearCurrent(nBlockHeight); unsigned int countedTxs = 0; // Repopulate the current block states @@ -522,6 +535,8 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, // Update all exponential averages with the current block state feeStats->UpdateMovingAverages(); + shortStats->UpdateMovingAverages(); + longStats->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()); @@ -538,7 +553,7 @@ CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget) const 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, DOUBLE_SUCCESS_PCT, true, nBestSeenHeight); if (median < 0) return CFeeRate(0); @@ -565,7 +580,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun confTarget = 2; while (median < 0 && (unsigned int)confTarget <= feeStats->GetMaxConfirms()) { - median = feeStats->EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); + median = feeStats->EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, DOUBLE_SUCCESS_PCT, true, nBestSeenHeight); } } // Must unlock cs_feeEstimator before taking mempool locks @@ -634,7 +649,7 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) std::map tempMap; - std::unique_ptr tempFeeStats(new TxConfirmStats(tempBuckets, tempMap, MAX_BLOCK_CONFIRMS, tempDecay)); + std::unique_ptr tempFeeStats(new TxConfirmStats(tempBuckets, tempMap, MED_BLOCK_CONFIRMS, tempDecay)); tempFeeStats->Read(filein, nVersionThatWrote, tempNum); // if nVersionThatWrote < 139900 then another TxConfirmStats (for priority) follows but can be ignored. @@ -653,9 +668,9 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) if (numBuckets <= 1 || numBuckets > 1000) throw std::runtime_error("Corrupt estimates file. Must have between 2 and 1000 feerate buckets"); - std::unique_ptr fileFeeStats(new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY)); - std::unique_ptr fileShortStats(new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY)); - std::unique_ptr fileLongStats(new TxConfirmStats(buckets, bucketMap, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY)); + std::unique_ptr fileFeeStats(new TxConfirmStats(buckets, bucketMap, MED_BLOCK_CONFIRMS, MED_DECAY)); + std::unique_ptr fileShortStats(new TxConfirmStats(buckets, bucketMap, SHORT_BLOCK_CONFIRMS, SHORT_DECAY)); + std::unique_ptr fileLongStats(new TxConfirmStats(buckets, bucketMap, LONG_BLOCK_CONFIRMS, LONG_DECAY)); fileFeeStats->Read(filein, nVersionThatWrote, numBuckets); fileShortStats->Read(filein, nVersionThatWrote, numBuckets); fileLongStats->Read(filein, nVersionThatWrote, numBuckets); @@ -690,7 +705,7 @@ FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee) { CAmount minFeeLimit = std::max(CAmount(1), minIncrementalFee.GetFeePerK() / 2); feeset.insert(0); - for (double bucketBoundary = minFeeLimit; bucketBoundary <= MAX_BUCKET_FEERATE; bucketBoundary *= FEE_SPACING) { + for (double bucketBoundary = minFeeLimit; bucketBoundary <= MAX_FILTER_FEERATE; bucketBoundary *= FEE_FILTER_SPACING) { feeset.insert(bucketBoundary); } } diff --git a/src/policy/fees.h b/src/policy/fees.h index 7aaba7d7d56..0df40df42a0 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -61,34 +61,6 @@ class TxConfirmStats; * they've been outstanding. */ -/** Track confirm delays up to 25 blocks, can't estimate beyond that */ -static const unsigned int MAX_BLOCK_CONFIRMS = 25; - -/** Decay of .998 is a half-life of 346 blocks or about 2.4 days */ -static const double DEFAULT_DECAY = .998; - -/** Require greater than 95% of X feerate transactions to be confirmed within Y blocks for X to be big enough */ -static const double MIN_SUCCESS_PCT = .95; - -/** Require an avg of 1 tx in the combined feerate bucket per block to have stat significance */ -static const double SUFFICIENT_FEETXS = 1; - -// Minimum and Maximum values for tracking feerates -// The MIN_BUCKET_FEERATE should just be set to the lowest reasonable feerate we -// might ever want to track. Historically this has been 1000 since it was -// inheriting DEFAULT_MIN_RELAY_TX_FEE and changing it is disruptive as it -// invalidates old estimates files. So leave it at 1000 unless it becomes -// necessary to lower it, and then lower it substantially. -static constexpr double MIN_BUCKET_FEERATE = 1000; -static const double MAX_BUCKET_FEERATE = 1e7; -static const double INF_FEERATE = MAX_MONEY; - -// We have to lump transactions into buckets based on feerate, but we want to be able -// to give accurate estimates over a large range of potential feerates -// Therefore it makes sense to exponentially space the buckets -/** Spacing of FeeRate buckets */ -static const double FEE_SPACING = 1.1; - /** * We want to be able to estimate feerates that are needed on tx's to be included in * a certain number of blocks. Every time a block is added to the best chain, this class records @@ -96,6 +68,46 @@ static const double FEE_SPACING = 1.1; */ class CBlockPolicyEstimator { +private: + /** Track confirm delays up to 12 blocks medium decay */ + static constexpr unsigned int SHORT_BLOCK_CONFIRMS = 12; + /** Track confirm delays up to 48 blocks medium decay */ + static constexpr unsigned int MED_BLOCK_CONFIRMS = 48; + /** Track confirm delays up to 1008 blocks for longer decay */ + static constexpr unsigned int LONG_BLOCK_CONFIRMS = 1008; + + /** Decay of .962 is a half-life of 18 blocks or about 3 hours */ + static constexpr double SHORT_DECAY = .962; + /** Decay of .998 is a half-life of 144 blocks or about 1 day */ + static constexpr double MED_DECAY = .9952; + /** Decay of .9995 is a half-life of 1008 blocks or about 1 week */ + static constexpr double LONG_DECAY = .99931; + + /** Require greater than 95% of X feerate transactions to be confirmed within Y blocks for X to be big enough */ + static constexpr double HALF_SUCCESS_PCT = .6; + static constexpr double SUCCESS_PCT = .85; + static constexpr double DOUBLE_SUCCESS_PCT = .95; + + /** Require an avg of 1 tx in the combined feerate bucket per block to have stat significance */ + static constexpr double SUFFICIENT_FEETXS = 1; + + /** Minimum and Maximum values for tracking feerates + * The MIN_BUCKET_FEERATE should just be set to the lowest reasonable feerate we + * might ever want to track. Historically this has been 1000 since it was + * inheriting DEFAULT_MIN_RELAY_TX_FEE and changing it is disruptive as it + * invalidates old estimates files. So leave it at 1000 unless it becomes + * necessary to lower it, and then lower it substantially. + */ + static constexpr double MIN_BUCKET_FEERATE = 1000; + static constexpr double MAX_BUCKET_FEERATE = 1e7; + + /** Spacing of FeeRate buckets + * We have to lump transactions into buckets based on feerate, but we want to be able + * to give accurate estimates over a large range of potential feerates + * Therefore it makes sense to exponentially space the buckets + */ + static constexpr double FEE_SPACING = 1.05; + public: /** Create new BlockPolicyEstimator and initialize stats tracking classes with default values */ CBlockPolicyEstimator(); @@ -159,6 +171,14 @@ private: class FeeFilterRounder { +private: + static constexpr double MAX_FILTER_FEERATE = 1e7; + /** FEE_FILTER_SPACING is just used to provide some quantization of fee + * filter results. Historically it reused FEE_SPACING, but it is completely + * unrelated, and was made a separate constant so the two concepts are not + * tied together */ + static constexpr double FEE_FILTER_SPACING = 1.1; + public: /** Create new FeeFilterRounder */ FeeFilterRounder(const CFeeRate& minIncrementalFee); diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index ed6782ea345..942efce0f76 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -50,8 +50,8 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) int blocknum = 0; // Loop through 200 blocks - // At a decay .998 and 4 fee transactions per block - // This makes the tx count about 1.33 per bucket, above the 1 threshold + // At a decay .9952 and 4 fee transactions per block + // This makes the tx count about 2.5 per bucket, well above the 0.1 threshold while (blocknum < 200) { for (int j = 0; j < 10; j++) { // For each fee for (int k = 0; k < 4; k++) { // add 4 fee txs @@ -75,18 +75,17 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) } mpool.removeForBlock(block, ++blocknum); block.clear(); - if (blocknum == 30) { - // 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% + // Check after just a few txs that combining buckets works as expected + if (blocknum == 3) { + // At this point we should need to combine 3 buckets to get enough data points + // So estimateFee(1) should fail and estimateFee(2) should return somewhere around + // 9*baserate. estimateFee(2) %'s are 100,100,90 = average 97% 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); + BOOST_CHECK(feeEst.estimateFee(2).GetFeePerK() < 9*baseRate.GetFeePerK() + deltaFee); + BOOST_CHECK(feeEst.estimateFee(2).GetFeePerK() > 9*baseRate.GetFeePerK() - deltaFee); int answerFound; - 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(1, &answerFound, mpool) == feeEst.estimateFee(2) && answerFound == 2); + BOOST_CHECK(feeEst.estimateSmartFee(2, &answerFound, mpool) == feeEst.estimateFee(2) && answerFound == 2); BOOST_CHECK(feeEst.estimateSmartFee(4, &answerFound, mpool) == feeEst.estimateFee(4) && answerFound == 4); BOOST_CHECK(feeEst.estimateSmartFee(8, &answerFound, mpool) == feeEst.estimateFee(8) && answerFound == 8); } @@ -113,6 +112,10 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) BOOST_CHECK(origFeeEst[i-1] == CFeeRate(0).GetFeePerK()); } } + // Fill out rest of the original estimates + for (int i = 10; i <= 48; i++) { + origFeeEst.push_back(feeEst.estimateFee(i).GetFeePerK()); + } // Mine 50 more blocks with no transactions happening, estimates shouldn't change // We haven't decayed the moving average enough so we still have enough data points in every bucket From d3e30bca1bfa9900bc31fec957bb001115acac7b Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 28 Feb 2017 17:29:42 -0500 Subject: [PATCH 03/15] Refactor to update moving average on fly --- src/policy/fees.cpp | 45 ++++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 42059cf94f8..0e9fb4dfeae 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -35,22 +35,16 @@ private: // 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] std::vector> failAvg; // future use // 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 @@ -79,7 +73,7 @@ public: TxConfirmStats(const std::vector& defaultBuckets, const std::map& defaultBucketMap, unsigned int maxConfirms, double decay); - /** Clear the state of the curBlock variables to start counting for the new block */ + /** Roll the circular buffer for unconfirmed txs*/ void ClearCurrent(unsigned int nBlockHeight); /** @@ -148,28 +142,20 @@ TxConfirmStats::TxConfirmStats(const std::vector& defaultBuckets, } void TxConfirmStats::resizeInMemoryCounters(size_t newbuckets) { - curBlockConf.resize(GetMaxConfirms()); // newbuckets must be passed in because the buckets referred to during Read have not been updated yet. unconfTxs.resize(GetMaxConfirms()); for (unsigned int i = 0; i < unconfTxs.size(); i++) { - curBlockConf[i].resize(newbuckets); unconfTxs[i].resize(newbuckets); } oldUnconfTxs.resize(newbuckets); - curBlockTxCt.resize(newbuckets); - curBlockVal.resize(newbuckets); } -// Zero out the data for the current block +// Roll the unconfirmed txs circular buffer void TxConfirmStats::ClearCurrent(unsigned int nBlockHeight) { for (unsigned int j = 0; j < buckets.size(); j++) { oldUnconfTxs[j] += unconfTxs[nBlockHeight%unconfTxs.size()][j]; unconfTxs[nBlockHeight%unconfTxs.size()][j] = 0; - for (unsigned int i = 0; i < curBlockConf.size(); i++) - curBlockConf[i][j] = 0; - curBlockTxCt[j] = 0; - curBlockVal[j] = 0; } } @@ -180,20 +166,20 @@ void TxConfirmStats::Record(int blocksToConfirm, double val) if (blocksToConfirm < 1) return; unsigned int bucketindex = bucketMap.lower_bound(val)->second; - for (size_t i = blocksToConfirm; i <= curBlockConf.size(); i++) { - curBlockConf[i - 1][bucketindex]++; + for (size_t i = blocksToConfirm; i <= confAvg.size(); i++) { + confAvg[i - 1][bucketindex]++; } - curBlockTxCt[bucketindex]++; - curBlockVal[bucketindex] += val; + txCtAvg[bucketindex]++; + avg[bucketindex] += val; } void TxConfirmStats::UpdateMovingAverages() { for (unsigned int j = 0; j < buckets.size(); j++) { for (unsigned int i = 0; i < confAvg.size(); i++) - confAvg[i][j] = confAvg[i][j] * decay + curBlockConf[i][j]; - avg[j] = avg[j] * decay + curBlockVal[j]; - txCtAvg[j] = txCtAvg[j] * decay + curBlockTxCt[j]; + confAvg[i][j] = confAvg[i][j] * decay; + avg[j] = avg[j] * decay; + txCtAvg[j] = txCtAvg[j] * decay; } } @@ -521,22 +507,23 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, // of unconfirmed txs to remove from tracking. nBestSeenHeight = nBlockHeight; - // Clear the current block state and update unconfirmed circular buffer + // Update unconfirmed circular buffer feeStats->ClearCurrent(nBlockHeight); shortStats->ClearCurrent(nBlockHeight); longStats->ClearCurrent(nBlockHeight); + // Decay all exponential averages + feeStats->UpdateMovingAverages(); + shortStats->UpdateMovingAverages(); + longStats->UpdateMovingAverages(); + unsigned int countedTxs = 0; - // Repopulate the current block states + // Update averages with data points from current block for (unsigned int i = 0; i < entries.size(); i++) { if (processBlockTx(nBlockHeight, entries[i])) countedTxs++; } - // Update all exponential averages with the current block state - feeStats->UpdateMovingAverages(); - shortStats->UpdateMovingAverages(); - longStats->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()); From 1ba43cc0ecaffcb6389dbbb6bcf038e78cf04023 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 2 Mar 2017 10:08:25 -0500 Subject: [PATCH 04/15] Make EstimateMedianVal smarter about small failures. Instead of stopping if it encounters a "sufficient" number of transactions which don't meet the threshold for being confirmed within the target, it keeps looking to add more transactions to see if there is a temporary blip in the data. This allows a smaller number of required data points. --- src/policy/fees.cpp | 8 +++----- src/policy/fees.h | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 0e9fb4dfeae..03a0df4610a 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -231,11 +231,9 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, double curPct = nConf / (totalNum + extraNum); // Check to see if we are no longer getting confirmed at the success rate - if (requireGreater && curPct < successBreakPoint) - break; - if (!requireGreater && curPct > successBreakPoint) - break; - + if ((requireGreater && curPct < successBreakPoint) || (!requireGreater && curPct > successBreakPoint)) { + continue; + } // Otherwise update the cumulative stats, and the bucket variables // and reset the counters else { diff --git a/src/policy/fees.h b/src/policy/fees.h index 0df40df42a0..c5955d7b04c 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -88,8 +88,8 @@ private: static constexpr double SUCCESS_PCT = .85; static constexpr double DOUBLE_SUCCESS_PCT = .95; - /** Require an avg of 1 tx in the combined feerate bucket per block to have stat significance */ - static constexpr double SUFFICIENT_FEETXS = 1; + /** Require an avg of 0.1 tx in the combined feerate bucket per block to have stat significance */ + static constexpr double SUFFICIENT_FEETXS = 0.1; /** Minimum and Maximum values for tracking feerates * The MIN_BUCKET_FEERATE should just be set to the lowest reasonable feerate we From 2681153af38324258dab8d1cf8e83c899324ece1 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 21 Feb 2017 22:18:13 -0500 Subject: [PATCH 05/15] minor refactor: explicitly track start of new bucket range and don't update curNearBucket on final loop. --- src/policy/fees.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 03a0df4610a..a0e56d244bf 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -214,9 +214,14 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, bool foundAnswer = false; unsigned int bins = unconfTxs.size(); + bool newBucketRange = true; // Start counting from highest(default) or lowest feerate transactions for (int bucket = startbucket; bucket >= 0 && bucket <= maxbucketindex; bucket += step) { + if (newBucketRange) { + curNearBucket = bucket; + newBucketRange = false; + } curFarBucket = bucket; nConf += confAvg[confTarget - 1][bucket]; totalNum += txCtAvg[bucket]; @@ -243,7 +248,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, extraNum = 0; bestNearBucket = curNearBucket; bestFarBucket = curFarBucket; - curNearBucket = bucket + step; + newBucketRange = true; } } } From 4186d3fdfd319b568b520dd587be27bdff45c53d Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 24 Jan 2017 16:30:03 -0500 Subject: [PATCH 06/15] Expose estimaterawfee Track information the ranges of fee rates that were used to calculate the fee estimates (the last range of fee rates in which the data points met the threshold and the first to fail) and provide an RPC call to return this information. --- src/policy/fees.cpp | 99 +++++++++++++++++++++++++++++++++++++++------ src/policy/fees.h | 29 +++++++++++++ src/rpc/client.cpp | 3 ++ src/rpc/mining.cpp | 74 +++++++++++++++++++++++++++++++++ 4 files changed, 193 insertions(+), 12 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index a0e56d244bf..a9a8335c617 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -107,7 +107,8 @@ public: * @param nBlockHeight the current block height */ double EstimateMedianVal(int confTarget, double sufficientTxVal, - double minSuccess, bool requireGreater, unsigned int nBlockHeight) const; + double minSuccess, bool requireGreater, unsigned int nBlockHeight, + EstimationResult *result = nullptr) const; /** Return the max number of confirms we're tracking */ unsigned int GetMaxConfirms() const { return confAvg.size(); } @@ -186,7 +187,7 @@ void TxConfirmStats::UpdateMovingAverages() // returns -1 on error conditions double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, double successBreakPoint, bool requireGreater, - unsigned int nBlockHeight) const + unsigned int nBlockHeight, EstimationResult *result) const { // Counters for a bucket (or range of buckets) double nConf = 0; // Number of tx's confirmed within the confTarget @@ -215,6 +216,9 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, bool foundAnswer = false; unsigned int bins = unconfTxs.size(); bool newBucketRange = true; + bool passing = true; + EstimatorBucket passBucket; + EstimatorBucket failBucket; // Start counting from highest(default) or lowest feerate transactions for (int bucket = startbucket; bucket >= 0 && bucket <= maxbucketindex; bucket += step) { @@ -237,14 +241,30 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, // Check to see if we are no longer getting confirmed at the success rate if ((requireGreater && curPct < successBreakPoint) || (!requireGreater && curPct > successBreakPoint)) { + if (passing == true) { + // First time we hit a failure record the failed bucket + unsigned int failMinBucket = std::min(curNearBucket, curFarBucket); + unsigned int failMaxBucket = std::max(curNearBucket, curFarBucket); + failBucket.start = failMinBucket ? buckets[failMinBucket - 1] : 0; + failBucket.end = buckets[failMaxBucket]; + failBucket.withinTarget = nConf; + failBucket.totalConfirmed = totalNum; + failBucket.inMempool = extraNum; + passing = false; + } continue; } // Otherwise update the cumulative stats, and the bucket variables // and reset the counters else { + failBucket = EstimatorBucket(); // Reset any failed bucket, currently passing foundAnswer = true; + passing = true; + passBucket.withinTarget = nConf; nConf = 0; + passBucket.totalConfirmed = totalNum; totalNum = 0; + passBucket.inMempool = extraNum; extraNum = 0; bestNearBucket = curNearBucket; bestFarBucket = curFarBucket; @@ -260,8 +280,8 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, // Find the bucket with the median transaction and then report the average feerate from that bucket // This is a compromise between finding the median which we can't since we don't save all tx's // and reporting the average which is less accurate - unsigned int minBucket = bestNearBucket < bestFarBucket ? bestNearBucket : bestFarBucket; - unsigned int maxBucket = bestNearBucket > bestFarBucket ? bestNearBucket : bestFarBucket; + unsigned int minBucket = std::min(bestNearBucket, bestFarBucket); + unsigned int maxBucket = std::max(bestNearBucket, bestFarBucket); for (unsigned int j = minBucket; j <= maxBucket; j++) { txSum += txCtAvg[j]; } @@ -275,13 +295,37 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, break; } } + + passBucket.start = minBucket ? buckets[minBucket-1] : 0; + passBucket.end = buckets[maxBucket]; } - LogPrint(BCLog::ESTIMATEFEE, "%3d: For conf success %s %4.2f need feerate %s: %12.5g from buckets %8g - %8g Cur Bucket stats %6.2f%% %8.1f/(%.1f+%d mempool)\n", - confTarget, requireGreater ? ">" : "<", successBreakPoint, - requireGreater ? ">" : "<", median, buckets[minBucket], buckets[maxBucket], - 100 * nConf / (totalNum + extraNum), nConf, totalNum, extraNum); + // If we were passing until we reached last few buckets with insufficient data, then report those as failed + if (passing && !newBucketRange) { + unsigned int failMinBucket = std::min(curNearBucket, curFarBucket); + unsigned int failMaxBucket = std::max(curNearBucket, curFarBucket); + failBucket.start = failMinBucket ? buckets[failMinBucket - 1] : 0; + failBucket.end = buckets[failMaxBucket]; + failBucket.withinTarget = nConf; + failBucket.totalConfirmed = totalNum; + failBucket.inMempool = extraNum; + } + LogPrint(BCLog::ESTIMATEFEE, "FeeEst: %d %s%.0f%% decay %.5f: need feerate: %g from (%g - %g) %.2f%% %.1f/(%.1f+%d mem) Fail: (%g - %g) %.2f%% %.1f/(%.1f+%d mem)\n", + confTarget, requireGreater ? ">" : "<", 100.0 * successBreakPoint, decay, + median, passBucket.start, passBucket.end, + 100 * passBucket.withinTarget / (passBucket.totalConfirmed + passBucket.inMempool), + passBucket.withinTarget, passBucket.totalConfirmed, passBucket.inMempool, + failBucket.start, failBucket.end, + 100 * failBucket.withinTarget / (failBucket.totalConfirmed + failBucket.inMempool), + failBucket.withinTarget, failBucket.totalConfirmed, failBucket.inMempool); + + + if (result) { + result->pass = passBucket; + result->fail = failBucket; + result->decay = decay; + } return median; } @@ -537,13 +581,44 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, 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) return CFeeRate(0); - double median = feeStats->EstimateMedianVal(confTarget, SUFFICIENT_FEETXS, DOUBLE_SUCCESS_PCT, true, nBestSeenHeight); + return estimateRawFee(confTarget, DOUBLE_SUCCESS_PCT, FeeEstimateHorizon::MED_HALFLIFE); +} + +CFeeRate CBlockPolicyEstimator::estimateRawFee(int confTarget, double successThreshold, FeeEstimateHorizon horizon, EstimationResult* result) const +{ + TxConfirmStats* stats; + double sufficientTxs = SUFFICIENT_FEETXS; + switch (horizon) { + case FeeEstimateHorizon::SHORT_HALFLIFE: { + stats = shortStats; + sufficientTxs = SUFFICIENT_TXS_SHORT; + break; + } + case FeeEstimateHorizon::MED_HALFLIFE: { + stats = feeStats; + break; + } + case FeeEstimateHorizon::LONG_HALFLIFE: { + stats = longStats; + break; + } + default: { + return CFeeRate(0); + } + } + + LOCK(cs_feeEstimator); + // Return failure if trying to analyze a target we're not tracking + if (confTarget <= 0 || (unsigned int)confTarget > stats->GetMaxConfirms()) + return CFeeRate(0); + if (successThreshold > 1) + return CFeeRate(0); + + double median = stats->EstimateMedianVal(confTarget, sufficientTxs, successThreshold, true, nBestSeenHeight, result); if (median < 0) return CFeeRate(0); diff --git a/src/policy/fees.h b/src/policy/fees.h index c5955d7b04c..f42fe7bda79 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -61,6 +61,28 @@ class TxConfirmStats; * they've been outstanding. */ +enum FeeEstimateHorizon { + SHORT_HALFLIFE = 0, + MED_HALFLIFE = 1, + LONG_HALFLIFE = 2 +}; + +struct EstimatorBucket +{ + double start = -1; + double end = -1; + double withinTarget = 0; + double totalConfirmed = 0; + double inMempool = 0; +}; + +struct EstimationResult +{ + EstimatorBucket pass; + EstimatorBucket fail; + double decay; +}; + /** * We want to be able to estimate feerates that are needed on tx's to be included in * a certain number of blocks. Every time a block is added to the best chain, this class records @@ -90,6 +112,8 @@ private: /** Require an avg of 0.1 tx in the combined feerate bucket per block to have stat significance */ static constexpr double SUFFICIENT_FEETXS = 0.1; + /** Require an avg of 0.5 tx when using short decay since there are fewer blocks considered*/ + static constexpr double SUFFICIENT_TXS_SHORT = 0.5; /** Minimum and Maximum values for tracking feerates * The MIN_BUCKET_FEERATE should just be set to the lowest reasonable feerate we @@ -132,6 +156,10 @@ public: */ CFeeRate estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool) const; + /** Return a specific fee estimate calculation with a given success threshold and time horizon. + */ + CFeeRate estimateRawFee(int confTarget, double successThreshold, FeeEstimateHorizon horizon, EstimationResult *result = nullptr) const; + /** Write estimation data to a file */ bool Write(CAutoFile& fileout) const; @@ -190,4 +218,5 @@ private: std::set feeset; FastRandomContext insecure_rand; }; + #endif /*BITCOIN_POLICYESTIMATOR_H */ diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 941bdd93796..afc1fa1c79f 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -106,6 +106,9 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getrawmempool", 0, "verbose" }, { "estimatefee", 0, "nblocks" }, { "estimatesmartfee", 0, "nblocks" }, + { "estimaterawfee", 0, "nblocks" }, + { "estimaterawfee", 1, "threshold" }, + { "estimaterawfee", 2, "horizon" }, { "prioritisetransaction", 1, "fee_delta" }, { "setban", 2, "bantime" }, { "setban", 3, "absolute" }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 4ce52a6c7fa..6851f210046 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -863,6 +863,78 @@ UniValue estimatesmartfee(const JSONRPCRequest& request) return result; } +UniValue estimaterawfee(const JSONRPCRequest& request) +{ + if (request.fHelp || request.params.size() < 1|| request.params.size() > 3) + throw std::runtime_error( + "estimaterawfee nblocks (threshold horizon)\n" + "\nWARNING: This interface is unstable and may disappear or change!\n" + "\nWARNING: This is an advanced API call that is tightly coupled to the specific\n" + " implementation of fee estimation. The parameters it can be called with\n" + " and the results it returns will change if the internal implementation changes.\n" + "\nEstimates the approximate fee per kilobyte needed for a transaction to begin\n" + "confirmation within nblocks blocks if possible. Uses virtual transaction size as defined\n" + "in BIP 141 (witness data is discounted).\n" + "\nArguments:\n" + "1. nblocks (numeric)\n" + "2. threshold (numeric, optional) The proportion of transactions in a given feerate range that must have been\n" + " confirmed within nblocks in order to consider those feerates as high enough and proceed to check\n" + " lower buckets. Default: 0.95\n" + "3. horizon (numeric, optional) How long a history of estimates to consider. 0=short, 1=medium, 2=long.\n" + " Default: 1\n" + "\nResult:\n" + "{\n" + " \"feerate\" : x.x, (numeric) estimate fee-per-kilobyte (in BTC)\n" + " \"decay\" : x.x, (numeric) exponential decay (per block) for historical moving average of confirmation data\n" + " \"pass.\" information about the lowest range of feerates to succeed in meeting the threshold\n" + " \"fail.\" information about the highest range of feerates to fail to meet the threshold\n" + " \"startrange\" : x.x, (numeric) start of feerate range\n" + " \"endrange\" : x.x, (numeric) end of feerate range\n" + " \"withintarget\" : x.x, (numeric) number of txs over history horizon in the feerate range that were confirmed within target\n" + " \"totalconfirmed\" : x.x, (numeric) number of txs over history horizon in the feerate range that were confirmed at any point\n" + " \"inmempool\" : x.x, (numeric) current number of txs in mempool in the feerate range unconfirmed for at least target blocks\n" + "}\n" + "\n" + "A negative feerate is returned if no answer can be given.\n" + "\nExample:\n" + + HelpExampleCli("estimaterawfee", "6 0.9 1") + ); + + RPCTypeCheck(request.params, boost::assign::list_of(UniValue::VNUM)(UniValue::VNUM)(UniValue::VNUM), true); + RPCTypeCheckArgument(request.params[0], UniValue::VNUM); + int nBlocks = request.params[0].get_int(); + double threshold = 0.95; + if (!request.params[1].isNull()) + threshold = request.params[1].get_real(); + FeeEstimateHorizon horizon = FeeEstimateHorizon::MED_HALFLIFE; + if (!request.params[2].isNull()) { + int horizonInt = request.params[2].get_int(); + if (horizonInt < 0 || horizonInt > 2) { + throw JSONRPCError(RPC_TYPE_ERROR, "Invalid horizon for fee estimates"); + } else { + horizon = (FeeEstimateHorizon)horizonInt; + } + } + UniValue result(UniValue::VOBJ); + CFeeRate feeRate; + EstimationResult buckets; + feeRate = ::feeEstimator.estimateRawFee(nBlocks, threshold, horizon, &buckets); + + result.push_back(Pair("feerate", feeRate == CFeeRate(0) ? -1.0 : ValueFromAmount(feeRate.GetFeePerK()))); + result.push_back(Pair("decay", buckets.decay)); + result.push_back(Pair("pass.startrange", round(buckets.pass.start))); + result.push_back(Pair("pass.endrange", round(buckets.pass.end))); + result.push_back(Pair("pass.withintarget", round(buckets.pass.withinTarget * 100.0) / 100.0)); + result.push_back(Pair("pass.totalconfirmed", round(buckets.pass.totalConfirmed * 100.0) / 100.0)); + result.push_back(Pair("pass.inmempool", round(buckets.pass.inMempool * 100.0) / 100.0)); + result.push_back(Pair("fail.startrange", round(buckets.fail.start))); + result.push_back(Pair("fail.endrange", round(buckets.fail.end))); + result.push_back(Pair("fail.withintarget", round(buckets.fail.withinTarget * 100.0) / 100.0)); + result.push_back(Pair("fail.totalconfirmed", round(buckets.fail.totalConfirmed * 100.0) / 100.0)); + result.push_back(Pair("fail.inmempool", round(buckets.fail.inMempool * 100.0) / 100.0)); + return result; +} + static const CRPCCommand commands[] = { // category name actor (function) okSafeMode // --------------------- ------------------------ ----------------------- ---------- @@ -877,6 +949,8 @@ static const CRPCCommand commands[] = { "util", "estimatefee", &estimatefee, true, {"nblocks"} }, { "util", "estimatesmartfee", &estimatesmartfee, true, {"nblocks"} }, + + { "hidden", "estimaterawfee", &estimaterawfee, true, {"nblocks", "threshold", "horizon"} }, }; void RegisterMiningRPCCommands(CRPCTable &t) From c7447ec30348b338e77bc6429fbfac9f93549ef6 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 9 Mar 2017 15:26:05 -0500 Subject: [PATCH 07/15] Track failures in fee estimation. Start tracking transactions which fail to confirm within the target and are then evicted or otherwise leave mempool. Fix slight error in unit test. --- src/init.cpp | 1 + src/policy/fees.cpp | 72 ++++++++++++++++++++++++------ src/policy/fees.h | 6 ++- src/rpc/mining.cpp | 3 ++ src/test/policyestimator_tests.cpp | 8 ++-- src/txmempool.cpp | 2 +- 6 files changed, 72 insertions(+), 20 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index f06c9e11000..3a112fdc931 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -213,6 +213,7 @@ void Shutdown() if (fFeeEstimatesInitialized) { + ::feeEstimator.FlushUnconfirmed(::mempool); fs::path est_path = GetDataDir() / FEE_ESTIMATES_FILENAME; CAutoFile est_fileout(fsbridge::fopen(est_path, "wb"), SER_DISK, CLIENT_VERSION); if (!est_fileout.IsNull()) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index a9a8335c617..e9d137e7f05 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -40,7 +40,9 @@ private: // Track the historical moving average of theses totals over blocks std::vector> confAvg; // confAvg[Y][X] - std::vector> failAvg; // future use + // Track moving avg of txs which have been evicted from the mempool + // after failing to be confirmed within Y blocks + std::vector> failAvg; // failAvg[Y][X] // Sum the total feerate of all tx's in each bucket // Track the historical moving average of this total over blocks @@ -89,7 +91,7 @@ public: /** Remove a transaction from mempool tracking stats*/ void removeTx(unsigned int entryHeight, unsigned int nBestSeenHeight, - unsigned int bucketIndex); + unsigned int bucketIndex, bool inBlock); /** Update our estimates by decaying our historical moving average and updating with the data gathered from the current block */ @@ -135,6 +137,10 @@ TxConfirmStats::TxConfirmStats(const std::vector& defaultBuckets, for (unsigned int i = 0; i < maxConfirms; i++) { confAvg[i].resize(buckets.size()); } + failAvg.resize(maxConfirms); + for (unsigned int i = 0; i < maxConfirms; i++) { + failAvg[i].resize(buckets.size()); + } txCtAvg.resize(buckets.size()); avg.resize(buckets.size()); @@ -179,6 +185,8 @@ void TxConfirmStats::UpdateMovingAverages() for (unsigned int j = 0; j < buckets.size(); j++) { for (unsigned int i = 0; i < confAvg.size(); i++) confAvg[i][j] = confAvg[i][j] * decay; + for (unsigned int i = 0; i < failAvg.size(); i++) + failAvg[i][j] = failAvg[i][j] * decay; avg[j] = avg[j] * decay; txCtAvg[j] = txCtAvg[j] * decay; } @@ -193,6 +201,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, double nConf = 0; // Number of tx's confirmed within the confTarget double totalNum = 0; // Total number of tx's that were ever confirmed int extraNum = 0; // Number of tx's still in mempool for confTarget or longer + double failNum = 0; // Number of tx's that were never confirmed but removed from the mempool after confTarget int maxbucketindex = buckets.size() - 1; @@ -229,6 +238,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, curFarBucket = bucket; nConf += confAvg[confTarget - 1][bucket]; totalNum += txCtAvg[bucket]; + failNum += failAvg[confTarget - 1][bucket]; for (unsigned int confct = confTarget; confct < GetMaxConfirms(); confct++) extraNum += unconfTxs[(nBlockHeight - confct)%bins][bucket]; extraNum += oldUnconfTxs[bucket]; @@ -237,7 +247,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, // (Only count the confirmed data points, so that each confirmation count // will be looking at the same amount of data and same bucket breaks) if (totalNum >= sufficientTxVal / (1 - decay)) { - double curPct = nConf / (totalNum + extraNum); + double curPct = nConf / (totalNum + failNum + extraNum); // Check to see if we are no longer getting confirmed at the success rate if ((requireGreater && curPct < successBreakPoint) || (!requireGreater && curPct > successBreakPoint)) { @@ -250,6 +260,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, failBucket.withinTarget = nConf; failBucket.totalConfirmed = totalNum; failBucket.inMempool = extraNum; + failBucket.leftMempool = failNum; passing = false; } continue; @@ -265,6 +276,8 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, passBucket.totalConfirmed = totalNum; totalNum = 0; passBucket.inMempool = extraNum; + passBucket.leftMempool = failNum; + failNum = 0; extraNum = 0; bestNearBucket = curNearBucket; bestFarBucket = curFarBucket; @@ -309,16 +322,17 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, failBucket.withinTarget = nConf; failBucket.totalConfirmed = totalNum; failBucket.inMempool = extraNum; + failBucket.leftMempool = failNum; } - LogPrint(BCLog::ESTIMATEFEE, "FeeEst: %d %s%.0f%% decay %.5f: need feerate: %g from (%g - %g) %.2f%% %.1f/(%.1f+%d mem) Fail: (%g - %g) %.2f%% %.1f/(%.1f+%d mem)\n", + LogPrint(BCLog::ESTIMATEFEE, "FeeEst: %d %s%.0f%% decay %.5f: need feerate: %g from (%g - %g) %.2f%% %.1f/(%.1f+%d mem+%.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f+%d mem+%.1f out)\n", confTarget, requireGreater ? ">" : "<", 100.0 * successBreakPoint, decay, median, passBucket.start, passBucket.end, - 100 * passBucket.withinTarget / (passBucket.totalConfirmed + passBucket.inMempool), - passBucket.withinTarget, passBucket.totalConfirmed, passBucket.inMempool, + 100 * passBucket.withinTarget / (passBucket.totalConfirmed + passBucket.inMempool + passBucket.leftMempool), + passBucket.withinTarget, passBucket.totalConfirmed, passBucket.inMempool, passBucket.leftMempool, failBucket.start, failBucket.end, - 100 * failBucket.withinTarget / (failBucket.totalConfirmed + failBucket.inMempool), - failBucket.withinTarget, failBucket.totalConfirmed, failBucket.inMempool); + 100 * failBucket.withinTarget / (failBucket.totalConfirmed + failBucket.inMempool + failBucket.leftMempool), + failBucket.withinTarget, failBucket.totalConfirmed, failBucket.inMempool, failBucket.leftMempool); if (result) { @@ -376,6 +390,19 @@ void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets if (nFileVersion >= 149900) { filein >> failAvg; + if (maxConfirms != failAvg.size()) { + throw std::runtime_error("Corrupt estimates file. Mismatch in confirms tracked for failures"); + } + for (unsigned int i = 0; i < maxConfirms; i++) { + if (failAvg[i].size() != numBuckets) { + throw std::runtime_error("Corrupt estimates file. Mismatch in one of failure average bucket counts"); + } + } + } else { + failAvg.resize(confAvg.size()); + for (unsigned int i = 0; i < failAvg.size(); i++) { + failAvg[i].resize(numBuckets); + } } // Resize the current block variables which aren't stored in the data file @@ -394,7 +421,7 @@ unsigned int TxConfirmStats::NewTx(unsigned int nBlockHeight, double val) return bucketindex; } -void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHeight, unsigned int bucketindex) +void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHeight, unsigned int bucketindex, bool inBlock) { //nBestSeenHeight is not updated yet for the new block int blocksAgo = nBestSeenHeight - entryHeight; @@ -422,6 +449,11 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe blockIndex, bucketindex); } } + if (!inBlock && blocksAgo >= 1) { + for (size_t i = 0; i < blocksAgo && i < failAvg.size(); i++) { + failAvg[i][bucketindex]++; + } + } } // This function is called from CTxMemPool::removeUnchecked to ensure @@ -429,14 +461,14 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe // tracked. Txs that were part of a block have already been removed in // processBlockTx to ensure they are never double tracked, but it is // of no harm to try to remove them again. -bool CBlockPolicyEstimator::removeTx(uint256 hash) +bool CBlockPolicyEstimator::removeTx(uint256 hash, bool inBlock) { LOCK(cs_feeEstimator); std::map::iterator pos = mapMemPoolTxs.find(hash); if (pos != mapMemPoolTxs.end()) { - feeStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex); - shortStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex); - longStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex); + feeStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex, inBlock); + shortStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex, inBlock); + longStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex, inBlock); mapMemPoolTxs.erase(hash); return true; } else { @@ -511,7 +543,7 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry) { - if (!removeTx(entry->GetTx().GetHash())) { + if (!removeTx(entry->GetTx().GetHash(), true)) { // This transaction wasn't being tracked for fee estimation return false; } @@ -766,6 +798,18 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) return true; } +void CBlockPolicyEstimator::FlushUnconfirmed(CTxMemPool& pool) { + int64_t startclear = GetTimeMicros(); + std::vector txids; + pool.queryHashes(txids); + LOCK(cs_feeEstimator); + for (auto& txid : txids) { + removeTx(txid, false); + } + int64_t endclear = GetTimeMicros(); + LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %ld micros\n",txids.size(), endclear - startclear); +} + FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee) { CAmount minFeeLimit = std::max(CAmount(1), minIncrementalFee.GetFeePerK() / 2); diff --git a/src/policy/fees.h b/src/policy/fees.h index f42fe7bda79..03adbac4d2c 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -74,6 +74,7 @@ struct EstimatorBucket double withinTarget = 0; double totalConfirmed = 0; double inMempool = 0; + double leftMempool = 0; }; struct EstimationResult @@ -145,7 +146,7 @@ public: void processTransaction(const CTxMemPoolEntry& entry, bool validFeeEstimate); /** Remove a transaction from the mempool tracking stats*/ - bool removeTx(uint256 hash); + bool removeTx(uint256 hash, bool inBlock); /** Return a feerate estimate */ CFeeRate estimateFee(int confTarget) const; @@ -166,6 +167,9 @@ public: /** Read estimation data from a file */ bool Read(CAutoFile& filein); + /** Empty mempool transactions on shutdown to record failure to confirm for txs still in mempool */ + void FlushUnconfirmed(CTxMemPool& pool); + private: CFeeRate minTrackedFee; //!< Passed to constructor to avoid dependency on main unsigned int nBestSeenHeight; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 6851f210046..74e5ba5f44c 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -893,6 +893,7 @@ UniValue estimaterawfee(const JSONRPCRequest& request) " \"withintarget\" : x.x, (numeric) number of txs over history horizon in the feerate range that were confirmed within target\n" " \"totalconfirmed\" : x.x, (numeric) number of txs over history horizon in the feerate range that were confirmed at any point\n" " \"inmempool\" : x.x, (numeric) current number of txs in mempool in the feerate range unconfirmed for at least target blocks\n" + " \"leftmempool\" : x.x, (numeric) number of txs over history horizon in the feerate range that left mempool unconfirmed after target\n" "}\n" "\n" "A negative feerate is returned if no answer can be given.\n" @@ -927,11 +928,13 @@ UniValue estimaterawfee(const JSONRPCRequest& request) result.push_back(Pair("pass.withintarget", round(buckets.pass.withinTarget * 100.0) / 100.0)); result.push_back(Pair("pass.totalconfirmed", round(buckets.pass.totalConfirmed * 100.0) / 100.0)); result.push_back(Pair("pass.inmempool", round(buckets.pass.inMempool * 100.0) / 100.0)); + result.push_back(Pair("pass.leftmempool", round(buckets.pass.leftMempool * 100.0) / 100.0)); result.push_back(Pair("fail.startrange", round(buckets.fail.start))); result.push_back(Pair("fail.endrange", round(buckets.fail.end))); result.push_back(Pair("fail.withintarget", round(buckets.fail.withinTarget * 100.0) / 100.0)); result.push_back(Pair("fail.totalconfirmed", round(buckets.fail.totalConfirmed * 100.0) / 100.0)); result.push_back(Pair("fail.inmempool", round(buckets.fail.inMempool * 100.0) / 100.0)); + result.push_back(Pair("fail.leftmempool", round(buckets.fail.leftMempool * 100.0) / 100.0)); return result; } diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 942efce0f76..94de72ba47d 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -159,16 +159,16 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) txHashes[j].pop_back(); } } - mpool.removeForBlock(block, 265); + mpool.removeForBlock(block, 266); block.clear(); BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0)); for (int i = 2; i < 10;i++) { - BOOST_CHECK(feeEst.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee); + BOOST_CHECK(feeEst.estimateFee(i) == CFeeRate(0) || feeEst.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee); } - // Mine 200 more blocks where everything is mined every block + // Mine 400 more blocks where everything is mined every block // Estimates should be below original estimates - while (blocknum < 465) { + while (blocknum < 665) { for (int j = 0; j < 10; j++) { // For each fee multiple for (int k = 0; k < 4; k++) { // add 4 fee txs tx.vin[0].prevout.n = 10000*blocknum+100*j+k; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index ac842da6bf3..a83805267b9 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -448,7 +448,7 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) mapLinks.erase(it); mapTx.erase(it); nTransactionsUpdated++; - if (minerPolicyEstimator) {minerPolicyEstimator->removeTx(hash);} + if (minerPolicyEstimator) {minerPolicyEstimator->removeTx(hash, false);} } // Calculates descendants of entry that are not already in setDescendants, and adds to From 3810e976d6a3956dff9e66077415cf04c1fe1f90 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 7 Mar 2017 11:33:44 -0500 Subject: [PATCH 08/15] Rewrite estimateSmartFee Change the logic of estimateSmartFee to check a 60% threshold at half the target, a 85% threshold at the target and a 95% threshold at double the target. Always check the shortest time horizon possible and ensure that estimates are monotonically decreasing. Add a conservative mode, which makes sure that the 95% threshold is also met at longer time horizons as well. --- src/policy/fees.cpp | 89 ++++++++++++++++++++++++++++-- src/policy/fees.h | 4 +- src/rpc/client.cpp | 1 + src/rpc/mining.cpp | 19 +++++-- src/test/policyestimator_tests.cpp | 9 +-- test/functional/smartfees.py | 4 +- 6 files changed, 104 insertions(+), 22 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index e9d137e7f05..2b22d59ffb6 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -658,31 +658,107 @@ CFeeRate CBlockPolicyEstimator::estimateRawFee(int confTarget, double successThr return CFeeRate(median); } -CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool) const + +/** Return a fee estimate at the required successThreshold from the shortest + * time horizon which tracks confirmations up to the desired target. If + * checkShorterHorizon is requested, also allow short time horizon estimates + * for a lower target to reduce the given answer */ +double CBlockPolicyEstimator::estimateCombinedFee(unsigned int confTarget, double successThreshold, bool checkShorterHorizon) const +{ + double estimate = -1; + if (confTarget >= 1 && confTarget <= longStats->GetMaxConfirms()) { + // Find estimate from shortest time horizon possible + if (confTarget <= shortStats->GetMaxConfirms()) { // short horizon + estimate = shortStats->EstimateMedianVal(confTarget, SUFFICIENT_TXS_SHORT, successThreshold, true, nBestSeenHeight); + } + else if (confTarget <= feeStats->GetMaxConfirms()) { // medium horizon + estimate = feeStats->EstimateMedianVal(confTarget, SUFFICIENT_FEETXS, successThreshold, true, nBestSeenHeight); + } + else { // long horizon + estimate = longStats->EstimateMedianVal(confTarget, SUFFICIENT_FEETXS, successThreshold, true, nBestSeenHeight); + } + if (checkShorterHorizon) { + // If a lower confTarget from a more recent horizon returns a lower answer use it. + if (confTarget > feeStats->GetMaxConfirms()) { + double medMax = feeStats->EstimateMedianVal(feeStats->GetMaxConfirms(), SUFFICIENT_FEETXS, successThreshold, true, nBestSeenHeight); + if (medMax > 0 && (estimate == -1 || medMax < estimate)) + estimate = medMax; + } + if (confTarget > shortStats->GetMaxConfirms()) { + double shortMax = shortStats->EstimateMedianVal(shortStats->GetMaxConfirms(), SUFFICIENT_TXS_SHORT, successThreshold, true, nBestSeenHeight); + if (shortMax > 0 && (estimate == -1 || shortMax < estimate)) + estimate = shortMax; + } + } + } + return estimate; +} + +double CBlockPolicyEstimator::estimateConservativeFee(unsigned int doubleTarget) const +{ + double estimate = -1; + if (doubleTarget <= shortStats->GetMaxConfirms()) { + estimate = feeStats->EstimateMedianVal(doubleTarget, SUFFICIENT_FEETXS, DOUBLE_SUCCESS_PCT, true, nBestSeenHeight); + } + if (doubleTarget <= feeStats->GetMaxConfirms()) { + double longEstimate = longStats->EstimateMedianVal(doubleTarget, SUFFICIENT_FEETXS, DOUBLE_SUCCESS_PCT, true, nBestSeenHeight); + if (longEstimate > estimate) { + estimate = longEstimate; + } + } + return estimate; +} + +CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool, bool conservative) const { if (answerFoundAtTarget) *answerFoundAtTarget = confTarget; double median = -1; - { 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 > longStats->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, DOUBLE_SUCCESS_PCT, true, nBestSeenHeight); + assert(confTarget > 0); //estimateCombinedFee and estimateConservativeFee take unsigned ints + + /** true is passed to estimateCombined fee for target/2 and target so + * that we check the max confirms for shorter time horizons as well. + * This is necessary to preserve monotonically increasing estimates. + * For non-conservative estimates we do the same thing for 2*target, but + * for conservative estimates we want to skip these shorter horizons + * checks for 2*target becuase we are taking the max over all time + * horizons so we already have monotonically increasing estimates and + * the purpose of conservative estimates is not to let short term + * fluctuations lower our estimates by too much. + */ + double halfEst = estimateCombinedFee(confTarget/2, HALF_SUCCESS_PCT, true); + double actualEst = estimateCombinedFee(confTarget, SUCCESS_PCT, true); + double doubleEst = estimateCombinedFee(2 * confTarget, DOUBLE_SUCCESS_PCT, !conservative); + median = halfEst; + if (actualEst > median) { + median = actualEst; + } + if (doubleEst > median) { + median = doubleEst; + } + + if (conservative || median == -1) { + double consEst = estimateConservativeFee(2 * confTarget); + if (consEst > median) { + median = consEst; + } } } // Must unlock cs_feeEstimator before taking mempool locks if (answerFoundAtTarget) - *answerFoundAtTarget = confTarget - 1; + *answerFoundAtTarget = confTarget; // If mempool is limiting txs , return at least the min feerate from the mempool CAmount minPoolFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); @@ -695,6 +771,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun return CFeeRate(median); } + bool CBlockPolicyEstimator::Write(CAutoFile& fileout) const { try { diff --git a/src/policy/fees.h b/src/policy/fees.h index 03adbac4d2c..7064ad15c13 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -155,7 +155,7 @@ public: * 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) const; + CFeeRate estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool, bool conservative = true) const; /** Return a specific fee estimate calculation with a given success threshold and time horizon. */ @@ -199,6 +199,8 @@ private: /** Process a transaction confirmed in a block*/ bool processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry); + double estimateCombinedFee(unsigned int confTarget, double successThreshold, bool checkShorterHorizon) const; + double estimateConservativeFee(unsigned int doubleTarget) const; }; class FeeFilterRounder diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index afc1fa1c79f..455d1dc722d 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -106,6 +106,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getrawmempool", 0, "verbose" }, { "estimatefee", 0, "nblocks" }, { "estimatesmartfee", 0, "nblocks" }, + { "estimatesmartfee", 1, "conservative" }, { "estimaterawfee", 0, "nblocks" }, { "estimaterawfee", 1, "threshold" }, { "estimaterawfee", 2, "horizon" }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 74e5ba5f44c..9f6cab861ca 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -828,16 +828,20 @@ UniValue estimatefee(const JSONRPCRequest& request) UniValue estimatesmartfee(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() != 1) + if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) throw std::runtime_error( - "estimatesmartfee nblocks\n" + "estimatesmartfee nblocks (conservative)\n" "\nWARNING: This interface is unstable and may disappear or change!\n" "\nEstimates the approximate fee per kilobyte needed for a transaction to begin\n" "confirmation within nblocks blocks if possible and return the number of blocks\n" "for which the estimate is valid. Uses virtual transaction size as defined\n" "in BIP 141 (witness data is discounted).\n" "\nArguments:\n" - "1. nblocks (numeric)\n" + "1. nblocks (numeric)\n" + "2. conservative (bool, optional, default=true) Whether to return a more conservative estimate which\n" + " also satisfies a longer history. A conservative estimate potentially returns a higher\n" + " feerate and is more likely to be sufficient for the desired target, but is not as\n" + " responsive to short term drops in the prevailing fee market\n" "\nResult:\n" "{\n" " \"feerate\" : x.x, (numeric) estimate fee-per-kilobyte (in BTC)\n" @@ -854,10 +858,15 @@ UniValue estimatesmartfee(const JSONRPCRequest& request) RPCTypeCheck(request.params, boost::assign::list_of(UniValue::VNUM)); int nBlocks = request.params[0].get_int(); + bool conservative = true; + if (request.params.size() > 1 && !request.params[1].isNull()) { + RPCTypeCheckArgument(request.params[1], UniValue::VBOOL); + conservative = request.params[1].get_bool(); + } UniValue result(UniValue::VOBJ); int answerFound; - CFeeRate feeRate = ::feeEstimator.estimateSmartFee(nBlocks, &answerFound, ::mempool); + CFeeRate feeRate = ::feeEstimator.estimateSmartFee(nBlocks, &answerFound, ::mempool, conservative); result.push_back(Pair("feerate", feeRate == CFeeRate(0) ? -1.0 : ValueFromAmount(feeRate.GetFeePerK()))); result.push_back(Pair("blocks", answerFound)); return result; @@ -951,7 +960,7 @@ static const CRPCCommand commands[] = { "generating", "generatetoaddress", &generatetoaddress, true, {"nblocks","address","maxtries"} }, { "util", "estimatefee", &estimatefee, true, {"nblocks"} }, - { "util", "estimatesmartfee", &estimatesmartfee, true, {"nblocks"} }, + { "util", "estimatesmartfee", &estimatesmartfee, true, {"nblocks", "conservative"} }, { "hidden", "estimaterawfee", &estimaterawfee, true, {"nblocks", "threshold", "horizon"} }, }; diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 94de72ba47d..0d7ca1251fa 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -83,11 +83,6 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0)); BOOST_CHECK(feeEst.estimateFee(2).GetFeePerK() < 9*baseRate.GetFeePerK() + deltaFee); BOOST_CHECK(feeEst.estimateFee(2).GetFeePerK() > 9*baseRate.GetFeePerK() - deltaFee); - int answerFound; - BOOST_CHECK(feeEst.estimateSmartFee(1, &answerFound, mpool) == feeEst.estimateFee(2) && answerFound == 2); - BOOST_CHECK(feeEst.estimateSmartFee(2, &answerFound, mpool) == feeEst.estimateFee(2) && answerFound == 2); - BOOST_CHECK(feeEst.estimateSmartFee(4, &answerFound, mpool) == feeEst.estimateFee(4) && answerFound == 4); - BOOST_CHECK(feeEst.estimateSmartFee(8, &answerFound, mpool) == feeEst.estimateFee(8) && answerFound == 8); } } @@ -143,10 +138,8 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) mpool.removeForBlock(block, ++blocknum); } - int answerFound; for (int i = 1; i < 10;i++) { 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 @@ -194,7 +187,7 @@ 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(feeEst.estimateSmartFee(i, NULL, mpool).GetFeePerK() >= feeEst.estimateFee(i).GetFeePerK()); + BOOST_CHECK(feeEst.estimateSmartFee(i, NULL, mpool).GetFeePerK() >= feeEst.estimateRawFee(i, 0.85, FeeEstimateHorizon::MED_HALFLIFE).GetFeePerK()); BOOST_CHECK(feeEst.estimateSmartFee(i, NULL, mpool).GetFeePerK() >= mpool.GetMinFee(1).GetFeePerK()); } } diff --git a/test/functional/smartfees.py b/test/functional/smartfees.py index b7f4b86e7b4..aafdb148462 100755 --- a/test/functional/smartfees.py +++ b/test/functional/smartfees.py @@ -116,8 +116,8 @@ def check_estimates(node, fees_seen, max_invalid, print_estimates = True): for i,e in enumerate(all_estimates): # estimate is for i+1 if e >= 0: valid_estimate = True - # estimatesmartfee should return the same result - assert_equal(node.estimatesmartfee(i+1)["feerate"], e) + if i >= 13: # for n>=14 estimatesmartfee(n/2) should be at least as high as estimatefee(n) + assert(node.estimatesmartfee((i+1)//2)["feerate"] > float(e) - delta) else: invalid_estimates += 1 From 10f7cbd2471ae289d2846e09c4b088fdc0330c8f Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 7 Mar 2017 15:01:50 -0500 Subject: [PATCH 09/15] Track first recorded height Track the first time we seen txs in a block that we have been tracking in our mempool. Used to evaluate validity of fee estimates for different targets. --- src/policy/fees.cpp | 7 ++++++- src/policy/fees.h | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 2b22d59ffb6..cba063aea90 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -477,7 +477,7 @@ bool CBlockPolicyEstimator::removeTx(uint256 hash, bool inBlock) } CBlockPolicyEstimator::CBlockPolicyEstimator() - : nBestSeenHeight(0), trackedTxs(0), untrackedTxs(0) + : nBestSeenHeight(0), firstRecordedHeight(0), trackedTxs(0), untrackedTxs(0) { static_assert(MIN_BUCKET_FEERATE > 0, "Min feerate must be nonzero"); minTrackedFee = CFeeRate(MIN_BUCKET_FEERATE); @@ -603,6 +603,11 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, countedTxs++; } + if (firstRecordedHeight == 0 && countedTxs > 0) { + firstRecordedHeight = nBestSeenHeight; + LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy first recorded height %u\n", firstRecordedHeight); + } + 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()); diff --git a/src/policy/fees.h b/src/policy/fees.h index 7064ad15c13..3184aa08aba 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -173,6 +173,10 @@ public: private: CFeeRate minTrackedFee; //!< Passed to constructor to avoid dependency on main unsigned int nBestSeenHeight; + unsigned int firstRecordedHeight; + unsigned int historicalFirst; + unsigned int historicalBest; + struct TxStatsInfo { unsigned int blockHeight; From aa19b8ea44b292cd612c4ce01e08c83324fa8296 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 10 Mar 2017 16:57:21 -0500 Subject: [PATCH 10/15] Clean up fee estimate debug printing --- src/policy/fees.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index cba063aea90..1b8ef85282e 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -325,7 +325,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, failBucket.leftMempool = failNum; } - LogPrint(BCLog::ESTIMATEFEE, "FeeEst: %d %s%.0f%% decay %.5f: need feerate: %g from (%g - %g) %.2f%% %.1f/(%.1f+%d mem+%.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f+%d mem+%.1f out)\n", + LogPrint(BCLog::ESTIMATEFEE, "FeeEst: %d %s%.0f%% decay %.5f: feerate: %g from (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n", confTarget, requireGreater ? ">" : "<", 100.0 * successBreakPoint, decay, median, passBucket.start, passBucket.end, 100 * passBucket.withinTarget / (passBucket.totalConfirmed + passBucket.inMempool + passBucket.leftMempool), From 5f1f0c64905491fea4ba50c0a8eb68fa59d65411 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 10 Mar 2017 16:57:40 -0500 Subject: [PATCH 11/15] Historical block span Store in fee estimate file the block span for which we were tracking estimates, so we know what targets we can successfully evaluate with the data in the file. When restarting use either this historical block span to set valid range of targets until our current span of tracking estimates is just as long. --- src/policy/fees.cpp | 57 ++++++++++++++++++++++++++++++++++++++------- src/policy/fees.h | 5 ++++ 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 1b8ef85282e..74ed0027246 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -477,7 +477,7 @@ bool CBlockPolicyEstimator::removeTx(uint256 hash, bool inBlock) } CBlockPolicyEstimator::CBlockPolicyEstimator() - : nBestSeenHeight(0), firstRecordedHeight(0), trackedTxs(0), untrackedTxs(0) + : nBestSeenHeight(0), firstRecordedHeight(0), historicalFirst(0), historicalBest(0), trackedTxs(0), untrackedTxs(0) { static_assert(MIN_BUCKET_FEERATE > 0, "Min feerate must be nonzero"); minTrackedFee = CFeeRate(MIN_BUCKET_FEERATE); @@ -609,8 +609,9 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, } - 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()); + LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy estimates updated by %u of %u block txs, since last block %u of %u tracked, mempool map size %u, max target %u from %s\n", + countedTxs, entries.size(), trackedTxs, trackedTxs + untrackedTxs, mapMemPoolTxs.size(), + MaxUsableEstimate(), HistoricalBlockSpan() > BlockSpan() ? "historical" : "current"); trackedTxs = 0; untrackedTxs = 0; @@ -663,6 +664,29 @@ CFeeRate CBlockPolicyEstimator::estimateRawFee(int confTarget, double successThr return CFeeRate(median); } +unsigned int CBlockPolicyEstimator::BlockSpan() const +{ + if (firstRecordedHeight == 0) return 0; + assert(nBestSeenHeight >= firstRecordedHeight); + + return nBestSeenHeight - firstRecordedHeight; +} + +unsigned int CBlockPolicyEstimator::HistoricalBlockSpan() const +{ + if (historicalFirst == 0) return 0; + assert(historicalBest >= historicalFirst); + + if (nBestSeenHeight - historicalBest > OLDEST_ESTIMATE_HISTORY) return 0; + + return historicalBest - historicalFirst; +} + +unsigned int CBlockPolicyEstimator::MaxUsableEstimate() const +{ + // Block spans are divided by 2 to make sure there are enough potential failing data points for the estimate + return std::min(longStats->GetMaxConfirms(), std::max(BlockSpan(), HistoricalBlockSpan()) / 2); +} /** Return a fee estimate at the required successThreshold from the shortest * time horizon which tracks confirmations up to the desired target. If @@ -731,6 +755,14 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun if (confTarget == 1) confTarget = 2; + unsigned int maxUsableEstimate = MaxUsableEstimate(); + if (maxUsableEstimate <= 1) + return CFeeRate(0); + + if ((unsigned int)confTarget > maxUsableEstimate) { + confTarget = maxUsableEstimate; + } + assert(confTarget > 0); //estimateCombinedFee and estimateConservativeFee take unsigned ints /** true is passed to estimateCombined fee for target/2 and target so @@ -784,8 +816,12 @@ bool CBlockPolicyEstimator::Write(CAutoFile& fileout) const fileout << 149900; // version required to read: 0.14.99 or later fileout << CLIENT_VERSION; // version that wrote the file fileout << nBestSeenHeight; - unsigned int future1 = 0, future2 = 0; - fileout << future1 << future2; + if (BlockSpan() > HistoricalBlockSpan()/2) { + fileout << firstRecordedHeight << nBestSeenHeight; + } + else { + fileout << historicalFirst << historicalBest; + } fileout << buckets; feeStats->Write(fileout); shortStats->Write(fileout); @@ -803,7 +839,7 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) try { LOCK(cs_feeEstimator); int nVersionRequired, nVersionThatWrote; - unsigned int nFileBestSeenHeight; + unsigned int nFileBestSeenHeight, nFileHistoricalFirst, nFileHistoricalBest; filein >> nVersionRequired >> nVersionThatWrote; if (nVersionRequired > CLIENT_VERSION) return error("CBlockPolicyEstimator::Read(): up-version (%d) fee estimate file", nVersionRequired); @@ -838,9 +874,10 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) } } else { // nVersionThatWrote >= 149900 - unsigned int future1, future2; - filein >> future1 >> future2; - + filein >> nFileHistoricalFirst >> nFileHistoricalBest; + if (nFileHistoricalFirst > nFileHistoricalBest || nFileHistoricalBest > nFileBestSeenHeight) { + throw std::runtime_error("Corrupt estimates file. Historical block range for estimates is invalid"); + } std::vector fileBuckets; filein >> fileBuckets; size_t numBuckets = fileBuckets.size(); @@ -871,6 +908,8 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) longStats = fileLongStats.release(); nBestSeenHeight = nFileBestSeenHeight; + historicalFirst = nFileHistoricalFirst; + historicalBest = nFileHistoricalBest; } } catch (const std::exception& e) { diff --git a/src/policy/fees.h b/src/policy/fees.h index 3184aa08aba..1f752415e72 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -98,6 +98,8 @@ private: static constexpr unsigned int MED_BLOCK_CONFIRMS = 48; /** Track confirm delays up to 1008 blocks for longer decay */ static constexpr unsigned int LONG_BLOCK_CONFIRMS = 1008; + /** Historical estimates that are older than this aren't valid */ + static const unsigned int OLDEST_ESTIMATE_HISTORY = 6 * 1008; /** Decay of .962 is a half-life of 18 blocks or about 3 hours */ static constexpr double SHORT_DECAY = .962; @@ -205,6 +207,9 @@ private: double estimateCombinedFee(unsigned int confTarget, double successThreshold, bool checkShorterHorizon) const; double estimateConservativeFee(unsigned int doubleTarget) const; + unsigned int BlockSpan() const; + unsigned int HistoricalBlockSpan() const; + unsigned int MaxUsableEstimate() const; }; class FeeFilterRounder From 3ee76d6de5c8f8705c4b9bbcc75da0df4d42c966 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Mon, 3 Apr 2017 11:31:27 -0400 Subject: [PATCH 12/15] Introduce a scale factor For the per confirmation number tracking of data, introduce a scale factor so that in the longer horizones confirmations are bucketed together at a resolution of the scale. (instead of 1008 individual data points for each fee bucket, have 42 data points each covering 24 different confirmation values.. (1-24), (25-48), etc.. ) --- src/policy/fees.cpp | 61 +++++++++++++++++------------- src/policy/fees.h | 10 +++-- src/rpc/mining.cpp | 2 + src/test/policyestimator_tests.cpp | 7 +--- 4 files changed, 45 insertions(+), 35 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 74ed0027246..44adb26d4cc 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -53,13 +53,14 @@ private: double decay; + // Resolution (# of blocks) with which confirmations are tracked unsigned int scale; // 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 + // transactions still unconfirmed after GetMaxConfirms for each bucket std::vector oldUnconfTxs; void resizeInMemoryCounters(size_t newbuckets); @@ -73,7 +74,7 @@ public: * @param decay how much to decay the historical moving average per block */ TxConfirmStats(const std::vector& defaultBuckets, const std::map& defaultBucketMap, - unsigned int maxConfirms, double decay); + unsigned int maxPeriods, double decay, unsigned int scale); /** Roll the circular buffer for unconfirmed txs*/ void ClearCurrent(unsigned int nBlockHeight); @@ -113,7 +114,7 @@ public: EstimationResult *result = nullptr) const; /** Return the max number of confirms we're tracking */ - unsigned int GetMaxConfirms() const { return confAvg.size(); } + unsigned int GetMaxConfirms() const { return scale * confAvg.size(); } /** Write state of estimation data to a file*/ void Write(CAutoFile& fileout) const; @@ -128,17 +129,17 @@ public: TxConfirmStats::TxConfirmStats(const std::vector& defaultBuckets, const std::map& defaultBucketMap, - unsigned int maxConfirms, double _decay) + unsigned int maxPeriods, double _decay, unsigned int _scale) : buckets(defaultBuckets), bucketMap(defaultBucketMap) { decay = _decay; - scale = 1; - confAvg.resize(maxConfirms); - for (unsigned int i = 0; i < maxConfirms; i++) { + scale = _scale; + confAvg.resize(maxPeriods); + for (unsigned int i = 0; i < maxPeriods; i++) { confAvg[i].resize(buckets.size()); } - failAvg.resize(maxConfirms); - for (unsigned int i = 0; i < maxConfirms; i++) { + failAvg.resize(maxPeriods); + for (unsigned int i = 0; i < maxPeriods; i++) { failAvg[i].resize(buckets.size()); } @@ -172,8 +173,9 @@ void TxConfirmStats::Record(int blocksToConfirm, double val) // blocksToConfirm is 1-based if (blocksToConfirm < 1) return; + int periodsToConfirm = (blocksToConfirm + scale - 1)/scale; unsigned int bucketindex = bucketMap.lower_bound(val)->second; - for (size_t i = blocksToConfirm; i <= confAvg.size(); i++) { + for (size_t i = periodsToConfirm; i <= confAvg.size(); i++) { confAvg[i - 1][bucketindex]++; } txCtAvg[bucketindex]++; @@ -202,6 +204,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, double totalNum = 0; // Total number of tx's that were ever confirmed int extraNum = 0; // Number of tx's still in mempool for confTarget or longer double failNum = 0; // Number of tx's that were never confirmed but removed from the mempool after confTarget + int periodTarget = (confTarget + scale - 1)/scale; int maxbucketindex = buckets.size() - 1; @@ -236,9 +239,9 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, newBucketRange = false; } curFarBucket = bucket; - nConf += confAvg[confTarget - 1][bucket]; + nConf += confAvg[periodTarget - 1][bucket]; totalNum += txCtAvg[bucket]; - failNum += failAvg[confTarget - 1][bucket]; + failNum += failAvg[periodTarget - 1][bucket]; for (unsigned int confct = confTarget; confct < GetMaxConfirms(); confct++) extraNum += unconfTxs[(nBlockHeight - confct)%bins][bucket]; extraNum += oldUnconfTxs[bucket]; @@ -339,6 +342,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, result->pass = passBucket; result->fail = failBucket; result->decay = decay; + result->scale = scale; } return median; } @@ -358,7 +362,7 @@ void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets // Read data file and do some very basic sanity checking // buckets and bucketMap are not updated yet, so don't access them // If there is a read failure, we'll just discard this entire object anyway - size_t maxConfirms; + size_t maxConfirms, maxPeriods; // The current version will store the decay with each individual TxConfirmStats and also keep a scale factor if (nFileVersion >= 149900) { @@ -366,7 +370,7 @@ void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets if (decay <= 0 || decay >= 1) { throw std::runtime_error("Corrupt estimates file. Decay must be between 0 and 1 (non-inclusive)"); } - filein >> scale; //Unused for now + filein >> scale; } filein >> avg; @@ -378,11 +382,13 @@ void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets throw std::runtime_error("Corrupt estimates file. Mismatch in tx count bucket count"); } filein >> confAvg; - maxConfirms = confAvg.size(); + maxPeriods = confAvg.size(); + maxConfirms = scale * maxPeriods; + if (maxConfirms <= 0 || maxConfirms > 6 * 24 * 7) { // one week throw std::runtime_error("Corrupt estimates file. Must maintain estimates for between 1 and 1008 (one week) confirms"); } - for (unsigned int i = 0; i < maxConfirms; i++) { + for (unsigned int i = 0; i < maxPeriods; i++) { if (confAvg[i].size() != numBuckets) { throw std::runtime_error("Corrupt estimates file. Mismatch in feerate conf average bucket count"); } @@ -390,10 +396,10 @@ void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets if (nFileVersion >= 149900) { filein >> failAvg; - if (maxConfirms != failAvg.size()) { + if (maxPeriods != failAvg.size()) { throw std::runtime_error("Corrupt estimates file. Mismatch in confirms tracked for failures"); } - for (unsigned int i = 0; i < maxConfirms; i++) { + for (unsigned int i = 0; i < maxPeriods; i++) { if (failAvg[i].size() != numBuckets) { throw std::runtime_error("Corrupt estimates file. Mismatch in one of failure average bucket counts"); } @@ -449,8 +455,9 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe blockIndex, bucketindex); } } - if (!inBlock && blocksAgo >= 1) { - for (size_t i = 0; i < blocksAgo && i < failAvg.size(); i++) { + if (!inBlock && (unsigned int)blocksAgo >= scale) { // Only counts as a failure if not confirmed for entire period + unsigned int periodsAgo = blocksAgo / scale; + for (size_t i = 0; i < periodsAgo && i < failAvg.size(); i++) { failAvg[i][bucketindex]++; } } @@ -490,9 +497,9 @@ CBlockPolicyEstimator::CBlockPolicyEstimator() bucketMap[INF_FEERATE] = bucketIndex; assert(bucketMap.size() == buckets.size()); - feeStats = new TxConfirmStats(buckets, bucketMap, MED_BLOCK_CONFIRMS, MED_DECAY); - shortStats = new TxConfirmStats(buckets, bucketMap, SHORT_BLOCK_CONFIRMS, SHORT_DECAY); - longStats = new TxConfirmStats(buckets, bucketMap, LONG_BLOCK_CONFIRMS, LONG_DECAY); + feeStats = new TxConfirmStats(buckets, bucketMap, MED_BLOCK_PERIODS, MED_DECAY, MED_SCALE); + shortStats = new TxConfirmStats(buckets, bucketMap, SHORT_BLOCK_PERIODS, SHORT_DECAY, SHORT_SCALE); + longStats = new TxConfirmStats(buckets, bucketMap, LONG_BLOCK_PERIODS, LONG_DECAY, LONG_SCALE); } CBlockPolicyEstimator::~CBlockPolicyEstimator() @@ -864,7 +871,7 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) std::map tempMap; - std::unique_ptr tempFeeStats(new TxConfirmStats(tempBuckets, tempMap, MED_BLOCK_CONFIRMS, tempDecay)); + std::unique_ptr tempFeeStats(new TxConfirmStats(tempBuckets, tempMap, MED_BLOCK_PERIODS, tempDecay, 1)); tempFeeStats->Read(filein, nVersionThatWrote, tempNum); // if nVersionThatWrote < 139900 then another TxConfirmStats (for priority) follows but can be ignored. @@ -884,9 +891,9 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) if (numBuckets <= 1 || numBuckets > 1000) throw std::runtime_error("Corrupt estimates file. Must have between 2 and 1000 feerate buckets"); - std::unique_ptr fileFeeStats(new TxConfirmStats(buckets, bucketMap, MED_BLOCK_CONFIRMS, MED_DECAY)); - std::unique_ptr fileShortStats(new TxConfirmStats(buckets, bucketMap, SHORT_BLOCK_CONFIRMS, SHORT_DECAY)); - std::unique_ptr fileLongStats(new TxConfirmStats(buckets, bucketMap, LONG_BLOCK_CONFIRMS, LONG_DECAY)); + std::unique_ptr fileFeeStats(new TxConfirmStats(buckets, bucketMap, MED_BLOCK_PERIODS, MED_DECAY, MED_SCALE)); + std::unique_ptr fileShortStats(new TxConfirmStats(buckets, bucketMap, SHORT_BLOCK_PERIODS, SHORT_DECAY, SHORT_SCALE)); + std::unique_ptr fileLongStats(new TxConfirmStats(buckets, bucketMap, LONG_BLOCK_PERIODS, LONG_DECAY, LONG_SCALE)); fileFeeStats->Read(filein, nVersionThatWrote, numBuckets); fileShortStats->Read(filein, nVersionThatWrote, numBuckets); fileLongStats->Read(filein, nVersionThatWrote, numBuckets); diff --git a/src/policy/fees.h b/src/policy/fees.h index 1f752415e72..45356d62f61 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -82,6 +82,7 @@ struct EstimationResult EstimatorBucket pass; EstimatorBucket fail; double decay; + unsigned int scale; }; /** @@ -93,11 +94,14 @@ class CBlockPolicyEstimator { private: /** Track confirm delays up to 12 blocks medium decay */ - static constexpr unsigned int SHORT_BLOCK_CONFIRMS = 12; + static constexpr unsigned int SHORT_BLOCK_PERIODS = 12; + static constexpr unsigned int SHORT_SCALE = 1; /** Track confirm delays up to 48 blocks medium decay */ - static constexpr unsigned int MED_BLOCK_CONFIRMS = 48; + static constexpr unsigned int MED_BLOCK_PERIODS = 24; + static constexpr unsigned int MED_SCALE = 2; /** Track confirm delays up to 1008 blocks for longer decay */ - static constexpr unsigned int LONG_BLOCK_CONFIRMS = 1008; + static constexpr unsigned int LONG_BLOCK_PERIODS = 42; + static constexpr unsigned int LONG_SCALE = 24; /** Historical estimates that are older than this aren't valid */ static const unsigned int OLDEST_ESTIMATE_HISTORY = 6 * 1008; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 9f6cab861ca..22bff1cf9e5 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -895,6 +895,7 @@ UniValue estimaterawfee(const JSONRPCRequest& request) "{\n" " \"feerate\" : x.x, (numeric) estimate fee-per-kilobyte (in BTC)\n" " \"decay\" : x.x, (numeric) exponential decay (per block) for historical moving average of confirmation data\n" + " \"scale\" : x, (numeric) The resolution of confirmation targets at this time horizon\n" " \"pass.\" information about the lowest range of feerates to succeed in meeting the threshold\n" " \"fail.\" information about the highest range of feerates to fail to meet the threshold\n" " \"startrange\" : x.x, (numeric) start of feerate range\n" @@ -932,6 +933,7 @@ UniValue estimaterawfee(const JSONRPCRequest& request) result.push_back(Pair("feerate", feeRate == CFeeRate(0) ? -1.0 : ValueFromAmount(feeRate.GetFeePerK()))); result.push_back(Pair("decay", buckets.decay)); + result.push_back(Pair("scale", (int)buckets.scale)); result.push_back(Pair("pass.startrange", round(buckets.pass.start))); result.push_back(Pair("pass.endrange", round(buckets.pass.end))); result.push_back(Pair("pass.withintarget", round(buckets.pass.withinTarget * 100.0) / 100.0)); diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 0d7ca1251fa..6bfd3156473 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -99,13 +99,10 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) BOOST_CHECK(origFeeEst[i-1] <= origFeeEst[i-2]); } int mult = 11-i; - if (i > 1) { + if (i % 2 == 0) { //At scale 2, test logic is only correct for even targets BOOST_CHECK(origFeeEst[i-1] < mult*baseRate.GetFeePerK() + deltaFee); BOOST_CHECK(origFeeEst[i-1] > mult*baseRate.GetFeePerK() - deltaFee); } - else { - BOOST_CHECK(origFeeEst[i-1] == CFeeRate(0).GetFeePerK()); - } } // Fill out rest of the original estimates for (int i = 10; i <= 48; i++) { @@ -177,7 +174,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) block.clear(); } BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0)); - for (int i = 2; i < 10; i++) { + for (int i = 2; i < 9; i++) { // At 9, the original estimate was already at the bottom (b/c scale = 2) BOOST_CHECK(feeEst.estimateFee(i).GetFeePerK() < origFeeEst[i-1] - deltaFee); } From ef589f8d40d926bbc78a556a149c367cf634c376 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Wed, 12 Apr 2017 12:05:10 -0400 Subject: [PATCH 13/15] minor cleanup: remove unnecessary variable --- src/policy/fees.cpp | 3 +-- src/policy/fees.h | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 44adb26d4cc..97e9536867e 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -487,9 +487,8 @@ CBlockPolicyEstimator::CBlockPolicyEstimator() : nBestSeenHeight(0), firstRecordedHeight(0), historicalFirst(0), historicalBest(0), trackedTxs(0), untrackedTxs(0) { static_assert(MIN_BUCKET_FEERATE > 0, "Min feerate must be nonzero"); - minTrackedFee = CFeeRate(MIN_BUCKET_FEERATE); size_t bucketIndex = 0; - for (double bucketBoundary = minTrackedFee.GetFeePerK(); bucketBoundary <= MAX_BUCKET_FEERATE; bucketBoundary *= FEE_SPACING, bucketIndex++) { + for (double bucketBoundary = MIN_BUCKET_FEERATE; bucketBoundary <= MAX_BUCKET_FEERATE; bucketBoundary *= FEE_SPACING, bucketIndex++) { buckets.push_back(bucketBoundary); bucketMap[bucketBoundary] = bucketIndex; } diff --git a/src/policy/fees.h b/src/policy/fees.h index 45356d62f61..aa179cfddd5 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -177,7 +177,6 @@ public: void FlushUnconfirmed(CTxMemPool& pool); private: - CFeeRate minTrackedFee; //!< Passed to constructor to avoid dependency on main unsigned int nBestSeenHeight; unsigned int firstRecordedHeight; unsigned int historicalFirst; From 2d2e17052c389948c511d2b5d41c4cc243cdc145 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Wed, 12 Apr 2017 12:29:03 -0400 Subject: [PATCH 14/15] Comments and improved documentation --- src/policy/fees.cpp | 10 +++++++ src/policy/fees.h | 72 ++++++++++++++++++++++++++++----------------- src/rpc/mining.cpp | 2 +- 3 files changed, 56 insertions(+), 28 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 97e9536867e..35e2857399e 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -729,6 +729,9 @@ double CBlockPolicyEstimator::estimateCombinedFee(unsigned int confTarget, doubl return estimate; } +/** Ensure that for a conservative estimate, the DOUBLE_SUCCESS_PCT is also met + * at 2 * target for any longer time horizons. + */ double CBlockPolicyEstimator::estimateConservativeFee(unsigned int doubleTarget) const { double estimate = -1; @@ -744,6 +747,13 @@ double CBlockPolicyEstimator::estimateConservativeFee(unsigned int doubleTarget) return estimate; } +/** estimateSmartFee returns the max of the feerates calculated with a 60% + * threshold required at target / 2, an 85% threshold required at target and a + * 95% threshold required at 2 * target. Each calculation is performed at the + * shortest time horizon which tracks the required target. Conservative + * estimates, however, required the 95% threshold at 2 * target be met for any + * longer time horizons also. + */ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool, bool conservative) const { if (answerFoundAtTarget) diff --git a/src/policy/fees.h b/src/policy/fees.h index aa179cfddd5..d5b63823adf 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -41,32 +41,39 @@ class TxConfirmStats; * within your desired 5 blocks. * * Here is a brief description of the implementation: - * When a transaction enters the mempool, we - * track the height of the block chain at entry. Whenever a block comes in, - * we count the number of transactions in each bucket and the total amount of feerate - * paid in each bucket. Then we calculate how many blocks Y it took each - * transaction to be mined and we track an array of counters in each bucket - * for how long it to took transactions to get confirmed from 1 to a max of 25 - * and we increment all the counters from Y up to 25. This is because for any - * number Z>=Y the transaction was successfully mined within Z blocks. We - * want to save a history of this information, so at any time we have a - * counter of the total number of transactions that happened in a given feerate - * bucket and the total number that were confirmed in each number 1-25 blocks - * or less for any bucket. We save this history by keeping an exponentially - * decaying moving average of each one of these stats. Furthermore we also - * keep track of the number unmined (in mempool) transactions in each bucket - * and for how many blocks they have been outstanding and use that to increase - * the number of transactions we've seen in that feerate bucket when calculating - * an estimate for any number of confirmations below the number of blocks - * they've been outstanding. + * When a transaction enters the mempool, we track the height of the block chain + * at entry. All further calculations are conducted only on this set of "seen" + * transactions. Whenever a block comes in, we count the number of transactions + * in each bucket and the total amount of feerate paid in each bucket. Then we + * calculate how many blocks Y it took each transaction to be mined. We convert + * from a number of blocks to a number of periods Y' each encompassing "scale" + * blocks. This is is tracked in 3 different data sets each up to a maximum + * number of periods. Within each data set we have an array of counters in each + * feerate bucket and we increment all the counters from Y' up to max periods + * representing that a tx was successfullly confirmed in less than or equal to + * that many periods. We want to save a history of this information, so at any + * time we have a counter of the total number of transactions that happened in a + * given feerate bucket and the total number that were confirmed in each of the + * periods or less for any bucket. We save this history by keeping an + * exponentially decaying moving average of each one of these stats. This is + * done for a different decay in each of the 3 data sets to keep relevant data + * from different time horizons. Furthermore we also keep track of the number + * unmined (in mempool or left mempool without being included in a block) + * transactions in each bucket and for how many blocks they have been + * outstanding and use both of these numbers to increase the number of transactions + * we've seen in that feerate bucket when calculating an estimate for any number + * of confirmations below the number of blocks they've been outstanding. */ +/* Identifier for each of the 3 different TxConfirmStats which will track + * history over different time horizons. */ enum FeeEstimateHorizon { SHORT_HALFLIFE = 0, MED_HALFLIFE = 1, LONG_HALFLIFE = 2 }; +/* Used to return detailed information about a feerate bucket */ struct EstimatorBucket { double start = -1; @@ -77,6 +84,7 @@ struct EstimatorBucket double leftMempool = 0; }; +/* Used to return detailed information about a fee estimate calculation */ struct EstimationResult { EstimatorBucket pass; @@ -93,13 +101,13 @@ struct EstimationResult class CBlockPolicyEstimator { private: - /** Track confirm delays up to 12 blocks medium decay */ + /** Track confirm delays up to 12 blocks for short horizon */ static constexpr unsigned int SHORT_BLOCK_PERIODS = 12; static constexpr unsigned int SHORT_SCALE = 1; - /** Track confirm delays up to 48 blocks medium decay */ + /** Track confirm delays up to 48 blocks for medium horizon */ static constexpr unsigned int MED_BLOCK_PERIODS = 24; static constexpr unsigned int MED_SCALE = 2; - /** Track confirm delays up to 1008 blocks for longer decay */ + /** Track confirm delays up to 1008 blocks for long horizon */ static constexpr unsigned int LONG_BLOCK_PERIODS = 42; static constexpr unsigned int LONG_SCALE = 24; /** Historical estimates that are older than this aren't valid */ @@ -112,9 +120,11 @@ private: /** Decay of .9995 is a half-life of 1008 blocks or about 1 week */ static constexpr double LONG_DECAY = .99931; - /** Require greater than 95% of X feerate transactions to be confirmed within Y blocks for X to be big enough */ + /** Require greater than 60% of X feerate transactions to be confirmed within Y/2 blocks*/ static constexpr double HALF_SUCCESS_PCT = .6; + /** Require greater than 85% of X feerate transactions to be confirmed within Y blocks*/ static constexpr double SUCCESS_PCT = .85; + /** Require greater than 95% of X feerate transactions to be confirmed within 2 * Y blocks*/ static constexpr double DOUBLE_SUCCESS_PCT = .95; /** Require an avg of 0.1 tx in the combined feerate bucket per block to have stat significance */ @@ -154,16 +164,19 @@ public: /** Remove a transaction from the mempool tracking stats*/ bool removeTx(uint256 hash, bool inBlock); - /** Return a feerate estimate */ + /** DEPRECATED. Return a feerate estimate */ 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. + /** 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 closest target where one can be given. 'conservative' estimates are + * valid over longer time horizons also. */ CFeeRate estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool, bool conservative = true) const; - /** Return a specific fee estimate calculation with a given success threshold and time horizon. + /** Return a specific fee estimate calculation with a given success + * threshold and time horizon, and optionally return detailed data about + * calculation */ CFeeRate estimateRawFee(int confTarget, double successThreshold, FeeEstimateHorizon horizon, EstimationResult *result = nullptr) const; @@ -208,10 +221,15 @@ private: /** Process a transaction confirmed in a block*/ bool processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry); + /** Helper for estimateSmartFee */ double estimateCombinedFee(unsigned int confTarget, double successThreshold, bool checkShorterHorizon) const; + /** Helper for estimateSmartFee */ double estimateConservativeFee(unsigned int doubleTarget) const; + /** Number of blocks of data recorded while fee estimates have been running */ unsigned int BlockSpan() const; + /** Number of blocks of recorded fee estimate data represented in saved data file */ unsigned int HistoricalBlockSpan() const; + /** Calculation of highest target that reasonable estimate can be provided for */ unsigned int MaxUsableEstimate() const; }; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 22bff1cf9e5..7fd95768a43 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -797,6 +797,7 @@ UniValue estimatefee(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 1) throw std::runtime_error( "estimatefee nblocks\n" + "\nDEPRECATED. Please use estimatesmartfee for more intelligent estimates." "\nEstimates the approximate fee per kilobyte needed for a transaction to begin\n" "confirmation within nblocks blocks. Uses virtual transaction size of transaction\n" "as defined in BIP 141 (witness data is discounted).\n" @@ -831,7 +832,6 @@ UniValue estimatesmartfee(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) throw std::runtime_error( "estimatesmartfee nblocks (conservative)\n" - "\nWARNING: This interface is unstable and may disappear or change!\n" "\nEstimates the approximate fee per kilobyte needed for a transaction to begin\n" "confirmation within nblocks blocks if possible and return the number of blocks\n" "for which the estimate is valid. Uses virtual transaction size as defined\n" From 38bc1ec4a473b5bd9b7bbce7b20a11e8edfe4b4c Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Wed, 17 May 2017 15:39:24 -0400 Subject: [PATCH 15/15] Make more json-like output from estimaterawfee --- src/rpc/mining.cpp | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 7fd95768a43..d744269df1c 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -896,14 +896,15 @@ UniValue estimaterawfee(const JSONRPCRequest& request) " \"feerate\" : x.x, (numeric) estimate fee-per-kilobyte (in BTC)\n" " \"decay\" : x.x, (numeric) exponential decay (per block) for historical moving average of confirmation data\n" " \"scale\" : x, (numeric) The resolution of confirmation targets at this time horizon\n" - " \"pass.\" information about the lowest range of feerates to succeed in meeting the threshold\n" - " \"fail.\" information about the highest range of feerates to fail to meet the threshold\n" - " \"startrange\" : x.x, (numeric) start of feerate range\n" - " \"endrange\" : x.x, (numeric) end of feerate range\n" - " \"withintarget\" : x.x, (numeric) number of txs over history horizon in the feerate range that were confirmed within target\n" - " \"totalconfirmed\" : x.x, (numeric) number of txs over history horizon in the feerate range that were confirmed at any point\n" - " \"inmempool\" : x.x, (numeric) current number of txs in mempool in the feerate range unconfirmed for at least target blocks\n" - " \"leftmempool\" : x.x, (numeric) number of txs over history horizon in the feerate range that left mempool unconfirmed after target\n" + " \"pass\" : { (json object) information about the lowest range of feerates to succeed in meeting the threshold\n" + " \"startrange\" : x.x, (numeric) start of feerate range\n" + " \"endrange\" : x.x, (numeric) end of feerate range\n" + " \"withintarget\" : x.x, (numeric) number of txs over history horizon in the feerate range that were confirmed within target\n" + " \"totalconfirmed\" : x.x, (numeric) number of txs over history horizon in the feerate range that were confirmed at any point\n" + " \"inmempool\" : x.x, (numeric) current number of txs in mempool in the feerate range unconfirmed for at least target blocks\n" + " \"leftmempool\" : x.x, (numeric) number of txs over history horizon in the feerate range that left mempool unconfirmed after target\n" + " }\n" + " \"fail\" : { ... } (json object) information about the highest range of feerates to fail to meet the threshold\n" "}\n" "\n" "A negative feerate is returned if no answer can be given.\n" @@ -934,18 +935,22 @@ UniValue estimaterawfee(const JSONRPCRequest& request) result.push_back(Pair("feerate", feeRate == CFeeRate(0) ? -1.0 : ValueFromAmount(feeRate.GetFeePerK()))); result.push_back(Pair("decay", buckets.decay)); result.push_back(Pair("scale", (int)buckets.scale)); - result.push_back(Pair("pass.startrange", round(buckets.pass.start))); - result.push_back(Pair("pass.endrange", round(buckets.pass.end))); - result.push_back(Pair("pass.withintarget", round(buckets.pass.withinTarget * 100.0) / 100.0)); - result.push_back(Pair("pass.totalconfirmed", round(buckets.pass.totalConfirmed * 100.0) / 100.0)); - result.push_back(Pair("pass.inmempool", round(buckets.pass.inMempool * 100.0) / 100.0)); - result.push_back(Pair("pass.leftmempool", round(buckets.pass.leftMempool * 100.0) / 100.0)); - result.push_back(Pair("fail.startrange", round(buckets.fail.start))); - result.push_back(Pair("fail.endrange", round(buckets.fail.end))); - result.push_back(Pair("fail.withintarget", round(buckets.fail.withinTarget * 100.0) / 100.0)); - result.push_back(Pair("fail.totalconfirmed", round(buckets.fail.totalConfirmed * 100.0) / 100.0)); - result.push_back(Pair("fail.inmempool", round(buckets.fail.inMempool * 100.0) / 100.0)); - result.push_back(Pair("fail.leftmempool", round(buckets.fail.leftMempool * 100.0) / 100.0)); + UniValue passbucket(UniValue::VOBJ); + passbucket.push_back(Pair("startrange", round(buckets.pass.start))); + passbucket.push_back(Pair("endrange", round(buckets.pass.end))); + passbucket.push_back(Pair("withintarget", round(buckets.pass.withinTarget * 100.0) / 100.0)); + passbucket.push_back(Pair("totalconfirmed", round(buckets.pass.totalConfirmed * 100.0) / 100.0)); + passbucket.push_back(Pair("inmempool", round(buckets.pass.inMempool * 100.0) / 100.0)); + passbucket.push_back(Pair("leftmempool", round(buckets.pass.leftMempool * 100.0) / 100.0)); + result.push_back(Pair("pass", passbucket)); + UniValue failbucket(UniValue::VOBJ); + failbucket.push_back(Pair("startrange", round(buckets.fail.start))); + failbucket.push_back(Pair("endrange", round(buckets.fail.end))); + failbucket.push_back(Pair("withintarget", round(buckets.fail.withinTarget * 100.0) / 100.0)); + failbucket.push_back(Pair("totalconfirmed", round(buckets.fail.totalConfirmed * 100.0) / 100.0)); + failbucket.push_back(Pair("inmempool", round(buckets.fail.inMempool * 100.0) / 100.0)); + failbucket.push_back(Pair("leftmempool", round(buckets.fail.leftMempool * 100.0) / 100.0)); + result.push_back(Pair("fail", failbucket)); return result; }