Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deadlock when closing an unavailable channel #2657

Merged

Conversation

Fedeparma74
Copy link
Contributor

Fix a deadlock issue when the user tries to close a channel that is not available anymore.
Tests have been added to cover the cases where the API is used with unavailable channels.

@codecov-commenter
Copy link

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (989304e) 88.99% compared to head (7cdb31a) 88.96%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2657      +/-   ##
==========================================
- Coverage   88.99%   88.96%   -0.04%     
==========================================
  Files         112      112              
  Lines       87168    87207      +39     
  Branches    87168    87207      +39     
==========================================
+ Hits        77575    77580       +5     
- Misses       7353     7392      +39     
+ Partials     2240     2235       -5     
Files Coverage Δ
lightning/src/ln/channelmanager.rs 81.56% <100.00%> (+0.11%) ⬆️
lightning/src/ln/payment_tests.rs 98.17% <100.00%> (ø)

... and 11 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@@ -4001,7 +4003,7 @@ where
for channel_id in channel_ids {
if !peer_state.has_channel(channel_id) {
return Err(APIError::ChannelUnavailable {
err: format!("Channel with ID {} was not found for the passed counterparty_node_id {}", channel_id, counterparty_node_id),
err: format!("Channel with id {} not found for the passed counterparty node_id {}", channel_id, counterparty_node_id),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is needed for the test to pass, as I'm checking the APIError::ChannelUnavailable with the same expected message in check_channel_unavailable_error()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay.

@@ -4112,7 +4114,7 @@ where
next_hop_channel_id, next_node_id)
}),
None => return Err(APIError::ChannelUnavailable {
err: format!("Channel with id {} not found for the passed counterparty node_id {}.",
err: format!("Channel with id {} not found for the passed counterparty node_id {}",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or this one, we dont usually add periods.

@TheBlueMatt TheBlueMatt added this to the 0.0.118 milestone Oct 12, 2023
@TheBlueMatt TheBlueMatt merged commit 60c1073 into lightningdevkit:main Oct 12, 2023
12 of 15 checks passed
@Fedeparma74 Fedeparma74 deleted the fix-close-channel-deadlock branch October 13, 2023 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants