Remove tunable threshold in g1_lincomb_fast

[jtraglia]

I think we should simplify this & always use Pippenger. This might be a little slower but having a single path is nice.

Also, removes the two remarks which are no longer relevant.

Before

go clean -cache && GOMAXPROCS=1 go test -bench=Benchmark
goos: darwin
goarch: arm64
pkg: github.com/ethereum/c-kzg-4844/v2/bindings/go
cpu: Apple M1
Benchmark/LoadTrustedSetupFile(precompute=0)                   1        1693199833 ns/op
Benchmark/LoadTrustedSetupFile(precompute=1)                   1        1709008041 ns/op
Benchmark/LoadTrustedSetupFile(precompute=2)                   1        1705220708 ns/op
Benchmark/LoadTrustedSetupFile(precompute=3)                   1        1715734417 ns/op
Benchmark/LoadTrustedSetupFile(precompute=4)                   1        1736569792 ns/op
Benchmark/LoadTrustedSetupFile(precompute=5)                   1        1767235541 ns/op
Benchmark/LoadTrustedSetupFile(precompute=6)                   1        1843047250 ns/op
Benchmark/LoadTrustedSetupFile(precompute=7)                   1        1983663583 ns/op
Benchmark/LoadTrustedSetupFile(precompute=8)                   1        2267864917 ns/op
Benchmark/BlobToKZGCommitment                                 28          41514667 ns/op
Benchmark/ComputeKZGProof                                     25          44049387 ns/op
Benchmark/ComputeBlobKZGProof                                 26          44093210 ns/op
Benchmark/VerifyKZGProof                                    1012           1174089 ns/op
Benchmark/VerifyBlobKZGProof                                 675           1756378 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=1)                   680           1754324 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=2)                   404           2962664 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=4)                   230           5166030 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=8)                   123           9603421 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=16)                   60          18727938 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=32)                   31          36382819 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=64)                   15          71920053 ns/op
Benchmark/ComputeCells                                       483           2462157 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=0)               4         264104677 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=1)               2         927725666 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=2)               3         501228472 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=3)               3         354618611 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=4)               4         284878386 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=5)               4         269898573 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=6)               5         211943975 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=7)               6         195196229 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=8)               6         186348722 ns/op
Benchmark/ComputeCellsAndKZGProofsParallel(count=8)            4         306334760 ns/op
Benchmark/RecoverCells(missing=50.0%)                         50          23444946 ns/op
Benchmark/RecoverCells(missing=25.0%)                         56          21227897 ns/op
Benchmark/RecoverCells(missing=12.5%)                         52          20988318 ns/op
Benchmark/RecoverCells(missing=1)                             56          20311724 ns/op
Benchmark/RecoverCells(missing=2)                             56          20337202 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=50.0%)              6         198702514 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=25.0%)              6         199000021 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=12.5%)              6         198837160 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=1)                  6         206478236 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=2)                  5         209806258 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=3)                  5         203379817 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=4)                  6         198578549 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=5)                  6         198164556 ns/op
Benchmark/VerifyCellKZGProofBatch                              2         642734625 ns/op
Benchmark/VerifyCellKZGProofBatchParallel                      7         152922089 ns/op
Benchmark/VerifyRows(count=1)                                 67          16363907 ns/op
Benchmark/VerifyRows(count=2)                                 40          27754874 ns/op
Benchmark/VerifyRows(count=4)                                 22          49136780 ns/op
Benchmark/VerifyRows(count=8)                                 12          91510226 ns/op
Benchmark/VerifyRows(count=16)                                 6         172457729 ns/op
Benchmark/VerifyRows(count=32)                                 4         330454594 ns/op
Benchmark/VerifyRows(count=64)                                 2         647002312 ns/op
Benchmark/VerifyColumns(count=1)                              75          14027568 ns/op
Benchmark/VerifyColumns(count=2)                              55          19877529 ns/op
Benchmark/VerifyColumns(count=4)                              37          31071466 ns/op
Benchmark/VerifyColumns(count=8)                              21          52392296 ns/op
Benchmark/VerifyColumns(count=16)                             12          93703622 ns/op
Benchmark/VerifyColumns(count=32)                              6         174161257 ns/op
Benchmark/VerifyColumns(count=64)                              4         332949021 ns/op
Benchmark/VerifyColumns(count=128)                             2         641776166 ns/op
PASS
ok      github.com/ethereum/c-kzg-4844/v2/bindings/go   158.880s

