o1-labs / o1js

TypeScript framework for zk-SNARKs and zkApps
https://docs.minaprotocol.com/en/zkapps/how-to-write-a-zkapp
Apache License 2.0
514 stars 116 forks source link

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

Closed sam-goodwin closed 2 weeks ago

sam-goodwin commented 6 months ago

I am having trouble using o1js in a simple NextJS 14 app.

You can find my repo here: https://github.com/sam-goodwin/private-wallet

Check out the repo and run:

pnpm i
pnpm next build

It will just hang:

next build
   ▲ Next.js 14.1.4
   - Environments: .env

   Creating an optimized production build ...
 ✓ Compiled successfully
 ✓ Linting and checking validity of types    
 ⚠ Sending SIGTERM signal to static worker due to timeout of 60 seconds. Subsequent errors may be a result of the worker exiting.
 ⚠ Restarted collecting page data for undefined because it took more than 60 seconds
 ⚠ See more info here https://nextjs.org/docs/messages/static-page-generation-timeout
   Collecting page data  .

Comment out the PrivateKey.random() and the error goes away.

When looking at the tutorials I spotted this bizarre code:

export default function Home() {

  useEffect(() => {
    (async () => {
      const { Mina, PrivateKey } = await import('o1js');
      const { Add } = await import('../../../contracts/build/src/');
    })();
  }, []);

This raises some red flags. Is o1js not designed to work as a normal module that can be imported?

sam-goodwin commented 6 months ago

Ok, I just found a NextJS config that might be something I am missing.

/** @type {import('next').NextConfig} */
const nextConfig = {
  reactStrictMode: false,

  webpack(config) {
    config.resolve.alias = {
      ...config.resolve.alias,
      o1js: require('path').resolve('node_modules/o1js')
    };
    config.experiments = { ...config.experiments, topLevelAwait: true };
    return config;
  },
  // To enable o1js for the web, we must set the COOP and COEP headers.
  // See here for more information: https://docs.minaprotocol.com/zkapps/how-to-write-a-zkapp-ui#enabling-coop-and-coep-headers
  async headers() {
    return [
      {
        source: '/(.*)',
        headers: [
          {
            key: 'Cross-Origin-Opener-Policy',
            value: 'same-origin',
          },
          {
            key: 'Cross-Origin-Embedder-Policy',
            value: 'require-corp',
          },
        ],
      },
    ];
  }
};

module.exports = nextConfig

Why does o1js require so much hackery to work? Can't it be an ordinary module? These kinds of things are what lead to people abandoning.

sam-goodwin commented 6 months ago

Why is reactStrictMode: false when it is highly recommended not to do this? https://nextjs.org/docs/pages/api-reference/next-config-js/reactStrictMode

Looks like there are some major bugs in the way o1js is distributed.

sam-goodwin commented 6 months ago

I've now tried creating a very simple React Server Component:

"use server";

import { Resource } from "sst";

export default async function Root() {
  const { PrivateKey } = await import("o1js");
  const privateKey = PrivateKey.fromBase58(Resource.RootKey.value);

  return (
    <main>
      <h1>Public Key</h1>
      <span>{privateKey?.toPublicKey().toBase58()}</span>
    </main>
  );
}

When I import o1js I get:

image

I am disappointed. I really wanted to experiment with Mina but o1js looks to have deviated far away from standard JavaScript distribution techniques and is fighting me every step of the way.

mitschabaude commented 6 months ago

Hey @sam-goodwin! I appreciate you trying it out and writing up the problems you encountered!

Is o1js not designed to work as a normal module that can be imported?

Can't it be an ordinary module?

o1js is a normal ES module. It is distributed in two different builds, one for Node.js and one for the web.

Both "just work" when imported in their target environments: Node.js with type: module, and in the browser as a <script type="module">

o1js is also pretty fancy technology so it relies on some web APIs that your typical Todo app doesn't need. One of them is SharedArrayBuffer, for multithreading (without which snark proving would just be too slow). To allow use of SharedArrayBuffer, browsers require the COEP and COOP headers which you saw in that nextjs config. I think an extra config step to enable that is fine.

The main problem here, as far as I can tell, is that NextJS doesn't "just work" when importing modern ES modules. It has trouble with top level await, which we use in our web export and which is supported in all major browsers since 3 years. Instead of serving these modules to the web in their existing, working form, NextJS runs them through the outdated webpack pipeline and messes them up.

I'm not sure but I assume that our hacky custom resolving config is working around exactly that. Maybe @ymekuria can confirm. And in turn, the custom resolving might cause NextJS to pick the wrong o1js export when running the React server component - obviously it should use the Nodejs export, but the "navigator not defined" error suggests that it tries to use the web export instead. (I'm just extrapolating from your error messages, we still need to debug this ourselves)

