• Proofs/Commitments vs KZGProofs/Commitments
• recover_cells_and_proofs vs recover_cells_and_kzg_proofs

See Java code where we check the contextPtr to be 0, free it on the rust side and then set the contextPtr to zero.

On one hand, this is ideally done on the Rust side, then bindings code can avoid keeping a flag to check whether the context has been freed.

On the otherhand, some bindings need to keep track of this because even after the pointer is freed, they can attempt to call a method on the freed context. (We could check this on the Rust side too, ie return an error if the context is null on the rust side)

l (ell) was copied from the FK20 paper, but its not actually clear what it is. Replace it with num_points_per_coset or something along those lines

Check each benchmark suite and re-eveluate whether we need the, or not, and add new benchmarks for things that seem reasonable to performance test

Check if it has any significance on recovery and if not, use the serial version

This will track the main integration PR(s) and also the refactor PRs which may need to be merged before integration.

Lighthouse

Reviewer: @jimmygchen

Integration

• sigp/lighthouse#5941

Refactor

• sigp/lighthouse#5938
• sigp/lighthouse#5946

Teku

Reviewer: @zilm13

Integration

• Nashatyrev/teku#62

Refactor

None yet

Lodestar

Reviewer: Possibly @matthewkeil / @g11tech

Integration

• ChainSafe/lodestar#6923

Refactor

None yet

Prysm

Prysm is integrating go-eth-kzg, its easier to track all clients in one place, so it is included here

Reviewer: @nalepae

Integration

• prysmaticlabs/prysm#14136

Refactor

• prysmaticlabs/prysm#14159
• prysmaticlabs/prysm#14160

Nimbus

Reviewer: Still unclear; tagging relevant nim-kzg folks @tersec, @cheatfate, @jangko, @arnetheduck

Integration

None yet

Refactor

None yet

A lot of the benchmark code needs to create random polynomials, blobs, etc -- we can group all of this into a test_utils/bench_utils crate to avoid deduplication

Currently when we publish to maven, JReleaser will try 61 times to publish to maven.

After the first time, Maven central repository picks it up but it then takes about an hour to publish the release.

After the 61st try, the CI will think that it has failed but its just Maven taking a long time to publish the release.

We could increase the timeout to 30 minutes or an hour and or check if there is a config in JReleaser for this seemingly common scenario.

Internally, we have FK20Verifier and FK20Prover. Benedikt Wagner corrrectly noted that this is not enirely a good name once you think about it more.

George (asn) noted further that the verifier can verify proofs that are not created by the FK20 strategy. (They can just use the naive method)

Leaving this issue open, incase we think of a better name.

RIght now, I've exported them in stabilize FK20 branch as Prover, and Verifier. Even though they are internally being referenced as FK20Prover and FK20Verifier. I think FK20Prover likely makes more sense to stay though.

Not a necessity, but something to consider. We would lose the ability to cross compile from linux to windows

When running cargo test on windows, we seem to be getting a lot of heap pressure causing the CI to fail:

running 19 tests
test fk20::cosets::tests::bit_reverse_fuzz ... ok
test fk20::cosets::tests::check_valid_cosets ... ok
test fk20::cosets::tests::bit_reverse_cosets_equality ... ok
test fk20::cosets::tests::test_data_distribution_bit_reverse_order ... ok
test fk20::h_poly::tests::smoke_test_downsample ... ok
test fk20::naive::tests::check_divide_by_monomial_floor ... ok
test fk20::batch_toeplitz::tests::smoke_aggregated_matrix_vector_mul ... ok
test fk20::cosets::tests::test_data_distribution_original_cosets ... ok
test fk20::prover::tests::show_data_distribution_on_recover_evaluations_in_domain_order ... ok
memory allocation of 75497472 bytes failed
error: test failed, to rerun pass `-p crate_crypto_kzg_multi_open_fk20 --lib`

Caused by:
  process didn't exit successfully: `D:\a\rust-eth-kzg\rust-eth-kzg\target\x86_64-pc-windows-gnu\debug\deps\crate_crypto_kzg_multi_open_fk20-d4b6db1a58f80145.exe` (exit code: 0xc0000409, STATUS_STACK_BUFFER_OVERRUN)
note: test exited abnormally; to see the full output pass --nocapture to the harness.
Error: Process completed with exit code 1.

This was hot-fixed by making the tests run on a single thread. We could figure out what in the fk20 tests are causing the issue and possibly release memory quickly

Currently this is only needed to compute the commitment to the blob in lagrange form -- saving an IDFT

zigbuild generally seems to be a better linker for C

Consider making fk20 compute proofs and the reed solomon module computing the evaluations.

For PRs, it makes sense to run on stock windows-latest, mac-latest and ubuntu-latest.

For master, we can run the full release-suite with qemu for linux arm (This will be a lot slower which is fine)

CI code could be made a bit more concise by using the armv64 docker image for testing aarch64-linux-gnu. Something like:

      - name: Set up QEMU
        uses: docker/setup-qemu-action@v2

      - name: Run tests in ARM64 container
        uses: addnab/docker-run-action@v3
        with:
          image: arm64v8/node:16-alpine
          options: -v ${{ github.workspace }}:/app
          run: |
            cd /app
            yarn install
            yarn test

In particular "domain_order" does not really have a good meaning

Currently the domain allows only one coset, which gets pregenerated. We can add a better interface for cosets, allowing multiple cosets to be created from a single domain, or for a single Domain to be able to do multiple coset_ffts on different cosets

Currently the released packages have their corresponding language prefix in the package name, ie rust has rust-eth-kzg and node has node-eth-kzg -- consider removing these and simply naming them eth-kzg since the language part is somewhat implied.

Note: This requires changing all of the published bindings too

Tracking issue to pin the versions of cargo xwin and zigbuild being used in the CI.

This makes the CI more deterministic

Currently bls12_381::G1Projective::batch_normalize seems to be doing an inversion per projective point.

We can move this into the bls12-381 crate and use batch_inversion to speed it up

This would mean that we would need to check their lengths at runtime instead of at compile time.

The PR I made has been added into the latest release: https://github.com/Cysharp/csbindgen/releases/tag/1.9.2

If we are compatible with java 11, we should set that in the java plugin in build.gradle, ie:

java {
    sourceCompatibility = 11
    targetCompatibility = 11
}

Currently there are functions that reference functions in the specs to let the reader know what function this is supposed to match.

This is a tracking issue to check those comments and ensure that they are standardized for folks who want to grep the codebase

Currently, the c methods are called verify_cell_kzg_proof for example, whereas we want to rename them to rust_eth_kzg_verify_cell_kzg_proof since nim will put all c functions into same namespace.

This will cause issues with methods that have the same name in different c projects like c-kzg

release-please already creates a tag when releasing so we should ensure that JReleaser for Java does not do the same

Currently we benchmark the naive versions of some algorithms. It was used to confirm that the optimized versions indeed did give a noticeable speedup and by how much.

We probably don't need to do this and just benchmarking the optimized versions should be enough

Leaving this open incase its wanted.

Currently we release pre-release versions to node, we could just release those to a github branch, and possibly have this solution for all bindings, ie just have a branch with precompiled binaries etc, instead of each language having its own prerelease registry