Platform: Code4rena
Start Date: 02/08/2022
Pot Size: $50,000 USDC
Total HM: 12
Participants: 69
Period: 5 days
Judge: gzeon
Total Solo HM: 5
Id: 150
League: ETH
Rank: 47/69
Findings: 1
Award: $88.17
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0xDjango, 0xNazgul, 0xc0ffEE, 8olidity, Bnke0x0, Chom, CodingNameKiki, Deivitto, Dravee, Funen, JC, JohnSmith, NoamYakov, ReyAdmirado, Rohan16, Rolezn, Sm4rty, SooYa, TomFrenchBlockchain, TomJ, Waze, __141345__, ajtra, ak1, aysha, bin2chen, bobirichman, brgltd, bulej93, c3phas, delfin454000, durianSausage, erictee, fatherOfBlocks, gogo, horsefacts, hyh, ladboy233, mics, natzuu, nxrblsrpr, oyc_109, rbserver, samruna, sikorico, simon135, tofunmi, wagmi
88.166 USDC - $88.17
Formula in the code is correct (with mcrB = vaultBMcr + mcrBuffer
):
/** Rebalance value is calculated by the formula below : targetRatio * (vaultDebt + fixedFee) - collateralValue ---------------------------------------------------------- targetRatio / mcrB - 1 - targetRatio * variableFee */
While one in README incorrectly states that the fraction equals rebalanceAmount
, while it is `rebalanceValue = rebalanceAmount * collateralBPARPrice':
rebalanceAmount = (targetRatio * (vaultADebt + fixedFee) - vaultACollateralValue) / (targetRatio / (vaultBMcr + mcrBuffer) - targetRatio * varFee - 1);
- rebalanceAmount = + rebalanceValue = (targetRatio * (vaultADebt + fixedFee) - vaultACollateralValue) / (targetRatio / (vaultBMcr + mcrBuffer) - targetRatio * varFee - 1);
/** @notice Helper function performing pre rebalance operation sanity checks @dev Checks that vault is managed, that rebalance was called by manager, and maximum daily operation was not reached @param managedVault ManagedVault struct of the vault to rebalance @param rbData RebalanceData struct of the vault to rebalance @param vaultsData Cached VaultsDataProvider interface for gas saving */ function _preRebalanceChecks( ManagedVault memory managedVault, IMIMORebalance.RebalanceData calldata rbData, IVaultsDataProvider vaultsData, uint256 rebalanceAmount ) internal view {
Consider adding @param rebalanceAmount
to the description
Managed Vault Config description talks about automation:
The default state for our V2 SuperVault starts with `isManaged` set to `false` - which corresponds to a state of the vault not being open to automated. To open the vault up for automation, `isManaged` will need to be set to `true`.
Consider clarifying, for example:
The default state for our V2 SuperVault starts with `isManaged` set to `false` - which corresponds to a state of the vault not being open to external management. To open the vault up for management, `isManaged` will need to be set to `true`.
MIMOAutoAction's _isVaultVariationAllowed():
/** @notice Helper function determining if a vault value variation is within vault's management parameters @return True if value change is below allowedVariation and false if it is above */ function _isVaultVariationAllowed( AutomatedVault memory autoVault, uint256 rebalanceValue, uint256 swapResultValue ) internal pure returns (bool) {
MIMOManagedAction's _isVaultVariationAllowed():
/** @notice Helper function determining if a vault value variation is within vault's management parameters @return True if value change is below allowedVariation and false if it is above */ function _isVaultVariationAllowed( ManagedVault memory managedVault, uint256 rebalanceValue, uint256 swapResultValue ) internal pure returns (bool) {
Consider adding arguments description to both functions.
There are several instances of other omitted argument descriptions, but there it is only one argument, which is straightforward. Here it is 3 arguments and proper description will be useful
asset argument description to be rebalanced
in both contracts:
@param assets Address array with one element corresponding to the address of the reblanced asset
@param assets Address array with one element corresponding to the address of the reblanced asset
setManagement() description, to be manager
:
@dev Can only be called by vault owner and can only appoint whitelisting managers as manger
_getAmounts() description, to be of the vault
:
@param vaultState VaultState struct og the vault to rebalance
README, to be Rebalance parameter Equation
:
## Rebalance paramater Equation Calculation
Filtering on unindexed events is disabled, which makes it harder to programmatically use and analyse the system.
MIMOAutoAction's event:
event AutomationSet(uint256 vaultId, AutomatedVault autoVault);
MIMOManagedAction's events:
event ManagerSet(address manager, bool isManager); event ManagementSet(uint256 vaultId, ManagedVault managedVault);
Consider adding indexes to vaultId and manager address to improve events usability