Free holding cell in remaining quiescence-exit code paths#4415
Free holding cell in remaining quiescence-exit code paths#4415wpaulino wants to merge 6 commits intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4415 +/- ##
==========================================
- Coverage 86.08% 86.08% -0.01%
==========================================
Files 156 156
Lines 103501 103525 +24
Branches 103501 103525 +24
==========================================
+ Hits 89099 89119 +20
+ Misses 11891 11889 -2
- Partials 2511 2517 +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:
|
wpaulino
dismissed
TheBlueMatt’s stale review
The merge-base changed after approval.
|
👋 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. |
|
note that github wont let this be merged until its rebased. Also not sure if @jkczyz wants to take a look or not. |
wpaulino
force-pushed
the
free-holding-cells-quiescence-exit
branch
from
d1ea893 to
5a610c7
Compare
|
Rebased and pushed fixups for #4412 |
wpaulino
force-pushed
the
free-holding-cells-quiescence-exit
branch
from
5a610c7 to
826113b
Compare
jkczyz
left a comment
jkczyz
left a comment
There was a problem hiding this comment.
Looks like fail_splice_on_tx_complete_error is failing in CI.
| debug_assert!(res.as_ref().err().map_or(true, |err| !err.closes_channel())); | ||
| let _ = self.handle_error(res, counterparty_node_id); | ||
| persist | ||
| NotifyOption::SkipPersistHandleEvents |
There was a problem hiding this comment.
IIUC, handle_holding_cell_free_result may "override" this and force a persistence?
There was a problem hiding this comment.
Correct and that's the intended behavior
wpaulino
force-pushed
the
free-holding-cells-quiescence-exit
branch
from
826113b to
4b7cfee
Compare
After cad88af, a few code paths that also lead to a quiescence exit were not accounted for. This commit addresses the path where we exit quiescence due to a processing error on a counterparty's `tx_add_input/output`, `tx_remove_input/output`, or `tx_complete` message.
After cad88af, a few code paths that also lead to a quiescence exit were not accounted for. This commit addresses the path where we exit quiescence due to processing a counterparty's `tx_abort` message.
After cad88af, a few code paths that also lead to a quiescence exit were not accounted for. This commit addresses the last remaining path where we exit quiescence when we exchange `tx_signatures` with the counterparty.
These errors will only ever affect our in-memory state, so there's no need to persist the ChannelManager when we come across one. Note that `tx_abort` is not included here because there is a possibility we force close the channel, which we should persist.
wpaulino
force-pushed
the
free-holding-cells-quiescence-exit
branch
from
4b7cfee to
f55c452
Compare
|
Had to rebase due to an import conflict in |
4 participants
Depends on #4412.
According to the spec, handling a
splice_initandsplice_ackmay returntx_abortto reject the splice, but we always send a warning and disconnect instead, so those are not included here.