feat: Support empty and inactive CL pools#4231
Conversation
Adds fallback mechanism to query chain directly when sidecar returns null for pools with no liquidity. Prevents crashes when tick data or historical data is unavailable for concentrated liquidity pools. Changes: - Add chain query fallback in pools index when sidecar returns null - Add safety checks in sidecar provider for empty pool data - Add null checks in poolmanager pools query - Add optional chaining for tick data in add liquidity config - Add safety checks for empty historical/depth data This ensures pools without liquidity (uninitialized or inactive) can be safely queried and displayed without runtime errors.
Adds translation keys for uninitialized and inactive pool states: - pool.initializePool: Button text for uninitialized pools - pool.inactivePool.title: "Pool Out of Range" - pool.inactivePool.description: Multi-line warning about passive liquidity and potential losses during rebalancing - pool.inactivePool.addLiquidity: "Add Passive Liquidity" button text These translations support the two-tier pool state detection system for concentrated liquidity pools without in range liquidity.
Implements two-tier pool state detection for concentrated liquidity pools: Tier 1 - Uninitialized Pool: - Condition: sqrt_price = 0 AND tick_liquidity = 0 AND TVL = 0 - UI: Show "Initialize Pool" button only Tier 2 - Inactive Pool: - Condition: tick_liquidity = 0 AND TVL > 0 - UI: Show "Pool Out of Range" message with warning and "Add Passive Liquidity" button For both tiers, hide: - 24hr trading volume - Historical price chart - Liquidity depth chart - User positions section Keep visible: - Pool liquidity - Spread factor Uses parseFloat() to properly compare decimal string values like "0.000000000000000000" from chain queries. Prioritizes TVL check first to correctly classify pools with out-of-range liquidity as inactive rather than uninitialized.
Routes users to different add liquidity flows based on pool state: Uninitialized Pools (Tier 1): - Show AddInitialLiquidity interface - Allows setting initial price and liquidity ranges - Uses tokens from pool's reserve coins Inactive Pools (Tier 2): - Show normal AddConcLiquidity flow - Will be restricted to passive strategy in next commit Normal Pools: - Standard concentrated liquidity addition flow Applies same pool state detection logic (using parseFloat for decimal string comparisons and prioritizing TVL check) to determine which interface to display.
Forces passive (full range) strategy for inactive pools to enable rebalancing: Disabled for inactive pools: - Managed strategy (via Magma vaults) - greyed out, not clickable - Custom strategy - greyed out with 40% opacity - Moderate strategy - greyed out with 40% opacity - Aggressive strategy - greyed out with 40% opacity Enabled for inactive pools: - Passive strategy only - auto-selected on modal open Rationale: Inactive pools have all liquidity out of range. Adding passive (full range) liquidity allows the pool to rebalance to current market rates through trading. Concentrated strategies would not help rebalance the pool since they might also be out of range. Implementation: - Detect inactive pool state in AddConcLiquidity component - Pass isInactivePool flag to Overview and AddConcLiqView - Add disabled prop to StrategySelector and PresetStrategyCard - Auto-select passive strategy via useEffect when isInactivePool is true - Apply opacity-40 cursor-not-allowed styling to disabled options
Allows use of custom liquidity provision, advanced users can use this to backstop a price movement when rebalancing manually.
Replace unsafe `as any` type assertions with proper ConcentratedPoolRawResponse type when accessing pool.raw data in concentrated liquidity components. This improves type safety and provides better IDE support without changing any runtime behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Standardize button text to "Add Liquidity" for both uninitialized and inactive pools - Simplify inactive pool description to be more concise and actionable - Specify $1 maximum amount to guide users on appropriate liquidity amount This makes the UI more consistent and the messaging clearer for users encountering out-of-range pools. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Apply prettier formatting to files modified in this branch to ensure consistent code style. Changes include proper spacing in imports, line wrapping for long strings, and consistent function signature formatting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
4 Skipped Deployments
|
|
Currently hangs on the add liquidity for out of range pools - In progress |
Add fallback logic to fetch low-liquidity pools on-demand when not found in cache, and ensure pool data refreshes after liquidity transactions to properly update pool state from inactive to active. Changes: - Add fetchRemainingPools fallback in sendCreateConcentratedLiquidityPoolPositionMsg - Invalidate pool data cache after createPosition and increaseLiquidity transactions - Ensures UI reflects updated pool state after adding liquidity to inactive pools 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughAdds a chain-backed fallback for pool retrieval when Sidecar misses, makes pool TVL optional with a tvlUnknown flag, and introduces detection, UI flows, and calculation paths for inactive and uninitialized concentrated pools across server, stores, hooks, components, pages, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client/App
participant PoolSvc as Pool Service\n(getPool)
participant Sidecar as Sidecar Service\n(getPoolsFromSidecar)
participant Chain as Chain RPC\n(queryPoolChain)
participant Convert as Pool Converter\n(makePoolFromChainPool)
App->>PoolSvc: getPool(poolId)
PoolSvc->>Sidecar: query pool by ID
Sidecar-->>PoolSvc: not found / null
PoolSvc->>Chain: queryPoolChain(poolId)
Chain-->>PoolSvc: raw chain pool data
PoolSvc->>Convert: makePoolFromChainPool(chainPool, assetLists)
Convert-->>PoolSvc: Pool | null
alt conversion successful
PoolSvc-->>App: return Pool
else conversion failed
PoolSvc-->>App: throw/log error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/server/src/queries/complex/pools/index.ts (1)
108-141: Good fallback strategy with one minor improvement opportunity.The chain fallback approach is well-implemented. However, the error at line 139 discards the original error details which could help with debugging.
Consider preserving the original error:
} catch (error) { console.error(`Failed to query pool ${poolId} from chain:`, error); - throw new Error(poolId + " not found"); + throw new Error(`${poolId} not found: ${error instanceof Error ? error.message : "unknown error"}`); }packages/web/modals/add-liquidity.tsx (1)
145-167: Simplify redundant conditional in title.The ternary at line 152 checks
isUninitializedPool, but we're already inside theif (isUninitializedPool)block, so the condition is always true.title={ - isUninitializedPool - ? t("pools.createSupercharged.addInitialLiquidity", { - poolNumber: pool.id, - }) - : t("addLiquidity.title") + " - Pool " + pool.id + t("pools.createSupercharged.addInitialLiquidity", { + poolNumber: pool.id, + }) }packages/web/components/complex/add-conc-liquidity.tsx (1)
88-92: Consider string comparison for zero check.Using
parseFloatworks, but ifcurrent_tick_liquidityfrom the chain is always a string representation (which is common for blockchain decimal values), a direct string comparison might be more explicit:- const isTickLiquidityZero = currentTickLiquidity - ? parseFloat(currentTickLiquidity) === 0 - : false; + const isTickLiquidityZero = currentTickLiquidity === "0";However, if the API might return
"0.0"or similar,parseFloatis more robust. Either approach works.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
packages/web/localizations/de.jsonis excluded by!**/*.jsonpackages/web/localizations/en.jsonis excluded by!**/*.jsonpackages/web/localizations/es.jsonis excluded by!**/*.jsonpackages/web/localizations/fa.jsonis excluded by!**/*.jsonpackages/web/localizations/fr.jsonis excluded by!**/*.jsonpackages/web/localizations/gu.jsonis excluded by!**/*.jsonpackages/web/localizations/hi.jsonis excluded by!**/*.jsonpackages/web/localizations/ja.jsonis excluded by!**/*.jsonpackages/web/localizations/ko.jsonis excluded by!**/*.jsonpackages/web/localizations/pl.jsonis excluded by!**/*.jsonpackages/web/localizations/pt-br.jsonis excluded by!**/*.jsonpackages/web/localizations/ro.jsonis excluded by!**/*.jsonpackages/web/localizations/ru.jsonis excluded by!**/*.jsonpackages/web/localizations/tr.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-cn.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-hk.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-tw.jsonis excluded by!**/*.json
📒 Files selected for processing (11)
packages/server/src/queries/complex/pools/index.ts(2 hunks)packages/server/src/queries/complex/pools/providers/sidecar.ts(1 hunks)packages/server/src/queries/osmosis/poolmanager/pools.ts(1 hunks)packages/stores/src/account/osmosis/index.ts(1 hunks)packages/web/components/complex/add-conc-liquidity.tsx(19 hunks)packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx(4 hunks)packages/web/components/pool-detail/concentrated.tsx(5 hunks)packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts(7 hunks)packages/web/hooks/ui-config/use-historical-and-depth-data.ts(2 hunks)packages/web/modals/add-liquidity.tsx(2 hunks)packages/web/pages/pool/[id].tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-08T14:01:51.343Z
Learnt from: JohnnyWyles
Repo: osmosis-labs/osmosis-frontend PR: 4171
File: packages/web/components/pages/asset-info-page/pools.tsx:21-0
Timestamp: 2025-10-08T14:01:51.343Z
Learning: In packages/web/components/pages/asset-info-page/pools.tsx, the poolTypesFilter intentionally excludes only "cosmwasm-transmuter" by default, not all CosmWasm pool types, because alloyed pools should remain visible to users.
Applied to files:
packages/web/hooks/ui-config/use-historical-and-depth-data.tspackages/web/pages/pool/[id].tsxpackages/server/src/queries/osmosis/poolmanager/pools.tspackages/web/components/complex/pool/create/cl/add-initial-liquidity.tsxpackages/server/src/queries/complex/pools/index.tspackages/web/components/pool-detail/concentrated.tsxpackages/server/src/queries/complex/pools/providers/sidecar.tspackages/web/hooks/ui-config/use-add-concentrated-liquidity-config.tspackages/web/modals/add-liquidity.tsxpackages/web/components/complex/add-conc-liquidity.tsxpackages/stores/src/account/osmosis/index.ts
🧬 Code graph analysis (6)
packages/web/hooks/ui-config/use-historical-and-depth-data.ts (2)
packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts (1)
pool(333-359)packages/server/src/queries/osmosis/poolmanager/pools.ts (1)
ConcentratedPoolRawResponse(75-90)
packages/web/pages/pool/[id].tsx (2)
packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts (2)
pool(333-359)error(658-674)packages/stores/src/ui-config/manage-liquidity/add-liquidity.ts (1)
error(552-579)
packages/server/src/queries/osmosis/poolmanager/pools.ts (1)
packages/server/src/queries/create-node-query.ts (1)
createNodeQuery(20-61)
packages/server/src/queries/complex/pools/index.ts (2)
packages/server/src/queries/osmosis/poolmanager/pools.ts (1)
queryPoolChain(118-122)packages/server/src/queries/complex/pools/providers/sidecar.ts (1)
makePoolFromChainPool(470-523)
packages/web/components/pool-detail/concentrated.tsx (6)
packages/server/src/queries/osmosis/poolmanager/pools.ts (1)
ConcentratedPoolRawResponse(75-90)packages/pools/src/concentrated/pool.ts (1)
currentTickLiquidity(123-125)packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts (3)
isInactivePool(362-369)pool(333-359)currentPrice(392-396)packages/web/utils/formatter.ts (1)
formatPretty(37-70)packages/web/components/chart/price-historical.tsx (1)
ChartUnavailable(428-438)packages/web/components/chart/concentrated-liquidity-depth.tsx (1)
ConcentratedLiquidityDepthChart(22-211)
packages/server/src/queries/complex/pools/providers/sidecar.ts (5)
packages/server/src/queries/osmosis/poolmanager/pools.ts (1)
PoolRawResponse(100-104)packages/types/src/asset-types.ts (1)
AssetList(6-10)packages/server/src/queries/complex/pools/index.ts (2)
Pool(37-46)PoolType(24-24)packages/pools/src/types.ts (1)
PoolType(25-31)packages/server/src/queries/complex/pools/env.ts (1)
getCosmwasmPoolTypeFromCodeId(12-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (27)
packages/web/hooks/ui-config/use-historical-and-depth-data.ts (1)
55-87: Query guards look correct once the type issue is addressed.The conditional enabling of queries for non-empty pools is a sound approach. Once the type safety issue for
isEmptyPoolis fixed, this logic will correctly prevent unnecessary API calls for empty CL pools.packages/server/src/queries/osmosis/poolmanager/pools.ts (1)
113-122: LGTM!The new
PoolResponsetype andqueryPoolChainfunction follow the established pattern ofqueryPoolsChainand correctly implement the single-pool chain query endpoint.packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (2)
104-121: LGTM!The layout change to full-width provides better responsiveness, and the
TokenLiquiditySelectorcomponents now properly fill available space.
235-240: Good accessibility improvement.Adding the
titleattribute provides a tooltip for the full denomination when text is truncated, improving UX.packages/server/src/queries/complex/pools/providers/sidecar.ts (2)
381-394: LGTM!The 6-decimal default is well-documented. The comment clearly explains the trade-off and that chain transactions remain unaffected since they use actual amounts.
469-523: Well-structured conversion function.The
makePoolFromChainPoolfunction handles pool type detection, spread factor extraction, and provides sensible defaults for unavailable data (zero TVL, no incentives, zero market metrics). This aligns well with the fallback use case.packages/web/pages/pool/[id].tsx (2)
29-33: LGTM - Error extraction from query for logging.Properly destructures
errorfrom the query result for use in the error logging effect.
79-87: LGTM - Error logging before redirect.Adding error logging before redirecting to
/poolsaids debugging for failed pool fetches. The dependency array correctly includeserror.packages/web/modals/add-liquidity.tsx (1)
96-114: LGTM - Pool state detection logic for uninitialized pools.The state detection correctly identifies uninitialized pools (zero price, zero liquidity, no TVL) and differentiates them from inactive pools which are handled by the
AddConcLiquiditycomponent.packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts (5)
174-180: LGTM - Parallel invalidation of pool and tick data.Good improvement to refresh both pool data and tick range data in parallel after adding liquidity, ensuring UI state consistency.
361-369: LGTM - Inactive pool detection.The
isInactivePoolcomputed property correctly identifies pools with TVL but no in-range liquidity (currentSqrtPrice is zero while TVL > 0).
538-588: LGTM - Defensive depositPercentages calculation.Good hardening with:
- Early guard for zero currentSqrtPrice
- Early guard for unloaded prices
- try/catch with fallback to equal percentages
This prevents runtime errors for edge cases.
866-870: LGTM - Inactive pool handling in autorun effects.The early return for inactive pools with max set correctly delegates to the specialized
calculateInactivePoolCounterpartyAmounthandler.Also applies to: 924-928
1047-1157: Verify no infinite loop when calculated amount exceeds balance.At lines 1106-1108 and 1147-1149, when the calculated counterparty amount exceeds balance, you call
setQuoteDepositAmountMax()orsetBaseDepositAmountMax(). These methods setisMaxto true and change the anchor asset, which could re-trigger the autorun effects at lines 854-910 and 915-968.Since those autoruns check
this.isInactivePool && this._baseDepositAmountIn.isMax(or quote), this could potentially create a loop:
- Base max triggers
calculateInactivePoolCounterpartyAmount("base")- If quote exceeds balance → calls
setQuoteDepositAmountMax()- Quote max triggers
calculateInactivePoolCounterpartyAmount("quote")- If base exceeds balance → calls
setBaseDepositAmountMax()→ back to step 1However, looking more closely, the balance check should eventually find a valid amount since both assets have the same dollar value target. Please verify this doesn't loop when user has very low balances of both assets.
packages/web/components/pool-detail/concentrated.tsx (3)
239-246: LGTM - Hide 24h volume for empty/inactive pools.Volume data is meaningless for pools without active liquidity, so hiding it is correct.
266-340: LGTM - Chart gating for active pools only.Charts and depth visualization are appropriately hidden for uninitialized and inactive pools since there's no meaningful data to display.
345-432: LGTM - Conditional UI for pool states.Good UX differentiation:
- Inactive pools show explanation text and "Add Liquidity" CTA
- Uninitialized pools show "Initialize Pool" CTA
- Active pools show full positions management UI
The
SuperchargePooland learn-more modal are correctly gated byisUserPositionsFetched.packages/stores/src/account/osmosis/index.ts (1)
606-618: Add error handling forfetchRemainingPoolscall.The
awaiton line 612 lacks error handling. If the fetch fails due to network issues or API errors, the function will throw without any recovery mechanism. Either wrap this in try-catch with appropriate fallback/error feedback, or ensure the calling context handles potential rejection.⛔ Skipped due to learnings
Learnt from: greg-nagy Repo: osmosis-labs/osmosis-frontend PR: 3913 File: packages/trpc/src/assets.ts:444-446 Timestamp: 2024-11-05T08:46:24.743Z Learning: In the codebase, it's acceptable for errors to propagate to the client from the `getUserDustAssets` procedure in `packages/trpc/src/assets.ts`, so adding try-catch blocks for handling potential price fetching errors is not required.Learnt from: greg-nagy Repo: osmosis-labs/osmosis-frontend PR: 3913 File: packages/trpc/src/assets.ts:444-446 Timestamp: 2024-11-05T08:46:24.743Z Learning: In `packages/trpc/src/assets.ts`, within the `getUserDustAssets` procedure, a prior filter ensures that `usdValue` is defined and valid, so additional error handling for `usdValue` may not be necessary in subsequent operations.Learnt from: yury-dubinin Repo: osmosis-labs/osmosis-frontend PR: 4012 File: packages/e2e/tests/swap.usdc.wallet.spec.ts:64-64 Timestamp: 2024-12-13T09:12:42.643Z Learning: In `packages/e2e/tests/swap.usdc.wallet.spec.ts`, it is acceptable to call `tradePage.isTransactionSuccesful()` without additional error handling.packages/web/components/complex/add-conc-liquidity.tsx (9)
83-101: Inactive pool detection logic looks correct.The derivation handles edge cases well:
currentTickLiquidityas"0"string is correctly detected viaparseFloat- Missing/undefined values safely result in
falseforisTickLiquidityZero- The
useEffectcorrectly auto-navigates inactive pools to manual viewOne minor note: the
useEffectdependency onaddLiquidityConfig(the entire observable object) works because the component is wrapped withobserver, but for clarity you could consider explicitly depending onaddLiquidityConfig.modalViewif MobX allows.
246-261: Disabled state handling is well-implemented.The
StrategySelectorcorrectly applies visual disabled styling (opacity-40,cursor-not-allowed) and nullifies the click handler whendisabledis true. The conditional class logic properly gates interactive states.
773-774: Strict boolean check is appropriate.Using
=== trueensures that only explicitly disabled cards are affected, treatingundefinedas enabled. This is the correct approach given the optional prop type.
826-833: Clear commenting on pegged currency logic.The reorganization with explicit comments improves readability. The early return for pegged currencies (where moderate = aggressive range) correctly hides redundant options.
836-846: Consistent disabled state implementation.The disabled handling mirrors the
StrategySelectorpattern with appropriate visual feedback and nullified click handlers. The implementation is consistent across components.
363-383: Back button and title handling aligns with inactive pool flow.Since inactive pools skip the overview step (auto-navigated via the
useEffecton line 96-100), hiding the back button and using a generic title is the correct UX approach.
697-730: Strategy disabling aligns with requirements.Moderate and Aggressive are correctly disabled for inactive pools via
props.isInactivePool, while Custom and Passive remain enabled. This matches the PR objectives.Note: The Custom strategy enablement conflicts with the
useEffectforcingfullRange=true(flagged in earlier comment at lines 331-336).
215-216: Managed strategy correctly disabled for inactive pools.This aligns with the PR objective to restrict managed (Magma vault) strategies for inactive pools.
83-89: No changes needed. The type assertionpool.raw as ConcentratedPoolRawResponseis safe given the preceding type guard check (pool.type === "concentrated"). ConcentratedPoolRawResponse is a well-defined exported type from the server package withcurrent_tick_liquidityas an explicit required field, and this pattern is consistently used throughout the codebase with optional chaining providing additional defensive safety.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/server/src/queries/complex/pools/index.ts (1)
108-146: Fallback logic is well-structured with proper error handling.The implementation correctly:
- Attempts Sidecar first (preferred source with market data)
- Falls back to chain query for unlisted assets
- Returns null-safe with descriptive errors when pool construction fails
One minor observation: the console.log statements work for debugging, but consider using a structured logger if available in this codebase for better production observability.
packages/server/src/queries/complex/pools/providers/sidecar.ts (1)
475-539: CL pool type detection (line 505) is unreachable dead code.Since
getBalancesFromChainPoolreturnsnullfor CL pools (line 437-439), the early return at lines 488-493 triggers before the pool type detection logic runs. The"current_sqrt_price" in chainPoolcheck at line 505 will never match because CL pools exit earlier.This is harmless but could be cleaned up for clarity. Consider moving the type detection before the null check, or adding a comment explaining why CL pools are excluded from this path.
export function makePoolFromChainPool({ chainPool, assetLists, }: { chainPool: PoolRawResponse; assetLists: AssetList[]; }): Pool | null { const pool_id = "pool_id" in chainPool ? chainPool.pool_id : chainPool.id; const balances = getBalancesFromChainPool(chainPool); const poolDenoms = getPoolDenomsFromChainPool(chainPool); - // Cannot construct valid pool without balances or denoms + // Cannot construct valid pool without balances or denoms + // Note: CL and CosmWasm pools will return null here as their balances + // require additional queries (bank module / contract state) if (balances === null || poolDenoms === null) { console.warn( `Pool ${pool_id} cannot be constructed from chain data: unsupported pool type (CL or CosmWasm)` ); return null; }packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts (1)
361-375: Consider usingDecfor consistent zero-checking.The
parseFloat(currentTickLiquidity) === 0check works, but for consistency with the codebase's use ofDecfor numeric comparisons, consider using:const isTickLiquidityZero = currentTickLiquidity ? new Dec(currentTickLiquidity).isZero() : false;This approach handles edge cases like very small non-zero values more reliably and aligns with the pattern used in line 372 for TVL checking.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/server/src/queries/complex/pools/index.ts(2 hunks)packages/server/src/queries/complex/pools/providers/sidecar.ts(1 hunks)packages/web/components/complex/add-conc-liquidity.tsx(18 hunks)packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts(7 hunks)packages/web/hooks/ui-config/use-historical-and-depth-data.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web/hooks/ui-config/use-historical-and-depth-data.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-08T14:01:51.343Z
Learnt from: JohnnyWyles
Repo: osmosis-labs/osmosis-frontend PR: 4171
File: packages/web/components/pages/asset-info-page/pools.tsx:21-0
Timestamp: 2025-10-08T14:01:51.343Z
Learning: In packages/web/components/pages/asset-info-page/pools.tsx, the poolTypesFilter intentionally excludes only "cosmwasm-transmuter" by default, not all CosmWasm pool types, because alloyed pools should remain visible to users.
Applied to files:
packages/server/src/queries/complex/pools/providers/sidecar.tspackages/web/components/complex/add-conc-liquidity.tsxpackages/server/src/queries/complex/pools/index.tspackages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts
🧬 Code graph analysis (4)
packages/server/src/queries/complex/pools/providers/sidecar.ts (5)
packages/server/src/queries/osmosis/poolmanager/pools.ts (1)
PoolRawResponse(100-104)packages/types/src/asset-types.ts (1)
AssetList(6-10)packages/server/src/queries/complex/pools/index.ts (2)
Pool(37-46)PoolType(24-24)packages/pools/src/types.ts (1)
PoolType(25-31)packages/server/src/queries/complex/pools/env.ts (1)
getCosmwasmPoolTypeFromCodeId(12-33)
packages/web/components/complex/add-conc-liquidity.tsx (3)
packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts (2)
pool(333-359)isInactivePool(362-375)packages/server/src/queries/osmosis/poolmanager/pools.ts (1)
ConcentratedPoolRawResponse(75-90)packages/pools/src/concentrated/pool.ts (1)
currentTickLiquidity(123-125)
packages/server/src/queries/complex/pools/index.ts (2)
packages/server/src/queries/osmosis/poolmanager/pools.ts (1)
queryPoolChain(118-122)packages/server/src/queries/complex/pools/providers/sidecar.ts (1)
makePoolFromChainPool(476-539)
packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts (3)
packages/server/src/queries/osmosis/poolmanager/pools.ts (1)
ConcentratedPoolRawResponse(75-90)packages/pools/src/concentrated/pool.ts (1)
currentTickLiquidity(123-125)packages/math/src/pool/concentrated/math.ts (1)
calcAmount0(256-280)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: lint-workspace
- GitHub Check: wait-for-deployment
- GitHub Check: test (22.x, web)
- GitHub Check: test (22.x, unit)
- GitHub Check: test (22.x, pools)
- GitHub Check: test (22.x, bridge)
- GitHub Check: test (22.x, server)
- GitHub Check: test (22.x, math)
- GitHub Check: test (22.x, bridge)
- GitHub Check: test (22.x, pools)
- GitHub Check: test (22.x, stores)
- GitHub Check: test (22.x, utils)
- GitHub Check: test (22.x, web)
- GitHub Check: test (22.x, tx)
- GitHub Check: test (22.x, server)
- GitHub Check: test (22.x, stores)
- GitHub Check: test (22.x, utils)
- GitHub Check: test (22.x, math)
- GitHub Check: Summary
🔇 Additional comments (15)
packages/server/src/queries/complex/pools/index.ts (1)
9-11: LGTM!The new imports properly bring in the chain query function and the pool transformation helper needed for the fallback path.
packages/server/src/queries/complex/pools/providers/sidecar.ts (4)
381-394: Clear documentation for the decimal assumption.The 6-decimal default is well-documented. This is a reasonable fallback since most Cosmos tokens use 6 decimals, and the chain handles actual amounts correctly regardless.
396-414: LGTM!The function correctly handles different pool shapes and returns
nullfor unsupported types, enabling safe fallback behavior.
416-444: LGTM! The null returns for CL and CosmWasm pools prevent broken pools from reaching the UI.This addresses the previous review concern about empty reserves crashing the frontend. The caller (
makePoolFromChainPool) properly handlesnullby returningnullitself, which causes the fallback path to throw a descriptive "not found" error.The TODO for querying bank module balances for CL pools is a reasonable future enhancement.
446-473: LGTM!The function gracefully handles unlisted assets by falling back to minimal currency objects, and safely defaults amounts to "0" when not found.
packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts (3)
174-180: LGTM! Parallel refresh improves performance.The change to use
Promise.allfor refreshing both pool data and tick data in parallel is a good optimization compared to sequential invalidation.Also applies to: 241-247
544-594: LGTM! Robust safety checks for deposit percentage calculation.The added guards for zero
currentSqrtPrice, missing prices, and the try/catch wrapper with equal-split fallback provide excellent defensive programming. This prevents potential crashes when dealing with edge cases in inactive or uninitialized pools.
872-876: LGTM! Inactive pool handling correctly gates the Max flow.The special handling for inactive pools when Max is clicked properly routes through
calculateInactivePoolCounterpartyAmountand early returns to prevent normal proportional calculations. This correctly distinguishes inactive pools (TVL > 0, liquidity = 0) from uninitialized pools (no TVL, price = 0).Also applies to: 930-934
packages/web/components/complex/add-conc-liquidity.tsx (7)
96-100: LGTM! Auto-navigation bypasses overview for inactive pools.The automatic navigation to manual view for inactive pools aligns with the PR's goal to guide users directly to the add liquidity flow when the pool is out of range.
215-215: LGTM! Managed strategy correctly disabled for inactive pools.The UI properly disables the managed strategy option for inactive pools with appropriate visual feedback (opacity, cursor) and prevents interaction by conditionally setting
onClicktoundefined.Also applies to: 246-261
327-338: LGTM! Ref-based guard addresses previous review concern.The
hasInitializedInactivePoolref ensures the passive strategy is only auto-selected once on mount for inactive pools, preventing the infinite override loop flagged in the previous review. This allows users to switch to custom strategy without being forced back to passive.Based on learnings from past review comments.
365-380: LGTM! Back button correctly hidden for inactive pools.Hiding the back button for inactive pools is consistent with the auto-navigation that bypasses the overview, preventing navigation to a view that's not relevant for inactive pool flows.
382-384: LGTM! Title appropriately adjusted for inactive pools.Using the generic "Add Liquidity" title instead of "Step 2" makes sense since inactive pools bypass the overview (step 1), improving clarity for users.
706-732: LGTM! Strategy restrictions correctly implemented for inactive pools.The implementation properly restricts moderate and aggressive strategies for inactive pools while allowing passive and custom strategies. This aligns with the PR objectives to guide users toward safe liquidity provision strategies for out-of-range pools.
Also applies to: 748-776
828-835: LGTM! Pegged currency handling hides redundant moderate strategy.The logic correctly hides the moderate strategy option when it would produce the same range as aggressive (for pegged currencies), avoiding redundant UI elements and user confusion.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/web/components/pool-detail/concentrated.tsx (1)
342-364: Consider adding descriptive text for uninitialized pools, similar to inactive pools.Inactive pools get a title and description (lines 344-353), but uninitialized pools only get a bare "Initialize Pool" button with no context. Users encountering an empty pool might benefit from a brief explanation of what initializing a pool means, similar to the inactive pool messaging.
💡 Example: add descriptive text for uninitialized pools
{isInactivePool && ( <div className="flex flex-col items-center gap-2 pb-4 text-center"> <h6 className="text-h6 font-h6"> {t("pool.inactivePool.title")} </h6> <p className="max-w-md whitespace-pre-line text-body2 text-osmoverse-200"> {t("pool.inactivePool.description")} </p> </div> )} + {isUninitializedPool && ( + <div className="flex flex-col items-center gap-2 pb-4 text-center"> + <h6 className="text-h6 font-h6"> + {t("pool.uninitializedPool.title")} + </h6> + <p className="max-w-md whitespace-pre-line text-body2 text-osmoverse-200"> + {t("pool.uninitializedPool.description")} + </p> + </div> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/components/pool-detail/concentrated.tsx` around lines 342 - 364, The UI shows explanatory text for isInactivePool but not for isUninitializedPool, causing the Initialize Pool button (rendered in the same conditional block with Button and setActiveModal("add-liquidity")) to appear without context; update the conditional render around isUninitializedPool/isInactivePool to include a similar title and description when isUninitializedPool is true (use the same structure as the inactive block: a div with heading and paragraph using t(...) strings, e.g., t("pool.uninitializedPool.title") and t("pool.uninitializedPool.description")), keeping the existing Button and click handler intact so users get descriptive text before they click initialize.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server/src/queries/complex/pools/providers/sidecar.ts`:
- Around line 475-556: makePoolFromChainPool currently sets totalFiatValueLocked
to zero for concentrated pools that had synthesized zero balances, which
prevents downstream inactive-detection from distinguishing "unknown" TVL from
actual zero; change the function to detect synthesized balances (when balances
=== null && "current_sqrt_price" in chainPool) and: 1) add an explicit flag on
the returned Pool (e.g., tvlUnknown or balancesSynthed) and set it true for that
case, and 2) avoid returning a misleading PricePretty(0) for
totalFiatValueLocked (set it to undefined/null or a distinct sentinel) so
consumers can treat unknown TVL differently; update references to
effectiveBalances, reserveCoins, and totalFiatValueLocked in
makePoolFromChainPool accordingly.
In `@packages/web/components/pool-detail/concentrated.tsx`:
- Around line 148-174: The code incorrectly references poolData (undefined) when
computing pool state flags; replace uses of poolData with the existing
chartConfig variable pool (e.g., use pool?.type, cast (pool.raw as
ConcentratedPoolRawResponse) for current_sqrt_price and current_tick_liquidity,
and use pool.totalFiatValueLocked for hasTVL), keeping the same null-safe checks
(?.) and parseFloat logic so isSqrtPriceZero, isTickLiquidityZero,
isInactivePool and isUninitializedPool evaluate correctly.
---
Nitpick comments:
In `@packages/web/components/pool-detail/concentrated.tsx`:
- Around line 342-364: The UI shows explanatory text for isInactivePool but not
for isUninitializedPool, causing the Initialize Pool button (rendered in the
same conditional block with Button and setActiveModal("add-liquidity")) to
appear without context; update the conditional render around
isUninitializedPool/isInactivePool to include a similar title and description
when isUninitializedPool is true (use the same structure as the inactive block:
a div with heading and paragraph using t(...) strings, e.g.,
t("pool.uninitializedPool.title") and t("pool.uninitializedPool.description")),
keeping the existing Button and click handler intact so users get descriptive
text before they click initialize.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/server/src/queries/complex/pools/providers/sidecar.tspackages/web/components/pool-detail/concentrated.tsxpackages/web/hooks/ui-config/use-historical-and-depth-data.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web/hooks/ui-config/use-historical-and-depth-data.ts
| /** Converts a chain pool response to Pool type, handling unlisted assets. */ | ||
| export function makePoolFromChainPool({ | ||
| chainPool, | ||
| assetLists, | ||
| }: { | ||
| chainPool: PoolRawResponse; | ||
| assetLists: AssetList[]; | ||
| }): Pool | null { | ||
| const pool_id = "pool_id" in chainPool ? chainPool.pool_id : chainPool.id; | ||
| const balances = getBalancesFromChainPool(chainPool); | ||
| const poolDenoms = getPoolDenomsFromChainPool(chainPool); | ||
|
|
||
| // Cannot construct valid pool without denoms at all | ||
| if (poolDenoms === null) { | ||
| console.warn( | ||
| `Pool ${pool_id} cannot be constructed from chain data: missing pool denoms (unsupported pool type or malformed data)` | ||
| ); | ||
| return null; | ||
| } | ||
|
|
||
| // For concentrated liquidity pools, balances may not be directly available from the chain | ||
| // response. In that case, synthesize zero balances for each denom so we can still construct | ||
| // a minimal Pool object for the chain fallback path. | ||
| const effectiveBalances = | ||
| balances ?? | ||
| ("current_sqrt_price" in chainPool | ||
| ? poolDenoms.map((denom) => ({ denom, amount: "0" })) | ||
| : null); | ||
|
|
||
| // For non-concentrated pools, we still require actual balances. | ||
| if (effectiveBalances === null) { | ||
| console.warn( | ||
| `Pool ${pool_id} cannot be constructed from chain data: missing balances (unsupported pool type or incomplete data)` | ||
| ); | ||
| return null; | ||
| } | ||
|
|
||
| const reserveCoins = getReservesFromChainPool( | ||
| assetLists, | ||
| chainPool, | ||
| effectiveBalances | ||
| ); | ||
|
|
||
| // Get pool type | ||
| let poolType: PoolType; | ||
| if ("pool_assets" in chainPool) poolType = "weighted"; | ||
| else if ("scaling_factors" in chainPool) poolType = "stable"; | ||
| else if ("current_sqrt_price" in chainPool) poolType = "concentrated"; | ||
| else if ("code_id" in chainPool) { | ||
| poolType = getCosmwasmPoolTypeFromCodeId(chainPool.code_id.toString()); | ||
| } else { | ||
| throw new Error("Unknown pool type: " + JSON.stringify(chainPool)); | ||
| } | ||
|
|
||
| // Get spread factor | ||
| let spreadFactor = "0"; | ||
| if ("spread_factor" in chainPool) { | ||
| spreadFactor = chainPool.spread_factor; | ||
| } else if ("pool_params" in chainPool && chainPool.pool_params?.swap_fee) { | ||
| spreadFactor = chainPool.pool_params.swap_fee; | ||
| } | ||
|
|
||
| return { | ||
| id: pool_id.toString(), | ||
| type: poolType, | ||
| raw: chainPool, | ||
| spreadFactor: new RatePretty(spreadFactor), | ||
| reserveCoins, | ||
| totalFiatValueLocked: new PricePretty(DEFAULT_VS_CURRENCY, 0), // Unknown for chain-only pools | ||
| // No incentives or market data available from chain | ||
| incentives: { | ||
| aprBreakdown: undefined, | ||
| incentiveTypes: ["none"], | ||
| }, | ||
| market: { | ||
| volume24hUsd: new PricePretty(DEFAULT_VS_CURRENCY, 0), | ||
| volume7dUsd: new PricePretty(DEFAULT_VS_CURRENCY, 0), | ||
| feesSpent24hUsd: new PricePretty(DEFAULT_VS_CURRENCY, 0), | ||
| feesSpent7dUsd: new PricePretty(DEFAULT_VS_CURRENCY, 0), | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Chain-fallback CL pools can be misclassified as non-inactive due to TVL=0.
totalFiatValueLocked is always set to 0 and CL balances are synthesized as zero, so downstream “inactive pool” detection that requires TVL > 0 will never trigger for out‑of‑range CL pools fetched via the chain fallback. Consider surfacing an explicit “TVL unknown / balances missing” flag (or computing TVL when possible) so UI logic can distinguish “unknown” from “zero.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/queries/complex/pools/providers/sidecar.ts` around lines
475 - 556, makePoolFromChainPool currently sets totalFiatValueLocked to zero for
concentrated pools that had synthesized zero balances, which prevents downstream
inactive-detection from distinguishing "unknown" TVL from actual zero; change
the function to detect synthesized balances (when balances === null &&
"current_sqrt_price" in chainPool) and: 1) add an explicit flag on the returned
Pool (e.g., tvlUnknown or balancesSynthed) and set it true for that case, and 2)
avoid returning a misleading PricePretty(0) for totalFiatValueLocked (set it to
undefined/null or a distinct sentinel) so consumers can treat unknown TVL
differently; update references to effectiveBalances, reserveCoins, and
totalFiatValueLocked in makePoolFromChainPool accordingly.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts
Outdated
Show resolved
Hide resolved
|
Bugbot Autofix prepared fixes for 3 of the 3 bugs found in the latest run.
Or push these changes by commenting: Preview (15c706fe37)diff --git a/packages/web/components/pool-detail/concentrated.tsx b/packages/web/components/pool-detail/concentrated.tsx
--- a/packages/web/components/pool-detail/concentrated.tsx
+++ b/packages/web/components/pool-detail/concentrated.tsx
@@ -1,4 +1,3 @@
-import type { ConcentratedPoolRawResponse } from "@osmosis-labs/server";
import { Dec } from "@osmosis-labs/unit";
import classNames from "classnames";
import { observer } from "mobx-react-lite";
@@ -32,6 +31,7 @@
import { ConcentratedLiquidityLearnMoreModal } from "~/modals/concentrated-liquidity-intro";
import { useStore } from "~/stores";
import { formatPretty, getPriceExtendedFormatOptions } from "~/utils/formatter";
+import { getConcentratedPoolState } from "~/utils/pool-state";
import { api } from "~/utils/trpc";
import { removeQueryParam } from "~/utils/url";
@@ -145,34 +145,10 @@
[currentPrice]
);
- // Pool state detection based on liquidity
- const poolRaw =
- poolData?.type === "concentrated"
- ? (poolData.raw as ConcentratedPoolRawResponse)
- : null;
- const currentSqrtPrice = poolRaw?.current_sqrt_price;
- const currentTickLiquidity = poolRaw?.current_tick_liquidity;
- const hasTVL = poolData && !poolData.totalFiatValueLocked.toDec().isZero();
+ // Pool state detection using shared utility
+ const { isUninitialized: isUninitializedPool, isInactive: isInactivePool } =
+ getConcentratedPoolState(pool);
- // Check if values are zero (handles both "0" and "0.000000..." strings)
- const isSqrtPriceZero = currentSqrtPrice
- ? parseFloat(currentSqrtPrice) === 0
- : false;
- const isTickLiquidityZero = currentTickLiquidity
- ? parseFloat(currentTickLiquidity) === 0
- : false;
-
- // Tier 2: Inactive Pool - has TVL but no in range liquidity at current price
- // This happens when all liquidity positions are out of range
- // Check this FIRST because a pool can have out-of-range liquidity (TVL > 0)
- // with zero tick liquidity and zero sqrt price
- const isInactivePool = isTickLiquidityZero && hasTVL;
-
- // Tier 1: Uninitialized Pool - has never been initialized (no price set)
- // Only classify as uninitialized if there's NO TVL at all
- const isUninitializedPool =
- isSqrtPriceZero && isTickLiquidityZero && !hasTVL;
-
return (
<main className="m-auto flex min-h-screen max-w-container flex-col gap-8 px-8 py-4 md:gap-4 md:p-4">
{pool && activeModal === "add-liquidity" && (
diff --git a/packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts b/packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts
--- a/packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts
+++ b/packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts
@@ -365,16 +365,22 @@
get isInactivePool(): boolean {
if (!this._pool || this._pool.type !== "concentrated") return false;
- // Inactive pool: has TVL but no in-range liquidity at current price
+ // Inactive pool: has been initialized but has no in-range liquidity
// This happens when all liquidity positions are out of range
+ // Note: We use !isSqrtPriceZero instead of hasTVL because chain-fallback pools
+ // always report TVL as 0 even when they have out-of-range liquidity
const poolRaw = this._pool.raw as ConcentratedPoolRawResponse;
+ const currentSqrtPrice = poolRaw?.current_sqrt_price;
const currentTickLiquidity = poolRaw?.current_tick_liquidity;
+
+ const isSqrtPriceZero = currentSqrtPrice
+ ? parseFloat(currentSqrtPrice) === 0
+ : true;
const isTickLiquidityZero = currentTickLiquidity
? parseFloat(currentTickLiquidity) === 0
- : false;
- const hasTVL = this._pool.totalFiatValueLocked.toDec().gt(new Dec(0));
+ : true;
- return isTickLiquidityZero && hasTVL;
+ return isTickLiquidityZero && !isSqrtPriceZero;
}
get sender(): string {
@@ -1057,6 +1063,32 @@
protected calculateInactivePoolCounterpartyAmount(
anchorAsset: "base" | "quote"
) {
+ const queryAccountBalances = this.queryBalances.getQueryBech32Address(
+ this.sender
+ );
+
+ // Guard check: if already adjusting, cap to balance to prevent infinite loop
+ // This handles the case where both base and quote exceed their balances
+ if (this._isAdjustingForBalance) {
+ if (anchorAsset === "base") {
+ const quoteBalance = queryAccountBalances.getBalanceFromCurrency(
+ this._quoteDepositAmountIn.sendCurrency
+ );
+ this._quoteDepositAmountIn.setAmount(
+ quoteBalance.trim(true).hideDenom(true).locale(false).toString()
+ );
+ } else {
+ const baseBalance = queryAccountBalances.getBalanceFromCurrency(
+ this._baseDepositAmountIn.sendCurrency
+ );
+ this._baseDepositAmountIn.setAmount(
+ baseBalance.trim(true).hideDenom(true).locale(false).toString()
+ );
+ }
+ this._isAdjustingForBalance = false;
+ return;
+ }
+
// Safety check: ensure prices are loaded and valid
if (!this._baseDepositPrice || !this._quoteDepositPrice) {
console.warn("Prices not loaded for inactive pool calculation");
@@ -1074,121 +1106,96 @@
return;
}
- const queryAccountBalances = this.queryBalances.getQueryBech32Address(
- this.sender
- );
+ if (anchorAsset === "base") {
+ // Base is max, calculate quote to match dollar value
+ const baseAmountRaw =
+ this.baseDepositAmountIn.getAmountPrimitive().amount;
+ const baseCoin = new CoinPretty(
+ this._baseDepositAmountIn.sendCurrency,
+ new Int(baseAmountRaw)
+ );
- // Set guard to prevent infinite loop when both balances are insufficient
- const wasAdjusting = this._isAdjustingForBalance;
- this._isAdjustingForBalance = true;
+ // Calculate dollar value of base amount
+ const baseValue = this._baseDepositPrice.mul(baseCoin);
- try {
- if (anchorAsset === "base") {
- // Base is max, calculate quote to match dollar value
- const baseAmountRaw =
- this.baseDepositAmountIn.getAmountPrimitive().amount;
- const baseCoin = new CoinPretty(
- this._baseDepositAmountIn.sendCurrency,
- new Int(baseAmountRaw)
- );
+ // Calculate quote amount with same dollar value: quoteAmount = baseValue / quotePrice
+ const quoteAmount = baseValue
+ .toDec()
+ .quo(this._quoteDepositPrice.toDec());
- // Calculate dollar value of base amount
- const baseValue = this._baseDepositPrice.mul(baseCoin);
-
- // Calculate quote amount with same dollar value: quoteAmount = baseValue / quotePrice
- const quoteAmount = baseValue
- .toDec()
- .quo(this._quoteDepositPrice.toDec());
-
- // Convert to quote currency decimals
- const quoteCoin = new CoinPretty(
- this._quoteDepositAmountIn.sendCurrency,
- quoteAmount
- .mul(
- DecUtils.getTenExponentN(
- this._quoteDepositAmountIn.sendCurrency.coinDecimals
- )
+ // Convert to quote currency decimals
+ const quoteCoin = new CoinPretty(
+ this._quoteDepositAmountIn.sendCurrency,
+ quoteAmount
+ .mul(
+ DecUtils.getTenExponentN(
+ this._quoteDepositAmountIn.sendCurrency.coinDecimals
)
- .truncate()
- );
+ )
+ .truncate()
+ );
- // Check if quote amount exceeds balance
- const quoteBalance = queryAccountBalances.getBalanceFromCurrency(
- this._quoteDepositAmountIn.sendCurrency
- );
+ // Check if quote amount exceeds balance
+ const quoteBalance = queryAccountBalances.getBalanceFromCurrency(
+ this._quoteDepositAmountIn.sendCurrency
+ );
- if (quoteCoin.toDec().gt(quoteBalance.toDec())) {
- // Calculated amount exceeds balance
- if (!wasAdjusting) {
- // First adjustment - flip to quote max and recalculate base
- this.setQuoteDepositAmountMax();
- } else {
- // Already adjusting - cap to balance to prevent infinite loop
- this._quoteDepositAmountIn.setAmount(
- quoteBalance.trim(true).hideDenom(true).locale(false).toString()
- );
- }
- } else {
- // Set the calculated quote amount
- this._quoteDepositAmountIn.setAmount(
- quoteCoin.hideDenom(true).locale(false).trim(true).toString()
- );
- }
+ if (quoteCoin.toDec().gt(quoteBalance.toDec())) {
+ // Quote exceeds balance - set guard and flip to quote max
+ // The guard will be seen by the next reaction and break the loop
+ this._isAdjustingForBalance = true;
+ this.setQuoteDepositAmountMax();
} else {
- // Quote is max, calculate base to match dollar value
- const quoteAmountRaw =
- this.quoteDepositAmountIn.getAmountPrimitive().amount;
- const quoteCoin = new CoinPretty(
- this._quoteDepositAmountIn.sendCurrency,
- new Int(quoteAmountRaw)
+ // Set the calculated quote amount (no flip needed)
+ this._quoteDepositAmountIn.setAmount(
+ quoteCoin.hideDenom(true).locale(false).trim(true).toString()
);
+ }
+ } else {
+ // Quote is max, calculate base to match dollar value
+ const quoteAmountRaw =
+ this.quoteDepositAmountIn.getAmountPrimitive().amount;
+ const quoteCoin = new CoinPretty(
+ this._quoteDepositAmountIn.sendCurrency,
+ new Int(quoteAmountRaw)
+ );
- // Calculate dollar value of quote amount
- const quoteValue = this._quoteDepositPrice.mul(quoteCoin);
+ // Calculate dollar value of quote amount
+ const quoteValue = this._quoteDepositPrice.mul(quoteCoin);
- // Calculate base amount with same dollar value: baseAmount = quoteValue / basePrice
- const baseAmount = quoteValue
- .toDec()
- .quo(this._baseDepositPrice.toDec());
+ // Calculate base amount with same dollar value: baseAmount = quoteValue / basePrice
+ const baseAmount = quoteValue
+ .toDec()
+ .quo(this._baseDepositPrice.toDec());
- // Convert to base currency decimals
- const baseCoin = new CoinPretty(
- this._baseDepositAmountIn.sendCurrency,
- baseAmount
- .mul(
- DecUtils.getTenExponentN(
- this._baseDepositAmountIn.sendCurrency.coinDecimals
- )
+ // Convert to base currency decimals
+ const baseCoin = new CoinPretty(
+ this._baseDepositAmountIn.sendCurrency,
+ baseAmount
+ .mul(
+ DecUtils.getTenExponentN(
+ this._baseDepositAmountIn.sendCurrency.coinDecimals
)
- .truncate()
- );
+ )
+ .truncate()
+ );
- // Check if base amount exceeds balance
- const baseBalance = queryAccountBalances.getBalanceFromCurrency(
- this._baseDepositAmountIn.sendCurrency
+ // Check if base amount exceeds balance
+ const baseBalance = queryAccountBalances.getBalanceFromCurrency(
+ this._baseDepositAmountIn.sendCurrency
+ );
+
+ if (baseCoin.toDec().gt(baseBalance.toDec())) {
+ // Base exceeds balance - set guard and flip to base max
+ // The guard will be seen by the next reaction and break the loop
+ this._isAdjustingForBalance = true;
+ this.setBaseDepositAmountMax();
+ } else {
+ // Set the calculated base amount (no flip needed)
+ this._baseDepositAmountIn.setAmount(
+ baseCoin.hideDenom(true).locale(false).trim(true).toString()
);
-
- if (baseCoin.toDec().gt(baseBalance.toDec())) {
- // Calculated amount exceeds balance
- if (!wasAdjusting) {
- // First adjustment - flip to base max and recalculate quote
- this.setBaseDepositAmountMax();
- } else {
- // Already adjusting - cap to balance to prevent infinite loop
- this._baseDepositAmountIn.setAmount(
- baseBalance.trim(true).hideDenom(true).locale(false).toString()
- );
- }
- } else {
- // Set the calculated base amount
- this._baseDepositAmountIn.setAmount(
- baseCoin.hideDenom(true).locale(false).trim(true).toString()
- );
- }
}
- } finally {
- // Reset guard flag
- this._isAdjustingForBalance = wasAdjusting;
}
}
diff --git a/packages/web/modals/add-liquidity.tsx b/packages/web/modals/add-liquidity.tsx
--- a/packages/web/modals/add-liquidity.tsx
+++ b/packages/web/modals/add-liquidity.tsx
@@ -2,7 +2,6 @@
NotInitializedError,
ObservableAddLiquidityConfig,
} from "@osmosis-labs/stores";
-import type { ConcentratedPoolRawResponse } from "@osmosis-labs/server";
import { observer } from "mobx-react-lite";
import { useState } from "react";
import { FunctionComponent } from "react";
@@ -20,6 +19,7 @@
} from "~/hooks";
import { ModalBase, ModalBaseProps } from "~/modals/base";
import { useStore } from "~/stores";
+import { getConcentratedPoolState } from "~/utils/pool-state";
import { api } from "~/utils/trpc";
import { SuperfluidValidatorModal } from "./superfluid-validator";
@@ -93,26 +93,10 @@
// add concentrated liquidity
if (pool?.type === "concentrated") {
- // Pool state detection based on liquidity
- const poolRaw = pool.raw as ConcentratedPoolRawResponse;
- const currentSqrtPrice = poolRaw?.current_sqrt_price;
- const currentTickLiquidity = poolRaw?.current_tick_liquidity;
- const hasTVL = !pool.totalFiatValueLocked.toDec().isZero();
+ // Pool state detection using shared utility
+ const { isUninitialized: isUninitializedPool } =
+ getConcentratedPoolState(pool);
- // Check if values are zero (handles both "0" and "0.000000..." strings)
- const isSqrtPriceZero = currentSqrtPrice
- ? parseFloat(currentSqrtPrice) === 0
- : false;
- const isTickLiquidityZero = currentTickLiquidity
- ? parseFloat(currentTickLiquidity) === 0
- : false;
-
- // Tier 1: Uninitialized Pool - has never been initialized (no price set)
- // Only classify as uninitialized if there's NO TVL at all
- // Note: Inactive pools (TVL > 0 but zero tick liquidity) are handled inside AddConcLiquidity component
- const isUninitializedPool =
- isSqrtPriceZero && isTickLiquidityZero && !hasTVL;
-
// For uninitialized pools (but NOT inactive pools), show the initial liquidity addition interface
// Inactive pools already have out-of-range liquidity, so they should use the normal add liquidity flow
if (isUninitializedPool) {
diff --git a/packages/web/utils/pool-state.ts b/packages/web/utils/pool-state.ts
new file mode 100644
--- /dev/null
+++ b/packages/web/utils/pool-state.ts
@@ -1,0 +1,49 @@
+import type { ConcentratedPoolRawResponse, Pool } from "@osmosis-labs/server";
+
+/**
+ * Determines the state of a concentrated liquidity pool.
+ * Centralizes pool state detection logic to ensure consistency across the app.
+ *
+ * @param pool - The pool to check (can be null/undefined)
+ * @returns Object with isUninitialized and isInactive flags
+ *
+ * Pool states:
+ * - Uninitialized: Pool has never been initialized (no price set, sqrt_price = 0)
+ * - Inactive: Pool has been initialized but has no in-range liquidity (sqrt_price != 0, tick_liquidity = 0)
+ * - Active: Pool has in-range liquidity (sqrt_price != 0, tick_liquidity != 0)
+ *
+ * Note: This detection does NOT rely on TVL because chain-fallback pools
+ * (pools with unlisted assets) report TVL as 0 even when they have liquidity.
+ * Instead, we use current_sqrt_price to determine if a pool has been initialized.
+ */
+export function getConcentratedPoolState(pool: Pool | null | undefined): {
+ isUninitialized: boolean;
+ isInactive: boolean;
+} {
+ if (!pool || pool.type !== "concentrated") {
+ return { isUninitialized: false, isInactive: false };
+ }
+
+ const poolRaw = pool.raw as ConcentratedPoolRawResponse;
+ const currentSqrtPrice = poolRaw?.current_sqrt_price;
+ const currentTickLiquidity = poolRaw?.current_tick_liquidity;
+
+ const isSqrtPriceZero = currentSqrtPrice
+ ? parseFloat(currentSqrtPrice) === 0
+ : true;
+ const isTickLiquidityZero = currentTickLiquidity
+ ? parseFloat(currentTickLiquidity) === 0
+ : true;
+
+ // Pool is uninitialized if it has no price set (sqrt_price = 0)
+ // This implies the pool has never had liquidity added
+ const isUninitialized = isSqrtPriceZero && isTickLiquidityZero;
+
+ // Pool is inactive if it has a price set but no in-range liquidity
+ // This happens when all liquidity positions are out of range
+ // Note: We use !isSqrtPriceZero instead of hasTVL because chain-fallback pools
+ // always report TVL as 0 even when they have out-of-range liquidity
+ const isInactive = isTickLiquidityZero && !isSqrtPriceZero;
+
+ return { isUninitialized, isInactive };
+} |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/web/modals/add-liquidity.tsx (1)
96-115: Duplicated pool state detection — same pattern asconcentrated.tsxand the config hook.As noted in the
concentrated.tsxreview, this is the third instance of the same detection logic. A shared helper would keep all three in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/modals/add-liquidity.tsx` around lines 96 - 115, Extract the duplicated pool state detection logic (the computation of currentSqrtPrice/currentTickLiquidity parsing, hasTVL check, and final isUninitializedPool determination) into a shared helper (e.g., a function named detectUninitializedPool or getPoolState) and replace the inline logic in add-liquidity.tsx (references: currentSqrtPrice, currentTickLiquidity, hasTVL, isUninitializedPool) with a call to that helper; ensure the helper accepts the pool/raw response types used here (ConcentratedPoolRawResponse and pool.tvlUnknown/totalFiatValueLocked) and returns the same boolean(s) so concentrated.tsx and the config hook can call it to keep logic consistent.packages/web/components/complex/add-conc-liquidity.tsx (1)
293-329:⚠️ Potential issue | 🟠 MajorAuto-passive guard can still override Custom when fullRange starts true.
IffullRangeis already true on the first render,hasInitializedInactivePoolnever flips, so selecting Custom later can be reverted back to full range. Consider marking initialization as soon as an inactive pool is detected, and only forcing full range once.Suggested fix
// Default to passive strategy for inactive pools (only on mount) useEffect(() => { - if (isInactivePool && !fullRange && !hasInitializedInactivePool.current) { - setFullRange(true); - hasInitializedInactivePool.current = true; - } + if (!isInactivePool || hasInitializedInactivePool.current) return; + hasInitializedInactivePool.current = true; + if (!fullRange) setFullRange(true); }, [isInactivePool, fullRange, setFullRange]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/components/complex/add-conc-liquidity.tsx` around lines 293 - 329, The useEffect currently only flips hasInitializedInactivePool when fullRange is false, which lets a first-render fullRange=true inactive pool later get overridden; update the effect for isInactivePool to mark initialization immediately by setting hasInitializedInactivePool.current = true as soon as isInactivePool is detected, and only call setFullRange(true) conditionally when fullRange is false—i.e., in the effect watching isInactivePool/fullRange, ensure hasInitializedInactivePool is set before (or regardless of) calling setFullRange so the auto-passive guard runs only once; locate the effect using isInactivePool, fullRange, hasInitializedInactivePool, and setFullRange to apply this change.
🧹 Nitpick comments (9)
packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (1)
56-87:accountis a duplicate ofwallet.
wallet(line 56) andaccount(line 87) are identical expressions — both callaccountStore.getWallet(accountStore.osmosisChainId). Onlywalletis used for the balance queries; onlyaccountis used for the TX call on line 165. One of them is redundant.♻️ Proposed cleanup
- const wallet = accountStore.getWallet(accountStore.osmosisChainId); + const account = accountStore.getWallet(accountStore.osmosisChainId); const { data: quoteUsdValue } = api.edge.assets.getAssetPrice.useQuery( { coinMinimalDenom: selectedQuote?.token.coinMinimalDenom ?? "", }, { enabled: !!selectedQuote?.token.coinMinimalDenom } ); const { data: baseAssetBalanceData, isLoading: isLoadingBaseAssetBalanceData, } = api.edge.assets.getUserAsset.useQuery( { - userOsmoAddress: wallet?.address ?? "", + userOsmoAddress: account?.address ?? "", findMinDenomOrSymbol: selectedBase?.token.coinMinimalDenom ?? "", }, - { enabled: !!wallet?.address } + { enabled: !!account?.address } ); const { data: quoteAssetBalanceData, isLoading: isLoadingQuoteAssetBalanceData, } = api.edge.assets.getUserAsset.useQuery( { - userOsmoAddress: wallet?.address ?? "", + userOsmoAddress: account?.address ?? "", findMinDenomOrSymbol: selectedQuote?.token.coinMinimalDenom ?? "", }, - { enabled: !!wallet?.address } + { enabled: !!account?.address } ); - const account = accountStore.getWallet(accountStore.osmosisChainId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx` around lines 56 - 87, The code defines both wallet and account by calling accountStore.getWallet(accountStore.osmosisChainId) twice; remove the duplicate by keeping a single variable (preferably wallet) and update all references (the getUserAsset queries that use wallet and the TX call that currently uses account) to use that single variable; ensure you remove the unused declaration (account or wallet) and adjust any related enabled checks or usages (e.g., wallet?.address) so all logic consistently references the chosen identifier.packages/web/modals/add-liquidity.tsx (1)
152-158: Dead code in title ternary — always takes theisUninitializedPoolbranch.This block is inside
if (isUninitializedPool)(line 119), so the ternary at line 153 will always evaluate the truthy branch. The else branch (t("addLiquidity.title") + " - Pool " + pool.id) is unreachable.🧹 Simplify
title={ - isUninitializedPool - ? t("pools.createSupercharged.addInitialLiquidity", { - poolNumber: pool.id, - }) - : t("addLiquidity.title") + " - Pool " + pool.id + t("pools.createSupercharged.addInitialLiquidity", { + poolNumber: pool.id, + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/modals/add-liquidity.tsx` around lines 152 - 158, The title prop currently uses a ternary on isUninitializedPool but this code is already inside an if (isUninitializedPool) block so the else branch is dead; simplify by replacing the ternary with the always-true branch value: call t("pools.createSupercharged.addInitialLiquidity", { poolNumber: pool.id }) directly for the title prop (referencing isUninitializedPool, title prop, t(), and pool.id to locate the change) and remove the unreachable t("addLiquidity.title") + " - Pool " + pool.id branch.packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts (1)
174-180: MissingapiUtils.local.pools.getPoolin theuseCallbackdependency array.
apiUtils.local.pools.getPool.invalidateis used inside the callback but not listed in the dependency array (lines 195–210). The same applies toincreaseLiquidity(lines 260–269). While tRPC'suseUtils()returns stable references in practice, this is technically an incomplete dependency list.🔧 Suggested fix for addLiquidity deps
[ poolId, account?.osmosis, apiUtils.local.concentratedLiquidity.getLiquidityPerTickRange, + apiUtils.local.pools.getPool, config.baseDepositAmountIn.sendCurrency,Apply the same addition to the
increaseLiquiditydependency array.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts` around lines 174 - 180, The useCallback for adding liquidity is missing apiUtils.local.pools.getPool.invalidate in its dependency array (and the increaseLiquidity callback likewise), so update the dependency arrays for the addLiquidity and increaseLiquidity callbacks in use-add-concentrated-liquidity-config.ts to include apiUtils.local.pools.getPool.invalidate (and ensure increaseLiquidity itself is listed where used as a dependency); this keeps dependencies complete for the callbacks that call apiUtils.local.pools.getPool.invalidate and use increaseLiquidity.packages/server/src/queries/complex/pools/index.ts (1)
111-149: Chain fallback looks solid; consider reducing log verbosity.The fallback logic is well-structured: try Sidecar → fallback to chain → convert via
makePoolFromChainPool→ propagate errors. The null checks at lines 126 and 136 are good.Minor note: the
console.logat lines 121-123 and 142-144 will fire on every request for unlisted/missing pools. For production, consider usingconsole.debugor gating behind a verbose flag to avoid noisy logs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/queries/complex/pools/index.ts` around lines 111 - 149, The current fallback uses console.log/console.error for run-of-the-mill messages which will be noisy in production; replace the console.log calls that announce "Pool ... not found in Sidecar..." and "Successfully retrieved pool ..." with a lower-verbosity logger method (e.g., console.debug or the app's processLogger.debug/info depending on existing conventions) and keep the error path using processLogger.error (or console.error) but include context; update the code around getPools, queryPoolChain, and makePoolFromChainPool usage so the verbose messages are either debug-level or gated behind a verbose flag/config option.packages/web/components/pool-detail/concentrated.tsx (1)
151-179: Pool state detection logic is duplicated across three files.This exact same pattern (parsing
current_sqrt_price,current_tick_liquidity, checkinghasTVL, derivingisInactivePool/isUninitializedPool) is repeated in:
concentrated.tsx(here, lines 151–179)add-liquidity.tsx(lines 96–115)use-add-concentrated-liquidity-config.ts(isInactivePoolgetter, lines 362–378)Consider extracting a shared utility (e.g.,
getConcentratedPoolState(pool)) that returns{ isInactivePool, isUninitializedPool }to keep these in sync and reduce maintenance burden.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/components/pool-detail/concentrated.tsx` around lines 151 - 179, Extract the duplicated detection logic into a shared utility function, e.g., getConcentratedPoolState(poolData), that reads poolData.type === "concentrated", parses current_sqrt_price and current_tick_liquidity, computes hasTVL, and returns { isInactivePool, isUninitializedPool } (based on the existing predicates using current_sqrt_price, current_tick_liquidity, and hasTVL). Replace the inline logic in concentrated.tsx (the currentSqrtPrice/currentTickLiquidity/isInactivePool/isUninitializedPool block), add-liquidity.tsx, and the isInactivePool getter in use-add-concentrated-liquidity-config.ts to call this utility and use the returned booleans instead, keeping the same semantics and handling of zero parsing.packages/web/components/pool-detail/share.tsx (1)
682-688: UseDEFAULT_VS_CURRENCYfrom@osmosis-labs/serverinstead of hardcoding the fallback object.The hardcoded
{ currency: "usd", symbol: "$", maxDecimals: 2, locale: "en-US" }exactly matches theDEFAULT_VS_CURRENCYconstant already exported from@osmosis-labs/server. Since the file already imports from that package, importing and using this constant would eliminate duplication and keep the code in sync with the shared definition used throughout the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/components/pool-detail/share.tsx` around lines 682 - 688, Replace the hardcoded fiat fallback object with the shared constant DEFAULT_VS_CURRENCY: import DEFAULT_VS_CURRENCY from '@osmosis-labs/server' (or add it to the existing import) and use it in the expression that currently reads pool.totalFiatValueLocked?.fiatCurrency ?? { currency: "usd", symbol: "$", maxDecimals: 2, locale: "en-US" } so it becomes pool.totalFiatValueLocked?.fiatCurrency ?? DEFAULT_VS_CURRENCY; ensure the import name matches the exported symbol and remove the hardcoded object.packages/server/src/queries/complex/pools/providers/sidecar.ts (3)
387-394: Hardcoded 6 decimals may misrepresent token amounts for non-standard assets.The comment documents this well, but be aware that tokens like WBTC (8 decimals), ETH-based tokens (18 decimals), or some stablecoins with non-standard decimals will display incorrect human-readable amounts. Since this is a fallback for unlisted assets only, the impact is limited — but downstream consumers that perform price calculations or display balances should be aware of this approximation.
Consider logging a warning when this fallback is used, so it's easier to diagnose display issues in production:
🔍 Suggested diagnostic aid
function makeUnlistedAssetCurrency(denom: string) { + console.warn( + `Using default 6-decimal currency for unlisted asset: ${denom}` + ); return { coinDenom: denom, coinMinimalDenom: denom, coinDecimals: 6, coinImageUrl: undefined, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/queries/complex/pools/providers/sidecar.ts` around lines 387 - 394, The fallback in makeUnlistedAssetCurrency hardcodes coinDecimals to 6 which can misrepresent non-standard tokens; update makeUnlistedAssetCurrency to attempt retrieving real decimals (e.g., from existing denom metadata lookup) and, if that lookup fails, emit a warning/log entry indicating the fallback was used and include the denom string so downstream debugging is easier; ensure the function (makeUnlistedAssetCurrency) either accepts an optional decimals parameter or performs the lookup before returning so callers can get correct decimals or see the diagnostic when the default 6 is applied.
460-472: Broad catch silently swallows all errors, not just "asset not found".The
catchon line 467 will suppress any error fromgetAsset, including unexpected failures (e.g., malformed asset list data, network errors ifgetAssetever becomes async). Consider narrowing the catch or at least logging a debug-level message to aid troubleshooting.🔍 Suggested improvement
try { const asset = getAsset({ assetLists, anyDenom: denom }); return new CoinPretty(asset, amount ?? "0"); - } catch { + } catch (e) { // Asset not in list, create minimal currency const currency = makeUnlistedAssetCurrency(denom); return new CoinPretty(currency, amount ?? "0"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/queries/complex/pools/providers/sidecar.ts` around lines 460 - 472, The current broad catch around getAsset in the poolDenoms map swallows all errors; change it so only the "asset not found" case falls back to makeUnlistedAssetCurrency while other errors are surfaced (or at least debug-logged). Specifically, update the block that calls getAsset({ assetLists, anyDenom: denom }) (used with CoinPretty) to either check presence first (e.g., via a safe lookup on assetLists) or catch errors and inspect the error (or an AssetNotFound sentinel) — if it indicates "not found" return new CoinPretty(makeUnlistedAssetCurrency(denom), amount ?? "0"), otherwise log a debug/error with the caught error and rethrow it so unexpected failures in getAsset are not silently swallowed.
522-531: Optional: Pool type detection logic is duplicated.Lines 523-531 re-implement the same structural checks as
getPoolTypeFromChainPool(lines 289-299). The only difference is the input type (PoolRawResponsevsSidecarPool["chain_model"]). Since both rely on the same property-existence checks ("pool_assets","scaling_factors","current_sqrt_price","code_id"), consider extracting a shared helper that accepts a broader type (e.g.,Record<string, unknown>) to eliminate this duplication.♻️ Example refactor
+/** Derives pool type from structural shape. Works with both SidecarPool chain_model and PoolRawResponse. */ +function derivePoolType(model: Record<string, unknown>): PoolType { + if ("pool_assets" in model) return "weighted"; + if ("scaling_factors" in model) return "stable"; + if ("current_sqrt_price" in model) return "concentrated"; + if ("code_id" in model) { + return getCosmwasmPoolTypeFromCodeId(String(model.code_id)); + } + throw new Error("Unknown pool type: " + JSON.stringify(model)); +} // In getPoolTypeFromChainPool: export function getPoolTypeFromChainPool( chain_model: SidecarPool["chain_model"] ): PoolType { - if ("pool_assets" in chain_model) return "weighted"; - if ("scaling_factors" in chain_model) return "stable"; - if ("current_sqrt_price" in chain_model) return "concentrated"; - if ("code_id" in chain_model) { - return getCosmwasmPoolTypeFromCodeId(chain_model.code_id.toString()); - } - throw new Error("Unknown pool type: " + JSON.stringify(chain_model)); + return derivePoolType(chain_model as Record<string, unknown>); } // In makePoolFromChainPool, replace lines 523-531 with: - let poolType: PoolType; - if ("pool_assets" in chainPool) poolType = "weighted"; - else if ("scaling_factors" in chainPool) poolType = "stable"; - else if ("current_sqrt_price" in chainPool) poolType = "concentrated"; - else if ("code_id" in chainPool) { - poolType = getCosmwasmPoolTypeFromCodeId(chainPool.code_id.toString()); - } else { - throw new Error("Unknown pool type: " + JSON.stringify(chainPool)); - } + const poolType = derivePoolType(chainPool as Record<string, unknown>);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/queries/complex/pools/providers/sidecar.ts` around lines 522 - 531, Duplicate pool-type detection in the sidecar code should be replaced by a shared helper: extract the property-existence logic into a single function (e.g., getPoolTypeFromRecord(record: Record<string, unknown>) or generalize getPoolTypeFromChainPool to accept Record<string, unknown>), implement the same checks for "pool_assets", "scaling_factors", "current_sqrt_price", and "code_id" and mapping via getCosmwasmPoolTypeFromCodeId, then replace the local block that sets poolType (the if/else chain referencing chainPool) with a call to that helper (passing chainPool) so both callers (the existing getPoolTypeFromChainPool and this sidecar assignment) share the same implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stores/src/account/osmosis/index.ts`:
- Around line 611-614: Wrap the queries.queryPools.fetchRemainingPools({
minLiquidity: 0 }) call in a try-catch so network errors don’t bubble up; call
fetchRemainingPools inside a try, optionally log the error (e.g.,
processLogger.error or console.error) including the error and poolId, and then
proceed to re-read queryPool = queries.queryPools.getPool(poolId) so the
function will fall back to the existing "Pool not found" behavior rather than
propagating the network rejection.
In `@packages/web/components/complex/add-conc-liquidity.tsx`:
- Around line 83-91: The effect reads addLiquidityConfig.modalView but doesn't
list it as a dependency, so extract modalView (e.g., const modalView =
addLiquidityConfig.modalView) and include it in the useEffect dependency array
alongside isInactivePool and addLiquidityConfig; then update the effect to check
if (isInactivePool && modalView === "overview") and call
addLiquidityConfig.setModalView("add_manual") so the auto-navigation re-runs
whenever modalView changes.
---
Duplicate comments:
In `@packages/web/components/complex/add-conc-liquidity.tsx`:
- Around line 293-329: The useEffect currently only flips
hasInitializedInactivePool when fullRange is false, which lets a first-render
fullRange=true inactive pool later get overridden; update the effect for
isInactivePool to mark initialization immediately by setting
hasInitializedInactivePool.current = true as soon as isInactivePool is detected,
and only call setFullRange(true) conditionally when fullRange is false—i.e., in
the effect watching isInactivePool/fullRange, ensure hasInitializedInactivePool
is set before (or regardless of) calling setFullRange so the auto-passive guard
runs only once; locate the effect using isInactivePool, fullRange,
hasInitializedInactivePool, and setFullRange to apply this change.
In `@packages/web/modals/add-liquidity.tsx`:
- Around line 96-115: Extract the duplicated pool state detection logic (the
computation of currentSqrtPrice/currentTickLiquidity parsing, hasTVL check, and
final isUninitializedPool determination) into a shared helper (e.g., a function
named detectUninitializedPool or getPoolState) and replace the inline logic in
add-liquidity.tsx (references: currentSqrtPrice, currentTickLiquidity, hasTVL,
isUninitializedPool) with a call to that helper; ensure the helper accepts the
pool/raw response types used here (ConcentratedPoolRawResponse and
pool.tvlUnknown/totalFiatValueLocked) and returns the same boolean(s) so
concentrated.tsx and the config hook can call it to keep logic consistent.
---
Nitpick comments:
In `@packages/server/src/queries/complex/pools/index.ts`:
- Around line 111-149: The current fallback uses console.log/console.error for
run-of-the-mill messages which will be noisy in production; replace the
console.log calls that announce "Pool ... not found in Sidecar..." and
"Successfully retrieved pool ..." with a lower-verbosity logger method (e.g.,
console.debug or the app's processLogger.debug/info depending on existing
conventions) and keep the error path using processLogger.error (or
console.error) but include context; update the code around getPools,
queryPoolChain, and makePoolFromChainPool usage so the verbose messages are
either debug-level or gated behind a verbose flag/config option.
In `@packages/server/src/queries/complex/pools/providers/sidecar.ts`:
- Around line 387-394: The fallback in makeUnlistedAssetCurrency hardcodes
coinDecimals to 6 which can misrepresent non-standard tokens; update
makeUnlistedAssetCurrency to attempt retrieving real decimals (e.g., from
existing denom metadata lookup) and, if that lookup fails, emit a warning/log
entry indicating the fallback was used and include the denom string so
downstream debugging is easier; ensure the function (makeUnlistedAssetCurrency)
either accepts an optional decimals parameter or performs the lookup before
returning so callers can get correct decimals or see the diagnostic when the
default 6 is applied.
- Around line 460-472: The current broad catch around getAsset in the poolDenoms
map swallows all errors; change it so only the "asset not found" case falls back
to makeUnlistedAssetCurrency while other errors are surfaced (or at least
debug-logged). Specifically, update the block that calls getAsset({ assetLists,
anyDenom: denom }) (used with CoinPretty) to either check presence first (e.g.,
via a safe lookup on assetLists) or catch errors and inspect the error (or an
AssetNotFound sentinel) — if it indicates "not found" return new
CoinPretty(makeUnlistedAssetCurrency(denom), amount ?? "0"), otherwise log a
debug/error with the caught error and rethrow it so unexpected failures in
getAsset are not silently swallowed.
- Around line 522-531: Duplicate pool-type detection in the sidecar code should
be replaced by a shared helper: extract the property-existence logic into a
single function (e.g., getPoolTypeFromRecord(record: Record<string, unknown>) or
generalize getPoolTypeFromChainPool to accept Record<string, unknown>),
implement the same checks for "pool_assets", "scaling_factors",
"current_sqrt_price", and "code_id" and mapping via
getCosmwasmPoolTypeFromCodeId, then replace the local block that sets poolType
(the if/else chain referencing chainPool) with a call to that helper (passing
chainPool) so both callers (the existing getPoolTypeFromChainPool and this
sidecar assignment) share the same implementation.
In `@packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx`:
- Around line 56-87: The code defines both wallet and account by calling
accountStore.getWallet(accountStore.osmosisChainId) twice; remove the duplicate
by keeping a single variable (preferably wallet) and update all references (the
getUserAsset queries that use wallet and the TX call that currently uses
account) to use that single variable; ensure you remove the unused declaration
(account or wallet) and adjust any related enabled checks or usages (e.g.,
wallet?.address) so all logic consistently references the chosen identifier.
In `@packages/web/components/pool-detail/concentrated.tsx`:
- Around line 151-179: Extract the duplicated detection logic into a shared
utility function, e.g., getConcentratedPoolState(poolData), that reads
poolData.type === "concentrated", parses current_sqrt_price and
current_tick_liquidity, computes hasTVL, and returns { isInactivePool,
isUninitializedPool } (based on the existing predicates using
current_sqrt_price, current_tick_liquidity, and hasTVL). Replace the inline
logic in concentrated.tsx (the
currentSqrtPrice/currentTickLiquidity/isInactivePool/isUninitializedPool block),
add-liquidity.tsx, and the isInactivePool getter in
use-add-concentrated-liquidity-config.ts to call this utility and use the
returned booleans instead, keeping the same semantics and handling of zero
parsing.
In `@packages/web/components/pool-detail/share.tsx`:
- Around line 682-688: Replace the hardcoded fiat fallback object with the
shared constant DEFAULT_VS_CURRENCY: import DEFAULT_VS_CURRENCY from
'@osmosis-labs/server' (or add it to the existing import) and use it in the
expression that currently reads pool.totalFiatValueLocked?.fiatCurrency ?? {
currency: "usd", symbol: "$", maxDecimals: 2, locale: "en-US" } so it becomes
pool.totalFiatValueLocked?.fiatCurrency ?? DEFAULT_VS_CURRENCY; ensure the
import name matches the exported symbol and remove the hardcoded object.
In `@packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts`:
- Around line 174-180: The useCallback for adding liquidity is missing
apiUtils.local.pools.getPool.invalidate in its dependency array (and the
increaseLiquidity callback likewise), so update the dependency arrays for the
addLiquidity and increaseLiquidity callbacks in
use-add-concentrated-liquidity-config.ts to include
apiUtils.local.pools.getPool.invalidate (and ensure increaseLiquidity itself is
listed where used as a dependency); this keeps dependencies complete for the
callbacks that call apiUtils.local.pools.getPool.invalidate and use
increaseLiquidity.
In `@packages/web/modals/add-liquidity.tsx`:
- Around line 152-158: The title prop currently uses a ternary on
isUninitializedPool but this code is already inside an if (isUninitializedPool)
block so the else branch is dead; simplify by replacing the ternary with the
always-true branch value: call t("pools.createSupercharged.addInitialLiquidity",
{ poolNumber: pool.id }) directly for the title prop (referencing
isUninitializedPool, title prop, t(), and pool.id to locate the change) and
remove the unreachable t("addLiquidity.title") + " - Pool " + pool.id branch.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
packages/web/localizations/de.jsonis excluded by!**/*.jsonpackages/web/localizations/en.jsonis excluded by!**/*.jsonpackages/web/localizations/es.jsonis excluded by!**/*.jsonpackages/web/localizations/fa.jsonis excluded by!**/*.jsonpackages/web/localizations/fr.jsonis excluded by!**/*.jsonpackages/web/localizations/gu.jsonis excluded by!**/*.jsonpackages/web/localizations/hi.jsonis excluded by!**/*.jsonpackages/web/localizations/ja.jsonis excluded by!**/*.jsonpackages/web/localizations/ko.jsonis excluded by!**/*.jsonpackages/web/localizations/pl.jsonis excluded by!**/*.jsonpackages/web/localizations/pt-br.jsonis excluded by!**/*.jsonpackages/web/localizations/ro.jsonis excluded by!**/*.jsonpackages/web/localizations/ru.jsonis excluded by!**/*.jsonpackages/web/localizations/tr.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-cn.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-hk.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-tw.jsonis excluded by!**/*.json
📒 Files selected for processing (13)
packages/server/src/queries/complex/pools/index.tspackages/server/src/queries/complex/pools/providers/sidecar.tspackages/server/src/queries/complex/pools/user.tspackages/server/src/queries/osmosis/poolmanager/pools.tspackages/stores/src/account/osmosis/index.tspackages/web/components/complex/add-conc-liquidity.tsxpackages/web/components/complex/pool/create/cl/add-initial-liquidity.tsxpackages/web/components/pool-detail/concentrated.tsxpackages/web/components/pool-detail/share.tsxpackages/web/hooks/ui-config/use-add-concentrated-liquidity-config.tspackages/web/hooks/ui-config/use-historical-and-depth-data.tspackages/web/modals/add-liquidity.tsxpackages/web/pages/pool/[id].tsx
| // Use inactive pool detection from config (has TVL but no in-range liquidity) | ||
| const isInactivePool = addLiquidityConfig.isInactivePool; | ||
|
|
||
| // Auto-navigate to manual view for inactive pools | ||
| useEffect(() => { | ||
| if (isInactivePool && addLiquidityConfig.modalView === "overview") { | ||
| addLiquidityConfig.setModalView("add_manual"); | ||
| } | ||
| }, [isInactivePool, addLiquidityConfig]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/web/components/complex/add-conc-liquidity.tsx | sed -n '70,100p'Repository: osmosis-labs/osmosis-frontend
Length of output: 1406
🏁 Script executed:
# Check the type/definition of addLiquidityConfig
rg -A 5 "addLiquidityConfig" packages/web/components/complex/add-conc-liquidity.tsx | head -40Repository: osmosis-labs/osmosis-frontend
Length of output: 1469
🏁 Script executed:
# Search for addLiquidityConfig hook/store definition
rg "export.*addLiquidityConfig|const.*addLiquidityConfig" packages/web --type tsx --type ts | head -20Repository: osmosis-labs/osmosis-frontend
Length of output: 100
🏁 Script executed:
rg "ObservableAddConcentratedLiquidityConfig" packages/web --type ts --type tsx -A 10 | head -60Repository: osmosis-labs/osmosis-frontend
Length of output: 100
🏁 Script executed:
# Also check if there are other similar useEffect patterns in the codebase
rg "useEffect.*\[.*addLiquidityConfig" packages/web --type ts --type tsx -B 2 -A 5Repository: osmosis-labs/osmosis-frontend
Length of output: 100
🏁 Script executed:
rg "ObservableAddConcentratedLiquidityConfig" packages/web -A 15 | head -80Repository: osmosis-labs/osmosis-frontend
Length of output: 6695
🏁 Script executed:
# Search for the type definition with broader search
rg "ObservableAddConcentratedLiquidityConfig\s*=" packages/webRepository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
# Look at the actual structure - check imports and class/interface definitions
rg "class ObservableAddConcentratedLiquidityConfig|interface ObservableAddConcentratedLiquidityConfig" packages/web -A 30Repository: osmosis-labs/osmosis-frontend
Length of output: 2986
🏁 Script executed:
# Check how modalView property is used and understand if it's observable
rg "modalView" packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts -B 2 -A 5Repository: osmosis-labs/osmosis-frontend
Length of output: 768
🏁 Script executed:
# Look for other useEffect patterns with observer components and observable objects
rg "useEffect.*observer\|observer.*useEffect" packages/web -A 10 -B 2 | head -50Repository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
# Check if there are other examples of extracting observable properties in dependency arrays
rg "const.*=.*config\." packages/web/components/complex/add-conc-liquidity.tsx -B 1 -A 1 | head -30Repository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
# Check if addLiquidityConfig is a stable reference (memoized or created once)
rg "useAddConcentratedLiquidityConfig\|new ObservableAddConcentratedLiquidityConfig" packages/web -B 3 -A 8 | head -80Repository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
# Look at the useEffect hook definition to confirm how dependencies work with MobX observers
rg "useEffect.*\[.*config\." packages/web -B 2 -A 5 | head -60Repository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
# Check if there's a pattern of how modal navigation works elsewhere
rg "setModalView\|modalView.*===" packages/web/components/complex/add-conc-liquidity.tsx -B 2 -A 2Repository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
# Verify the exact behavior: does addLiquidityConfig reference change or just its properties?
rg "addLiquidityConfig.*=" packages/web/components/complex/add-conc-liquidity.tsx | head -20Repository: osmosis-labs/osmosis-frontend
Length of output: 998
🏁 Script executed:
# Check if there are similar patterns in other components/hooks using observer
rg "observer.*useEffect.*@observable" packages/web -l | head -5Repository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
# Check if there's a wrapper component or if this is called within observer directly
rg -B 20 "const isInactivePool = addLiquidityConfig.isInactivePool" packages/web/components/complex/add-conc-liquidity.tsx | head -30Repository: osmosis-labs/osmosis-frontend
Length of output: 809
🏁 Script executed:
# Understand the control flow: when would modalView reset back to "overview"?
rg "setModalView.*overview\|modalView.*overview" packages/web/components/complex/add-conc-liquidity.tsx -B 2 -A 2Repository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
# Check if there's any logic that resets modalView back to overview after this effect sets it
rg "setModalView\(.*overview" packages/web -B 5 -A 2 | head -40Repository: osmosis-labs/osmosis-frontend
Length of output: 2298
🏁 Script executed:
# Check the Overview component to see if it changes modalView
rg "class Overview\|function Overview\|const Overview" packages/web/components/complex/add-conc-liquidity.tsx -A 50 | head -60Repository: osmosis-labs/osmosis-frontend
Length of output: 55
Include modalView in useEffect dependency array.
When the user is on an inactive pool and navigates away from the manual view (back to "overview" via the back button or managed strategy redirect), the effect won't re-run to re-trigger the auto-navigation because modalView is read but not declared as a dependency. Extract modalView and include it alongside isInactivePool to ensure the effect reliably re-runs whenever the modal state changes.
Suggested fix
+ const modalView = addLiquidityConfig.modalView;
// Auto-navigate to manual view for inactive pools
useEffect(() => {
- if (isInactivePool && addLiquidityConfig.modalView === "overview") {
+ if (isInactivePool && modalView === "overview") {
addLiquidityConfig.setModalView("add_manual");
}
- }, [isInactivePool, addLiquidityConfig]);
+ }, [isInactivePool, modalView, addLiquidityConfig]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Use inactive pool detection from config (has TVL but no in-range liquidity) | |
| const isInactivePool = addLiquidityConfig.isInactivePool; | |
| // Auto-navigate to manual view for inactive pools | |
| useEffect(() => { | |
| if (isInactivePool && addLiquidityConfig.modalView === "overview") { | |
| addLiquidityConfig.setModalView("add_manual"); | |
| } | |
| }, [isInactivePool, addLiquidityConfig]); | |
| // Use inactive pool detection from config (has TVL but no in-range liquidity) | |
| const isInactivePool = addLiquidityConfig.isInactivePool; | |
| const modalView = addLiquidityConfig.modalView; | |
| // Auto-navigate to manual view for inactive pools | |
| useEffect(() => { | |
| if (isInactivePool && modalView === "overview") { | |
| addLiquidityConfig.setModalView("add_manual"); | |
| } | |
| }, [isInactivePool, modalView, addLiquidityConfig]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/components/complex/add-conc-liquidity.tsx` around lines 83 - 91,
The effect reads addLiquidityConfig.modalView but doesn't list it as a
dependency, so extract modalView (e.g., const modalView =
addLiquidityConfig.modalView) and include it in the useEffect dependency array
alongside isInactivePool and addLiquidityConfig; then update the effect to check
if (isInactivePool && modalView === "overview") and call
addLiquidityConfig.setModalView("add_manual") so the auto-navigation re-runs
whenever modalView changes.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web/modals/add-liquidity.tsx (1)
1-1:⚠️ Potential issue | 🟡 MinorFix Prettier formatting to unblock CI.
The pipeline reports a Prettier formatting violation for this file. Run
prettier --writeon this file to resolve the lint check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/modals/add-liquidity.tsx` at line 1, Run Prettier to fix formatting violations in add-liquidity.tsx; the CI failure is due to style only. Either run prettier --write on this file or reformat the imports/JSX to match the project's Prettier/ESLint config (e.g., use the project's Prettier config and apply to the AddLiquidity component and its import block). After formatting, stage the updated file and push to unblock the lint CI.
🧹 Nitpick comments (2)
packages/web/modals/add-liquidity.tsx (1)
148-159: Redundant ternary — the else branch on Line 158 is dead code.Since this block is inside
if (isUninitializedPool)(Line 120), theisUninitializedPoolcondition in thetitleternary is alwaystrue. The else branch (t("addLiquidity.title") + " - Pool " + pool.id) can never execute.♻️ Simplify to remove dead branch
title={ - isUninitializedPool - ? t("pools.createSupercharged.addInitialLiquidity", { - poolNumber: pool.id, - }) - : t("addLiquidity.title") + " - Pool " + pool.id + t("pools.createSupercharged.addInitialLiquidity", { + poolNumber: pool.id, + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/modals/add-liquidity.tsx` around lines 148 - 159, The title prop passed to ModalBase contains a redundant ternary because the surrounding conditional guarantees isUninitializedPool is true; replace the ternary in the title prop (inside ModalBase) with only the true branch value — t("pools.createSupercharged.addInitialLiquidity", { poolNumber: pool.id }) — removing the dead else branch (the expression t("addLiquidity.title") + " - Pool " + pool.id) to simplify the code.packages/server/src/queries/complex/pools/providers/sidecar.ts (1)
382-394: Consider resolving decimals for unlisted assets instead of defaulting to 6.Defaulting to 6 decimals can mis-scale balances and ratios for tokens with different precision. If feasible, consider pulling denom metadata (or at least flagging “decimals unknown”) so downstream calculations don’t treat the values as authoritative.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/queries/complex/pools/providers/sidecar.ts` around lines 382 - 394, The fallback in makeUnlistedAssetCurrency currently hardcodes coinDecimals = 6 which can mis-scale assets; update makeUnlistedAssetCurrency to attempt resolving decimals (e.g., call existing denom metadata resolver such as getDenomMetadata or query bank/denomMetadata for denom) and set coinDecimals to the resolved value when available, and when not available set coinDecimals to undefined and include a flag like decimalsUnknown: true (or coinDecimalsKnown: false) so downstream callers (functions that consume makeUnlistedAssetCurrency) can treat decimals as non-authoritative and handle undefined/unknown cases appropriately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/trpc/src/__tests_e2e__/swap-router.spec.ts`:
- Around line 385-387: The test currently uses a non-null assertion on
mediumVolumePool.totalFiatValueLocked when computing desiredPrice, which can
throw for inactive/uninitialized pools; update the test to first ensure the pool
has a defined TVL (and optionally !tvlUnknown) before using it—either by
selecting a pool with totalFiatValueLocked defined when constructing
mediumVolumePool or by asserting/guarding (e.g., if
(!mediumVolumePool.totalFiatValueLocked || mediumVolumePool.tvlUnknown) throw
new Error(...)) prior to computing desiredPrice; apply the same guard/fix to the
other occurrence around the lines referencing totalFiatValueLocked (the second
test at the other location).
In `@packages/web/components/pool-detail/concentrated.tsx`:
- Around line 151-181: The current hasTVL treats tvlUnknown as true which causes
pools with current_sqrt_price/current_tick_liquidity both "0" to be marked
inactive; update the logic so "confirmed TVL" is distinguished from
"tvlUnknown": change hasTVL to require either totalFiatValueLocked is present
and non-zero OR (tvlUnknown === false and any other confirmed indicator), or
simpler—keep hasTVL for confirmed TVL only (e.g., const hasConfirmedTVL =
poolData && poolData.tvlUnknown === false &&
poolData.totalFiatValueLocked?.toDec().isZero() === false) and then use
hasConfirmedTVL in isInactivePool (leave isUninitializedPool condition unchanged
using tvlUnknown truthiness), or alternatively modify isInactivePool to require
!poolData.tvlUnknown in addition to isTickLiquidityZero so uninitialized
(tvlUnknown true + zero price/liquidity) are classified as uninitialized via
isUninitializedPool.
In `@packages/web/modals/add-liquidity.tsx`:
- Around line 120-145: The code destructures pool.reserveCoins into baseCoin and
quoteCoin without ensuring there are two elements, which can lead to missing
selectedBase/selectedQuote and a broken AddInitialLiquidity UI; update the
isUninitializedPool branch in add-liquidity.tsx to first validate that
pool.reserveCoins has at least two entries (or that both baseCoin and quoteCoin
are defined) and if not either return early (render a fallback/error state) or
set explicit fallback SelectionToken values, then only construct selectedBase
and selectedQuote when both coins exist; reference the isUninitializedPool
check, pool.reserveCoins, selectedBase, selectedQuote and the component that
renders AddInitialLiquidity to locate where to add the guard and fallback.
---
Outside diff comments:
In `@packages/web/modals/add-liquidity.tsx`:
- Line 1: Run Prettier to fix formatting violations in add-liquidity.tsx; the CI
failure is due to style only. Either run prettier --write on this file or
reformat the imports/JSX to match the project's Prettier/ESLint config (e.g.,
use the project's Prettier config and apply to the AddLiquidity component and
its import block). After formatting, stage the updated file and push to unblock
the lint CI.
---
Nitpick comments:
In `@packages/server/src/queries/complex/pools/providers/sidecar.ts`:
- Around line 382-394: The fallback in makeUnlistedAssetCurrency currently
hardcodes coinDecimals = 6 which can mis-scale assets; update
makeUnlistedAssetCurrency to attempt resolving decimals (e.g., call existing
denom metadata resolver such as getDenomMetadata or query bank/denomMetadata for
denom) and set coinDecimals to the resolved value when available, and when not
available set coinDecimals to undefined and include a flag like decimalsUnknown:
true (or coinDecimalsKnown: false) so downstream callers (functions that consume
makeUnlistedAssetCurrency) can treat decimals as non-authoritative and handle
undefined/unknown cases appropriately.
In `@packages/web/modals/add-liquidity.tsx`:
- Around line 148-159: The title prop passed to ModalBase contains a redundant
ternary because the surrounding conditional guarantees isUninitializedPool is
true; replace the ternary in the title prop (inside ModalBase) with only the
true branch value — t("pools.createSupercharged.addInitialLiquidity", {
poolNumber: pool.id }) — removing the dead else branch (the expression
t("addLiquidity.title") + " - Pool " + pool.id) to simplify the code.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/server/src/queries/complex/pools/providers/sidecar.tspackages/trpc/src/__tests_e2e__/swap-router.spec.tspackages/web/components/pool-detail/concentrated.tsxpackages/web/modals/add-liquidity.tsx
| // Pool state detection based on liquidity | ||
| const poolRaw = | ||
| poolData?.type === "concentrated" | ||
| ? (poolData.raw as ConcentratedPoolRawResponse) | ||
| : null; | ||
| const currentSqrtPrice = poolRaw?.current_sqrt_price; | ||
| const currentTickLiquidity = poolRaw?.current_tick_liquidity; | ||
| const hasTVL = | ||
| poolData && | ||
| (poolData.tvlUnknown || | ||
| poolData.totalFiatValueLocked?.toDec().isZero() === false); | ||
|
|
||
| // Check if values are zero (handles both "0" and "0.000000..." strings) | ||
| const isSqrtPriceZero = currentSqrtPrice | ||
| ? parseFloat(currentSqrtPrice) === 0 | ||
| : false; | ||
| const isTickLiquidityZero = currentTickLiquidity | ||
| ? parseFloat(currentTickLiquidity) === 0 | ||
| : false; | ||
|
|
||
| // Tier 2: Inactive Pool - has TVL but no in range liquidity at current price | ||
| // This happens when all liquidity positions are out of range | ||
| // Check this FIRST because a pool can have out-of-range liquidity (TVL > 0) | ||
| // with zero tick liquidity and zero sqrt price | ||
| const isInactivePool = isTickLiquidityZero && hasTVL; | ||
|
|
||
| // Tier 1: Uninitialized Pool - has never been initialized (no price set) | ||
| // Only classify as uninitialized if there's NO TVL at all | ||
| const isUninitializedPool = | ||
| isSqrtPriceZero && isTickLiquidityZero && !hasTVL; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "tvlUnknown" -C 3Repository: osmosis-labs/osmosis-frontend
Length of output: 4619
🏁 Script executed:
rg -n "balancesSynthed" -C 5Repository: osmosis-labs/osmosis-frontend
Length of output: 3385
🏁 Script executed:
rg -n "ConcentratedPoolRawResponse" -A 15 | head -50Repository: osmosis-labs/osmosis-frontend
Length of output: 3943
🏁 Script executed:
rg -n "isUninitializedPool\|isInactivePool" -B 5 -A 5Repository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
cat -n packages/web/components/pool-detail/concentrated.tsx | sed -n '140,200p'Repository: osmosis-labs/osmosis-frontend
Length of output: 2996
🏁 Script executed:
fd -e "test\|spec" --type f | xargs rg -l "uninitialized\|Uninitialized" | head -5Repository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
rg -n "current_sqrt_price.*0\|uninitialized" --type ts --type tsx -C 3Repository: osmosis-labs/osmosis-frontend
Length of output: 100
🏁 Script executed:
rg -n "current_sqrt_price.*0\|uninitialized" -C 3Repository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
cat -n packages/server/src/queries/complex/pools/providers/sidecar.ts | sed -n '480,570p'Repository: osmosis-labs/osmosis-frontend
Length of output: 3843
🏁 Script executed:
rg -n "getBalancesFromChainPool\|balances.*null" -B 3 -A 3 | head -60Repository: osmosis-labs/osmosis-frontend
Length of output: 55
🏁 Script executed:
rg -n "getBalancesFromChainPool" -B 2 -A 15 | head -80Repository: osmosis-labs/osmosis-frontend
Length of output: 3897
🏁 Script executed:
rg -n "balances === null" -B 10 -A 5Repository: osmosis-labs/osmosis-frontend
Length of output: 1859
🏁 Script executed:
rg -n "sidecar\|fallback" -C 2 packages/server/src/queries/complex/pools/providers/ | grep -A 5 -B 5 "concentrated"Repository: osmosis-labs/osmosis-frontend
Length of output: 55
Address potential mislabeling of uninitialized pools as inactive when TVL data is unavailable.
Concentrated pools fetched from chain fallback (without sidecar data) have tvlUnknown=true and synthesized zero balances. The current logic treats tvlUnknown as "has TVL", causing pools with current_sqrt_price="0" and current_tick_liquidity="0" to be classified as inactive (line 175) instead of uninitialized (line 180), since isUninitializedPool requires !hasTVL.
Consider distinguishing between "TVL unknown" and "confirmed TVL exists"—either by separating the conditions or gating tvlUnknown when both price and liquidity are zero, so uninitialized pools aren't mislabeled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/components/pool-detail/concentrated.tsx` around lines 151 - 181,
The current hasTVL treats tvlUnknown as true which causes pools with
current_sqrt_price/current_tick_liquidity both "0" to be marked inactive; update
the logic so "confirmed TVL" is distinguished from "tvlUnknown": change hasTVL
to require either totalFiatValueLocked is present and non-zero OR (tvlUnknown
=== false and any other confirmed indicator), or simpler—keep hasTVL for
confirmed TVL only (e.g., const hasConfirmedTVL = poolData &&
poolData.tvlUnknown === false && poolData.totalFiatValueLocked?.toDec().isZero()
=== false) and then use hasConfirmedTVL in isInactivePool (leave
isUninitializedPool condition unchanged using tvlUnknown truthiness), or
alternatively modify isInactivePool to require !poolData.tvlUnknown in addition
to isTickLiquidityZero so uninitialized (tvlUnknown true + zero price/liquidity)
are classified as uninitialized via isUninitializedPool.
| if (isUninitializedPool) { | ||
| const [baseCoin, quoteCoin] = pool.reserveCoins; | ||
|
|
||
| const selectedBase: SelectionToken | undefined = baseCoin?.currency | ||
| ? { | ||
| token: { | ||
| coinDenom: baseCoin.currency.coinDenom, | ||
| coinDecimals: baseCoin.currency.coinDecimals, | ||
| coinMinimalDenom: baseCoin.currency.coinMinimalDenom, | ||
| coinImageUrl: baseCoin.currency.coinImageUrl, | ||
| }, | ||
| chainName: "osmosis", // Default to osmosis chain | ||
| } | ||
| : undefined; | ||
|
|
||
| const selectedQuote: SelectionToken | undefined = quoteCoin?.currency | ||
| ? { | ||
| token: { | ||
| coinDenom: quoteCoin.currency.coinDenom, | ||
| coinDecimals: quoteCoin.currency.coinDecimals, | ||
| coinMinimalDenom: quoteCoin.currency.coinMinimalDenom, | ||
| coinImageUrl: quoteCoin.currency.coinImageUrl, | ||
| }, | ||
| chainName: "osmosis", // Default to osmosis chain | ||
| } | ||
| : undefined; |
There was a problem hiding this comment.
Potential runtime error if reserveCoins has fewer than 2 elements.
Line 121 destructures pool.reserveCoins expecting two items. While concentrated pools should always have exactly two reserve coins (base/quote), if this invariant is ever violated (e.g., an uninitialized pool with no reserve data), baseCoin or quoteCoin would be undefined. The downstream code at Lines 123 and 135 does guard against this with ?.currency checks, so this won't crash — but AddInitialLiquidity would receive undefined for both selectedBase and selectedQuote, which may produce a broken UI.
Consider adding an early return or fallback when either coin is missing.
🛡️ Suggested guard
const [baseCoin, quoteCoin] = pool.reserveCoins;
+
+ if (!baseCoin?.currency || !quoteCoin?.currency) {
+ // Pool reserve coins not available — cannot initialize
+ return null;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (isUninitializedPool) { | |
| const [baseCoin, quoteCoin] = pool.reserveCoins; | |
| const selectedBase: SelectionToken | undefined = baseCoin?.currency | |
| ? { | |
| token: { | |
| coinDenom: baseCoin.currency.coinDenom, | |
| coinDecimals: baseCoin.currency.coinDecimals, | |
| coinMinimalDenom: baseCoin.currency.coinMinimalDenom, | |
| coinImageUrl: baseCoin.currency.coinImageUrl, | |
| }, | |
| chainName: "osmosis", // Default to osmosis chain | |
| } | |
| : undefined; | |
| const selectedQuote: SelectionToken | undefined = quoteCoin?.currency | |
| ? { | |
| token: { | |
| coinDenom: quoteCoin.currency.coinDenom, | |
| coinDecimals: quoteCoin.currency.coinDecimals, | |
| coinMinimalDenom: quoteCoin.currency.coinMinimalDenom, | |
| coinImageUrl: quoteCoin.currency.coinImageUrl, | |
| }, | |
| chainName: "osmosis", // Default to osmosis chain | |
| } | |
| : undefined; | |
| if (isUninitializedPool) { | |
| const [baseCoin, quoteCoin] = pool.reserveCoins; | |
| if (!baseCoin?.currency || !quoteCoin?.currency) { | |
| // Pool reserve coins not available — cannot initialize | |
| return null; | |
| } | |
| const selectedBase: SelectionToken | undefined = baseCoin?.currency | |
| ? { | |
| token: { | |
| coinDenom: baseCoin.currency.coinDenom, | |
| coinDecimals: baseCoin.currency.coinDecimals, | |
| coinMinimalDenom: baseCoin.currency.coinMinimalDenom, | |
| coinImageUrl: baseCoin.currency.coinImageUrl, | |
| }, | |
| chainName: "osmosis", // Default to osmosis chain | |
| } | |
| : undefined; | |
| const selectedQuote: SelectionToken | undefined = quoteCoin?.currency | |
| ? { | |
| token: { | |
| coinDenom: quoteCoin.currency.coinDenom, | |
| coinDecimals: quoteCoin.currency.coinDecimals, | |
| coinMinimalDenom: quoteCoin.currency.coinMinimalDenom, | |
| coinImageUrl: quoteCoin.currency.coinImageUrl, | |
| }, | |
| chainName: "osmosis", // Default to osmosis chain | |
| } | |
| : undefined; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/modals/add-liquidity.tsx` around lines 120 - 145, The code
destructures pool.reserveCoins into baseCoin and quoteCoin without ensuring
there are two elements, which can lead to missing selectedBase/selectedQuote and
a broken AddInitialLiquidity UI; update the isUninitializedPool branch in
add-liquidity.tsx to first validate that pool.reserveCoins has at least two
entries (or that both baseCoin and quoteCoin are defined) and if not either
return early (render a fallback/error state) or set explicit fallback
SelectionToken values, then only construct selectedBase and selectedQuote when
both coins exist; reference the isUninitializedPool check, pool.reserveCoins,
selectedBase, selectedQuote and the component that renders AddInitialLiquidity
to locate where to add the guard and fallback.
|
Deployment failed with the following error: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts (2)
369-371: Prefer an explicit zero-string check overparseFloatforcurrent_tick_liquidity.
parseFloat(currentTickLiquidity) === 0is safe for the exact-zero case, butparseFloatis imprecise for large decimal strings and makes intent less obvious. A direct string comparison or aDec-based check communicates "is the chain-reported value exactly zero" more clearly.♻️ Proposed refactor
- const isTickLiquidityZero = currentTickLiquidity - ? parseFloat(currentTickLiquidity) === 0 - : false; + const isTickLiquidityZero = currentTickLiquidity === "0";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts` around lines 369 - 371, The check using parseFloat on currentTickLiquidity to determine zero (in isTickLiquidityZero) should be replaced with an explicit exact-zero string check or a Dec-based comparison to reflect chain-reported exactness; update isTickLiquidityZero to return currentTickLiquidity === "0" (or use the Dec type comparison if available) instead of parseFloat(currentTickLiquidity) === 0, keeping the same null/undefined guard.
196-211:apiUtils.local.pools.getPoolis used inside the callback but absent from the dep array.
apiUtils.local.pools.getPool.invalidateis called at line 179 but not listed as a dependency.apiUtils.local.concentratedLiquidity.getLiquidityPerTickRange(the pre-existing invalidation call) is listed. SinceapiUtilsfromapi.useUtils()is a stable tRPC reference this doesn't create a stale closure in practice, but the dep array is now inconsistent. The same omission exists at lines 260–269 forincreaseLiquidity.♻️ Proposed fix
[ poolId, account?.osmosis, apiUtils.local.concentratedLiquidity.getLiquidityPerTickRange, + apiUtils.local.pools.getPool, config.baseDepositAmountIn.sendCurrency, // ...rest of deps ]Apply the same change to the
increaseLiquiditydep array.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts` around lines 196 - 211, The dependency arrays for the callbacks in use-add-concentrated-liquidity-config.ts are missing apiUtils.local.pools.getPool.invalidate even though apiUtils.local.pools.getPool is used (and invalidated) inside the callback; update the dep array for the affected callback (the one that references apiUtils.local.concentratedLiquidity.getLiquidityPerTickRange and calls apiUtils.local.pools.getPool.invalidate) to include apiUtils.local.pools.getPool.invalidate, and apply the same addition to the increaseLiquidity callback's dependency array so both callbacks list apiUtils.local.pools.getPool.invalidate alongside apiUtils.local.concentratedLiquidity.getLiquidityPerTickRange.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts`:
- Around line 174-180: The Promise.all call that invalidates cache entries
(apiUtils.local.concentratedLiquidity.getLiquidityPerTickRange.invalidate and
apiUtils.local.pools.getPool.invalidate) must handle rejections so callers don't
hang; change the chain to call .then(() => resolve()) and add .catch(err =>
reject(err)) (or use .finally to always resolve/reject appropriately) so the
outer Promise<void> always settles; apply the exact same change to the second
Promise.all block at the later lines (the other invalidate calls at lines
242–247) to ensure both paths propagate errors to the caller.
---
Nitpick comments:
In `@packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts`:
- Around line 369-371: The check using parseFloat on currentTickLiquidity to
determine zero (in isTickLiquidityZero) should be replaced with an explicit
exact-zero string check or a Dec-based comparison to reflect chain-reported
exactness; update isTickLiquidityZero to return currentTickLiquidity === "0" (or
use the Dec type comparison if available) instead of
parseFloat(currentTickLiquidity) === 0, keeping the same null/undefined guard.
- Around line 196-211: The dependency arrays for the callbacks in
use-add-concentrated-liquidity-config.ts are missing
apiUtils.local.pools.getPool.invalidate even though apiUtils.local.pools.getPool
is used (and invalidated) inside the callback; update the dep array for the
affected callback (the one that references
apiUtils.local.concentratedLiquidity.getLiquidityPerTickRange and calls
apiUtils.local.pools.getPool.invalidate) to include
apiUtils.local.pools.getPool.invalidate, and apply the same addition to the
increaseLiquidity callback's dependency array so both callbacks list
apiUtils.local.pools.getPool.invalidate alongside
apiUtils.local.concentratedLiquidity.getLiquidityPerTickRange.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/stores/src/account/osmosis/index.tspackages/trpc/src/__tests_e2e__/swap-router.spec.tspackages/web/hooks/ui-config/use-add-concentrated-liquidity-config.tspackages/web/modals/add-liquidity.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/web/modals/add-liquidity.tsx
- packages/stores/src/account/osmosis/index.ts
- packages/trpc/src/tests_e2e/swap-router.spec.ts
packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| )} | ||
| <MyPositionsSection forPoolId={poolId} /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Inactive pools hide existing user positions and rewards
High Severity
The isUninitializedPool || isInactivePool ternary completely replaces the positions section with just a warning and "Add Liquidity" button. Inactive pools by definition have TVL from existing out-of-range positions, yet MyPositionsSection, UserAssetsAndExternalIncentives, the "Collect All Rewards" button, and the "Create a Position" button are all hidden. Users with positions on an inactive pool lose access to position management and reward collection from the pool detail page.
Additional Locations (1)
| console.error(`Failed to fetch remaining pools for pool ${poolId}:`, e); | ||
| } | ||
| queryPool = queries.queryPools.getPool(poolId); | ||
| } |
There was a problem hiding this comment.
Fetching all pools blocks transaction sending path
High Severity
When a pool isn't found in the cache, fetchRemainingPools({ minLiquidity: 0 }) fetches potentially thousands of pools inline during sendCreateConcentratedLiquidityPositionMsg. This blocks the transaction-sending path, causing the application to hang while waiting for the bulk fetch to complete. This is very likely the cause of the "Currently hangs on the add liquidity for out of range pools" issue noted in the PR discussion.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| }; | ||
| } | ||
|
|
||
| @computed |
There was a problem hiding this comment.
Missing error handling for BigDec construction crashes
High Severity
The new BigDec(pool.raw.current_sqrt_price) constructor calls will crash if current_sqrt_price contains an invalid numeric string. The same code pattern at line 60 of use-historical-and-depth-data.ts wraps this in a try-catch block, but these two locations lack error handling. For pools fetched from chain with malformed data or edge cases, this will cause the computed property to throw an unhandled exception, breaking the entire add liquidity flow and pool detail page.
This PR adds support for empty and inactive concentrated liquidity pools, improving the user experience when interacting with pools that have no liquidity or have all liquidity positioned out of range.
Two pool states are now handled:
liquidity strategies only.
Changelog
Backend improvements:
Frontend improvements:
Linear
https://linear.app/osmosis/issue/FE-1557/support-empty-and-inactive-cl-pools-on-frontend
Testing
Note
Medium Risk
Touches core pool-querying and liquidity-add flows across server, store, and UI; main risk is misclassifying pool state/TVL leading to misleading displays or disabled actions, but changes are largely additive and guarded.
Overview
Adds end-to-end support for concentrated liquidity pools that are uninitialized (no price/TVL) or inactive (TVL but zero in-range liquidity), including new UI states that hide charts/positions, show guidance CTAs, and restrict/auto-default strategy selection for safer deposits.
Backend pool fetching now falls back to a direct
poolmanagerchain query when Sidecar can’t return a pool (e.g., unlisted assets), including synthesizing minimal reserve/asset metadata and introducingPool.totalFiatValueLocked?+tvlUnknownto distinguish unknown TVL from zero. Client/store logic and tests were updated to tolerate missing TVL and avoid fetching liquidity/history data for empty pools, plus invalidate pool+tick queries after add-liquidity transactions.Written by Cursor Bugbot for commit 349d4b3. This will update automatically on new commits. Configure here.