Don't prune pending_requests in persist
#4201
Conversation
Previously, we would also prune any pending `GetInfo` or expired `Buy` requests before we `persist`. This could have lead to races where we drop a pending request and even remove the peer when calling `persist`. Here, we simply split the pruning logic for the `pending_requests` and expired JIT channel state, and only prune the latter before persisting. This generally makes sense, as `pending_requests` isn't currently persisted, so there is no need to prune before repersisting. Both are however still pruned on peer disconnection.
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4201 +/- ##
=======================================
Coverage 88.86% 88.86%
=======================================
Files 180 180
Lines 137910 137913 +3
Branches 137910 137913 +3
=======================================
+ Hits 122547 122560 +13
+ Misses 12556 12540 -16
- Partials 2807 2813 +6
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:
|
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Oops, right, thanks.
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
|
All our integration tests succeed now with this change on top of |
|
Backported in #4221 |
4 participants
Previously, we would also prune any pending
GetInfoor expiredBuyrequests before wepersist. This could have lead to races where we drop a pending request and even remove the peer when callingpersist.Here, we simply split the pruning logic for the
pending_requestsand expired JIT channel state, and only prune the latter before persisting. This generally makes sense, aspending_requestsisn't currently persisted, so there is no need to prune before repersisting.Both are however still pruned on peer disconnection.
Thanks @wvanlint for reporting.