#36182 fees: return `block_policy` fee rate estimate when `mempool_policy` is not ready

ReviewabilityReviewsAgreementSize
#36182 ismaelsadeeq fees: return `block_policy` fee rate estimate when `mempool_policy` is not readyNeeds rebase0 (+1)Mild108 + 145 tests
  • Prevent transaction fee underestimation and stuck transactions when mempool data is sparse or unready
  • Provide reliable fee estimates by falling back to block history instead of failing or using the fee floor
  • Ready to review
  • Nothing known that would invalidate a review now
  • Support for falling back to block policy to resolve downstream failures (instagibbs, stickies-v, achow101)
  • Confirmed to fix the issue through testing (tankyleo, polespinasa)
  • Objection that starved mempools will overestimate fees was answered with no reply (davidgumberg)
  • Unaddressed suggestion to add test coverage for block policy fallback (l0rinc)
  • 108 lines added or modified outside tests
  • 145 lines added or modified in tests
  • 139 lines removed in total
  • 14 files, 5 commits
Mempool and policy#2 of 14
P2 · bug fix
  • P2 because it prevents severe fee underestimation that risks stuck transactions after node restarts
  • Resolves total fee estimation failures reported by downstream services like LDK when mempools are not ready
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Makes the default combined fee estimator fall back to the block policy estimate whenever the mempool estimator is unavailable, returning an error only if the block policy estimate also fails. Changes the mempool estimator to return an error rather than the relay fee floor when the mempool is too sparse to calculate block-template percentiles. Also clears the mempool estimator's tracked mined-block history if the mempool fails to load at startup, and renames the combined estimator RPC option and enum from "none" to "auto" (retaining "none" for backward compatibility).

Problem: When the mempool estimator is not ready (such as during initial startup, before tracking enough blocks, or under low coverage), the default combined estimator failed completely instead of using available block policy data. Furthermore, when the mempool was empty or sparse (such as after a restart without persistence or with a corrupt mempool.dat), the mempool estimator returned the fee floor, causing significant fee underestimation and risking stuck transactions.

Evidence of importance:

  • PR description: "Previously, case (ii) with a healthy mempool will returned the fee rate floor (the higher of the minimum relay fee and the current mempool minimum fee). This can be a significant underestimation, since the now-missing mempool may have held much higher-feerate transactions."
  • Commit 305f7df895: "When the mempool fails to load at startup... init notifies the fee_rate_estimator_man, which clears its tracked mined-block window. Otherwise the estimator would keep a window describing a mempool the node no longer has and, once the mempool refilled, serve an estimate built on it."

Risk of a mistake: medium (mempool_policy, rpc_api). A flaw in fee estimation fallback or percentile calculation could cause wallet users or RPC callers to significantly underpay or overpay transaction fees, resulting in delayed confirmations or wasted funds.

Changed since the previous assessment: No functional changes to the design; includes minor test additions and rebases since the previous version.

Reviewability

Ready · Needs rebase

Conflicts with master; review the logic now, the rebase is mechanical.

Waiting on (from the thread read):

  • author: unanswered suggestion from l0rinc to add test coverage for block policy fallback 2026-09-23
  • reviewer: w0xlt to verify fixes for comments and logging 2026-09-28
  • decision: further discussion on distinguishing empty mempool from missing backlog 2026-09-28

Agreement

Mild

objection that needs a change was answered, no reply from the objector (davidgumberg: Falling back to block policy instead of returning a floored estimate when a healthy mempool has fewer txs than target pe)

