-
Notifications
You must be signed in to change notification settings - Fork 492
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
Avoid stuck channels after fee increase with additional reserve #740
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
16de185
Avoid stuck channels after fee increase
t-bast 159bea7
fixup! Avoid stuck channels after fee increase
t-bast 95c74fe
Allow implementations for choose the fee factor
t-bast f5490f1
Address joost's comments
t-bast afb7313
Rephrase the fee spike buffer requirement
t-bast 60eeb09
fixup! Rephrase the fee spike buffer requirement
t-bast 1ff8083
fixup! Rephrase the fee spike buffer requirement
t-bast 7d5b94d
fixup! Rephrase the fee spike buffer requirement
t-bast efc9ef0
fixup! fixup! Rephrase the fee spike buffer requirement
t-bast File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this sounds like the sender must be able to send another HTLC before this one settles. I think we should say just "the remaining balance after it has settled doesn't..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the contrary, I think that "after it has settled" would be incorrect for two reasons:
Does this make sense or am I missing something?
Note that while the htlc is pending, its value is still subtracted from your balance, so it is taken into account.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it comes down to how strict you want to be. The channel being stuck because you have a pending HTLC on it is arguably not stuck, since it will eventually settle or cancel.
Not sure how important this is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's more that the current wording allows the sender to send another HTLC before this one settles (if he has the balance to do so). IIUC your proposal disallows that, and I don't see why we'd need to be that restrictive.
If you disallow that, that means you potentially need to wait for a cltv-timeout before you can unblock your channel. True, it will eventually settle, but there's no reason to wait for that and be penalized by a stuck HTLC. If you can accept an incoming HTLC even though the outgoing is still pending, that allows you to increase your balance and then send other outgoing HTLCs, which increases your relay throughput.
I think the current wording is less strict than what you propose: you just deal with the present situation instead of predicting the situation after the htlc is settled. Is this correct or am I misunderstanding your point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, if the sender has some balance left, he must reserve this for fees to receive another HTLC. What I suggested allows using more of this balance, since only after it settles we require an HTLC to be received.
This is true, the current wording makes sure the channel is stuck "less" 😛 What I meant with the current working being more strict is that it also disallows the "stuck-while-HTLC-pending" case.
To summarize the way I interpret this: the current wording requires the initiator to always keep enough balance to receive another HTLC. My suggestion was to loosen this to require the initiator to keep enough balance to receive another HTLC when there are no HTLCs pending.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is a lot clearer with that explanation. It is an alternative that makes sense, but it has a small additional trade-off and I prefer the current proposal 😄.
This issue happens when the balance on your side is low (very close to the reserve). When that happens, I think you should prioritize incoming HTLCs (that will increase your balance), not outgoing HTLCs (that keep getting you closer to a stuck/depleted channel).
I see two reasons for that. The first one is that the additional outgoing HTLC that you could send is tiny (it's on the order of an htlc fee), so it isn't a game-changer to allow it to go through. The second reason is that you're reducing the window of time where you allow incoming HTLCs to restore balance in your channel. While you have that last outgoing HTLC in your commitment, you are rejecting incoming HTLCs (which could have helped your channel balance). When that last outgoing HTLC settles, if you re-add another outgoing one you missed your opportunity window to receive an incoming HTLC and you need to wait for another outgoing HTLC to settle. And I think that incoming HTLCs should be prioritized because they are likely to be bigger (since the only outgoing HTLC you can add is very tiny).
Thanks for taking the time to detail your point, I think it's really a matter of preference and both could apply, so it's an interesting discussion to have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that makes sense to suggest, always being able to receive definitely keeps your channel more healthy more of the time!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made receiving an HTLC more explicitly the goal of this requirement in afb7313, let me know if that's better.