Remove top-level await #1583

mitschabaude · 2024-04-15T16:29:59Z

Addresses Make every API that needs OCaml/Rust async instead of awaiting at the top level #1205
Removes top-level await to help simplifying integration in UI frameworks
- see Can't use PrivateKey on server in a NextJS 14 app #1575
Side-effect: Mina.LocalBlockchain() becomes async

Details

Instead of awaiting bindings imports at the top-level, we hide them behind a new async function initializeBindings().

initializeBindings() does not have to be called by users of o1js, because this PR takes care to call it first wherever bindings are needed.

Almost all user-facing APIs which need to call initializeBindings() were already async -- making the remaining two async constitutes the two breaking changes of this PR:

LocalBlockchain()
Proof.fromJSON()

We also export initializeBindings() for explicit control, which can be useful to front-load the bindings import in web apps.

Loading time improvements

Because loading of the bindings is now deferred until it's needed, this comes with a great improvement for initial loading time of o1js. It will enable us to remove hacks to load o1js in a useEffect() or similar.

$ ./run-in-browser.js src/examples/benchmarks/import.web.ts

logs in the browser console:

import o1js: 29ms
initialize bindings: 318ms

sam-goodwin · 2024-04-15T19:01:57Z

Thanks for putting this together. I'm a little concerned that we haven't totally confirmed this as the root cause for next. I think there's also a bug in package.json and exports.

I wonder if we can add a test?

next build of a React server component will test the node path.

For client side (web) we'd have to test rendering an app. Is that what playwright is for?

dfstio · 2024-04-15T19:31:59Z

Removing top-level await is also very helpful for non-web environments. For example, it is a blocker for using Netlify functions or some typescript configurations.

mitschabaude · 2024-04-16T05:52:39Z

Thanks for putting this together. I'm a little concerned that we haven't totally confirmed this as the root cause for next. I think there's also a bug in package.json and exports.

I agree that it might seem like the wrong order, but

we want to get rid of top level await anyway
we are currently very focused on delivering v1.0, the first stable version of o1js
which means we need to make all breaking changes now, asap
and also means I don't have time this week to debug the Next setup, which should be fixable without other breaking changes

I'll add the package.json tweak in this PR though!

sam-goodwin · 2024-04-16T06:09:55Z

No I think it's the right order. This is very important. My main point is that we should add a test for client and server side components in next. Full steam ahead on removing top level await! Thanks for getting that going.

sam-goodwin · 2024-04-16T06:10:30Z

But if you wanna follow that up in a later PR, that also makes sense.

mitschabaude

most of the PR is fixing tests -- highlighted some of the actual changes

mitschabaude · 2024-04-16T12:18:21Z

src/lib/mina/account-update.ts