mitschabaude commented 6 months ago

So, there's an existing plan that would allow us to get rid of top level await #1205

I'm not sure if that already solves everything though. It might still be necessary to tell webpack not to transpile our web export when bundling, since it might still not handle everything there.

And we still need to find out if that's really what causes the Nodejs export to be used in the browser and vice versa, which are the two NextJS bugs reported here

mitschabaude commented 6 months ago

When looking at the tutorials I spotted this bizarre code

      const { Mina, PrivateKey } = await import('o1js');
      const { Add } = await import('../../../contracts/build/src/');

Btw @sam-goodwin this "bizarre code" is just about loading the library lazily, to reduce initial loading time. Actually I think it's a common pattern to use dynamic import for that 😅

sam-goodwin commented 6 months ago

Thanks for the responses, @mitschabaude.

Btw @sam-goodwin this "bizarre code" is just about loading the library lazily, to reduce initial loading time. Actually I think it's a common pattern to use dynamic import for that 😅

Bundlers will optimize the bundled code and if there's expensive initialization code, we can move that out of the module import path and put it in a function. Give control to the user through explicit functions instead of through the await import mechanism.

Sticking to ordinary practices is going to have far less bugs and also scare less people off. I don't think I've ever seen an await import inside a useEffect.

Is this the only place where top-level await is required? For the bindings?

https://github.com/o1-labs/o1js/blob/main/src/snarky.js

Could we instead defer this evaluation by placing it in an init function:

const Mina = await initMina();

Avoiding global state and expensive async processing when importing a module is generally good practice.

To allow use of SharedArrayBuffer, browsers require the COEP and COOP headers which you saw in that nextjs config.

I think this is fine. Seems unavoidable.

This bit scares me:

  reactStrictMode: false,
  webpack(config) {
    config.resolve.alias = {
      ...config.resolve.alias,
      o1js: require('path').resolve('node_modules/o1js')
    };
    config.experiments = { ...config.experiments, topLevelAwait: true };
    return config;
  },

Anything we can do to remove that would be a win.

The main problem here, as far as I can tell, is that NextJS doesn't "just work" when importing modern ES modules. It has trouble with top level await, which we use in our web export and which is supported in all major browser as since 3 years.

I appreciate that top-level await has been supported by browsers, but for Mina to succeed, I think prioritizing smooth integration with the popular web frameworks is more important than using a less supported, modern feature.

RE: https://github.com/o1-labs/o1js/issues/1205 - glad to see there is a plan to remove top-level await. Anything I can do to help? I'd very much like to be able to use o1js just like any other library in my NextJS's client and server side code.

mitschabaude commented 6 months ago

In the web version, this is the only top level await: https://github.com/o1-labs/o1js/blob/c4271420f581c5299eb0b91fa009ee6894f5f773/src/snarky.web.js#L4

mitschabaude commented 6 months ago

And the plan to get rid of it is to:

sam-goodwin commented 6 months ago

In the web version, this is the only top level await:

https://github.com/o1-labs/o1js/blob/c4271420f581c5299eb0b91fa009ee6894f5f773/src/snarky.web.js#L4

What about the node version? I'd like to run this on the server side not just the client. Is that more work?

sam-goodwin commented 6 months ago

Where does this get initialized:

let snarky = globalThis.__snarky;
sam-goodwin commented 6 months ago

call that function in a selected set of places that depend on it

