Skip to content
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

Async circuit, pt. 3 - contract methods #1477

Merged
merged 20 commits into from
Mar 8, 2024

Conversation

mitschabaude
Copy link
Collaborator

@mitschabaude mitschabaude commented Mar 4, 2024

Closes #735

Most breaking change ever -- requires all smart contract and zkprogram methods to be async.

This PR is stacked behind #1450 and #1468 where most of the internal changes were already made

Here, there aren't much changes apart from

  • The type signatures of @method and ZKProgram; introducing @method.returns(..) for methods with a return value
  • Ensuring methods are awaited in the method wrapper
  • Fixing all examples

TODO:

  • changelog (will add after merging main post release)
  • update bindings to make tests succeed (will do after ocaml side is final and reviewed)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted this entire folder, it was a left-over from CI tests in the Mina repo that don't exist anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleted this test, don't think it was still useful

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this example was too similar to reducer-composite.ts, I'm trying to purge examples that don't really add anything, as they are a burden to maintain

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an actual change here. I think the try-catch was unused because it didn't await the promise, so removed it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most of the actual changes in this file!

Comment on lines +99 to +102
function method<K extends string, T extends SmartContract>(
target: T & {
[k in K]: (...args: any) => Promise<void>;
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the decorator type now enforces a particular function signature of the method that is decorated:

(...args: any) => Promise<void>

'design:returntype',
target,
methodName
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we no longer infer the method return type, instead there is @method.returns(...)

Comment on lines +240 to +243
result = await assertPromise(
method.apply(this, clonedArgs),
noPromiseError
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throughout the wrapped method, we are now awaiting the method result and also throwing a helpful runtime error if it's not returning a promise

* });
* tx.sign([senderKey, zkAppKey]);
* ```
*/
deploy({
async deploy({
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deploy() needs to be async because it calls init(), which can be a method so might be async

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ZKProgram was already supporting async methods at runtime, but not at the type level. This here is the change that forces the method type to be async.

@mitschabaude mitschabaude marked this pull request as ready for review March 6, 2024 07:19
@mitschabaude mitschabaude requested a review from a team as a code owner March 6, 2024 07:19
@mitschabaude mitschabaude changed the title Async circuit pt 3 - contract methods Async circuit, pt. 3 - contract methods Mar 7, 2024
Base automatically changed from feature/async-methods to main March 7, 2024 10:08
Copy link
Member

@Trivo25 Trivo25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! We should definitely make sure all the tutorial are up to date as soon as we release this. Happy to help

@mitschabaude mitschabaude merged commit 7f750b6 into main Mar 8, 2024
13 checks passed
@mitschabaude mitschabaude deleted the feature/async-methods-for-real branch March 8, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make circuits async: JS side
2 participants