Skip to content

Commit 75e4a86

Browse files
address feedback
1 parent 4931fbc commit 75e4a86

File tree

3 files changed

+29
-29
lines changed

3 files changed

+29
-29
lines changed

spell/spell-crafter-mainnet-workflow.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,10 @@ Repo: https://github.com/makerdao/spells-mainnet
128128
* IF Prime Agent spell is provided
129129
* [ ] Handover message matches `XXX spell YYYY-MM-DD deployed to 0x… with hash 0x…, direct execution: yes / no` template
130130
* [ ] IF `direct execution` is `no`
131-
* [ ] The spell is plotted using `StarGuardLike(XXX_STARGUARD).plot(XXX_SPELL, XXX_SPELL_HASH)`
131+
* [ ] The Prime Agent spell is plotted using `StarGuardLike(XXX_STARGUARD).plot(XXX_SPELL, XXX_SPELL_HASH)`
132132
* [ ] IF `direct execution` is `yes`
133133
* [ ] The hash is checked via `require(XXX_SPELL.codehash == XXX_SPELL_HASH, "XXX_SPELL/wrong-codehash");` inside Core spell
134-
* [ ] The spell is executed via `ProxyLike(XXX_PROXY).exec(XXX_SPELL, abi.encodeWithSignature("execute()"));`
134+
* [ ] The Prime Agent spell is executed via `ProxyLike(XXX_PROXY).exec(XXX_SPELL, abi.encodeWithSignature("execute()"));`
135135
* Add specific tests in `DssSpell.t.sol` to have sufficient test coverage for every spell action
136136
* [ ] Test new collaterals
137137
* [ ] Test new ilk registry values

spell/spell-reviewer-mainnet-checklist.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -242,24 +242,24 @@
242242
* [ ] `MCD_VEST_DAI` chainlog address is used for DAI stream `yank`
243243
* [ ] Tested via `testYankDAI` or `testYankMKR`
244244
* IF content related to a Prime Agent is present
245-
* IF Prime spell is provided
245+
* IF Prime Agent spell is provided
246246
* [ ] Handover message matches `XXX spell YYYY-MM-DD deployed to 0x… with hash 0x…, direct execution: yes / no` template
247247
* [ ] IF `direct execution` is `no`
248-
* [ ] The Prime spell is plotted using `StarGuardLike(XXX_STARGUARD).plot(XXX_SPELL, XXX_SPELL_HASH)`
248+
* [ ] The Prime Agent spell is plotted using `StarGuardLike(XXX_STARGUARD).plot(XXX_SPELL, XXX_SPELL_HASH)`
249249
* [ ] `XXX` in `XXX_STARGUARD` matches the name of the Prime Agent
250250
* [ ] `XXX_STARGUARD` is fetched from chainlog
251-
* [ ] The test ensures the `XXX_SPELL` Prime spell is executable via `StarGuardLike(XXX_STARGUARD).exec()` before `XXX_STARGUARD.maxDelay`
252-
* [ ] IF plotted but not yet executed spell is still present in the `XXX_STARGUARD`, Governance Facilitators are aware, already notified
251+
* [ ] The test ensures the `XXX_SPELL` Prime Agent spell is executable via `StarGuardLike(XXX_STARGUARD).exec()` before `XXX_STARGUARD.maxDelay`
252+
* [ ] IF plotted but not yet executed spell is still present in the `XXX_STARGUARD`, Governance Facilitators are aware or already notified
253253
* [ ] IF `direct execution` is `yes`
254254
* [ ] Provided mandatory explanation of why direct execution is required makes sense on the technical level
255255
* [ ] The hash is checked via `require(XXX_SPELL.codehash == XXX_SPELL_HASH, "XXX_SPELL/wrong-codehash");` inside the Core spell
256-
* [ ] The Prime spell is executed via `ProxyLike(XXX_PROXY).exec(XXX_SPELL, abi.encodeWithSignature("execute()"));`
256+
* [ ] The Prime Agent spell is executed via `ProxyLike(XXX_PROXY).exec(XXX_SPELL, abi.encodeWithSignature("execute()"));`
257257
* [ ] `XXX` in `XXX_PROXY` matches the name of the Prime Agent
258258
* [ ] `XXX_PROXY` is fetched from chainlog
259-
* [ ] Prime spell address (`XXX_SPELL`) matches Exec Sheet
260-
* [ ] Prime spell hash (`XXX_SPELL_HASH`) matches Exec Sheet
259+
* [ ] Prime Agent spell address (`XXX_SPELL`) matches Exec Sheet
260+
* [ ] Prime Agent spell hash (`XXX_SPELL_HASH`) matches Exec Sheet
261261
* [ ] Execution is NOT delegate call
262-
* [ ] IF Prime 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 Prime Agent spell deployer is a smart contract (e.g. multisig or factory), ensure the deployer address is in `addresses_deployers.sol` as an entry
263263
* IF Prime Agent provides instructions to be executed by the main spell (i.e. that will operate within Pause Proxy `DelegateCall` context)
264264
* [ ] No Prime Agent contract being interacted with is authed on a core contract like `vat`, etc. (Check comprehensively where the risk is high)
265265
* [ ] Prime Agent contract licensing and optimizations generally do not matter (except where they pose a security risk)