Thread notes: Updated claims from w0xlt as fixed following author's inline replies and fixes. Updated davidgumberg's objection to answered after the author noted an offline agreement to split the PR and converted it to draft.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
jeanpablojp (contributor)suggestion, correctnessrefill the mined-block window before each cold restart test case so the assertions test dropping statsfixed · fix pushed · author refilled the window before each test caseansweredyes2026-09-07: 'test_fallback_when_mempool_estimator_not_ready deletes `fees/mempool_policy_estimator.dat` just above and nothing mines after it, so all three cases here start from an empty window, where `MempoolRelo'
Status from: 2026-09-08: ismaelsadeeq: 'Indeed, fixed with some modifications.'
pseudoramdom (member)suggestion, interfaceuse util::Expected<void, MempoolEstimationFailure> instead of std::optional for GetMempoolHealthCheck return typefixed · fix pushed · author updated health check methods to return util::Expectedansweredyes2026-09-10: 'I think this would read better if we made the return type as `util::Expected<void, MempoolEstimationFailure>`'
Status from: 2026-09-11: ismaelsadeeq: 'Good idea, taken.'
willcl-ark (member)objection, approachClear the full mined-block window rather than keeping partial stats when mempool reload failsfixed · fix pushed · author implemented clearing all stats in pushansweredyes2026-09-14: 'I still find myself thinking we should clear the whole mined-block window when the mempool does not reload.'
Status from: 2026-09-14: ismaelsadeeq: 'I pushed both in the new push, clearing the stats and also having a warm-up time where we suspect the mempool is sparse'
willcl-ark (member)objection, interfaceFall back to block policy on LOW_COVERAGE in combined modefixed · fix pushed · author force-pushed fix to fall back on LOW_COVERAGEansweredyes2026-09-15: 'Should `LOW_COVERAGE` also fall back to the block-policy estimate in combined mode?'
Status from: 2026-09-15: ismaelsadeeq: 'Indeed, I force-pushed fb385bfaac...a623083db1 to fix that.'
davidgumberg (member)objection, styleReturn FeeRateEstimationError directly rather than wrapped in util::Unexpectedfixed · fix pushed · author removed the helper function in a later simplification pushansweredyes2026-09-15: 'Should return `FeeRateEstimationError` directly instead of wrapped in `util::Unexpected`? This makes the callers much clearer IMO'
Status from: 2026-09-23: ismaelsadeeq: 'This is gone now.'
davidgumberg (member)objection, correctnessWhy does warmup end height need to be persisted across restarts?fixed · fix pushed · author explained, pushed a fix, and later removed the warmup logic entirelyansweredyes2026-09-16: 'Why does this need to be persisted? In what case would it make sense for a warmup to start and get interrupted and then continue?'
Status from: 2026-09-17: ismaelsadeeq: 'Pushed a fix, warmup now re-starts whenever the tracked mined-block window is empty'
davidgumberg (member)objection, approachFall back to block policy on any mempool estimator error, including while mempool is loadingfixed · fix pushed · author adopted the suggestion in pushansweredyes2026-09-16: 'I am slightly inclined to suggest for the sake of simplicity that whatever the error is, just fall back to block policy.'
Status from: 2026-09-17: ismaelsadeeq: 'Hmm, yeah, for simplicity, I took your suggestion.'
willcl-ark (member)objection, correctnessShould the 144-block warmup count blocks processed during IBD catch-up?fixed · fix pushed · author addressed in push before warmup logic was completely replacedansweredyes2026-09-21: 'is the 144-block warm-up meant to count blocks processed during IBD catch-up?'
Status from: 2026-09-22: ismaelsadeeq: 'Good catch, I attempted to fix it by adding a new bool to the notification that tells us whether the chainstate is curre'
l0rinc (member)objection, correctnessCould fee estimator accept stale data during IBD catch-up and slow down IBD?fixed · fix pushed · author simplified the logic in push, removing the warmup mechanismansweredyes2026-09-22: 'this check may accept saved fee-estimator data that becomes stale during IBD catch-up. While it makes sense to me superficially, I haven't dug very deep. Could you check whether it's valid and should '
Status from: 2026-09-23: ismaelsadeeq: 'So I simplify by disallowing the fee rate return flow completely; this is simpler and should handle the most important l'
l0rinc (member)objection, approachShould returning an error when block policy has no result be covered, or is relying on mempool alone desirable?answered · author clarified it is intentional belt-and-suspenders behavioransweredyes2026-09-23: 'if the block-policy estimator has no result, this returns an error even when the mempool estimator can provide a fee rate. I put a small test and proposed change in https://github.com/l0rinc/bitcoin/p'
Status from: 2026-09-23: ismaelsadeeq: 'This is the desired behaviour, it's a belt-and-suspenders on `mempool_policy` fee rate estimator'
l0rinc (member)suggestion, correctnessadd test coverage documenting that estimator_man returns an error when block policy has no result even if the mempool estimator can provide a fee rateopenansweredno2026-09-23: 'If it's a desired behavior, consider covering it with a test to document that - please see my attempts above.'
davidgumberg (member)suggestion, interfaceavoid logging that mined-block stats are cleared when starting fresh with an empty vectorfixed · fix pushed · author prevented logging when stats vector is emptyansweredyes2026-09-23: 'clearing `m_prev_mined_blocks` seems totally fine but the log message is a bit odd, but this is also fine as-is or addressable in a follow-up.'
Status from: 2026-09-24: davidgumberg: 'Latest push fixed this, thanks @ismaelsadeeq.'
davidgumberg (member)objection, approachFalling back to block policy instead of returning a floored estimate when a healthy mempool has fewer txs than target percentile overestimates fees when mempool is legitimately starvedanswered · author discussed offline with reviewer and agreed to split the PR and move forward with only the first three commits, converting this to draftchangedyes2026-09-23: 'In the case of a node with a healthy view of the mempool, but a mempool that has less than a p50 or p75 of transactions, don't we actually want to return the floored estimate instead of falling back t'
Status from: 2026-09-28: ismaelsadeeq: 'We decided to move forward with the first three commits and discuss the issue further'
willcl-ark (member)suggestion, interfacerename the default estimatesmartfee fee_rate_estimator option from 'none' to 'auto'fixed · fix pushed · author added a commit renaming the default estimator option to autoansweredyes2026-09-24: 'should we rename `fee_rate_estimator=none` to `auto`, as suggested above? I agree that `none` reads as though fee estimation is disabled, whereas it selects the default combined behavior.'
Status from: 2026-09-25: ismaelsadeeq: 'I added a commit here to fix https://github.com/bitcoin/bitcoin/pull/36182#issuecomment-5817784558'
w0xlt (contributor)suggestion, maintenanceupdate stale comments in estimator_man.h to reference AUTO instead of NONE and clarify MempoolLoadFailed descriptionfixed · fix pushed · author updated stale commentsansweredyes2026-09-26: 'Stale comments:'
Status from: 2026-09-28: ismaelsadeeq: 'Fixed'
w0xlt (contributor)suggestion, maintenanceupdate comments in mempool_estimator.cpp and drop redundant check since p75 empty check covers p50fixed · fix pushed · author updated comments and removed redundant checkansweredyes2026-09-26: 'Stale comments + redundant check.'
Status from: 2026-09-28: ismaelsadeeq: 'Good catch, taken.'
w0xlt (contributor)suggestion, interfacerestore fallback debug logging that was lost in a later rewritefixed · fix pushed · author restored debug logging, omitting redundant fallback wordingansweredyes2026-09-26: '@polespinasa 's suggestion to log the fallback ([r3988008559](https://github.com/bitcoin/bitcoin/pull/36182#discussion_r3988008559)) was marked fixed, but it seems to have been lost in a later rewrite'
Status from: 2026-09-28: ismaelsadeeq: 'Fixed, and leave out the falling back to part; it is redundant because right below we log the selected fee rate.'

Support:

  • sedited (member) (Concept ACK, 2026-09-07): no reason given: Concept ACK [not substantive]
  • polespinasa (member) (Concept ACK, 2026-09-07): substantive: Would be nice to fix this, specially case 1 if https://github.com/bitcoin/bitcoin/pull/34075 is getting into v32.
  • achow101 (member) (Concept ACK, 2026-09-07): substantive: makes sense to fallback to block policy if it is available and mempool policy is not
  • jeanpablojp (contributor) (Concept ACK, 2026-09-07): no reason given: Concept ACK [not substantive]
  • pseudoramdom (member) (Concept ACK, 2026-09-10): no reason given: Concept ACK. Left a few comments. [not substantive]
  • polespinasa (member) (ACK, 2026-09-15): no reason given: Reviewed again, don't have anything to say :) Just left a small style suggestion that you can freely ignore. [not substantive]
  • instagibbs (member) (2026-09-16): substantive: got a report from LDK dev this is happening, where block estimates are working but mempool is not
  • tankyleo (none) (2026-09-17) [tested]: substantive: tested the PR, can confirm this PR resolves the issue
  • willcl-ark (member) (ACK, 2026-09-21): no reason given: Left one inline comment regarding the catch-up period which could be clarified or touched up as necessary, but on the whole I think this is OK now. [not substantive]
  • stickies-v (member) (Approach ACK, 2026-09-23): substantive: current approach of simply erroring (and falling back to block policy) whenever the mempool seems like it can't produce an estimate is the best choice for this backport

Participants:

  • achow101 (member, support, 1 statements 2026-09-07 to 2026-09-07): Concept ACK on falling back to block policy
  • jeanpablojp (contributor, support, 3 statements 2026-09-07 to 2026-09-07): Concept ACK with comments on tests and docs
  • polespinasa (member, support, 20 statements 2026-09-07 to 2026-09-17): ACKed after testing and reviewing diffs
  • sedited (member, support, 2 statements 2026-09-07 to 2026-09-07): Concept ACK and confirmed release window
  • davidgumberg (member, objection, 25 statements 2026-09-10 to 2026-09-24): objected to erroring instead of flooring when mempool is starved, and reviewed error handling and tests
  • pseudoramdom (member, support, 8 statements 2026-09-10 to 2026-09-10): Concept ACK, suggested util::Expected and naming auto
  • willcl-ark (member, objection, 6 statements 2026-09-11 to 2026-09-24): advocated clearing block stats on failed reload, questioned warmup during IBD catch-up, suggested renaming none to auto, and ACKed
  • instagibbs (member, support, 2 statements 2026-09-16 to 2026-09-16): supported, citing downstream LDK reports
  • tankyleo (none, support, 2 statements 2026-09-16 to 2026-09-17): confirmed PR resolves issue after testing
  • l0rinc (member, objection, 3 statements 2026-09-22 to 2026-09-23): raised concern on stale estimator data during IBD catch-up and questioned erroring when block policy has no result
  • stickies-v (member, support, 1 statements 2026-09-23 to 2026-09-23): Approach ACK on simplified design of erroring and falling back
  • w0xlt (contributor, neutral, 4 statements 2026-09-26 to 2026-09-26): pointed out stale comments, redundant empty checks, and lost fallback logging

Files

200 lines under test/bench/ci.

  • test/functional/feature_fee_estimation.py +113/-18
  • src/policy/fees/mempool_estimator.cpp +45/-38
  • src/test/mempool_fee_estimator_tests.cpp +25/-33
  • src/policy/fees/mempool_estimator.h +16/-18
  • src/policy/fees/estimator_man.cpp +15/-12
  • src/rpc/fees.cpp +9/-8
  • src/policy/fees/estimator_man.h +12/-2
  • src/test/fees_util_tests.cpp +5/-3
  • src/init.cpp +6/-1
  • src/util/fees.cpp +3/-3
  • src/node/interfaces.cpp +1/-1
  • src/test/fuzz/fees.cpp +1/-1
  • src/util/fees.h +1/-1
  • test/functional/rpc_estimatefee.py +1/-0

Card

The short text the ranking pass compares across PRs.

Makes the default combined fee estimator in estimatesmartfee fall back to the block policy estimate whenever the mempool estimator is unready, instead of failing or returning an error. Changes the mempool estimator to return an error rather than the relay fee floor when the mempool is too sparse to fill block percentiles, preventing severe fee underestimation on nodes with an empty or freshly reset mempool. Additionally, clears the mempool estimator's mined-block history when mempool loading fails at startup to avoid stale estimates, and renames the RPC estimator option from 'none' to 'auto'.

Data

dossier JSON · extract JSON · model ledger, generated 2026-09-29T14:29, confidence high, input hash 31fe54caa3dcada7
thread read through 2026-09-28 (132 statements), head eb28c156ee
labels: TX fees and policy, Needs rebase · draft

Categories

Mempool and policy

P2 · bug fix

Prevents severe fee underestimation and stuck transactions by returning block policy estimates rather than the relay fee floor or an outright error when the mempool estimator is unready or sparse. Downstream consumers like LDK specifically hit issues where block estimates worked but mempool estimation failed completely.

Membership: Carries the 'TX fees and policy' label and directly modifies mempool fee estimation logic in src/policy/fees/.

Factors: security/stability 1, bug 2, performance 0, user value 2, leverage 0