Skip to content

Cap fee rate estimates at a maximum - #1183

Open
ajaysehwal wants to merge 1 commit into
lightningdevkit:mainfrom
ajaysehwal:fix-fee-rate-ceiling
Open

ajaysehwal wants to merge 1 commit into
lightningdevkit:mainfrom
ajaysehwal:fix-fee-rate-ceiling

Conversation

@ajaysehwal

Copy link
Copy Markdown
Contributor

We previously only enforced a lower bound (FEERATE_FLOOR_SATS_PER_KW) on the fee rates reported by our chain source. A faulty or malicious Esplora/Electrum server could therefore return an absurdly high estimate and make any transaction that uses it pay a fee limited only by our spendable balance. This affects on-chain sends without an explicit fee rate, funding and splice transactions, sweeps, anchor bumps, and update_fees, as they all take their feerate from OnchainFeeEstimator. Since the broadcast usually goes through the same server, Bitcoin Core's maxfeerate safeguard doesn't help here.

Here, we additionally cap all estimates at a new MAX_FEERATE_SATS_PER_KW (2,000 sat/vB) in estimate_fee_rate. Doing it there covers all chain sources (Esplora, Electrum, bitcoind) and all consumers, including LDK's FeeEstimator, without having to touch each backend. As a side effect, the estimate now always fits into the u32 we hand to LDK, so the existing fallback in get_est_sat_per_1000_weight can no longer silently replace an absurdly high estimate with a low default.

Notes for reviewers

  • Choice of the cap. 2,000 sat/vB is roughly 2x the highest confirmation-target estimates seen on mainnet (a bit above 1,000 sat/vB during the 2017 and 2024 peaks). It bounds the damage a bad chain source can do, but doesn't remove it: a malicious server can still make a ~150 vB transaction pay ~300k sats. If we'd rather be stricter, or make the cap configurable, I'm happy to follow up. A lower cap risks under-paying in a genuine fee spike, which matters most for force-close sweeps.
  • Clamp rather than fall back. An estimate above the cap is clamped rather than replaced with the static default, so we still pay a high fee when fees are genuinely extreme, instead of a low one.
  • Not covered. Fee rates explicitly passed by the user are not capped, and neither is a chain source that reports too low estimates (see Sparse Esplora fee estimates may fall back to 1 sat/vB for urgent targets #1028 for the Esplora fallback case).
  • Behavior change. Only nodes whose chain source reports more than 2,000 sat/vB are affected. I didn't add a changelog entry; happy to add one if you'd like.

Testing

  • New unit test estimates_are_bounded: absurd estimates (cap + 1, 50,000 sat/vB, u64::MAX) are capped for all confirmation targets, including through the LDK FeeEstimator interface; in-range estimates pass through and too-low ones are floored.
  • cargo test --lib passes (337 tests), and cargo check --lib compiles with the uniffi feature and without storage-sqlite. I did not run the integration tests.

AI disclosure: this change was prepared with the assistance of AI tools (Claude Code)

Previously, we only enforced a lower bound (`FEERATE_FLOOR_SATS_PER_KW`)
on the fee rates reported by our chain source. A faulty or malicious
Esplora/Electrum server could therefore return an absurdly high estimate
and make any transaction using it (on-chain sends without an explicit fee
rate, funding and splice transactions, sweeps, anchor bumps, etc.) pay a
fee limited only by our spendable balance.

Here, we additionally cap all estimates at `MAX_FEERATE_SATS_PER_KW`
(2,000 sat/vB) when handing them out in `estimate_fee_rate`, which covers
all chain sources and consumers, including LDK's `FeeEstimator`. This
also ensures the `u32` conversion for LDK can no longer overflow.

This change was prepared with the assistance of AI tools (Claude Code).
@ldk-reviews-bot

ldk-reviews-bot commented Oct 20, 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.

@ajaysehwal
ajaysehwal marked this pull request as ready for review October 20, 2026 16:44
@ldk-reviews-bot
ldk-reviews-bot requested a review from tnull October 20, 2026 16:49
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.

2 participants