-
Notifications
You must be signed in to change notification settings - Fork 105
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
latest API of snarkyjs #36
Conversation
…js into feature/workshop-examples
…feature/workshop-examples
export class Perm { | ||
constant: Bool; | ||
signatureNecessary: Bool; | ||
signatureSufficient: Bool; |
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.
It would be less confusing to me if "signatureSufficient" would instead be called "proofNecessary", and the Bool negated (if that's accurate)
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.
What exactly does constant
mean? I thought whether no one has the permission, but below it's also used for the case where everyone has the permission..
} from '@o1labs/snarkyjs'; | ||
|
||
class SimpleSnapp extends SmartContract { | ||
@state(Field) x = State<Field>(); |
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.
Reminder: we need to PR the docs after this update is merged please, so the changes are reflected in the docs.
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.
And change the examples (at least the ones in snapp CLI)
return new Int64(this.value.sub(y.value)); | ||
} | ||
|
||
repr(): { magnitude: Field; isPos: Sgn } { |
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.
What does this method do?
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.
@@ -15,9 +24,24 @@ class Main extends Circuit { | |||
} | |||
} | |||
|
|||
console.log('generating keypair...'); |
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.
are these console logs all junk?
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.
To be honest - I run this example file often for testing, and I like them because the whole operation takes more than 10 seconds and this waiting time is less confusing/annoying if you see what is currently happening. We could remove the timings
]; | ||
} | ||
|
||
y_ = y_.seal(); |
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.
and seal
here (or more broadly, anywhere). What does the seal function do? Can we document this better?
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.
seal
and rangeCheckHelper
both have TODOs in snarky.d.ts to document cc @imeckler to provide color for what these methods do
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 can see what rangeCheckHelper
does from context -- is there a good reason this needs to be bridged instead of just reimplemented in typescript? is it doing something particularly special under-the-hood that we can't remake in user-space?
General comment: Our builds are output to |
I agree, at least with I'll change the bindings directories' names after I changed them in the mina repo to keep copying simple, but will wait for that until everything is merged together there because merging changed directories is a pain |
FYI, I now merged in the latest version of the |
Remove uses of JS classes in OCaml, part 2
No description provided.