From 35dd991776c8dcd19fa4dca524d8efc150cdef0d Mon Sep 17 00:00:00 2001
From: pascal-fischer <32096965+pascal-fischer@users.noreply.github.com>
Date: Thu, 2 May 2024 11:51:03 +0200
Subject: [PATCH] Fix best route selection (#1903)

* fix route comparison to current selected route + adding tests

* add comment and debug log

* adjust log message

---------

Co-authored-by: Maycon Santos <mlsmaycon@gmail.com>
---
 client/internal/routemanager/client.go      | 15 +++++-----
 client/internal/routemanager/client_test.go | 33 ++++++++++++++++++++-
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/client/internal/routemanager/client.go b/client/internal/routemanager/client.go
index a8f0c8677..ef71bb60d 100644
--- a/client/internal/routemanager/client.go
+++ b/client/internal/routemanager/client.go
@@ -153,15 +153,16 @@ func (c *clientNetwork) getBestRouteFromStatuses(routePeerStatuses map[string]ro
 
 		log.Warnf("the network %s has not been assigned a routing peer as no peers from the list %s are currently connected", c.network, peers)
 	case chosen != currID:
-		if currScore != 0 && currScore < chosenScore+0.1 {
+		// we compare the current score + 10ms to the chosen score to avoid flapping between routes
+		if currScore != 0 && currScore+0.01 > chosenScore {
+			log.Debugf("keeping current routing peer because the score difference with latency is less than 0.01(10ms), current: %f, new: %f", currScore, chosenScore)
 			return currID
-		} else {
-			var peer string
-			if route := c.routes[chosen]; route != nil {
-				peer = route.Peer
-			}
-			log.Infof("new chosen route is %s with peer %s with score %f for network %s", chosen, peer, chosenScore, c.network)
 		}
+		var p string
+		if rt := c.routes[chosen]; rt != nil {
+			p = rt.Peer
+		}
+		log.Infof("new chosen route is %s with peer %s with score %f for network %s", chosen, p, chosenScore, c.network)
 	}
 
 	return chosen
diff --git a/client/internal/routemanager/client_test.go b/client/internal/routemanager/client_test.go
index d24d42b8e..ca1456c92 100644
--- a/client/internal/routemanager/client_test.go
+++ b/client/internal/routemanager/client_test.go
@@ -241,7 +241,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 					connected: true,
 					relayed:   false,
 					direct:    true,
-					latency:   12 * time.Millisecond,
+					latency:   15 * time.Millisecond,
 				},
 				"route2": {
 					connected: true,
@@ -265,6 +265,37 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 			currentRoute:    "route1",
 			expectedRouteID: "route1",
 		},
+		{
+			name: "current route with bad score should be changed to route with better score",
+			statuses: map[string]routerPeerStatus{
+				"route1": {
+					connected: true,
+					relayed:   false,
+					direct:    true,
+					latency:   200 * time.Millisecond,
+				},
+				"route2": {
+					connected: true,
+					relayed:   false,
+					direct:    true,
+					latency:   10 * time.Millisecond,
+				},
+			},
+			existingRoutes: map[string]*route.Route{
+				"route1": {
+					ID:     "route1",
+					Metric: route.MaxMetric,
+					Peer:   "peer1",
+				},
+				"route2": {
+					ID:     "route2",
+					Metric: route.MaxMetric,
+					Peer:   "peer2",
+				},
+			},
+			currentRoute:    "route1",
+			expectedRouteID: "route2",
+		},
 		{
 			name: "current chosen route doesn't exist anymore",
 			statuses: map[string]routerPeerStatus{