feat: option to hide locked markets#333
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a "locked markets" filter feature that lets users toggle visibility of markets with exceptionally high APY (over 1500%). Includes a new feature flag in market preferences, UI toggles across settings panels, filter logic to identify locked markets, a refactor of the BlacklistedMarketsDetail component, and two new blacklisted market entries. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modals/settings/monarch-settings/details/BlacklistedMarketsDetail.tsx (1)
60-66: ClampcurrentPagewhen results shrink.If the user is on a later page and the result set shrinks (e.g., after blacklisting),
paginatedMarketscan go empty while results still exist. Clamp or reset the page whentotalPagesdrops.Proposed fix
const totalPages = Math.ceil(filteredAvailableMarkets.length / ITEMS_PER_PAGE); +useEffect(() => { + if (totalPages === 0) { + if (currentPage !== 1) setCurrentPage(1); + return; + } + if (currentPage > totalPages) { + setCurrentPage(totalPages); + } +}, [totalPages, currentPage]); const paginatedMarkets = useMemo(() => { const startIndex = (currentPage - 1) * ITEMS_PER_PAGE; const endIndex = startIndex + ITEMS_PER_PAGE; return filteredAvailableMarkets.slice(startIndex, endIndex); }, [filteredAvailableMarkets, currentPage]);
🧹 Nitpick comments (2)
src/utils/marketFilters.ts (1)
110-120: Make non‑finite APY handling explicit.Right now
NaN/Infinityare implicitly treated as “locked” via the comparison. If that’s intended, make it explicit so the behavior is clear to readers.Possible tweak
return (market) => { - const supplyApy = market.state?.supplyApy ?? 0; - return supplyApy <= LOCKED_MARKET_APY_THRESHOLD; + const supplyApy = market.state?.supplyApy; + if (supplyApy === undefined || supplyApy === null) return true; + if (!Number.isFinite(supplyApy)) return false; + return supplyApy <= LOCKED_MARKET_APY_THRESHOLD; };src/features/positions/components/markets-filter-compact.tsx (1)
145-165: Avoid hardcoding the APY threshold in copy.
Use the shared constant so UI stays in sync if the threshold changes.Proposed change
-import { MONARCH_PRIMARY } from '@/constants/chartColors'; +import { MONARCH_PRIMARY } from '@/constants/chartColors'; +import { LOCKED_MARKET_APY_THRESHOLD } from '@/constants/markets'; ... - description="Display frozen markets with extreme APY (> 1500%)." + description={`Display frozen markets with extreme APY (> ${LOCKED_MARKET_APY_THRESHOLD}%).`}
starksama
left a comment
There was a problem hiding this comment.
Clean and straightforward implementation. Few notes:
What I like:
- APY threshold approach is smart — way simpler than trying to detect locked vaults via on-chain state.
supplyApy > 15(1500%) catches dead/frozen markets without false positives on legit high-yield markets. - Filter composition pattern matches
createUnknownTokenFilter/createUnknownOracleFilterexactly — easy to follow. - Default
false(hidden) is the right UX — users don't want to see garbage 999999% APY markets unless they explicitly opt in. - BlacklistedMarketsDetail redesign looks tighter — the card-based layout with
bg-surface→bg-surface-softnesting is consistent with other settings panels.
Minor observations (non-blocking):
- The account-actions-popover (DeBank link) is unrelated to locked markets — might be cleaner as a separate atomic commit, but fine to ship together.
LOCKED_MARKET_APY_THRESHOLD = 15comment says "where 1.0 = 100%, so 15 = 1500%" which is clear. Just noting the Morpho API returns APY as a raw decimal (e.g.0.05= 5%), so the threshold is actually checkingmarket.state.supplyApy > 15which is > 1500% — confirmed correct.
LGTM ✅
Summary by CodeRabbit
New Features
Improvements
Updates
✏️ Tip: You can customize this high-level summary in your review settings.