fix: yield fixes and improvements#11778
Conversation
…letes After completing a yield enter transaction and navigating away (e.g., clicking "View position"), the user could previously go back to /earn/confirm which shouldn't be accessible anymore. Fixes: 1. Reorder guards - check for success state BEFORE checking for selectedYield, ensuring the success screen renders even if Redux state becomes undefined 2. Clear tradeEarnInput Redux state on unmount when in success state, preventing re-access via browser back button or navigation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Links were navigating to /yields/${yieldId} which resulted in 404s.
The correct route is /yield/${yieldId} (singular).
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Moved success buttons from YieldSuccess body content to footerContent prop in EarnConfirm, matching the pattern used by input/confirm steps. Added showButtons prop to YieldSuccess for backwards compatibility. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When fiat amount is zero, return empty string instead of '0.00' to trigger placeholder styling (greyed out) matching crypto mode behavior. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
For yields with a default validator (Cosmos ATOM, Solana SOL native staking), filter out positions from other validators in useAllYieldBalances query. This ensures only ShapeShift DAO positions show for Cosmos and only Figment positions show for Solana, hiding positions staked externally with other validators. Removed redundant validator-specific filtering from YieldsList.tsx since filtering now happens at the data layer. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Liquid staking yields (like ETH Lido) were missing the Provider row because they are classified as staking but have no validators. The condition now shows the provider when there's no validator metadata. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Document that PRs opened via CLI tools should always use the PULL_REQUEST_TEMPLATE.md as the base for the PR body. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR integrates YieldXyz features into the yield earning flow with new utilities for selecting best yields, introduces a multi-validator balance filtering system, adds dynamic components for crypto amount input display, and updates navigation paths from pluralized Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Shows a loading spinner with tooltip when legacy positions are still loading, allowing the yield table to render immediately with yield.xyz data. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds getBestActionableYield utility that filters out disabled yields (enter disabled, under maintenance, deprecated) before selecting the highest APY option. Prevents showing CTAs for opportunities users can't act on. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When DeFiEarn is rendered outside a YieldAccountProvider (like in the wallet drawer), the default accountNumber: 0 was incorrectly filtering to only Account #0's balances. Now falls through to enabledWalletAccountIds when no context is present, properly aggregating balances across all accounts. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Opening for a sec so @coderabbitai rabbits. |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/react-queries/queries/yieldxyz/useAllYieldBalances.ts (1)
288-308: RecomputehighestAmountUsdValidatorafter default-validator filtering.
Right now it’s computed before the default-validator filter, so for yields with enforced validators it can point to a validator that gets excluded. That can leak inconsistent “highest” metadata downstream.✅ Suggested fix (keep highest within retained balances)
- let highestAmountUsd = bnOrZero(0) - let highestAmountUsdValidator: string | undefined - - for (const balance of augmentedBalances) { - const usd = bnOrZero(balance.amountUsd) - if (balance.validator?.address && usd.gt(highestAmountUsd)) { - highestAmountUsd = usd - highestAmountUsdValidator = balance.validator.address - } - } + const defaultValidator = DEFAULT_VALIDATOR_BY_YIELD_ID[item.yieldId] + let highestAmountUsd = bnOrZero(0) + let highestAmountUsdValidator: string | undefined + + for (const balance of augmentedBalances) { + if (defaultValidator && balance.validator?.address !== defaultValidator) continue + const usd = bnOrZero(balance.amountUsd) + if (balance.validator?.address && usd.gt(highestAmountUsd)) { + highestAmountUsd = usd + highestAmountUsdValidator = balance.validator.address + } + } @@ - for (const balance of augmentedBalances) { - const defaultValidator = DEFAULT_VALIDATOR_BY_YIELD_ID[item.yieldId] - if (defaultValidator && balance.validator?.address !== defaultValidator) { - continue - } + for (const balance of augmentedBalances) { + if (defaultValidator && balance.validator?.address !== defaultValidator) continue
- Wrap DeFiEarn in memo to prevent unnecessary re-renders - Remove empty useEffect in YieldForm - Consolidate isStakingYieldType utility (remove redundant wrapper) - Extract CryptoAmountInput to shared component - Extract useYieldDisplayInfo hook from YieldsList - Move static searchIcon outside component - Fix highestAmountUsdValidator computed after validator filtering - Remove YIELD_IMPROVEMENTS.md dev notes file Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/CryptoAmountInput/CryptoAmountInput.tsx`:
- Around line 26-41: Compute a resolved font size that respects prop overrides
and keeps lineHeight in sync: derive valueLength with valueLength/useMemo and
computedFontSize via getInputFontSize(valueLength), then create resolvedFontSize
= props.fontSize ?? computedFontSize and pass fontSize={resolvedFontSize} and
lineHeight={resolvedFontSize} to the Input (so a provided props.fontSize
overrides but lineHeight stays matched). Also memoize the color by creating a
colorMemo = useMemo(() => (props.value ? 'text.base' : 'text.subtle'),
[props.value]) and pass color={colorMemo} to Input to follow memoization
guidelines.
🧹 Nitpick comments (5)
src/components/StakingVaults/DeFiEarn.tsx (1)
101-107: Consider removing redundant.filter(Boolean).Per the
fromAssetIdimplementation in the relevant snippets, it always returns achainId(or throws on invalid input). The.filter(Boolean)on line 105 is redundant sincechainIdwill never be falsy for valid asset IDs.♻️ Optional cleanup
const chainIds = useMemo(() => { if (!isYieldXyzEnabled || !yieldOpportunities?.length) return chainIdsFromWallet - const yieldChainIds = yieldOpportunities - .map(item => fromAssetId(item.assetId).chainId) - .filter(Boolean) + const yieldChainIds = yieldOpportunities.map(item => fromAssetId(item.assetId).chainId) return Array.from(new Set([...chainIdsFromWallet, ...yieldChainIds])) }, [chainIdsFromWallet, isYieldXyzEnabled, yieldOpportunities])src/pages/Yields/hooks/useYieldDisplayInfo.ts (1)
21-61: Add an explicit return type for the hook.This keeps the hook aligned with the TS explicit-typing guideline and makes the public API clearer.
As per coding guidelines, explicit return types are required.♻️ Proposed fix
-export const useYieldDisplayInfo = (providers: Record<string, ProviderDto> | undefined) => { +export const useYieldDisplayInfo = ( + providers: Record<string, ProviderDto> | undefined, +): ((yieldItem: AugmentedYieldDto) => YieldDisplayInfo) => {src/components/CryptoAmountInput/CryptoAmountInput.tsx (3)
5-14: Consider enum-backed breakpoint keys for typed constants.Guidelines prefer enums for constants; a string enum for breakpoint keys + a typed
Recordreduces typo risk as usage grows.
As per coding guidelines.♻️ Possible refactor
-const INPUT_LENGTH_BREAKPOINTS = { - FOR_XS_FONT: 22, - FOR_SM_FONT: 14, - FOR_MD_FONT: 10, -} as const +enum InputLengthBreakpoint { + ForXsFont = 'FOR_XS_FONT', + ForSmFont = 'FOR_SM_FONT', + ForMdFont = 'FOR_MD_FONT', +} + +const INPUT_LENGTH_BREAKPOINTS: Record<InputLengthBreakpoint, number> = { + [InputLengthBreakpoint.ForXsFont]: 22, + [InputLengthBreakpoint.ForSmFont]: 14, + [InputLengthBreakpoint.ForMdFont]: 10, +} -const getInputFontSize = (length: number): string => { - if (length >= INPUT_LENGTH_BREAKPOINTS.FOR_XS_FONT) return '24px' - if (length >= INPUT_LENGTH_BREAKPOINTS.FOR_SM_FONT) return '30px' - if (length >= INPUT_LENGTH_BREAKPOINTS.FOR_MD_FONT) return '38px' +const getInputFontSize = (length: number): string => { + if (length >= INPUT_LENGTH_BREAKPOINTS[InputLengthBreakpoint.ForXsFont]) return '24px' + if (length >= INPUT_LENGTH_BREAKPOINTS[InputLengthBreakpoint.ForSmFont]) return '30px' + if (length >= INPUT_LENGTH_BREAKPOINTS[InputLengthBreakpoint.ForMdFont]) return '38px' return '48px' }
18-23: TightenCryptoAmountInputPropstyping (avoidunknownprops).The index signature drops Chakra’s prop typing and allows invalid props. Prefer
InputPropsand name the event parameter.
As per coding guidelines.♻️ Suggested typing update
-import { Input } from '@chakra-ui/react' +import { Input, type InputProps } from '@chakra-ui/react' import type { ChangeEvent } from 'react' -export type CryptoAmountInputProps = { - value?: string - onChange?: (e: ChangeEvent<HTMLInputElement>) => void - placeholder?: string - [key: string]: unknown -} +export type CryptoAmountInputProps = Omit<InputProps, 'value' | 'onChange' | 'placeholder'> & { + value?: string + onChange?: (event: ChangeEvent<HTMLInputElement>) => void + placeholder?: string +}
25-27: Add explicit return type; confirm ErrorBoundary coverage.The memoized component should declare an explicit return type, and please verify it’s rendered under an ErrorBoundary (or wrap at usage).
As per coding guidelines.♻️ Suggested signature tweak
-export const CryptoAmountInput = memo((props: CryptoAmountInputProps) => { +export const CryptoAmountInput = memo((props: CryptoAmountInputProps): JSX.Element => {
premiumjibles
left a comment
There was a problem hiding this comment.
Navigate to Yields page, click "Other Yields" → should go to /yield/ not /yields/ - 🤷 wasn't sure on this one. Where's the "Other yields" button?
Stake ETH via Lido → Provider row should show "Lido" 👍
Stake Cosmos → Validator row should show (not Provider) 👍
Check asset page CTA for asset with disabled yields → should show next best actionable yield or no CTA (test with Solana, which is currently the affected one) 👍 - confirmed SOL yield on asset pages now matches what's on the /yield page for solana
Navigate to DeFi earn tab → yields should load first, legacy positions show spinner while loading 👍
Complete a yield transaction → should not be able to navigate back to confirm step 👍
Open DeFi drawer with yield positions on multiple accounts → fiat value should aggregate across all accounts 👍
This guy! https://jam.dev/c/27fd1ab2-ccf8-48d0-8f5a-7b5f2f3cc0ca |
Description
Collection of yield.xyz polish fixes and improvements identified during QA testing.
Fixes
Provider row for liquid staking yields - Fixed missing Provider row for ETH liquid staking (like Lido) in yield modals. Liquid staking yields are classified as "staking" but have no validators, so the condition now shows the provider when there's no validator metadata.
Filter non-default validator positions - Positions from non-ShapeShift validators are now filtered at the data layer to prevent showing staking positions we don't manage.
Fiat zero amount styling - Zero fiat amounts in yield modals now display as placeholder text instead of actual values to avoid confusion (e.g., showing available balance when user has none).
Yield success step footer spacing - Eliminated dead space in the yield success step footer for cleaner UI.
Yield routing fix - Corrected "Other Yields" links from
/yields/(404) to/yield/.Prevent earn confirm re-access - Users can no longer navigate back to
/earn/confirmafter completing a yield transaction.Spinner loading state for legacy positions - DeFi earn page now shows a spinner with tooltip when legacy positions are still loading, allowing the yield table to render immediately with yield.xyz data.
Best actionable yield selection - Asset page CTA now filters out disabled yields (enter disabled, under maintenance, deprecated) before selecting the highest APY option. Prevents showing CTAs for opportunities users can't act on.
Aggregate yield balances in drawer - DeFi drawer now properly aggregates yield balances across all accounts instead of only showing Account #0's balance.
Issue (if applicable)
N/A - QA polish fixes
Risk
Low - UI/UX fixes and data filtering. No transaction logic changes.
None - purely display logic and data filtering fixes.
Testing
Engineering
/yield/not/yields/Operations
Screenshots (if applicable)
https://jam.dev/c/55c3f07e-45c7-4891-b06e-d568ea5ab6e6
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.