After

go clean -cache && GOMAXPROCS=1 go test -bench=Benchmark
goos: darwin
goarch: arm64
pkg: github.com/ethereum/c-kzg-4844/v2/bindings/go
cpu: Apple M1
Benchmark/LoadTrustedSetupFile(precompute=0)                   1        1700119458 ns/op
Benchmark/LoadTrustedSetupFile(precompute=1)                   1        1704077959 ns/op
Benchmark/LoadTrustedSetupFile(precompute=2)                   1        1704675167 ns/op
Benchmark/LoadTrustedSetupFile(precompute=3)                   1        1710133042 ns/op
Benchmark/LoadTrustedSetupFile(precompute=4)                   1        1733323500 ns/op
Benchmark/LoadTrustedSetupFile(precompute=5)                   1        1771712291 ns/op
Benchmark/LoadTrustedSetupFile(precompute=6)                   1        1839507584 ns/op
Benchmark/LoadTrustedSetupFile(precompute=7)                   1        1982198500 ns/op
Benchmark/LoadTrustedSetupFile(precompute=8)                   1        2270373875 ns/op
Benchmark/BlobToKZGCommitment                                 28          41597188 ns/op
Benchmark/ComputeKZGProof                                     26          43767647 ns/op
Benchmark/ComputeBlobKZGProof                                 26          43847963 ns/op
Benchmark/VerifyKZGProof                                    1015           1179913 ns/op
Benchmark/VerifyBlobKZGProof                                 680           1750964 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=1)                   660           1755099 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=2)                   404           2962814 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=4)                   231           5191379 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=8)                   121          10072490 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=16)                   62          18542815 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=32)                   32          36474408 ns/op
Benchmark/VerifyBlobKZGProofBatch(count=64)                   15          72282608 ns/op
Benchmark/ComputeCells                                       481           2456323 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=0)               4         268698844 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=1)               2         909398270 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=2)               3         485661778 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=3)               3         349665361 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=4)               4         278063042 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=5)               5         240018167 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=6)               5         216613592 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=7)               6         194131875 ns/op
Benchmark/ComputeCellsAndKZGProofs(precompute=8)               6         178952833 ns/op
Benchmark/ComputeCellsAndKZGProofsParallel(count=8)            4         282520083 ns/op
Benchmark/RecoverCells(missing=50.0%)                         58          19987313 ns/op
Benchmark/RecoverCells(missing=25.0%)                         58          20075807 ns/op
Benchmark/RecoverCells(missing=12.5%)                         58          20135303 ns/op
Benchmark/RecoverCells(missing=1)                             57          20235536 ns/op
Benchmark/RecoverCells(missing=2)                             56          20166790 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=50.0%)              6         197870826 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=25.0%)              6         198377264 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=12.5%)              6         198562153 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=1)                  6         205000028 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=2)                  6         198785403 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=3)                  6         199190347 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=4)                  6         197957882 ns/op
Benchmark/RecoverCellsAndKZGProofs(missing=5)                  6         197973556 ns/op
Benchmark/VerifyCellKZGProofBatch                              2         648307438 ns/op
Benchmark/VerifyCellKZGProofBatchParallel                      7         151296345 ns/op
Benchmark/VerifyRows(count=1)                                 70          16464570 ns/op
Benchmark/VerifyRows(count=2)                                 42          27613890 ns/op
Benchmark/VerifyRows(count=4)                                 24          49321425 ns/op
Benchmark/VerifyRows(count=8)                                 12          92656260 ns/op
Benchmark/VerifyRows(count=16)                                 6         171807347 ns/op
Benchmark/VerifyRows(count=32)                                 4         329526864 ns/op
Benchmark/VerifyRows(count=64)                                 2         640655146 ns/op
Benchmark/VerifyColumns(count=1)                              82          14095777 ns/op
Benchmark/VerifyColumns(count=2)                              57          19915917 ns/op
Benchmark/VerifyColumns(count=4)                              37          31018149 ns/op
Benchmark/VerifyColumns(count=8)                              21          52415782 ns/op
Benchmark/VerifyColumns(count=16)                             12          93236528 ns/op
Benchmark/VerifyColumns(count=32)                              6         174671250 ns/op
Benchmark/VerifyColumns(count=64)                              4         331865458 ns/op
Benchmark/VerifyColumns(count=128)                             2         642836938 ns/op
PASS
ok      github.com/ethereum/c-kzg-4844/v2/bindings/go   159.073s

