feat: colocate pools' active data in usePool()#10265
Conversation
📝 WalkthroughWalkthroughIntroduces a reusable isLpDepositEnabled utility, extends Pool with per-pool status fields, and updates pool fetching to query Thorchain inbound addresses and MIMIR to compute per-pool isTradingActive and isLpDepositEnabled. AvailablePools now renders using these per-pool fields and loading flags. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as AvailablePools
participant Pools as usePools
participant Midgard as Midgard API
participant Inbound as Thorchain Inbound API
participant Mimir as Thorchain MIMIR
UI->>Pools: request pools
Pools->>Midgard: fetch pools list
Midgard-->>Pools: pools
Pools->>Inbound: fetch inbound addresses (Thorchain)
Inbound-->>Pools: inbound addresses
Pools->>Mimir: fetch MIMIR (Thorchain)
Mimir-->>Pools: mimir flags
Pools->>Pools: compute isTradingActive per pool
Pools->>Pools: compute isLpDepositEnabled per pool
Pools-->>UI: pools with status & loading flags
UI->>UI: render using per-pool fields
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
gomesalexandre
changed the title
usePool()
![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
🔭 Outside diff range comments (1)
src/pages/ThorChainLP/AvailablePools.tsx (1)
90-94: Consider handling undefined states explicitlyThe default case returns a subtle color with APY, but this might be reached when
pool.isTradingActiveisundefined(still loading). Consider whether this is the intended behavior.Consider explicitly handling the loading state:
case pool.isTradingActive === true && pool.status === 'staged': return { color: 'yellow.500', element: <Text translation='common.staged' />, } + case pool.isTradingActive === undefined: + return { + color: 'text.subtle', + element: <Text translation='common.loading' />, + } default: return { color: 'text.subtle', element: <Amount.Percent value={pool.annualPercentageRate} suffix='APY' />, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsx(1 hunks)src/pages/ThorChainLP/AvailablePools.tsx(4 hunks)src/pages/ThorChainLP/queries/hooks/usePool.ts(1 hunks)src/pages/ThorChainLP/queries/hooks/usePools.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/pages/ThorChainLP/queries/hooks/usePool.tssrc/pages/ThorChainLP/queries/hooks/usePools.tssrc/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsxsrc/pages/ThorChainLP/AvailablePools.tsx
**/*
📄 CodeRabbit Inference Engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/pages/ThorChainLP/queries/hooks/usePool.tssrc/pages/ThorChainLP/queries/hooks/usePools.tssrc/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsxsrc/pages/ThorChainLP/AvailablePools.tsx
**/use*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/naming-conventions.mdc)
**/use*.{ts,tsx}: ALWAYS use use prefix for custom hooks
ALWAYS use descriptive names that indicate the hook's purpose
ALWAYS use camelCase after the use prefix for custom hooks
Files:
src/pages/ThorChainLP/queries/hooks/usePool.tssrc/pages/ThorChainLP/queries/hooks/usePools.tssrc/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/pages/ThorChainLP/queries/hooks/usePool.tssrc/pages/ThorChainLP/queries/hooks/usePools.tssrc/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsxsrc/pages/ThorChainLP/AvailablePools.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsxsrc/pages/ThorChainLP/AvailablePools.tsx
**/*.{tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsxsrc/pages/ThorChainLP/AvailablePools.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: gomesalexandre
PR: shapeshift/web#10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
PR: shapeshift/web#10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
🧬 Code Graph Analysis (2)
src/pages/ThorChainLP/queries/hooks/usePools.ts (8)
src/react-queries/queries/thornode.ts (1)
getInboundAddressesQuery(72-88)packages/swapper/src/thorchain-utils/index.ts (1)
getChainIdBySwapper(40-49)src/lib/utils/thorchain/hooks/useThorchainMimir.ts (1)
useThorchainMimir(15-28)packages/caip/src/constants.ts (1)
thorchainChainId(71-71)src/pages/ThorChainLP/queries/hooks/usePool.ts (1)
Pool(20-28)packages/swapper/src/swappers/ThorchainSwapper/utils/poolAssetHelpers/poolAssetHelpers.ts (1)
thorPoolAssetIdToAssetId(10-11)src/react-queries/selectors/index.ts (2)
selectInboundAddressData(18-28)selectIsTradingActive(30-73)src/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsx (1)
isLpDepositEnabled(8-32)
src/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsx (5)
src/lib/utils/thorchain/types.ts (1)
ThorchainMimir(9-209)packages/caip/src/assetId/assetId.ts (1)
AssetId(17-17)packages/caip/src/constants.ts (2)
thorchainAssetId(49-49)thorchainChainId(71-71)packages/swapper/src/swappers/ThorchainSwapper/utils/poolAssetHelpers/poolAssetHelpers.ts (1)
assetIdToThorPoolAssetId(13-14)src/lib/utils/thorchain/hooks/useThorchainMimir.ts (1)
useThorchainMimir(15-28)
🔇 Additional comments (4)
src/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsx (1)
25-25: The script didn’t surface howpoolAssetIdis later used or validated against known Thorchain asset IDs, nor any tests demonstrating the expected dot/dash placement. To confirm the correct transformation (first dash only vs. all dashes) we need to see:
- How
poolAssetIdis matched or looked up (e.g. in an enabled‐pools map).- Any existing normalization logic (e.g. in
assetIdToThorPoolAssetIdMap).Could you please share the surrounding lines in
src/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsx—specifically howpoolAssetIdis consumed—or add tests/logs showing lookup failures for multi-dash IDs?src/pages/ThorChainLP/queries/hooks/usePool.ts (1)
24-28: Good addition of pool status fieldsThe addition of
isTradingActive,isTradingActiveLoading, andisLpDepositEnabledfields to thePooltype properly supports the colocation of pool status data as intended by the PR.src/pages/ThorChainLP/AvailablePools.tsx (1)
63-96: Clean refactor to use pool-level status dataThe switch to using
pool.isLpDepositEnabledandpool.isTradingActivesuccessfully eliminates the need for external hooks and improves performance by colocating the data fetching. The status logic is now cleaner and more maintainable.src/pages/ThorChainLP/queries/hooks/usePools.ts (1)
132-143: Good implementation of LP deposit statusThe implementation correctly integrates the new
isLpDepositEnabledfunction to compute per-pool deposit status, which aligns well with the PR's objective of colocating pool data.
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
Seems like working as expected, happy with that
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/pages/ThorChainLP/queries/hooks/usePools.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/pages/ThorChainLP/queries/hooks/usePools.ts
**/*
📄 CodeRabbit Inference Engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/pages/ThorChainLP/queries/hooks/usePools.ts
**/use*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/naming-conventions.mdc)
**/use*.{ts,tsx}: ALWAYS use use prefix for custom hooks
ALWAYS use descriptive names that indicate the hook's purpose
ALWAYS use camelCase after the use prefix for custom hooks
Files:
src/pages/ThorChainLP/queries/hooks/usePools.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/pages/ThorChainLP/queries/hooks/usePools.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: gomesalexandre
PR: shapeshift/web#10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
PR: shapeshift/web#10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
📚 Learning: 2025-08-13T13:45:25.710Z
Learnt from: gomesalexandre
PR: shapeshift/web#10265
File: src/pages/ThorChainLP/queries/hooks/usePools.ts:93-0
Timestamp: 2025-08-13T13:45:25.710Z
Learning: In the ShapeShift web app, inbound addresses data for Thorchain pools requires aggressive caching settings (staleTime: 0, gcTime: 0, refetchInterval: 60_000) to ensure trading status and LP deposit availability are always current. This is intentional business-critical behavior, not a performance issue to be optimized.
Applied to files:
src/pages/ThorChainLP/queries/hooks/usePools.ts
📚 Learning: 2025-08-07T11:22:16.983Z
Learnt from: gomesalexandre
PR: shapeshift/web#10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
Applied to files:
src/pages/ThorChainLP/queries/hooks/usePools.ts
🧬 Code Graph Analysis (1)
src/pages/ThorChainLP/queries/hooks/usePools.ts (8)
src/react-queries/queries/thornode.ts (1)
getInboundAddressesQuery(72-88)packages/swapper/src/thorchain-utils/index.ts (1)
getChainIdBySwapper(40-49)src/lib/utils/thorchain/hooks/useThorchainMimir.ts (1)
useThorchainMimir(15-28)packages/caip/src/constants.ts (1)
thorchainChainId(71-71)src/pages/ThorChainLP/queries/hooks/usePool.ts (1)
Pool(20-28)packages/swapper/src/swappers/ThorchainSwapper/utils/poolAssetHelpers/poolAssetHelpers.ts (1)
thorPoolAssetIdToAssetId(10-11)src/react-queries/selectors/index.ts (2)
selectInboundAddressData(18-28)selectIsTradingActive(30-73)src/lib/utils/thorchain/hooks/useIsThorchainLpDepositEnabled.tsx (1)
isLpDepositEnabled(8-32)
⏰ 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: Call / Static
🔇 Additional comments (2)
src/pages/ThorChainLP/queries/hooks/usePools.ts (2)
93-103: Aggressive refetch config acknowledged and appropriate hereThe always-stale + 60s refetch strategy for inbound addresses is intentional for real-time trading/LP status. Looks good given business requirements.
132-145: LGTM: No silent drops when Mimir is unavailableEarly-returning poolsWithActiveData when mimir is missing avoids silently dropping pools. Solid improvement for predictability.
|
|
||
| export type { Pool } from './usePool' | ||
|
|
||
| export const usePools = () => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add an explicit return type for usePools
Project guidelines require explicit return types. This also documents the hook’s contract.
-import { useQueries, useQuery } from '@tanstack/react-query'
+import { useQueries, useQuery, type UseQueryResult } from '@tanstack/react-query'-export const usePools = () => {
+export const usePools = (): UseQueryResult<Pool[]> => {Also applies to: 4-4
🤖 Prompt for AI Agents
In src/pages/ThorChainLP/queries/hooks/usePools.ts around line 22 (and also
applies to lines 4-4), the usePools hook is missing an explicit return type;
update the function signature to include a concrete return type that reflects
the hook's contract (for example the specific data/result shape, or a React
Query return type like ReturnType<typeof useQuery> or Promise/Array of Pool
items as appropriate), and import or define any required types; ensure the
exported usePools signature explicitly declares that return type.
| const { data: mimir } = useThorchainMimir({ | ||
| chainId: thorchainChainId, | ||
| }) |
There was a problem hiding this comment.
Include Mimir loading state to avoid false “inactive” during initial load
selectIsTradingActive returns false for native fee assets until mimir is available. Without including mimir’s loading state, rows can briefly show “inactive” instead of “loading”.
- const { data: mimir } = useThorchainMimir({
+ const { data: mimir, isLoading: isMimirLoading } = useThorchainMimir({
chainId: thorchainChainId,
})- acc.push({ ...pool, isTradingActive, isTradingActiveLoading: isInboundAddressesDataLoading })
+ acc.push({
+ ...pool,
+ isTradingActive,
+ isTradingActiveLoading: isInboundAddressesDataLoading || isMimirLoading,
+ })Also applies to: 124-126
| const assetId = thorPoolAssetIdToAssetId(pool.asset) | ||
| if (!assetId) return acc | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use pool.assetId instead of recomputing it; remove now-unneeded import
Pool already carries a non-optional assetId. Re-deriving it risks mismatches and unnecessary failures. This also lets you drop thorPoolAssetIdToAssetId from imports.
-import { getChainIdBySwapper, SwapperName, thorPoolAssetIdToAssetId } from '@shapeshiftoss/swapper'
+import { getChainIdBySwapper, SwapperName } from '@shapeshiftoss/swapper'- const assetId = thorPoolAssetIdToAssetId(pool.asset)
- if (!assetId) return acc
+ const { assetId } = pool- const assetId = thorPoolAssetIdToAssetId(pool.asset)
- if (!assetId) return acc
+ const { assetId } = poolAlso applies to: 136-138, 3-3
🤖 Prompt for AI Agents
In src/pages/ThorChainLP/queries/hooks/usePools.ts around lines 112-114 (and
likewise update 136-138 and the import at line 3), the code recomputes an
assetId via thorPoolAssetIdToAssetId(pool.asset) and guards on its falsiness;
instead use the existing pool.assetId property directly (e.g. const assetId =
pool.assetId) and remove the now-unneeded thorPoolAssetIdToAssetId import from
the top of the file so you no longer recompute or guard on a derived value.
2 participants
Description
Makes it so that
usePools()now return active data (isTradingActive/isTradingActiveLoading/isLpDepositEnabled) for later use in sorting, vs. currently being fetched by each row.Issue (if applicable)
Risk
Testing
Engineering
Operations
Screenshots (if applicable)
https://jam.dev/c/f4394434-a9fe-4709-b9ca-63725d5a6897
Summary by CodeRabbit
New Features
Refactor
Bug Fixes