From ed4408fd972af6f651440f7798e5b0e5b500314e Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 2 Aug 2024 00:00:44 +0000 Subject: [PATCH] fix bug with /pools filter being applied incorrectly --- CHANGELOG.md | 6 ++++ pools/delivery/http/pools_handler.go | 12 +++++-- tests/sqs_service.py | 20 ++++++++++-- tests/test_pools.py | 48 +++++++++++++++++++++++++++- 4 files changed, 81 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3324925e..dc339179 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,12 @@ Ref: https://keepachangelog.com/en/1.0.0/ # Changelog +## 25.7.0 + +- Fix bug with filters n /pools API + +Chain compatibility: v25.x-c8140e68-1722532245 + ## 25.6.0 - Mitigate the alloy rebalancing router breakage by invalidating route cache if all routes fail and ordering denoms by balances. diff --git a/pools/delivery/http/pools_handler.go b/pools/delivery/http/pools_handler.go index 3839d3ba..3684293a 100644 --- a/pools/delivery/http/pools_handler.go +++ b/pools/delivery/http/pools_handler.go @@ -91,10 +91,18 @@ func (a *PoolsHandler) GetPools(c echo.Context) error { } } + filters := []domain.PoolsOption{ + domain.WithMinPoolsLiquidityCap(minLiquidityCap), + } + + // Only add pool ID filter if it is not empty. + if len(poolIDs) > 0 { + filters = append(filters, domain.WithPoolIDFilter(poolIDs)) + } + // Get pools pools, err = a.PUsecase.GetPools( - domain.WithMinPoolsLiquidityCap(minLiquidityCap), - domain.WithPoolIDFilter(poolIDs), + filters..., ) if err != nil { return c.JSON(getStatusCode(err), ResponseError{Message: err.Error()}) diff --git a/tests/sqs_service.py b/tests/sqs_service.py index 34d9d2d3..d84fd791 100644 --- a/tests/sqs_service.py +++ b/tests/sqs_service.py @@ -52,12 +52,28 @@ def get_config(self): return self.config - def get_pool(self, pool_id): + def get_pools(self, pool_ids=None, min_liquidity_cap=None): """ Fetches the pool from the specified endpoint and returns it. Raises error if non-200 is returned from the endpoint. """ - response = requests.get(self.url + f"{POOLS_URL}?IDs={pool_id}", headers=self.headers) + url_ext = f"{POOLS_URL}" + + is_pool_id_filter_provided = pool_ids is not None + is_min_liquidity_cap_filter_provided = min_liquidity_cap is not None + if pool_ids is not None or is_min_liquidity_cap_filter_provided: + url_ext += "?" + + if is_pool_id_filter_provided: + url_ext += f"IDs={pool_ids}" + + if is_pool_id_filter_provided and is_min_liquidity_cap_filter_provided: + url_ext += "&" + + if is_min_liquidity_cap_filter_provided: + url_ext += f"min_liquidity_cap={min_liquidity_cap}" + + response = requests.get(self.url + url_ext, headers=self.headers) if response.status_code != 200: raise Exception(f"Error fetching pool: {response.text}") diff --git a/tests/test_pools.py b/tests/test_pools.py index 5dc20a1f..fb03a257 100644 --- a/tests/test_pools.py +++ b/tests/test_pools.py @@ -11,6 +11,7 @@ # Min liquidity capitalization in USDC for a pool to be considered # in tests. Arbitrarily chosen as to avoid flakiness. min_pool_liquidity_cap_usdc = 50_000 +num_all_pools_expected = 1500 def filter_pools(pools_data, min_pool_liquidity_cap_usdc): @@ -50,7 +51,7 @@ def test_pools_pool_liquidity_cap(self, environment_url, pool_data): if pool_id in skip_alloyed_pool_id: pytest.skip("Skipping alloyed pool since it has flakiness on Numia side") - sqs_pool = sqs_service.get_pool(pool_id) + sqs_pool = sqs_service.get_pools(pool_ids=pool_id) # Skip white whale pool since it has flakiness on Numia side code_id = pool_data.get("code_id") @@ -97,3 +98,48 @@ def test_canonical_orderbook(self, environment_url): # This may fail as we keep adding more orderbooks. If it does, we need to refactor this test. assert didFind, "Expected canonical orderbook was not found" + + def test_pool_filters(self, environment_url): + """ + Test the pool filters for the /pools endpoint. + - No filters + - Only pool ID filter + - Min Liquidity cap filter + - Both pool ID and min liquidity cap filter + """ + + sqs_service = SERVICE_MAP[environment_url] + + # $100k + min_liquidity_cap = 100_000 + # # Pool 1 and 135 are major pools but 37 is junk. + pool_id_filter = "1,1135,37" + + expected_num_filtered_pools = 3 + # Filter pool id 37 which s junk + expected_num_pools_both_filters = expected_num_filtered_pools - 1 + + # 50 is chosen arbitrarily low to avoid flakiness + expected_num_min_liq_cap_pools = 50 + + # Test with all filters + all_pools = sqs_service.get_pools() + assert all_pools is not None, "All pools are None" + assert len(all_pools) > num_all_pools_expected, f"Number of all pools should be greater than {len(all_pools)}" + + + filtered_pools = sqs_service.get_pools(pool_ids=pool_id_filter) + assert filtered_pools is not None, "Fitlered pools are None" + assert len(filtered_pools) == expected_num_filtered_pools, f"Number of filtered pools should be {expected_num_filtered_pools}, actual {len(filtered_pools)}" + + # Test with min liquidity cap filter + filtered_pools = sqs_service.get_pools(min_liquidity_cap=min_liquidity_cap) + assert filtered_pools is not None, "Fitlered pools are None" + + assert len(filtered_pools) > expected_num_min_liq_cap_pools, f"Number of filtered pools should be greater than {expected_num_min_liq_cap_pools}" + + # Test with both pool ID and min liquidity cap filter + filtered_pools = sqs_service.get_pools(pool_ids=pool_id_filter, min_liquidity_cap=min_liquidity_cap) + assert filtered_pools is not None, "Fitlered pools are None" + + assert len(filtered_pools) == expected_num_pools_both_filters, f"Number of filtered pools should be {expected_num_pools_both_filters}, actual {len(filtered_pools)}"