diff --git a/src/controller/VaultManager.sol b/src/controller/VaultManager.sol index 7e07b04..7a11598 100644 --- a/src/controller/VaultManager.sol +++ b/src/controller/VaultManager.sol @@ -81,6 +81,10 @@ abstract contract VaultManager is BaseController, PriceFeedManager { * @notice Thrown when trying to set a vault as the main vault for an asset it already is */ error Vault_AlreadyMainVaultForAsset(); + /** + * @notice Thrown when the overall vaults settings are invalid + */ + error Vault_InvalidVaultsSettings(); /** * @notice Initializes the VaultManager with an empty vaults linked list @@ -121,6 +125,7 @@ abstract contract VaultManager is BaseController, PriceFeedManager { _checkVaultSettings(settings); vaultSettings[vault] = settings; emit VaultSettingsUpdated(vault, settings.maxCapacity, settings.maxProportionality, settings.minProportionality); + _checkVaultsProportionalities(); if (isMainVaultForAsset || _vaultFor[asset] == address(0)) { emit MainVaultForAssetUpdated(asset, _vaultFor[asset], vault); @@ -146,6 +151,7 @@ abstract contract VaultManager is BaseController, PriceFeedManager { delete vaultSettings[vault]; emit VaultSettingsUpdated(vault, 0, 0, 0); + _checkVaultsProportionalities(); address asset = IControlledVault(vault).asset(); if (_vaultFor[asset] == vault) { @@ -173,6 +179,7 @@ abstract contract VaultManager is BaseController, PriceFeedManager { _checkVaultSettings(settings); vaultSettings[vault] = settings; emit VaultSettingsUpdated(vault, settings.maxCapacity, settings.maxProportionality, settings.minProportionality); + _checkVaultsProportionalities(); } /** @@ -234,6 +241,34 @@ abstract contract VaultManager is BaseController, PriceFeedManager { require(settings.minProportionality <= settings.maxProportionality, Vault_MinProportionalityNotLessThanMax()); } + /** + * @dev Validates that the sum of any two vaults' maximum proportionalities is greater than or equal to 100%. + * This ensures that the protocol cannot get into a deadlock due to proportionality constraints. + */ + function _checkVaultsProportionalities() internal view { + address[] memory allVaults = vaults(); + uint256 count = allVaults.length; + if (count == 0) return; // No vaults to check + if (count == 1) { + // Single vault must be allowed to cover 100% of the assets + require(vaultSettings[allVaults[0]].maxProportionality == MAX_BPS, Vault_InvalidVaultsSettings()); + } else if (count == 2) { + // Sum of both vaults' max proportionalities must be at least 120% + uint256 sum = + vaultSettings[allVaults[0]].maxProportionality + vaultSettings[allVaults[1]].maxProportionality; + require(sum >= 12_000, Vault_InvalidVaultsSettings()); + } else { + // For 3 or more vaults, ensure that the sum of the two smallest max proportionalities is at least 100% + uint256 maxProportionality; + uint256 minMaxProportionality = MAX_BPS; + for (uint256 i; i < count; ++i) { + maxProportionality = vaultSettings[allVaults[i]].maxProportionality; + require(maxProportionality + minMaxProportionality >= MAX_BPS, Vault_InvalidVaultsSettings()); + minMaxProportionality = minMaxProportionality.min(maxProportionality); + } + } + } + /** * @notice Retrieves the asset managed by a given vault * @param vault The address of the vault diff --git a/tests/harness/ControllerHarness.sol b/tests/harness/ControllerHarness.sol index c9861fc..f6878bf 100644 --- a/tests/harness/ControllerHarness.sol +++ b/tests/harness/ControllerHarness.sol @@ -51,6 +51,10 @@ contract ControllerHarness is Controller { return _vaultsOverview(calculateTotalValue); } + function exposed_checkVaultsProportionalities() external view { + _checkVaultsProportionalities(); + } + function exposed_maxDepositLimit(address vault) external view returns (uint256 max) { return _maxDepositLimit(_vaultsOverview(false), vault); } diff --git a/tests/unit/controller/Controller.vaultManager.t.sol b/tests/unit/controller/Controller.vaultManager.t.sol index 1170f81..1336bac 100644 --- a/tests/unit/controller/Controller.vaultManager.t.sol +++ b/tests/unit/controller/Controller.vaultManager.t.sol @@ -35,6 +35,8 @@ abstract contract Controller_VaultManager_Test is ControllerTest { vm.prank(admin); controller.grantRole(managerRole, manager); + settings = BaseController.VaultSettings({ maxCapacity: 1, minProportionality: 0, maxProportionality: MAX_BPS }); + vm.label(vault[0], "Vault 1"); vm.label(vault[1], "Vault 2"); vm.label(vault[2], "Vault 3"); @@ -102,7 +104,7 @@ contract Controller_VaultManager_AddVault_Test is Controller_VaultManager_Test { } function testFuzz_shouldRevert_whenInvalidVaultSettingsMaxProportionality(uint256 maxProportionality) public { - maxProportionality = bound(maxProportionality, controller.MAX_BPS() + 1, type(uint16).max); + maxProportionality = bound(maxProportionality, MAX_BPS + 1, type(uint16).max); // forge-lint: disable-next-line(unsafe-typecast) settings.maxProportionality = uint16(maxProportionality); @@ -113,7 +115,7 @@ contract Controller_VaultManager_AddVault_Test is Controller_VaultManager_Test { } function testFuzz_shouldRevert_whenInvalidVaultSettingsMinProportionality(uint256 minProportionality) public { - minProportionality = bound(minProportionality, controller.MAX_BPS() + 1, type(uint16).max); + minProportionality = bound(minProportionality, MAX_BPS + 1, type(uint16).max); // forge-lint: disable-next-line(unsafe-typecast) settings.minProportionality = uint16(minProportionality); @@ -123,7 +125,7 @@ contract Controller_VaultManager_AddVault_Test is Controller_VaultManager_Test { controller.addVault(vault[0], settings, isMainVaultForAsset); } - function testFuzz_shouldRevert_whenInvalidVaultSettingsMinNotLessThanMax() public { + function test_shouldRevert_whenInvalidVaultSettingsMinNotLessThanMax() public { settings.maxProportionality = 5000; settings.minProportionality = 6000; @@ -132,6 +134,26 @@ contract Controller_VaultManager_AddVault_Test is Controller_VaultManager_Test { controller.addVault(vault[0], settings, isMainVaultForAsset); } + function test_shouldRevert_whenInvalidVaultsSettings() public { + settings.maxProportionality = 5000; // not 100% + + vm.prank(manager); + vm.expectRevert(VaultManager.Vault_InvalidVaultsSettings.selector); + controller.addVault(vault[0], settings, isMainVaultForAsset); + + // Mock two vaults with valid settings + controller.workaround_addVault(vault[0]); + controller.workaround_addVault(vault[1]); + controller.workaround_setVaultSettings(vault[0], 1, 0, 6000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 5000); + + settings.maxProportionality = 4000; + + vm.prank(manager); + vm.expectRevert(VaultManager.Vault_InvalidVaultsSettings.selector); + controller.addVault(vault[2], settings, isMainVaultForAsset); + } + function test_shouldAddVaultToLinkedList() public { vm.prank(manager); controller.addVault(vault[0], settings, isMainVaultForAsset); @@ -149,18 +171,22 @@ contract Controller_VaultManager_AddVault_Test is Controller_VaultManager_Test { } function testFuzz_shouldSetVaultSettings(uint224 maxCapacity, uint16 maxProp, uint16 minProp) public { - maxProp = uint16(bound(maxProp, 0, controller.MAX_BPS())); - minProp = uint16(bound(minProp, 0, controller.MAX_BPS())); + maxProp = uint16(bound(maxProp, 2000, MAX_BPS)); + minProp = uint16(bound(minProp, 0, MAX_BPS)); vm.assume(minProp <= maxProp); + // First vault with default settings to allow any settings for the second vault + vm.prank(manager); + controller.addVault(vault[0], settings, isMainVaultForAsset); + settings.maxCapacity = maxCapacity; settings.minProportionality = minProp; settings.maxProportionality = maxProp; vm.prank(manager); - controller.addVault(vault[0], settings, isMainVaultForAsset); + controller.addVault(vault[1], settings, isMainVaultForAsset); - (uint224 actualMaxCapacity, uint16 actualMinProp, uint16 actualMaxProp) = controller.vaultSettings(vault[0]); + (uint224 actualMaxCapacity, uint16 actualMinProp, uint16 actualMaxProp) = controller.vaultSettings(vault[1]); assertEq(actualMaxCapacity, settings.maxCapacity); assertEq(actualMinProp, settings.minProportionality); assertEq(actualMaxProp, settings.maxProportionality); @@ -175,17 +201,21 @@ contract Controller_VaultManager_AddVault_Test is Controller_VaultManager_Test { } function test_shouldEmit_VaultSettingsUpdated() public { + // First vault with default settings to allow any settings for the second vault + vm.prank(manager); + controller.addVault(vault[0], settings, isMainVaultForAsset); + settings.maxCapacity = 1e24; settings.maxProportionality = 8000; settings.minProportionality = 2000; vm.expectEmit(); emit VaultManager.VaultSettingsUpdated( - vault[0], settings.maxCapacity, settings.maxProportionality, settings.minProportionality + vault[1], settings.maxCapacity, settings.maxProportionality, settings.minProportionality ); vm.prank(manager); - controller.addVault(vault[0], settings, isMainVaultForAsset); + controller.addVault(vault[1], settings, isMainVaultForAsset); } function test_shouldSetVaultAsMainForAsset_whenIsMainVaultForAssetFalse_whenNoMainVault() public { @@ -245,8 +275,6 @@ contract Controller_VaultManager_RemoveVault_Test is Controller_VaultManager_Tes function setUp() public override { super.setUp(); - settings = BaseController.VaultSettings(1, 2, 3); - vm.prank(manager); controller.addVault(vault[0], settings, true); vm.prank(manager); @@ -310,6 +338,16 @@ contract Controller_VaultManager_RemoveVault_Test is Controller_VaultManager_Tes assertEq(actualMinProp, 0); } + function test_shouldRevert_whenInvalidVaultsSettings() public { + // Mock two vaults with valid settings + controller.workaround_setVaultSettings(vault[0], 1, 0, 6000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 5000); + + vm.prank(manager); + vm.expectRevert(VaultManager.Vault_InvalidVaultsSettings.selector); + controller.removeVault(vault[0], prevVault); + } + function test_shouldEmit_VaultRemoved() public { vm.expectEmit(); emit VaultManager.VaultRemoved(vault[0]); @@ -363,10 +401,10 @@ contract Controller_VaultManager_UpdateVaultSettings_Test is Controller_VaultMan function setUp() public override { super.setUp(); - settings = BaseController.VaultSettings(1, 2, 3); - vm.prank(manager); controller.addVault(vault[0], settings, true); + vm.prank(manager); + controller.addVault(vault[1], settings, true); } function testFuzz_shouldRevert_whenCallerNotVaultManager(address caller) public { @@ -388,7 +426,7 @@ contract Controller_VaultManager_UpdateVaultSettings_Test is Controller_VaultMan } function testFuzz_shouldRevert_whenInvalidVaultSettingsMaxProportionality(uint256 maxProportionality) public { - maxProportionality = bound(maxProportionality, controller.MAX_BPS() + 1, type(uint16).max); + maxProportionality = bound(maxProportionality, MAX_BPS + 1, type(uint16).max); // forge-lint: disable-next-line(unsafe-typecast) settings.maxProportionality = uint16(maxProportionality); @@ -399,7 +437,7 @@ contract Controller_VaultManager_UpdateVaultSettings_Test is Controller_VaultMan } function testFuzz_shouldRevert_whenInvalidVaultSettingsMinProportionality(uint256 minProportionality) public { - minProportionality = bound(minProportionality, controller.MAX_BPS() + 1, type(uint16).max); + minProportionality = bound(minProportionality, MAX_BPS + 1, type(uint16).max); // forge-lint: disable-next-line(unsafe-typecast) settings.minProportionality = uint16(minProportionality); @@ -418,9 +456,21 @@ contract Controller_VaultManager_UpdateVaultSettings_Test is Controller_VaultMan controller.updateVaultSettings(vault[0], settings); } + function test_shouldRevert_whenInvalidVaultsSettings() public { + // Mock two vaults with valid settings + controller.workaround_setVaultSettings(vault[0], 1, 0, 6000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 5000); + + settings.maxProportionality = 4000; + + vm.prank(manager); + vm.expectRevert(VaultManager.Vault_InvalidVaultsSettings.selector); + controller.updateVaultSettings(vault[0], settings); + } + function testFuzz_shouldSetVaultSettings(uint224 maxCapacity, uint16 maxProp, uint16 minProp) public { - maxProp = uint16(bound(maxProp, 0, controller.MAX_BPS())); - minProp = uint16(bound(minProp, 0, controller.MAX_BPS())); + maxProp = uint16(bound(maxProp, 2000, MAX_BPS)); + minProp = uint16(bound(minProp, 0, MAX_BPS)); vm.assume(minProp <= maxProp); settings.maxCapacity = maxCapacity; @@ -459,8 +509,6 @@ contract Controller_VaultManager_SetMainVault_Test is Controller_VaultManager_Te _mockVault(mainVault, asset[0]); - settings = BaseController.VaultSettings(1, 2, 3); - vm.prank(manager); controller.addVault(mainVault, settings, true); vm.prank(manager); @@ -513,8 +561,6 @@ contract Controller_VaultManager_IsVault_Test is Controller_VaultManager_Test { function setUp() public override { super.setUp(); - settings = BaseController.VaultSettings(1, 2, 3); - vm.prank(manager); controller.addVault(vault[0], settings, true); vm.prank(manager); @@ -554,8 +600,6 @@ contract Controller_VaultManager_Vaults_Test is Controller_VaultManager_Test { } function test_shouldReturnAllAddedVaultsInOrder() public { - BaseController.VaultSettings memory settings = BaseController.VaultSettings(1, 2, 3); - vm.prank(manager); controller.addVault(vault[0], settings, true); vm.prank(manager); @@ -657,3 +701,123 @@ contract Controller_VaultManager_VaultsOverview_Test is Controller_VaultManager_ assertEq(overview.totalValue, 0); } } + +contract Controller_VaultManager_CheckVaultsProportionalities_Test is Controller_VaultManager_Test { + function test_shouldPass_whenNoVaults() public view { + controller.exposed_checkVaultsProportionalities(); + } + + function test_shouldPass_whenOneVault_whenMaxProportionalityIsMaxBPS() public { + controller.workaround_addVault(vault[0]); + controller.workaround_setVaultSettings(vault[0], 1, 0, MAX_BPS); + + controller.exposed_checkVaultsProportionalities(); + } + + function testFuzz_shouldRevert_whenOneVault_whenMaxProportionalityIsNotMaxBPS(uint16 maxProportionality) public { + maxProportionality = uint16(bound(maxProportionality, 0, MAX_BPS - 1)); + + controller.workaround_addVault(vault[0]); + controller.workaround_setVaultSettings(vault[0], 1, 0, maxProportionality); + + vm.expectRevert(VaultManager.Vault_InvalidVaultsSettings.selector); + controller.exposed_checkVaultsProportionalities(); + } + + function testFuzz_shouldPass_whenTwoVaults_whenMaxProportionalityInLimits( + uint16 maxP1, + uint16 maxP2 + ) + public + { + maxP1 = uint16(bound(maxP1, 2000, 10_000)); + maxP2 = uint16(bound(maxP2, 12_000 - maxP1, 10_000)); + + controller.workaround_addVault(vault[0]); + controller.workaround_addVault(vault[1]); + controller.workaround_setVaultSettings(vault[0], 1, 0, maxP1); + controller.workaround_setVaultSettings(vault[1], 1, 0, maxP2); + + controller.exposed_checkVaultsProportionalities(); + } + + function testFuzz_shouldRevert_whenTwoVaults_whenMaxProportionalityNotInLimits( + uint16 maxP1, + uint16 maxP2 + ) + public + { + maxP1 = uint16(bound(maxP1, 0, 10_000 - 1)); + maxP2 = uint16(bound(maxP2, 0, 12_000 - (maxP1 > 2000 ? maxP1 : 2000))); + + controller.workaround_addVault(vault[0]); + controller.workaround_addVault(vault[1]); + controller.workaround_setVaultSettings(vault[0], 1, 0, maxP1); + controller.workaround_setVaultSettings(vault[1], 1, 0, maxP2); + + vm.expectRevert(VaultManager.Vault_InvalidVaultsSettings.selector); + controller.exposed_checkVaultsProportionalities(); + } + + function test_shouldPass_whenMoreVaults_whenMaxProportionalityInLimits() public { + controller.workaround_addVault(vault[0]); + controller.workaround_addVault(vault[1]); + controller.workaround_addVault(vault[2]); + + controller.workaround_setVaultSettings(vault[0], 1, 0, 8000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 3000); + controller.workaround_setVaultSettings(vault[2], 1, 0, 7000); + controller.exposed_checkVaultsProportionalities(); + + controller.workaround_setVaultSettings(vault[0], 1, 0, 5000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 5000); + controller.workaround_setVaultSettings(vault[2], 1, 0, 5000); + controller.exposed_checkVaultsProportionalities(); + + controller.workaround_setVaultSettings(vault[0], 1, 0, 9000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 9000); + controller.workaround_setVaultSettings(vault[2], 1, 0, 1000); + controller.exposed_checkVaultsProportionalities(); + + controller.workaround_setVaultSettings(vault[0], 1, 0, 10_000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 10_000); + controller.workaround_setVaultSettings(vault[2], 1, 0, 0); + controller.exposed_checkVaultsProportionalities(); + } + + function test_shouldRevert_whenMoreVaults_whenMaxProportionalityNotInLimits() public { + controller.workaround_addVault(vault[0]); + controller.workaround_addVault(vault[1]); + controller.workaround_addVault(vault[2]); + + controller.workaround_setVaultSettings(vault[0], 1, 0, 8000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 2000); + controller.workaround_setVaultSettings(vault[2], 1, 0, 7000); + vm.expectRevert(VaultManager.Vault_InvalidVaultsSettings.selector); + controller.exposed_checkVaultsProportionalities(); + + controller.workaround_setVaultSettings(vault[0], 1, 0, 5000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 5000); + controller.workaround_setVaultSettings(vault[2], 1, 0, 4999); + vm.expectRevert(VaultManager.Vault_InvalidVaultsSettings.selector); + controller.exposed_checkVaultsProportionalities(); + + controller.workaround_setVaultSettings(vault[0], 1, 0, 9000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 8000); + controller.workaround_setVaultSettings(vault[2], 1, 0, 1000); + vm.expectRevert(VaultManager.Vault_InvalidVaultsSettings.selector); + controller.exposed_checkVaultsProportionalities(); + + controller.workaround_setVaultSettings(vault[0], 1, 0, 9000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 10_000); + controller.workaround_setVaultSettings(vault[2], 1, 0, 0); + vm.expectRevert(VaultManager.Vault_InvalidVaultsSettings.selector); + controller.exposed_checkVaultsProportionalities(); + + controller.workaround_setVaultSettings(vault[0], 1, 0, 6000); + controller.workaround_setVaultSettings(vault[1], 1, 0, 5000); + controller.workaround_setVaultSettings(vault[2], 1, 0, 4000); + vm.expectRevert(VaultManager.Vault_InvalidVaultsSettings.selector); + controller.exposed_checkVaultsProportionalities(); + } +}