-
Notifications
You must be signed in to change notification settings - Fork 773
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
Review KZG Interface & Integration / Integrate pure JS KZG #3662
Comments
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 wevm/viem#2720 |
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. |
Trusted setups are available in #3670 You can test them out using this:
|
@paulmillr Is there a bug in how the 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 |
«fast” var must have the property encoding === 'fast_v1' you can add it manually to fast.js for now |
#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 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). |
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? |
We generally deal with everything as |
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 |
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. |
Uint8arrays can't be passed via network, they need to be serialized first into some format. |
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. |
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". |
The package is now live on NPM at |
Closed by #3674 |
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 fromkzg-wasm
) in mind. We should see if all makes sense there + is necessary. I wonder e.g. ifloadTrustedSetup()
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.
The text was updated successfully, but these errors were encountered: