Conversation
- Implemented the MidasArk contract for managing token supply and yield generation through Midas vaults. - Added interfaces for DepositVault, RedemptionVault, MidasOracle, and MidasWithdrawalManager to facilitate interactions with the Midas protocol. - Created a test suite for MidasArk to validate functionality including asset boarding, withdrawal requests, and total asset calculations using oracle prices. - Established error handling for invalid vault addresses and ensured proper initialization of state variables.
…nstants - Added RequestStatus import to manage redemption request states. - Updated assetsInWithdrawalQueue function to check for pending requests before returning assets. - Refactored requestWithdrawal function to prevent multiple withdrawal requests. - Introduced MIDAS_ROUNDING_OFFSET constant for slippage calculations. - Added internal functions for validating board and disembark data, ensuring no additional parameters are required.
…e-core subproject status - Removed the console log statement for shares calculation in MidasArk test to clean up output. - Updated pendle-core subproject commit to indicate a dirty state, reflecting local changes.
…ality - Updated import statements for clarity and added necessary interfaces. - Refactored state variables into distinct sections for better organization. - Improved asset calculation methods by introducing internal functions for shares to assets conversion. - Enhanced withdrawal logic to streamline request handling and ensure accurate asset retrieval. - Removed redundant code and comments to improve readability and maintainability.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Removed unnecessary comment regarding total shares calculation in MidasArk contract to enhance code clarity and maintainability. - Updated pendle-core subproject status to reflect a dirty state, indicating local modifications.
robercano
left a comment
There was a problem hiding this comment.
Looks great! I've gone very specific to iterate on our best practices and have a discussion about what we want to do in the code. Also, what's the plan for the test TODOs?
|
|
||
| import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; | ||
|
|
||
| error InvalidIssuanceVault(); |
There was a problem hiding this comment.
Chain Security audit pointed out that errors should be defined in a separate file. It is also my preference. What do you think about it?
| * @notice 100 percent with base 100 | ||
| * @dev for example, 10% will be (10 * 100)% | ||
| */ | ||
| uint256 public constant ONE_HUNDRED_PERCENT = 100 * 100; |
There was a problem hiding this comment.
Is this Midas specific? Or could we use Percentage instead? If Percentage cannot be used I'd add a comment here explaining why
| * @notice Default slippage for the swap | ||
| * @dev 50 is 50% (50/10000) | ||
| */ | ||
| uint256 public constant DEFAULT_SLIPPAGE = 50; |
There was a problem hiding this comment.
Same reasoning for slippage as for the hundrer percent, can we use Percent?
There was a problem hiding this comment.
it's alredy done this way in the base contract - let's keep the changes for another PR
|
|
||
| // Verify mToken matches vaults | ||
| IMToken issuanceVaultMToken = issuanceVault.mToken(); | ||
| if (address(issuanceVaultMToken) != _mToken) |
There was a problem hiding this comment.
This makes me wonder if we really need to pass the _mToken parameter or just fetch it from the protocol and that's it. Why are we passing _mToken?
There was a problem hiding this comment.
yeah correct. That's been changed
| if (address(issuanceVaultMToken) != _mToken) | ||
| revert InvalidMTokenAddress(); | ||
|
|
||
| IMToken redemptionVaultMToken = redemptionVault.mToken(); |
| */ | ||
| function setMaxSupplyCap(uint256 newValue) external; | ||
|
|
||
| function tokensConfig( |
There was a problem hiding this comment.
Can we document these functions, even if it is to say that there is nothing to document? It stands out when reading the interface
| address token | ||
| ) external view returns (TokenConfig memory); | ||
|
|
||
| function mTokenDataFeed() external view returns (address); |
| bool stable; | ||
| } | ||
|
|
||
| enum RequestStatus { |
There was a problem hiding this comment.
Documentation for enum, and the next three structs? Or is this copied from Midas and they didn't document it? If so, please add a comment at the top mentioning this, so auditors will know why
| // SPDX-License-Identifier: MIT | ||
| pragma solidity ^0.8.0; | ||
|
|
||
| interface IMidasOracle { |
| // SPDX-License-Identifier: MIT | ||
| pragma solidity 0.8.28; | ||
|
|
||
| interface IMidasWithdrawalManager { |
There was a problem hiding this comment.
And here? (just marking the places where this may be needed)
- Added HyperBeatCoreArk contract to facilitate integration with the HyperBeatCore protocol. - Implemented MidasArk contract for managing Midas protocol interactions. - Created interfaces for HyperBeat components including IHyperBeatDepositor, IHyperBeatOracle, IHyperBeatPricer, and IHyperBeatVaultToken. - Updated deployment scripts and configuration files to support new ark types and their parameters. - Enhanced test coverage for the new contracts and their functionalities.
…asArk contracts - Introduced custom error interfaces IHyperBeatCoreArkErrors and IMidasArkErrors for better error management. - Replaced existing revert statements with specific error calls in HyperBeatCoreArk and MidasArk constructors. - Enhanced clarity and maintainability of error handling in both contracts. - Updated documentation for withdrawal functions to clarify special withdrawal conditions.
…asArk contracts - Added clarifying comments regarding the handling of token decimals in the conversion factor calculations. - Specified that vault tokens and mTokens are expected to have 18 decimals, and noted the rarity of assets with more than 18 decimals in the context of these contracts.
- Introduced MidasArk contract for managing token supply and yield generation through Midas vaults. - Updated deployment configurations to include MidasArk with USDC asset and associated parameters. - Enhanced deployment artifacts and journal entries to reflect the new MidasArk integration.
- Added checks in MidasArk to ensure payment tokens are stable and have no associated fees. - Introduced a new error for invalid token configurations in IMidasArkErrors. - Updated IDepositVault and IRedemptionVault interfaces to include tokensConfig function documentation. - Modified deployment configurations for improved clarity and consistency.
Description
This PR implements the
MidasArk, a new Ark integration for the Summer Earn Protocol that enables yield generation through Midas Protocol vaults.The implementation follows the
ArkWithWithdrawalRequestpattern, handling the specific interactions required for Midas's dual-vault architecture (Issuance Vault for deposits, Redemption Vault for withdrawals) and their mToken share system.Changes
MidasArk.sol: The core contract implementingIArkand inheritingArkWithWithdrawalRequest.board()logic to deposit assets into the Midas Issuance Vault (depositInstant).requestWithdrawal()to handle asynchronous redemptions via the Midas Redemption Vault.withdrawUsingSwap()to support instant withdrawals via the Redemption Vault's instant redemption mechanism (treated as a swap).IMidasOraclefor accurate share-to-asset valuation.MidasArkUSDC.fork.t.solcovering boarding, withdrawals, and value accounting.Benefits
Testing
MidasArkUSDC.fork.t.sol.test_Board_Midas_fork: Verifies deposits into the Issuance Vault.test_RequestPartialRedeem_Midas_fork: Verifies queuing of withdrawal requests.test_WithdrawUsingSwap_Midas: Verifies instant redemption capabilities.test_TotalAssets_UsesOracle: Ensures asset reporting matches oracle rates.To run tests:
forge test --match-contract MidasArkTestFork