Skip to content

Commit 3bf9379

Browse files
add StarGuard support to both checklist, rename SubDAOs to Agents
1 parent 2ee04f7 commit 3bf9379

File tree

2 files changed

+36
-18
lines changed

2 files changed

+36
-18
lines changed

spell/spell-crafter-mainnet-workflow.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,13 @@ Repo: https://github.com/makerdao/spells-mainnet
125125
* [ ] Changes are tested via `testNewOrUpdatedChainlogValues`
126126
* [ ] Adjust system values, collateral values inside `config.sol`
127127
* [ ] Ensure every spell variable is declared as public/internal
128+
* IF Agent spell is provided
129+
* [ ] Handover message matches `XXX spell YYYY-MM-DD deployed to 0x… with hash 0x…, direct execution: yes / no` template
130+
* [ ] IF `direct execution` is `no`
131+
* [ ] The spell is plotted using `StarGuardLike(XXX_STARGUARD).plot(XXX_SPELL, XXX_SPELL_HASH)`
132+
* [ ] IF `direct execution` is `yes`
133+
* [ ] The hash is checked via `require(XXX_SPELL.codehash == XXX_SPELL_HASH, "XXX_SPELL/wrong-codehash");`
134+
* [ ] The spell is executed via `ProxyLike(XXX_PROXY).exec(XXX_SPELL, abi.encodeWithSignature("execute()"));`
128135
* Add specific tests in `DssSpell.t.sol` to have sufficient test coverage for every spell action
129136
* [ ] Test new collaterals
130137
* [ ] Test new ilk registry values

