Skip to content

routing: remove second chance logic #6891

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 0 additions & 33 deletions routing/missioncontrol.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,6 @@ const (
// channel is back at 50% probability.
DefaultPenaltyHalfLife = time.Hour

// minSecondChanceInterval is the minimum time required between
// second-chance failures.
//
// If nodes return a channel policy related failure, they may get a
// second chance to forward the payment. It could be that the channel
// policy that we are aware of is not up to date. This is especially
// important in case of mobile apps that are mostly offline.
//
// However, we don't want to give nodes the option to endlessly return
// new channel updates so that we are kept busy trying to route through
// that node until the payment loop times out.
//
// Therefore we only grant a second chance to a node if the previous
// second chance is sufficiently long ago. This is what
// minSecondChanceInterval defines. If a second policy failure comes in
// within that interval, we will apply a penalty.
//
// Second chances granted are tracked on the level of node pairs. This
// means that if a node has multiple channels to the same peer, they
// will only get a single second chance to route to that peer again.
// Nodes forward non-strict, so it isn't necessary to apply a less
// restrictive channel level tracking scheme here.
minSecondChanceInterval = time.Minute

// DefaultMaxMcHistory is the default maximum history size.
DefaultMaxMcHistory = 1000

Expand Down Expand Up @@ -475,15 +451,6 @@ func (m *MissionControl) applyPaymentResult(
result.failure,
)

if i.policyFailure != nil {
if m.state.requestSecondChance(
result.timeReply,
i.policyFailure.From, i.policyFailure.To,
) {
return nil
}
}

// If there is a node-level failure, record a failure for every tried
// connection of that node. A node-level failure can be considered as a
// failure that would have occurred with any of the node's channels.
Expand Down
37 changes: 0 additions & 37 deletions routing/missioncontrol_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ type missionControlState struct {
// particular node.
lastPairResult map[route.Vertex]NodeResults

// lastSecondChance tracks the last time a second chance was granted for
// a directed node pair.
lastSecondChance map[DirectedNodePair]time.Time

// minFailureRelaxInterval is the minimum time that must have passed
// since the previously recorded failure before the failure amount may
// be raised.
Expand All @@ -32,7 +28,6 @@ func newMissionControlState(

return &missionControlState{
lastPairResult: make(map[route.Vertex]NodeResults),
lastSecondChance: make(map[DirectedNodePair]time.Time),
minFailureRelaxInterval: minFailureRelaxInterval,
}
}
Expand All @@ -49,7 +44,6 @@ func (m *missionControlState) getLastPairResult(node route.Vertex) (NodeResults,
// if no payment attempts have been made.
func (m *missionControlState) resetHistory() {
m.lastPairResult = make(map[route.Vertex]NodeResults)
m.lastSecondChance = make(map[DirectedNodePair]time.Time)
}

// setLastPairResult stores a result for a node pair.
Expand Down Expand Up @@ -153,37 +147,6 @@ func (m *missionControlState) setAllFail(node route.Vertex,
}
}

// requestSecondChance checks whether the node fromNode can have a second chance
// at providing a channel update for its channel with toNode.
func (m *missionControlState) requestSecondChance(timestamp time.Time,
fromNode, toNode route.Vertex) bool {

// Look up previous second chance time.
pair := DirectedNodePair{
From: fromNode,
To: toNode,
}
lastSecondChance, ok := m.lastSecondChance[pair]

// If the channel hasn't already be given a second chance or its last
// second chance was long ago, we give it another chance.
if !ok || timestamp.Sub(lastSecondChance) > minSecondChanceInterval {
m.lastSecondChance[pair] = timestamp

log.Debugf("Second chance granted for %v->%v", fromNode, toNode)

return true
}

// Otherwise penalize the channel, because we don't allow channel
// updates that are that frequent. This is to prevent nodes from keeping
// us busy by continuously sending new channel updates.
log.Debugf("Second chance denied for %v->%v, remaining interval: %v",
fromNode, toNode, timestamp.Sub(lastSecondChance))

return false
}

// GetHistorySnapshot takes a snapshot from the current mission control state
// and actual probability estimates.
func (m *missionControlState) getSnapshot() *MissionControlSnapshot {
Expand Down
20 changes: 0 additions & 20 deletions routing/missioncontrol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,23 +202,3 @@ func TestMissionControl(t *testing.T) {
// Test reporting a success.
ctx.reportSuccess()
}

// TestMissionControlChannelUpdate tests that the first channel update is not
// penalizing the channel yet.
func TestMissionControlChannelUpdate(t *testing.T) {
ctx := createMcTestContext(t)

// Report a policy related failure. Because it is the first, we don't
// expect a penalty.
ctx.reportFailure(
0, lnwire.NewFeeInsufficient(0, lnwire.ChannelUpdate{}),
)
ctx.expectP(100, testAprioriHopProbability)

// Report another failure for the same channel. We expect it to be
// pruned.
ctx.reportFailure(
0, lnwire.NewFeeInsufficient(0, lnwire.ChannelUpdate{}),
)
ctx.expectP(100, 0)
}
20 changes: 0 additions & 20 deletions routing/result_interpretation.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,6 @@ type interpretedResult struct {
// sense to start another payment attempt. It will contain the reason
// why.
finalFailureReason *channeldb.FailureReason

// policyFailure is set to a node pair if there is a policy failure on
// that connection. This is used to control the second chance logic for
// policy failures.
policyFailure *DirectedNodePair
}

// interpretResult interprets a payment outcome and returns an object that
Expand Down Expand Up @@ -338,14 +333,6 @@ func (i *interpretedResult) processPaymentOutcomeIntermediate(
// 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()

// All nodes up to the failing pair must have forwarded
Expand All @@ -368,13 +355,6 @@ func (i *interpretedResult) processPaymentOutcomeIntermediate(
*lnwire.FailFeeInsufficient,
*lnwire.FailIncorrectCltvExpiry:

// 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,
}

// We report incoming channel. If a second pair is granted in
// mission control, this report is ignored.
reportIncoming()
Expand Down
7 changes: 0 additions & 7 deletions routing/result_interpretation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@ func getTestPair(from, to int) DirectedNodePair {
return NewDirectedNodePair(hops[from], hops[to])
}

func getPolicyFailure(from, to int) *DirectedNodePair {
pair := getTestPair(from, to)
return &pair
}

type resultTestCase struct {
name string
route *route.Route
Expand Down Expand Up @@ -207,7 +202,6 @@ var resultTestCases = []resultTestCase{
getTestPair(1, 2): {},
getTestPair(2, 1): {},
},
policyFailure: getPolicyFailure(2, 3),
},
},

Expand Down Expand Up @@ -363,7 +357,6 @@ var resultTestCases = []resultTestCase{
getTestPair(2, 1): failPairResult(0),
getTestPair(0, 1): successPairResult(100),
},
policyFailure: getPolicyFailure(1, 2),
},
},
}
Expand Down