Make full_stack_target more stable in the face of LDK changes
#4006
Conversation
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
|
👋 I see @joostjager was un-assigned. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4006 +/- ##
=======================================
Coverage 88.85% 88.85%
=======================================
Files 175 175
Lines 127710 127710
Branches 127710 127710
=======================================
+ Hits 113475 113478 +3
+ Misses 11672 11667 -5
- Partials 2563 2565 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
valentinewallace
previously approved these changes
Aug 12, 2025
|
I get a fuzz test failure after rebasing on main, I believe due to a silent rebase conflict with #3999. |
The `full_stack` fuzzer tries to just expose much of the entire library to the fuzzer, and as such when a request comes in from LDK to estimate the current fee it tries to read two bytes of fuzzing input and returns that as the fee to LDK. However, because LDK regularly changes when it requests a fee estimate from the user this causes the fuzz input required to reach a codepath to change regularly, making any existing fuzz corpus stale. Instead, here, we allow the fuzz input to load a fee estimate result into a buffer, and if its empty simply return 253. This allows the fuzzer to still reach fee-estimate-triggered overflows, but only invalidates fuzzing seeds that relied on fee estimate inputs rather than all seeds whenever LDK changes fee estimate calls.
The `full_stack` fuzzer ensures that RNG output is unique by keeping a counter of the number of `get_secure_random_bytes` calls and using it to determine the "random" value to return. However, because LDK regularly changes when it requests RNG output this causes the fuzz input required to reach a codepath to change regularly, making any existing fuzz corpus stale. Instead, here, we allow the fuzz input to set a new RNG output value, but otherwise always return the same output. This allows the fuzzer to still reach RNG-output-specifc paths, but fuzzing seeds aren't invalidated when LDK changes.
The previous two changes should materially reduce how finicky the `full_stack_target` tests are, which we reflect in the comments here.
TheBlueMatt
force-pushed
the
2025-08-robust-fst
branch
from
cc86e37 to
ee06a90
Compare
|
Indeed. Rebased. |
valentinewallace
approved these changes
Aug 14, 2025
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 regularly make changes to LDK which results in changes to the
full_stack_targetinput to reach a given codepath, invalidating existing fuzzing corpuses and breaking thetest_no_existing_test_breakagetest (which is somewhat annoying to fix). Instead of reading every time we get a fee estimate request or counting for RNG, use static values to ensure they're robust against changes in LDK that request more/less fee estimates or more/less RNG output.