-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Integrate Morpho issues #6492
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
WalkthroughThe pull request introduces significant updates to the staking functionality across various files. Key changes enhance reward calculations, improve provider name handling, and expand support for staking providers like Lido and Morpho. A new utility module, Changes
Sequence DiagramsequenceDiagram
participant User
participant StakingComponent
participant EarnUtils
participant Provider
User->>StakingComponent: Initiate Staking
StakingComponent->>EarnUtils: Get Provider Name
EarnUtils-->>StakingComponent: Normalized Provider Name
StakingComponent->>Provider: Fetch Staking Details
Provider-->>StakingComponent: Return Staking Info
StakingComponent->>StakingComponent: Calculate Rewards
StakingComponent->>User: Display Staking Options
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
packages/kit/src/views/Staking/components/ApproveBaseStake/index.tsx (1)
Line range hint
493-499
: EnsuredaysSpent
is Defined When UsedWhen passing
daysSpent
toshowEstimateGasAlert
, make sure it's defined to prevent potential issues.Provide a default value if
daysSpent
is undefined:onPress={() => { showEstimateGasAlert({ daysConsumed: daysSpent ?? 0, estFiatValue: estimateFeeResp.feeFiatValue, }); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
packages/kit/src/views/Staking/components/ApproveBaseStake/index.tsx
(7 hunks)packages/kit/src/views/Staking/components/ProtocolDetails/ProviderSection.tsx
(2 hunks)packages/kit/src/views/Staking/pages/ApproveBaseStake/index.tsx
(4 hunks)packages/kit/src/views/Staking/pages/Claim/index.tsx
(2 hunks)packages/kit/src/views/Staking/pages/ClaimOptions/index.tsx
(2 hunks)packages/kit/src/views/Staking/pages/InvestmentDetails/index.tsx
(3 hunks)packages/kit/src/views/Staking/pages/ProtocolDetails/index.tsx
(2 hunks)packages/kit/src/views/Staking/pages/Stake/index.tsx
(4 hunks)packages/kit/src/views/Staking/pages/Withdraw/index.tsx
(4 hunks)packages/shared/src/utils/earnUtils.ts
(1 hunks)packages/shared/types/earn/earnProvider.constants.ts
(1 hunks)packages/shared/types/staking.ts
(2 hunks)
🔇 Additional comments (14)
packages/kit/src/views/Staking/components/ApproveBaseStake/index.tsx (2)
264-267
: 🧹 Nitpick (assertive)Implement the TODO for Fiat Value Calculation
There's a TODO to add fiat value for extra rewards. Completing this will provide users with comprehensive reward information.
Would you like assistance in calculating and displaying the fiat value for extra rewards?
349-351
:⚠️ Potential issueCheck for Undefined
daySpent
Before ComparisonThe variable
daySpent
might be undefined ifcalcDaysSpent
returns undefined. Add a check before comparing it to avoid runtime errors.Update the condition:
- if (daySpent && daySpent > 5) { + if (daySpent !== undefined && daySpent > 5) {Likely invalid or redundant comment.
packages/shared/types/earn/earnProvider.constants.ts (1)
38-38
: Verify USDC Staking Support Throughout the AppYou've added 'USDC' to the supported staking symbols. Confirm that all staking functionalities properly support USDC.
Please run the following script to check for USDC staking implementations:
✅ Verification successful
USDC Staking Support Properly Implemented
The codebase shows complete USDC staking support with proper configurations, type definitions, and earn provider implementations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for USDC staking support in the codebase rg 'USDC' packages/ --type=typescriptLength of output: 72
Script:
#!/bin/bash # Search for USDC in all files rg "USDC" packages/ # Search for staking-related files containing USDC rg -l "staking|earn" packages/ | xargs rg "USDC" # Look for USDC in configuration files fd -e ts -e js config packages/ | xargs rg "USDC" # Search for USDC in earn provider files fd -e ts "earn.*provider" packages/ | xargs rg "USDC"Length of output: 12312
packages/kit/src/views/Staking/pages/Claim/index.tsx (1)
52-54
: Consistent Use of Provider NameGood job using
earnUtils.getEarnProviderName
to standardize the provider name. This ensures consistency across the application.packages/kit/src/views/Staking/pages/ApproveBaseStake/index.tsx (1)
44-47
: Good improvement: Clear labeling for staking actionsChanged from generic 'Unknown' to specific 'Stake' label, improving clarity.
packages/kit/src/views/Staking/pages/Stake/index.tsx (3)
89-91
: LGTM! Standardized provider name retrieval.The change improves consistency by using the centralized
earnUtils.getEarnProviderName
function.
149-155
: LGTM! Enhanced provider type checking.The condition now properly handles both Lido and Morpho providers for estimated receive information.
159-175
: LGTM! Well-structured token rate calculation.The memoized value properly handles different provider types and includes a safe default.
packages/kit/src/views/Staking/pages/InvestmentDetails/index.tsx (2)
61-72
: LGTM! Well-implemented reward check function.The function safely handles undefined values and uses BigNumber for accurate comparisons.
178-178
: LGTM! Enhanced badge visibility logic.The condition now properly considers both claimable status and positive rewards.
packages/kit/src/views/Staking/pages/ProtocolDetails/index.tsx (1)
203-205
: LGTM! Standardized protocol name handling.The change consistently uses
earnUtils.getEarnProviderName
for protocol name retrieval.packages/shared/src/utils/earnUtils.ts (1)
53-53
: LGTM! Proper export of the new function.The function is correctly added to the default export object.
packages/kit/src/views/Staking/pages/ClaimOptions/index.tsx (2)
17-17
: Clean import addition!The earnUtils import follows the project's import organization pattern.
62-64
: Good use of centralized provider name handling!The change standardizes provider name handling through earnUtils.
Let's verify consistent usage across the codebase:
✅ Verification successful
Provider name handling is consistently implemented
All staking-related components use earnUtils.getEarnProviderName for protocol assignment. No inconsistencies found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for direct provider assignments that should use earnUtils rg -g '*.{ts,tsx}' 'protocol:\s+provider' packages/ # Search for existing earnUtils.getEarnProviderName usage rg -g '*.{ts,tsx}' 'earnUtils\.getEarnProviderName' packages/Length of output: 778
packages/kit/src/views/Staking/components/ProtocolDetails/ProviderSection.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
♻️ Duplicate comments (3)
packages/kit/src/views/Staking/pages/Withdraw/index.tsx (1)
100-116
: 🛠️ Refactor suggestionExtract token rate logic to a shared hook
This token rate logic duplicates the pattern seen elsewhere in the codebase.
As suggested in previous reviews, consider extracting this to a shared hook:
// useProviderTokenRate.ts export function useProviderTokenRate(provider: IStakeProviderInfo) { return useMemo(() => { const defaultRate = '1'; if (earnUtils.isLidoProvider({ providerName: provider.name })) { return provider.lidoStTokenRate ?? defaultRate; } if (earnUtils.isMorphoProvider({ providerName: provider.name })) { return provider.morphoTokenRate ?? defaultRate; } return defaultRate; }, [provider]); }packages/shared/types/staking.ts (2)
46-46
: 🧹 Nitpick (assertive)Add documentation for morphoTokenRate
The type definition needs documentation for better maintainability.
+ /** Token rate for Morpho protocol, represented as a decimal string */ morphoTokenRate?: string;
381-381
: 🧹 Nitpick (assertive)Add documentation for rewardNum
The type definition needs documentation for better maintainability.
+ /** Map of token addresses to their reward amounts as decimal strings */ rewardNum?: Record<string, string>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
packages/kit-bg/src/services/ServiceStaking.ts
(4 hunks)packages/kit/src/views/Staking/hooks/useUniversalHooks.ts
(6 hunks)packages/kit/src/views/Staking/pages/ApproveBaseStake/index.tsx
(4 hunks)packages/kit/src/views/Staking/pages/Stake/index.tsx
(4 hunks)packages/kit/src/views/Staking/pages/Withdraw/index.tsx
(4 hunks)packages/shared/types/staking.ts
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: lint (20.x)
🔇 Additional comments (9)
packages/kit/src/views/Staking/pages/ApproveBaseStake/index.tsx (4)
44-47
: Good use of utility function for protocol name!Using
earnUtils.getEarnProviderName
standardizes protocol name handling across the codebase.
73-79
: Clean provider type checking!The boolean expression clearly handles both Lido and Morpho providers.
83-99
: Add type safety for token ratesThe token rate logic needs type checking for undefined values.
54-58
: 🧹 Nitpick (assertive)Add type safety for morphoVault
Add type checking to ensure vault property exists when provider is Morpho.
morphoVault: earnUtils.isMorphoProvider({ providerName: provider.name, }) - ? provider.vault + ? provider.vault ?? undefined : undefined,Likely invalid or redundant comment.
packages/kit/src/views/Staking/pages/Stake/index.tsx (3)
89-91
: Consistent use of protocol name utilityMatches the standardization in ApproveBaseStake.
98-102
: Type safety needed for morphoVaultSame type safety improvement needed as in ApproveBaseStake.
164-180
: Token rate type safety neededSame type safety concerns as in ApproveBaseStake.
packages/kit/src/views/Staking/pages/Withdraw/index.tsx (2)
51-55
: Clean implementation of Morpho vault handling!The conditional assignment of morphoVault is well-structured and follows best practices.
60-62
: Good use of centralized provider name handling!Using earnUtils.getEarnProviderName ensures consistent provider name handling across the application.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Technical Updates
These updates improve the flexibility and accuracy of staking-related features across the application.