Skip to content
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

Open
wants to merge 1 commit into
base: master
from

Conversation

@t-bast
Copy link
Collaborator

t-bast commented Feb 11, 2020

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)

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.
transaction at the current `feerate_per_kw` (see "Updating Fees") while
maintaining its channel reserve.
- MUST NOT offer `amount_msat` if the resulting balance doesn't allow it to
pay the fee for a future additional HTLC at `2*feerate_per_kw` while

This comment has been minimized.

Copy link
@t-bast

t-bast Feb 11, 2020

Author Collaborator

This factor 2 should be challenged. Matt proposed 5, Rusty implemented 1.5, meeting in the middle was an ugly 3.25 so I chose 2 completely randomly.

This comment has been minimized.

Copy link
@joostjager

joostjager Feb 17, 2020

Collaborator

I think hard-coding the 2* via MUST (and later also making this a hard requirement on the receiver side) is not necessary and could be downgraded to SHOULD.

This comment has been minimized.

Copy link
@t-bast

t-bast Feb 18, 2020

Author Collaborator

I'm ok with making that one a SHOULD. However please note that if we go down the road of this "extra reserve", I think it's important that implementations agree on the factor (current 2). That allows a node to compute exactly how much the remote is able to send or receive without having to try and fail. Does that make sense?

- MUST NOT offer `amount_msat` it cannot pay for in the remote commitment
transaction at the current `feerate_per_kw` (see "Updating Fees") while
maintaining its channel reserve.
- MUST NOT offer `amount_msat` if the resulting balance doesn't allow it to

This comment has been minimized.

Copy link
@halseth

halseth Feb 13, 2020

Contributor

is "resulting balance" here referring to balance after the HTLC is settled or while it is still on the commitment?

This comment has been minimized.

Copy link
@t-bast

t-bast Feb 13, 2020

Author Collaborator

This is where I struggled the most with the wording...

It's both in my opinion, but unless I'm missing something (correct me if I'm wrong there, this really needs some challenging) the more restrictive case is the one where the HTLC is still in the commitment tx (because the commit tx fee is higher in that case). If you're able to satisfy the condition while the HTLC is in the commitment, you're able to satisfy it once it's resolved.

If we agree that's what we expect, I can try to find a more accurate wording to reflect that. Maybe "MUST NOT offer amount_msat if, once that HTLC is added to the remote commitment transaction, the sender cannot pay the fee for a future additional HTLC at 2*feerate_per_kw while maintaining its channel reserve."?

This comment has been minimized.

Copy link
@halseth

halseth Feb 13, 2020

Contributor

Mhm, maybe we have to be this restrictive, otherwise the other node possibly cannot add more HTLCs while this HTLC is still pending. If we only required that the balance was high enough after the HTLC had been settled, then the channel would essentially be "locked" while this HTLC was still pending. But it would also make less of you balance available to send of course...

@cdecker cdecker mentioned this pull request Feb 14, 2020
5 of 12 tasks complete
transaction at the current `feerate_per_kw` (see "Updating Fees") while
maintaining its channel reserve.
- MUST NOT offer `amount_msat` if the resulting balance doesn't allow it to
pay the fee for a future additional HTLC at `2*feerate_per_kw` while

This comment has been minimized.

Copy link
@joostjager

joostjager Feb 17, 2020

Collaborator

Specify that it is a 'future non-dust htlc'?

This comment has been minimized.

Copy link
@t-bast

t-bast Feb 18, 2020

Author Collaborator

Good point, I'll add that.

- if it is _responsible_ for paying the Bitcoin fee:
- MUST NOT offer `amount_msat` it cannot pay for in the remote commitment
transaction at the current `feerate_per_kw` (see "Updating Fees") while
maintaining its channel reserve.

This comment has been minimized.

Copy link
@joostjager

joostjager Feb 17, 2020

Collaborator

Must also not offer an htlc if it cannot be paid for when the remote adds this htlc to the funder's local commitment tx. There can be a different dust limit and it therefore needs to be checked as well.

This comment has been minimized.

Copy link
@t-bast

t-bast Feb 18, 2020

Author Collaborator

True, this sentence also needs clarifying, it seems that lnd, c-lightning and eclair all understood it the same way but the spec isn't clear enough. I'll give a shot at clarifying that.

This comment has been minimized.

Copy link
@t-bast

t-bast Feb 24, 2020

Author Collaborator

Can we simply change in the remote commitment transaction to in either the local or remote commitment transaction?
Does that correctly account for the case you're mentioning?

@t-bast t-bast changed the title Avoid stuck channels after fee increase Avoid stuck channels after fee increase with additional reserve Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.