Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default to fallbacks for huge bogus values in fee estimation conversion #430

Conversation

tnull
Copy link
Collaborator

@tnull tnull commented Jan 15, 2025

Based on #426.

Previously, we would cast FeeRate::to_sat_per_kwu to u32, which however might result in u32::max_value being used if our fee estimation source delivers huge bogus data. Here, we make sure to use the fallback rate if we would do so otherwise.

We also make sure our post-estimation adjustments could never underflow.

Both of these changes aren't super critical but make the code a bit more robust.

@tnull tnull force-pushed the 2025-01-fix-potential-fee-rate-conversion-overflow branch from b3fea0e to fe61011 Compare January 15, 2025 08:42
@tnull tnull force-pushed the 2025-01-fix-potential-fee-rate-conversion-overflow branch 2 times, most recently from 3534f78 to cf7ac78 Compare January 16, 2025 09:29
@tnull tnull added this to the 0.5 milestone Jan 16, 2025
@tnull tnull force-pushed the 2025-01-fix-potential-fee-rate-conversion-overflow branch 2 times, most recently from 75b4733 to 715d56b Compare January 16, 2025 09:51
@tnull tnull changed the title Fix potential overflow in fee estimation conversion Default to fallbacks for huge bogus values in fee estimation conversion Jan 16, 2025
tnull added 2 commits January 17, 2025 09:45
Previously, we would cast `FeeRate::to_sat_per_kwu` to `u32`, which
however might result in `u32::max_value` being used if our fee
estimation source delivers huge bogus data. Here, we make sure to use
the fallback rate if we would do so otherwise.
@tnull tnull force-pushed the 2025-01-fix-potential-fee-rate-conversion-overflow branch from 715d56b to 15c3e0e Compare January 17, 2025 08:45
@tnull
Copy link
Collaborator Author

tnull commented Jan 17, 2025

Rebased on main after #426 landed.

@tnull tnull requested a review from arik-so January 17, 2025 08:45
@tnull tnull merged commit 9953d2b into lightningdevkit:main Jan 17, 2025
8 of 13 checks passed
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