From f18b8ec7cf6ebfff9eef839c6a5630ad2e6e7db6 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 31 Jul 2015 16:41:06 +0200 Subject: Make sure LogPrintf strings are line-terminated Fix the cases where LogPrint[f] was accidentally called without line terminator, which resulted in concatenated log lines. (see e.g. #6492) --- src/policy/fees.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index b1491cec0..cdee541d2 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -249,7 +249,7 @@ unsigned int TxConfirmStats::NewTx(unsigned int nBlockHeight, double val) unsigned int bucketindex = bucketMap.lower_bound(val)->second; unsigned int blockIndex = nBlockHeight % unconfTxs.size(); unconfTxs[blockIndex][bucketindex]++; - LogPrint("estimatefee", "adding to %s\n", dataTypeString); + LogPrint("estimatefee", "adding to %s", dataTypeString); return bucketindex; } @@ -390,8 +390,9 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo mapMemPoolTxs[hash].bucketIndex = feeStats.NewTx(txHeight, (double)feeRate.GetFeePerK()); } else { - LogPrint("estimatefee", "not adding\n"); + LogPrint("estimatefee", "not adding"); } + LogPrint("estimatefee", "\n"); } void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry& entry) -- cgit v1.2.3 From 9f68ed6b6d1a9c6436ce37913666165f2b180ee3 Mon Sep 17 00:00:00 2001 From: Veres Lajos Date: Sun, 9 Aug 2015 00:17:27 +0100 Subject: typofixes (found by misspell_fixer) --- src/policy/fees.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index cdee541d2..ffe31d194 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -261,7 +261,7 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe blocksAgo = 0; if (blocksAgo < 0) { LogPrint("estimatefee", "Blockpolicy error, blocks ago is negative for mempool tx\n"); - return; //This can't happen becasue we call this with our best seen height, no entries can have higher + return; //This can't happen because we call this with our best seen height, no entries can have higher } if (blocksAgo >= (int)unconfTxs.size()) { -- cgit v1.2.3 From 22eca7da22b67409d757d6859b1cf212e445dd39 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Mon, 16 Nov 2015 15:10:22 -0500 Subject: Add smart fee estimation functions These are more useful fee and priority estimation functions. If there is no fee/pri high enough for the target you are aiming for, it will give you the estimate for the lowest target that you can reliably obtain. This is better than defaulting to the minimum. It will also pass back the target for which it returned an answer. --- src/policy/fees.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index ffe31d194..eb6e9cc8b 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -504,6 +504,28 @@ CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget) return CFeeRate(median); } +CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoundAtTarget) +{ + if (answerFoundAtTarget) + *answerFoundAtTarget = confTarget; + // Return failure if trying to analyze a target we're not tracking + if (confTarget <= 0 || (unsigned int)confTarget > feeStats.GetMaxConfirms()) + return CFeeRate(0); + + double median = -1; + while (median < 0 && (unsigned int)confTarget <= feeStats.GetMaxConfirms()) { + median = feeStats.EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); + } + + if (answerFoundAtTarget) + *answerFoundAtTarget = confTarget - 1; + + if (median < 0) + return CFeeRate(0); + + return CFeeRate(median); +} + double CBlockPolicyEstimator::estimatePriority(int confTarget) { // Return failure if trying to analyze a target we're not tracking @@ -513,6 +535,25 @@ double CBlockPolicyEstimator::estimatePriority(int confTarget) return priStats.EstimateMedianVal(confTarget, SUFFICIENT_PRITXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); } +double CBlockPolicyEstimator::estimateSmartPriority(int confTarget, int *answerFoundAtTarget) +{ + if (answerFoundAtTarget) + *answerFoundAtTarget = confTarget; + // Return failure if trying to analyze a target we're not tracking + if (confTarget <= 0 || (unsigned int)confTarget > priStats.GetMaxConfirms()) + return -1; + + double median = -1; + while (median < 0 && (unsigned int)confTarget <= priStats.GetMaxConfirms()) { + median = priStats.EstimateMedianVal(confTarget++, SUFFICIENT_PRITXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); + } + + if (answerFoundAtTarget) + *answerFoundAtTarget = confTarget - 1; + + return median; +} + void CBlockPolicyEstimator::Write(CAutoFile& fileout) { fileout << nBestSeenHeight; -- cgit v1.2.3 From 63030514701828a06040413837f5eced9deeee03 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Mon, 16 Nov 2015 15:21:51 -0500 Subject: EstimateSmart functions consider mempool min fee --- src/policy/fees.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index eb6e9cc8b..e139b06c7 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -4,6 +4,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include "policy/fees.h" +#include "policy/policy.h" #include "amount.h" #include "primitives/transaction.h" @@ -504,7 +505,7 @@ CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget) return CFeeRate(median); } -CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoundAtTarget) +CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool *pool) { if (answerFoundAtTarget) *answerFoundAtTarget = confTarget; @@ -520,6 +521,11 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun if (answerFoundAtTarget) *answerFoundAtTarget = confTarget - 1; + // If mempool is limiting txs , return at least the min fee from the mempool + CAmount minPoolFee = pool->GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); + if (minPoolFee > 0 && minPoolFee > median) + return CFeeRate(minPoolFee); + if (median < 0) return CFeeRate(0); @@ -535,7 +541,7 @@ double CBlockPolicyEstimator::estimatePriority(int confTarget) return priStats.EstimateMedianVal(confTarget, SUFFICIENT_PRITXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); } -double CBlockPolicyEstimator::estimateSmartPriority(int confTarget, int *answerFoundAtTarget) +double CBlockPolicyEstimator::estimateSmartPriority(int confTarget, int *answerFoundAtTarget, const CTxMemPool *pool) { if (answerFoundAtTarget) *answerFoundAtTarget = confTarget; @@ -543,6 +549,11 @@ double CBlockPolicyEstimator::estimateSmartPriority(int confTarget, int *answerF if (confTarget <= 0 || (unsigned int)confTarget > priStats.GetMaxConfirms()) return -1; + // If mempool is limiting txs, no priority txs are allowed + CAmount minPoolFee = pool->GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); + if (minPoolFee > 0) + return INF_PRIORITY; + double median = -1; while (median < 0 && (unsigned int)confTarget <= priStats.GetMaxConfirms()) { median = priStats.EstimateMedianVal(confTarget++, SUFFICIENT_PRITXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); @@ -551,6 +562,7 @@ double CBlockPolicyEstimator::estimateSmartPriority(int confTarget, int *answerF if (answerFoundAtTarget) *answerFoundAtTarget = confTarget - 1; + return median; } -- cgit v1.2.3 From e30443244a7a50f2db70e593ec8a57e5086db3d9 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Tue, 24 Nov 2015 08:53:14 -0500 Subject: Pass reference to estimateSmartFee and cleanup whitespace --- src/policy/fees.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index e139b06c7..980ecf10d 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -505,7 +505,7 @@ CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget) return CFeeRate(median); } -CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool *pool) +CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool) { if (answerFoundAtTarget) *answerFoundAtTarget = confTarget; @@ -522,7 +522,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun *answerFoundAtTarget = confTarget - 1; // If mempool is limiting txs , return at least the min fee from the mempool - CAmount minPoolFee = pool->GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); + CAmount minPoolFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); if (minPoolFee > 0 && minPoolFee > median) return CFeeRate(minPoolFee); @@ -541,7 +541,7 @@ double CBlockPolicyEstimator::estimatePriority(int confTarget) return priStats.EstimateMedianVal(confTarget, SUFFICIENT_PRITXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); } -double CBlockPolicyEstimator::estimateSmartPriority(int confTarget, int *answerFoundAtTarget, const CTxMemPool *pool) +double CBlockPolicyEstimator::estimateSmartPriority(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool) { if (answerFoundAtTarget) *answerFoundAtTarget = confTarget; @@ -550,7 +550,7 @@ double CBlockPolicyEstimator::estimateSmartPriority(int confTarget, int *answerF return -1; // If mempool is limiting txs, no priority txs are allowed - CAmount minPoolFee = pool->GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); + CAmount minPoolFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); if (minPoolFee > 0) return INF_PRIORITY; @@ -562,7 +562,6 @@ double CBlockPolicyEstimator::estimateSmartPriority(int confTarget, int *answerF if (answerFoundAtTarget) *answerFoundAtTarget = confTarget - 1; - return median; } -- cgit v1.2.3 From 9d263bd17c2bdd5ba9e31bd5fb110c332eb80691 Mon Sep 17 00:00:00 2001 From: Chris Wheeler Date: Sun, 17 Jan 2016 11:03:56 +0000 Subject: Typo fixes in comments --- src/policy/fees.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 980ecf10d..de3c060d6 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -87,7 +87,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, int maxbucketindex = buckets.size() - 1; // requireGreater means we are looking for the lowest fee/priority such that all higher - // values pass, so we start at maxbucketindex (highest fee) and look at succesively + // values pass, so we start at maxbucketindex (highest fee) and look at successively // smaller buckets until we reach failure. Otherwise, we are looking for the highest // fee/priority such that all lower values fail, and we go in the opposite direction. unsigned int startbucket = requireGreater ? maxbucketindex : 0; -- cgit v1.2.3 From 9e072a6e66efbda7d39bf61eded21d2b324323be Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 12 Feb 2016 15:57:15 -0500 Subject: Implement "feefilter" P2P message. The "feefilter" p2p message is used to inform other nodes of your mempool min fee which is the feerate that any new transaction must meet to be accepted to your mempool. This will allow them to filter invs to you according to this feerate. --- src/policy/fees.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index de3c060d6..7b0e8b7d0 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -8,6 +8,7 @@ #include "amount.h" #include "primitives/transaction.h" +#include "random.h" #include "streams.h" #include "txmempool.h" #include "util.h" @@ -580,3 +581,21 @@ void CBlockPolicyEstimator::Read(CAutoFile& filein) priStats.Read(filein); nBestSeenHeight = nFileBestSeenHeight; } + +FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee) +{ + CAmount minFeeLimit = minIncrementalFee.GetFeePerK() / 2; + feeset.insert(0); + for (double bucketBoundary = minFeeLimit; bucketBoundary <= MAX_FEERATE; bucketBoundary *= FEE_SPACING) { + feeset.insert(bucketBoundary); + } +} + +CAmount FeeFilterRounder::round(CAmount currentMinFee) +{ + std::set::iterator it = feeset.lower_bound(currentMinFee); + if ((it != feeset.begin() && insecure_rand() % 3 != 0) || it == feeset.end()) { + it--; + } + return *it; +} -- cgit v1.2.3 From 5eaaa83ac1f5eb525f93e2808fafd73f5ed97013 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 13 Oct 2016 16:19:20 +0200 Subject: Kill insecure_random and associated global state There are only a few uses of `insecure_random` outside the tests. This PR replaces uses of insecure_random (and its accompanying global state) in the core code with an FastRandomContext that is automatically seeded on creation. This is meant to be used for inner loops. The FastRandomContext can be in the outer scope, or the class itself, then rand32() is used inside the loop. Useful e.g. for pushing addresses in CNode or the fee rounding, or randomization for coin selection. As a context is created per purpose, thus it gets rid of cross-thread unprotected shared usage of a single set of globals, this should also get rid of the potential race conditions. - I'd say TxMempool::check is not called enough to warrant using a special fast random context, this is switched to GetRand() (open for discussion...) - The use of `insecure_rand` in ConnectThroughProxy has been replaced by an atomic integer counter. The only goal here is to have a different credentials pair for each connection to go on a different Tor circuit, it does not need to be random nor unpredictable. - To avoid having a FastRandomContext on every CNode, the context is passed into PushAddress as appropriate. There remains an insecure_random for test usage in `test_random.h`. --- src/policy/fees.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 7b0e8b7d0..c07cd2eff 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -594,7 +594,7 @@ FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee) CAmount FeeFilterRounder::round(CAmount currentMinFee) { std::set::iterator it = feeset.lower_bound(currentMinFee); - if ((it != feeset.begin() && insecure_rand() % 3 != 0) || it == feeset.end()) { + if ((it != feeset.begin() && insecure_rand.rand32() % 3 != 0) || it == feeset.end()) { it--; } return *it; -- cgit v1.2.3 From 4b04e32c20b3e00ce30f3555677d9c937ca7482f Mon Sep 17 00:00:00 2001 From: isle2983 Date: Sun, 6 Nov 2016 10:12:50 -0700 Subject: [copyright] copyright header style uniform Three categories of modifications: 1) 1 instance of 'The Bitcoin Core developers \n', 1 instance of 'the Bitcoin Core developers\n', 3 instances of 'Bitcoin Core Developers\n', and 12 instances of 'The Bitcoin developers\n' are made uniform with the 443 instances of 'The Bitcoin Core developers\n' 2) 3 instances of 'BitPay, Inc\.\n' are made uniform with the other 6 instances of 'BitPay Inc\.\n' 3) 4 instances where there was no '(c)' between the 'Copyright' and the year where it deviates from the style of the local directory. --- src/policy/fees.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index c07cd2eff..dae00d08a 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2015 The Bitcoin developers +// Copyright (c) 2009-2015 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -- cgit v1.2.3 From b2322e0fc6def0baf8581bbd2f4135e61c47d142 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Mon, 21 Mar 2016 13:04:40 -0400 Subject: Remove priority estimation --- src/policy/fees.cpp | 168 ++++++++++------------------------------------------ 1 file changed, 32 insertions(+), 136 deletions(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index c07cd2eff..77510b564 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -14,10 +14,9 @@ #include "util.h" void TxConfirmStats::Initialize(std::vector& defaultBuckets, - unsigned int maxConfirms, double _decay, std::string _dataTypeString) + unsigned int maxConfirms, double _decay) { decay = _decay; - dataTypeString = _dataTypeString; for (unsigned int i = 0; i < defaultBuckets.size(); i++) { buckets.push_back(defaultBuckets[i]); bucketMap[defaultBuckets[i]] = i; @@ -87,10 +86,10 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, int maxbucketindex = buckets.size() - 1; - // requireGreater means we are looking for the lowest fee/priority such that all higher - // values pass, so we start at maxbucketindex (highest fee) and look at successively + // requireGreater means we are looking for the lowest feerate such that all higher + // values pass, so we start at maxbucketindex (highest feerate) and look at successively // smaller buckets until we reach failure. Otherwise, we are looking for the highest - // fee/priority such that all lower values fail, and we go in the opposite direction. + // feerate such that all lower values fail, and we go in the opposite direction. unsigned int startbucket = requireGreater ? maxbucketindex : 0; int step = requireGreater ? -1 : 1; @@ -107,7 +106,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, bool foundAnswer = false; unsigned int bins = unconfTxs.size(); - // Start counting from highest(default) or lowest fee/pri transactions + // Start counting from highest(default) or lowest feerate transactions for (int bucket = startbucket; bucket >= 0 && bucket <= maxbucketindex; bucket += step) { curFarBucket = bucket; nConf += confAvg[confTarget - 1][bucket]; @@ -145,8 +144,8 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, double median = -1; double txSum = 0; - // Calculate the "average" fee of the best bucket range that met success conditions - // Find the bucket with the median transaction and then report the average fee from that bucket + // Calculate the "average" feerate of the best bucket range that met success conditions + // 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; @@ -166,8 +165,8 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, } } - LogPrint("estimatefee", "%3d: For conf success %s %4.2f need %s %s: %12.5g from buckets %8g - %8g Cur Bucket stats %6.2f%% %8.1f/(%.1f+%d mempool)\n", - confTarget, requireGreater ? ">" : "<", successBreakPoint, dataTypeString, + LogPrint("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); @@ -200,10 +199,10 @@ void TxConfirmStats::Read(CAutoFile& filein) filein >> fileBuckets; numBuckets = fileBuckets.size(); if (numBuckets <= 1 || numBuckets > 1000) - throw std::runtime_error("Corrupt estimates file. Must have between 2 and 1000 fee/pri buckets"); + 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 fee/pri average bucket count"); + 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"); @@ -213,9 +212,9 @@ void TxConfirmStats::Read(CAutoFile& filein) 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 fee/pri conf average bucket count"); + throw std::runtime_error("Corrupt estimates file. Mismatch in feerate conf average bucket count"); } - // Now that we've processed the entire fee estimate data file and not + // 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; @@ -242,8 +241,8 @@ void TxConfirmStats::Read(CAutoFile& filein) for (unsigned int i = 0; i < buckets.size(); i++) bucketMap[buckets[i]] = i; - LogPrint("estimatefee", "Reading estimates: %u %s buckets counting confirms up to %u blocks\n", - numBuckets, dataTypeString, maxConfirms); + LogPrint("estimatefee", "Reading estimates: %u buckets counting confirms up to %u blocks\n", + numBuckets, maxConfirms); } unsigned int TxConfirmStats::NewTx(unsigned int nBlockHeight, double val) @@ -251,7 +250,6 @@ unsigned int TxConfirmStats::NewTx(unsigned int nBlockHeight, double val) unsigned int bucketindex = bucketMap.lower_bound(val)->second; unsigned int blockIndex = nBlockHeight % unconfTxs.size(); unconfTxs[blockIndex][bucketindex]++; - LogPrint("estimatefee", "adding to %s", dataTypeString); return bucketindex; } @@ -291,12 +289,10 @@ void CBlockPolicyEstimator::removeTx(uint256 hash) hash.ToString().c_str()); return; } - TxConfirmStats *stats = pos->second.stats; unsigned int entryHeight = pos->second.blockHeight; unsigned int bucketIndex = pos->second.bucketIndex; - if (stats != NULL) - stats->removeTx(entryHeight, nBestSeenHeight, bucketIndex); + feeStats.removeTx(entryHeight, nBestSeenHeight, bucketIndex); mapMemPoolTxs.erase(hash); } @@ -309,45 +305,14 @@ CBlockPolicyEstimator::CBlockPolicyEstimator(const CFeeRate& _minRelayFee) vfeelist.push_back(bucketBoundary); } vfeelist.push_back(INF_FEERATE); - feeStats.Initialize(vfeelist, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY, "FeeRate"); - - minTrackedPriority = AllowFreeThreshold() < MIN_PRIORITY ? MIN_PRIORITY : AllowFreeThreshold(); - std::vector vprilist; - for (double bucketBoundary = minTrackedPriority; bucketBoundary <= MAX_PRIORITY; bucketBoundary *= PRI_SPACING) { - vprilist.push_back(bucketBoundary); - } - vprilist.push_back(INF_PRIORITY); - priStats.Initialize(vprilist, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY, "Priority"); - - feeUnlikely = CFeeRate(0); - feeLikely = CFeeRate(INF_FEERATE); - priUnlikely = 0; - priLikely = INF_PRIORITY; -} - -bool CBlockPolicyEstimator::isFeeDataPoint(const CFeeRate &fee, double pri) -{ - if ((pri < minTrackedPriority && fee >= minTrackedFee) || - (pri < priUnlikely && fee > feeLikely)) { - return true; - } - return false; -} - -bool CBlockPolicyEstimator::isPriDataPoint(const CFeeRate &fee, double pri) -{ - if ((fee < minTrackedFee && pri >= minTrackedPriority) || - (fee < feeUnlikely && pri > priLikely)) { - return true; - } - return false; + feeStats.Initialize(vfeelist, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); } void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, bool fCurrentEstimate) { unsigned int txHeight = entry.GetHeight(); uint256 hash = entry.GetTx().GetHash(); - if (mapMemPoolTxs[hash].stats != NULL) { + if (mapMemPoolTxs.count(hash)) { LogPrint("estimatefee", "Blockpolicy error mempool tx %s already being tracked\n", hash.ToString().c_str()); return; @@ -371,30 +336,11 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo return; } - // Fees are stored and reported as BTC-per-kb: + // Feerates are stored and reported as BTC-per-kb: CFeeRate feeRate(entry.GetFee(), entry.GetTxSize()); - // Want the priority of the tx at confirmation. However we don't know - // what that will be and its too hard to continue updating it - // so use starting priority as a proxy - double curPri = entry.GetPriority(txHeight); mapMemPoolTxs[hash].blockHeight = txHeight; - - LogPrint("estimatefee", "Blockpolicy mempool tx %s ", hash.ToString().substr(0,10)); - // Record this as a priority estimate - if (entry.GetFee() == 0 || isPriDataPoint(feeRate, curPri)) { - mapMemPoolTxs[hash].stats = &priStats; - mapMemPoolTxs[hash].bucketIndex = priStats.NewTx(txHeight, curPri); - } - // Record this as a fee estimate - else if (isFeeDataPoint(feeRate, curPri)) { - mapMemPoolTxs[hash].stats = &feeStats; - mapMemPoolTxs[hash].bucketIndex = feeStats.NewTx(txHeight, (double)feeRate.GetFeePerK()); - } - else { - LogPrint("estimatefee", "not adding"); - } - LogPrint("estimatefee", "\n"); + mapMemPoolTxs[hash].bucketIndex = feeStats.NewTx(txHeight, (double)feeRate.GetFeePerK()); } void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry& entry) @@ -417,21 +363,10 @@ void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM return; } - // Fees are stored and reported as BTC-per-kb: + // Feerates are stored and reported as BTC-per-kb: CFeeRate feeRate(entry.GetFee(), entry.GetTxSize()); - // Want the priority of the tx at confirmation. The priority when it - // entered the mempool could easily be very small and change quickly - double curPri = entry.GetPriority(nBlockHeight); - - // Record this as a priority estimate - if (entry.GetFee() == 0 || isPriDataPoint(feeRate, curPri)) { - priStats.Record(blocksToConfirm, curPri); - } - // Record this as a fee estimate - else if (isFeeDataPoint(feeRate, curPri)) { - feeStats.Record(blocksToConfirm, (double)feeRate.GetFeePerK()); - } + feeStats.Record(blocksToConfirm, (double)feeRate.GetFeePerK()); } void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, @@ -452,41 +387,15 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, if (!fCurrentEstimate) return; - // Update the dynamic cutoffs - // a fee/priority is "likely" the reason your tx was included in a block if >85% of such tx's - // were confirmed in 2 blocks and is "unlikely" if <50% were confirmed in 10 blocks - LogPrint("estimatefee", "Blockpolicy recalculating dynamic cutoffs:\n"); - priLikely = priStats.EstimateMedianVal(2, SUFFICIENT_PRITXS, MIN_SUCCESS_PCT, true, nBlockHeight); - if (priLikely == -1) - priLikely = INF_PRIORITY; - - double feeLikelyEst = feeStats.EstimateMedianVal(2, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBlockHeight); - if (feeLikelyEst == -1) - feeLikely = CFeeRate(INF_FEERATE); - else - feeLikely = CFeeRate(feeLikelyEst); - - priUnlikely = priStats.EstimateMedianVal(10, SUFFICIENT_PRITXS, UNLIKELY_PCT, false, nBlockHeight); - if (priUnlikely == -1) - priUnlikely = 0; - - double feeUnlikelyEst = feeStats.EstimateMedianVal(10, SUFFICIENT_FEETXS, UNLIKELY_PCT, false, nBlockHeight); - if (feeUnlikelyEst == -1) - feeUnlikely = CFeeRate(0); - else - feeUnlikely = CFeeRate(feeUnlikelyEst); - - // Clear the current block states + // Clear the current block state feeStats.ClearCurrent(nBlockHeight); - priStats.ClearCurrent(nBlockHeight); // Repopulate the current block states for (unsigned int i = 0; i < entries.size(); i++) processBlockTx(nBlockHeight, entries[i]); - // Update all exponential averages with the current block states + // Update all exponential averages with the current block state feeStats.UpdateMovingAverages(); - priStats.UpdateMovingAverages(); LogPrint("estimatefee", "Blockpolicy after updating estimates for %u confirmed entries, new mempool map size %u\n", entries.size(), mapMemPoolTxs.size()); @@ -522,7 +431,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun if (answerFoundAtTarget) *answerFoundAtTarget = confTarget - 1; - // If mempool is limiting txs , return at least the min fee from the mempool + // 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(); if (minPoolFee > 0 && minPoolFee > median) return CFeeRate(minPoolFee); @@ -535,51 +444,38 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun double CBlockPolicyEstimator::estimatePriority(int confTarget) { - // Return failure if trying to analyze a target we're not tracking - if (confTarget <= 0 || (unsigned int)confTarget > priStats.GetMaxConfirms()) - return -1; - - return priStats.EstimateMedianVal(confTarget, SUFFICIENT_PRITXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); + return -1; } double CBlockPolicyEstimator::estimateSmartPriority(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool) { if (answerFoundAtTarget) *answerFoundAtTarget = confTarget; - // Return failure if trying to analyze a target we're not tracking - if (confTarget <= 0 || (unsigned int)confTarget > priStats.GetMaxConfirms()) - return -1; // If mempool is limiting txs, no priority txs are allowed CAmount minPoolFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); if (minPoolFee > 0) return INF_PRIORITY; - double median = -1; - while (median < 0 && (unsigned int)confTarget <= priStats.GetMaxConfirms()) { - median = priStats.EstimateMedianVal(confTarget++, SUFFICIENT_PRITXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); - } - - if (answerFoundAtTarget) - *answerFoundAtTarget = confTarget - 1; - - return median; + return -1; } void CBlockPolicyEstimator::Write(CAutoFile& fileout) { fileout << nBestSeenHeight; feeStats.Write(fileout); - priStats.Write(fileout); } -void CBlockPolicyEstimator::Read(CAutoFile& filein) +void CBlockPolicyEstimator::Read(CAutoFile& filein, int nFileVersion) { int nFileBestSeenHeight; filein >> nFileBestSeenHeight; feeStats.Read(filein); - priStats.Read(filein); nBestSeenHeight = nFileBestSeenHeight; + if (nFileVersion < 139900) { + TxConfirmStats priStats; + priStats.Read(filein); + } } FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee) -- cgit v1.2.3 From d824ad030e70bc72e0c63e1b0d00b08413024b55 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 29 Nov 2016 12:18:44 -0500 Subject: Disable fee estimates for a confirm target of 1 block --- src/policy/fees.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 7113390cd..9eb831bc1 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -404,7 +404,8 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget) { // Return failure if trying to analyze a target we're not tracking - if (confTarget <= 0 || (unsigned int)confTarget > feeStats.GetMaxConfirms()) + // It's not possible to get reasonable estimates for confTarget of 1 + if (confTarget <= 1 || (unsigned int)confTarget > feeStats.GetMaxConfirms()) return CFeeRate(0); double median = feeStats.EstimateMedianVal(confTarget, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); @@ -423,6 +424,10 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun if (confTarget <= 0 || (unsigned int)confTarget > feeStats.GetMaxConfirms()) return CFeeRate(0); + // It's not possible to get reasonable estimates for confTarget of 1 + if (confTarget == 1) + confTarget = 2; + double median = -1; while (median < 0 && (unsigned int)confTarget <= feeStats.GetMaxConfirms()) { median = feeStats.EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight); -- cgit v1.2.3 From eab8e1b1725f7458e3db6f0ea0bc04ec603d8e9b Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Mon, 5 Dec 2016 21:46:08 -0500 Subject: fix a bug if the min fee is 0 for FeeFilterRounder --- src/policy/fees.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 9eb831bc1..9f01c24d2 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -299,6 +299,7 @@ void CBlockPolicyEstimator::removeTx(uint256 hash) CBlockPolicyEstimator::CBlockPolicyEstimator(const CFeeRate& _minRelayFee) : nBestSeenHeight(0) { + static_assert(MIN_FEERATE > 0, "Min feerate must be nonzero"); minTrackedFee = _minRelayFee < CFeeRate(MIN_FEERATE) ? CFeeRate(MIN_FEERATE) : _minRelayFee; std::vector vfeelist; for (double bucketBoundary = minTrackedFee.GetFeePerK(); bucketBoundary <= MAX_FEERATE; bucketBoundary *= FEE_SPACING) { @@ -485,7 +486,7 @@ void CBlockPolicyEstimator::Read(CAutoFile& filein, int nFileVersion) FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee) { - CAmount minFeeLimit = minIncrementalFee.GetFeePerK() / 2; + CAmount minFeeLimit = std::max(CAmount(1), minIncrementalFee.GetFeePerK() / 2); feeset.insert(0); for (double bucketBoundary = minFeeLimit; bucketBoundary <= MAX_FEERATE; bucketBoundary *= FEE_SPACING) { feeset.insert(bucketBoundary); -- cgit v1.2.3 From 27765b6403cece54320374b37afb01a0cfe571c3 Mon Sep 17 00:00:00 2001 From: isle2983 Date: Sat, 31 Dec 2016 11:01:21 -0700 Subject: Increment MIT Licence copyright header year on files modified in 2016 Edited via: $ contrib/devtools/copyright_header.py update . --- src/policy/fees.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 9eb831bc1..0f2e3c100 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2015 The Bitcoin Core developers +// Copyright (c) 2009-2016 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -- cgit v1.2.3 From 4df44794c9f71d47648e858385d37eae6d0a9db3 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 10 Nov 2016 14:16:42 -0500 Subject: Remove extraneous LogPrint from fee estimation Once priority estimation was removed, not all transactions in the mempool are tracked in the fee estimation mempool tracking. So there is no error if a transaction is not found for removal. --- src/policy/fees.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index a1b785e3e..52d0e5c5e 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -281,19 +281,16 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe } } -void CBlockPolicyEstimator::removeTx(uint256 hash) +bool CBlockPolicyEstimator::removeTx(uint256 hash) { std::map::iterator pos = mapMemPoolTxs.find(hash); - if (pos == mapMemPoolTxs.end()) { - LogPrint("estimatefee", "Blockpolicy error mempool tx %s not found for removeTx\n", - hash.ToString().c_str()); - return; + if (pos != mapMemPoolTxs.end()) { + feeStats.removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex); + mapMemPoolTxs.erase(hash); + return true; + } else { + return false; } - unsigned int entryHeight = pos->second.blockHeight; - unsigned int bucketIndex = pos->second.bucketIndex; - - feeStats.removeTx(entryHeight, nBestSeenHeight, bucketIndex); - mapMemPoolTxs.erase(hash); } CBlockPolicyEstimator::CBlockPolicyEstimator(const CFeeRate& _minRelayFee) -- cgit v1.2.3 From 84f7ab08d2e8e83a584d72fdf44f68b34baf8165 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 11 Nov 2016 11:57:51 -0500 Subject: Remove member variable hadNoDependencies from CTxMemPoolEntry Fee estimation can just check its own mapMemPoolTxs to determine the same information. Note that now fee estimation for block processing must happen before those transactions are removed, but this shoudl be a speedup. --- src/policy/fees.cpp | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 52d0e5c5e..ac2d7edae 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -327,13 +327,6 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo if (!fCurrentEstimate) return; - if (!entry.WasClearAtEntry()) { - // This transaction depends on other transactions in the mempool to - // be included in a block before it will be able to be included, so - // we shouldn't include it in our calculations - return; - } - // Feerates are stored and reported as BTC-per-kb: CFeeRate feeRate(entry.GetFee(), entry.GetTxSize()); @@ -343,10 +336,8 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry& entry) { - if (!entry.WasClearAtEntry()) { - // This transaction depended on other transactions in the mempool to - // be included in a block before it was able to be included, so - // we shouldn't include it in our calculations + if (!removeTx(entry.GetTx().GetHash())) { + // This transaction wasn't being tracked for fee estimation return; } @@ -378,14 +369,18 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, // transaction fees." return; } - nBestSeenHeight = nBlockHeight; // Only want to be updating estimates when our blockchain is synced, // otherwise we'll miscalculate how many blocks its taking to get included. if (!fCurrentEstimate) return; - // Clear the current block state + // Must update nBestSeenHeight in sync with ClearCurrent so that + // calls to removeTx (via processBlockTx) correctly calculate age + // of unconfirmed txs to remove from tracking. + nBestSeenHeight = nBlockHeight; + + // Clear the current block state and update unconfirmed circular buffer feeStats.ClearCurrent(nBlockHeight); // Repopulate the current block states -- cgit v1.2.3 From 6f06b268c1f383affb2cf397f325d48d25bc8880 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 11 Nov 2016 12:48:01 -0500 Subject: rename bool to validFeeEstimate --- src/policy/fees.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index ac2d7edae..6b4567d19 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -306,7 +306,7 @@ CBlockPolicyEstimator::CBlockPolicyEstimator(const CFeeRate& _minRelayFee) feeStats.Initialize(vfeelist, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); } -void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, bool fCurrentEstimate) +void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, bool validFeeEstimate) { unsigned int txHeight = entry.GetHeight(); uint256 hash = entry.GetTx().GetHash(); @@ -324,7 +324,7 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo // Only want to be updating estimates when our blockchain is synced, // otherwise we'll miscalculate how many blocks its taking to get included. - if (!fCurrentEstimate) + if (!validFeeEstimate) return; // Feerates are stored and reported as BTC-per-kb: -- cgit v1.2.3 From d825838e6472f73c491f93506cb003472f071602 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 11 Nov 2016 13:14:45 -0500 Subject: Always update fee estimates on new blocks. All decisions about whether the transactions are valid data points are made at the time the transaction arrives. Updating on blocks all the time will now cause stale fee estimates to decay quickly when we restart a node. --- src/policy/fees.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 6b4567d19..eb9fdc77d 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -359,7 +359,7 @@ void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM } void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, - std::vector& entries, bool fCurrentEstimate) + std::vector& entries) { if (nBlockHeight <= nBestSeenHeight) { // Ignore side chains and re-orgs; assuming they are random @@ -370,11 +370,6 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, return; } - // Only want to be updating estimates when our blockchain is synced, - // otherwise we'll miscalculate how many blocks its taking to get included. - if (!fCurrentEstimate) - return; - // Must update nBestSeenHeight in sync with ClearCurrent so that // calls to removeTx (via processBlockTx) correctly calculate age // of unconfirmed txs to remove from tracking. -- cgit v1.2.3 From ebafdcabb10a89b491cdb8430bc43b0220d436df Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 11 Nov 2016 14:16:42 -0500 Subject: Pass pointers to existing CTxMemPoolEntries to fee estimation --- src/policy/fees.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index eb9fdc77d..cb83bcf71 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -334,9 +334,9 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo mapMemPoolTxs[hash].bucketIndex = feeStats.NewTx(txHeight, (double)feeRate.GetFeePerK()); } -void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry& entry) +void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry) { - if (!removeTx(entry.GetTx().GetHash())) { + if (!removeTx(entry->GetTx().GetHash())) { // This transaction wasn't being tracked for fee estimation return; } @@ -344,7 +344,7 @@ void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM // How many blocks did it take for miners to include this transaction? // blocksToConfirm is 1-based, so a transaction included in the earliest // possible block has confirmation count of 1 - int blocksToConfirm = nBlockHeight - entry.GetHeight(); + int blocksToConfirm = nBlockHeight - entry->GetHeight(); if (blocksToConfirm <= 0) { // This can't happen because we don't process transactions from a block with a height // lower than our greatest seen height @@ -353,13 +353,13 @@ void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM } // Feerates are stored and reported as BTC-per-kb: - CFeeRate feeRate(entry.GetFee(), entry.GetTxSize()); + CFeeRate feeRate(entry->GetFee(), entry->GetTxSize()); feeStats.Record(blocksToConfirm, (double)feeRate.GetFeePerK()); } void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, - std::vector& entries) + std::vector& entries) { if (nBlockHeight <= nBestSeenHeight) { // Ignore side chains and re-orgs; assuming they are random -- cgit v1.2.3 From 5fe0f47aa7d4cd4ed58ad37e6e4035f5c0625a23 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 11 Nov 2016 13:40:27 -0500 Subject: Add extra logging to processBlock in fee estimation. --- src/policy/fees.cpp | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index cb83bcf71..ec1dd492c 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -294,7 +294,7 @@ bool CBlockPolicyEstimator::removeTx(uint256 hash) } CBlockPolicyEstimator::CBlockPolicyEstimator(const CFeeRate& _minRelayFee) - : nBestSeenHeight(0) + : nBestSeenHeight(0), trackedTxs(0), untrackedTxs(0) { static_assert(MIN_FEERATE > 0, "Min feerate must be nonzero"); minTrackedFee = _minRelayFee < CFeeRate(MIN_FEERATE) ? CFeeRate(MIN_FEERATE) : _minRelayFee; @@ -324,8 +324,11 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo // Only want to be updating estimates when our blockchain is synced, // otherwise we'll miscalculate how many blocks its taking to get included. - if (!validFeeEstimate) + if (!validFeeEstimate) { + untrackedTxs++; return; + } + trackedTxs++; // Feerates are stored and reported as BTC-per-kb: CFeeRate feeRate(entry.GetFee(), entry.GetTxSize()); @@ -334,11 +337,11 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo mapMemPoolTxs[hash].bucketIndex = feeStats.NewTx(txHeight, (double)feeRate.GetFeePerK()); } -void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry) +bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry) { if (!removeTx(entry->GetTx().GetHash())) { // This transaction wasn't being tracked for fee estimation - return; + return false; } // How many blocks did it take for miners to include this transaction? @@ -349,13 +352,14 @@ void CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM // This can't happen because we don't process transactions from a block with a height // lower than our greatest seen height LogPrint("estimatefee", "Blockpolicy error Transaction had negative blocksToConfirm\n"); - return; + return false; } // Feerates are stored and reported as BTC-per-kb: CFeeRate feeRate(entry->GetFee(), entry->GetTxSize()); feeStats.Record(blocksToConfirm, (double)feeRate.GetFeePerK()); + return true; } void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, @@ -378,15 +382,21 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, // Clear the current block state and update unconfirmed circular buffer feeStats.ClearCurrent(nBlockHeight); + unsigned int countedTxs = 0; // Repopulate the current block states - for (unsigned int i = 0; i < entries.size(); i++) - processBlockTx(nBlockHeight, entries[i]); + 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(); - LogPrint("estimatefee", "Blockpolicy after updating estimates for %u confirmed entries, new mempool map size %u\n", - entries.size(), mapMemPoolTxs.size()); + LogPrint("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()); + + trackedTxs = 0; + untrackedTxs = 0; } CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget) -- cgit v1.2.3 From 78ae62d2646c5e6db410f324046d0c3b15e0ad0a Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 29 Nov 2016 13:55:26 -0500 Subject: Add clarifying comments to fee estimation --- src/policy/fees.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index ec1dd492c..f1c93a497 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -281,6 +281,11 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe } } +// This function is called from CTxMemPool::removeUnchecked to ensure +// txs removed from the mempool for any reason are no longer +// 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) { std::map::iterator pos = mapMemPoolTxs.find(hash); -- cgit v1.2.3 From 44b64b933dec10f5ffcd7f34e7bf5e4ed97f671e Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 29 Nov 2016 15:40:03 -0500 Subject: Fix edge case with stale fee estimates --- src/policy/fees.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/policy/fees.cpp') diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index f1c93a497..5407aefb4 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -321,9 +321,11 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo return; } - if (txHeight < nBestSeenHeight) { + if (txHeight != nBestSeenHeight) { // Ignore side chains and re-orgs; assuming they are random they don't // affect the estimate. We'll potentially double count transactions in 1-block reorgs. + // Ignore txs if BlockPolicyEstimator is not in sync with chainActive.Tip(). + // It will be synced next time a block is processed. return; } -- cgit v1.2.3