From 8c5a0f308153df91413f5c33e5b3635565689f61 Mon Sep 17 00:00:00 2001 From: George Tsagkarelis Date: Wed, 8 Oct 2025 16:14:21 +0200 Subject: [PATCH 1/3] routing: adjust impact of pathfinding second chance Previously our mission control would not punish a route if a policy failure was provided to us, in the form of a "second chance". For various reasons outlined in https://github.com/lightningnetwork/lnd/issues/6883 we may not want to blindly retry a route if that case is true. With this commit we change the effect of the second chance to instead apply penalties with half the value. This way we do not exclude this route from future attempts and also encourage alternative route selection. --- routing/missioncontrol.go | 30 +++++++++++++++++++++++++----- routing/missioncontrol_test.go | 7 +++++-- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/routing/missioncontrol.go b/routing/missioncontrol.go index b03724a2e62..9fba76ab2e8 100644 --- a/routing/missioncontrol.go +++ b/routing/missioncontrol.go @@ -672,12 +672,15 @@ func (m *MissionControl) applyPaymentResult( // Interpret result. i := interpretResult(&result.route.Val, result.failure.ValOpt()) + // This flag will be true if we grant a second chance to this route. + secondChance := false + if i.policyFailure != nil { if m.state.requestSecondChance( time.Unix(0, int64(result.timeReply.Val)), i.policyFailure.From, i.policyFailure.To, ) { - return nil + secondChance = true } } @@ -698,7 +701,10 @@ func (m *MissionControl) applyPaymentResult( // difference. The largest difference occurs when aprioriWeight is 1. In // that case, a node-level failure would not be applied to untried // channels. - if i.nodeFailure != nil { + // + // If a second chance is being given, skip the node-level failure + // recording. + if i.nodeFailure != nil && !secondChance { m.log.Debugf("Reporting node failure to Mission Control: "+ "node=%v", *i.nodeFailure) @@ -721,10 +727,24 @@ func (m *MissionControl) applyPaymentResult( pair, pairResult.amt) } + ts := time.Unix(0, int64(result.timeReply.Val)) + + // If we want to grant a second chance, we'll apply half of the + // full penalty in order to not totally exclude this route from + // future attempts, but also encourage alternative route + // selection. + if secondChance { + // To create this effect of applying half of the penalty + // we'll record this failure as if it happened a bit + // further in the past. The distance between current and + // recorded time is equal to the penalty half life, + // which is the time duration it takes for a penalty + // to decay to its half value. + ts = ts.Add(-1 * DefaultPenaltyHalfLife) + } + m.state.setLastPairResult( - pair.From, pair.To, - time.Unix(0, int64(result.timeReply.Val)), &pairResult, - false, + pair.From, pair.To, ts, &pairResult, false, ) } diff --git a/routing/missioncontrol_test.go b/routing/missioncontrol_test.go index aa13a064a61..b95c8db9296 100644 --- a/routing/missioncontrol_test.go +++ b/routing/missioncontrol_test.go @@ -228,7 +228,8 @@ func TestMissionControl(t *testing.T) { } // TestMissionControlChannelUpdate tests that the first channel update is not -// penalizing the channel yet. +// fully penalizing the channel. Instead we should see a half penalty being +// applied to the probability. func TestMissionControlChannelUpdate(t *testing.T) { ctx := createMcTestContext(t) @@ -237,7 +238,9 @@ func TestMissionControlChannelUpdate(t *testing.T) { ctx.reportFailure( 0, lnwire.NewFeeInsufficient(0, lnwire.ChannelUpdate1{}), ) - ctx.expectP(100, testAprioriHopProbability) + + // Apply the second chance half-penalty. + ctx.expectP(100, testAprioriHopProbability*0.5) // Report another failure for the same channel. We expect it to be // pruned. From 2efc7afcd10d3a7d7e7b69adeb95f09386c42e75 Mon Sep 17 00:00:00 2001 From: George Tsagkarelis Date: Wed, 8 Oct 2025 16:15:29 +0200 Subject: [PATCH 2/3] routing: add test with ineffective second chance We add a test to demonstrate that the second chance penalty can be effective. In this test an initially cheap route is attempted but errors with a new returned channel policy. After applying that policy we should pick the cheaper route again, but the second-chance penalty will prevent us from doing so. Instead an alternative route succeeds. --- routing/router_test.go | 123 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/routing/router_test.go b/routing/router_test.go index b811793d258..f7345d9b137 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -781,6 +781,129 @@ func TestSendPaymentErrorFeeInsufficientPrivateEdge(t *testing.T) { ) } +// TestSendPaymentErrorFeeInsufficientSecondChance tests that the half-penalty +// behavior of the second chance logic is properly applied when receiving +// errors that contain channel updates. We expose the path where the second +// chance penalty along with the fee update are sufficient for a different route +// to be picked. +func TestSendPaymentErrorFeeInsufficientSecondChance(t *testing.T) { + t.Parallel() + + const startingBlockHeight = 101 + ctx := createTestCtxFromFile(t, startingBlockHeight, basicGraphFilePath) + + // Get the channel ID for the first hop of the route that will retrieve + // the second chance. + roasbeefSongoku := lnwire.NewShortChanIDFromInt( + ctx.getChannelIDFromAlias(t, "roasbeef", "songoku"), + ) + + var ( + preImage [32]byte + amt = lnwire.NewMSatFromSatoshis(1000) + expiryDelta = uint16(32) + ) + + // Craft a LightningPayment struct that'll send a payment from roasbeef + // to elst. Initially a path via songoku will be picked as it's the one + // with significantly lower fees. + payment := createDummyLightningPayment( + t, ctx.aliases["elst"], amt, + ) + + // Grab the chan ID that we'll apply the channel update on. + chanID := ctx.getChannelIDFromAlias(t, "songoku", "sophon") + + // Grab the chan ID of the expensive channel. We'll use this channel's + // policy as a reference to what will be provided in the other channel's + // update. + expChanID := ctx.getChannelIDFromAlias(t, "roasbeef", "phamnuwen") + _, expPolicy, _, err := ctx.graph.FetchChannelEdgesByID(expChanID) + require.NoError(t, err) + + // Prepare an error update for the channel where the fees are updated to + // be equal to the alternative (more expensive) route. We also subtract + // some minimal fee delta to make it look as if this route is still more + // favorable (by a small margin). Although, because of the second-chance + // penalty, we'll end up using the alternative route. + feeDelta := uint32(1000) + newFeeBase := uint32(expPolicy.FeeBaseMSat) - feeDelta + newFeePpm := uint32(expPolicy.FeeProportionalMillionths) - feeDelta + + errChanUpdate := lnwire.ChannelUpdate1{ + ShortChannelID: lnwire.NewShortChanIDFromInt(chanID), + Timestamp: uint32(testTime.Add(time.Minute).Unix()), + BaseFee: newFeeBase, + FeeRate: newFeePpm, + TimeLockDelta: expiryDelta, + } + signErrChanUpdate(t, ctx.privKeys["songoku"], &errChanUpdate) + + // We'll now modify the SendHTLC method to return an error for the + // songoku->sophon channel. + errorReturned := false + copy(preImage[:], bytes.Repeat([]byte{9}, 32)) + + dispatch, ok := ctx.router.cfg.Payer.(*mockPaymentAttemptDispatcherOld) + require.True(t, ok) + + dispatch.setPaymentResult( + func(firstHop lnwire.ShortChannelID) ([32]byte, error) { + if firstHop != roasbeefSongoku || errorReturned { + return preImage, nil + } + + errorReturned = true + + return [32]byte{}, htlcswitch.NewForwardingError( + // Within our error, we'll add a + // channel update which is meant to + // reflect the new fee schedule for the + // node/channel. + &lnwire.FailFeeInsufficient{ + Update: errChanUpdate, + }, 1, + ) + }, + ) + + // Send off the payment request to the router, what's expected to happen + // here is: + // * attempt 1: roasbeef -> songoku -> sophon -> elst, with an error + // that contains a channel update for songoku->sophon. + // * attempt 2: roasbeef -> phamnuwen -> sophon -> elst, with success. + paymentPreImage, route, err := ctx.router.SendPayment(payment) + require.NoErrorf(t, err, "unable to send payment: %v", + payment.paymentHash) + + require.True(t, errorReturned, + "failed to simulate error in the first payment attempt", + ) + + // The route selected should have three hops. Make sure that, + // path: roasbeef -> pham nuwen -> sophon -> elst + // is selected. + require.Equal(t, 3, len(route.Hops), "incorrect route length") + + // The preimage should match up with the one created above. + require.Equal(t, + paymentPreImage[:], preImage[:], "incorrect preimage used", + ) + + // The route should have pham nuwen as the first hop. + require.Equal(t, route.Hops[0].PubKeyBytes, ctx.aliases["phamnuwen"], + "route should go through pham nuwen as first hop", + ) + + expectedFinalHop := ctx.getChannelIDFromAlias(t, "sophon", "elst") + + // The route should pass via the expected final hop. + require.Equal(t, + expectedFinalHop, route.FinalHop().ChannelID, + "route did not pass through expected final hop", + ) +} + // TestSendPaymentPrivateEdgeUpdateFeeExceedsLimit tests that upon receiving a // ChannelUpdate in a fee related error from the private channel, we won't // choose the route in our second attempt if the updated fee exceeds our fee From 96f7165f317cc7512a882ec7d8553aa86ab04c14 Mon Sep 17 00:00:00 2001 From: George Tsagkarelis Date: Wed, 8 Oct 2025 16:55:43 +0200 Subject: [PATCH 3/3] docs: add release note --- docs/release-notes/release-notes-0.21.0.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/release-notes/release-notes-0.21.0.md b/docs/release-notes/release-notes-0.21.0.md index 9749b580763..14e41b4bcc8 100644 --- a/docs/release-notes/release-notes-0.21.0.md +++ b/docs/release-notes/release-notes-0.21.0.md @@ -24,6 +24,12 @@ # New Features ## Functional Enhancements +- The second chance mechanism of the pathfinder was adjusted to apply a +significant penalty to the route instead of immediately retrying it. This means +that if a failure carries a fee policy update we'll still take it into account +but will retry that route with a lower priority. See more in the +[PR](https://github.com/lightningnetwork/lnd/pull/10278). + ## RPC Additions ## lncli Additions