More Checklist Items + Refinements (#11)

* Revert "Cleanups & Refinements (#10)" This reverts commit 4bdc4fa. * cleanup * update readme * static interface checks + cleanups * add risk parameter changes checklists * update wordlist * fix teleport onboarding file name * fix teleport onboarding file name in readme * add rates manual check via script * typos * fix + cleanup spell crafter goerli * add community repo link + refinements to spell mainnet checklists * only list static interfaces functions used in spell code * more match typos * static description on goerli spell * avoid using same deployer for mainnet and testnets * check if contract requires rely the ESM for ES denyProxy keeper tasks * update wordlist * consider using dss-exec-lib actions where possible to avoid introducing interfaces * cleanup * restrict immutable visibility to addrs fetched from chainlog and use constant for literal addrs * move archive step together with spell deployment * address review comments + cleanups * update wordlist * improve archive checks * update deployed spell checklists * update deployment crafting checklists * update exec hash crafting checklists * update wordlist * update exec hash review checklists * indentation * add mom checklists * update readme * missing ) Co-authored-by: Chris Smith <[email protected]> * +patches bump notation Co-authored-by: Chris Smith <[email protected]> * address review comments * update wordlist * add lib checklists + improve dss-interfaces checks * cleanup * diff cleanups for review checklists * diff cleanups crafter checklists --------- Co-authored-by: Chris Smith <[email protected]>
makerdao · Apr 15, 2023 · 492326a · 492326a
1 parent 4bdc4fa
commit 492326a
Show file tree

Hide file tree

Showing 10 changed files with 219 additions and 53 deletions.
diff --git a/.wordlist.txt b/.wordlist.txt
@@ -123,3 +123,10 @@ SPDX
 prepended
 OpenZeppelin
 Solmate
+setIlkAutoLineDebtCeiling
+ESM
+testnet
+authing
+hardcoded
+fileables
+configs
diff --git a/README.md b/README.md
@@ -8,11 +8,11 @@
 >These checklists emerged during the review and crafting processes as ways to avoid pitfalls, allowing protocol engineers to focus on more in-depth reviews.
 >Future workflow improvements will be reflected in these checklists.
 
-## Core Contracts
+## Checklists
 
-- [Standards](core/standards.md)
+### Core Contracts
 
-## Checklists
+- [Standards](core/standards.md)
 
 ### Spells
 
@@ -27,4 +27,5 @@
 - [Spell Reviewer Mainnet Checklist](./spell/spell-reviewer-mainnet-checklist.md)
 - [Spell Reviewer Collateral Onboarding Checklist](./spell/collateral-onboarding-checklist.md)
 - [Spell Reviewer RWA Onboarding Checklist](./spell/rwa-onboarding-checklist.md)
-- [Spell Reviewer Teleport Onboarding Checklist](./spell/teleport-oboarding-checklist.md)
+- [Spell Reviewer Teleport Onboarding Checklist](./spell/teleport-onboarding-checklist.md)
+- [Spell Reviewer Mom Onboarding Checklist](./spell/mom-onboarding-checklist.md)
diff --git a/spell/collateral-onboarding-checklist.md b/spell/collateral-onboarding-checklist.md
@@ -4,7 +4,7 @@
       * [ ] deployed via deployer (OSM)
         * [ ] contract is verified on etherscan
           * [ ] ensure solc version matches source
-          * [ ] ensure optimization matches source
+          * [ ] ensure optimization matches source configs
           * [ ] ensure license `AGPLv3` is specified
           * [ ] ensure source matches github code (i.e. diffcheck via vscode `code --diff etherscan.sol github.sol`)
         * [ ] constructor args are correct

