ethers.js icon indicating copy to clipboard operation
ethers.js copied to clipboard

Latest `kzg-wasm` Release `0.5.0` Breaks `ethers` API

Open pcaversaccio opened this issue 1 year ago • 14 comments

Ethers Version

6.13.2

Search Terms

kzg-wasm

Describe the Problem

kzg-wasm released their latest 0.5.0 version with the major changeset coming from this PR: https://github.com/ethereumjs/kzg-wasm/pull/17. You can see how the renamed e.g. blobToKzgCommitmentWasm to blobToKZGCommitmentWasm and thus if you do something like that tx.kzg = kzg;, there will be a TypeScript compilation error:

return new TSError(diagnosticText, diagnosticCodes, diagnostics);
           ^
TSError: ⨯ Unable to compile TypeScript:
scripts/sign-eip4844.ts:66:5 - error TS2739: Type '{ loadTrustedSetup: (trustedSetup?: TrustedSetup | undefined) => number; freeTrustedSetup: () => void; blobToKZGCommitment: (blob: string) => string; computeBlobKZGProof: (blob: string, commitment: string) => string; verifyBlobKZGProofBatch: (blobs: string[], commitments: string[], proofs: string[]) => boolean; veri...' is missing the following properties from type 'KzgLibrary': blobToKzgCommitment, computeBlobKzgProof

66     tx.kzg = kzg;

Code Snippet

You can use my repo here: https://github.com/pcaversaccio/raw-tx

1. `pnpm install`
2. `pnpm upgrade --latest` (will upgrade `kzg-wasm` to version `0.5.0`)
3. `pnpm generate:eip4844`

Contract ABI

No response

Errors

No response

Environment

No response

Environment (Other)

No response

pcaversaccio avatar Sep 21 '24 18:09 pcaversaccio

Aiya. I will look into what makes the most sense to do; I suppose just allow that field to be either and pick the one at runtime that exists?

ricmoo avatar Sep 21 '24 19:09 ricmoo

Aiya. I will look into what makes the most sense to do; I suppose just allow that field to be either and pick the one at runtime that exists?

Sounds reasonable to me.

pcaversaccio avatar Sep 21 '24 19:09 pcaversaccio

@pcaversaccio

Can you please check out the above commit? I've made changes that should fix it (in a backwards compatible way), but it was far more involved than expected. I've tested on 0.3.1 and 0.5.0, but would love extra eyes on it as it changes some signatures and would also love for you to try it out in your TypeScript environment to make sure the types are all happy.

ricmoo avatar Sep 23 '24 22:09 ricmoo

@pcaversaccio

Can you please check out the above commit? I've made changes that should fix it (in a backwards compatible way), but it was far more involved than expected. I've tested on 0.3.1 and 0.5.0, but would love extra eyes on it as it changes some signatures and would also love for you to try it out in your TypeScript environment to make sure the types are all happy.

Left a review here: https://github.com/ethers-io/ethers.js/commit/e5036e778624fea957dd847f7be1a9f148b8997e#commitcomment-147131361. The types are not happy yet :)

pcaversaccio avatar Sep 24 '24 13:09 pcaversaccio

@pcaversaccio Can you please check out the above commit? I've made changes that should fix it (in a backwards compatible way), but it was far more involved than expected. I've tested on 0.3.1 and 0.5.0, but would love extra eyes on it as it changes some signatures and would also love for you to try it out in your TypeScript environment to make sure the types are all happy.

Left a review here: e5036e7#commitcomment-147131361. The types are not happy yet :)

Somehow my GitHub UI is broken and I can't reply in the commit so will do here. So there is still the TypeScript compilation error:

image

this is the line: https://github.com/pcaversaccio/raw-tx/blob/main/scripts/sign-eip4844.ts#L66

The underlying type issue is due to: Transaction.kzg: ethers.KzgLibrary | null

image

pcaversaccio avatar Sep 25 '24 11:09 pcaversaccio

I'm wondering if I'm testing it correctly. Installing locally via pnpm can be done via pnpm add ethers@github:ethers-io/ethers.js#wip-v6.14 correct? (Sorry I'm usually a Python guy...)

pcaversaccio avatar Sep 25 '24 11:09 pcaversaccio

It looks like maybe it didn’t pick up the change?

I’ve never used pnpm (I only use npm), but I would expect that behaves the same.

I think I know the issue; the dist and lib files don’t get built on branches, so while the Ethers TypeScript source is updated, the generated JavaScript is still the old code.

If you clone the library, change to that branch and then npm install && npm run build it should do all that for you and then you can test locally. But I can also build a throw-away branch wip-v6.14-build for you’d when I get home.

ricmoo avatar Sep 25 '24 16:09 ricmoo

Can you maybe do a release candidate like v6.14.0-alpha.1 so I can simply install it from the npm registry? That would be easiest for me tbh

pcaversaccio avatar Sep 25 '24 17:09 pcaversaccio

Sure, that should be doable. :)

ricmoo avatar Sep 25 '24 17:09 ricmoo

Awesome - so ping me here after the release and I will test again!

pcaversaccio avatar Sep 25 '24 17:09 pcaversaccio

Sure, that should be doable. :)

any update here :D?

pcaversaccio avatar Oct 03 '24 08:10 pcaversaccio

I have made pure JS KZG, which is now used in ethereumjs by default.

It is available in the following import.

paulmillr avatar Oct 17 '24 19:10 paulmillr

@paulmillr thanks! I’m adding a page in the documents to show how to use ethers with your pure-JS implementation too. I’ve also heard it is substantially faster than the WASM version. :)

ricmoo avatar Oct 17 '24 19:10 ricmoo

(and am adding support along with the other change to accept the API function names you use)

ricmoo avatar Oct 17 '24 19:10 ricmoo

The Transaction object will accept any of [email protected], [email protected] and micro-ecc-signer KZG libraries.

// kzg-wasm (either v0.4 or v0.5)
import { loadKZG } from "kzg-wasm-4";
tx.kzg = loadKSG();

// micro-ecc-signer
import { KZG } from 'micro-eth-signer/kzg';
import { trustedSetup } from '@paulmillr/trusted-setups/fast.js';
tx.kzg = new KZG(trustedSetup);

Internally, the libraries are all normalized to the [email protected] API, since that is what was first supported and we don't want to break backwards compatibility.

Try it out and let me know if you have any issues.

Thanks! :)

ricmoo avatar May 07 '25 02:05 ricmoo

(quick aside, I've heard the pure-JS version provided by micro-ecc-signer is faster than the WASM version; any benchmarks are welcome on this issue ;))

ricmoo avatar May 07 '25 02:05 ricmoo

Tested on Sepolia with [email protected] and worked smoothly: https://sepolia.etherscan.io/tx/0xbf2c7e26b37e272674ba6c586a4d5ec83760b4542f4fafe548387739e4dbc6b2

Thx for fixing this. Will test micro-eth-signer/kzg some day.

pcaversaccio avatar May 07 '25 11:05 pcaversaccio