Fix: resolve issues pointed out by ChainSecurity's audit report (#12)

* fix(SubProxyInit): rely on `MCD_ESM` instead of `MCD_END` * refactor: separate `StakingRewards` deploy and init scripts * feat: make deploy and init scripts configurable through env vars * chore: update .env.example with the required config * fix: remove duplicate entry from .env.example
makerdao · Oct 3, 2023 · 5efa2ca · 5efa2ca
1 parent 2a9215e
commit 5efa2ca
Show file tree

Hide file tree

Showing 14 changed files with 240 additions and 90 deletions.
diff --git a/.env.example b/.env.example
@@ -1,5 +1,12 @@
-export FOUNDRY_ETH_FROM='string: wallet address'
+# ./bash/* config
 export FOUNDRY_ETH_KEYSTORE_DIR='string: keystore directory'
 export FOUNDRY_ETH_PASSWORD_FILE='string: keystore file password'
+export FOUNDRY_ETH_FROM='address: the msg.sender address for scripts'
 
+# ScriptTools config
+export FOUNDRY_EXPORTS_OVERWRITE_LATEST='boolean: whether to overwrite the *-latest.json when running scripts. Should be set to `true`.'
+export FOUNDRY_ROOT_CHAINID='number: the chain ID where the script should run'
+
+# General config
+export ETH_RPC_URL='string: the JSON RPC provider URL'
 export ETHERSCAN_API_KEY='string: API key'
diff --git a/script/StakingRewardsDeploy.s.sol → script/01-StakingRewardsDeploy.s.sol b/script/StakingRewardsDeploy.s.sol → script/01-StakingRewardsDeploy.s.sol
@@ -19,43 +19,34 @@ import {Script} from "forge-std/Script.sol";
 import {stdJson} from "forge-std/StdJson.sol";
 import {ScriptTools} from "dss-test/ScriptTools.sol";
 
-import {ConfigReader} from "./helpers/Config.sol";
+import {Reader} from "./helpers/Reader.sol";
 import {StakingRewardsDeploy, StakingRewardsDeployParams} from "./dependencies/StakingRewardsDeploy.sol";
 import {
     VestedRewardsDistributionDeploy,
     VestedRewardsDistributionDeployParams
 } from "./dependencies/VestedRewardsDistributionDeploy.sol";
-import {StakingRewardsInit, StakingRewardsInitParams} from "./dependencies/StakingRewardsInit.sol";
-import {
-    VestedRewardsDistributionInit,
-    VestedRewardsDistributionInitParams
-} from "./dependencies/VestedRewardsDistributionInit.sol";
-import {VestInit, VestInitParams, VestCreateParams} from "./dependencies/VestInit.sol";
 
