Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions spell/spell-crafter-mainnet-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,17 @@ Repo: https://github.com/sky-ecosystem/spells-mainnet
```
* [ ] Exec hash is correct (use `cast keccak -- "$(curl '$URL' -o - 2>/dev/null)"` where `wget` doesn't work)
* [ ] Ensure `description` date in `DssSpell.sol` matches target date inside Exec Doc
* Octane Review
* [ ] IF no Octane analysis exists on the latest commit from [Sky Ecosystem: Spells mainnet](https://app.octane.security/projects/p/fa7414c7-d44a-4a2d-b767-2ed7462547a5/analysis/23), trigger a new one (Press "Run new analysis" in the top right corner -> Select correct PR -> Select "PR-only" for the "PR analysis mode" field -> Click "Run analysis")
* [ ] Ensure no filters are applied to the Octane results (press "All filters" -> set "All" for each filter group)
* [ ] Ensure all onchain dependencies are correctly fetched (Click on the "On Chain dependencies" tab -> inspect "Contracts" table). In case "Source skipped" contracts are present, select correct chain for each contract -> "Save overrides" and rerun the analysis
* [ ] Thoroughly inspect analysis settings for signs of manipulation (Click on the "Settings" tab -> check "Analysis Scope", "Install dependencies", and other relevant sections)
* [ ] List every reported vulnerability and warning (split into separate items when one finding contains multiple different issues):
1. Title [Octane URL]
* Link to the relevant code block: [GitHub permalink]
* Is this issue blocking spell deployment?
* Required reason (explain why it can be a problem or can't be):
* [ ] IF any blocking issue is found, raise it to the current spell signal group
Comment on lines +209 to +214

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the high false positive ratio still present in Octane, this part could become very lengthy with the explanations of why a finding must be dismissed.
I'd suggest simply posting the URL to the report so the reviewers can check.
Optionally, the crafter could provide only the valid findings after discussing with the spell teams.

* [ ] Make sure all review comments are either addressed or explicitly answered
* [ ] Make sure all items in the Exec Sheet are confirmed, OTHERWISE notify Responsible Governance Facilitator
* [ ] Notify the reviewers (e.g. "Exec Hash is added, reviews are addressed")
Expand Down
16 changes: 14 additions & 2 deletions spell/spell-reviewer-mainnet-checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -380,11 +380,23 @@ _Insert your local test logs here_
* [ ] Exec Doc URL in the spell comment refers to the [https://github.com/sky-ecosystem/executive-votes](https://github.com/sky-ecosystem/executive-votes) repository
* [ ] Every action present in the spell code is present in the Exec Doc
* [ ] Every action in the Exec Doc is present in the spell code
* Octane Review
* [ ] IF no Octane analysis exists on the latest commit from [Sky Ecosystem: Spells mainnet](https://app.octane.security/projects/p/fa7414c7-d44a-4a2d-b767-2ed7462547a5/analysis/23), trigger a new one (Press "Run new analysis" in the top right corner -> Select correct PR -> Select "PR-only" for the "PR analysis mode" field -> Click "Run analysis")
* [ ] Ensure no filters are applied to the Octane results (press "All filters" -> set "All" for each filter group)
* [ ] Ensure all onchain dependencies are correctly fetched (Click on the "On Chain dependencies" tab -> inspect "Contracts" table). In case "Source skipped" contracts are present, select correct chain for each contract -> "Save overrides" and rerun the analysis
* [ ] Thoroughly inspect analysis settings for signs of manipulation (Click on the "Settings" tab -> check "Analysis Scope", "Install dependencies", and other relevant sections)
* [ ] List every reported vulnerability and warning (split into separate items when one finding contains multiple different issues):
1. Title [Octane URL]
* Link to the relevant code block: [GitHub permalink]
* Is this issue blocking spell deployment?
* Required reason (explain why it can be a problem or can't be):
* [ ] IF any blocking issue is found, raise it to the current spell signal group
* IF new commits are present in the spell
* [ ] Copy relevant checklist items from the above and redo them
* [ ] Ensure newly added code is covered by tests
* [ ] Check if chainlog needs to be updated
* [ ] Copy over and redo "Tests" section from the above
* [ ] IF the spell code has been updated, redo the "Octane Review" section from above
* [ ] IF all checks pass, make sure to include explicit "Good to deploy" comment

## Deployed Stage
Expand Down Expand Up @@ -448,7 +460,7 @@ _Insert your local test logs here_
* [ ] Check that the spell address posted by the crafter in the [Sky Core Executive Vote Address Handover Thread](https://forum.skyeco.com/t/sky-core-executive-vote-address-handover-thread/27995) is correct
* [ ] Confirm the address in the thread – via the forum "Reply" button on the post containing the current spell address, restating the address to make later edits detectable
* Example: ``Confirming {YYYY-MM-DD} Core Spell at [`{SPELL_ADDRESS}`]({BLOCK_EXPLORER_URL}).``
* [ ] Wait until both spell reviewers confirm the spell address in the Handover Thread
* [ ] Wait until Responsible Governance Facilitator confirms handover in the Handover Thread
* [ ] Wait until both spell reviewers confirm the spell address in the Handover Thread
* [ ] Wait until Responsible Governance Facilitator confirms handover in the Handover Thread
* [ ] Ensure that no changes were made to the code since the spell was deployed and archived
* [ ] Approve spell PR for merge via 'Approve' review option
Loading