diff --git a/src/policy/fees/mempool_estimator.cpp b/src/policy/fees/mempool_estimator.cpp index 5e86fc9571e..7c9af99cb7a 100644 --- a/src/policy/fees/mempool_estimator.cpp +++ b/src/policy/fees/mempool_estimator.cpp @@ -17,6 +17,7 @@ #include #include +#include MemPoolFeeRateEstimator::Percentiles MemPoolFeeRateEstimator::CalculateMaxWeightPercentiles(std::span chunk_feerates) { @@ -39,6 +40,25 @@ MemPoolFeeRateEstimator::Percentiles MemPoolFeeRateEstimator::CalculateMaxWeight return percentiles; } +bool MemPoolFeeRateEstimatorCache::IsStale() const +{ + return !m_fee_rate_estimation || (m_last_updated + CACHE_LIFE) < NodeClock::now(); +} + +std::optional +MemPoolFeeRateEstimatorCache::GetCachedEstimate(const uint256& tip_hash) const +{ + if (IsStale() || tip_hash != m_tip_hash) return std::nullopt; + return m_fee_rate_estimation; +} + +void MemPoolFeeRateEstimatorCache::Update(FeePerVSize conservative, FeePerVSize economical, const uint256& tip_hash) +{ + m_fee_rate_estimation = {conservative, economical}; + m_tip_hash = tip_hash; + m_last_updated = NodeClock::now(); +} + //! Build the error result for a failed mempool fee rate estimation. static util::Unexpected EstimationError(std::string error) { @@ -51,6 +71,25 @@ util::Expected MemPoolFeeRateEstimato if (!m_mempool.GetLoadTried()) { return EstimationError(strprintf("%s: Mempool not loaded yet, no fee rate estimate available", FeeRateEstimatorTypeToString(estimator_type))); } + // The estimator lock is not held while building a block template, so + // in a rare edge case concurrent callers may duplicate work. + // + // Cached fee rate estimates are tagged with the chain tip they were computed on + // and only served from the cache while that tip is current. + // + // The fee rate estimate returned directly below may still reflect a tip that went + // stale during the call; that is an accepted tradeoff of not holding + // locks across block assembly. + { + const uint256 tip_hash{WITH_LOCK(::cs_main, return Assume(m_chainman.CurrentChainstate().m_chain.Tip())->GetBlockHash())}; + LOCK(cs); + const auto cached_estimate = m_cache.GetCachedEstimate(tip_hash); + if (cached_estimate) { + const auto cached_feerate{ + conservative ? cached_estimate->m_conservative : cached_estimate->m_economical}; + return FeeRateEstimation{estimator_type, cached_feerate, MEMPOOL_FEE_ESTIMATOR_MAX_TARGET}; + } + } node::BlockCreateOptions options; options.test_block_validity = false; const auto blocktemplate = WITH_LOCK(::cs_main, return (node::BlockAssembler{m_chainman.CurrentChainstate(), &m_mempool, options}).CreateNewBlock()); @@ -63,6 +102,7 @@ util::Expected MemPoolFeeRateEstimato const FeePerVSize floor{std::max(m_mempool.m_opts.min_relay_feerate, m_mempool.GetMinFee()).GetFeePerVSize()}; const FeePerVSize p50{percentiles.p50.IsEmpty() ? floor : percentiles.p50}; const FeePerVSize p75{percentiles.p75.IsEmpty() ? floor : percentiles.p75}; + WITH_LOCK(cs, m_cache.Update(p50, p75, blocktemplate->block.hashPrevBlock)); LogDebug(BCLog::ESTIMATEFEE, "%s: conservative/economical fee rate: %s/%s %s/kvB", FeeRateEstimatorTypeToString(estimator_type), CFeeRate(p50).GetFeePerK(), CFeeRate(p75).GetFeePerK(), CURRENCY_ATOM); diff --git a/src/policy/fees/mempool_estimator.h b/src/policy/fees/mempool_estimator.h index 147c233667e..82a90600aa5 100644 --- a/src/policy/fees/mempool_estimator.h +++ b/src/policy/fees/mempool_estimator.h @@ -5,10 +5,16 @@ #ifndef BITCOIN_POLICY_FEES_MEMPOOL_ESTIMATOR_H #define BITCOIN_POLICY_FEES_MEMPOOL_ESTIMATOR_H +#include +#include +#include #include #include #include +#include +#include +#include #include class ChainstateManager; @@ -17,6 +23,35 @@ class CTxMemPool; // Fee rate estimate for confirmation target above this is not reliable, // as mempool conditions are likely to change. constexpr int MEMPOOL_FEE_ESTIMATOR_MAX_TARGET{2}; +constexpr std::chrono::seconds CACHE_LIFE{7}; + +/** + * MemPoolFeeRateEstimatorCache holds a cache of recent fee rate estimates. + * A cached fee rate is only provided while it is not older than CACHE_LIFE + * and the chain tip has not changed. + */ +class MemPoolFeeRateEstimatorCache +{ +public: + MemPoolFeeRateEstimatorCache() = default; + MemPoolFeeRateEstimatorCache(const MemPoolFeeRateEstimatorCache&) = delete; + MemPoolFeeRateEstimatorCache& operator=(const MemPoolFeeRateEstimatorCache&) = delete; + /** Returns true if the cache is empty or older than CACHE_LIFE. */ + bool IsStale() const; + struct FeeRateEstimate { + FeePerVSize m_conservative; + FeePerVSize m_economical; + }; + /** Returns cached estimates if not stale and computed on tip_hash, nullopt otherwise. */ + std::optional GetCachedEstimate(const uint256& tip_hash) const; + /** Update the cache with new estimates computed on tip_hash. */ + void Update(FeePerVSize conservative, FeePerVSize economical, const uint256& tip_hash); + +private: + std::optional m_fee_rate_estimation; + uint256 m_tip_hash; + NodeClock::time_point m_last_updated{}; +}; /** * Estimate the fee rate required for a transaction to be included in the next block. @@ -44,7 +79,8 @@ public: * @param[in] chunk_feerates Block template chunk fee rates sorted by descending mining score. */ static Percentiles CalculateMaxWeightPercentiles(std::span chunk_feerates); - util::Expected EstimateFeeRate(bool conservative) const; + util::Expected EstimateFeeRate(bool conservative) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); unsigned int MaximumTarget() const { return MEMPOOL_FEE_ESTIMATOR_MAX_TARGET; @@ -53,6 +89,8 @@ public: private: const CTxMemPool& m_mempool; ChainstateManager& m_chainman; + mutable Mutex cs; + mutable MemPoolFeeRateEstimatorCache m_cache GUARDED_BY(cs); }; #endif // BITCOIN_POLICY_FEES_MEMPOOL_ESTIMATOR_H diff --git a/src/test/mempool_fee_estimator_tests.cpp b/src/test/mempool_fee_estimator_tests.cpp index 0bad545962a..2331ae7c6b7 100644 --- a/src/test/mempool_fee_estimator_tests.cpp +++ b/src/test/mempool_fee_estimator_tests.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -73,6 +74,31 @@ BOOST_AUTO_TEST_CASE(calculate_max_weight_percentiles) BOOST_CHECK(ByRatio{percentiles.p50} > ByRatio{percentiles.p75}); } +BOOST_AUTO_TEST_CASE(mempool_fee_rate_estimator_cache) +{ + MemPoolFeeRateEstimatorCache cache; + const uint256 tip_hash{uint256::ONE}; + const uint256 next_tip_hash{uint256{2}}; + const FeePerVSize conservative{2, 1}; + const FeePerVSize economical{1, 1}; + + BOOST_CHECK(cache.IsStale()); + BOOST_CHECK(!cache.GetCachedEstimate(tip_hash)); + + cache.Update(conservative, economical, tip_hash); + BOOST_CHECK(!cache.IsStale()); + const auto cached{cache.GetCachedEstimate(tip_hash)}; + BOOST_REQUIRE(cached); + BOOST_CHECK(cached->m_conservative == conservative); + BOOST_CHECK(cached->m_economical == economical); + BOOST_CHECK(!cache.GetCachedEstimate(next_tip_hash)); + + SetMockTime(GetTime() + CACHE_LIFE + std::chrono::seconds{1}); + BOOST_CHECK(cache.IsStale()); + BOOST_CHECK(!cache.GetCachedEstimate(tip_hash)); + SetMockTime(0); +} + BOOST_AUTO_TEST_CASE(MempoolFeeRateEstimator) { auto mempool_estimator = MemPoolFeeRateEstimator(*m_node.mempool, *m_node.chainman); @@ -100,12 +126,18 @@ BOOST_AUTO_TEST_CASE(MempoolFeeRateEstimator) BOOST_CHECK(result->feerate == floor); BOOST_CHECK(result->feerate_estimator == FeeRateEstimatorType::MEMPOOL_POLICY); BOOST_CHECK_EQUAL(result->returned_target, MEMPOOL_FEE_ESTIMATOR_MAX_TARGET); + + // The floor estimate is cached like any other; a second call returns the same value. + const auto cached_result = mempool_estimator.EstimateFeeRate(/*conservative=*/true); + BOOST_REQUIRE(cached_result.has_value()); + BOOST_CHECK(cached_result->feerate == floor); } TestMemPoolEntryHelper entry; const auto tx_vsize = entry.FromTx(MakeRandomTx()).GetTxSize(); const CAmount low_fee{CENT / 3000}; const CAmount med_fee{CENT / 100}; const CAmount high_fee{CENT / 10}; + const CAmount very_high_fee{CENT}; // A mempool that cannot fill 50% of a block leaves both percentiles empty, // so both estimate still fall back to the floor. { @@ -116,6 +148,8 @@ BOOST_AUTO_TEST_CASE(MempoolFeeRateEstimator) TryAddToMempool(*m_node.mempool, entry.Fee(high_fee).FromTx(MakeRandomTx())); } } + // Expire the cached floor estimate so the denser mempool is observed. + SetMockTime(GetTime() + CACHE_LIFE + std::chrono::seconds{1}); const auto result = mempool_estimator.EstimateFeeRate(/*conservative=*/true); BOOST_REQUIRE(result.has_value()); BOOST_CHECK(result->feerate == floor); @@ -130,6 +164,7 @@ BOOST_AUTO_TEST_CASE(MempoolFeeRateEstimator) TryAddToMempool(*m_node.mempool, entry.Fee(med_fee).FromTx(MakeRandomTx())); } } + SetMockTime(GetTime() + CACHE_LIFE + std::chrono::seconds{1}); const auto conservative = mempool_estimator.EstimateFeeRate(/*conservative=*/true); const auto economical = mempool_estimator.EstimateFeeRate(/*conservative=*/false); BOOST_REQUIRE(conservative.has_value()); @@ -147,6 +182,8 @@ BOOST_AUTO_TEST_CASE(MempoolFeeRateEstimator) TryAddToMempool(*m_node.mempool, entry.Fee(low_fee).FromTx(MakeRandomTx())); } } + // Expire the sparse-result cache before expecting the estimator to observe the denser mempool. + SetMockTime(GetTime() + CACHE_LIFE + std::chrono::seconds{1}); const auto result_conservative = mempool_estimator.EstimateFeeRate(/*conservative=*/true); const auto result_economical = mempool_estimator.EstimateFeeRate(/*conservative=*/false); BOOST_CHECK(result_conservative.has_value()); @@ -158,6 +195,22 @@ BOOST_AUTO_TEST_CASE(MempoolFeeRateEstimator) BOOST_CHECK(result_economical->feerate_estimator == FeeRateEstimatorType::MEMPOOL_POLICY); BOOST_CHECK_EQUAL(result_conservative->returned_target, MEMPOOL_FEE_ESTIMATOR_MAX_TARGET); BOOST_CHECK_EQUAL(result_economical->returned_target, MEMPOOL_FEE_ESTIMATOR_MAX_TARGET); + + // Adding another 30% of very-high-fee transactions should change the + // estimates after recomputation, but not while the cached estimate is fresh. + { + LOCK2(cs_main, m_node.mempool->cs); + while ((m_node.mempool->GetTotalTxSize() * WITNESS_SCALE_FACTOR) <= + (DEFAULT_BLOCK_MAX_WEIGHT * 105 / 100)) { + TryAddToMempool(*m_node.mempool, entry.Fee(very_high_fee).FromTx(MakeRandomTx())); + } + } + BOOST_CHECK(mempool_estimator.EstimateFeeRate(/*conservative=*/false).value().feerate == FeeFrac(low_fee, tx_vsize)); + BOOST_CHECK(mempool_estimator.EstimateFeeRate(/*conservative=*/true).value().feerate == FeeFrac(med_fee, tx_vsize)); + // Expire the cache by advancing mock time past CACHE_LIFE so the next call recomputes. + SetMockTime(GetTime() + CACHE_LIFE + std::chrono::seconds{1}); + BOOST_CHECK(mempool_estimator.EstimateFeeRate(/*conservative=*/false).value().feerate == FeeFrac(med_fee, tx_vsize)); + BOOST_CHECK(mempool_estimator.EstimateFeeRate(/*conservative=*/true).value().feerate == FeeFrac(high_fee, tx_vsize)); } }