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

Node.js crypto module dependency breaks browser bundling #35

Closed
holgerd77 opened this issue Jun 17, 2024 · 9 comments
Closed

Node.js crypto module dependency breaks browser bundling #35

holgerd77 opened this issue Jun 17, 2024 · 9 comments

Comments

@holgerd77
Copy link

Hi @herumi,
Holger from EthereumJS here again (just re-discovered that I opened a PR here alread couple of years ago #17). 👋

I was just playing with some bundler for the web (esbuild) to test browser compatibility for our libraries, especially VM/EVM and realized that there is a dependency (aka: usage) of the native Node.js crypto module in the getRandomValues.ts file which breaks browser bundling:

grafik

(so there I trie to create a bundle from our @ethereumjs/vm code)

Is this getRandomValues.ts file even necessary/used or did this just "sneak into the build/release"?

If possible to remove and this could get a small fix (can also submit a PR if this eases things) and bugfix release that would be great! 🙂 This would improve browser UX for several of our libraries a lot!

@herumi
Copy link
Owner

herumi commented Jun 18, 2024

The current getRandomValues.ts does not contain requires. What version do you see?

@holgerd77
Copy link
Author

Ah, the version thing, sorry, should have checked.

Yes, I am on 1.4.0. Have done a manual 1.5.0 installation and went into the node_modules/mcl-wasm/dist/getRandomValues.js files to compare and - you are right - also the final .js files now have the require gone.

Thanks for your quick answer, will close here!

@holgerd77
Copy link
Author

Ah, and a side question, only if you are interested though, so just do let me know if you do not have the time to deal with it:

We have a somewhat huge https://github.com/ethereumjs/rustbn-wasm WASM build for the elliptic curve precompiles for the EVM which would be wonderful if we could reduce in size or remove.

In this context we thought if it would be possible to use the BN254 functionality from this library for the BN128 curve addition, multiplication and paring functionality. Do you have any insight if this might be working or can you directly reject? 🤔 We are very much "Elliptic Curve" noobs respectively rather "users" and no mathematicians.

//cc @acolytec3

@herumi
Copy link
Owner

herumi commented Jun 18, 2024

If BN128 means alt_bn128, then I think mcl.BN_SNARK1 corresponds to it.
cf. https://github.com/herumi/mcl/blob/master/api.md#curve-parameter

@holgerd77
Copy link
Author

Thanks a lot! 🙏

@acolytec3
Copy link

One more related question, is there somewhere in the API to convert a a point on the curve represented as x,y coordinates to its scalar value? The input for the ADD precompile in EIP-196 is 2 points represented by their x,y coordinates (I guess what is referred to as a "jacobian representation"(?) on the curve and it's not obvious to me what the correct method on the mcl object would accept those coordinated as inputs for mcl.add.

I've tried using the below snippet for testing out various mcl constructor methods to try and generate a result that matches the output of our existing implementation. The input is a 256 character string representing two pairs of x/y coordinates (which each coordinate each is represented as a 32 byte number/64 character hexadecimal string) concatenated end to end and this corresponds to the inputs passed to the precompile in the EVM.

The snippet uses deserializeHexStr here but this errors with err _wrapDeserialize: 32 != 64 (which I assume implies that deserializeHexStr only accepts 32 byte numbers as inputs. Is there another method on the mcl API that accepts the x/y coordinate representation you can point me to?

    await mcl.init(mcl.BN_SNARK1)
    const input =
'0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002'
    const output =
'030644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd315ed738c0e0a7c92e7845f96b2ae9c0a68a6a449e3538fc7ff3ebf7a5a18a2c4'

    const p1 = new mcl.Fp()
    p1.deserializeHexStr(input.slice(0, 128))
    const p2 = new mcl.Fp()
    p2.deserializeHexStr(input.slice(128, 256))
    const res = mcl.add(p1, p2)
    console.log(res.getStr(16))

@herumi
Copy link
Owner

herumi commented Jun 19, 2024

mcl does not support the encoding defined in eip-197.
The format of mcl is https://github.com/herumi/mcl/blob/master/api.md#get-string.
If you want to add strings encoded by eip-197, cut the string every 64 bytes, make two G1 instances by getStr, and use mcl.add. Format the result by getStr by padding zero and concatenating them according to eip-197.

await mcl.init(mcl.BN_SNARK1)
const a = '0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002'
'0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002'

const p1 = new mcl.G1()
const p2 = new mcl.G1()
p1.setStr(`1 ${a.slice(64*0, 64*1)} ${a.slice(64,64*2)}`)
p2.setStr(`1 ${a.slice(64*2,64*3)} ${a.slice(64*3,64*4)}`)
mcl.add(p1, p2).getStr(16)

>'1 30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd3 15ed738c0e0a7c92e7845f96b2ae9c0a68a6a449e3538fc7ff3ebf7a5a18a2c4'

Remark. The top 1 means affine coordinate (mcl's rule).

@holgerd77
Copy link
Author

🙏 🙏 🙏 🙏 🙏

@herumi
Copy link
Owner

herumi commented Jun 20, 2024

I should have used setStr(1 ${x} ${y}, 16) since the input string is in hexadecimal.

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