Transaction Construction, Pt 1. @method and compile()
#115
Conversation
| @method async update(y: Field) { | ||
| let x = await this.x.get(); | ||
| @method update(y: Field) { | ||
| let x = this.x.get(); |
There was a problem hiding this comment.
How will the .get function work synchronously? I figured that it would eventually make a network call to get the state of a contract on the chain, and that would need to be async?
There was a problem hiding this comment.
I laid out here how I think it will work: #113
The only alternative I see would be to refactor Pickles so that it can handle async circuits. This would be better for users (enables them to make their methods async, which adds a lot of flexibility), but is probably much harder to implement.
| console.log('compile'); | ||
| let { | ||
| provers: [updateProver], | ||
| } = SimpleSnapp.compile(snappPubKey); |
There was a problem hiding this comment.
Should references of snapp be scrubbed for zkapp? Not sure if that's already done in a future PR :P
There was a problem hiding this comment.
Yeah that's in a future PR :P
| 'Cannot deploy SmartContract outside a transaction.' | ||
| ); | ||
| } catch { | ||
| throw new Error('Cannot deploy SmartContract outside a transaction.'); |
There was a problem hiding this comment.
Do these errors get surfaced up to the user? Maybe we can add an additional text snippet alongside this message that helpfully guides the user on how to fix this if there is a common fix. For this, something like Make sure you are using Mina.transaction ? Wondering what everyone else thinks.
There was a problem hiding this comment.
In the current version, this particular error is actually not present any more, but a very similar error mentioning Mina.transaction would be thrown: https://github.com/o1-labs/snarkyjs/blob/f0014217b9911438da621751a33f7bd3c97f3586/src/lib/zkapp.ts#L485
| let snarkySpec = require('./snarky-class-spec.json'); | ||
|
|
||
| let { picklesCompile } = snarky; |
There was a problem hiding this comment.
I think it would be really helpful to have a short comment above on what picklesCompile is doing. Since it's defined in the OCaml side of things, it would be nice to know what it does without swapping to a different project.
There was a problem hiding this comment.
addressed here: f001421
mitschabaude
changed the base branch from
main
to
feature/transaction-construction
| provers: [updateProver], | ||
| } = SimpleSnapp.compile(snappPubKey); | ||
|
|
||
| console.log('deploy'); |
There was a problem hiding this comment.
I assume the console logs are removed in further PRs
There was a problem hiding this comment.
They aren't! Are you against logging out what's happening in examples? I tend to find that helpful, especially logging the initial and final states here helps developers confirm that it's working
There was a problem hiding this comment.
that's fair -- these seem okay to keep. I think we should do something more fancy in the future, but we can do that later.
| } | ||
| return fields; | ||
| } | ||
| return []; |
There was a problem hiding this comment.
Is it less surprising for this to throw or somehow accumulate the skipped data instead of return [] for non-toFields-able fields of an object? I'm thinking this will lead to lots of subtle errors
There was a problem hiding this comment.
This function isn't used right now, but being able to return [] is important to handle mixed objects, which contain both circuit- and non-circuit properties. For example, a party.body.update.verificationKey contains a property value: string, which holds the base58-formatted verification key. This string should be ignored by toFieldsMagic(), so that a Party can be turned into field elements.
There was a problem hiding this comment.
Accumulating the skipped data could be an option!
There was a problem hiding this comment.
I decided to remove the function instead of changing it (b80d39f), since it's unused and most likely won't be useful, because if we want to compute hash inputs we'll have to conform to the more elaborate system used in Mina.
| witnessArgs.push(Parameter); | ||
| } else { | ||
| throw Error( | ||
| `Argument ${i} of method ${methodName} is not a valid circuit value.` |
There was a problem hiding this comment.
(not in this PR, I made #130 to track it).
We should include details about why this is a not a valid circuit value and what a circuit value is in this error.
| 'Cannot deploy SmartContract outside a transaction.' | ||
| ); | ||
| } catch { | ||
| throw new Error('Cannot deploy SmartContract outside a transaction.'); |
Pickles.compileand add methods to compile SmartContracts@methoddecorator to easily specify SmartContract sub-circuits / branches