Added support for multiple KZG backends #88
Conversation
Yes, currently build fails as no backends are specified. But I believe this could be fixed by specifying |
Seems there was some issue with setting a default feature. Let us know your findings. |
ArtiomTr
force-pushed
the
Add-support-for-multiple-kzg-backends
branch
3 times, most recently
from
8d5d9e0 to
34a8354
Compare
|
Not exactly sure why it works like that, but default feature needs to be specified in If |
ArtiomTr
force-pushed
the
Add-support-for-multiple-kzg-backends
branch
from
34a8354 to
4a34617
Compare
| blob_to_kzg_commitment::<Mainnet>(&blob).expect("should compute blob to kzg commitment"); | ||
|
|
||
| assert_eq!(commitment, expected_commitment); | ||
| let backends = [ |
There was a problem hiding this comment.
issue: Trying to run tests with zkcrypto backend results in compile errors. Try running:
cargo test --release --lib -p kzg_utils --features zkcrypto
results in
error[E0433]: failed to resolve: use of undeclared crate or module `rust_kzg_blst`
--> kzg_utils/src/spec_tests/mod.rs:1:5
|
1 | use rust_kzg_blst::types::{fr::FsFr, g1::FsG1};
| ^^^^^^^^^^^^^ use of undeclared crate or module `rust_kzg_blst`
There was a problem hiding this comment.
Also, it would be great if build automatically ran tests for both backends
There was a problem hiding this comment.
I've refactored tests - there was some additional checks before running actual eip_4844 functions, e.g.:
grandine/kzg_utils/src/spec_tests/tests.rs
Lines 53 to 56 in c0678b0
I think these checks were needed previously, as rust-kzg was panicking, when invalid parameters given. But now rust-kzg returns errors, so these checks are no longer required. But if you think that we should keep them, I can bring them back
There was a problem hiding this comment.
Also, it would be great if build automatically ran tests for both backends
You mean when running cargo test all backends should be tested? Or adding --features zkcrypto to CI is enough?
ArtiomTr
force-pushed
the
Add-support-for-multiple-kzg-backends
branch
from
4a34617 to
3637fc0
Compare
|
I have a couple of comments:
|
|
@ArtiomTr - can we also add GPU optimised backends? |
In this PR:
blstorzkcryptobackends for kzg_utils package. Both can be included at the same time, and then selected at the runtime.fork_choice_store::StoreConfig-kzg_backend--kzg-backend.Closes #74.