nodeCrypto is truthy when disabled

[webmaster128]

When using

  "browser": {
    "crypto": false
  },

the rollup tool generates the following code (taken from https://github.com/paulmillr/noble-secp256k1/releases/download/1.7.0/noble-secp256k1.js):

    const _nodeResolve_empty = {};

    const nodeCrypto = /*#__PURE__*/Object.freeze({
        __proto__: null,
        'default': _nodeResolve_empty
    });

Now the resulting nodeCrypto is truthy.

$ node b.js
true

with b.js being:

    const _nodeResolve_empty = {};

    const nodeCrypto = /*#__PURE__*/Object.freeze({
        __proto__: null,
        'default': _nodeResolve_empty
    });

    console.log(!!nodeCrypto)

This also means the checks in e.g.

            if (crypto.web) {
                return crypto.web.getRandomValues(new Uint8Array(bytesLength));
            }
            else if (crypto.node) {
                const { randomBytes } = crypto.node;
                return Uint8Array.from(randomBytes(bytesLength));
            }
            else {
                throw new Error("The environment doesn't have randomBytes function");
            }

will always be true, even if crypto.node is empty.

I think this is no immediate issue but I am wondering if there is a way to increase the robustness here.

[webmaster128]

Webpack 5 behaves the same way. This is why I used this node crypto availability check:

export async function getCryptoModule(): Promise<any | undefined> {
  try {
    const crypto = await import("crypto");
    // We get `Object{default: Object{}}` as a fallback when using
    // `crypto: false` in Webpack 5, which we interprete as unavailable.
    if (typeof crypto === "object" && Object.keys(crypto).length <= 1) {
      return undefined;
    }
    return crypto;
  } catch {
    return undefined;
  }
}

[paulmillr]

maybe we check for typeof randomBytes === function

[webmaster128]

Yeah, this is a good solution for sure. Something like typeof crypto.node?.randomBytes === "function".

I'm just slightly worried about the duplication this brings as we need the same pattern in multiple places. So maybe it would be better to change the code where crypto.node is assigned.

[paulmillr]

@webmaster128 in the process of gh-86, i've discovered an issue.

git checkout noble-curves && npm install && npm run build && npm run build:release

Rollup simply removes if (crypto.node) from the UMD build. Which means the UMD build is not universal and cannot be used in node.js.

This can be fixed by doing two manual changes, first in const crypto

        node: typeof require === 'function' && require('crypto'),

second in function randomBytes, adding the clause back:

        else if (crypto.node) {
            return crypto.node.randomBytes(bytesLength);
        }

This makes UMD build with both in browser and node.js

[irustm]

this will also remove this dependency.

imports.json: {"imports": {"crypto": "https://deno.land/std@0.153.0/node/crypto.ts"}}

you don't need it because deno has its own crypto

[paulmillr]

v2.0 will use webcrypto, which is present in all browsers and node v19+. for older nodes a 1-line import will be necessary before importing secp.

For old behavior there'll be noble-curves.