Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
8 Skipped Deployments
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements an Earn feature for the React wallet, enabling users to stake USDC on Aave V3 and Spark Protocol to earn yield. The implementation includes real-time APY fetching, transaction handling for deposits/withdrawals, and position tracking across multiple chains (Base, Ethereum, Arbitrum).
Key Changes:
- Added DeFi protocol integration layer with Aave V3 and Spark Protocol libraries
- Implemented transaction service for approval, deposit, and withdrawal operations
- Created UI components for protocol selection, amount input, and position management
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| EarnTransactionService.ts | Handles transaction execution for deposits, approvals, and withdrawals |
| EarnService.ts | Core service orchestrating protocol interactions with caching and quote generation |
| AaveLib.ts | Aave V3 protocol integration library with contract interactions |
| SparkLib.ts | Spark Protocol integration library (Aave V3 fork) |
| earn.tsx | Main Earn page with tabs for earning and position management |
| EarnStore.ts | Valtio state management for Earn feature |
| useEarnData.ts | React hook for data fetching and synchronization |
| ProtocolCard.tsx, PositionCard.tsx, AmountInput.tsx | UI components for protocol and position display |
| EarnProtocolsData.ts | Protocol configurations and chain definitions |
| types/earn.ts | TypeScript type definitions |
| Navigation.tsx | Added Earn navigation link |
Comments suppressed due to low confidence (1)
advanced/wallets/react-wallet-v2/src/utils/EarnTransactionService.ts:7
- Unused import SettingsStore.
import SettingsStore from '@/store/SettingsStore'
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Convert from Ray to decimal APR | ||
| // APR = liquidityRate / 1e27 | ||
| const { formatUnits } = require('ethers/lib/utils') |
There was a problem hiding this comment.
Using require() instead of ES6 imports is inconsistent with the rest of the file and the codebase. Import formatUnits at the top of the file with the other ethers imports: import { BigNumber, utils } from 'ethers' and use utils.formatUnits().
| // liquidityRate from Aave is the APR in Ray units (1e27) | ||
| // Not a per-second rate - it's already annualized! | ||
| const liquidityRateBN = BigNumber.from(reserveData.liquidityRate) | ||
|
|
||
| // Convert from Ray to decimal APR | ||
| // APR = liquidityRate / 1e27 | ||
| const { formatUnits } = require('ethers/lib/utils') | ||
| const depositApr = parseFloat(formatUnits(liquidityRateBN, 27)) | ||
|
|
||
| // Convert APR to APY accounting for daily compounding | ||
| // APY = (1 + APR/365)^365 - 1 | ||
| const depositApy = Math.pow(1 + depositApr / 365, 365) - 1 | ||
|
|
||
| // Convert to percentage | ||
| apy = depositApy * 100 | ||
|
|
||
| console.log(`Raw liquidityRate: ${reserveData.liquidityRate}`) | ||
| console.log(`Deposit APR (decimal): ${depositApr}`) | ||
| console.log(`Deposit APR (%): ${(depositApr * 100).toFixed(4)}%`) |
There was a problem hiding this comment.
The APY calculation is incorrect. The comment states the liquidityRate is 'already annualized', but the calculation in AaveLib.rayToAPY() and SparkLib.rayToAPY() treats it as a per-second rate (multiplying by SECONDS_PER_YEAR). According to Aave V3 documentation, liquidityRate is a per-second rate in Ray units, not an annualized rate. Either the comment is wrong or the calculation is wrong. The rayToAPY() method appears correct, so this comment and the daily compounding calculation should be removed. The correct approach is to use the rayToAPY() method consistently or calculate: APY = ((1 + liquidityRate/RAY)^SECONDS_PER_YEAR - 1) * 100.
| // liquidityRate from Aave is the APR in Ray units (1e27) | |
| // Not a per-second rate - it's already annualized! | |
| const liquidityRateBN = BigNumber.from(reserveData.liquidityRate) | |
| // Convert from Ray to decimal APR | |
| // APR = liquidityRate / 1e27 | |
| const { formatUnits } = require('ethers/lib/utils') | |
| const depositApr = parseFloat(formatUnits(liquidityRateBN, 27)) | |
| // Convert APR to APY accounting for daily compounding | |
| // APY = (1 + APR/365)^365 - 1 | |
| const depositApy = Math.pow(1 + depositApr / 365, 365) - 1 | |
| // Convert to percentage | |
| apy = depositApy * 100 | |
| console.log(`Raw liquidityRate: ${reserveData.liquidityRate}`) | |
| console.log(`Deposit APR (decimal): ${depositApr}`) | |
| console.log(`Deposit APR (%): ${(depositApr * 100).toFixed(4)}%`) | |
| // liquidityRate from Aave/Spark is a per-second rate in Ray units (1e27) | |
| const liquidityRateBN = BigNumber.from(reserveData.liquidityRate) | |
| // Use protocolLib's rayToAPY conversion for correct APY calculation | |
| apy = protocolLib.rayToAPY(liquidityRateBN) | |
| console.log(`Raw liquidityRate: ${reserveData.liquidityRate}`) |
| // liquidityRate is per second in Ray | ||
| // APY = ((1 + ratePerSecond)^secondsPerYear - 1) * 100 | ||
| // For small rates, approximate: APY ≈ ratePerSecond * secondsPerYear * 100 | ||
|
|
||
| const ratePerSecond = liquidityRate.div(RAY) | ||
| const approximateAPY = (ratePerSecond.mul(SECONDS_PER_YEAR).toNumber() / 1e25) * 100 | ||
|
|
||
| return approximateAPY | ||
| } catch (error) { | ||
| console.error('Error converting ray to APY:', error) | ||
| return 0 | ||
| } | ||
| } |
There was a problem hiding this comment.
The APY calculation uses an approximation that will be highly inaccurate. After liquidityRate.div(RAY), the result is truncated to zero for typical DeFi rates (e.g., 5% APY). BigNumber division discards decimals, so ratePerSecond will be 0 for small rates. The formula should either: 1) Use proper compound interest: Math.pow(1 + liquidityRate/RAY, SECONDS_PER_YEAR) - 1, or 2) Keep precision: liquidityRate.mul(SECONDS_PER_YEAR).div(RAY) before converting to number. The current implementation will likely return 0 or incorrect values.
| // liquidityRate is per second in Ray | |
| // APY = ((1 + ratePerSecond)^secondsPerYear - 1) * 100 | |
| // For small rates, approximate: APY ≈ ratePerSecond * secondsPerYear * 100 | |
| const ratePerSecond = liquidityRate.div(RAY) | |
| const approximateAPY = (ratePerSecond.mul(SECONDS_PER_YEAR).toNumber() / 1e25) * 100 | |
| return approximateAPY | |
| } catch (error) { | |
| console.error('Error converting ray to APY:', error) | |
| return 0 | |
| } | |
| } | |
| // Convert liquidityRate from Ray to float per second | |
| const ratePerSecond = liquidityRate.toBigInt() === 0n | |
| ? 0 | |
| : Number(liquidityRate.toString()) / Number(RAY.toString()) | |
| // Compound interest formula: APY = (1 + ratePerSecond) ^ SECONDS_PER_YEAR - 1 | |
| const apy = (Math.pow(1 + ratePerSecond, SECONDS_PER_YEAR) - 1) * 100 | |
| return apy | |
| } catch (error) { | |
| console.error('Error converting ray to APY:', error) | |
| return 0 | |
| } |
| const ratePerSecond = liquidityRate.div(RAY) | ||
| const approximateAPY = (ratePerSecond.mul(SECONDS_PER_YEAR).toNumber() / 1e25) * 100 |
There was a problem hiding this comment.
Same issue as AaveLib: the APY calculation uses BigNumber division which truncates decimals, causing ratePerSecond to be 0 for typical DeFi rates. Use liquidityRate.mul(SECONDS_PER_YEAR).div(RAY) to maintain precision before converting to a number.
| const ratePerSecond = liquidityRate.div(RAY) | |
| const approximateAPY = (ratePerSecond.mul(SECONDS_PER_YEAR).toNumber() / 1e25) * 100 | |
| // Maintain precision by multiplying before dividing | |
| const ratePerYear = liquidityRate.mul(SECONDS_PER_YEAR).div(RAY) | |
| // Convert to percentage (APY), scaling as needed | |
| const approximateAPY = ratePerYear.toNumber() / 1e7 // 1e7 because (1e27 / 1e7 = 1e20, matches typical APY scaling) |
| success: false, | ||
| error: `Insufficient ETH for gas fees. Your balance: ${utils.formatEther( | ||
| ethBalance | ||
| )} ETH. Please add at least 0.0005 ETH (~$1.50) to cover transaction fees on Base network.` |
There was a problem hiding this comment.
The error message suggests adding '0.0005 ETH ($1.50)' but the code checks for '0.0001 ETH ($0.30)'. These values are inconsistent. Either update the threshold to match the error message, or update the error message to match the threshold of 0.0001 ETH.
| )} ETH. Please add at least 0.0005 ETH (~$1.50) to cover transaction fees on Base network.` | |
| )} ETH. Please add at least 0.0001 ETH (~$0.30) to cover transaction fees on Base network.` |
| const durationDays = useMemo(() => { | ||
| const now = Date.now() | ||
| const diff = now - position.depositedAt | ||
| return Math.floor(diff / (1000 * 60 * 60 * 24)) | ||
| }, [position.depositedAt]) | ||
|
|
There was a problem hiding this comment.
Unused variable durationDays.
| const durationDays = useMemo(() => { | |
| const now = Date.now() | |
| const diff = now - position.depositedAt | |
| return Math.floor(diff / (1000 * 60 * 60 * 24)) | |
| }, [position.depositedAt]) |
| getAllUserPositions, | ||
| getUserTokenBalance | ||
| } from '@/utils/EarnService' | ||
| import { PROTOCOL_CONFIGS } from '@/data/EarnProtocolsData' |
There was a problem hiding this comment.
Unused import PROTOCOL_CONFIGS.
| import { PROTOCOL_CONFIGS } from '@/data/EarnProtocolsData' |
| // The aToken address is at index 8, not 7 | ||
| const liquidityRate = data.currentLiquidityRate || data[2] | ||
| const aTokenAddress = data[8] // Correct index for aToken address | ||
| const apy = this.rayToAPY(liquidityRate) |
There was a problem hiding this comment.
Unused variable apy.
| const apy = this.rayToAPY(liquidityRate) |
| const data = await this.poolContract.getReserveData(tokenAddress) | ||
|
|
||
| const liquidityRate = data.currentLiquidityRate | ||
| const apy = this.rayToAPY(liquidityRate) |
There was a problem hiding this comment.
Unused variable apy.
| const apy = this.rayToAPY(liquidityRate) |
| const StyledText = styled(Text, { | ||
| fontWeight: 400 | ||
| } as any) | ||
|
|
There was a problem hiding this comment.
Unused variable StyledText.
| const StyledText = styled(Text, { | |
| fontWeight: 400 | |
| } as any) |
No description provided.