Skip to content

MPP receive timeout not implemented? #1050

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

Closed
t-bast opened this issue Aug 18, 2021 · 8 comments · Fixed by #1353
Closed

MPP receive timeout not implemented? #1050

t-bast opened this issue Aug 18, 2021 · 8 comments · Fixed by #1353
Labels
good first issue Good for newcomers
Milestone

Comments

@t-bast
Copy link

t-bast commented Aug 18, 2021

I'm testing the latest version of the ldk-sample and I noticed that the receiver MPP timeout mechanism doesn't seem to work.

I sent a partial HTLC to the LDK sample node, it's correctly accepted, but even after 5 minutes without receiving the remaining HTLCs the LDK sample node doesn't fail the incoming HTLC.

I'm referring to the following requirement:

- otherwise, if the total amount_msat of this HTLC set is less than total_msat:
    - MUST fail all HTLCs in the HTLC set after some reasonable timeout.
        - SHOULD wait for at least 60 seconds after the initial HTLC.
        - SHOULD use mpp_timeout for the failure message.

Is that expected?

@TheBlueMatt
Copy link
Collaborator

TheBlueMatt commented Aug 18, 2021

Hmm, I believe we currently fail it some blocks before the CLTV timeout, not on a timer, though I admit maybe that's not exactly a "reasonable timeout"

@TheBlueMatt TheBlueMatt added the good first issue Good for newcomers label Oct 7, 2021
@1nF0rmed
Copy link
Contributor

1nF0rmed commented Oct 8, 2021

In this case, what would be considered a reasonable timeout? The spec mentions a minimum of 60 seconds after the initial HTLC.

Should we consider implementing a timer of sorts?

@t-bast
Copy link
Author

t-bast commented Oct 8, 2021

I believe you really should indeed.

Imagine the case where a sender splits a payment in two parts.
The first part reaches your node, the second doesn't because of an error with an intermediate node.

If the sender is unable to find other routes to send the second part and abandons the payment, it will still have the first part locked for many blocks: this consumes liquidity for a long time for no good reason for every node in the path.

@1nF0rmed
Copy link
Contributor

1nF0rmed commented Oct 8, 2021

If we have to setup a timer (seconds/minutes based) we would have to make it event-based, am I correct in this?
Then what event would we issue for this, and who would keep track of this time?

@TheBlueMatt
Copy link
Collaborator

While we don't assume access to the current time, we do have timer_tick_occurred which users should/will call ~every 30 seconds, so we can shove more logic in there. I really don't know what "reasonable" is here either, but a few minutes is probably fine.

@dunxen
Copy link
Contributor

dunxen commented Dec 13, 2021

Might make sense if I take a look at this after #1148.
I think LND timeout is around 2 minutes. I was chatting with Carla about this.
I was thinking something simple like 5 or so ticks.
The comments there mention once every 1 minute which I assume would also be fine for 5 ticks.

@TheBlueMatt TheBlueMatt added this to the 0.1 milestone Dec 23, 2021
@TheBlueMatt
Copy link
Collaborator

Marking 0.1 cause this can interact natively with some senders to make a really poor UX.

@dunxen
Copy link
Contributor

dunxen commented Feb 3, 2022

Busy working on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants