From fea8da2aadc552e3c3867431239db79c2da9e3b4 Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Wed, 20 Sep 2023 16:21:31 +0200 Subject: [PATCH 1/3] feat(metamorpho): add no pending value error --- src/MetaMorpho.sol | 1 + src/libraries/ErrorsLib.sol | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/MetaMorpho.sol b/src/MetaMorpho.sol index 9746434b..668a3638 100644 --- a/src/MetaMorpho.sol +++ b/src/MetaMorpho.sol @@ -97,6 +97,7 @@ contract MetaMorpho is ERC4626, Ownable2Step, IMetaMorpho { } modifier timelockElapsed(uint64 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 34468919..31d67bf0 100644 --- a/src/libraries/ErrorsLib.sol +++ b/src/libraries/ErrorsLib.sol @@ -22,6 +22,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"; From 7e5a256432bec742e2d9fb4e78eeb8fa1298b898 Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Wed, 20 Sep 2023 18:03:05 +0200 Subject: [PATCH 2/3] fix(test): adapt test --- test/forge/RoleTest.sol | 24 ++++-------------------- test/forge/TimelockTest.sol | 25 ++++++++++++++++++++----- 2 files changed, 24 insertions(+), 25 deletions(-) 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..48e519a2 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,6 +150,11 @@ contract TimelockTest is BaseTest { assertEq(Id.unwrap(vault.withdrawQueue(0)), Id.unwrap(id), "withdrawQueue"); } + function testAcceptCapNoPendingValue() public { + vm.expectRevert(bytes(ErrorsLib.NO_PENDING_VALUE)); + vault.acceptCap(allMarkets[0].id()); + } + function testAcceptCapTimelockNotElapsed(uint256 alpsed) public { alpsed = bound(alpsed, 0, TIMELOCK - 1); From db6c69b065361b92fb14f2468b82499f5b2a9184 Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Wed, 20 Sep 2023 18:04:53 +0200 Subject: [PATCH 3/3] refactor(test): rename timeElapsed to elapsed --- test/forge/TimelockTest.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/forge/TimelockTest.sol b/test/forge/TimelockTest.sol index 48e519a2..520273ff 100644 --- a/test/forge/TimelockTest.sol +++ b/test/forge/TimelockTest.sol @@ -155,32 +155,32 @@ contract TimelockTest is BaseTest { vault.acceptCap(allMarkets[0].id()); } - function testAcceptCapTimelockNotElapsed(uint256 alpsed) public { - alpsed = bound(alpsed, 0, TIMELOCK - 1); + 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());