Repository navigation
Cap fee rate estimates at a maximum - #1183
Open
ajaysehwal wants to merge 1 commit into
Open
ajaysehwal wants to merge 1 commit into
ajaysehwal wants to merge 1 commit into
Conversation
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).
|
I've assigned @tnull as a reviewer! |
ajaysehwal
marked this pull request as ready for review
October 20, 2026 16:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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, andupdate_fees, as they all take their feerate fromOnchainFeeEstimator. Since the broadcast usually goes through the same server, Bitcoin Core'smaxfeeratesafeguard doesn't help here.Here, we additionally cap all estimates at a new
MAX_FEERATE_SATS_PER_KW(2,000 sat/vB) inestimate_fee_rate. Doing it there covers all chain sources (Esplora, Electrum, bitcoind) and all consumers, including LDK'sFeeEstimator, without having to touch each backend. As a side effect, the estimate now always fits into theu32we hand to LDK, so the existing fallback inget_est_sat_per_1000_weightcan no longer silently replace an absurdly high estimate with a low default.Notes for reviewers
Testing
estimates_are_bounded: absurd estimates (cap + 1, 50,000 sat/vB,u64::MAX) are capped for all confirmation targets, including through the LDKFeeEstimatorinterface; in-range estimates pass through and too-low ones are floored.cargo test --libpasses (337 tests), andcargo check --libcompiles with theuniffifeature and withoutstorage-sqlite. I did not run the integration tests.AI disclosure: this change was prepared with the assistance of AI tools (Claude Code)