Skip to content

Handle fulfill not acked upstream#1079

Merged
t-bast merged 8 commits intomasterfrom
fulfill-not-acked
Jul 23, 2019
Merged

Handle fulfill not acked upstream#1079
t-bast merged 8 commits intomasterfrom
fulfill-not-acked

Conversation

@t-bast
Copy link
Member

@t-bast t-bast commented Jul 17, 2019

No description provided.

@t-bast t-bast requested a review from pm47 July 17, 2019 11:01
@codecov-io
Copy link

codecov-io commented Jul 17, 2019

Codecov Report

Merging #1079 into master will increase coverage by 0.1%.
The diff coverage is 90%.

@@            Coverage Diff            @@
##           master    #1079     +/-   ##
=========================================
+ Coverage    82.6%   82.71%   +0.1%     
=========================================
  Files          99       99             
  Lines        7594     7612     +18     
  Branches      293      295      +2     
=========================================
+ Hits         6273     6296     +23     
+ Misses       1321     1316      -5
Impacted Files Coverage Δ
...re/src/main/scala/fr/acinq/eclair/NodeParams.scala 83.9% <75%> (+0.37%) ⬆️
...in/scala/fr/acinq/eclair/channel/Commitments.scala 87.02% <81.81%> (+0.09%) ⬆️
...c/main/scala/fr/acinq/eclair/channel/Channel.scala 84.74% <93.33%> (+0.57%) ⬆️
...nq/eclair/blockchain/electrum/ElectrumClient.scala 73.23% <0%> (-1.12%) ⬇️
...-core/src/main/scala/fr/acinq/eclair/io/Peer.scala 75.23% <0%> (+0.3%) ⬆️
.../scala/fr/acinq/eclair/payment/CommandBuffer.scala 100% <0%> (+25%) ⬆️

Add more tests and fix edge cases.
Copy link
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

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

There is still the issue of the uptream channel being in state OFFLINE when we receive the fulfill. We could check the PendingRelayDb for available CMD_FULFILL_HTLC when we enter the fulfill-safety-before-timeout-blocks and fail the channel, which would cause the CommandBuffer to re-send the command.

@t-bast t-bast requested a review from pm47 July 23, 2019 07:59
@t-bast t-bast merged commit 189b11e into master Jul 23, 2019
@t-bast t-bast deleted the fulfill-not-acked branch July 23, 2019 09:11
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.

5 participants