feat: enhance deployment configurations with address profiles; mFONE … - #213
feat: enhance deployment configurations with address profiles; mFONE …#213dmytro-horbatenko wants to merge 1 commit into
Conversation
…Unloop deployment
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a 'deployment profiles' system to support alternative configurations and address sets for tokens, specifically adding the mfone-unloop profile. The changes involve updating deployment scripts, role management, and address book tasks to resolve addresses and configurations through these profiles. Feedback suggests ensuring that swapper vault address lookups are also profile-aware, improving the robustness of chainId retrieval, and verifying if the change from mandatory to optional manager role assignments in the roles script was intentional.
| } else { | ||
| swapperVaultAddress = | ||
| addresses[swapperVault.mToken]?.[swapperVault.redemptionVaultType]; | ||
| addresses?.[swapperVault.mToken]?.[swapperVault.redemptionVaultType]; |
There was a problem hiding this comment.
While this change correctly adds optional chaining to prevent a crash, it does not account for the new 'address profiles' feature. If the swapperVault was deployed under a specific profile, its address should be retrieved using getDeploymentTokenAddresses rather than directly from the base addresses object. This ensures consistency when multiple deployment profiles are used for different tokens.
| ) => { | ||
| const { grantRoles: networkConfig } = getNetworkConfig( | ||
| hre, | ||
| const chainId = hre.network.config.chainId!; |
There was a problem hiding this comment.
Using the non-null assertion operator (!) on chainId can lead to a runtime crash if the network configuration is incomplete. It is safer to handle the case where chainId might be undefined or retrieve it directly from the provider.
| const chainId = hre.network.config.chainId!; | |
| const chainId = hre.network.config.chainId ?? (await hre.ethers.provider.getNetwork()).chainId; |
| const addressBatch: string[] = []; | ||
|
|
||
| // Token / vault / oracle managers | ||
| if (managerGrantConfig) { |
There was a problem hiding this comment.
The logic for granting manager roles has changed from being mandatory (throwing an error if missing) to being optional (silently skipping if managerGrantConfig is undefined). If this change in behavior is unintended, it could lead to incomplete deployments where critical roles are not assigned to the expected manager addresses.
1 participant
…Unloop deployment