ethereumjs / ethereumjs-monorepo

Monorepo for the Ethereum VM TypeScript Implementation
2.6k stars 756 forks source link

Review KZG Interface & Integration / Integrate pure JS KZG #3662

Closed holgerd77 closed 1 month ago

holgerd77 commented 1 month ago

Thanks to @paulmillr there is now a pure JS KZG implementation available here within the micro-eth-signer library! 🎉

We should take this occasion and generally review our interface definition from Util which we use to type the CustomCrypto input in Common (for re-use e.g. in the EVM) or also the 4844 txs.

Guess this (the Kzg interface) was very much designed with our own code (respectively the code we had/have from kzg-wasm) in mind. We should see if all makes sense there + is necessary. I wonder e.g. if loadTrustedSetup() really needs to be in the interface? Or is this a left-over from when we called this ourselves?

Another thing is that we might e.g. want to make this verifyBlobKzgProofBatch() batch function optional and rather adjust our implementation so that if the method is not available the single verification method is called.

Generally we likely want to coordinate with @paulmillr here so that we get to a practical and matching interface. Not sure if we still need to provide some small wrapper here at the end (because the APIs remain slightly different), but that would also be ok I guess.

At the end we should also adjust our KZG documentation like the Setup section in the tx library and generalize for both the JS and WASM versions.

And lastly we should also give testing some additional thought (might be a separate PR/also someone else from the team), and see that both the JS and WASM versions are tested for both the tx library and the EVM in our CI.

Will assign @acolytec3 here since he has done most of the KZG work and is likely best suited to work on this integration.

paulmillr commented 1 month ago

Found this doc https://github.com/wevm/viem/blob/408948bb74685e9d562cd39f50768f9b282463fe/site/pages/docs/utilities/defineKzg.md?plain=1#L16

Also I want to create separate pkg for trusted setups https://github.com/wevm/viem/issues/2720

acolytec3 commented 1 month ago

I'll pick this up either today or next week. Will try and look at some benchmarks to see if there's much difference between the pure JS version and wasm along the way.

paulmillr commented 1 month ago

Trusted setups are available in https://github.com/ethereumjs/ethereumjs-monorepo/issues/3670

You can test them out using this:

git clone git@github.com:paulmillr/trusted-setups.git
cd trusted-setups
npm install && npm run build && npm pack
mv trusted-setups-0.1.0.tgz <repository>
cd <repository>
npm install ./trusted-setups-0.1.0.tgz
acolytec3 commented 1 month ago

@paulmillr Is there a bug in how the fast setup is constructed? I followed the above instructions as far as building and then installed locally via npm link and then using this code to import:

import { trustedSetup as fast } from 'trusted-setups/fast.js'

const fastKZG = new KZG(fast)

and it's giving this error:

Error: pointHex must be valid hex string, got "00413c0dcafec6dbc9f47d66785cf1e8c981044f7d13cfe3e4fcbb71b5408dfde6312493cb3c1d30516cb3ca88c03654 1690c1ade165e7c0b1fbdd0dc7ce71a8cfccbb16708de5164b32f31166b7a6bed225d39038457e05214cfda6f567b61c". Cause: Error: padded hex string expected, got unpadded hex of length 193
 ❯ ensureBytes ../../node_modules/@noble/curves/src/abstract/utils.ts:128:13
 ❯ Function.fromHex ../../node_modules/@noble/curves/src/abstract/weierstrass.ts:415:44
 ❯ parseG1 ../../node_modules/micro-eth-signer/src/kzg.ts:144:39
 ❯ new KZG ../../node_modules/micro-eth-signer/src/kzg.ts:135:35

The "slow" setup seems to load. Going to start testing outputs against kzg-wasm today.

paulmillr commented 1 month ago

«fast” var must have the property

encoding === 'fast_v1'

https://github.com/paulmillr/micro-eth-signer/blob/17183a114ab581a11710f197345a02a655ee540c/src/kzg.ts#L132

you can add it manually to fast.js for now

acolytec3 commented 1 month ago

3674 Here's a start. Looks like everything works (pending full CI run)

@paulmillr is it necessary to use strings for all of the inputs for the kzg library? I don't know enough about the inner workings of the kzg code in micro-eth-signer to know if it's more/less efficient for your library to use strings versus Uint8Arrays. In our code, we generally have all the fields in Uint8Arrays and pass that to the kzg code directly.

Because your library uses strings for the inputs/outputs, we do conversions from bytes to strings for each input for your kzg library and then back from strings to bytes for the outputs in our code.

I don't have a strong opinion other than the fact that we already use Uint8Arrays and it'd be nice to not have to do conversions coming and going since that will have a performance drag (though I suspect not major since we don't generate/verify KZG proofs as a major bottleneck in our code).

paulmillr commented 1 month ago

The strings are converted to bigint, which is fast. Converting Uint8Arrays to bigints would in any case be done via strings.

The question here what's more correct. E.g. spec-wise or something like that. How are you building such Uint8Arrays in your library? Are you just "passing" data around, without building it?

acolytec3 commented 1 month ago

We generally deal with everything as Uint8Arrays since we treat the kzg-wasm package as a black box and just pass bytes in and get the correct sized responses back as bytes.

acolytec3 commented 1 month ago

We will build the blob from whatever underlying data a tx submitter provides but everything else from there on is just bytes. The kzg-wasm library does all of the serializing and deserializing of data under the hood and just gives us the proofs/commitments and we really on their results for proof verification

acolytec3 commented 1 month ago

We will build the blob from whatever underlying data a tx submitter provides but everything else from there on is just bytes. The kzg-wasm library does all of the serializing and deserializing of data under the hood and just gives us the proofs/commitments and we really on the results for proof verification.

paulmillr commented 1 month ago

Uint8arrays can't be passed via network, they need to be serialized first into some format.

acolytec3 commented 1 month ago

Correct, so everything sent over the wire is sent as hex strings. So fair point to us. @holgerd77 I will look through our codebase see if there's ever a place where we "need" the KZG values (blobs/commitments/versionedHashes/proofs) as bytes or whether we can just handle them as strings.

paulmillr commented 1 month ago

I of course can make everything U8A, just want to get to the bottom of this, whether the types they've picked out of necessity are "correct ones".

paulmillr commented 1 month ago

The package is now live on NPM at @paulmillr/trusted-setups.

holgerd77 commented 1 month ago

https://github.com/paulmillr/trusted-setups

holgerd77 commented 1 month ago

Closed by #3674