Skip to content

Fall back to per-target defaults for sparse Esplora fee estimates - #1042

Open
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:fix-sparse-esplora-fee-estimates
Open

Fall back to per-target defaults for sparse Esplora fee estimates#1042
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:fix-sparse-esplora-fee-estimates

Conversation

@Jolah1

@Jolah1 Jolah1 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #1028.

Esplora may return a non-empty estimate map that still has no usable entry for a given confirmation target, since convert_fee_rate only yields a value if the map holds a block count at or below the requested one. A map of {144:
2.0, 1008: 1.0} therefore leaves every target below 144 blocks — UrgentOnChainSweep, OnchainPayment, ChannelCloseMinimum, ... — without an estimate.

Today we substitute 1 sat/vb in that case. That's not a missing fallback but an actively harmful one: OnchainFeeEstimator::estimate_fee_rate already falls back to get_fallback_rate_for_target whenever it misses the cache, so
writing 1 sat/vb into the cache only serves to shadow the value we'd otherwise have used. For UrgentOnChainSweep that's 250 sats/kwu where the default is 5000 — a 20x underestimate on a target whose whole point is timeliness.

So here we fall back to the target's own default instead. The post-estimation adjustments are deliberately skipped for it: there's no estimate to adjust, and skipping them lands us on exactly the rate the cache-miss path would
have produced, which keeps the two paths consistent.

One open question for reviewers: whether that's the behaviour you want, or whether the fallback should go through apply_post_estimation_adjustments anyway. It only matters for MaximumFeeEstimate (8000 unadjusted vs. 11300
adjusted) and MinAllowedNonAnchorChannelRemoteFee (unchanged, as the subtraction clamps back to the floor). Happy to flip it — it's a one-line change plus one expected value in the test.

To make this testable without an Esplora server, the cache construction moves out of update_fee_rate_estimates into a pure build_fee_rate_cache. The new tests cover sparse maps, empty maps on and off Mainnet, and assert the
complete-map behaviour is unchanged.

I confirmed the sparse test fails against the previous behaviour (250 vs. 5000 sats/kwu for UrgentOnChainSweep, 250 vs. 5000 for OnchainPayment).

Esplora may return a non-empty estimate map that still has no usable entry
for some of our confirmation targets, as `convert_fee_rate` only yields a
value if the map holds a block count at or below the requested one. We'd then
substitute 1 sat/vb, which is well below the default we'd otherwise use for
urgent targets: `UrgentOnChainSweep` would end up at 250 sats/kwu instead of
5000.

Note this isn't a case of a missing fallback, but of an actively harmful one:
`OnchainFeeEstimator::estimate_fee_rate` already falls back to
`get_fallback_rate_for_target` whenever it misses the cache, so inserting
1 sat/vb only serves to shadow the value we'd have used anyway.

Here we therefore fall back to the target's own default. We deliberately skip
the post-estimation adjustments for it, as there is no estimate to adjust, and
so that we land on the same rate the cache-miss path would have given us.

To allow testing this without an Esplora server, we move the cache
construction to `build_fee_rate_cache` and add coverage for sparse maps, for
empty maps on and off Mainnet, and for the unchanged complete-map behavior.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ldk-reviews-bot

ldk-reviews-bot commented Aug 11, 2026

Copy link
Copy Markdown

I've assigned @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-reviews-bot
ldk-reviews-bot requested a review from tnull August 11, 2026 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sparse Esplora fee estimates may fall back to 1 sat/vB for urgent targets

2 participants