Add error handling for channels which fail to be created in funding_transaction_generated_intern
#3029
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TheBlueMatt
force-pushed
the
2024-04-fix-batch-funding-failures
branch
from
0ffb568
to
0626f8e
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #3029 +/- ##
=======================================
Coverage 89.14% 89.15%
=======================================
Files 118 118
Lines 97850 97889 +39
Branches 97850 97889 +39
=======================================
+ Hits 87230 87271 +41
+ Misses 8376 8372 -4
- Partials 2244 2246 +2 ☔ View full report in Codecov by Sentry. |
TheBlueMatt
force-pushed
the
2024-04-fix-batch-funding-failures
branch
2 times, most recently
from
76d13e2
to
5231799
Compare
In `funding_transaction_generated_intern`, if `find_funding_output` fails (e.g. due to the require output not being present in the provided funding transaction) we'd previously not generated a `ChannelClosed` event which leaves users possibly in a confused state. Here we fix this, also fixing the relevant tests to check for the new event. Fixes lightningdevkit#2843.
If we fail to fund a batch open we'll force-close all channels in the batch, however would previously fail to send error messages to our peers for any channels we were due to test after the one that failed. This commit fixes that issue, sending the required error messages to allow our peers to clean up their state.
TheBlueMatt
force-pushed
the
2024-04-fix-batch-funding-failures
branch
from
5231799
to
b811cba
Compare
wpaulino
approved these changes
May 3, 2024
| get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); | ||
| let msgs = nodes[0].node.get_and_clear_pending_msg_events(); | ||
| assert_eq!(msgs.len(), 2); | ||
| // We currently spuriously send `FundingCreated` for the first channel and then immediately |
There was a problem hiding this comment.
Is it too much trouble to hold back sending until we know the batch succeeds?
There was a problem hiding this comment.
I mean we could but I don't really want to add any more error handling than we need to....
Yea, we improved it a bit in a previous PR, but, yea, more would be good... |
TheBlueMatt
added a commit
to TheBlueMatt/rust-lightning
that referenced
this pull request
May 6, 2024
…batch-funding-failures Add error handling for channels which fail to be created in `funding_transaction_generated_intern`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commits for more details.