diff --git a/src/MetaMorpho.sol b/src/MetaMorpho.sol index 20bbdb70..ac39a718 100644 --- a/src/MetaMorpho.sol +++ b/src/MetaMorpho.sol @@ -97,19 +97,20 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, IMetaMorpho { _; } - modifier onlyAllocator() { - require(isAllocator(_msgSender()), ErrorsLib.NOT_ALLOCATOR); + modifier onlyGuardian() { + require(_msgSender() == guardian, ErrorsLib.NOT_GUARDIAN); _; } - modifier onlyGuardian() { - require(_msgSender() == guardian, ErrorsLib.NOT_GUARDIAN); + modifier onlyAllocator() { + require(isAllocator(_msgSender()), ErrorsLib.NOT_ALLOCATOR); _; } modifier timelockElapsed(uint256 submittedAt) { + require(submittedAt != 0, ErrorsLib.NO_PENDING_VALUE); require(block.timestamp >= submittedAt + timelock, ErrorsLib.TIMELOCK_NOT_ELAPSED); require(block.timestamp <= submittedAt + timelock + TIMELOCK_EXPIRATION, ErrorsLib.TIMELOCK_EXPIRATION_EXCEEDED); diff --git a/src/libraries/ErrorsLib.sol b/src/libraries/ErrorsLib.sol index fe935923..59a53f20 100644 --- a/src/libraries/ErrorsLib.sol +++ b/src/libraries/ErrorsLib.sol @@ -26,6 +26,8 @@ library ErrorsLib { string internal constant MISSING_MARKET = "missing market"; + string internal constant NO_PENDING_VALUE = "no pending value"; + string internal constant WITHDRAW_FAILED_MORPHO = "withdraw failed on Morpho"; string internal constant MARKET_NOT_CREATED = "market not created"; diff --git a/test/forge/RoleTest.sol b/test/forge/RoleTest.sol index 95edd293..96682640 100644 --- a/test/forge/RoleTest.sol +++ b/test/forge/RoleTest.sol @@ -11,23 +11,20 @@ contract RoleTest is BaseTest { vm.startPrank(caller); - vm.expectRevert("Ownable: caller is not the owner"); - vault.submitTimelock(1); - - vm.expectRevert("Ownable: caller is not the owner"); - vault.acceptTimelock(); - vm.expectRevert("Ownable: caller is not the owner"); vault.setRiskManager(caller); vm.expectRevert("Ownable: caller is not the owner"); vault.setIsAllocator(caller, true); + vm.expectRevert("Ownable: caller is not the owner"); + vault.submitTimelock(1); + vm.expectRevert("Ownable: caller is not the owner"); vault.submitFee(1); vm.expectRevert("Ownable: caller is not the owner"); - vault.acceptFee(); + vault.submitGuardian(address(1)); vm.expectRevert("Ownable: caller is not the owner"); vault.setFeeRecipient(caller); @@ -43,9 +40,6 @@ contract RoleTest is BaseTest { vm.expectRevert(bytes(ErrorsLib.NOT_RISK_MANAGER)); vault.submitCap(allMarkets[0], CAP); - vm.expectRevert(bytes(ErrorsLib.NOT_RISK_MANAGER)); - vault.acceptCap(allMarkets[0].id()); - vm.stopPrank(); } @@ -74,18 +68,8 @@ contract RoleTest is BaseTest { vm.prank(OWNER); vault.submitCap(allMarkets[0], CAP); - vm.warp(block.timestamp + vault.timelock()); - - vm.prank(OWNER); - vault.acceptCap(allMarkets[0].id()); - vm.prank(RISK_MANAGER); vault.submitCap(allMarkets[1], CAP); - - vm.warp(block.timestamp + vault.timelock()); - - vm.prank(RISK_MANAGER); - vault.acceptCap(allMarkets[1].id()); } function testAllocatorOrRiskManagerOrOwnerShouldTriggerAllocatorFunctions() public { diff --git a/test/forge/TimelockTest.sol b/test/forge/TimelockTest.sol index 775a2bfd..520273ff 100644 --- a/test/forge/TimelockTest.sol +++ b/test/forge/TimelockTest.sol @@ -77,7 +77,19 @@ contract TimelockTest is BaseTest { assertEq(submittedAt, 0, "submittedAt"); } - function testAcceptTimelockNotOwner() public { + function testAcceptTimelockNoPendingValue() public { + vm.expectRevert(bytes(ErrorsLib.NO_PENDING_VALUE)); + vault.acceptTimelock(); + } + + function testAcceptTimelockNotOwner(uint256 timelock) public { + timelock = bound(timelock, 0, TIMELOCK - 1); + + vm.prank(OWNER); + vault.submitTimelock(timelock); + + vm.warp(block.timestamp + TIMELOCK); + vm.expectRevert("Ownable: caller is not the owner"); vault.acceptTimelock(); } @@ -95,12 +107,10 @@ contract TimelockTest is BaseTest { vault.acceptTimelock(); } - function testAcceptTimelockExpirationExceeded(uint256 timelock, uint256 elapsed) public { - timelock = bound(timelock, 0, MAX_TIMELOCK); + function testAcceptTimelockDecreasedTimelockExpirationExceeded(uint256 timelock, uint256 elapsed) public { + timelock = bound(timelock, 0, TIMELOCK - 1); elapsed = bound(elapsed, TIMELOCK + TIMELOCK_EXPIRATION + 1, type(uint64).max); - vm.assume(timelock != TIMELOCK); - vm.prank(OWNER); vault.submitTimelock(timelock); @@ -140,32 +150,37 @@ contract TimelockTest is BaseTest { assertEq(Id.unwrap(vault.withdrawQueue(0)), Id.unwrap(id), "withdrawQueue"); } - function testAcceptCapTimelockNotElapsed(uint256 alpsed) public { - alpsed = bound(alpsed, 0, TIMELOCK - 1); + function testAcceptCapNoPendingValue() public { + vm.expectRevert(bytes(ErrorsLib.NO_PENDING_VALUE)); + vault.acceptCap(allMarkets[0].id()); + } + + function testAcceptCapTimelockNotElapsed(uint256 elapsed) public { + elapsed = bound(elapsed, 0, TIMELOCK - 1); vm.prank(RISK_MANAGER); vault.submitCap(allMarkets[0], CAP); - vm.warp(block.timestamp + alpsed); + vm.warp(block.timestamp + elapsed); vm.prank(RISK_MANAGER); vm.expectRevert(bytes(ErrorsLib.TIMELOCK_NOT_ELAPSED)); vault.acceptCap(allMarkets[0].id()); } - function testAcceptCapTimelockExpirationExceeded(uint256 timelock, uint256 timeElapsed) public { + function testAcceptCapTimelockExpirationExceeded(uint256 timelock, uint256 elapsed) public { timelock = bound(timelock, 1, MAX_TIMELOCK); vm.assume(timelock != vault.timelock()); _setTimelock(timelock); - timeElapsed = bound(timeElapsed, timelock + TIMELOCK_EXPIRATION + 1, type(uint64).max); + elapsed = bound(elapsed, timelock + TIMELOCK_EXPIRATION + 1, type(uint64).max); vm.startPrank(RISK_MANAGER); vault.submitCap(allMarkets[0], CAP); - vm.warp(block.timestamp + timeElapsed); + vm.warp(block.timestamp + elapsed); vm.expectRevert(bytes(ErrorsLib.TIMELOCK_EXPIRATION_EXCEEDED)); vault.acceptCap(allMarkets[0].id());