Fix HTLC fulfill race condition in integration spec #1666
Merged
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
We were extracting F's commit tx from its internal state right after receiving the `PaymentSent` event. The issue is that this could happen before the fulfill was completely signed on both sides, so the commit tx we obtained would still contain the HTLC and would be different from the one F would publish when closing.
Codecov Report
@@ Coverage Diff @@
## master #1666 +/- ##
==========================================
+ Coverage 85.94% 85.97% +0.02%
==========================================
Files 151 151
Lines 11471 11471
Branches 505 505
==========================================
+ Hits 9859 9862 +3
+ Misses 1612 1609 -3
|
|
Wasn't it a nice corner case to test though? (force close with only the unsigned fulfill). |
|
True, but we can't test it reliably in an integration test because we can't control the timing to ensure we're exactly at that step. |
Done in 079920b |
pm47
approved these changes
Jan 19, 2021
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.
We were extracting F's commit tx from its internal state right after receiving the
PaymentSentevent. The issue is that this could happen before the fulfill was completely signed on both sides, so the commit tx we obtained would still contain the HTLC and would be different from the one F would publish when closing.This is the race condition that was sometimes happening:
And F's commit tx1 and commit tx2 would be different, so we were trying to get the wrong tx from bitcoind.