feat: add meaningful done() methods

makerdao · May 16, 2024 · 7cd769c · 7cd769c
1 parent 5d70199
commit 7cd769c
Show file tree

Hide file tree

Showing 11 changed files with 130 additions and 5 deletions.
diff --git a/src/DssEmergencySpell.sol b/src/DssEmergencySpell.sol
@@ -41,7 +41,6 @@ abstract contract DssEmergencySpell is DssEmergencySpellLike {
     address public constant log = address(_log);
     uint256 public constant eta = 0;
     bytes public constant sig = "";
-    bool public constant done = false;
     uint256 public constant expiration = type(uint256).max;
     address public immutable action = address(this);
     bytes32 public immutable tag = keccak256(abi.encodePacked(address(this)));

diff --git a/src/auto-line-wipe/SingleAutoLineWipeSpell.sol b/src/auto-line-wipe/SingleAutoLineWipeSpell.sol
@@ -19,6 +19,14 @@ import {DssEmergencySpell} from "../DssEmergencySpell.sol";
 
 interface LineMomLike {
     function wipe(bytes32 ilk) external returns (uint256);
+    function autoLine() external view returns (address);
+}
+
+interface AutoLineLike {
+    function ilks(bytes32 ilk)
+        external
+        view
+        returns (uint256 maxLine, uint256 gap, uint48 ttl, uint48 last, uint48 lastInc);
 }
 
 contract SingleAutoLineWipeSpell is DssEmergencySpell {
@@ -39,6 +47,17 @@ contract SingleAutoLineWipeSpell is DssEmergencySpell {
         uint256 prevLine = lineMom.wipe(ilk);
         emit Wipe(ilk, prevLine);
     }
+
+    /**
+     * @notice Return whether the spell is done or not.
+     * @dev Check if the ilk has been wiped from auto-line.
+     */
+    function done() external view returns (bool) {
+        (uint256 maxLine, uint256 gap, uint48 ttl, uint48 last, uint48 lastInc) =
+            AutoLineLike(lineMom.autoLine()).ilks(ilk);
+
+        return maxLine == 0 && gap == 0 && ttl == 0 && last == 0 && lastInc == 0;
+    }
 }
 
 contract SingleAutoLineWipeFactory {

diff --git a/src/auto-line-wipe/SingleAutoLineWipeSpell.t.integration.sol b/src/auto-line-wipe/SingleAutoLineWipeSpell.t.integration.sol
@@ -59,6 +59,7 @@ contract SingleAutoLineWipeSpellTest is DssTest {
         (uint256 pmaxLine, uint256 pgap,,,) = autoLine.ilks(ilk);
         assertGt(pmaxLine, 0, "before: auto-line already wiped");
         assertGt(pgap, 0, "before: auto-line already wiped");
+        assertFalse(spell.done(), "before: spell already done");
 
         vm.expectEmit(true, true, true, false);
         // Ignore prevLine for now
@@ -68,6 +69,7 @@ contract SingleAutoLineWipeSpellTest is DssTest {
         (uint256 maxLine, uint256 gap,,,) = autoLine.ilks(ilk);
         assertEq(maxLine, 0, "after: auto-line not wiped (maxLine)");
         assertEq(gap, 0, "after: auto-line not wiped (gap)");
+        assertTrue(spell.done(), "after: spell not done");
     }
 
     function testRevertAutoLineWipeWhenItDoesNotHaveTheHat() public {
@@ -76,13 +78,15 @@ contract SingleAutoLineWipeSpellTest is DssTest {
         (uint256 pmaxLine, uint256 pgap,,,) = autoLine.ilks(ilk);
         assertGt(pmaxLine, 0, "before: auto-line already wiped");
         assertGt(pgap, 0, "before: auto-line already wiped");
+        assertFalse(spell.done(), "before: spell already done");
 
         vm.expectRevert();
         spell.schedule();
 
         (uint256 maxLine, uint256 gap,,,) = autoLine.ilks(ilk);
         assertGt(maxLine, 0, "after: auto-line wiped unexpectedly");
         assertGt(gap, 0, "after: auto-line wiped unexpectedly");
+        assertFalse(spell.done(), "after: spell done unexpectedly");
     }
 
     event Wipe(bytes32 indexed autoLine, uint256 prevLine);

diff --git a/src/clip-breaker/SingleClipBreakerSpell.sol b/src/clip-breaker/SingleClipBreakerSpell.sol
@@ -21,6 +21,10 @@ interface ClipperMomLike {
     function setBreaker(address clip, uint256 level, uint256 delay) external;
 }
 
+interface ClipLike {
+    function stopped() external view returns (uint256);
+}
+
 interface IlkRegistryLike {
     function xlip(bytes32 ilk) external view returns (address);
 }
@@ -50,6 +54,14 @@ contract SingleClipBreakerSpell is DssEmergencySpell {
         clipperMom.setBreaker(clip, BREAKER_LEVEL, BREAKER_DELAY);
         emit SetBreaker(clip);
     }
+
+    /**
+     * @notice Return whether the spell is done or not.
+     * @dev Check if the Clip instance has stopped = 3.
+     */
+    function done() external view returns (bool) {
+        return ClipLike(ilkReg.xlip(ilk)).stopped() == BREAKER_LEVEL;
+    }
 }
 
 contract SingleClipBreakerFactory {

diff --git a/src/clip-breaker/SingleClipBreakerSpell.t.integration.sol b/src/clip-breaker/SingleClipBreakerSpell.t.integration.sol
@@ -64,23 +64,27 @@ contract SingleClipBreakerSpellTest is DssTest {
 
     function testClipBreakerOnSchedule() public {
         assertEq(clip.stopped(), 0, "before: clip already stopped");
+        assertFalse(spell.done(), "before: spell already done");
 
         vm.expectEmit(true, true, true, true);
         emit SetBreaker(address(clip));
         spell.schedule();
 
         assertEq(clip.stopped(), 3, "after: clip not stopped");
+        assertTrue(spell.done(), "after: spell not done");
     }
 
     function testRevertClipBreakerWhenItDoesNotHaveTheHat() public {
         stdstore.target(chief).sig("hat()").checked_write(address(0));
 
         assertEq(clip.stopped(), 0, "before: clip already stopped");
+        assertFalse(spell.done(), "before: spell already done");
 
         vm.expectRevert();
         spell.schedule();
 
         assertEq(clip.stopped(), 0, "after: clip stopped unexpectedly");
+        assertFalse(spell.done(), "after: spell done unexpectedly");
     }
 
     event SetBreaker(address indexed clip);

diff --git a/src/clip-breaker/UniversalClipBreakerSpell.sol b/src/clip-breaker/UniversalClipBreakerSpell.sol
@@ -28,8 +28,9 @@ interface ClipperMomLike {
     function setBreaker(address clip, uint256 level, uint256 delay) external;
 }
 
-interface WardsLike {
+interface ClipLike {
     function wards(address who) external view returns (uint256);
+    function stopped() external view returns (uint256);
 }
 
 contract UniversalClipBreakerSpell is DssEmergencySpell {
@@ -76,7 +77,7 @@ contract UniversalClipBreakerSpell is DssEmergencySpell {
 
             if (clip == address(0)) continue;
 
-            try WardsLike(clip).wards(address(clipperMom)) returns (uint256 ward) {
+            try ClipLike(clip).wards(address(clipperMom)) returns (uint256 ward) {
                 // Ignore Clip instances that have not relied on ClipperMom.
                 if (ward != 1) continue;
             } catch Error(string memory reason) {
@@ -92,4 +93,34 @@ contract UniversalClipBreakerSpell is DssEmergencySpell {
             }
         }
     }
+
+    /**
+     * @notice Return whether the spell is done or not.
+     * @dev Check if all possible Clip instances from the ilk registry have stopped = 3.
+     */
+    function done() external view returns (bool) {
+        bytes32[] memory ilks = ilkReg.list();
+        for (uint256 i = 0; i < ilks.length; i++) {
+            bytes32 ilk = ilks[i];
+            address clip = ilkReg.xlip(ilk);
+
+            if (clip == address(0)) continue;
+
+            try ClipLike(clip).wards(address(clipperMom)) returns (uint256 ward) {
+                // Ignore Clip instances that have not relied on ClipperMom.
+                if (ward != 1) continue;
+            } catch {
+                // Ignore any errors.
+                continue;
+            }
+
+            try ClipLike(clip).stopped() returns (uint256 stopped) {
+                if (stopped != BREAKER_LEVEL) return false;
+            } catch {
+                // Ignore any errors.
+                continue;
+            }
+        }
+        return true;
+    }
 }
diff --git a/src/clip-breaker/UniversalClipBreakerSpell.t.integration.sol b/src/clip-breaker/UniversalClipBreakerSpell.t.integration.sol
@@ -107,10 +107,12 @@ contract UniversalClipBreakerSpellTest is DssTest {
 
     function testUniversalClipBreakerOnSchedule() public {
         _checkClipMaxStoppedStatus({ilks: ilkReg.list(), maxExpected: 2});
+        assertFalse(spell.done(), "before: spell already done");
 
         spell.schedule();
 
         _checkClipStoppedStatus({ilks: ilkReg.list(), expected: 3});
+        assertTrue(spell.done(), "after: spell not done");
     }
 
     function testUniversalClipBreakerInBatches_Fuzz(uint256 batchSize) public {
@@ -155,11 +157,13 @@ contract UniversalClipBreakerSpellTest is DssTest {
         stdstore.target(chief).sig("hat()").checked_write(address(0));
 
         _checkClipMaxStoppedStatus({ilks: ilkReg.list(), maxExpected: 2});
+        assertFalse(spell.done(), "before: spell already done");
 
         vm.expectRevert();
         spell.schedule();
 
         _checkClipMaxStoppedStatus({ilks: ilkReg.list(), maxExpected: 2});
+        assertFalse(spell.done(), "false: spell done unexpectedly");
     }
 
     function _checkClipMaxStoppedStatus(bytes32[] memory ilks, uint256 maxExpected) internal view {

diff --git a/src/osm-stop/SingleOsmStopSpell.sol b/src/osm-stop/SingleOsmStopSpell.sol
@@ -22,6 +22,10 @@ interface OsmMomLike {
     function stop(bytes32 ilk) external;
 }
 
+interface OsmLike {
+    function stopped() external view returns (uint256);
+}
+
 contract SingleOsmStopSpell is DssEmergencySpell {
     OsmMomLike public immutable osmMom = OsmMomLike(_log.getAddress("OSM_MOM"));
     bytes32 public immutable ilk;
@@ -40,6 +44,14 @@ contract SingleOsmStopSpell is DssEmergencySpell {
         osmMom.stop(ilk);
         emit Stop(osmMom.osms(ilk));
     }
+
+    /**
+     * @notice Return whether the spell is done or not.
+     * @dev Check if the OSM instance is stopped.
+     */
+    function done() external view returns (bool) {
+        return OsmLike(osmMom.osms(ilk)).stopped() == 1;
+    }
 }
 
 contract SingleOsmStopFactory {

diff --git a/src/osm-stop/SingleOsmStopSpell.t.integration.sol b/src/osm-stop/SingleOsmStopSpell.t.integration.sol
@@ -56,25 +56,29 @@ contract SingleOsmStopSpellTest is DssTest {
         vm.makePersistent(chief);
     }
 
-    function testOracleStopOnSchedule() public {
+    function testOsmStopOnSchedule() public {
         assertEq(osm.stopped(), 0, "before: oracle already frozen");
+        assertFalse(spell.done(), "before: spell already done");
 
         vm.expectEmit(true, true, true, true);
         emit Stop(address(osm));
         spell.schedule();
 
         assertEq(osm.stopped(), 1, "after: oracle not frozen");
+        assertTrue(spell.done(), "after: spell not done");
     }
 
-    function testRevertOracleStopWhenItDoesNotHaveTheHat() public {
+    function testRevertOsmStopWhenItDoesNotHaveTheHat() public {
         stdstore.target(chief).sig("hat()").checked_write(address(0));
 
         assertEq(osm.stopped(), 0, "before: oracle already frozen");
+        assertFalse(spell.done(), "before: spell already done");
 
         vm.expectRevert();
         spell.schedule();
 
         assertEq(osm.stopped(), 0, "after: oracle frozen unexpectedly");
+        assertFalse(spell.done(), "after: spell done unexpectedly");
     }
 
     event Stop(address indexed osm);

diff --git a/src/osm-stop/UniversalOsmStopSpell.sol b/src/osm-stop/UniversalOsmStopSpell.sol
@@ -28,6 +28,10 @@ interface OsmMomLike {
     function osms(bytes32 ilk) external view returns (address);
 }
 
+interface OsmLike {
+    function stopped() external view returns (uint256);
+}
+
 interface WardsLike {
     function wards(address who) external view returns (uint256);
 }
@@ -88,4 +92,34 @@ contract UniversalOsmStopSpell is DssEmergencySpell {
             }
         }
     }
+
+    /**
+     * @notice Return whether the spell is done or not.
+     * @dev Check if all possible OSM instances from the ilk registry are stopped.
+     */
+    function done() external view returns (bool) {
+        bytes32[] memory ilks = ilkReg.list();
+        for (uint256 i = 0; i < ilks.length; i++) {
+            address osm = osmMom.osms(ilks[i]);
+
+            if (osm == address(0)) continue;
+
+            try WardsLike(osm).wards(address(osmMom)) returns (uint256 ward) {
+                // Ignore Osm instances that have not relied on OsmMom.
+                if (ward != 1) continue;
+            } catch {
+                // Ignore any errors.
+                continue;
+            }
+
+            try OsmLike(osm).stopped() returns (uint256 stopped) {
+                // If any of the OSMs that match the conditions is not stopped, the spell was not executed yet.
+                if (stopped == 0) return false;
+            } catch {
+                // Ignore any errors.
+                continue;
+            }
+        }
+        return true;
+    }
 }
diff --git a/src/osm-stop/UniversalOsmStopSpell.t.integration.sol b/src/osm-stop/UniversalOsmStopSpell.t.integration.sol
@@ -113,10 +113,12 @@ contract UniversalOsmStopSpellTest is DssTest {
 
     function testUniversalOracleStopOnSchedule() public {
         _checkOsmStoppedStatus({ilks: ilkReg.list(), expected: 0});
+        assertFalse(spell.done(), "before: spell already done");
 
         spell.schedule();
 
         _checkOsmStoppedStatus({ilks: ilkReg.list(), expected: 1});
+        assertTrue(spell.done(), "after: spell not done");
     }
 
     function testUniversalOracleStopInBatches_Fuzz(uint256 batchSize) public {