refactor(SQS): filter out duplicate pool ID routes (#7099) (#7102)

(cherry picked from commit 2ee6709)

Co-authored-by: Roman <[email protected]>

ingest/sqs/router/usecase/export_test.go

@@ -48,3 +48,7 @@ func (r Router) GetSortedPoolIDs() []uint64 {
 	}
 	return sortedPoolIDs
 }
+
+func FilterDuplicatePoolIDRoutes(rankedRoutes []route.RouteImpl) []route.RouteImpl {
+	return filterDuplicatePoolIDRoutes(rankedRoutes)
+}

ingest/sqs/router/usecase/optimized_routes_test.go

@@ -614,7 +614,7 @@ func (s *RouterTestSuite) TestGetOptimalQuote() {
 
 			amountIn: osmomath.NewInt(100_000_000),
 
-			expectedRoutesCount: 2,
+			expectedRoutesCount: 1,
 		},
 		// This test validates that with a greater max routes value, SQS is able to find
 		// the path from umee to stOsmo

ingest/sqs/router/usecase/router_usecase.go

@@ -90,6 +90,9 @@ func (r *routerUseCaseImpl) GetOptimalQuote(ctx context.Context, tokenIn sdk.Coi
 			return nil, fmt.Errorf("no ranked routes found")
 		}
 
+		// Update ranked routes with filtered ranked routes
+		rankedRoutes = filterDuplicatePoolIDRoutes(rankedRoutes)
+
 		// TODO: Cache ranked routes
 	}
 
@@ -128,6 +131,52 @@ func (r *routerUseCaseImpl) GetOptimalQuote(ctx context.Context, tokenIn sdk.Coi
 	return finalQuote, nil
 }
 
+// filterDuplicatePoolIDRoutes filters routes that contain duplicate pool IDs.
+// CONTRACT: rankedRoutes are sorted in decreasing order by amount out
+// from first to last.
+func filterDuplicatePoolIDRoutes(rankedRoutes []route.RouteImpl) []route.RouteImpl {
+	// We use two maps for all routes and for the current route.
+	// This is so that if a route ends up getting filtered, its pool IDs are not added to the combined map.
+	combinedPoolIDsMap := make(map[uint64]struct{})
+	filteredRankedRoutes := make([]route.RouteImpl, 0)
+
+	for _, route := range rankedRoutes {
+		pools := route.GetPools()
+
+		currentRoutePoolIDsMap := make(map[uint64]struct{})
+
+		existsPoolID := false
+
+		for _, pool := range pools {
+			poolID := pool.GetId()
+
+			_, existsPoolID = combinedPoolIDsMap[poolID]
+
+			// If found a pool ID that was already seen, break and skip the route.
+			if existsPoolID {
+				break
+			}
+
+			// Add pool ID to current pool IDs map
+			currentRoutePoolIDsMap[poolID] = struct{}{}
+		}
+
+		// If pool ID exists, we skip this route
+		if existsPoolID {
+			continue
+		}
+
+		// Merge current route pool IDs map into the combined map
+		for poolID := range currentRoutePoolIDsMap {
+			combinedPoolIDsMap[poolID] = struct{}{}
+		}
+
+		// Add route to filtered ranked routes
+		filteredRankedRoutes = append(filteredRankedRoutes, route)
+	}
+	return filteredRankedRoutes
+}
+
 // rankRoutesByDirectQuote ranks the given candidate routes by estimating direct quotes over each route.
 // Returns the top quote as well as the ranked routes in decrease order of amount out.
 // Returns error if:

ingest/sqs/router/usecase/router_usecase_test.go

@@ -199,3 +199,140 @@ func (s *RouterTestSuite) TestHandleRoutes() {
 		})
 	}
 }
