rust-lightning
rust-lightning copied to clipboard
lightningdevkit
Enforce Trampoline Constraints (replacement)
This PR replaces #3983, adding validation of trampoline onions (as compared to the outer onion). It makes some quite significant changes to the tests in the original PR to consolidate blinded and unblinded tests for success/failure scenarios into a single helper (apologies to reviewers who've already looked at the tests, but I think this DRYs it up quite nicely).
While we're here, it also moves rustfmt::skip to a per-function level on blinded_payment_tests.rs and formats the existing test helper that we're modifying in a pre-factor so that the new code can be formatted.
👋 Thanks for assigning @tankyleo 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.
Assigning reviewers who looked at the original PR - please free yourself if not appropriate!
Codecov Report
:x: Patch coverage is 92.33129% with 25 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 89.32%. Comparing base (6d9c676) to head (785c781).
:warning: Report is 17 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #4226 +/- ##
==========================================
- Coverage 89.32% 89.32% -0.01%
==========================================
Files 180 180
Lines 138641 138916 +275
Branches 138641 138916 +275
==========================================
+ Hits 123844 124088 +244
- Misses 12174 12205 +31
Partials 2623 2623
| Flag | Coverage Δ | |
|---|---|---|
| fuzzing | 34.97% <0.00%> (-1.01%) |
:arrow_down: |
| tests | 88.69% <92.33%> (+<0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
One thing to note about the tests here: we've currently only got coverage for blinded receives checking the constraints (this is what the original PR had). We could also add coverage for blinded forwards in the failure case (can't do for success because we just fail the forwards rn), which would make codecov a bit happier.
I think this is worth doing, but it would mean adding a bit more code to the mega test helper - interested on hearing thoughts on how readable others find it before adding another layer to an already quite dense test! Can also easily be a small follow. up.
Needs rebase :(
I did find the new testing not ideal from a readability PoV at first glance, going to take a closer look in a bit but let me know if you see any obvious ways to improve things. It may be that the nature of what we're testing makes it hard to improve things though.
🔔 1st Reminder
Hey @tankyleo! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
Rebased + addressed review in fixups, removing validation of trampoline forwards because we'll need a different check/error there anyway.
🔔 2nd Reminder
Hey @tankyleo! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 3rd Reminder
Hey @tankyleo! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 1st Reminder
Hey @valentinewallace! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 2nd Reminder
Hey @valentinewallace! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 3rd Reminder
Hey @valentinewallace! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.