spell/spell-reviewer-mainnet-checklist.md

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -241,27 +241,38 @@
241241
* [ ] `MCD_VEST_MKR_TREASURY` chainlog address is used for MKR stream `yank`
242242
* [ ] `MCD_VEST_DAI` chainlog address is used for DAI stream `yank`
243243
* [ ] Tested via `testYankDAI` or `testYankMKR`
244-
* IF SubDAO-related content is present
245-
* IF SubDAO provides SubProxy spell address
246-
* [ ] SubDAO spell address matches Exec Sheet
247-
* [ ] Executed via `ProxyLike(SUBDAO_PROXY).exec(SUBDAO_SPELL, abi.encodeWithSignature("execute()"));`
244+
* IF Agent-related content is present
245+
* IF Agent spell is provided
246+
* [ ] Handover message matches `XXX spell YYYY-MM-DD deployed to 0x… with hash 0x…, direct execution: yes / no` template
247+
* [ ] IF `direct execution` is `no`
248+
* [ ] The spell is plotted using `StarGuardLike(XXX_STARGUARD).plot(XXX_SPELL, XXX_SPELL_HASH)`
249+
* [ ] `XXX` in `XXX_STARGUARD` matches the name of the Agent
250+
* [ ] `XXX_STARGUARD` is fetched from chainlog
251+
* [ ] The test ensures the spell is executable via `StarGuardLike(XXX_STARGUARD).exec()` before `XXX_STARGUARD.maxDelay`
252+
* [ ] IF `direct execution` is `yes`
253+
* [ ] Provided mandatory explanation of why direct execution is required makes sense on the technical level
254+
* [ ] The hash is checked via `require(XXX_SPELL.codehash == XXX_SPELL_HASH, "XXX_SPELL/wrong-codehash");`
255+
* [ ] The spell is executed via `ProxyLike(XXX_PROXY).exec(XXX_SPELL, abi.encodeWithSignature("execute()"));`
256+
* [ ] `XXX` in `XXX_PROXY` matches the name of the Agent
257+
* [ ] `XXX_PROXY` is fetched from chainlog
258+
* [ ] Agent spell address (`XXX_SPELL`) matches Exec Sheet
259+
* [ ] Agent spell hash (`XXX_SPELL_HASH`) matches Exec Sheet
248260
* [ ] Execution is NOT delegate call
249-
* [ ] IF SubDAO spell deployer is a smart contract (e.g. multisig or factory), ensure the deployer address is in `addresses_deployers.sol` as an entry
250-
* [ ] Ensure that SubDAO spell have enough gas and does not revert with "out of gas" error inside simulation. Note: low level call gas estimation is not done by our scripts
251-
* IF SubDAO provides instructions to be executed by the main spell (i.e. that will operate within Pause Proxy `DelegateCall` context)
252-
* [ ] No SubDAO contract being interacted with is authed on a core contract like `vat`, etc. (Check comprehensively where the risk is high)
253-
* [ ] SubDAO contract licensing and optimizations generally do not matter (except where they pose a security risk)
254-
* [ ] SubDAO contracts and all libraries / dependencies have verified source code (Blocking)
255-
* Upgradable SubDAO contracts
261+
* [ ] IF Agent spell deployer is a smart contract (e.g. multisig or factory), ensure the deployer address is in `addresses_deployers.sol` as an entry
262+
* IF Agent provides instructions to be executed by the main spell (i.e. that will operate within Pause Proxy `DelegateCall` context)
263+
* [ ] No Agent contract being interacted with is authed on a core contract like `vat`, etc. (Check comprehensively where the risk is high)
264+
* [ ] Agent contract licensing and optimizations generally do not matter (except where they pose a security risk)
265+
* [ ] Agent contracts and all libraries / dependencies have verified source code (Blocking)
266+
* Upgradable Agent contracts
256267
* [ ] Upgradable contracts have the `PAUSE_PROXY` as their `admin` (i.e. the party that can upgrade)
257-
* [ ] Any upgradable SubDAO contracts with an `admin` that is not `PAUSE_PROXY` are not authed on any core contracts (Blocking)
258-
* [ ] All SubDAO content addresses (i.e. provided contract addresses or EOAs) present in the Maker Core spell are present in the Exec Sheet and are correct. SubDAO addresses being authed or given any permissions MUST be in the Exec Sheet. SubDAO addresses being called must be confirmed by the SubDAO spell team.
259-
* [ ] IF addresses not PR'ed in by the SubDAO team (use git blame for example), SubDAO content addresses all have inline comment for provenance or source being OKed by SubDAO
260-
* [ ] SubDAO actions match Exec Sheet (only where inline with main spell code) and do not affect core contracts
268+
* [ ] Any upgradable Agent contracts with an `admin` that is not `PAUSE_PROXY` are not authed on any core contracts (Blocking)
269+
* [ ] All Agent content addresses (i.e. provided contract addresses or EOAs) present in the Maker Core spell are present in the Exec Sheet and are correct. Agent addresses being authed or given any permissions MUST be in the Exec Sheet. Agent addresses being called must be confirmed by the Agent spell team.
270+
* [ ] IF addresses not PR'ed in by the Agent team (use git blame for example), Agent content addresses all have inline comment for provenance or source being OKed by the Agent
271+
* [ ] Agent actions match Exec Sheet (only where inline with main spell code) and do not affect core contracts
261272
* [ ] Core contract knock-on actions (such as offboarding or setting DC to 0) are present in the exec and match the code
262-
* [ ] External calls for SubDAO content are NOT delegate call
263-
* [ ] Code does not have untoward behavior within the scope of Maker Core Contracts (e.g. up to the SubDAO proxy)
264-
* IF external contracts calls are present (Not SubDAOs, e.g. Starknet)
273+
* [ ] External calls for Agent content are NOT delegate call
274+
* [ ] Code does not have untoward behavior within the scope of Maker Core Contracts (e.g. up to the Agent proxy)
275+
* IF external contracts calls are present (Not Agents, e.g. Starknet)
265276
* [ ] Target Contract doesn't block spell execution
266277
* [ ] External call is NOT `delegatecall`
267278
* [ ] Target Contract doesn't have permissions on the Vat

0 commit comments

Comments
 (0)