Feat/eip4844 precompile

[ThomasPiellard]

Description

Implementation of the EIP4844 precompile.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How has this been tested?

See marshall_test.go, kzg_point_evaluation_test.go

How has this been benchmarked?

Unmarshall #constraints: 1772 when compiled on BN254, emulating BLS12-381 base field
UnmarshalCompressed #constraints: 585600 when compiled on BN254, emulating BLS12-381 base field
KzgPointEvaluation #constraints: 3772424 when compiled on BN254, emulating BLS12-381 base field

Checklist:

• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• I did not modify files generated from templates
• golangci-lint does not output errors locally
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[ivokub]

Bug: Variable Length Conversion Causes Panic

The code incorrectly assumes conversion.NativeToBytes returns at least 32 bytes for frontend.Variable inputs. However, frontend.Variable represents a native field element, which for many curves (e.g., those with native fields smaller than 256 bits) results in res being shorter than 32 bytes. This leads to an index out of bounds error when attempting to access res[16:] in the loops converting versionedHash, commitmentCompressed, and proofCompressed to byte arrays, causing a panic.

std/evmprecompiles/10-kzg_point_evaluation.go#L124-L149

gnark/std/evmprecompiles/10-kzg_point_evaluation.go

Lines 124 to 149 in 77daf89

	for i := range versionedHash {
	res, err := conversion.NativeToBytes(api, versionedHash[len(versionedHash)-1-i])
	if err != nil {
	return fmt.Errorf("convert versioned hash element %d to bytes: %w", i, err)
	}
	copy(versionedHashBytes[i*16:(i+1)*16], res[16:])
	}
	// commitment
	var comSerializedBytes [48]uints.U8
	for i := range commitmentCompressed {
	res, err := conversion.NativeToBytes(api, commitmentCompressed[len(commitmentCompressed)-1-i])
	if err != nil {
	return fmt.Errorf("convert commitment element %d to bytes: %w", i, err)
	}
	copy(comSerializedBytes[i*16:(i+1)*16], res[16:])
	}
	// proof
	var proofSerialisedBytes [48]uints.U8
	for i := range proofCompressed {
	res, err := conversion.NativeToBytes(api, proofCompressed[len(proofCompressed)-1-i])
	if err != nil {
	return fmt.Errorf("convert proof element %d to bytes: %w", i, err)
	}
	copy(proofSerialisedBytes[i*16:(i+1)*16], res[16:])
	}

Fix in Cursor • Fix in Web

Was this report helpful? Give feedback by reacting with 👍 or 👎

Thanks for the reply. Indeed it is true, but for now we know that it will be called on bls12-377. When we migrate to small fields in Linea, then we'll change it accordingly.

[ivokub]

LGTM, I made some changes.

Please wait for #1541 and #1542 to be merged before merging this PR.

[ivokub]

I updated against master. Now it is good to merge on my side. You can have a look at the changes I made:

• embedded the SRS verification key
• added tests against SRS to see eveyrthing works
• added tests for a lot of different unmarshal edge cases
• used new uints.Bytes and conversion package for converting between bytes and elements