-
Notifications
You must be signed in to change notification settings - Fork 491
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
Conversation
@t-bast note: lets think about if we need signaling. A very nice feature is that we currently know exactly how much msat a channel can receive or send, if we don't add signaling for this change, we can't know anymore, since a remote might or might not support this (yet), which kinda eliminates the reason I started this discussion with my little circular routing remote channel lockup POC... Edit: maybe this is overkill, and we just assume any remote to behave like this, because this would just prevent us from using 100% capacity, which is unsafe to use anyway. |
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.
Concept ACK
In practice, unstucking must be done proactively by the fundee, routed HTLCs are likely going to overflow the additional reserve (and fail).
Estimating how much you can receive will always be only a best effort, not a guarantee (see some of the reasons in the comments on this PR). After thinking about it I think that signalling this explicitly would be overkill. It would be better to just assume that the remote (if he's funder) uses this extra reserve. If he doesn't, that means you underestimated a bit how much you were able to receive, but that's probably not a big deal. If you use that value in invoices you create, that's even better because you help the remote artificially keep this additional reserve even if he doesn't explicitly support it, so you're helping him avoid getting stuck. |
I agree |
Add an additional "reserve" for funders on top of the real reserve to avoid getting in a state where the channel is unusable because of the increased commit tx cost of a new HTLC. Requirements are only added for the funder sending an HTLC. Fundee receiving HTLCs may choose to verify that funders apply this, but it may lead to an unusable UX. Fixes #728.
eace10d
to
16de185
Compare
Recommend N=2, but allow implementations to diverge from that value.
@TheBlueMatt about your irc comment on commit fee and packaged relays: Suppose the packaged relays are there, wouldn't you rather use 0 sat/b than any other fixed fee rate on the commitment txes? With zero fees, this pr wouldn't be necessary. Htlcs can always be sent. |
ACK 95c74fe From my side, I can live this PR as a mitigation. Not perfect, but adding signaling on the applied factor seems overkill. It mostly 'fixes the symptom' and makes it hard to exploit until we have eltoo. |
transaction, it cannot pay the fee for either the local or remote commitment | ||
transaction at the current `feerate_per_kw` while maintaining its channel | ||
reserve (see [Updating Fees](#updating-fees-update_fee)). | ||
- SHOULD NOT offer `amount_msat` if, after adding that HTLC to its commitment |
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:
- this paragraph shouldn't prevent you from sending multiple HTLCs (not settled) as long as your fee buffer allows it
- when you send an outgoing htlc, you should be able to receive an incoming htlc: this way even if your outgoing htlc is stuck in your commit tx, you can receive an incoming to increase your balance and prevent your channel from being stuck. The case where the first outgoing htlc is still in the commit tx costs more fees than the case where it has settled, so that should be handled.
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.
even if your outgoing htlc is stuck in your commit tx, you can receive an incoming
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.
Now this sounds like the sender must be able to send another HTLC before this one settles.
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.
Not sure how important this is?
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.
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).
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.
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.
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.
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.
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.
02-peer-protocol.md
Outdated
transaction, its remaining balance doesn't allow it to pay the fee for a | ||
future additional non-dust HTLC at a higher feerate while maintaining its | ||
channel reserve ("fee spike buffer"). A buffer of `2*feerate_per_kw` is | ||
recommended to ensure predictability. |
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 suppose the wording is not clear enough, given both c-lightning and eclair now implemented this, but in incompatible ways due to different interpretations.
- The wording here is basically "ctx plus 1 more HTLC at twice the current feerate".
- In eclair, the implementation checks "ctx plus (1 more HTLC at twice the current feerate)":
there is an "additional reserve", which is the on-chain fee cost of 2 more HTLC outputs at the current feerate. (just the outputs! the base fee of the commitment transaction and the fee of the existing HTLCs is not multiplied) - In c-lightning however, the check is "(ctx plus 1 more HTLC) at twice the current feerate":
balance after reserve must be at least how much fees the funder would need to pay for the whole commitment transaction (with an extra dummy HTLC). (the fee for the whole ctx is multiplied)
(to be clear, in both cases, the "ctx" already includes the HTLC being offered, and the "1 more HTLC" is an additional one)
EDIT: by "ctx", I mean commitment transaction
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.
Interesting, that means we'll have to discuss this further on IRC tonight to get more bandwidth than discussing it on Github ;)
Feel free to suggest a better wording in the meantime though.
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 don't know what the wording should be because I don't know what the intended meaning 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.
I think c-lightning got it right. An increase of the feerate must be applied to the whole tx, not only an htlc, so my implementation in eclair is a bit weird. I tried rephrasing in afb7313, I hope it's clearer. Feel free to suggest something if still unclear (I'm still not 100% happy with what I came up with).
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.
What about now? Do you have a suggestion on how to make it more explicit @SomberNight?
@joostjager @halseth what do you think of the latest version? |
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.
Ack efc9ef0
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.
Ack efc9ef0
Add an additional "reserve" for funders on top of the real reserve to
avoid getting in a state where the channel is unusable because
of the increased commit tx cost of a new HTLC.
Requirements are only added for the funder sending an HTLC.
Fundee receiving HTLCs may choose to verify that funders apply
this, but it may lead to an unusable UX.
@halseth do you want to chime in since you've been confronted
with this issue on the lnd side?
Fixes #728. (see the issue for a longer discussion)