Browse Source

Force explicit double -> int conversion for CFeeRate constructor

This resolves an issue where estimatesmartfee would return 999
sat/byte instead of 1000, due to floating point loss of precision

Thanks to sipa for suggesting is_integral.
0.16
Matt Corallo 7 years ago
parent
commit
1789e4675b
  1. 7
      src/policy/feerate.h
  2. 4
      src/policy/fees.cpp
  3. 6
      src/test/mempool_tests.cpp
  4. 4
      src/txmempool.cpp

7
src/policy/feerate.h

@ -20,10 +20,15 @@ class CFeeRate
{ {
private: private:
CAmount nSatoshisPerK; // unit is satoshis-per-1,000-bytes CAmount nSatoshisPerK; // unit is satoshis-per-1,000-bytes
public: public:
/** Fee rate of 0 satoshis per kB */ /** Fee rate of 0 satoshis per kB */
CFeeRate() : nSatoshisPerK(0) { } CFeeRate() : nSatoshisPerK(0) { }
explicit CFeeRate(const CAmount& _nSatoshisPerK): nSatoshisPerK(_nSatoshisPerK) { } template<typename I>
CFeeRate(const I _nSatoshisPerK): nSatoshisPerK(_nSatoshisPerK) {
// We've previously had bugs creep in from silent double->int conversion...
static_assert(std::is_integral<I>::value, "CFeeRate should be used without floats");
}
/** Constructor for a fee rate in satoshis per kB. The size in bytes must not exceed (2^63 - 1)*/ /** Constructor for a fee rate in satoshis per kB. The size in bytes must not exceed (2^63 - 1)*/
CFeeRate(const CAmount& nFeePaid, size_t nBytes); CFeeRate(const CAmount& nFeePaid, size_t nBytes);
/** /**

4
src/policy/fees.cpp

@ -714,7 +714,7 @@ CFeeRate CBlockPolicyEstimator::estimateRawFee(int confTarget, double successThr
if (median < 0) if (median < 0)
return CFeeRate(0); return CFeeRate(0);
return CFeeRate(median); return CFeeRate(llround(median));
} }
unsigned int CBlockPolicyEstimator::HighestTargetTracked(FeeEstimateHorizon horizon) const unsigned int CBlockPolicyEstimator::HighestTargetTracked(FeeEstimateHorizon horizon) const
@ -901,7 +901,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation
if (median < 0) return CFeeRate(0); // error condition if (median < 0) return CFeeRate(0); // error condition
return CFeeRate(median); return CFeeRate(llround(median));
} }

6
src/test/mempool_tests.cpp

@ -559,15 +559,15 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
// ... we should keep the same min fee until we get a block // ... we should keep the same min fee until we get a block
pool.removeForBlock(vtx, 1); pool.removeForBlock(vtx, 1);
SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE); SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE);
BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/2); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), llround((maxFeeRateRemoved.GetFeePerK() + 1000)/2.0));
// ... then feerate should drop 1/2 each halflife // ... then feerate should drop 1/2 each halflife
SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2); SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2);
BOOST_CHECK_EQUAL(pool.GetMinFee(pool.DynamicMemoryUsage() * 5 / 2).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/4); BOOST_CHECK_EQUAL(pool.GetMinFee(pool.DynamicMemoryUsage() * 5 / 2).GetFeePerK(), llround((maxFeeRateRemoved.GetFeePerK() + 1000)/4.0));
// ... with a 1/2 halflife when mempool is < 1/2 its target size // ... with a 1/2 halflife when mempool is < 1/2 its target size
SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4); SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4);
BOOST_CHECK_EQUAL(pool.GetMinFee(pool.DynamicMemoryUsage() * 9 / 2).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/8); BOOST_CHECK_EQUAL(pool.GetMinFee(pool.DynamicMemoryUsage() * 9 / 2).GetFeePerK(), llround((maxFeeRateRemoved.GetFeePerK() + 1000)/8.0));
// ... with a 1/4 halflife when mempool is < 1/4 its target size // ... with a 1/4 halflife when mempool is < 1/4 its target size
SetMockTime(42 + 7*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4); SetMockTime(42 + 7*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4);

4
src/txmempool.cpp

@ -981,7 +981,7 @@ const CTxMemPool::setEntries & CTxMemPool::GetMemPoolChildren(txiter entry) cons
CFeeRate CTxMemPool::GetMinFee(size_t sizelimit) const { CFeeRate CTxMemPool::GetMinFee(size_t sizelimit) const {
LOCK(cs); LOCK(cs);
if (!blockSinceLastRollingFeeBump || rollingMinimumFeeRate == 0) if (!blockSinceLastRollingFeeBump || rollingMinimumFeeRate == 0)
return CFeeRate(rollingMinimumFeeRate); return CFeeRate(llround(rollingMinimumFeeRate));
int64_t time = GetTime(); int64_t time = GetTime();
if (time > lastRollingFeeUpdate + 10) { if (time > lastRollingFeeUpdate + 10) {
@ -999,7 +999,7 @@ CFeeRate CTxMemPool::GetMinFee(size_t sizelimit) const {
return CFeeRate(0); return CFeeRate(0);
} }
} }
return std::max(CFeeRate(rollingMinimumFeeRate), incrementalRelayFee); return std::max(CFeeRate(llround(rollingMinimumFeeRate)), incrementalRelayFee);
} }
void CTxMemPool::trackPackageRemoved(const CFeeRate& rate) { void CTxMemPool::trackPackageRemoved(const CFeeRate& rate) {

Loading…
Cancel
Save