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

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

Open
sam-goodwin opened this issue Apr 14, 2024 · 25 comments
Open

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

sam-goodwin opened this issue Apr 14, 2024 · 25 comments

Comments

@sam-goodwin
Copy link

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

mitschabaude commented Apr 14, 2024

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
Copy link
Member

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
Copy link
Member

mitschabaude commented Apr 14, 2024

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
Copy link
Author

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: #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
Copy link
Member

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

await initO1();

@mitschabaude
Copy link
Member

And the plan to get rid of it is to:

  • Refactor that function so that it won't repeat its work when called a second time
  • call that function in a selected set of places that depend on it
  • make a dummy Nodejs version of it so we can do the above universally
  • Also export the init function, so that people can trigger the initialization work explicitly. Note that doing this should be optional, because we also guarantee that it's called when needed

@sam-goodwin
Copy link
Author

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

await initO1();

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
Copy link
Author

Where does this get initialized:

let snarky = globalThis.__snarky;

@sam-goodwin
Copy link
Author

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

Which places? Is that what is described here: #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
Copy link
Author

sam-goodwin commented Apr 15, 2024

Is this what you mean?

  async runAndCheck(f: (() => Promise<void>) | (() => void)) {
    await initO1(); // call it here?
    await generateWitness(f, { checkConstraints: true });
  },

@sam-goodwin
Copy link
Author

sam-goodwin commented Apr 15, 2024

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
Copy link
Author

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
Copy link
Member

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

Nice catch!! 😮

@mitschabaude
Copy link
Member

mitschabaude commented Apr 15, 2024

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
Copy link
Member

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
Copy link
Member

mitschabaude commented Apr 15, 2024

Which places? Is that what is described here: #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:

  • compileProgram() in proof-system.ts
  • generateKeypair() and prove() in circuit.ts
  • Proof.dummy()
  • I'm probably forgetting something right now

@mitschabaude
Copy link
Member

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
Copy link
Author

Opened a PR to update the node-backend.js and web-backend.js: o1-labs/o1js-bindings#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
Copy link
Member

mitschabaude commented Apr 15, 2024

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

  • always work from o1js, with the submodule checked out
  • submit 2 PRs, one from the submodule, and one from the main repo, with the main repo one also updating the submodule to the right commit
  • we usually make our lives easier by using the same branch names and PR titles for both PRs

@sam-goodwin
Copy link
Author

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
Copy link
Member

I got most of the way towards removing TLA here: #1583
will finish tomorrow

@ymekuria
Copy link
Contributor

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;
 },

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

No branches or pull requests

3 participants