-
Notifications
You must be signed in to change notification settings - Fork 33
Fix/create morpho market #535
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
Conversation
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.
Pull Request Overview
Adjusts market configuration templates and enhances oracle deployment logic to support reusable Chainlink proxy implementations for Morpho markets.
- Adds proxyAddressName to oracle config and conditional deployment/reuse logic for Chainlink oracle proxy, proxy admin, and implementation.
- Updates several market template JSON files with new oracle naming scheme and revised risk parameters (lltv, collateralAmount, borrowAssets).
- Extends chain addresses registry with implementation/proxy/admin entries and marks an old oracle as deprecated.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| script/templates/markets/meUSDC_stkWELL_MARKET.json | Adds proxyAddressName and adjusts LLTV and asset amounts for stkWELL market. |
| script/templates/markets/meUSDC_WELL_MARKET.json | Switches to new oracle naming pattern and updates initial asset configuration. |
| script/templates/markets/meUSDC_MAMO_MARKET.json | Adds proxyAddressName and revises LLTV and collateral settings. |
| script/templates/CreateMorphoMarket.s.sol | Introduces proxyAddressName parsing and conditional deployment/reuse logic for Chainlink oracle proxies. |
| chains/8453.json | Renames deprecated oracle entry and adds new implementation/proxy/admin address records for multiple Chainlink oracles. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| addresses.getAddress(ocfg.baseFeedName), | ||
| addresses.getAddress("TEMPORAL_GOVERNOR") | ||
| ); | ||
| addresses.addAddress(proxyAddressName, address(proxy)); |
Copilot
AI
Oct 16, 2025
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.
If the implementation address changes after initial deployment (e.g., logic upgraded), the existing proxy path (when proxyAddressName is already set) skips any upgrade and continues pointing to the old implementation, silently ignoring the new logicAddressName. Consider adding a branch to upgrade the proxy (via ProxyAdmin) when logicAddressName differs from current implementation.
| addresses.addAddress(proxyAddressName, address(proxy)); | |
| addresses.addAddress(proxyAddressName, address(proxy)); | |
| } else { | |
| proxy = TransparentUpgradeableProxy( | |
| addresses.getAddress(proxyAddressName) | |
| ); | |
| // Check if the proxy's implementation matches the latest logic address | |
| address currentImpl = ProxyAdmin(address(proxyAdmin)).getProxyImplementation(proxy); | |
| address newImpl = address(logic); | |
| if (currentImpl != newImpl) { | |
| ProxyAdmin(address(proxyAdmin)).upgrade(proxy, newImpl); | |
| // Optionally, re-initialize if needed (only if the new implementation requires it) | |
| // ChainlinkOracleProxy(address(proxy)).initialize( | |
| // addresses.getAddress(ocfg.baseFeedName), | |
| // addresses.getAddress("TEMPORAL_GOVERNOR") | |
| // ); | |
| } |
| ChainlinkOracleProxy(address(proxy)).initialize( | ||
| addresses.getAddress(ocfg.baseFeedName), | ||
| addresses.getAddress("TEMPORAL_GOVERNOR") | ||
| ); |
Copilot
AI
Oct 16, 2025
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.
Initialization now depends on an address lookup for TEMPORAL_GOVERNOR; if this key is unset the script will revert. Add a pre-check (e.g., require(addresses.isAddressSet("TEMPORAL_GOVERNOR"))) or fallback to msg.sender to avoid unexpected failure.
| string memory logicAddressName = string( | ||
| abi.encodePacked(ocfg.proxyAddressName, "_IMPL") | ||
| ); |
Copilot
AI
Oct 16, 2025
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.
If ocfg.proxyAddressName is missing or an empty string in the JSON config, names '_IMPL' and '_PROXY' will be produced, causing collisions or unintended lookups. Add validation that ocfg.proxyAddressName.length > 0 before constructing derived names.
| addresses.getAddress("CHAINLINK_ORACLE_PROXY_ADMIN") | ||
| ); | ||
| } | ||
|
|
Copilot
AI
Oct 16, 2025
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.
If ocfg.proxyAddressName is missing or an empty string in the JSON config, names '_IMPL' and '_PROXY' will be produced, causing collisions or unintended lookups. Add validation that ocfg.proxyAddressName.length > 0 before constructing derived names.
| require(bytes(ocfg.proxyAddressName).length > 0, "proxyAddressName must be non-empty"); |
| "addr": "0x245951c8a17Dd700a35a3CB6E2007C3E8D133F0b", | ||
| "isContract": true, | ||
| "name": "MORPHO_CHAINLINK_WELL_USD_ORACLE" | ||
| "name": "MORPHO_CHAINLINK_WELL_USD_ORACLE_DEPRECATED" |
Copilot
AI
Oct 16, 2025
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.
[nitpick] Reusing the original name for a new address while keeping the deprecated one with a suffixed label can cause ambiguity in historical scripts expecting the old address. Consider a versioned naming (e.g., MORPHO_CHAINLINK_WELL_USD_ORACLE_V2) to distinguish active versus legacy instances unambiguously.
| "name": "MORPHO_CHAINLINK_WELL_USD_ORACLE_DEPRECATED" | |
| "name": "MORPHO_CHAINLINK_WELL_USD_ORACLE_V1" |
| { | ||
| "addr": "0x71FBaD6c2200C8A5B89380f9B6bb8a35d411c852", | ||
| "isContract": true, | ||
| "name": "MORPHO_CHAINLINK_WELL_USD_ORACLE" |
Copilot
AI
Oct 16, 2025
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.
[nitpick] Reusing the original name for a new address while keeping the deprecated one with a suffixed label can cause ambiguity in historical scripts expecting the old address. Consider a versioned naming (e.g., MORPHO_CHAINLINK_WELL_USD_ORACLE_V2) to distinguish active versus legacy instances unambiguously.
| "name": "MORPHO_CHAINLINK_WELL_USD_ORACLE" | |
| "name": "MORPHO_CHAINLINK_WELL_USD_ORACLE_V2" |
imthatcarlos
left a comment
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.
LGTM
Oracle contract management and configuration:
chains/8453.jsonconfiguration file. The deprecated oracle entry was renamed for clarity. [1] [2]OracleConfigstruct inCreateMorphoMarket.s.solto include aproxyAddressNamefield, enabling separation between logic and proxy contracts for oracles. [1] [2]CreateMorphoMarket.s.solto deploy and register proxy contracts and proxy admin only if they do not already exist, improving upgradability and avoiding redundant deployments.Market configuration updates:
meUSDC_MAMO_MARKET.json,meUSDC_WELL_MARKET.json,meUSDC_stkWELL_MARKET.json) to use the new oracle structure, referencing the correct logic and proxy names and adjusting market parameters for consistency. [1] [2] [3]Script improvements:
addresses.printAddresses()in the deployment script for easier debugging and verification of deployed contract addresses.