-
Notifications
You must be signed in to change notification settings - Fork 14
feat: validate space strategies #513
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?
Conversation
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.
PR Overview
This PR introduces a new validation mechanism for space strategies by fetching a list of approved strategies from an external API.
- Importing fetchWithKeepAlive and a new SCORE_API_URL from environment variables
- Adding a try-catch block in the verify function to validate strategy names and deprecation status
- Rejecting the process if any strategy is invalid or deprecated
Reviewed Changes
File | Description |
---|---|
src/writer/settings.ts | Validates strategies by calling an external API to ensure strategy validity and status |
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
0188a18
to
5d8276d
Compare
5d8276d
to
fac6311
Compare
@@ -80,6 +81,26 @@ export async function verify(body): Promise<any> { | |||
return Promise.reject(`max number of strategies is ${strategiesLimit}`); | |||
} | |||
|
|||
try { | |||
const strategiesList = await (await fetchWithKeepAlive(`${scoreAPIUrl}/api/strategies`)).json(); |
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.
Maybe we should call it every 5 mins instead of calling it everytime, but can make things look complicated 🙈
@@ -80,6 +81,26 @@ export async function verify(body): Promise<any> { | |||
return Promise.reject(`max number of strategies is ${strategiesLimit}`); | |||
} | |||
|
|||
try { |
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.
Maybe we should move this check and strategies limit check to a new function similar to validateSpaceSettings
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.
utAck, other than above this looks good
Toward https://github.com/snapshot-labs/workflow/issues/227
This PR adds validation to the space top level strategies.
It will ensure that:
Test
multichain
strategy