+
+// Tests that routes that overlap in pools IDs get filtered out.
+// Tests that the order of the routes is in decreasing priority.
+// That is, if routes A and B overlap where A comes before B, then B is filtered out.
+// Additionally, tests that overlapping within the same route has no effect on fitlering.
+// Lastly, validates that if a route overlaps with subsequent routes in the list but gets filtered out,
+// then subesequent routes are not affected by filtering.
+func (s *RouterTestSuite) TestFilterDuplicatePoolIDRoutes() {
+	var (
+		deafaultPool = &mocks.MockRoutablePool{ID: defaultPoolID}
+
+		otherPool = &mocks.MockRoutablePool{ID: defaultPoolID + 1}
+
+		defaultSingleRoute = WithRoutePools(route.RouteImpl{}, []domain.RoutablePool{
+			deafaultPool,
+		})
+	)
+
+	tests := map[string]struct {
+		routes []route.RouteImpl
+
+		expectedRoutes []route.RouteImpl
+	}{
+		"empty routes": {
+			routes:         []route.RouteImpl{},
+			expectedRoutes: []route.RouteImpl{},
+		},
+
+		"single route single pool": {
+			routes: []route.RouteImpl{
+				defaultSingleRoute,
+			},
+
+			expectedRoutes: []route.RouteImpl{
+				defaultSingleRoute,
+			},
+		},
+
+		"single route two different pools": {
+			routes: []route.RouteImpl{
+				WithRoutePools(route.RouteImpl{}, []domain.RoutablePool{
+					deafaultPool,
+					otherPool,
+				}),
+			},
+
+			expectedRoutes: []route.RouteImpl{
+				WithRoutePools(route.RouteImpl{}, []domain.RoutablePool{
+					deafaultPool,
+					otherPool,
+				}),
+			},
+		},
+
+		// Note that filtering only happens if pool ID duplciated across different routes.
+		// Duplicate pool IDs within the same route are filtered out at a different step
+		// in the router logic.
+		"single route two same pools (have no effect on filtering)": {
+			routes: []route.RouteImpl{
+				WithRoutePools(route.RouteImpl{}, []domain.RoutablePool{
+					deafaultPool,
+					deafaultPool,
+				}),
+			},
+
+			expectedRoutes: []route.RouteImpl{
+				WithRoutePools(route.RouteImpl{}, []domain.RoutablePool{
+					deafaultPool,
+					deafaultPool,
+				}),
+			},
+		},
+
+		"two single hop routes and no duplicates": {
+			routes: []route.RouteImpl{
+				defaultSingleRoute,
+
+				WithRoutePools(route.RouteImpl{}, []domain.RoutablePool{
+					otherPool,
+				}),
+			},
+
+			expectedRoutes: []route.RouteImpl{
+				defaultSingleRoute,
+
+				WithRoutePools(route.RouteImpl{}, []domain.RoutablePool{
+					otherPool,
+				}),
+			},
+		},
+
+		"two single hop routes with duplicates (second filtered)": {
+			routes: []route.RouteImpl{
+				defaultSingleRoute,
+
+				defaultSingleRoute,
+			},
+
+			expectedRoutes: []route.RouteImpl{
+				defaultSingleRoute,
+			},
+		},
+
+		"three route. first and second overlap. second and third overlap. second is filtered out but not third": {
+			routes: []route.RouteImpl{
+				defaultSingleRoute,
+
+				WithRoutePools(route.RouteImpl{}, []domain.RoutablePool{
+					deafaultPool, // first and second overlap
+					otherPool,    // second and third overlap
+				}),
+
+				WithRoutePools(route.RouteImpl{}, []domain.RoutablePool{
+					otherPool,
+				}),
+			},
+
+			expectedRoutes: []route.RouteImpl{
+				defaultSingleRoute,
+
+				WithRoutePools(route.RouteImpl{}, []domain.RoutablePool{
+					otherPool,
+				}),
+			},
+		},
+	}
+
+	for name, tc := range tests {
+		tc := tc
+		s.Run(name, func() {
+
+			actualRoutes := usecase.FilterDuplicatePoolIDRoutes(tc.routes)
+
+			s.Require().Equal(len(tc.expectedRoutes), len(actualRoutes))
+		})
+	}
+}