From 78ce3abee522b760b33344d1f497d9ac15f83e43 Mon Sep 17 00:00:00 2001 From: Juan Pablo Civile Date: Mon, 2 Aug 2021 15:50:06 -0300 Subject: [PATCH 1/3] routing: relax penalties for channel disabled errors It was being considered a misbehaviour from the intermediate hop which penalized that hop, which is a bit harsh considering mobile nodes. We change it be considered as other channel update types and only penalize the channel that failed. --- docs/release-notes/release-notes-0.14.0.md | 6 ++++++ routing/result_interpretation.go | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/docs/release-notes/release-notes-0.14.0.md b/docs/release-notes/release-notes-0.14.0.md index 311c92dae..3dd0398e0 100644 --- a/docs/release-notes/release-notes-0.14.0.md +++ b/docs/release-notes/release-notes-0.14.0.md @@ -296,6 +296,12 @@ you. * [Save compressed log files from logrorate during itest](https://github.com/lightningnetwork/lnd/pull/5354). +## Mission control + +* [Interpretation of channel disabled errors was changed to only penalize t + he destination node to consider mobile wallets with hub + nodes.](https://github.com/lightningnetwork/lnd/pull/5598) + ## Bug Fixes * A bug has been fixed that would cause `lnd` to [try to bootstrap using the diff --git a/routing/result_interpretation.go b/routing/result_interpretation.go index 70cc08f6a..1de2e128d 100644 --- a/routing/result_interpretation.go +++ b/routing/result_interpretation.go @@ -335,6 +335,19 @@ func (i *interpretedResult) processPaymentOutcomeIntermediate( case *lnwire.FailUnknownNextPeer: reportOutgoing() + // Some implementations use this error when the next hop is offline, so we + // do the same as FailUnknownNextPeer and also process the channel update. + case *lnwire.FailChannelDisabled: + + // Set the node pair for which a channel update may be out of + // date. The second chance logic uses the policyFailure field. + i.policyFailure = &DirectedNodePair{ + From: route.Hops[errorSourceIdx-1].PubKeyBytes, + To: route.Hops[errorSourceIdx].PubKeyBytes, + } + + reportOutgoing() + // If we get a permanent channel, we'll prune the channel set in both // directions and continue with the rest of the routes. case *lnwire.FailPermanentChannelFailure: @@ -349,8 +362,7 @@ func (i *interpretedResult) processPaymentOutcomeIntermediate( // control. case *lnwire.FailAmountBelowMinimum, *lnwire.FailFeeInsufficient, - *lnwire.FailIncorrectCltvExpiry, - *lnwire.FailChannelDisabled: + *lnwire.FailIncorrectCltvExpiry: // Set the node pair for which a channel update may be out of // date. The second chance logic uses the policyFailure field. From 100abb4a30ca9b2d3160663bd03b4355de5d0852 Mon Sep 17 00:00:00 2001 From: Juan Pablo Civile Date: Mon, 6 Sep 2021 18:40:15 -0300 Subject: [PATCH 2/3] routing: add test case for result interpretation of Channel Disabled failure --- routing/result_interpretation_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/routing/result_interpretation_test.go b/routing/result_interpretation_test.go index 2fc0a7d90..fff9eaa08 100644 --- a/routing/result_interpretation_test.go +++ b/routing/result_interpretation_test.go @@ -349,6 +349,23 @@ var resultTestCases = []resultTestCase{ nodeFailure: nil, }, }, + + // Test a channel disabled failure from the final hop in two hops. Only the + // disabled channel should be penalized for any amount. + { + name: "two hop channel disabled", + route: &routeTwoHop, + failureSrcIdx: 1, + failure: &lnwire.FailChannelDisabled{}, + + expectedResult: &interpretedResult{ + pairResults: map[DirectedNodePair]pairResult{ + getTestPair(1, 2): failPairResult(0), + getTestPair(2, 1): failPairResult(0), + }, + policyFailure: getPolicyFailure(1, 2), + }, + }, } // TestResultInterpretation executes a list of test cases that test the result From 04c07184013d5e5f476e3935d4d4619f8a04dcb5 Mon Sep 17 00:00:00 2001 From: Juan Pablo Civile Date: Sun, 24 Oct 2021 16:53:50 -0300 Subject: [PATCH 3/3] routing: report success up to the failing node on FailChannelDisabled --- routing/result_interpretation.go | 4 ++++ routing/result_interpretation_test.go | 1 + 2 files changed, 5 insertions(+) diff --git a/routing/result_interpretation.go b/routing/result_interpretation.go index 1de2e128d..19c43e4a1 100644 --- a/routing/result_interpretation.go +++ b/routing/result_interpretation.go @@ -348,6 +348,10 @@ func (i *interpretedResult) processPaymentOutcomeIntermediate( reportOutgoing() + // All nodes up to the failing pair must have forwarded + // successfully. + i.successPairRange(route, 0, errorSourceIdx-1) + // If we get a permanent channel, we'll prune the channel set in both // directions and continue with the rest of the routes. case *lnwire.FailPermanentChannelFailure: diff --git a/routing/result_interpretation_test.go b/routing/result_interpretation_test.go index fff9eaa08..954f07981 100644 --- a/routing/result_interpretation_test.go +++ b/routing/result_interpretation_test.go @@ -362,6 +362,7 @@ var resultTestCases = []resultTestCase{ pairResults: map[DirectedNodePair]pairResult{ getTestPair(1, 2): failPairResult(0), getTestPair(2, 1): failPairResult(0), + getTestPair(0, 1): successPairResult(100), }, policyFailure: getPolicyFailure(1, 2), },