diff --git a/spell/l2s-relay-checklist.md b/spell/l2s-relay-checklist.md
diff --git a/spell/mom-onboarding-checklist.md b/spell/mom-onboarding-checklist.md
@@ -0,0 +1,30 @@
+# Mom Onboarding Checklist
+  * [ ] Deployed Contracts
+    * [ ] `Mom`
+      * [ ] deployed via deployer
+        * [ ] ensure different deployer is used for mainnet and testnet to avoid contracts with same address and different sources
+        * [ ] contract is verified on etherscan
+          * [ ] ensure solc version matches source
+          * [ ] ensure optimization are off
+          * [ ] ensure license `AGPLv3` is specified
+          * [ ] ensure source matches github code (i.e. diffcheck via vscode `code --diff etherscan.sol github.sol`)
+        * [ ] constructor args are correct
+        * [ ] check `owner`
+          * [ ] `MCD_PAUSE_PROXY` is set as `owner` via `setOwner` (that will remove the deployer as `owner`)
+        * [ ] ensure deployer is included in `addresses_deployer.sol` if not already present 
+    * [ ] Onboarding Actions
+      * [ ] authorize `Mom` on the circuit breaker target modules (e.g. `vat.rely(LineMom)`)
+      * [ ] ensure `owner` is Pause Proxy via sanity check (e.g. `require(MomLike(MOM).owner() == PAUSE_PROXY);`)
+      * [ ] file eventual fileables
+      * [ ] set `authority` the `chief` (e.g. `MomLike(MOM).setAuthority(CHIEF);`)
+      * [ ] consider other spell actions required to complete the onboarding like adding target contracts to local mom list (e.g. `LineMom`).
+    * [ ] New Chainlog Entry
+        * [ ] via `DssExecLib.setChangelogAddres("MOM", MOM);`
+    * [ ] Chainlog Bump
+      * [ ] Patch `x.x.+1`
+      * [ ] via `DssExecLib.setChangelogVersion("1.14.x");`
+    * [ ] Test Coverage (Follow Previous Test Patterns)
+      * [ ] `testMom`
+      * [ ] ensure new chainlog entries are included in `addresses_<mainnet, goerli>.sol`
+      * [ ] ensure deployer addresses are included into `addresses_deployers.sol` (**to keep up to date**)
+      * [ ] `config.sol` chainlog version bump
diff --git a/spell/spell-crafter-goerli-workflow.md b/spell/spell-crafter-goerli-workflow.md
@@ -10,13 +10,15 @@ PR: https://github.com/makerdao/spells-goerli/pull/TODO
 
 ### Steps:
 * [ ] Create Branch on GitHub `PE-<kanban ticket issue number>`
-  * [ ] Note: for now current consensus is to use the same ticket issue number as per `spells-mainnet` (this could be revised)
+  * [ ] Ensure the same ticket issue number is used for `spells-mainnet` spell
 * [ ] Pull `master` Locally and Checkout Branch (IF Branch is created via GitHub)
 * [ ] Pull `master` Locally, Create and Checkout Branch (IF Branch was not created via GitHub)
 * [ ] Cleanup Previous Spell's Actions in `DssSpell.sol`
   * [ ] Check previous spells in the `archive` folder for cleanup patterns
-* [ ] Cleanup `src/test/config.sol` 
+* [ ] Cleanup `src/test/config.sol`
   * [ ] Set `deployed_spell` to `address(0)`
+  * [ ] Set `deployed_spell_created` to `0`
+  * [ ] Set `deployed_spell_block` to `0`
 * [ ] Cleanup Specific Tests in `DssSpell.t.sol`
   * [ ] Check previous spells in the `archive` folder for cleanup patterns
   * [ ] Disable specific tests IF Not Used (e.g. `testCollateralIntegrations`, `testNewChainlogValues`, `testNewIlkRegistryValues`, ...)
@@ -45,6 +47,7 @@ PR: https://github.com/makerdao/spells-goerli/pull/TODO
   * [ ] Pragma
     * [ ] Current solc version `0.8.16`
   * [ ] Interfaces