-contract StakingRewardsDeployScript is Script {
+contract Phase0StakingRewardsDeployScript is Script {
     using stdJson for string;
     using ScriptTools for string;
 
-    string internal constant NAME = "StakingRewards";
+    string internal constant NAME = "Phase0StakingRewardsDeploy";
 
     function run() external {
-        ConfigReader reader = new ConfigReader(ScriptTools.loadConfig());
+        Reader reader = new Reader(ScriptTools.loadConfig());
 
-        address admin = reader.readAddress(".admin");
-        address ngt = reader.readAddress(".ngt");
-        address nst = reader.readAddress(".nst");
+        address admin = reader.envOrReadAddress(".admin", "FOUNDRY_ADMIN");
+        address ngt = reader.envOrReadAddress(".ngt", "FOUNDRY_NGT");
+        address nst = reader.envOrReadAddress(".nst", "FOUNDRY_NST");
         address dist = reader.readAddressOptional(".dist");
         address farm = reader.readAddressOptional(".farm");
         address vest = reader.readAddressOptional(".vest");
-        uint256 vestTot = reader.readUintOptional(".vestTot");
-        uint256 vestBgn = reader.readUintOptional(".vestBgn");
-        uint256 vestTau = reader.readUintOptional(".vestTau");
 
         vm.startBroadcast();
 
         if (vest == address(0)) {
             vest = deployCode("DssVest.sol:DssVestMintable", abi.encode(ngt));
-            VestInit.init(VestInitParams({vest: vest, cap: type(uint256).max}));
+            ScriptTools.switchOwner(vest, msg.sender, admin);
         }
 
         if (farm == address(0)) {
@@ -70,26 +61,13 @@ contract StakingRewardsDeployScript is Script {
             );
         }
 
-        StakingRewardsInit.init(StakingRewardsInitParams({farm: farm, dist: dist}));
-
-        uint256 vestId;
-
-        if (vestTot > 0) {
-            vestId = VestInit.create(
-                VestCreateParams({vest: vest, usr: dist, tot: vestTot, bgn: vestBgn, tau: vestTau, eta: 0})
-            );
-
-            VestedRewardsDistributionInit.init(VestedRewardsDistributionInitParams({dist: dist, vestId: vestId}));
-        }
-
         vm.stopBroadcast();
 
         ScriptTools.exportContract(NAME, "admin", admin);
-        ScriptTools.exportContract(NAME, "dist", dist);
-        ScriptTools.exportContract(NAME, "farm", farm);
         ScriptTools.exportContract(NAME, "ngt", ngt);
         ScriptTools.exportContract(NAME, "nst", nst);
+        ScriptTools.exportContract(NAME, "dist", dist);
+        ScriptTools.exportContract(NAME, "farm", farm);
         ScriptTools.exportContract(NAME, "vest", vest);
-        ScriptTools.exportContract(NAME, "vestId", address(uint160(vestId)));
     }
 }
diff --git a/script/02-StakingRewardsInit.s.sol b/script/02-StakingRewardsInit.s.sol
@@ -0,0 +1,98 @@
+// SPDX-FileCopyrightText: © 2023 Dai Foundation <www.daifoundation.org>
+// SPDX-License-Identifier: AGPL-3.0-or-later
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+pragma solidity ^0.8.0;
+
+import {Script} from "forge-std/Script.sol";
+import {stdJson} from "forge-std/StdJson.sol";
+import {ScriptTools} from "dss-test/ScriptTools.sol";
+
+import {Reader} from "./helpers/Reader.sol";
+import {StakingRewardsInit, StakingRewardsInitParams} from "./dependencies/StakingRewardsInit.sol";
+import {
+    VestedRewardsDistributionInit,
+    VestedRewardsDistributionInitParams
+} from "./dependencies/VestedRewardsDistributionInit.sol";
+import {VestInit, VestInitParams, VestCreateParams} from "./dependencies/VestInit.sol";
+
+interface RelyLike {
+    function rely(address who) external;
+}
+
+interface WithGemLike {
+    function gem() external view returns (address);
+}
+
+interface StakingRewardsLike {
+    function rewardsToken() external view returns (address);
+}
+
+contract Phase0StakingRewardsInitScript is Script {
+    using stdJson for string;
+    using ScriptTools for string;
+
+    string internal constant NAME = "Phase0StakingRewardsInit";
+
+    function run() external {
+        Reader deps = new Reader(ScriptTools.loadDependencies());
+
+        address ngt = deps.envOrReadAddress(".ngt", "FOUNDRY_NGT");
+        address dist = deps.envOrReadAddress(".dist", "FOUNDRY_DIST");
+        address farm = deps.envOrReadAddress(".farm", "FOUNDRY_FARM");
+        address vest = deps.envOrReadAddress(".vest", "FOUNDRY_VEST");
+
+        Reader config = new Reader(ScriptTools.loadConfig());
+
+        uint256 vestCap = config.readUint(".vestCap");
+        uint256 vestTot = config.readUint(".vestTot");
+        uint256 vestBgn = config.readUint(".vestBgn");
+        uint256 vestTau = config.readUint(".vestTau");
+
+        require(WithGemLike(dist).gem() == ngt, "VestedRewardsDistribution/invalid-gem");
+        require(WithGemLike(vest).gem() == ngt, "DssVest/invalid-gem");
+        require(StakingRewardsLike(farm).rewardsToken() == ngt, "StakingRewards/invalid-rewards-token");
+
+        vm.startBroadcast();
+
+        // Grant minting rights on `ngt` to `vest`.
+        RelyLike(ngt).rely(vest);
+
+        // Define global max vesting ratio on `vest`.
+        VestInit.init(vest, VestInitParams({cap: vestCap}));
+
+        // Set `dist` with  `rewardsDistribution` role in `farm`.
+        StakingRewardsInit.init(farm, StakingRewardsInitParams({dist: dist}));
+
+        // Create the proper vesting stream for rewards distribution.
+        uint256 vestId = VestInit.create(
+            vest,
+            VestCreateParams({usr: dist, tot: vestTot, bgn: vestBgn, tau: vestTau, eta: 0})
+        );
+
+        // Set the `vestId` in `dist`
+        VestedRewardsDistributionInit.init(dist, VestedRewardsDistributionInitParams({vestId: vestId}));
+
+        vm.stopBroadcast();
+
+        ScriptTools.exportContract(NAME, "ngt", ngt);
+        ScriptTools.exportContract(NAME, "dist", dist);
+        ScriptTools.exportContract(NAME, "farm", farm);
+        ScriptTools.exportContract(NAME, "vest", vest);
+        ScriptTools.exportContract(NAME, "vestId", address(uint160(vestId)));
+        ScriptTools.exportContract(NAME, "vestTot", address(uint160(vestTot)));
+        ScriptTools.exportContract(NAME, "vestBgn", address(uint160(vestBgn)));
+        ScriptTools.exportContract(NAME, "vestTau", address(uint160(vestTau)));
+    }
+}
diff --git a/script/CheckStakingRewardsDeploy.s.sol → ...pt/09-CheckStakingRewardsDeployment.s.sol b/script/CheckStakingRewardsDeploy.s.sol → ...pt/09-CheckStakingRewardsDeployment.s.sol
@@ -19,7 +19,7 @@ import {Script} from "forge-std/Script.sol";
 import {stdJson} from "forge-std/StdJson.sol";
 import {ScriptTools} from "dss-test/ScriptTools.sol";
 
-import {ConfigReader} from "./helpers/Config.sol";
+import {Reader} from "./helpers/Reader.sol";
 
 interface WardsLike {
     function wards(address who) external view returns (uint256);
@@ -65,23 +65,24 @@ interface DssVestWithGemLike {
     function valid(uint256 _id) external view returns (bool);
 }
 
-contract CheckStakingRewardsDeployScript is Script {
+contract Phase0CheckStakingRewardsDeploymentScript is Script {
     using stdJson for string;
     using ScriptTools for string;
 
     function run() external returns (bool) {
-        ConfigReader reader = new ConfigReader(ScriptTools.loadConfig());
-
-        address admin = reader.readAddress(".admin");
-        address ngt = reader.readAddress(".ngt");
-        address nst = reader.readAddress(".nst");
-        address dist = reader.readAddress(".dist");
-        address farm = reader.readAddress(".farm");
-        address vest = reader.readAddress(".vest");
-        uint256 vestId = reader.readUint(".vestId");
-        uint256 vestTot = reader.readUint(".vestTot");
-        uint256 vestBgn = reader.readUint(".vestBgn");
-        uint256 vestTau = reader.readUint(".vestTau");
+        Reader deps = new Reader("");
+        deps.loadDependenciesOrConfig();
+
+        address admin = deps.envOrReadAddress(".admin", "FOUNDRY_ADMIN");
+        address ngt = deps.envOrReadAddress(".ngt", "FOUNDRY_NGT");
+        address nst = deps.envOrReadAddress(".nst", "FOUNDRY_NST");
+        address dist = deps.readAddress(".dist");
+        address farm = deps.readAddress(".farm");
+        address vest = deps.readAddress(".vest");
+        uint256 vestId = deps.readUint(".vestId");
+        uint256 vestTot = deps.readUint(".vestTot");
+        uint256 vestBgn = deps.readUint(".vestBgn");
+        uint256 vestTau = deps.readUint(".vestTau");
 
         require(WardsLike(dist).wards(admin) == 1, "VestedRewardsDistribution/pause-proxy-not-relied");
         require(VestedRewardsDistributionLike(dist).dssVest() == vest, "VestedRewardsDistribution/invalid-vest");

diff --git a/script/dependencies/SDAODeploy.sol b/script/dependencies/SDAODeploy.sol
@@ -18,15 +18,17 @@ pragma solidity ^0.8.0;
 import {ScriptTools} from "dss-test/ScriptTools.sol";
 import {SDAO} from "../../src/SDAO.sol";
 
+struct SDAODeployParams {
+    address deployer;
+    address owner;
+    string name;
+    string symbol;
+}
+
 library SDAODeploy {
-    function deploy(
-        address deployer,
-        address owner,
-        string memory name,
-        string memory symbol
-    ) internal returns (address token) {
-        token = address(new SDAO(name, symbol));
+    function deploy(SDAODeployParams memory p) internal returns (address sdao) {
+        sdao = address(new SDAO(p.name, p.symbol));
 
-        ScriptTools.switchOwner(token, deployer, owner);
+        ScriptTools.switchOwner(sdao, p.deployer, p.owner);
     }
 }
diff --git a/script/dependencies/StakingRewardsDeploy.sol b/script/dependencies/StakingRewardsDeploy.sol
@@ -24,7 +24,7 @@ struct StakingRewardsDeployParams {
 }
 
 library StakingRewardsDeploy {
-    function deploy(StakingRewardsDeployParams memory p) internal returns (address stakingRewards) {
-        stakingRewards = address(new StakingRewards(p.owner, address(0), p.rewardsToken, p.stakingToken));
+    function deploy(StakingRewardsDeployParams memory p) internal returns (address farm) {
+        farm = address(new StakingRewards(p.owner, address(0), p.rewardsToken, p.stakingToken));
     }
 }
diff --git a/script/dependencies/StakingRewardsInit.sol b/script/dependencies/StakingRewardsInit.sol
@@ -17,15 +17,32 @@ pragma solidity ^0.8.0;
 
 interface StakingRewardsLike {
     function setRewardsDistribution(address _rewardsDistribution) external;
+
+    function acceptOwnership() external;
+
+    function nominateNewOwner(address _owner) external;
 }
 
 struct StakingRewardsInitParams {
-    address farm;
     address dist;
 }
 
+struct StakingRewardsNominateNewOwnerParams {
+    address newOwner;
+}
+
 library StakingRewardsInit {
-    function init(StakingRewardsInitParams memory p) internal {
-        StakingRewardsLike(p.farm).setRewardsDistribution(p.dist);
+    function init(address farm, StakingRewardsInitParams memory p) internal {
+        StakingRewardsLike(farm).setRewardsDistribution(p.dist);
+    }
+
+    /// @dev `StakingRewards` ownership transfer is a 2-step process: nominate + acceptance.
+    function nominateNewOwner(address farm, StakingRewardsNominateNewOwnerParams memory p) internal {
+        StakingRewardsLike(farm).nominateNewOwner(p.newOwner);
+    }
+
+    /// @dev `StakingRewards` ownership transfer requires the new owner to explicitly accept it.
+    function acceptOwnership(address farm) internal {
+        StakingRewardsLike(farm).acceptOwnership();
     }
 }
diff --git a/script/dependencies/SubProxyDeploy.sol b/script/dependencies/SubProxyDeploy.sol
@@ -18,10 +18,15 @@ pragma solidity ^0.8.0;
 import {ScriptTools} from "dss-test/ScriptTools.sol";
 import {SubProxy} from "../../src/SubProxy.sol";
 
+struct SubProxyDeployParams {
+    address deployer;
+    address owner;
+}
+
 library SubProxyDeploy {
-    function deploy(address deployer, address owner) internal returns (address subProxy) {
+    function deploy(SubProxyDeployParams memory p) internal returns (address subProxy) {
         subProxy = address(new SubProxy());
 
-        ScriptTools.switchOwner(subProxy, deployer, owner);
+        ScriptTools.switchOwner(subProxy, p.deployer, p.owner);
     }
 }
diff --git a/script/dependencies/SubProxyInit.sol b/script/dependencies/SubProxyInit.sol
@@ -22,17 +22,22 @@ interface SubProxyLike {
     function rely(address who) external;
 }
 
+struct SubProxyInitParams {
+    address chainlog;
+    string name;
+}
+
 library SubProxyInit {
     using ScriptTools for string;
 
-    function init(address chainlog, address subProxy, string memory name) internal {
-        DssInstance memory mcd = MCD.loadFromChainlog(chainlog);
-        init(mcd, subProxy, name);
+    function init(address subProxy, SubProxyInitParams memory p) internal {
+        DssInstance memory mcd = MCD.loadFromChainlog(p.chainlog);
+        init(subProxy, mcd, p.name);
     }
 
-    function init(DssInstance memory mcd, address subProxy, string memory name) internal {
-        // Rely on `MCD_END` to allow `deny`ing `MCD_PAUSE_PROXY` after Emergency Shutdown.
-        SubProxyLike(subProxy).rely(address(mcd.end));
+    function init(address subProxy, DssInstance memory mcd, string memory name) internal {
+        // Rely on `MCD_ESM` to allow `deny`ing `MCD_PAUSE_PROXY` after Emergency Shutdown.
+        SubProxyLike(subProxy).rely(address(mcd.esm));
         // Add `SUBPROXY_{NAME}` to the chainlog.
         mcd.chainlog.setAddress(string.concat("SUBPROXY_", name).stringToBytes32(), subProxy);
     }

diff --git a/script/dependencies/VestInit.sol b/script/dependencies/VestInit.sol
@@ -33,12 +33,10 @@ interface DssVestLike {
 }
 
 struct VestInitParams {
-    address vest;
     uint256 cap;
 }
 
 struct VestCreateParams {
-    address vest;
     address usr;
     uint256 tot;
     uint256 bgn;
@@ -49,12 +47,12 @@ struct VestCreateParams {
 library VestInit {
     using ScriptTools for string;
 
-    function init(VestInitParams memory p) internal {
-        DssVestLike(p.vest).file("cap", p.cap);
+    function init(address vest, VestInitParams memory p) internal {
+        DssVestLike(vest).file("cap", p.cap);
     }
 
-    function create(VestCreateParams memory p) internal returns (uint256 vestId) {
-        vestId = DssVestLike(p.vest).create(
+    function create(address vest, VestCreateParams memory p) internal returns (uint256 vestId) {
+        vestId = DssVestLike(vest).create(
             p.usr,
             p.tot,
             p.bgn,
@@ -63,6 +61,6 @@ library VestInit {
             address(0) // mgr
         );
 
-        DssVestLike(p.vest).restrict(vestId);
+        DssVestLike(vest).restrict(vestId);
     }
 }