Skip to content

Commit 628bcc3

Browse files
Address review
1 parent 9902cd7 commit 628bcc3

File tree

2 files changed

+15
-32
lines changed

2 files changed

+15
-32
lines changed

spell/spell-crafter-mainnet-workflow.md

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
3535
## Development Stage
3636

3737
* Install stable Foundry version
38-
* [ ] Find the first [Foundry release](https://github.com/foundry-rs/foundry/releases) that is older than 7 days from now
39-
* [ ] Insert the release URL here:
40-
* [ ] Install the specified version via `foundryup --install stable`
38+
* [ ] Install the stable version of Foundry via `foundryup --install stable`
4139
```
4240
Document the installation logs containing installed versions below:
4341
```
@@ -114,9 +112,9 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
114112
* [ ] Check if oracle deployment is required (e.g. univ3-lp-oracle, new ilk pip, ...) with responsible ecosystem actor
115113
* IF addresses are used in the spell
116114
* [ ] Use `immutable` visibility when declaring addresses using `DssExecLib.getChangelogAddress`, OTHERWISE use `constant` for statically defined addresses
117-
* [ ] Fetch addresses as type `address` and wrap with `Like` suffix interfaces inline (when making calls), EXCEPT `MKR` and vesting contracts
115+
* [ ] Fetch addresses as type `address` and wrap with `Like` suffix interfaces inline (when making calls), EXCEPT where the archive pattern differs from this pattern (e.g. SKY)
118116
* [ ] Use the [DssExecLib address helpers](https://github.com/sky-ecosystem/dss-exec-lib/blob/master/src/DssExecLib.sol#L192) where possible (e.g. `DssExecLib.vat()`)
119-
* [ ] Where addresses are fetched from the ChainLog, the variable name must match the value of the ChainLog key for that address (e.g. `MCD_VAT` rather than `vat`), EXCEPT where the archive pattern differs from this pattern (e.g. `MKR`)
117+
* [ ] Where addresses are fetched from the ChainLog, the variable name must match the value of the ChainLog key for that address (e.g. `MCD_VAT` rather than `vat`), EXCEPT where the archive pattern differs from this pattern
120118
* IF new addresses need to be added to the ChainLog
121119
* [ ] Add new addresses to the ChainLog
122120
* [ ] Increment ChainLog version, according to the update type
@@ -132,8 +130,8 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
132130
* [ ] Test new collaterals
133131
* [ ] Test new ilk registry values
134132
* [ ] Test new ChainLog values
135-
* [ ] Test DAI/MKR/USDS/SKY/SPK streams and payments, lerps
136-
* [ ] Test the sum of all DAI/MKR/USDS/SKY/SPK payments matches the Exec Sheet
133+
* [ ] Test DAI/USDS/SKY/SPK streams and payments, lerps
134+
* [ ] Test the sum of all DAI/USDS/SKY/SPK payments matches the Exec Sheet
137135
* Run tests via `make test` (or `make test match=<test_name>` to inspect debug traces)
138136
* [ ] Ensure good coverage (every spell action is tested)
139137
* [ ] Ensure every test function is declared as `public`

spell/spell-reviewer-mainnet-checklist.md

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
55
## Development Stage
66

77
* Install stable Foundry version
8-
* [ ] Find the first [Foundry release](https://github.com/foundry-rs/foundry/releases) that is older than 7 days from now
9-
* [ ] Insert the release URL here:
10-
* [ ] Install the specified version via `foundryup --install stable`
8+
* [ ] Install the stable version of Foundry via `foundryup --install stable`
119
```
1210
Document the installation logs containing installed versions below:
1311
```
@@ -96,7 +94,7 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
9694
* [ ] Source code is verified on etherscan
9795
* [ ] Compilation optimizations match deployment settings defined in the source code repo
9896
* [ ] `GNU AGPLv3` license
99-
* [ ] Every maker-related constructor argument matches chainlog (e.g. `vat`, `dai`, `dog`, ...)
97+
* [ ] Every protocol-related constructor argument matches chainlog (e.g. `vat`, `dai`, `dog`, ...)
10098
* IF new contract have concept of `wards` or access control
10199
* [ ] Ensure `PAUSE_PROXY` address was `relied` (`wards(PAUSE_PROXY)` is `1`)
102100
* [ ] Ensure that contract deployer address was `denied` (`wards(deployer)` is `0`)
@@ -196,15 +194,6 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
196194
* [ ] Insert and follow the relevant checklists below:
197195
* [RWA Offboarding](./rwa-checklists.md#rwa-offboarding-checklist)
198196
* IF payments are present in the spell
199-
* IF `MKR` transfers are present
200-
* [ ] Recipient address in the instruction is in the checksummed format
201-
* [ ] Recipient address matches Exec Sheet
202-
* [ ] Recipient address variable name matches one found in `addresses_wallets.sol`
203-
* [ ] Transfer amount matches Exec Sheet
204-
* [ ] Transfer amount is specified with (at least) 2 decimals using `ether` keyword
205-
* [ ] IF `ether` keyword is used, comment is present on the same line `// Note: ether is a keyword that represents 10**18, not the ETH token`
206-
* [ ] The transfers are tested via `testPayments` test
207-
* [ ] Sum of all MKR transfers tested in `testPayments` matches number in the Exec Sheet
208197
* IF `SKY` transfers are present
209198
* [ ] Recipient address in the instruction is in the checksummed format
210199
* [ ] Recipient address matches Exec Sheet
@@ -226,7 +215,7 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
226215
* [ ] Transfer amount matches Exec Sheet
227216
* [ ] The transfers are tested via `testPayments` test
228217
* [ ] Sum of all USDS transfers tested in `testPayments` matches number in the Exec Sheet
229-
* IF `MKR` / `DAI` / `SKY` / `USDS` / `SPK` streams (`DssVest`) are created
218+
* IF `DAI` / `SKY` / `USDS` / `SPK` streams (`DssVest`) are created
230219
* [ ] `VestAbstract` interface is imported from `dss-interfaces/dss/VestAbstract.sol`
231220
* [ ] `restrict` is used for each stream, UNLESS otherwise explicitly stated in the Exec Sheet
232221
* [ ] `usr` (Vest recipient address) matches Exec Sheet
@@ -236,8 +225,10 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
236225
* [ ] IF `ether` keyword is used, comment is present on the same line `// Note: ether is a keyword that represents 10**18, not the ETH token`
237226
* [ ] IF vest amount is expressed in 'per year' or similar in the Exec Sheet, account for leap days
238227
* [ ] `bgn` (Vest start timestamp) matches Exec Sheet
239-
* [ ] `tau` is expressed as `fin - bgn` (i.e. `MONTH_DD_YYYY - MONTH_DD_YYYY`)
240-
* [ ] `fin` (Vest end timestamp) matches Exec Sheet
228+
* [ ] `tau` is expressed as EITHER:
229+
* `fin - bgn` (i.e. `MONTH_DD_YYYY - MONTH_DD_YYYY`)
230+
* [ ] `fin` (Vest end timestamp) matches Exec Sheet
231+
* time interval (e.g. `365 days`)
241232
* [ ] `eta` (Vest cliff duration) matches the following logic
242233
* IF `eta` is explicitly specified in the Exec Sheet, then the values match
243234
* IF `eta` and `clf` (Cliff end timestamp) are not specified in the Exec Sheet, then `eta` is `0`
@@ -252,9 +243,6 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
252243
* [ ] Governance facilitators were notified
253244
* [ ] Exec Sheet contains explicit instruction
254245
* [ ] Exec Doc contains explicit instruction
255-
* IF MKR stream ([DssVestTransferrable](https://github.com/sky-ecosystem/dss-vest/blob/master/src/DssVest.sol#L463)) is present
256-
* [ ] Vest contract's MKR allowance increased by the cumulative `total` (the sum of all `tot` values)
257-
* [ ] Ensure allowance increase follows archive patterns
258246
* IF SKY stream ([DssVestTransferrable](https://github.com/sky-ecosystem/dss-vest/blob/master/src/DssVest.sol#L463)) is present
259247
* [ ] Vest contract's SKY allowance increased by the cumulative `total` (the sum of all `tot` values)
260248
* [ ] Ensure allowance increase follows archive patterns
@@ -263,21 +251,18 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
263251
* [ ] Ensure allowance increase follows archive patterns
264252
* [ ] Tested via:
265253
* `testVestDai`
266-
* `testVestMkr`
267254
* `testVestSky`
268255
* `testVestSkyMint`
269256
* `testVestUsds`
270257
* `testVestSpk`
271-
* IF `MKR` / `DAI` / `SKY` / `USDS` / `SPK` vest termination (`Yank`) is present
258+
* IF `DAI` / `SKY` / `USDS` / `SPK` vest termination (`Yank`) is present
272259
* [ ] Yanked stream ID matches Exec Sheet
273-
* [ ] `MCD_VEST_MKR_TREASURY` chainlog address is used for MKR stream `yank`
274260
* [ ] `MCD_VEST_SKY_TREASURY` chainlog address is used for SKY stream `yank`
275261
* [ ] `MCD_VEST_SPK_TREASURY` chainlog address is used for SPK stream `yank`
276262
* [ ] `MCD_VEST_DAI` chainlog address is used for DAI stream `yank`
277263
* [ ] `MCD_VEST_USDS` chainlog address is used for USDS stream `yank`
278264
* [ ] Tested via:
279265
* `testVestDai`
280-
* `testVestMkr`
281266
* `testVestSky`
282267
* `testVestSkyMint`
283268
* `testVestUsds`
@@ -322,9 +307,9 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
322307
* [ ] Changes are tested via `testChainlogIntegrity`, `testChainlogValues`, `testAddedChainlogKeys` and `testRemovedChainlogKeys`
323308
* [ ] Ensure every spell variable is declared as `public`/`internal`
324309
* [ ] Ensure `immutable` visibility is only used when fetching addresses from the `ChainLog` via `DssExecLib.getChangelogAddress(key)` and `constant` is used instead for static addresses
325-
* [ ] Fetch addresses as type `address` and wrap with `Like` suffix interfaces inline (when making calls), UNLESS archive patterns permit otherwise (Such as `MKR`)
310+
* [ ] Fetch addresses as type `address` and wrap with `Like` suffix interfaces inline (when making calls), UNLESS archive patterns permit otherwise (such as `SKY`)
326311
* [ ] Use the [DssExecLib Core Address Helpers](https://github.com/sky-ecosystem/dss-exec-lib/blob/master/src/DssExecLib.sol#L166) where possible (e.g. `DssExecLib.vat()`)
327-
* [ ] Where addresses are fetched from the `ChainLog`, the variable name must match the value of the ChainLog key for that address (e.g. `MCD_VAT` rather than `vat`), except where the archive pattern differs from this pattern (e.g. MKR)
312+
* [ ] Where addresses are fetched from the ChainLog, the variable name must match the value of the ChainLog key for that address (e.g. `MCD_VAT` rather than `vat`), EXCEPT where the archive pattern differs from this pattern
328313
* Tests
329314
* [ ] Ensure that the `DssExecLib.address` file is not being modified by the spell PR
330315
* [ ] Check all CI tests are passing as at the latest commit

0 commit comments

Comments
 (0)