Modify officeHours to be in action (#36)

brianmcmichael · web-flow · commit d8d3aecbd5a0 · 2021-01-08T16:00:28.000-05:00
diff --git a/README.md b/README.md
@@ -23,12 +23,11 @@ import {DssAction} from "lib/dss-exec-lib/src/DssAction.sol";
 
 contract SpellAction is DssAction {
 
-    // This can be hardcoded away later or can use the chain-log
-    constructor(address lib) DssAction(lib) public {}
+    constructor(address lib, bool officeHours) DssAction(lib, officeHours) public {}
 
     uint256 constant MILLION  = 10 ** 6;
 
-    function execute() external {
+    function actions() public override {
         setGlobalDebtCeiling(1500 * MILLION);
         setIlkDebtCeiling("ETH-A", 10 * MILLION);
     }
@@ -37,13 +36,16 @@ contract SpellAction is DssAction {
 
 The `SpellAction.sol` file must always inherit `DssAction` from `lib/dss-exec-lib`.
 
+The developer must override the `actions()` function and place all spell actions within. This is called by the `execute()` function in the pause, which is subject to an optional limiter for office hours.
+
+*Note:* All variables within the SpellAction MUST be defined as constants, or assigned at runtime inside of the `actions()` function. Variable memory storage is not available within a Spell Action due to the underlying delegatecall mechanisms.
+
 The spell itself is deployed as follows:
 
 ```js
 new DssExec(
     "A test dss exec spell",           // Description
     now + 30 days,                     // Expiration
-    true,                              // OfficeHours enabled
     address(new SpellAction(execlib))
 );
 ```
@@ -158,7 +160,7 @@ CollateralOpts memory XMPL_A = CollateralOpts({
     gem:                   0xCE4F3774620764Ea881a8F8840Cbe0F701372283,
     join:                  0xa30925910067a2d9eB2a7358c017E6075F660842,
     flip:                  0x32c6DF17f8E94694977aa41A595d8dc583836A51,
-    pip:                   0x9eb923339c24c40Bef2f4AF4961742AA7C23EF3a, 
+    pip:                   0x9eb923339c24c40Bef2f4AF4961742AA7C23EF3a,
     isLiquidatable:        true,
     isOSM:                 true,
     whitelistOSM:          true,
diff --git a/src/DssAction.sol b/src/DssAction.sol
@@ -52,16 +52,39 @@ interface OracleLike {
     function diss(address[] calldata) external;
 }
 
-contract DssAction {
+abstract contract DssAction {
 
     address public immutable lib;
+    bool    public immutable officeHours;
 
     // Changelog address applies to MCD deployments on
     //        mainnet, kovan, rinkeby, ropsten, and goerli
     address constant public LOG = 0xdA0Ab1e0017DEbCd72Be8599041a2aa3bA7e740F;
 
-    constructor(address lib_) public {
+    constructor(address lib_, bool officeHours_) public {
         lib = lib_;
+        officeHours = officeHours_;
+    }
+
+    // DssExec calls execute. We limit this function subject to officeHours modifier.
+    function execute() external limited {
+        actions();
+    }
+
+    // DssAction developer must override `actions()` and place all actions to be called inside.
+    //   The DssExec function will call this subject to the officeHours limiter
+    //   By keeping this function public we allow simulations of `execute()` on the actions outside of the cast time.
+    function actions() public virtual;
+
+    // Modifier required to
+    modifier limited {
+        if (officeHours) {
+            uint day = (block.timestamp / 1 days + 3) % 7;
+            require(day < 5, "Can only be cast on a weekday");
+            uint hour = block.timestamp / 1 hours % 24;
+            require(hour >= 14 && hour < 21, "Outside office hours");
+        }
+        _;
     }
 
     /****************************/
diff --git a/src/DssExec.sol b/src/DssExec.sol
@@ -29,6 +29,10 @@ interface Changelog {
     function getAddress(bytes32) external view returns (address);
 }
 
+interface SpellAction {
+    function officeHours() external view returns (bool);
+}
+
 contract DssExec {
 
     Changelog      constant public log   = Changelog(0xdA0Ab1e0017DEbCd72Be8599041a2aa3bA7e740F);
@@ -38,23 +42,24 @@ contract DssExec {
     bytes32       immutable public tag;
     address       immutable public action;
     uint256       immutable public expiration;
-    bool          immutable public officeHours;
     PauseAbstract immutable public pause;
 
     // Provides a descriptive tag for bot consumption
     // This should be modified weekly to provide a summary of the actions
     // Hash: seth keccak -- "$(wget https://<executive-vote-canonical-post> -q -O - 2>/dev/null)"
     string                  public description;
 
+    function officeHours() external view returns (bool) {
+        return SpellAction(action).officeHours();
+    }
+
     // @param _description  A string description of the spell
     // @param _expiration   The timestamp this spell will expire. (Ex. now + 30 days)
-    // @param _officeHours  Limits the executive cast time to office hours (true for limit)
     // @param _spellAction  The address of the spell action
-    constructor(string memory _description, uint256 _expiration, bool _officeHours, address _spellAction) public {
+    constructor(string memory _description, uint256 _expiration, address _spellAction) public {
         pause       = PauseAbstract(log.getAddress("MCD_PAUSE"));
         description = _description;
         expiration  = _expiration;
-        officeHours = _officeHours;
         action      = _spellAction;
 
         sig = abi.encodeWithSignature("execute()");
@@ -64,24 +69,14 @@ contract DssExec {
         tag = _tag;
     }
 
-    modifier limited {
-        if(officeHours) {
-            uint day = (now / 1 days + 3) % 7;
-            require(day < 5, "Can only be cast on a weekday");
-            uint hour = now / 1 hours % 24;
-            require(hour >= 14 && hour < 21, "Outside office hours");
-        }
-        _;
-    }
-
     function schedule() public {
         require(now <= expiration, "This contract has expired");
         require(eta == 0, "This spell has already been scheduled");
         eta = now + PauseAbstract(pause).delay();
         pause.plot(action, tag, sig, eta);
     }
 
-    function cast() limited public {
+    function cast() public {
         require(!done, "spell-already-cast");
         done = true;
         pause.exec(action, tag, sig, eta);
diff --git a/src/DssExecFactory.sol b/src/DssExecFactory.sol
@@ -26,17 +26,16 @@ contract DssExecFactory {
     //
     // @param description  A string description of the spell
     // @param expiration   The timestamp this spell will expire. (Ex. now + 30 days)
-    // @param officeHours  Limits the executive cast time to office hours (true for limit)
     // @param spellAction  The address of the spell action contract (DssAction)
-    function newExec(string memory description, uint256 expiration, bool officeHours, address spellAction) public returns (address exec) {
-        exec = address(new DssExec(description, expiration, officeHours, spellAction));
+    function newExec(string memory description, uint256 expiration, address spellAction) public returns (address exec) {
+        exec = address(new DssExec(description, expiration, spellAction));
     }
 
     function newWeeklyExec(string memory description, address spellAction) public returns (address exec) {
-        exec = newExec(description, now + 30 days, true, spellAction);
+        exec = newExec(description, now + 30 days, spellAction);
     }
 
     function newMonthlyExec(string memory description, address spellAction) public returns (address exec) {
-        exec = newExec(description, now + 4 days, true, spellAction);
+        exec = newExec(description, now + 4 days, spellAction);
     }
 }
diff --git a/src/test/DssAction.t.sol b/src/test/DssAction.t.sol
@@ -274,7 +274,7 @@ contract ActionTest is DSTest {
 
         lib = new DssExecLib();
 
-        action = new DssTestAction(address(lib));
+        action = new DssTestAction(address(lib), true);
 
         init_collateral("gold", address(action));
 
diff --git a/src/test/DssExec.t.sol b/src/test/DssExec.t.sol
@@ -41,11 +41,11 @@ interface Hevm {
 contract DssLibSpellAction is DssAction { // This could be changed to a library if the lib is hardcoded and the constructor removed
 
     // This can be hardcoded away later or can use the chain-log
-    constructor(address lib) DssAction(lib) public {}
+    constructor(address lib, bool ofcHrs) DssAction(lib, ofcHrs) public {}
 
     uint256 constant MILLION  = 10 ** 6;
 
-    function execute() external {
+    function actions() public override {
         CollateralOpts memory XMPL_A = CollateralOpts({
             ilk:                   "XMPL-A",
             gem:                   0xCE4F3774620764Ea881a8F8840Cbe0F701372283,
@@ -206,8 +206,7 @@ contract DssLibExecTest is DSTest, DSMath {
         spell = new DssExec(
             "A test dss exec spell",                    // Description
             now + 30 days,                              // Expiration
-            true,                                       // OfficeHours enabled
-            address(new DssLibSpellAction(execlib))
+            address(new DssLibSpellAction(execlib, true))
         );
 
         //
@@ -581,8 +580,7 @@ contract DssLibExecTest is DSTest, DSMath {
         new DssExec(
             "Basic Spell",                              // Description
             now + 30 days,                              // Expiration
-            true,                                       // OfficeHours enabled
-            address(new DssLibSpellAction(execlib))
+            address(new DssLibSpellAction(execlib, true))
         );
     }
 }
diff --git a/src/test/DssExecFactory.t.sol b/src/test/DssExecFactory.t.sol
@@ -42,11 +42,11 @@ interface Hevm {
 contract DssLibSpellAction is DssAction { // This could be changed to a library if the lib is hardcoded and the constructor removed
 
     // This can be hardcoded away later or can use the chain-log
-    constructor(address lib) DssAction(lib) public {}
+    constructor(address lib, bool ofcHrs) DssAction(lib, ofcHrs) public {}
 
     uint256 constant MILLION  = 10 ** 6;
 
-    function execute() external {
+    function actions() public override {
         CollateralOpts memory XMPL_A = CollateralOpts({
             ilk:                   "XMPL-A",
             gem:                   0xCE4F3774620764Ea881a8F8840Cbe0F701372283,
@@ -211,12 +211,11 @@ contract DssLibExecTest is DSTest, DSMath {
         factory = new DssExecFactory();
 
         // Only an action needs to be crafted, it can be passed to the factory which is already deployed.
-        action = new DssLibSpellAction(execlib);
+        action = new DssLibSpellAction(execlib, true);
 
         address _factorySpell = factory.newExec(
             "A test dss exec spell",                    // Description
             now + 30 days,                              // Expiration
-            true,                                       // OfficeHours enabled
             address(action)
         );
 
@@ -599,7 +598,6 @@ contract DssLibExecTest is DSTest, DSMath {
         factory.newExec(
             "A test dss exec spell",                    // Description
             now + 30 days,                              // Expiration
-            true,                                       // OfficeHours enabled
             address(action)
         );
     }
diff --git a/src/test/DssTestAction.sol b/src/test/DssTestAction.sol
@@ -24,7 +24,9 @@ import "../DssAction.sol";
 
 contract DssTestAction is DssAction {
 
-    constructor(address lib) DssAction(lib) public {}
+    constructor(address lib, bool ofcHrs) DssAction(lib, ofcHrs) public {}
+
+    function actions() public override {}
 
     /**********************/
     /*** Authorizations ***/

Original file line number	Diff line number	Diff line change
`@@ -26,17 +26,16 @@ contract DssExecFactory {`
`26`	`26`	`//`
`27`	`27`	`// @param description A string description of the spell`
`28`	`28`	`// @param expiration The timestamp this spell will expire. (Ex. now + 30 days)`
`29`		`- // @param officeHours Limits the executive cast time to office hours (true for limit)`
`30`	`29`	`// @param spellAction The address of the spell action contract (DssAction)`
`31`		`- function newExec(string memory description, uint256 expiration, bool officeHours, address spellAction) public returns (address exec) {`
`32`		`- exec = address(new DssExec(description, expiration, officeHours, spellAction));`
	`30`	`+ function newExec(string memory description, uint256 expiration, address spellAction) public returns (address exec) {`
	`31`	`+ exec = address(new DssExec(description, expiration, spellAction));`
`33`	`32`	`}`
`34`	`33`
`35`	`34`	`function newWeeklyExec(string memory description, address spellAction) public returns (address exec) {`
`36`		`- exec = newExec(description, now + 30 days, true, spellAction);`
	`35`	`+ exec = newExec(description, now + 30 days, spellAction);`
`37`	`36`	`}`
`38`	`37`
`39`	`38`	`function newMonthlyExec(string memory description, address spellAction) public returns (address exec) {`
`40`		`- exec = newExec(description, now + 4 days, true, spellAction);`
	`39`	`+ exec = newExec(description, now + 4 days, spellAction);`
`41`	`40`	`}`
`42`	`41`	`}`