-
Notifications
You must be signed in to change notification settings - Fork 10
Add SVM Payload Generation Review Checklist and Complement Star Spell Checklist with SVM Payload. #59
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?
Add SVM Payload Generation Review Checklist and Complement Star Spell Checklist with SVM Payload. #59
Conversation
SidestreamColdMelon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a brief look, most of the comments are not specific to SVM (as I think all of the checks are). There are a few general suggestions:
- Perhaps you can make checks more SVM-specific. Otherwise there is no much point of those checks and it's just enough to compare payload.txt verified by the auditor (and found at the commit specified in the external report) with the payload in the spell. This can be described inside
star-spell-reviewer-checklist.mdvia a few points- For example, if all of the checks are correct here, but upgrade authority was not passed to the SKY Oapp (which is done outside of the payload generation scripts) or if program bytecode doesn't match – none of the checks listed here doesn't make any sense
- In the PR description you mention Wormhole, but it's no longer functional and is not planned to be used anymore
|
|
||
| **Simulation Execution** | ||
|
|
||
| - [ ] Run validation script: **`NETWORK=[network] ts-node ./scripts/SPELL_NAME/validate.ts --file FILENAME`** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in order for the validate output to make sense, its contents first needs to be inspected (with the same attention to addresses as the in the generate-payload file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll need more context for this one, but as far as I know, there will be an audit reviewing the generation code. We'll only be confirming whether the generated text is correct through simulation and re-running the generation. The process is still under construction
|
Regarding SVM-specific functionality, there will be an audit for the SVM part. Any calls added should be reviewed before using them in payload generation. So I think, for this list, it's only necessary to validate the correct generation of the payload. |
Summary
Introduces a review checklist for SVM payload generation in cross-chain governance actions, covering both the SVM checklist and EVM spell integration.
Rationale
Sky's governance will execute cross-chain actions on SVM via LayerZero bridge. We need systematic review processes for SVM payloads similar to our EVM spell reviews.
Cross-chain governance actions follow this two-stage review process:
SVM Payload Generation
EVM Spell Integration
Both checklists work together to ensure end-to-end correctness of cross-chain governance actions.
SVM Payload Generation: https://github.com/keel-fi/crosschain-gov-solana-spell-payloads