spell/star-spell-reviewer-checklist.md

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
# Star Spells Reviewer Checklist
1+
# Prime Agent Spells Reviewer Checklist
22

3-
This checklist provides guidance for crafting and reviewing spells for Star protocols (Spark, Bloom, etc.). It focuses on common practices and should be used alongside protocol-specific knowledge.
3+
This checklist provides guidance for crafting and reviewing spells for the Prime Agents (Spark, Grove, Keel, etc.). It focuses on common practices and should be used alongside protocol-specific knowledge.
44

55
## Governance Architecture Considerations
66

77
### Spells Contracts Relationships
88

9-
The diagram below illustrates that, despite the name, `StarSpell` is functionally more similar to `DssSpellAction` than to `DssSpell`. Both `StarSpell` and `DssSpellAction` contain executable logic, whereas `DssSpell` primarily handles scheduling and execution triggering through `MCD_PAUSE`. This architectural similarity is important to understand when reviewing and developing spells for Star protocols.
9+
The diagram below illustrates that, despite the name, `PrimeAgentSpell` is functionally more similar to `DssSpellAction` than to `DssSpell`. Both `PrimeAgentSpell` and `DssSpellAction` contain executable logic, whereas `DssSpell` primarily handles scheduling and execution triggering through `MCD_PAUSE`. This architectural similarity is important to understand when reviewing and developing spells for the Prime Agent protocols.
1010

1111
<details>
1212
<summary><b>View Diagram</b></summary>
@@ -26,7 +26,7 @@ classDiagram
2626
DssSpell ..> DssSpellAction : instantiates
2727
2828
%% Functional similarity
29-
DssSpellAction <.. StarSpell : functionally similar
29+
DssSpellAction <.. PrimeAgentSpell : functionally similar
3030
3131
%% Reference relationship
3232
DssExec --> DssAction : references
@@ -36,7 +36,7 @@ classDiagram
3636

3737
### Execution Flow
3838

39-
The diagram bellow illustrates the execution flow of a spell from the Core up to the Star protocol.
39+
The diagram bellow illustrates the execution flow of a spell from the Core up to the Prime Agent protocol.
4040