+    * [ ] Consider using `DssExecLib` actions where possible (to avoid introducing interfaces where not required)
     * [ ] Avoid `dss-interfaces` multi-import layout (see [issue #69](https://github.com/makerdao/dss-interfaces/issues/69))
     * [ ] Prefer single import layout
       * [ ] `import { VatAbstract } from "dss-interfaces/dss/VatAbstract.sol";`
@@ -70,6 +73,7 @@ PR: https://github.com/makerdao/spells-goerli/pull/TODO
     * [ ] Check with Oracle CU
       * [ ] Check IF oracle deployment is required (e.g. univ3-lp-oracle, new ilk pip, ...)
   * [ ] Ensure every spell variable is declared as public/internal
+  * [ ] Consider `immutable` visibility when fetching addresses from the `ChainLog` via `DssExecLib.getChangelogAddress` and use `constant` for static addresses
   * [ ] Add New Addresses in the ChainLog
   * [ ] Bump ChainLog, accordingly with spec (`major`, `minor`, `patch`)
     * [ ] MAJOR -> New Vat
@@ -87,7 +91,7 @@ PR: https://github.com/makerdao/spells-goerli/pull/TODO
 * [ ] Add new ChainLog Address in `addresses_goerli.sol` (e.g. Collateral Onboarding)
 * [ ] Run Tests `make test` or `make test match=<test_name>` to inspect debug traces
   * [ ] Ensure Good Coverage
-  * [ ] Ensure every test function is declared as public if enabled or private if not
+  * [ ] Ensure every test function is declared as public if enabled or private if disabled
   * [ ] Tests PASS via `make test`
 * [ ] Open PR & Add Reviewers
 * [ ] Iterate until polls are ended and exec doc is confirmed
@@ -96,6 +100,7 @@ PR: https://github.com/makerdao/spells-goerli/pull/TODO
 * [ ] Pre-Deploy Setup and Checks (currently via `dapptools`)
   * [ ] Set local env (`.sethrc`)
     * [ ] Deployer
+      * [ ] Avoid using the same deployer for mainnet and testnet (to avoid deploying contracts with the same address but different sources)
       * [ ] `export ETH_PASSWORD=~/.env/password.txt`
       * [ ] `export ETH_KEYSTORE=~/.ethereum/keystore`
       * [ ] `export ETH_FROM=<address>`
@@ -105,8 +110,8 @@ PR: https://github.com/makerdao/spells-goerli/pull/TODO
       * [ ] `export ETH_GAS_PRICE=$(seth --to-wei X gwei)`
         * [ ] Check current gas price using `seth gas-price` and Set `ETH_GAS_PRICE` accordingly (e.g. 40 gwei)
       * [ ] `export ETH_PRIO_FEE=$(seth --to-wei X gwei)`
-        * [ ] Check [current gas priority fee](https://etherscan.io/gastracker) and set `ETH_PRIO_FEE` accordingly (e.g. 2 gwei)
-    * [ ] Mainnet RPC URL
+        * [ ] Check current gas priority fee and set `ETH_PRIO_FEE` accordingly (e.g. 2 gwei)
+    * [ ] Goerli RPC URL
       * [ ] `export ETH_RPC_URL=<url>`
     * [ ] Etherscan API KEY
       * [ ] `export ETHERSCAN_API_KEY=<key>`
@@ -115,19 +120,21 @@ PR: https://github.com/makerdao/spells-goerli/pull/TODO
     * [ ] `seth ls`
     * [ ] `seth chain`
 * [ ] Deploy spell on Goerli via `make deploy`
+  * [ ] Ensure `src/test/config.sol` is edited correctly
+    * [ ] `deployed_spell: address(<deployed_spell_address>)`
+    * [ ] `deployed_spell_created: <timestamp>`
+    * [ ] `deployed_spell_block: <block number>`
+    * [ ] validate the above values via `make deploy-info tx=<tx_hash>`
   * [ ] Ensure spell is verified on etherscan
-* [ ] Add deployed spell address to `config.sol`
-  * [ ] `deployed_spell: address(<deployed_spell_address>)`
-* [ ] Run Tests Locally with deployed spell address
+  * [ ] Ensure local tests PASS against deployed spell run via the deploy script
+  * [ ] Push auto-generated commit
+* [ ] Archive Spell via `make archive-spell` for current date or `date="YYYY-MM-DD" make archive-spell` (date as per cast timestamp)
 * [ ] Commit & Push for Review
 * [ ] Wait for CI to PASS
 * [ ] Wait for at Least Two Approvals
-* [ ] Archive spell via `date="YYYY-MM-DD" make archive-spell` (date as per the exec doc)
-* [ ] Commit & Push Archive for Review
-* [ ] Wait for Merge Approvals and CI to PASS
-* [ ] Squash & Merge
 * [ ] Cast Spell via `make cast-spell` (ONLY ON GOERLI)
 * [ ] Check `cast()` trace (via [EthTx](https://ethtx.info/))
   * [ ] Ensure no reverts are present that block execution
     * [ ] Inspect low level call reverts if expected
   * [ ] Ensure all actions are executed and not out-of-gas errors are present
+* [ ] Squash & Merge
diff --git a/spell/spell-crafter-mainnet-workflow.md b/spell/spell-crafter-mainnet-workflow.md
@@ -10,13 +10,15 @@ Repo: https://github.com/makerdao/spells-mainnet
 
 ### Steps:
 * [ ] Create Branch on GitHub `PE-<kanban ticket issue number>`
-  * [ ] Note: for now current consensus is to use the same ticket issue number as per `spells-goerli` (this could be revised)
+  * [ ] Ensure the same ticket issue number is used as per `spells-goerli` spell
 * [ ] Pull `master` Locally and Checkout Branch (IF Branch is created via GitHub)
 * [ ] Pull `master` Locally, Create and Checkout Branch (IF Branch was not created via GitHub)
 * [ ] Cleanup Previous Spell's Actions in `DssSpell.sol` (diffcheck with Goerli)
   * [ ] Check previous spells in the `archive` folder for cleanup patterns
 * [ ] Cleanup `src/test/config.sol`
   * [ ] Set `deployed_spell` to `address(0)`
+  * [ ] Set `deployed_spell_created` to `0`
+  * [ ] Set `deployed_spell_block` to `0`
   * [ ] Consider to add `previous_spell` address if it haven't been executed yet
 * [ ] Cleanup Specific Tests in `DssSpell.t.sol`
   * [ ] Check previous spells in the `archive` folder for cleanup patterns
@@ -47,6 +49,7 @@ Repo: https://github.com/makerdao/spells-mainnet
   * [ ] Pragma
     * [ ] Current solc version `0.8.16`
   * [ ] Interfaces
+    * [ ] Consider using `DssExecLib` actions where possible (to avoid introducing interfaces where not required)
     * [ ] Avoid `dss-interfaces` multi-import layout (see [issue #69](https://github.com/makerdao/dss-interfaces/issues/69))
     * [ ] Prefer single import layout
       * [ ] `import { VatAbstract } from "dss-interfaces/dss/VatAbstract.sol";`
@@ -74,6 +77,7 @@ Repo: https://github.com/makerdao/spells-mainnet
       * [ ] Check IF oracle deployment is required (e.g. univ3-lp-oracle, new ilk pip, ...)
       * [ ] Note: oracle should be deployed on mainnet before Friday (usually Wed-Thu)
   * [ ] Ensure every spell variable is declared as public/internal
+  * [ ] Consider `immutable` visibility when fetching addresses from the `ChainLog` via `DssExecLib.getChangelogAddress` and use `constant` for static addresses
   * [ ] Add New Addresses in the ChainLog
   * [ ] Bump ChainLog, accordingly with spec (`major`, `minor`, `patch`)
     * [ ] MAJOR -> New Vat
@@ -98,13 +102,17 @@ Repo: https://github.com/makerdao/spells-mainnet
 * [ ] Confirm Exec Doc Actions
 * [ ] Make sure CI PASS
 * [ ] Add Exec Hash
-  * [ ] Ensure exec copy is merged
-  * [ ] Reference either latest change or merge commit
+  * [ ] Run `make exec-hash` for current date, or `date=YYYY-MM-DD` and update spell code accordingly
+    * [ ] Executive vote file name and date is correct
+    * [ ] [community](https://github.com/makerdao/community) repo commit hash corresponds to latest change
+    * [ ] Raw GitHub URL is correct
+    * [ ] Exec hash is correct
   * [ ] Ensure `description` date in `DssSpell.sol` matches exec copy one
 * [ ] Wait for at Least Two Approvals with local tests to deploy
 * [ ] Pre-Deploy Setup and Checks (currently via `dapptools`)
   * [ ] Set local env (`.sethrc`)
     * [ ] Deployer
+      * [ ] Avoid using the same deployer for mainnet and testnet (to avoid deploying contracts with the same address but different sources)
       * [ ] `export ETH_PASSWORD=~/.env/password.txt`
       * [ ] `export ETH_KEYSTORE=~/.ethereum/keystore`
       * [ ] `export ETH_FROM=<address>`
@@ -125,19 +133,21 @@ Repo: https://github.com/makerdao/spells-mainnet
     * [ ] `seth ls`
     * [ ] `seth chain`
 * [ ] Deploy spell on Goerli via `make deploy`
+  * [ ] Ensure `src/test/config.sol` is edited correctly
+    * [ ] `deployed_spell: address(<deployed_spell_address>)`
+    * [ ] `deployed_spell_created: <timestamp>`
+    * [ ] `deployed_spell_block: <block number>`
+    * [ ] validate the above values via `make deploy-info tx=<tx_hash>`
   * [ ] Ensure spell is verified on etherscan
-* [ ] Add deployed spell address to `config.sol`
-  * [ ] `deployed_spell: address(<deployed_spell_address>)`
-* [ ] Run Tests Locally with deployed spell address
+  * [ ] Ensure local tests PASS against deployed spell run via the deploy script
+  * [ ] Push auto-generated commit
+* [ ] Archive Spell via `make archive-spell` for current date or `date="YYYY-MM-DD" make archive-spell` (date as per target exec date)
 * [ ] Commit & Push for Review
 * [ ] Wait for CI to PASS
 * [ ] Wait for at Least two Approvals to Share for Publishing to GovAlpha
 * [ ] Share Deployed Address in `new-spells`
-* [ ] Archive Spell via `make archive-spell` for current date (or `date="YYYY-MM-DD" make archive-spell`)
-* [ ] Commit & Push Archive for Review
-* [ ] Wait for Merge Approvals and CI to PASS
-* [ ] Squash & Merge
 * [ ] Fill Spell Crafter Related Boxes in the Main Exec Doc Sheet
+* [ ] Squash & Merge
 
 ## Next Steps
 * [ ] `MegaPoker` Contract Updates (handed over to Oracle Core Unit (OCU))