From e5007bae35ce22036a816505038277d99c84e3f7 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 9 Mar 2017 14:02:00 -0500 Subject: [PATCH] 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 0f3f2c63d2..42059cf94f 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 7aaba7d7d5..0df40df42a 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 ed6782ea34..942efce0f7 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