Use prefixed hex strings in outer API input/outputs

[acolytec3]

Addresses #16

[gabrocheleau]

Also addresses ethereumjs/ethereumjs-monorepo#3656

[ricmoo]

@gabrocheleau

This seems to have broken Ethers dramatically... :s

I understand this is a 0.x release, so semver-compliance isn't expected yet, but a few notes I'm working around now:

• the API changed blobToKzgCommitment to blobToKZGCommitment (notice the case of "KZG")
• the API no longer accepts binary data (as a Uint8Array) and must now take a hex string; is there a reason for this change (perhaps concern over the arrayBuffer being shared and leaking side-channel state?)

I'm adding support for the first change, the change in the function name (either will be accepted by Ethers), but would love a heads up in the future. ;)

The second, I think has to be passed along to the developer, since I don't want Ethers to have to process the input types, as that would require an Ethers-specific wrapper for the KZG goodness. But I'd also wonder if we could change index.ts#40 to something more akin to:

<strike>const blobToKZGCommitment = (blob: ***string***) => {</strike>
const blobToKZGCommitment = (blob: ***string | Uint8Array***) => {
  <strike>const blobHex = '0x' + blobToKZGCommitmentWasm(***hexToBytes(blob)***)</strike>
  const blobHex = '0x' + blobToKZGCommitmentWasm(***typeof(blob) === "string" ? hexToBytes(blob): blob***)
  return blobHex
}

That would help ensure that existing code continues to work, and save a bit of extra overhead when we already have binary data. :)

[ricmoo]

(actually, I do have a point in Ethers I can hook into to convert into a string; but to ensure forwards/backwards compatibility can we ensure this signature will always accept at least a string; accepting string | Uint8Array in the future is still fine, as long as string is involved. :))

[ricmoo]

Oh!! One more ask.

Can the loadKzg also include a .version string? So when other libraries are trying to re-shape the API, it knows how? :)

[ricmoo]

If curious on the changes I've made to support this, they are here.

[acolytec3]

@ricmoo well, I guess I've discovered somebody who actually uses this code besides us. Sorry for breaking ethers 🤦

This change was driven mainly by the fact we are migrating our default kzg library to latest release of micro-eth-signer which now includes support for kzg cryptography. In its API, the inputs and outputs are all hex strings and we want to have a single interface for kzg "stuff" instead of having to implement multiple wrappers around each library. The reason to use strings is that the lower level math in the library is done using bigints. Since the ethereumjs codebase doesn't do anything with any of the kzg inputs/outputs other than pass them around as opaque bytes and they have to be converted to hex strings when being sent over JSON-RPC (either in Engine API to a consensus client or in responses in our own client's RPC), it made sense for us to go ahead and make that change now. Not to mention, the wasm wrapper code I built for kzg-wasm actually converts everything to strings under the hood anyway.

Anyway, all that to say, I'm so sorry for breaking ethers. I don't anticipate any more changes in the near future (or ever) to this library since I have little anticipation of it gaining wider adoption since we now have that native JS library (that is actually faster on some fronts - notably the blobtoKZGCommitment function where it's 3x faster than WASM.

In the version string, are you just wanting the v0.5.0 or whatever we report in our package.json?

[ricmoo]

Ha ha, no worries. :)

Totally fair, and I found a point within Ethers where I have the opportunity to coerce the strings to/from Uint8Array, so that is no longer an issue. The Transaction object, wraps the functions if they are the new-age string-base signatures.

Do you have a link to that other library? The non-WASM one? I'd love to make sure Ethers accepts that one as well, and then I will use that in the docs on how to use Ethers to send blobs. :)

Yeah, I was just thinking the version string would be "v0.5.0", but it should only be necessary if there are further API changes, which is sounds like there won't be.

Thanks! For your time and explanation. Oh, ant the library itself. :)

[acolytec3]

Here's the link to the other library. The KZG docs are towards the bottom. https://github.com/paulmillr/micro-eth-signer

[acolytec3]

Okay, sounds good. For now, I'm just going to leave as is so as to not rock any more boats. We aren't using this library internally anymore so will likely deprecate it at some point since it's not remarkably difference performance-wise from the JS implementation (i.e. it's faster on generating and verifying proofs but slower at generating commitments).

[HIT2022]

S/O for supporting this @acolytec3 & Co. We use it, but I wasn't aware of micro-eth-signer, and will probably switch if the speed increase is really that notable.

[acolytec3]

For reference, here's what vitest benchmarking reported for the main functions in the two kzg libraries (blobToKzgCommitment, generateBlobProof, and verifyBlobProof). Here's the actual benchmark script if you want to reproduce.