4141
<details>
4242
<summary><b>View Diagram</b></summary>
@@ -62,9 +62,9 @@ graph TB
6262
MCD_PAUSE_PROXY[MCD_PAUSE_PROXY]
6363
end
6464
65-
subgraph StarCode ["Star Protocol"]
65+
subgraph StarCode ["Prime Agent Protocol"]
6666
STAR_PROXY[STAR_PROXY]
67-
StarSpell[StarSpell]
67+
PrimeAgentSpell[PrimeAgentSpell]
6868
end
6969
7070
%% Execution Flow with multi-level numbered steps
@@ -75,7 +75,7 @@ graph TB
7575
MCD_PAUSE -->|2.2\. exec| MCD_PAUSE_PROXY
7676
MCD_PAUSE_PROXY -->|2.3\. delegatecall| DssSpellAction
7777
DssSpellAction -->|2.4\. call| STAR_PROXY
78-
STAR_PROXY -->|2.5\. delegatecall| StarSpell
78+
STAR_PROXY -->|2.5\. delegatecall| PrimeAgentSpell
7979
end
8080
8181
%% Legend as a single node with HTML table - one item per row - no borders
@@ -87,7 +87,7 @@ graph TB
8787
</tr>
8888
<tr style='background:transparent;'>
8989
<td width='20' height='20' bgcolor='#f0e6d1' style='border:none;'></td>
90-
<td align='left' style='background:transparent; border:none;'>Star Contracts</td>
90+
<td align='left' style='background:transparent; border:none;'>Prime Agent Contracts</td>
9191
</tr>
9292
<tr style='background:transparent;'>
9393
<td width='20' height='20' bgcolor='#f9e8e8' style='border:none;'></td>
@@ -102,14 +102,14 @@ graph TB
102102
class DssSpell,DssSpellAction core
103103
class MCD_PAUSE,MCD_PAUSE_PROXY core
104104
class Governance governance
105-
class STAR_PROXY,StarSpell star
105+
class STAR_PROXY,PrimeAgentSpell star
106106
class MainDiagram transparent
107107
class CoreCode,StarCode boundary
108108
```
109109

110110
</details>
111111

112-
## Star Spells Review Process
112+
## Prime Agent Spells Review Process
113113

114114
This section outlines the review process and provides concrete action items for both the crafter and reviewers of the spell. The document is divided into separate stages ("Development", "Deployment", "Handover"). Both reviewers must complete all checks in the relevant stage and publish them as the PR comment at the end of each stage.
115115

@@ -120,7 +120,7 @@ This section outlines the review process and provides concrete action items for
120120
- `COMMIT_TITLE`, URL_TO_THE_PR_OR_THE_COMMIT
121121
- [ ] Content matches description: no unrelated changes.
122122
- [ ] No security-related changes are present in this commit.
123-
- [ ] Verify solc version matches the Star protocol standard based on prior Star contracts.
123+
- [ ] Verify solc version matches the Prime Agent protocol standard based on prior contracts.
124124

125125
#### Spell Description & Comments
126126
- [ ] Spell PR has clear description.
@@ -130,12 +130,12 @@ This section outlines the review process and provides concrete action items for
130130
- [ ] Every parameter change is clearly commented with before/after values.
131131

132132
#### Proposed changes
133-
- LIST every forum post proposing changes for this particular Star, particular target date:
133+
- LIST every forum post proposing changes for this particular Prime Agent, particular target date:
134134
- FORUM_POST_TITLE, FORUM_POST_URL
135135
- [ ] Forum post follows the [known template](https://docs.google.com/document/d/1vLqeP-zXmxKo2OpoxnL2z0ZczPe4nWN49-3URx-iKVA/edit?tab=t.nkz4n7by2dnh).
136136
- [ ] Verify spell content matches the combined scope of the forum posts listed above.
137137
- [ ] Verify forum posts contain all new addresses directly or indirectly used in the spell, their constructor arguments and rate limits.
138-
- [ ] IF the Star Spell introduces a major change that can affect external parties, suggest Governance Facilitators to set Core Spell office hours to `true`.
138+
- [ ] IF the Prime Agent spell introduces a major change that can affect external parties, suggest Governance Facilitators to set Core Spell office hours to `true`.
139139

140140
#### Contract Structure & Code Quality
141141
- [ ] The only external non-view function in the spell contract is `execute()`.
@@ -149,12 +149,12 @@ This section outlines the review process and provides concrete action items for
149149
- [ ] Matches valid external source (previously approved forum post, external docs, etc).
150150

151151
### StarGuard execution
152-
- [ ] IF a [StarGuard module](https://github.com/sky-ecosystem/star-guard) is onboarded for this Star, the following additional checks are done:
152+
- [ ] IF a [StarGuard module](https://github.com/sky-ecosystem/star-guard) is onboarded for this Prime Agent, the following additional checks are done:
153153
- [ ] The spell exposes view-only interface `function isExecutable() external view returns (bool result)`.
154154
- [ ] `isExecutable` either simply returns `true` or implements additional logic communicated via the relevant forum post (e.g.: by describing "earliest launch date" or "office hours" logic, etc).
155155
- [ ] The test ensures the spell is executable before expiration (i.e. `isExecutable` outputs `true` before `StarGuard.maxDelay()` is passed).
156156
- [ ] Third-party actors can not take advantage of the fact that Spell will be executed in a later block than the Core spell, otherwise suggest `direct execution`.
157-
- [ ] IF Prime spell can not be executed in a later block OR have to be executed sequentially to another Prime spell, `direct execution` is clearly mentioned in the forum post together with elaborated explanation why it is needed.
157+
- [ ] IF Prime Agent spell can not be executed in a later block OR have to be executed sequentially to another Prime Agent spell, `direct execution` is clearly mentioned in the forum post together with elaborated explanation why it is needed.
158158
- [ ] The `direct execution` explanation makes sense on the technical level and can not be circumvented by the use of `isExecutable()` interface.
159159

160160
#### On-boarding New Contracts
@@ -211,7 +211,7 @@ This section outlines the review process and provides concrete action items for
211211
- [ ] No privileged functions accessible by unauthorized users.
212212

213213
#### Parameter Changes & Protocol Integration
214-
- [ ] Star Protocol invariants are maintained after spell execution.
214+
- [ ] Prime Agent protocol invariants are maintained after spell execution.
215215
- [ ] All parameter changes use the appropriate helper functions IF available.
216216
- [ ] Parameter changes match the Executive Sheet exactly.
217217
- [ ] Spell interacts correctly with existing protocol components.
@@ -276,8 +276,8 @@ EXECUTED_TESTS_LOGS
276276
- [ ] Both reviewers gave explicit "Good to handover".
277277
- [ ] All review comments have been addressed or resolved.
278278
- [ ] The spell address, the codehash and the direct execution are posted by the crafter in the `#govops` in the `XXX spell YYYY-MM-DD deployed to 0x… with hash 0x…, direct execution: yes / no` format.
279-
- [ ] Posted spell address matches the spell evaluated above.
280-
- [ ] Posted spell codehash matches the one noted down above.
279+
- [ ] Posted spell address matches spell address approved for handover.
280+
- [ ] Posted spell codehash matches codehash that you verified locally.
281281
- [ ] Posted direct execution value matches the forum post.
282282
- [ ] Confirm the address (via a separate "reply to" message, restating the address to avoid edits).
283283
- [ ] Ensure that no changes were made to the code since the spell was deployed and archived.

0 commit comments

Comments
 (0)