- 
                Notifications
    You must be signed in to change notification settings 
- Fork 9
Add StarGuard support, rename SubDAOs to Agents #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
3bf9379
              7d3e2cc
              cc3d405
              9c510f7
              7ea352c
              61630f4
              4931fbc
              75e4a86
              b79b8b6
              f106c3f
              5b0c821
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,12 +1,12 @@ | ||||||
| # Star Spells Reviewer Checklist | ||||||
| # Prime Agent Spells Reviewer Checklist | ||||||
|  | ||||||
| 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. | ||||||
| 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. | ||||||
|  | ||||||
| ## Governance Architecture Considerations | ||||||
|  | ||||||
| ### Spells Contracts Relationships | ||||||
|  | ||||||
| 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. | ||||||
| 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. | ||||||
|  | ||||||
| <details> | ||||||
| <summary><b>View Diagram</b></summary> | ||||||
|  | @@ -26,7 +26,7 @@ classDiagram | |||||
| DssSpell ..> DssSpellAction : instantiates | ||||||
|  | ||||||
| %% Functional similarity | ||||||
| DssSpellAction <.. StarSpell : functionally similar | ||||||
| DssSpellAction <.. PrimeAgentSpell : functionally similar | ||||||
|  | ||||||
| %% Reference relationship | ||||||
| DssExec --> DssAction : references | ||||||
|  | @@ -36,7 +36,7 @@ classDiagram | |||||
|  | ||||||
| ### Execution Flow | ||||||
|  | ||||||
| The diagram bellow illustrates the execution flow of a spell from the Core up to the Star protocol. | ||||||
| The diagram bellow illustrates the execution flow of a spell from the Core up to the Prime Agent protocol. | ||||||
|  | ||||||
| <details> | ||||||
| <summary><b>View Diagram</b></summary> | ||||||
|  | @@ -62,9 +62,9 @@ graph TB | |||||
| MCD_PAUSE_PROXY[MCD_PAUSE_PROXY] | ||||||
| end | ||||||
|  | ||||||
| subgraph StarCode ["Star Protocol"] | ||||||
| subgraph StarCode ["Prime Agent Protocol"] | ||||||
| STAR_PROXY[STAR_PROXY] | ||||||
| StarSpell[StarSpell] | ||||||
| PrimeAgentSpell[PrimeAgentSpell] | ||||||
| end | ||||||
|  | ||||||
| %% Execution Flow with multi-level numbered steps | ||||||
|  | @@ -75,7 +75,7 @@ graph TB | |||||
| MCD_PAUSE -->|2.2\. exec| MCD_PAUSE_PROXY | ||||||
| MCD_PAUSE_PROXY -->|2.3\. delegatecall| DssSpellAction | ||||||
| DssSpellAction -->|2.4\. call| STAR_PROXY | ||||||
| STAR_PROXY -->|2.5\. delegatecall| StarSpell | ||||||
| STAR_PROXY -->|2.5\. delegatecall| PrimeAgentSpell | ||||||
| end | ||||||
|  | ||||||
| %% Legend as a single node with HTML table - one item per row - no borders | ||||||
|  | @@ -87,7 +87,7 @@ graph TB | |||||
| </tr> | ||||||
| <tr style='background:transparent;'> | ||||||
| <td width='20' height='20' bgcolor='#f0e6d1' style='border:none;'></td> | ||||||
| <td align='left' style='background:transparent; border:none;'>Star Contracts</td> | ||||||
| <td align='left' style='background:transparent; border:none;'>Prime Agent Contracts</td> | ||||||
| </tr> | ||||||
| <tr style='background:transparent;'> | ||||||
| <td width='20' height='20' bgcolor='#f9e8e8' style='border:none;'></td> | ||||||
|  | @@ -102,14 +102,14 @@ graph TB | |||||
| class DssSpell,DssSpellAction core | ||||||
| class MCD_PAUSE,MCD_PAUSE_PROXY core | ||||||
| class Governance governance | ||||||
| class STAR_PROXY,StarSpell star | ||||||
| class STAR_PROXY,PrimeAgentSpell star | ||||||
| class MainDiagram transparent | ||||||
| class CoreCode,StarCode boundary | ||||||
| ``` | ||||||
|  | ||||||
| </details> | ||||||
|  | ||||||
| ## Star Spells Review Process | ||||||
| ## Prime Agent Spells Review Process | ||||||
|  | ||||||
| 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. | ||||||
|  | ||||||
|  | @@ -120,7 +120,7 @@ This section outlines the review process and provides concrete action items for | |||||
| - `COMMIT_TITLE`, URL_TO_THE_PR_OR_THE_COMMIT | ||||||
| - [ ] Content matches description: no unrelated changes. | ||||||
| - [ ] No security-related changes are present in this commit. | ||||||
| - [ ] Verify solc version matches the Star protocol standard based on prior Star contracts. | ||||||
| - [ ] Verify solc version matches the Prime Agent protocol standard based on prior contracts. | ||||||
|  | ||||||
| #### Spell Description & Comments | ||||||
| - [ ] Spell PR has clear description. | ||||||
|  | @@ -130,12 +130,12 @@ This section outlines the review process and provides concrete action items for | |||||
| - [ ] Every parameter change is clearly commented with before/after values. | ||||||
|  | ||||||
| #### Proposed changes | ||||||
| - LIST every forum post proposing changes for this particular Star, particular target date: | ||||||
| - LIST every forum post proposing changes for this particular Prime Agent, particular target date: | ||||||
| - FORUM_POST_TITLE, FORUM_POST_URL | ||||||
| - [ ] Forum post follows the [known template](https://docs.google.com/document/d/1vLqeP-zXmxKo2OpoxnL2z0ZczPe4nWN49-3URx-iKVA/edit?tab=t.nkz4n7by2dnh). | ||||||
| - [ ] Verify spell content matches the combined scope of the forum posts listed above. | ||||||
| - [ ] Verify forum posts contain all new addresses directly or indirectly used in the spell, their constructor arguments and rate limits. | ||||||
| - [ ] IF the Star Spell introduces a major change that can affect external parties, suggest Governance Facilitators to set Core Spell office hours to `true`. | ||||||
| - [ ] 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`. | ||||||
|  | ||||||
| #### Contract Structure & Code Quality | ||||||
| - [ ] The only external non-view function in the spell contract is `execute()`. | ||||||
|  | @@ -148,6 +148,15 @@ This section outlines the review process and provides concrete action items for | |||||
| - [CHAIN_NAME] `0xADDRESS`, EXTERNAL_SOURCE_URL | ||||||
| - [ ] Matches valid external source (previously approved forum post, external docs, etc). | ||||||
|  | ||||||
| ### StarGuard execution | ||||||
| - [ ] IF a [StarGuard module](https://github.com/sky-ecosystem/star-guard) is onboarded for this Prime Agent, the following additional checks are done: | ||||||
| - [ ] The spell exposes view-only interface `function isExecutable() external view returns (bool result)`. | ||||||
| - [ ] `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). | ||||||
| - [ ] The test ensures the spell is executable before expiration (i.e. `isExecutable` outputs `true` before `StarGuard.maxDelay()` is passed). | ||||||
| - [ ] 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`. | ||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
        Suggested change
       
 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have read this rule: 
 But I am not a native speaker so I might be wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think both forms are acceptable, but as there are plenty of existing occasions in the checklists, I will keep it as is (if you don't mind) | ||||||
| - [ ] 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. | ||||||
| - [ ] The `direct execution` explanation makes sense on the technical level and can not be circumvented by the use of `isExecutable()` interface. | ||||||
|         
                  SidestreamColdMelon marked this conversation as resolved.
              Show resolved
            Hide resolved | ||||||
|  | ||||||
| #### On-boarding New Contracts | ||||||
| - LIST every new contract present in the spell: | ||||||
| - [CHAIN_NAME] `CONTRACT_NAME`, LINK_TO_THE_DEPLOYED_CONTRACT | ||||||
|  | @@ -202,7 +211,7 @@ This section outlines the review process and provides concrete action items for | |||||
| - [ ] No privileged functions accessible by unauthorized users. | ||||||
|  | ||||||
| #### Parameter Changes & Protocol Integration | ||||||
| - [ ] Star Protocol invariants are maintained after spell execution. | ||||||
| - [ ] Prime Agent protocol invariants are maintained after spell execution. | ||||||
| - [ ] All parameter changes use the appropriate helper functions IF available. | ||||||
| - [ ] Parameter changes match the Executive Sheet or the corresponding Atlas edit exactly. | ||||||
| - [ ] Spell interacts correctly with existing protocol components. | ||||||
|  | @@ -244,6 +253,8 @@ EXECUTED_TESTS_LOGS | |||||
| #### Deployed Contract | ||||||
| - [ ] Both reviewers gave explicit "Good to deploy". | ||||||
| - [ ] A new comment in the PR contains link to the deployed spell(s) and Tenderly vnet(s). | ||||||
| - [ ] The comment also contains codehash of the deployed mainnet spell. | ||||||
| - [ ] The codehash matches one produced locally from the reviewed source code. | ||||||
| - [ ] Every spell is verified on Etherscan or other primary block explorer for this chain. | ||||||
| - [ ] Every spell code matches local source code at the "good to deploy" commit. | ||||||
| - [ ] Etherscan settings (optimizer, EVM version, license) match local ones. | ||||||
|  | @@ -268,7 +279,10 @@ EXECUTED_TESTS_LOGS | |||||
| #### Confirmed Handover | ||||||
| - [ ] Both reviewers gave explicit "Good to handover". | ||||||
| - [ ] All review comments have been addressed or resolved. | ||||||
| - [ ] The spell address posted by the crafter in the `#govops` thread matches the spell evaluated above. | ||||||
| - [ ] 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. | ||||||
| - [ ] Posted spell address matches spell address approved for handover. | ||||||
| - [ ] Posted spell codehash matches codehash that you verified locally. | ||||||
| - [ ] Posted direct execution value matches the forum post. | ||||||
| - [ ] Confirm the address (via a separate "reply to" message, restating the address to avoid edits). | ||||||
| - [ ] Ensure that no changes were made to the code since the spell was deployed and archived. | ||||||
| - [ ] IF no blockers were found, post the completed "Handover Stage" checklist stage with the explicit pull request approval via 'Approve' review option. | ||||||
Uh oh!
There was an error while loading. Please reload this page.