Which places? Is that what is described here: https://github.com/o1-labs/o1js/issues/1205

  • Provable.runAndCheck() and runUnchecked() -- mostly used as part of Mina.transaction() which is already async
  • SmartContract.analyzeMethods()
  • Provable.constraintSystem() -- mostly used in analyzeMethods()
  • Mina.LocalBlockchain()
sam-goodwin commented 6 months ago

Is this what you mean?

  async runAndCheck(f: (() => Promise<void>) | (() => void)) {
    await initO1(); // call it here?
    await generateWitness(f, { checkConstraints: true });
  },
sam-goodwin commented 6 months ago

It's not obvious how to call initO1 from there since it's in a web-backend.js file which is only meant to be imported into a web distribution.

sam-goodwin commented 6 months ago

Managed to get the node version of o1js used in NextJS by removing the main from package.json.

  "main": "./dist/web/index.js", // <-remove this
  "exports": {
    "types": "./dist/node/index.d.ts",
    "browser": "./dist/web/index.js",
    "node": {
      "import": "./dist/node/index.js",
      "require": "./dist/node/index.cjs"
    },
    "default": "./dist/web/index.js"
  },

Still hanging but at least running the right version now I think (not getting navigator error.

mitschabaude commented 6 months ago

Managed to get the node version of o1js used in NextJS by removing the main from package.json.

Nice catch!! 😮

mitschabaude commented 6 months ago

What about the node version? I'd like to run this on the server side not just the client. Is that more work?

Node version uses the one in snarky.js that you found

mitschabaude commented 6 months ago

It's not obvious how to call initO1 from there since it's in a web-backend.js file which is only meant to be imported into a web distribution.

The mechanism is that there are sometimes xyz.web.js or xyz.web.ts files which replace their sibling xyz.js or xyz.ts files during the web build.

So in this case init() would be exported from snarky.web.js and another version of it would be exported from snarky.js (node version). You'd also declare init() in snarky.d.ts, and then import it from there.

mitschabaude commented 6 months ago

Which places? Is that what is described here: https://github.com/o1-labs/o1js/issues/1205

Yes, but that only listed the ones that weren't already async at the time of writing. (And therefore their external API needed to change in a breaking way).

Others are:

mitschabaude commented 6 months ago

Where does this get initialized:

let snarky = globalThis.__snarky;

here: https://github.com/o1-labs/o1js-bindings/blob/13d42834c3ccbe5ce35285979fbe667aa59dfdb7/ocaml/lib/o1js_bindings_lib.ml#L20

sam-goodwin commented 6 months ago

Opened a PR to update the node-backend.js and web-backend.js: https://github.com/o1-labs/o1js-bindings/pull/267

I believe this is required as a first step.

Is there a reason why that code warrants a separate repo vs just being in this repo for simplicity? How can I make changes to both repos in 1 PR? Or is that not possible?

mitschabaude commented 6 months ago

Is there a reason why that code warrants a separate repo vs just being in this repo for simplicity?

it's inconvenient for sure, has to do with how the code is licensed

How can I make changes to both repos in 1 PR? Or is that not possible?

o1js-bindings is a git submodule of o1js. so you

sam-goodwin commented 6 months ago

has to do with how the code is licensed

Oh that's interesting. I thought it was possible to license different folders within a single repository. Major bummer if that's not possible here.

mitschabaude commented 6 months ago

I got most of the way towards removing TLA here: https://github.com/o1-labs/o1js/pull/1583 will finish tomorrow

ymekuria commented 6 months ago

Hi @sam-goodwin! Thanks for all your feedback and clear descriptions of the problems you are facing. I agree that there are opportunities to remove the friction to create an app. The current NextJS scaffold in the zkApp-CLI was originally developed with versions that utilized the pages folder structure and an older version of webpack. We will update the scaffold to utilize Next14 as well the app router and turboPack. With these updates, we can simplify configurations like this.

  reactStrictMode: false,
  webpack(config) {
    config.resolve.alias = {
      ...config.resolve.alias,
      o1js: require('path').resolve('node_modules/o1js')
    };
    config.experiments = { ...config.experiments, topLevelAwait: true };
    return config;
  },