@@ -875,28 +875,11 @@ class AccountUpdate implements Types.AccountUpdate {
  }

  hash(): Field {
-    // these two ways of hashing are (and have to be) consistent / produce the same hash


finally had to remove this, because it would have meant account update hashing depends on the bindings

mitschabaude · 2024-04-16T12:19:03Z

src/lib/mina/local-blockchain.ts

@@ -45,10 +45,11 @@ export { LocalBlockchain };
 /**
 * A mock Mina blockchain running locally and useful for testing.
 */
-function LocalBlockchain({
+async function LocalBlockchain({


LocalBlockchain -> async

mitschabaude · 2024-04-16T12:20:11Z

src/lib/mina/zkapp.ts

+        await initializeBindings();
        let [, data, hash] = Pickles.dummyVerificationKey();


awaiting bindings in deploy(), to use Pickles

mitschabaude · 2024-04-16T12:20:41Z

src/lib/proof-system/circuit.ts

awaiting bindings in Circuit APIs

mitschabaude · 2024-04-16T12:21:35Z

src/lib/proof-system/zkprogram.ts

@@ -110,18 +110,21 @@ class Proof<Input, Output> {
      proof: Pickles.proofToBase64([this.maxProofsVerified, this.proof]),
    };
  }
-  static fromJSON<S extends Subclass<typeof Proof>>(
+  static async fromJSON<S extends Subclass<typeof Proof>>(


Proof.fromJSON() becomes async because it needs Pickles and there's the chance people call this without calling compile() first

mitschabaude · 2024-04-16T12:24:11Z

src/lib/provable/bool.ts

@@ -198,14 +198,13 @@ class Bool {
   * This can only be called on non-witness values.
   */
  toBoolean(): boolean {


I restructured this so that toBoolean() doesn't need Snarky to be defined when the input is constant.

When the input is a variable, it's fine to rely on Snarky because one of the circuit running methods must have been called first, and they are all guarded by initializeBindings()

mitschabaude · 2024-04-16T12:24:56Z

src/lib/provable/core/provable-context.ts

we're awaiting bindings in every method that runs provable code, so that this code is able to use the Snarky import

mitschabaude · 2024-04-16T12:26:20Z

src/lib/provable/crypto/poseidon.ts

@@ -33,6 +33,7 @@ type ProvableHashable<T> = Provable<T> & Hashable<T>;
 class Sponge {
  #sponge: unknown;

+  // TODO: implement constant version in TS. currently, you need to call `initializeBindings()` before successfully calling this


I left this as TODO. Poseidon.Sponge() cannot be used before initializing bindings first, which could lead to users experiencing errors. I think the function is rarely used though, and there's an easy workaround: call initializeBindings(). The eventual solution is to implement Sponge() in TS (should be similar to Poseidon.hash())

Would it be easy/worthwhile to add an error message to call initializeBindings() until Sponge() is implemented in TS? It could be done in the future if so.

mitschabaude · 2024-04-16T12:28:01Z

src/snarky.js

this is how the Node.js bindings work now

mitschabaude · 2024-04-16T12:28:14Z

src/snarky.web.js

this is how the web bindings work now

ymekuria

I'm excited for theses changes! Nice work! I had some minor comments and questions.

ymekuria · 2024-04-16T16:46:28Z

CHANGELOG.md

@@ -28,19 +28,25 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Add assertion to the foreign EC addition gadget that prevents degenerate cases https://github.com/o1-labs/o1js/pull/1545
  - Fixes soundness of ECDSA; slightly increases its constraints from ~28k to 29k
  - Breaks circuits that used EC addition, like ECDSA
+- `Mina.LocalBlockchain()` is made async https://github.com/o1-labs/o1js/pull/1583


This will require updates in the docs as well as the CLI.

ymekuria · 2024-04-16T17:15:12Z

src/lib/provable/crypto/poseidon.ts

@@ -33,6 +33,7 @@ type ProvableHashable<T> = Provable<T> & Hashable<T>;
 class Sponge {
  #sponge: unknown;

+  // TODO: implement constant version in TS. currently, you need to call `initializeBindings()` before successfully calling this


Would it be easy/worthwhile to add an error message to call initializeBindings() until Sponge() is implemented in TS? It could be done in the future if so.

sam-goodwin · 2024-04-17T23:15:26Z

So glad these changes were merged. Is there a timeline for release to NPM?

ymekuria · 2024-04-17T23:38:20Z

So glad these changes were merged. Is there a timeline for release to NPM?

@sam-goodwin There is a plan to release early next week. We will let you know if things change.

mitschabaude added 6 commits April 15, 2024 17:08

defer bindings import to initialize call

60b236c

make local blockchain async and call initialize

ca0d732

change local blockchain usage in examples

cdb2f53

guard uses of Snarky and Pickles bindings by initialize call

c4932b0

refactor to avoid bindings call

24ebf5e

make ml Test module an async function

963e0f0

mitschabaude mentioned this pull request Apr 15, 2024

Can't use PrivateKey on server in a NextJS 14 app #1575

Open

mitschabaude linked an issue Apr 16, 2024 that may be closed by this pull request

Make every API that needs OCaml/Rust async instead of awaiting at the top level #1205

Closed

mitschabaude added 7 commits April 16, 2024 11:57

fix proof and cjs examples

c93e1e4

wasm js wrapper is only created on initialize

31727a6

export initialize and tweak import benchmarks

3e5442a

Merge branch 'main' into feature/no-top-level-await

c624638

changelog

5da8090

call initialize up front in benchmarks

98306b7

move sync runners behind an async function

55605a3

mitschabaude mentioned this pull request Apr 16, 2024

Remove top-level await o1-labs/o1js-bindings#268

Merged

mitschabaude added 2 commits April 16, 2024 14:09

more changelog

e756b7b

fix token test

5376059

mitschabaude commented Apr 16, 2024

View reviewed changes

mitschabaude added 3 commits April 16, 2024 15:20

Merge branch 'main' into feature/no-top-level-await

9d50eda

fix encryption example

4600db8

remove "main" export which confused webpack

0a56798

ymekuria approved these changes Apr 16, 2024

View reviewed changes

This was referenced Apr 16, 2024

Update Mina.LocalBlochain() and Proof.fromJSON() to be async o1-labs/zkapp-cli#622

Closed

Update Mina.LocalBlochain() and Proof.fromJSON() to be async o1-labs/docs2#905

Closed

MartinMinkov approved these changes Apr 16, 2024

View reviewed changes

mitschabaude added 2 commits April 16, 2024 21:26

changelog

4e12d7e

feedback: better error message

9d8eb95

mitschabaude merged commit f74abb3 into main Apr 17, 2024
13 checks passed

mitschabaude deleted the feature/no-top-level-await branch April 17, 2024 05:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove top-level await #1583

Remove top-level await #1583

mitschabaude commented Apr 15, 2024 •

edited

Loading

sam-goodwin commented Apr 15, 2024

dfstio commented Apr 15, 2024

mitschabaude commented Apr 16, 2024 •

edited

Loading

sam-goodwin commented Apr 16, 2024

sam-goodwin commented Apr 16, 2024

mitschabaude left a comment

mitschabaude Apr 16, 2024

mitschabaude Apr 16, 2024

mitschabaude Apr 16, 2024

mitschabaude Apr 16, 2024

mitschabaude Apr 16, 2024

mitschabaude Apr 16, 2024

mitschabaude Apr 16, 2024

mitschabaude Apr 16, 2024

ymekuria Apr 16, 2024 •

edited

Loading

mitschabaude Apr 16, 2024

mitschabaude Apr 16, 2024

ymekuria left a comment •

edited

Loading

ymekuria Apr 16, 2024 •

edited

Loading

ymekuria Apr 16, 2024 •

edited

Loading

sam-goodwin commented Apr 17, 2024

ymekuria commented Apr 17, 2024

		await initializeBindings();
		let [, data, hash] = Pickles.dummyVerificationKey();

Remove top-level await #1583

Remove top-level await #1583

Conversation

mitschabaude commented Apr 15, 2024 • edited Loading

Details

Loading time improvements

sam-goodwin commented Apr 15, 2024

dfstio commented Apr 15, 2024

mitschabaude commented Apr 16, 2024 • edited Loading

sam-goodwin commented Apr 16, 2024

sam-goodwin commented Apr 16, 2024

mitschabaude left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ymekuria Apr 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ymekuria left a comment • edited Loading

Choose a reason for hiding this comment

ymekuria Apr 16, 2024 • edited Loading

Choose a reason for hiding this comment

ymekuria Apr 16, 2024 • edited Loading

Choose a reason for hiding this comment

sam-goodwin commented Apr 17, 2024

ymekuria commented Apr 17, 2024

mitschabaude commented Apr 15, 2024 •

edited

Loading

mitschabaude commented Apr 16, 2024 •

edited

Loading

ymekuria Apr 16, 2024 •

edited

Loading

ymekuria left a comment •

edited

Loading

ymekuria Apr 16, 2024 •

edited

Loading

ymekuria Apr 16, 2024 •

edited

Loading