-
Notifications
You must be signed in to change notification settings - Fork 110
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
Add ZkProgram name as required argument #1200
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.
Just had one question, but approving nonetheless :D
@@ -273,7 +275,7 @@ function ZkProgram< | |||
let publicInputType: ProvablePure<any> = config.publicInput! ?? Undefined; | |||
let publicOutputType: ProvablePure<any> = config.publicOutput! ?? Void; | |||
|
|||
let selfTag = { name: `Program${i++}` }; | |||
let selfTag = { name: config.name }; |
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.
Do we want to sanitize this input in any way since this will be written to disk? Some thoughts that come to mind:
- Safe encoding (making sure only alphanumeric characters and separators are specified)
- String length
These are just some thoughts, it's entirely up to you if you find them appropriate :D
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.
Yes very appropriate! I'll figure it out in the caching PR, maybe serialize them before using as filename but leave the name string on the ZkProgram
in original form
oh thanks to Maki for noticing! |
Adds
name
argument to ZkProgram.name
is required in the new non-experimental API, but still optional in the oldExperimental.ZkProgram