Handle feerates of u32::MAX without overflowing
#3147
Conversation
Though we generally shouldn't be seeing these, and the `get_dust_buffer_feerate` implementation will still return `u32::MAX` in spite of the overflow, we should handle the overflow to avoid panic when `debug_assertions` are enabled. Found by the `full_stack_target` fuzzer
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #3147 +/- ##
==========================================
+ Coverage 89.83% 89.94% +0.11%
==========================================
Files 121 121
Lines 98900 100282 +1382
Branches 98900 100282 +1382
==========================================
+ Hits 88847 90199 +1352
- Misses 7457 7529 +72
+ Partials 2596 2554 -42 ☔ View full report in Codecov by Sentry. |
| @@ -2768,7 +2768,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider { | |||
| feerate_per_kw = cmp::max(feerate_per_kw, feerate); | |||
| } | |||
| let feerate_plus_quarter = feerate_per_kw.checked_mul(1250).map(|v| v / 1000); | |||
| cmp::max(feerate_per_kw + 2530, feerate_plus_quarter.unwrap_or(u32::max_value())) | |||
| cmp::max(feerate_per_kw.saturating_add(2530), feerate_plus_quarter.unwrap_or(u32::MAX)) | |||
There was a problem hiding this comment.
nit: I think u32::MAX is deprecated?
There was a problem hiding this comment.
I don't think so? Its relatively "new" - only added in 1.43 - https://doc.rust-lang.org/std/primitive.u32.html#associatedconstant.MAX
There was a problem hiding this comment.
I think you're confusing it with max_value() which is deprecated.
There was a problem hiding this comment.
Ah, nvm, was looking at std::u32::MAX, which was also deprecated.
Though we generally shouldn't be seeing these, and the
get_dust_buffer_feerateimplementation will still returnu32::MAXin spite of the overflow, we should handle the overflow to avoid panic whendebug_assertionsare enabled.Found by the
full_stack_targetfuzzer