[ethereum-code-reviewer]

Security Review

Confidence Score: 85%
Detected Security Issues: Yes

Summary

The code change removes a critical safety mechanism that prevented calling the blst_p1s_mult_pippenger() function with insufficient data. The original threshold check ensured that the function was only called with arrays of length 8 or more, falling back to a naive implementation for smaller arrays. This change exposes the system to potential failures when the blst library is called with arrays of length 0 or 1, which the library cannot handle properly. While three analyses failed due to connection issues, the successful Anthropic analysis identified this vulnerability with high confidence (85%). The removal of this safety check represents a regression in defensive programming practices and could lead to runtime failures in cryptographic operations.

Detailed Findings

MEDIUM Severity Issue

• Description: Removal of minimum length threshold check in g1_lincomb_fast() allows blst_p1s_mult_pippenger() to be called with len < 2, which can cause the blst library to fail or behave unexpectedly
• Recommendation: Restore the minimum length threshold check or add explicit validation to ensure len >= 2 before calling blst_p1s_mult_pippenger()
• Confidence: 85%

📑 Detailed Explanation

What is the issue?

The code change removes a critical safety check that prevented calling blst_p1s_mult_pippenger() with arrays of length less than 2. According to the original comment, 'blst fails for 0 or 1' elements. This removal exposes the function to potential failures when called with small arrays, which could lead to undefined behavior, crashes, or incorrect cryptographic computations.

What can happen?

If exploited, this could cause the cryptographic library to fail unexpectedly, potentially leading to denial of service, incorrect cryptographic operations, or system crashes. In a security-critical context, this could compromise the integrity of cryptographic protocols that depend on this function.

How to fix it?

Add back the minimum length validation before calling blst_p1s_mult_pippenger(). The threshold should be at least 2 as indicated in the original code. For cases where len < 2, either fall back to g1_lincomb_naive() or handle the edge cases explicitly with appropriate error handling.

Code example:

/* PROBLEMATIC: Removed safety check */
C_KZG_RET g1_lincomb_fast(g1_t *out, const g1_t *p, const fr_t *coeffs, size_t len) {
    blst_p1_affine *p_affine = NULL;
    blst_scalar *scalars = NULL;

    /* <!-- MISSING: Minimum length validation --> */
    /* <!-- SHOULD ADD: if (len < 2) { handle edge case or return error } --> */

    /* Allocate space for arrays */
    ret = c_kzg_calloc((void **)&p_affine, len, sizeof(blst_p1_affine));
    if (ret != C_KZG_OK) goto out;

Additional resources:

• https://github.com/supranational/blst - BLST library documentation and source code for understanding the constraints of blst_p1s_mult_pippenger()

[jtraglia]

• Description: Removal of minimum length threshold check in g1_lincomb_fast() allows blst_p1s_mult_pippenger() to be called with len < 2, which can cause the blst library to fail or behave unexpectedly

The fact that the tests pass show that this is no longer true. See:

https://github.com/supranational/blst/blob/f48500c1fdbefa7c0bf9800bccd65d28236799c1/src/multi_scalar.c#L426-L432

[b-wagn]

When we remove this remark, does it mean that we can no longer input the point at infinity?
If so, we should say that. If we still can, then why does removing the remark make sense?
Keeping the remark then would not hurt and would be more explicit.

[jtraglia]

Ah thanks for bringing this up. I removed this because I did not think it made sense to state anymore. No remark means no restrictions. For example, we do not state that g1_lincomb_naive can be used with points at infinity either.

[asn-d6]

LGTM! Thanks!