Skip to content

Commit

Permalink
fix bug with /pools filter being applied incorrectly
Browse files Browse the repository at this point in the history
  • Loading branch information
p0mvn committed Aug 2, 2024
1 parent ba4275b commit ed4408f
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 5 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 10 additions & 2 deletions pools/delivery/http/pools_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()})
Expand Down
20 changes: 18 additions & 2 deletions tests/sqs_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
48 changes: 47 additions & 1 deletion tests/test_pools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)}"

0 comments on commit ed4408f

Please sign in to comment.