Split out `receive_htlcs` from the forwarding pipeline
This is another preparatory step for receiver-side delays that we split out to err on the side of smaller, more reviewable PRs, especially since these changes are a nice cleanup in any case, IMO.
Previously, we'd store receiving HTLCs side-by-side with HTLCs forwards in the forwards_htlcs map under SCID 0.
Here, we opt to split out a separate receive_htlcs field which cleans up the logic, also omitting the 0 magic value.
Moreover, some of the tests manipulating forward_htlcs were previously just iterating, but not actually checking whether the expected entries were present and they were actually changed. Here we make these test cases stricter to ensure they'd able to catch any unwanted behavior we'd introduce while introducing receive_htlcs.
👋 Thanks for assigning @TheBlueMatt 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.
Codecov Report
:x: Patch coverage is 79.82456% with 23 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 88.61%. Comparing base (96f9242) to head (e08d3dc).
Additional details and impacted files
@@ Coverage Diff @@
## main #3973 +/- ##
==========================================
- Coverage 88.61% 88.61% -0.01%
==========================================
Files 174 174
Lines 127640 127684 +44
Branches 127640 127684 +44
==========================================
+ Hits 113113 113148 +35
- Misses 12046 12056 +10
+ Partials 2481 2480 -1
| Flag | Coverage Δ | |
|---|---|---|
| tests | 88.61% <79.82%> (-0.01%) |
:arrow_down: |
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.
✅ Added second reviewer: @valentinewallace
🔔 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.
Basically LGTM!
Could you confirm one thing for me: if we fail to receive an HTLC that's destined for us, it will then be put into the
forward_htlcsmap keyed with the scid of the previous hop? That's how I'm reading the behavior offail_htlc_backwards_internalatm. If that's the case, may want to note that on theforward_htlcsfield.
Yes, I think this is correct, and the same hold for pending_intercepted_htlcs, AFAICT. I now added respective comments.