lnwallet: fix payment regression introduced by MayAddOutgoingHtlc#5478
Merged
Conversation
Roasbeef
added
wallet
As is, if the remote party proposes a min HTLC of 0 `mSat` to us, then we won't ever be able to _send outgoing_ in the channel as the `MayAddOutgoingHtlc` will attempt to add a zero-value HTLC, which isn't allowed within the protocol. The default channels created actually already use a min HTLC value of zero within the tests, so this test case fails as is.
In this commit, in order to allow the test added in the prior commit to pass, we'll increment the mockHTLCAmt value by 1 to ensure we never attempt to add a zero value HTLC. Fixes lightningnetwork#5468
Roasbeef
force-pushed
the
fix-may-add-htlc
branch
from
2fd194d to
af43a86
Compare
guggero
approved these changes
Jul 7, 2021
Collaborator
guggero
left a comment
guggero
left a comment
There was a problem hiding this comment.
tACK, can confirm that I no longer get the FAILURE_REASON_INSUFFICIENT_BALANCE error when trying to do a circular rebalance through a channel with c-lightning.
Crypt-iQ
approved these changes
Jul 7, 2021
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In this PR, we fix a payment regression introduced by #5355. The newly added
MayAddOutgoingHtlcmethod uses the min HTLC value to add a "mock HTLC" to see if any constraints are violated. However in practice, it's possible that an implementation sends over a min HTLC amount of0 mSAT. This would cause us to add a zero value HTLC, which would then trigger theErrInvalidHTLCAmterror, causing the bandwidth hints for the link to show up as "zero".The initial fix is straight forward: ensure zero is never used.
The proper fix here is to use the actual HTLC amount as discuseed in #5355.
Fixes #5468.