Skip to content

Handle only-last-hop temp failure better #1241

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
TheBlueMatt opened this issue Jan 14, 2022 · 1 comment · Fixed by #1600
Closed

Handle only-last-hop temp failure better #1241

TheBlueMatt opened this issue Jan 14, 2022 · 1 comment · Fixed by #1600
Milestone

Comments

@TheBlueMatt
Copy link
Collaborator

Currently if we try to send to a node with a single last-hop hint in the invoice, and the second-to-last node rejects the payment with a temp failure, we'll retry several times through the same channel that we just learned is failed. This is particularly visible on mobile receivers where the receiving device may have gone offline and for some LSPs the payment will just fail again and again.

@TheBlueMatt TheBlueMatt added this to the 0.1 milestone Jan 14, 2022
@TheBlueMatt
Copy link
Collaborator Author

Working on this one.

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Jan 18, 2022
If we try to pay a mobile client behind an LSP, its not strange for
the singular last-hop hint to fail with a Temporary Channel Failure
(indicating the mobile app is not currently open and connected to
the LSP). In this case, we will penalize the last-hop channel but
try again along the same path anyway, because we have no other
path. This changes the retryer to simply refuse to do so, failing
the payment instead.

Fixes lightningdevkit#1241.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Jul 6, 2022
When an HTLC fails, we currently rely on the scorer learning the
failed channel and assigning an infinite (`u64::max_value()`)
penalty to the channel so as to avoid retrying over the exact same
path (if there's only one available path). This is common when
trying to pay a mobile client behind an LSP if the mobile client is
currently offline.

This leads to the scorer being overly conservative in some cases -
returning `u64::max_value()` when a given path hasn't been tried
for a given payment may not be the best decision, even if that
channel failed 50 minutes ago.

By tracking channels which failed on a payment level and explicitly
refusing to route over them we can relax the requirements on the
scorer, allowing it to make different decisions on how to treat
channels that failed relatively recently without causing payments
to retry the same path forever.

Closes lightningdevkit#1241, superseding lightningdevkit#1241.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Jul 6, 2022
When an HTLC fails, we currently rely on the scorer learning the
failed channel and assigning an infinite (`u64::max_value()`)
penalty to the channel so as to avoid retrying over the exact same
path (if there's only one available path). This is common when
trying to pay a mobile client behind an LSP if the mobile client is
currently offline.

This leads to the scorer being overly conservative in some cases -
returning `u64::max_value()` when a given path hasn't been tried
for a given payment may not be the best decision, even if that
channel failed 50 minutes ago.

By tracking channels which failed on a payment level and explicitly
refusing to route over them we can relax the requirements on the
scorer, allowing it to make different decisions on how to treat
channels that failed relatively recently without causing payments
to retry the same path forever.

Closes lightningdevkit#1241, superseding lightningdevkit#1252.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Jul 7, 2022
When an HTLC fails, we currently rely on the scorer learning the
failed channel and assigning an infinite (`u64::max_value()`)
penalty to the channel so as to avoid retrying over the exact same
path (if there's only one available path). This is common when
trying to pay a mobile client behind an LSP if the mobile client is
currently offline.

This leads to the scorer being overly conservative in some cases -
returning `u64::max_value()` when a given path hasn't been tried
for a given payment may not be the best decision, even if that
channel failed 50 minutes ago.

By tracking channels which failed on a payment level and explicitly
refusing to route over them we can relax the requirements on the
scorer, allowing it to make different decisions on how to treat
channels that failed relatively recently without causing payments
to retry the same path forever.

Closes lightningdevkit#1241, superseding lightningdevkit#1252.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Jul 13, 2022
When an HTLC fails, we currently rely on the scorer learning the
failed channel and assigning an infinite (`u64::max_value()`)
penalty to the channel so as to avoid retrying over the exact same
path (if there's only one available path). This is common when
trying to pay a mobile client behind an LSP if the mobile client is
currently offline.

This leads to the scorer being overly conservative in some cases -
returning `u64::max_value()` when a given path hasn't been tried
for a given payment may not be the best decision, even if that
channel failed 50 minutes ago.

By tracking channels which failed on a payment level and explicitly
refusing to route over them we can relax the requirements on the
scorer, allowing it to make different decisions on how to treat
channels that failed relatively recently without causing payments
to retry the same path forever.

Closes lightningdevkit#1241, superseding lightningdevkit#1252.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Jul 14, 2022
When an HTLC fails, we currently rely on the scorer learning the
failed channel and assigning an infinite (`u64::max_value()`)
penalty to the channel so as to avoid retrying over the exact same
path (if there's only one available path). This is common when
trying to pay a mobile client behind an LSP if the mobile client is
currently offline.

This leads to the scorer being overly conservative in some cases -
returning `u64::max_value()` when a given path hasn't been tried
for a given payment may not be the best decision, even if that
channel failed 50 minutes ago.

By tracking channels which failed on a payment part level and
explicitly refusing to route over them we can relax the
requirements on the scorer, allowing it to make different decisions
on how to treat channels that failed relatively recently without
causing payments to retry the same path forever.

This does have the drawback that it could allow two separate part
of a payment to traverse the same path even though that path just
failed, however this should only occur if the payment is going to
fail anyway, at least as long as the scorer is properly learning.

Closes lightningdevkit#1241, superseding lightningdevkit#1252.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Jul 14, 2022
When an HTLC fails, we currently rely on the scorer learning the
failed channel and assigning an infinite (`u64::max_value()`)
penalty to the channel so as to avoid retrying over the exact same
path (if there's only one available path). This is common when
trying to pay a mobile client behind an LSP if the mobile client is
currently offline.

This leads to the scorer being overly conservative in some cases -
returning `u64::max_value()` when a given path hasn't been tried
for a given payment may not be the best decision, even if that
channel failed 50 minutes ago.

By tracking channels which failed on a payment part level and
explicitly refusing to route over them we can relax the
requirements on the scorer, allowing it to make different decisions
on how to treat channels that failed relatively recently without
causing payments to retry the same path forever.

This does have the drawback that it could allow two separate part
of a payment to traverse the same path even though that path just
failed, however this should only occur if the payment is going to
fail anyway, at least as long as the scorer is properly learning.

Closes lightningdevkit#1241, superseding lightningdevkit#1252.
G8XSU pushed a commit to G8XSU/rust-lightning that referenced this issue Jul 18, 2022
When an HTLC fails, we currently rely on the scorer learning the
failed channel and assigning an infinite (`u64::max_value()`)
penalty to the channel so as to avoid retrying over the exact same
path (if there's only one available path). This is common when
trying to pay a mobile client behind an LSP if the mobile client is
currently offline.

This leads to the scorer being overly conservative in some cases -
returning `u64::max_value()` when a given path hasn't been tried
for a given payment may not be the best decision, even if that
channel failed 50 minutes ago.

By tracking channels which failed on a payment part level and
explicitly refusing to route over them we can relax the
requirements on the scorer, allowing it to make different decisions
on how to treat channels that failed relatively recently without
causing payments to retry the same path forever.

This does have the drawback that it could allow two separate part
of a payment to traverse the same path even though that path just
failed, however this should only occur if the payment is going to
fail anyway, at least as long as the scorer is properly learning.

Closes lightningdevkit#1241, superseding lightningdevkit#1252.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant