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

Productionize KZG EIP-4844 #304

Merged
merged 12 commits into from
Dec 7, 2023
Merged

Productionize KZG EIP-4844 #304

merged 12 commits into from
Dec 7, 2023

Conversation

mratsim
Copy link
Owner

@mratsim mratsim commented Nov 23, 2023

This addresses #300

@sauliusgrigaitis
Copy link

@mratsim let me know if you have any questions regarding adding constantine to rust-kzg. The latest work now is in the integration branch. After recent refactorings in that branch it's not that hard to add a new ECC library. Let me know when you are planning to work on it. If you are overloaded I can also check with my students.

@mratsim
Copy link
Owner Author

mratsim commented Dec 6, 2023

3-way benchmarks vs c-kzg-4844 (cc @jtraglia) and go-kzg-4844 (cc @kevaundray)

image

Bench c-kzg-4844 (serial) go-kzg-4844 (serial) go-kzg-4844 (parallel) constantine (serial) constantine (parallel)
blob_to_kzg_commitment 37.773 ms - 5.823 ms 1.987 ms 0.532 ms
compute_kzg_proof 39.945 ms - 7.146 ms 23.984 ms 4.654 ms
compute_blob_kzg_proof 40.212 ms - 7.205 ms 24.092 ms 4.712 ms
verify_kzg_proof 0.915 ms 0.923 ms - 0.780 ms -
verify_blob_kzg_proof 1.531 ms - 1.390 ms 1.260 ms 1.101 ms
verify_blob_kzg_proof_batch 1 1.528 ms 1.392 ms 1.405 ms 1.287 ms 1.176 ms
verify_blob_kzg_proof_batch 2 2.589 ms 3.233 ms 1.591 ms 2.015 ms 1.490 ms
verify_blob_kzg_proof_batch 4 4.553 ms 4.671 ms 1.914 ms 3.413 ms 1.843 ms
verify_blob_kzg_proof_batch 8 8.446 ms 7.410 ms 2.738 ms 6.136 ms 3.134 ms
verify_blob_kzg_proof_batch 16 16.228 ms 12.734 ms 3.542 ms 11.354 ms 5.205 ms
verify_blob_kzg_proof_batch 32 32.016 ms 23.048 ms 7.215 ms 21.666 ms 9.790 ms
verify_blob_kzg_proof_batch 64 63.415 ms 43.224 ms 14.438 ms 42.903 ms 17.738 ms

@sauliusgrigaitis
Copy link

Impressive numbers! Howevere, blob_to_kzg_commitment looks very suspicious. Did you fuzz your implementation against go-kzg-4844 and c-kzg-4844

@mratsim
Copy link
Owner Author

mratsim commented Dec 6, 2023

Not too sure why blob_to_kzg_commitment is so fast, AFAIK I generate random values similar to the other benchmarks.

Anyway to give an upper bound, because blob_to_kzg_commitment is just a 4096-sized MSM, here is the pure MSM bench
image

26ms serial, 4.5 ms parallel

@mratsim
Copy link
Owner Author

mratsim commented Dec 6, 2023

Impressive numbers! Howevere, blob_to_kzg_commitment looks very suspicious. Did you fuzz your implementation against go-kzg-4844 and c-kzg-4844

Not fuzzed yet but I do pass the tests, I assume it's my random values that are weird because the code is really just forwarding to the MSM.

@jtraglia
Copy link

jtraglia commented Dec 6, 2023

Yes, very impressive if accurate! I agree with Saulius that the blob_to_kzg_commitment benchmark looks fishy. Are you sure it's being done serially? Can you artificially limit the CPU to only use one core? Also, what are the random values exactly? Is the blob composed of valid field elements?

Regarding the go-kzg-4844 benchmarks, even the serial functions still use multithreading so it's slightly misleading. It's due to how gnark-crypto is implemented. Here's a better comparison, ignoring the parallel benchmarks:

image image

@jtraglia
Copy link

jtraglia commented Dec 6, 2023

To my eyes, it appears that blobs are only initialized with a single field element. This would explain things.

proc new(T: type BenchSet, ctx: ptr EthereumKZGContext): T =
new(result)
for i in 0 ..< result.N:
let t {.noInit.} = rng.random_unsafe(Fr[BLS12_381])
result.blobs[i].marshal(t, bigEndian)
discard ctx.blob_to_kzg_commitment(result.commitments[i], result.blobs[i].addr)
discard ctx.compute_blob_kzg_proof(result.proofs[i], result.blobs[i].addr, result.commitments[i])

@mratsim
Copy link
Owner Author

mratsim commented Dec 6, 2023

To my eyes, it appears that blobs are only initialized with a single field element. This would explain things.

Good catch, this was it, new bench (didn't rebench c-kzg-4844 and go-kzg-4844)

image

Bench c-kzg-4844 (serial) go-kzg-4844 (serial) go-kzg-4844 (parallel) constantine (serial) constantine (parallel)
blob_to_kzg_commitment 37.773 ms - 5.823 ms 24.987 ms 4.465 ms
compute_kzg_proof 39.945 ms - 7.146 ms 24.332 ms 4.720 ms
compute_blob_kzg_proof 40.212 ms - 7.205 ms 24.291 ms 4.747 ms
verify_kzg_proof 0.915 ms 0.923 ms - 0.789 ms -
verify_blob_kzg_proof 1.531 ms - 1.390 ms 1.265 ms 1.115 ms
verify_blob_kzg_proof_batch 1 1.528 ms 1.392 ms 1.405 ms 1.337 ms 1.180 ms
verify_blob_kzg_proof_batch 2 2.589 ms 3.233 ms 1.591 ms 2.008 ms 1.466 ms
verify_blob_kzg_proof_batch 4 4.553 ms 4.671 ms 1.914 ms 3.492 ms 1.830 ms
verify_blob_kzg_proof_batch 8 8.446 ms 7.410 ms 2.738 ms 6.207 ms 3.137 ms
verify_blob_kzg_proof_batch 16 16.228 ms 12.734 ms 3.542 ms 11.483 ms 5.204 ms
verify_blob_kzg_proof_batch 32 32.016 ms 23.048 ms 7.215 ms 23.165 ms 9.590 ms
verify_blob_kzg_proof_batch 64 63.415 ms 43.224 ms 14.438 ms 45.152 ms 17.493 ms

@jtraglia
Copy link

jtraglia commented Dec 6, 2023

Nice, that's still really impressive. Good job!

@mratsim
Copy link
Owner Author

mratsim commented Dec 7, 2023

Turned out I wasn't parallelizing kzg batch verification but only preprocessing. See a1ced2d

There is a whopping 52% speedup on parallel batch verification.

image

Bench c-kzg-4844 (serial) go-kzg-4844 (serial) go-kzg-4844 (parallel) constantine (serial) constantine (parallel)
blob_to_kzg_commitment 37.773 ms - 5.823 ms 23.765 ms 4.425 ms
compute_kzg_proof 39.945 ms - 7.146 ms 24.255 ms 4.710 ms
compute_blob_kzg_proof 40.212 ms - 7.205 ms 24.288 ms 4.794 ms
verify_kzg_proof 0.915 ms 0.923 ms - 0.782 ms -
verify_blob_kzg_proof 1.531 ms - 1.390 ms 1.266 ms 1.113 ms
verify_blob_kzg_proof_batch 1 1.528 ms 1.392 ms 1.405 ms 1.286 ms 1.130 ms
verify_blob_kzg_proof_batch 2 2.589 ms 3.233 ms 1.591 ms 2.006 ms 1.152 ms
verify_blob_kzg_proof_batch 4 4.553 ms 4.671 ms 1.914 ms 3.437 ms 1.250 ms
verify_blob_kzg_proof_batch 8 8.446 ms 7.410 ms 2.738 ms 6.115 ms 1.891 ms
verify_blob_kzg_proof_batch 16 16.228 ms 12.734 ms 3.542 ms 11.567 ms 3.091 ms
verify_blob_kzg_proof_batch 32 32.016 ms 23.048 ms 7.215 ms 21.779 ms 6.764 ms
verify_blob_kzg_proof_batch 64 63.415 ms 43.224 ms 14.438 ms 43.099 ms 11.538 ms

@mratsim mratsim marked this pull request as ready for review December 7, 2023 07:26
@mratsim
Copy link
Owner Author

mratsim commented Dec 7, 2023

PR too long due to trusted setup updates and .yaml test vectors.

The parallel C API, Go and Rust bindings will be provided in a separate.

@mratsim mratsim merged commit 761c720 into master Dec 7, 2023
16 checks passed
@mratsim mratsim deleted the kzg-prod branch December 7, 2023 07:28
@mratsim mratsim mentioned this pull request Dec 26, 2023
12 tasks
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

Successfully merging this pull request may close these issues.

3 participants