chore: handle new morpho token and wrapper #4050
Conversation
WalkthroughThe changes include the introduction of a new function Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
features/reclaimCollateral/reclaimCollateralView.tsx (2)
19-20: Consider using a logging utility instead of direct console calls.While debugging is valuable during development, direct console calls in production code are not recommended. Consider using a proper logging utility that can be controlled via environment configuration.
-console.debug('ReclaimCollateralButton state:', state) +import { logger } from 'helpers/logger' +logger.debug('ReclaimCollateralButton state:', state)
Handle MORPHO token scenarios in ReclaimCollateralButton.
The
ReclaimCollateralButtoncomponent lacks handling for MORPHO tokens. Please implement support for both legacy and new MORPHO tokens during the reclaim process.🔗 Analysis chain
Line range hint
8-39: Verify handling of MORPHO token scenarios.Given that this PR introduces MORPHO token migration support, we should verify that the ReclaimCollateralButton component correctly handles both legacy and new MORPHO tokens during the reclaim process.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for MORPHO token handling in the codebase echo "Checking for MORPHO token handling in reclaimCollateral$ stream..." rg -A 5 "reclaimCollateral\$" echo "Checking for MORPHO token specific conditions..." ast-grep --pattern 'if ($_ === "MORPHO" || $_ === "MORPHO_LEGACY") { $$$ }'Length of output: 360
blockchain/better-calls/erc20.ts (1)
204-224: Enhance documentation clarity and completeness.The documentation is comprehensive but could benefit from these additions:
- Clarify which proxy is referenced in step 2 ("from proxy")
- Add a note about gas considerations for the combined approve and wrap operation
- Include an @example section demonstrating usage
/** * Encodes a transaction to approve and wrap an ERC20 token using OpenZeppelin's ERC20Wrapper pattern. * This function prepares a transaction that will: * 1. Approve the wrapper contract to spend the old token - * 2. Deposit the old token into the wrapper contract ( from proxy) + * 2. Deposit the old token into the wrapper contract (from DPM proxy account) * 3.Send the new wrapped token to the owner * * The wrapper contract must implement the IERC20Wrapper interface which includes: * - depositFor(address account, uint256 value) * - withdrawTo(address account, uint256 value) * + * Note: This operation requires two state changes (approve and wrap) which may result + * in higher gas costs compared to separate operations. + * * @param {object} params - The parameters object * @param {NetworkIds} params.networkId - The network ID where the transaction will be executed * @param {string} params.oldToken - The symbol of the token to be wrapped (underlying token) * @param {string} params.newToken - The symbol of the wrapped token to receive * @param {string} params.wrapper - The address of the ERC20Wrapper contract * @param {BigNumber} params.amount - The amount of tokens to wrap * @returns {Promise<OmniTxData>} The encoded transaction data ready to be executed * @throws Will throw if the contracts or tokens don't exist in the network configuration * @throws Will throw if the token addresses cannot be resolved + * + * @example + * const txData = await encodeApproveAndWrapProxyAction({ + * networkId: 1, + * oldToken: 'MORPHO_LEGACY', + * newToken: 'MORPHO', + * wrapper: '0x...', + * amount: new BigNumber('1000000000000000000') // 1 token + * }); */blockchain/token-metadata-list/token-configs.ts (1)
1000-1008: Consider adding price feed tickers for the MORPHO_LEGACY token.The configuration structure follows the established pattern and maintains consistency with the MORPHO token. However, consider adding the following fields for better integration with external services:
coinGeckoTickerfor price feed integrationbackgroundfor consistent UI styling{ symbol: 'MORPHO_LEGACY', precision: 18, digits: 5, name: 'Legacy Morpho Blue', icon: morpho_circle_color, iconCircle: morpho_circle_color, + coinGeckoTicker: 'morpho', + background: 'linear-gradient(286.73deg, #B6509E 2.03%, #2EBAC6 100%)', tags: [], },features/omni-kit/components/details-section/OmniDetailSectionRewardsClaims.tsx (1)
Line range hint
69-185: Include eligibility flags in theuseEffectdependency arrayThe
useEffecthook depends onisEligibleForErc20Claims,isEligibleForMorphoLegacy, andisEligibleForProtocolRewards, but these variables are not included in the dependency array. Omitting them might lead to the effect not running when their values change, causing potential inconsistencies in the claims state.Apply this diff to include the eligibility flags:
69 useEffect(() => { // existing effect code... - }, [dpmProxy, networkId, protocol, quoteAddress]) + }, [ + dpmProxy, + networkId, + protocol, + quoteAddress, + isEligibleForErc20Claims, + isEligibleForMorphoLegacy, + isEligibleForProtocolRewards, + ])🧰 Tools
🪛 GitHub Check: Linter
[failure] 96-96:
Unexpected console statement
[failure] 98-98:
Unexpected console statement
[failure] 100-100:
Unexpected console statement
[failure] 104-104:
Unexpected console statement🪛 eslint
[error] 96-96: Unexpected console statement.
(no-console)
[error] 98-98: Unexpected console statement.
(no-console)
[error] 100-100: Unexpected console statement.
(no-console)
[error] 104-104: Unexpected console statement.
(no-console)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
blockchain/abi/erc20-proxy-actions.json(1 hunks)blockchain/better-calls/erc20.ts(1 hunks)blockchain/token-metadata-list/token-configs.ts(1 hunks)blockchain/tokens/mainnet.ts(1 hunks)features/notices/VaultsNoticesView.tsx(1 hunks)features/omni-kit/components/details-section/OmniDetailSectionRewardsClaims.tsx(6 hunks)features/omni-kit/protocols/erc-4626/settings.ts(8 hunks)features/reclaimCollateral/reclaimCollateralView.tsx(1 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- features/notices/VaultsNoticesView.tsx
🧰 Additional context used
📓 Learnings (1)
features/omni-kit/components/details-section/OmniDetailSectionRewardsClaims.tsx (1)
Learnt from: halaprix
PR: OasisDEX/oasis-borrow#4046
File: features/omni-kit/components/details-section/OmniDetailSectionRewardsClaims.tsx:53-53
Timestamp: 2024-11-12T11:56:49.827Z
Learning: In TypeScript code, we rely on TypeScript's type checking to catch cases where `NetworkIds` are modified and used as an index, ensuring that accessing `claimableErc20[networkId]` is safe.
🪛 GitHub Check: Linter
features/omni-kit/components/details-section/OmniDetailSectionRewardsClaims.tsx
[failure] 96-96:
Unexpected console statement
[failure] 98-98:
Unexpected console statement
[failure] 100-100:
Unexpected console statement
[failure] 104-104:
Unexpected console statement
🪛 eslint
features/omni-kit/components/details-section/OmniDetailSectionRewardsClaims.tsx
[error] 96-96: Unexpected console statement.
(no-console)
[error] 98-98: Unexpected console statement.
(no-console)
[error] 100-100: Unexpected console statement.
(no-console)
[error] 104-104: Unexpected console statement.
(no-console)
🔇 Additional comments (5)
blockchain/tokens/mainnet.ts (2)
77-77: Verify token metadata configuration.
Since this is a new token, ensure that corresponding metadata (symbol, precision, digits, name, icon, tags) is configured in the token metadata list.
#!/bin/bash
# Description: Check if token metadata is configured
# Expected: MORPHO_LEGACY configuration should exist in token-configs.ts
# Search for MORPHO_LEGACY in token configuration files
rg -l "MORPHO_LEGACY" blockchain/token-metadata-list/77-77: LGTM! Verify MORPHO_LEGACY address.
The MORPHO_LEGACY token addition follows the correct pattern and is properly positioned in the alphabetical order.
Let's verify the MORPHO_LEGACY address from @oasisdex/addresses package:
features/omni-kit/protocols/erc-4626/settings.ts (1)
95-98: LGTM! Well-structured constant definition.
The legacyMorphoRewards constant follows the established pattern and clearly identifies its purpose for legacy token support.
package.json (1)
46-46: Verify @oasisdex/addresses version compatibility
The version bump to 0.1.89 aligns with the PR's objective to support MORPHO_LEGACY token migration.
Let's verify the version contains the required addresses and check for any security advisories:
blockchain/better-calls/erc20.ts (1)
243-267: 🛠️ Refactor suggestion
Verify contract relationships and add safety checks.
The implementation should verify that:
- The wrapper contract implements the IERC20Wrapper interface
- The new token is actually the wrapped version of the old token
Additionally, consider adding these safety checks:
const { erc20ProxyActions, tokens } = contracts
const oldTokenAddress = tokens[oldToken].address
const newTokenAddress = tokens[newToken].address
+ // Verify wrapper contract implements required interface
+ const wrapperContract = new ethers.Contract(
+ wrapper,
+ ['function depositFor(address,uint256) external returns (bool)'],
+ getRpcProvider(networkId)
+ )
+ try {
+ await wrapperContract.estimateGas.depositFor(erc20ProxyActions.address, 0)
+ } catch (e) {
+ throw new Error('Wrapper contract does not implement required interface')
+ }
+
const proxyActionContract = Erc20ProxyActions__factory.connect(
erc20ProxyActions.address,
getRpcProvider(networkId),
)| console.debug('Recv aimCollateralButton txStatus:', txStatus) | ||
| console.debug('ReclaimCollateralButton isLoading:', isLoading) | ||
|
|
There was a problem hiding this comment.
Fix typo in debug statement and maintain consistent logging format.
There's a typo in the first debug statement ("Recv aim" instead of "Reclaim"), and the logging format is inconsistent across statements.
-console.debug('Recv aimCollateralButton txStatus:', txStatus)
-console.debug('ReclaimCollateralButton isLoading:', isLoading)
+import { logger } from 'helpers/logger'
+logger.debug('ReclaimCollateralButton txStatus:', txStatus)
+logger.debug('ReclaimCollateralButton isLoading:', isLoading)Committable suggestion skipped: line range outside the PR's diff.
| { | ||
| "inputs": [ | ||
| { | ||
| "internalType": "address", | ||
| "name": "oldToken", | ||
| "type": "address" | ||
| }, | ||
| { | ||
| "internalType": "address", | ||
| "name": "newToken", | ||
| "type": "address" | ||
| }, | ||
| { | ||
| "internalType": "address", | ||
| "name": "wrapper", | ||
| "type": "address" | ||
| }, | ||
| { | ||
| "internalType": "uint256", | ||
| "name": "value", | ||
| "type": "uint256" | ||
| } | ||
| ], | ||
| "name": "approveAndWrap", | ||
| "outputs": [], | ||
| "stateMutability": "nonpayable", | ||
| "type": "function" | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Wrapper contract integration not found
No implementation of the wrapper contract was detected in the codebase. Please ensure that the wrapper contract is properly implemented and integrated.
🔗 Analysis chain
Implementation looks good with security considerations.
The approveAndWrap function ABI is well-structured for the MORPHO token migration process. The parameter ordering and types are appropriate for the intended functionality.
Let's verify the wrapper contract integration:
Security Considerations:
- Ensure the wrapper contract is audited and properly verified on Etherscan
- Verify that the wrapper contract follows the check-effects-interactions pattern
- Consider adding explicit approval checks before wrapping
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the wrapper contract implementation and integration
# Expected: Find wrapper contract implementation and any direct interactions with it
# Search for wrapper contract implementation
ast-grep --pattern 'contract $_ is $_* {
$$$
function wrap($$$) {
$$$
}
$$$
}'
# Search for direct wrapper contract interactions
rg -A 5 'wrapper.*wrap'
Length of output: 475
| @@ -112,7 +116,7 @@ export const erc4626Vaults: Erc4626Config[] = [ | |||
| networkId: NetworkIds.MAINNET, | |||
| pricePicker: morphoPricePicker, | |||
| protocol: LendingProtocol.MorphoBlue, | |||
| rewards: [morphoRewards, wstethRewards], | |||
| rewards: [morphoRewards, wstethRewards, legacyMorphoRewards], | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent reward order in one MetaMorpho vault.
- The
Flagship ETHvault includeswstethRewardsin its rewards array, differing from other vaults.
Consider standardizing the rewards array across all MetaMorpho vaults for consistency.
🔗 Analysis chain
Verify completeness of vault updates and consider reward order consistency.
- The updates consistently add
legacyMorphoRewardsto all MetaMorpho vaults, which aligns with the PR objectives. - Consider standardizing the order of rewards across all vaults for consistency (e.g., always placing
legacyMorphoRewardsin the same position relative to other rewards).
Let's verify if all MetaMorpho vaults are properly updated:
Also applies to: 136-136, 153-153, 170-170, 187-187, 204-204, 221-221
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all MetaMorpho vaults include legacy rewards
# Expected: All vaults with rewardsType: Erc4626RewardsType.MetaMorpho should include MORPHO_LEGACY
# Search for MetaMorpho vault configurations
echo "Checking MetaMorpho vault configurations..."
rg "rewardsType:\s+Erc4626RewardsType\.MetaMorpho" -B 5 -A 5
# Search for any remaining references to MORPHO rewards that might need updating
echo "Checking for other MORPHO reward references..."
rg "rewards.*MORPHO" --type ts
Length of output: 7179
Script:
#!/bin/bash
# Description: Verify the order of rewards in all MetaMorpho vaults
# Expected: All MetaMorpho vaults should have 'legacyMorphoRewards' in the same position within the rewards array
echo "Extracting rewards order for MetaMorpho vaults..."
rg "rewardsType:\s+Erc4626RewardsType\.MetaMorpho" -B 10 -A 20 --type ts | grep -E "rewards:\s*\[.*\]" | awk -F'[' '{print $2}' | awk -F']' '{print $1}' | sort | uniq -c
Length of output: 381
| export async function encodeApproveAndWrapProxyAction({ | ||
| networkId, | ||
| oldToken, | ||
| newToken, | ||
| wrapper, | ||
| amount, | ||
| }: { | ||
| networkId: NetworkIds | ||
| oldToken: string | ||
| newToken: string | ||
| wrapper: string | ||
| amount: BigNumber | ||
| }): Promise<OmniTxData> { | ||
| const contracts = getNetworkContracts(networkId) | ||
|
|
||
| ensureContractsExist(networkId, contracts, ['erc20ProxyActions']) | ||
| ensureGivenTokensExist(networkId, contracts, [oldToken, newToken]) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add essential input validation.
The function should validate the wrapper address format and verify that required contracts exist.
export async function encodeApproveAndWrapProxyAction({
networkId,
oldToken,
newToken,
wrapper,
amount,
}: {
networkId: NetworkIds
oldToken: string
newToken: string
wrapper: string
amount: BigNumber
}): Promise<OmniTxData> {
+ if (!ethers.utils.isAddress(wrapper)) {
+ throw new Error('Invalid wrapper address format')
+ }
+
const contracts = getNetworkContracts(networkId)
ensureContractsExist(networkId, contracts, ['erc20ProxyActions'])
ensureGivenTokensExist(networkId, contracts, [oldToken, newToken])📝 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.
| export async function encodeApproveAndWrapProxyAction({ | |
| networkId, | |
| oldToken, | |
| newToken, | |
| wrapper, | |
| amount, | |
| }: { | |
| networkId: NetworkIds | |
| oldToken: string | |
| newToken: string | |
| wrapper: string | |
| amount: BigNumber | |
| }): Promise<OmniTxData> { | |
| const contracts = getNetworkContracts(networkId) | |
| ensureContractsExist(networkId, contracts, ['erc20ProxyActions']) | |
| ensureGivenTokensExist(networkId, contracts, [oldToken, newToken]) | |
| export async function encodeApproveAndWrapProxyAction({ | |
| networkId, | |
| oldToken, | |
| newToken, | |
| wrapper, | |
| amount, | |
| }: { | |
| networkId: NetworkIds | |
| oldToken: string | |
| newToken: string | |
| wrapper: string | |
| amount: BigNumber | |
| }): Promise<OmniTxData> { | |
| if (!ethers.utils.isAddress(wrapper)) { | |
| throw new Error('Invalid wrapper address format') | |
| } | |
| const contracts = getNetworkContracts(networkId) | |
| ensureContractsExist(networkId, contracts, ['erc20ProxyActions']) | |
| ensureGivenTokensExist(networkId, contracts, [oldToken, newToken]) |
| console.log('morphoLegacyToken', morphoLegacyToken) | ||
| if (morphoLegacyToken) { | ||
| console.log('morphoLegacyToken', morphoLegacyToken) | ||
| const network = networkIdToLibraryNetwork(networkId) | ||
| if ( | ||
| protocol === LendingProtocol.AaveV3 && | ||
| network !== Network.HARDHAT && | ||
| network !== Network.LOCAL && | ||
| network !== Network.TENDERLY | ||
| ) { | ||
| rewardsControllerAddress = ADDRESSES[network].aave.v3.RewardsController | ||
| poolDataProviderAddress = ADDRESSES[network].aave.v3.PoolDataProvider | ||
| } else if ( | ||
| protocol === LendingProtocol.SparkV3 && | ||
| network !== Network.HARDHAT && | ||
| network !== Network.LOCAL && | ||
| network !== Network.TENDERLY | ||
| ) { | ||
| rewardsControllerAddress = ADDRESSES[network].spark.RewardsController | ||
| poolDataProviderAddress = ADDRESSES[network].spark.PoolDataProvider | ||
| } else { | ||
| console.warn(`Unsupported protocol or network for rewards: ${protocol} on ${network}`) | ||
| throw new Error(`Unsupported protocol or network for rewards: ${protocol} on ${network}`) | ||
| console.log('network', network) | ||
| if (network === Network.MAINNET) { | ||
| tokenBalance({ token: morphoLegacyToken, account: dpmProxy, networkId }) | ||
| .then((balance) => { | ||
| console.log('balance mmm', balance) |
There was a problem hiding this comment.
Remove debugging console.log statements
The console.log statements at lines 96, 98, 100, and 104 are likely meant for debugging purposes and should be removed to keep the production code clean and to pass the linter checks.
Apply this diff to remove the unnecessary console.log statements:
96,98,100,104
- console.log('morphoLegacyToken', morphoLegacyToken)
- console.log('morphoLegacyToken', morphoLegacyToken)
- console.log('network', network)
- console.log('balance mmm', balance)Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: Linter
[failure] 96-96:
Unexpected console statement
[failure] 98-98:
Unexpected console statement
[failure] 100-100:
Unexpected console statement
[failure] 104-104:
Unexpected console statement
🪛 eslint
[error] 96-96: Unexpected console statement.
(no-console)
[error] 98-98: Unexpected console statement.
(no-console)
[error] 100-100: Unexpected console statement.
(no-console)
[error] 104-104: Unexpected console statement.
(no-console)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
features/omni-kit/components/details-section/OmniDetailSectionRewardsClaims.tsx (3)
Line range hint
30-47: Consider moving network configurations to a dedicated config fileThe network-specific token configurations should be moved to a dedicated configuration file to improve maintainability and follow separation of concerns.
Consider creating a new file like
config/network-tokens.tsand moving bothclaimableErc20ByNetworkandmorphoLegacyByNetworkthere.
Line range hint
69-93: Consider implementing proper error logging and parallelizing token balance checksThe error handling could be improved by:
- Replacing console.error with proper error logging
- Adding error boundaries for React error handling
Additionally, token balance checks could be parallelized for better performance.
Consider this implementation:
if (isEligibleForErc20Claims) { - claimableErc20ByNetwork[networkId].forEach((token) => { - tokenBalance({ token, account: dpmProxy, networkId: networkId }) + Promise.all( + claimableErc20ByNetwork[networkId].map((token) => + tokenBalance({ token, account: dpmProxy, networkId: networkId }) + .then((balance) => ({ token, balance })) + .catch((error) => { + logger.error(`Error fetching token balance for ${token}:`, error) + return { token, balance: zero } + }) + ) + ).then((results) => { + results.forEach(({ token, balance }) => { if (balance.gt(zero)) { encodeTransferToOwnerProxyAction({ token, networkId, amount: balance, dpmAccount: dpmProxy, }) .then((tx) => { dispatchClaim({ token, claimable: balance, tx }) }) .catch((error) => { - console.error(`Error encoding transfer all proxy action for ${token}: ${error}`) + logger.error(`Error encoding transfer all proxy action for ${token}:`, error) }) } - }) - }) + }))
205-217: Move rewards eligible tokens to configurationThe hardcoded list of rewards eligible tokens should be moved to a configuration file for better maintainability.
Consider moving
rewardsEligibleTokensto the same configuration file suggested earlier:- const rewardsEligibleTokens = ['SUSDE', 'USDE', 'WETH', 'ETH'] + import { rewardsEligibleTokens } from 'config/network-tokens'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
features/omni-kit/components/details-section/OmniDetailSectionRewardsClaims.tsx(6 hunks)features/reclaimCollateral/reclaimCollateralView.tsx(0 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- features/reclaimCollateral/reclaimCollateralView.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
📓 Learnings (1)
features/omni-kit/components/details-section/OmniDetailSectionRewardsClaims.tsx (1)
Learnt from: halaprix
PR: OasisDEX/oasis-borrow#4046
File: features/omni-kit/components/details-section/OmniDetailSectionRewardsClaims.tsx:53-53
Timestamp: 2024-11-12T11:56:49.827Z
Learning: In TypeScript code, we rely on TypeScript's type checking to catch cases where `NetworkIds` are modified and used as an index, ensuring that accessing `claimableErc20[networkId]` is safe.
| if (isEligibleForMorphoLegacy) { | ||
| const morphoLegacyToken = morphoLegacyByNetwork[networkId] | ||
| if (morphoLegacyToken) { | ||
| const network = networkIdToLibraryNetwork(networkId) | ||
| if ( | ||
| protocol === LendingProtocol.AaveV3 && | ||
| network !== Network.HARDHAT && | ||
| network !== Network.LOCAL && | ||
| network !== Network.TENDERLY | ||
| ) { | ||
| rewardsControllerAddress = ADDRESSES[network].aave.v3.RewardsController | ||
| poolDataProviderAddress = ADDRESSES[network].aave.v3.PoolDataProvider | ||
| } else if ( | ||
| protocol === LendingProtocol.SparkV3 && | ||
| network !== Network.HARDHAT && | ||
| network !== Network.LOCAL && | ||
| network !== Network.TENDERLY | ||
| ) { | ||
| rewardsControllerAddress = ADDRESSES[network].spark.RewardsController | ||
| poolDataProviderAddress = ADDRESSES[network].spark.PoolDataProvider | ||
| } else { | ||
| console.warn(`Unsupported protocol or network for rewards: ${protocol} on ${network}`) | ||
| throw new Error(`Unsupported protocol or network for rewards: ${protocol} on ${network}`) | ||
| if (network === Network.MAINNET) { | ||
| tokenBalance({ token: morphoLegacyToken, account: dpmProxy, networkId }) | ||
| .then((balance) => { | ||
| if (balance.gt(zero)) { | ||
| encodeApproveAndWrapProxyAction({ | ||
| oldToken: morphoLegacyToken, | ||
| newToken: 'MORPHO', | ||
| wrapper: ADDRESSES[network].morphoblue.Wrapper, | ||
| amount: balance, | ||
| networkId, | ||
| }) | ||
| .then((tx) => { | ||
| dispatchClaim({ token: 'MORPHO', claimable: balance, tx }) | ||
| }) | ||
| .catch((error) => { | ||
| console.error( | ||
| `Error encoding approve and wrap action for MORPHO_LEGACY: ${error}`, | ||
| ) | ||
| }) | ||
| } | ||
| }) | ||
| .catch((error) => { | ||
| console.error(`Error fetching MORPHO_LEGACY balance: ${error}`) | ||
| }) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and type safety in Morpho legacy token handling
The Morpho legacy token handling could be improved with better type safety and error handling.
Consider this implementation:
if (isEligibleForMorphoLegacy) {
const morphoLegacyToken = morphoLegacyByNetwork[networkId]
- if (morphoLegacyToken) {
+ if (!morphoLegacyToken) {
+ logger.warn(`No Morpho legacy token configured for network ${networkId}`)
+ return
+ }
const network = networkIdToLibraryNetwork(networkId)
- if (network === Network.MAINNET) {
- tokenBalance({ token: morphoLegacyToken, account: dpmProxy, networkId })
- .then((balance) => {
- if (balance.gt(zero)) {
- encodeApproveAndWrapProxyAction({
- oldToken: morphoLegacyToken,
- newToken: 'MORPHO',
- wrapper: ADDRESSES[network].morphoblue.Wrapper,
- amount: balance,
- networkId,
- })
- .then((tx) => {
- dispatchClaim({ token: 'MORPHO', claimable: balance, tx })
- })
- .catch((error) => {
- console.error(
- `Error encoding approve and wrap action for MORPHO_LEGACY: ${error}`,
- )
- })
- }
- })
- .catch((error) => {
- console.error(`Error fetching MORPHO_LEGACY balance: ${error}`)
- })
+ if (network !== Network.MAINNET) {
+ logger.warn(`Morpho legacy token wrapping is only supported on mainnet`)
+ return
}
+ try {
+ const balance = await tokenBalance({ token: morphoLegacyToken, account: dpmProxy, networkId })
+ if (balance.gt(zero)) {
+ try {
+ const tx = await encodeApproveAndWrapProxyAction({
+ oldToken: morphoLegacyToken,
+ newToken: 'MORPHO',
+ wrapper: ADDRESSES[network].morphoblue.Wrapper,
+ amount: balance,
+ networkId,
+ })
+ dispatchClaim({ token: 'MORPHO', claimable: balance, tx })
+ } catch (error) {
+ logger.error(`Error encoding approve and wrap action for MORPHO_LEGACY:`, error)
+ }
+ }
+ } catch (error) {
+ logger.error(`Error fetching MORPHO_LEGACY balance:`, error)
+ }
}📝 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 (isEligibleForMorphoLegacy) { | |
| const morphoLegacyToken = morphoLegacyByNetwork[networkId] | |
| if (morphoLegacyToken) { | |
| const network = networkIdToLibraryNetwork(networkId) | |
| if ( | |
| protocol === LendingProtocol.AaveV3 && | |
| network !== Network.HARDHAT && | |
| network !== Network.LOCAL && | |
| network !== Network.TENDERLY | |
| ) { | |
| rewardsControllerAddress = ADDRESSES[network].aave.v3.RewardsController | |
| poolDataProviderAddress = ADDRESSES[network].aave.v3.PoolDataProvider | |
| } else if ( | |
| protocol === LendingProtocol.SparkV3 && | |
| network !== Network.HARDHAT && | |
| network !== Network.LOCAL && | |
| network !== Network.TENDERLY | |
| ) { | |
| rewardsControllerAddress = ADDRESSES[network].spark.RewardsController | |
| poolDataProviderAddress = ADDRESSES[network].spark.PoolDataProvider | |
| } else { | |
| console.warn(`Unsupported protocol or network for rewards: ${protocol} on ${network}`) | |
| throw new Error(`Unsupported protocol or network for rewards: ${protocol} on ${network}`) | |
| if (network === Network.MAINNET) { | |
| tokenBalance({ token: morphoLegacyToken, account: dpmProxy, networkId }) | |
| .then((balance) => { | |
| if (balance.gt(zero)) { | |
| encodeApproveAndWrapProxyAction({ | |
| oldToken: morphoLegacyToken, | |
| newToken: 'MORPHO', | |
| wrapper: ADDRESSES[network].morphoblue.Wrapper, | |
| amount: balance, | |
| networkId, | |
| }) | |
| .then((tx) => { | |
| dispatchClaim({ token: 'MORPHO', claimable: balance, tx }) | |
| }) | |
| .catch((error) => { | |
| console.error( | |
| `Error encoding approve and wrap action for MORPHO_LEGACY: ${error}`, | |
| ) | |
| }) | |
| } | |
| }) | |
| .catch((error) => { | |
| console.error(`Error fetching MORPHO_LEGACY balance: ${error}`) | |
| }) | |
| } | |
| } | |
| } | |
| if (isEligibleForMorphoLegacy) { | |
| const morphoLegacyToken = morphoLegacyByNetwork[networkId] | |
| if (!morphoLegacyToken) { | |
| logger.warn(`No Morpho legacy token configured for network ${networkId}`) | |
| return | |
| } | |
| const network = networkIdToLibraryNetwork(networkId) | |
| if (network !== Network.MAINNET) { | |
| logger.warn(`Morpho legacy token wrapping is only supported on mainnet`) | |
| return | |
| } | |
| try { | |
| const balance = await tokenBalance({ token: morphoLegacyToken, account: dpmProxy, networkId }) | |
| if (balance.gt(zero)) { | |
| try { | |
| const tx = await encodeApproveAndWrapProxyAction({ | |
| oldToken: morphoLegacyToken, | |
| newToken: 'MORPHO', | |
| wrapper: ADDRESSES[network].morphoblue.Wrapper, | |
| amount: balance, | |
| networkId, | |
| }) | |
| dispatchClaim({ token: 'MORPHO', claimable: balance, tx }) | |
| } catch (error) { | |
| logger.error(`Error encoding approve and wrap action for MORPHO_LEGACY:`, error) | |
| } | |
| } | |
| } catch (error) { | |
| logger.error(`Error fetching MORPHO_LEGACY balance:`, error) | |
| } | |
| } |
| if (isEligibleForProtocolRewards) { | ||
| let rewardsControllerAddress: string | undefined | ||
| let poolDataProviderAddress: string | undefined | ||
| const network = networkIdToLibraryNetwork(networkId) | ||
| if ( | ||
| protocol === LendingProtocol.AaveV3 && | ||
| network !== Network.HARDHAT && | ||
| network !== Network.LOCAL && | ||
| network !== Network.TENDERLY | ||
| ) { | ||
| rewardsControllerAddress = ADDRESSES[network].aave.v3.RewardsController | ||
| poolDataProviderAddress = ADDRESSES[network].aave.v3.PoolDataProvider | ||
| } else if ( | ||
| protocol === LendingProtocol.SparkV3 && | ||
| network !== Network.HARDHAT && | ||
| network !== Network.LOCAL && | ||
| network !== Network.TENDERLY | ||
| ) { | ||
| rewardsControllerAddress = ADDRESSES[network].spark.RewardsController | ||
| poolDataProviderAddress = ADDRESSES[network].spark.PoolDataProvider | ||
| } else { | ||
| console.warn(`Unsupported protocol or network for rewards: ${protocol} on ${network}`) | ||
| throw new Error(`Unsupported protocol or network for rewards: ${protocol} on ${network}`) | ||
| } | ||
|
|
||
| getAllUserRewards({ | ||
| networkId, | ||
| token: quoteAddress, | ||
| account: dpmProxy, | ||
| rewardsController: rewardsControllerAddress as string, | ||
| poolDataProvider: poolDataProviderAddress as string, | ||
| }) | ||
| .then(async ({ rewardsList, unclaimedAmounts, assets }) => { | ||
| if (unclaimedAmounts.some((amount) => amount.gt(zero))) { | ||
| const tx = encodeClaimAllRewards({ | ||
| networkId, | ||
| assets: assets as string[], | ||
| dpmAccount: dpmProxy, | ||
| rewardsController: rewardsControllerAddress as string, | ||
| }) | ||
| getAllUserRewards({ | ||
| networkId, | ||
| token: quoteAddress, | ||
| account: dpmProxy, | ||
| rewardsController: rewardsControllerAddress as string, | ||
| poolDataProvider: poolDataProviderAddress as string, | ||
| }) | ||
| .then(async ({ rewardsList, unclaimedAmounts, assets }) => { | ||
| if (unclaimedAmounts.some((amount) => amount.gt(zero))) { | ||
| const tx = encodeClaimAllRewards({ | ||
| networkId, | ||
| assets: assets as string[], | ||
| dpmAccount: dpmProxy, | ||
| rewardsController: rewardsControllerAddress as string, | ||
| }) | ||
|
|
||
| rewardsList.forEach((token, index) => { | ||
| if (unclaimedAmounts[index].gt(zero)) { | ||
| dispatchClaim({ | ||
| token: getTokenByAddress(token, networkId).symbol, | ||
| claimable: unclaimedAmounts[index], | ||
| tx, | ||
| }) | ||
| } | ||
| }) | ||
| } | ||
| }) | ||
| .catch((error) => { | ||
| console.error(`Error fetching ${protocol} rewards:`, error) | ||
| }) | ||
| } | ||
| rewardsList.forEach((token, index) => { | ||
| if (unclaimedAmounts[index].gt(zero)) { | ||
| dispatchClaim({ | ||
| token: getTokenByAddress(token, networkId).symbol, | ||
| claimable: unclaimedAmounts[index], | ||
| tx, | ||
| }) | ||
| } | ||
| }) | ||
| } | ||
| }) | ||
| .catch((error) => { | ||
| console.error(`Error fetching ${protocol} rewards:`, error) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve protocol rewards handling with early validation and proper error handling
The protocol rewards handling could be improved with early validation and proper error handling.
Consider this implementation:
if (isEligibleForProtocolRewards) {
+ const network = networkIdToLibraryNetwork(networkId)
+ if ([Network.HARDHAT, Network.LOCAL, Network.TENDERLY].includes(network)) {
+ logger.warn(`Protocol rewards not supported on network: ${network}`)
+ return
+ }
+
let rewardsControllerAddress: string | undefined
let poolDataProviderAddress: string | undefined
- const network = networkIdToLibraryNetwork(networkId)
- if (
- protocol === LendingProtocol.AaveV3 &&
- network !== Network.HARDHAT &&
- network !== Network.LOCAL &&
- network !== Network.TENDERLY
- ) {
- rewardsControllerAddress = ADDRESSES[network].aave.v3.RewardsController
- poolDataProviderAddress = ADDRESSES[network].aave.v3.PoolDataProvider
- } else if (
- protocol === LendingProtocol.SparkV3 &&
- network !== Network.HARDHAT &&
- network !== Network.LOCAL &&
- network !== Network.TENDERLY
- ) {
- rewardsControllerAddress = ADDRESSES[network].spark.RewardsController
- poolDataProviderAddress = ADDRESSES[network].spark.PoolDataProvider
- } else {
- console.warn(`Unsupported protocol or network for rewards: ${protocol} on ${network}`)
+
+ switch (protocol) {
+ case LendingProtocol.AaveV3:
+ rewardsControllerAddress = ADDRESSES[network].aave.v3.RewardsController
+ poolDataProviderAddress = ADDRESSES[network].aave.v3.PoolDataProvider
+ break
+ case LendingProtocol.SparkV3:
+ rewardsControllerAddress = ADDRESSES[network].spark.RewardsController
+ poolDataProviderAddress = ADDRESSES[network].spark.PoolDataProvider
+ break
+ default:
+ logger.warn(`Unsupported protocol for rewards: ${protocol}`)
+ return
}
+
+ if (!rewardsControllerAddress || !poolDataProviderAddress) {
+ logger.error(`Missing contract addresses for protocol ${protocol} on network ${network}`)
throw new Error(`Unsupported protocol or network for rewards: ${protocol} on ${network}`)
}📝 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 (isEligibleForProtocolRewards) { | |
| let rewardsControllerAddress: string | undefined | |
| let poolDataProviderAddress: string | undefined | |
| const network = networkIdToLibraryNetwork(networkId) | |
| if ( | |
| protocol === LendingProtocol.AaveV3 && | |
| network !== Network.HARDHAT && | |
| network !== Network.LOCAL && | |
| network !== Network.TENDERLY | |
| ) { | |
| rewardsControllerAddress = ADDRESSES[network].aave.v3.RewardsController | |
| poolDataProviderAddress = ADDRESSES[network].aave.v3.PoolDataProvider | |
| } else if ( | |
| protocol === LendingProtocol.SparkV3 && | |
| network !== Network.HARDHAT && | |
| network !== Network.LOCAL && | |
| network !== Network.TENDERLY | |
| ) { | |
| rewardsControllerAddress = ADDRESSES[network].spark.RewardsController | |
| poolDataProviderAddress = ADDRESSES[network].spark.PoolDataProvider | |
| } else { | |
| console.warn(`Unsupported protocol or network for rewards: ${protocol} on ${network}`) | |
| throw new Error(`Unsupported protocol or network for rewards: ${protocol} on ${network}`) | |
| } | |
| getAllUserRewards({ | |
| networkId, | |
| token: quoteAddress, | |
| account: dpmProxy, | |
| rewardsController: rewardsControllerAddress as string, | |
| poolDataProvider: poolDataProviderAddress as string, | |
| }) | |
| .then(async ({ rewardsList, unclaimedAmounts, assets }) => { | |
| if (unclaimedAmounts.some((amount) => amount.gt(zero))) { | |
| const tx = encodeClaimAllRewards({ | |
| networkId, | |
| assets: assets as string[], | |
| dpmAccount: dpmProxy, | |
| rewardsController: rewardsControllerAddress as string, | |
| }) | |
| getAllUserRewards({ | |
| networkId, | |
| token: quoteAddress, | |
| account: dpmProxy, | |
| rewardsController: rewardsControllerAddress as string, | |
| poolDataProvider: poolDataProviderAddress as string, | |
| }) | |
| .then(async ({ rewardsList, unclaimedAmounts, assets }) => { | |
| if (unclaimedAmounts.some((amount) => amount.gt(zero))) { | |
| const tx = encodeClaimAllRewards({ | |
| networkId, | |
| assets: assets as string[], | |
| dpmAccount: dpmProxy, | |
| rewardsController: rewardsControllerAddress as string, | |
| }) | |
| rewardsList.forEach((token, index) => { | |
| if (unclaimedAmounts[index].gt(zero)) { | |
| dispatchClaim({ | |
| token: getTokenByAddress(token, networkId).symbol, | |
| claimable: unclaimedAmounts[index], | |
| tx, | |
| }) | |
| } | |
| }) | |
| } | |
| }) | |
| .catch((error) => { | |
| console.error(`Error fetching ${protocol} rewards:`, error) | |
| }) | |
| } | |
| rewardsList.forEach((token, index) => { | |
| if (unclaimedAmounts[index].gt(zero)) { | |
| dispatchClaim({ | |
| token: getTokenByAddress(token, networkId).symbol, | |
| claimable: unclaimedAmounts[index], | |
| tx, | |
| }) | |
| } | |
| }) | |
| } | |
| }) | |
| .catch((error) => { | |
| console.error(`Error fetching ${protocol} rewards:`, error) | |
| }) | |
| if (isEligibleForProtocolRewards) { | |
| const network = networkIdToLibraryNetwork(networkId) | |
| if ([Network.HARDHAT, Network.LOCAL, Network.TENDERLY].includes(network)) { | |
| logger.warn(`Protocol rewards not supported on network: ${network}`) | |
| return | |
| } | |
| let rewardsControllerAddress: string | undefined | |
| let poolDataProviderAddress: string | undefined | |
| switch (protocol) { | |
| case LendingProtocol.AaveV3: | |
| rewardsControllerAddress = ADDRESSES[network].aave.v3.RewardsController | |
| poolDataProviderAddress = ADDRESSES[network].aave.v3.PoolDataProvider | |
| break | |
| case LendingProtocol.SparkV3: | |
| rewardsControllerAddress = ADDRESSES[network].spark.RewardsController | |
| poolDataProviderAddress = ADDRESSES[network].spark.PoolDataProvider | |
| break | |
| default: | |
| logger.warn(`Unsupported protocol for rewards: ${protocol}`) | |
| return | |
| } | |
| if (!rewardsControllerAddress || !poolDataProviderAddress) { | |
| logger.error(`Missing contract addresses for protocol ${protocol} on network ${network}`) | |
| throw new Error(`Unsupported protocol or network for rewards: ${protocol} on ${network}`) | |
| } | |
| getAllUserRewards({ | |
| networkId, | |
| token: quoteAddress, | |
| account: dpmProxy, | |
| rewardsController: rewardsControllerAddress as string, | |
| poolDataProvider: poolDataProviderAddress as string, | |
| }) | |
| .then(async ({ rewardsList, unclaimedAmounts, assets }) => { | |
| if (unclaimedAmounts.some((amount) => amount.gt(zero))) { | |
| const tx = encodeClaimAllRewards({ | |
| networkId, | |
| assets: assets as string[], | |
| dpmAccount: dpmProxy, | |
| rewardsController: rewardsControllerAddress as string, | |
| }) | |
| rewardsList.forEach((token, index) => { | |
| if (unclaimedAmounts[index].gt(zero)) { | |
| dispatchClaim({ | |
| token: getTokenByAddress(token, networkId).symbol, | |
| claimable: unclaimedAmounts[index], | |
| tx, | |
| }) | |
| } | |
| }) | |
| } | |
| }) | |
| .catch((error) => { | |
| console.error(`Error fetching ${protocol} rewards:`, error) | |
| }) |
Add MORPHO Legacy token migration support
Changes 👷♀️
approveAndWrapfunction to ERC20 proxy actions to support MORPHO legacy token migrationencodeApproveAndWrapProxyActionin better-calls/erc20.tsHow to test 🧪
The PR implements support for migrating legacy MORPHO tokens to the new MORPHO token format using OpenZeppelin's ERC20Wrapper pattern. This allows users to easily upgrade their tokens through the UI while interacting with MetaMorpho vaults.
Summary by CodeRabbit
Release Notes
New Features
approveAndWrapfunction for enhanced token approval and wrapping.MORPHO_LEGACYtoken, expanding token management capabilities.legacyMorphoRewardsin reward distributions.Bug Fixes
Chores
@oasisdex/addressespackage dependency to the latest version.