fix(screener): drop market_cap, which duplicated liquidity, and scope to one network - #195
Merged
Miracle656 merged 1 commit intoSep 30, 2026
Conversation
… to one network /screener returned COALESCE(pl.liquidity, 0) under both liquidity and market_cap, so sorting or filtering by market cap silently used liquidity. Lens has no circulating-supply data, so remove the field from the response, sort allowlist and filter set, and answer ?market_cap= and sortBy=market_cap with a 400 instead of ignoring them. The three CTEs also had no network predicate and pooled testnet and mainnet rows. Scope them to req.network (default: active network). Add route tests covering sort, filters, pagination, cursor and network.
|
@blockchain-maxis Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Miracle656
added a commit
that referenced
this pull request
Sep 30, 2026
Every function in src/aggregator/vwap.ts now takes a required network and filters price_points and pool_snapshots on it, getAMMPrice filters both legs of its pool lookup, /price/:assetA/:assetB passes req.network through to the aggregator, and the aggregate refresh worker runs once per enabled network instead of pinning itself to whichever network the instance happens to be indexing. Merged locally: src/__tests__/price.test.ts conflicted only because #184 and 1c7e200 appended test blocks to the same tail; resolved as a union. The README paragraph was corrected on merge — #195, #202 and #205 landed network scoping for /screener, /pools, /depth and /prices/history after this branch was written, so the list of still-unscoped endpoints was stale. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #186
Summary
/screenerreturnedCOALESCE(pl.liquidity::float, 0)under bothliquidityandmarket_cap, so sorting or filtering by market cap silently sorted or filtered liquidity. Lens has no circulating-supply data, so a real market cap can't be computed. Per the issue, the honest default is to remove it.Changes
market_capremoved from the response, thesortByallowlist, the filter set and the CTE.?market_cap=and?sortBy=market_capnow return400with an explanation, rather than being silently ignored. An ignored filter would be another confidently wrong answer.price_points,pool_snapshots,price_aggregates) now filter onnetwork = $1, taken fromreq.network(?network=/x-network, default: active network). Before this, testnet and mainnet rows were pooled and a pair could be ranked on the other chain's data.src/__tests__/screener.test.ts(15 tests): nomarket_capin the response, both 400s, sort direction and default, every filter bound to the right parameter, page size and cursor (keyset condition), malformed cursor, network scoping on all three CTEs,x-network, an unknown network, and a 500 that doesn't leak the error.patch).Docs
/screenerandmarket_capappear in neitherREADME.mdnoropenapi.yaml/openapi.json(nordocs/orclients/), so there was nothing to remove there. I did not add the route to the OpenAPI spec, to keep this PR to the issue.Verification
Fork PRs run no CI here, so this was run locally:
npx tsc --noEmit: cleannpx vitest run src/__tests__/screener.test.ts: 15 passedscreener.ts, 7 of those 15 fail (themarket_capand network cases); the other 8 cover behaviour that was already correctnpx vitest run(full suite): 415 passed, 1 skipped, 0 failed