From 5efa2cac3b22fa94b2599cd83cb4bfea19747091 Mon Sep 17 00:00:00 2001 From: amusingaxl <112016538+amusingaxl@users.noreply.github.com> Date: Tue, 3 Oct 2023 06:53:27 -0300 Subject: [PATCH] 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 --- .env.example | 9 +- ...oy.s.sol => 01-StakingRewardsDeploy.s.sol} | 42 ++------ script/02-StakingRewardsInit.s.sol | 98 +++++++++++++++++++ ...=> 09-CheckStakingRewardsDeployment.s.sol} | 29 +++--- script/dependencies/SDAODeploy.sol | 18 ++-- script/dependencies/StakingRewardsDeploy.sol | 4 +- script/dependencies/StakingRewardsInit.sol | 23 ++++- script/dependencies/SubProxyDeploy.sol | 9 +- script/dependencies/SubProxyInit.sol | 17 ++-- script/dependencies/VestInit.sol | 12 +-- .../VestedRewardsDistributionInit.sol | 5 +- script/helpers/{Config.sol => Reader.sol} | 49 ++++++++-- .../5/template-staking-rewards-deploy.json | 8 ++ ...son => template-staking-rewards-init.json} | 7 +- 14 files changed, 240 insertions(+), 90 deletions(-) rename script/{StakingRewardsDeploy.s.sol => 01-StakingRewardsDeploy.s.sol} (62%) create mode 100644 script/02-StakingRewardsInit.s.sol rename script/{CheckStakingRewardsDeploy.s.sol => 09-CheckStakingRewardsDeployment.s.sol} (85%) rename script/helpers/{Config.sol => Reader.sol} (57%) create mode 100644 script/input/5/template-staking-rewards-deploy.json rename script/input/5/{template-staking-rewards.json => template-staking-rewards-init.json} (55%) diff --git a/.env.example b/.env.example index 319dec8..e60cf34 100644 --- 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 b/script/01-StakingRewardsDeploy.s.sol similarity index 62% rename from script/StakingRewardsDeploy.s.sol rename to script/01-StakingRewardsDeploy.s.sol index 2bd41e9..bfdbb77 100644 --- a/script/StakingRewardsDeploy.s.sol +++ b/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 new file mode 100644 index 0000000..6ac23a6 --- /dev/null +++ b/script/02-StakingRewardsInit.s.sol @@ -0,0 +1,98 @@ +// SPDX-FileCopyrightText: © 2023 Dai Foundation +// 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 . +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 b/script/09-CheckStakingRewardsDeployment.s.sol similarity index 85% rename from script/CheckStakingRewardsDeploy.s.sol rename to script/09-CheckStakingRewardsDeployment.s.sol index 79bf0b3..7f567f6 100644 --- a/script/CheckStakingRewardsDeploy.s.sol +++ b/script/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 index e6238e2..551e392 100644 --- 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 index 71cab55..9e8c605 100644 --- 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 index c0d5ba6..fc457b3 100644 --- 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 index cdb923f..e5ff0dd 100644 --- 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 index 8b22561..4505484 100644 --- 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 index 61d6e10..c09dd5e 100644 --- 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); } } diff --git a/script/dependencies/VestedRewardsDistributionInit.sol b/script/dependencies/VestedRewardsDistributionInit.sol index f01aa73..8f45797 100644 --- a/script/dependencies/VestedRewardsDistributionInit.sol +++ b/script/dependencies/VestedRewardsDistributionInit.sol @@ -20,12 +20,11 @@ interface VestedRewardsDistributionLike { } struct VestedRewardsDistributionInitParams { - address dist; uint256 vestId; } library VestedRewardsDistributionInit { - function init(VestedRewardsDistributionInitParams memory p) internal { - VestedRewardsDistributionLike(p.dist).file("vestId", p.vestId); + function init(address dist, VestedRewardsDistributionInitParams memory p) internal { + VestedRewardsDistributionLike(dist).file("vestId", p.vestId); } } diff --git a/script/helpers/Config.sol b/script/helpers/Reader.sol similarity index 57% rename from script/helpers/Config.sol rename to script/helpers/Reader.sol index 8cdd93d..6ccd0ae 100644 --- a/script/helpers/Config.sol +++ b/script/helpers/Reader.sol @@ -15,23 +15,44 @@ // along with this program. If not, see . pragma solidity ^0.8.0; +import {Vm} from "forge-std/Vm.sol"; import {stdJson} from "forge-std/StdJson.sol"; +import {ScriptTools} from "dss-test/ScriptTools.sol"; -contract ConfigReader { +contract Reader { using stdJson for string; - string internal config; + Vm private constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code"))))); + string internal data; - constructor(string memory _config) { - config = _config; + constructor(string memory _data) { + data = _data; + } + + function loadConfig() external { + data = ScriptTools.loadConfig(); + } + + function loadDependencies() external { + data = ScriptTools.loadDependencies(); + } + + function loadDependenciesOrConfig() external { + try this.loadDependencies() { + if (bytes(data).length == 0) { + revert(""); + } + } catch { + this.loadConfig(); + } } function readAddress(string memory key) external returns (address) { - return config.readAddress(key); + return data.readAddress(key); } function readUint(string memory key) external returns (uint256) { - return config.readUint(key); + return data.readUint(key); } function readAddressOptional(string memory key) external returns (address) { @@ -57,4 +78,20 @@ contract ConfigReader { return def; } } + + function envOrReadAddress(string memory key, string memory envKey) external returns (address) { + try vm.envAddress(envKey) returns (address addr) { + return addr; + } catch { + return this.readAddress(key); + } + } + + function envOrReadUint(string memory key, string memory envKey) external returns (uint256) { + try vm.envUint(envKey) returns (uint256 n) { + return n; + } catch { + return this.readUint(key); + } + } } diff --git a/script/input/5/template-staking-rewards-deploy.json b/script/input/5/template-staking-rewards-deploy.json new file mode 100644 index 0000000..df73510 --- /dev/null +++ b/script/input/5/template-staking-rewards-deploy.json @@ -0,0 +1,8 @@ +{ + "admin": "address: usually the MCD_PAUSE_PROXY address", + "ngt": "address", + "nst": "address", + "dist": "address", + "farm": "address", + "vest": "address" +} diff --git a/script/input/5/template-staking-rewards.json b/script/input/5/template-staking-rewards-init.json similarity index 55% rename from script/input/5/template-staking-rewards.json rename to script/input/5/template-staking-rewards-init.json index 9bf2cf9..daaf29b 100644 --- a/script/input/5/template-staking-rewards.json +++ b/script/input/5/template-staking-rewards-init.json @@ -1,10 +1,5 @@ { - "admin": "address: usually the MCD_PAUSE_PROXY address", - "ngt": "address", - "nst": "address", - "dist": "address", - "farm": "address", - "vest": "address", + "vestCap": "uint256: global max issuance rate per token", "vestTot": "uint256: total amount of the vesting stream", "vestBgn": "uint256: timestamp of the beginning of the vesting stream", "vestTau": "uint256: duration of the vesting stream in seconds"