Add support for EIP-7594 #452

jtraglia · 2024-07-12T14:11:50Z

This is a big PR which merges the das branch into main.

Client teams: when you have time, please review the changes to the bindings you use.

When we merge this, please squash the commits.

Notable changes to the bindings

Add compute_cells_and_kzg_proofs, recover_cells_and_kzg_proofs, verify_cell_kzg_proof_batch bindings.
Extend the trusted setup with G1 points in monomial form; there are three groups of points now.
Change load_trusted_setup (bytes version) to take flat array of bytes rather than list of point bytes.
- This means the size params represent number of total bytes, not the number of points.
Add precompute parameter to functions which load the trusted setup.
- This is an integer between 0 and 15. It's used to speed up proof computations.
- Refer to this HackMD document to compare tradeoffs.
~~Make some of the previously public constants private.~~

Notable changes to the core implementation

Replace big stack allocations with heap allocations in load_trusted_setup_file.
Modify g1_lincomb_fast to work with points at infinity.
- Relatively simple change, but EIP-4844 functions use it.
Replace uses of CHECK macro with normal conditions.
- There was a risk of misusing this and returning without freeing allocations.

Other notable changes

~~Replace old C LLVMFuzzer with new cargo-fuzz implementation in Rust.~~
- ~~This will also differentially fuzz with Constantine & PeerDAS-KZG where possible.~~

I have included Go bindings for this new method. I will add Rust bindings later. This will break the Rust bindings build until then, sorry about that.

The sample structure now includes the data, proof, and indices. This should make things easier to read and understand. Bindings will be broken for a little while. Going to remove the 2d functions, as I don't think they are that useful at the moment.

bindings/csharp/Ckzg.Bindings/Ckzg.cs

flcl42 · 2024-07-12T19:17:56Z

bindings/csharp/Ckzg.Test/ReferenceTests.cs

+
+        foreach (string testFile in testFiles)
+        {
+            string yaml = File.ReadAllText(testFile);


In general cases could be separated to a source passed to TestCaseSource attribute

Good idea. I only did this for the new ones. Let's follow-up with a PR for the old tests later.

flcl42 · 2024-07-12T19:18:48Z

bindings/csharp/Ckzg.Test/ReferenceTests.cs

+            {
+                Ckzg.ComputeCellsAndKzgProofs(cells, proofs, blob, _ts);
+                Assert.That(test.Output, Is.Not.EqualTo(null));
+                byte[] expectedCells = GetFlatBytes(test.Output.ElementAt(0));


Can the type of it be an array of arrays instead? Could you access elements just by [index]?

Hmm I'm not sure about this one. Let's look into this afterwards.

StefanBratanov · 2024-07-15T11:22:15Z

bindings/java/src/main/java/ethereum/ckzg4844/CKZG4844JNI.java

@@ -43,24 +43,36 @@ public static void loadNativeLibrary() {
    }
  }

-  /** Scalar field modulus of BLS12-381 */
-  public static final BigInteger BLS_MODULUS =


Why did we decide to make the constants private. They could be useful to projects using the library. We use some of them in Teku.

Hmm, protecting those constants was a conscious decision. It was an attempt to remove cryptographic details from the clients. In some cases cryptography code can be easy to mess up, and client devs might not be aware of the various cryptographic intricacies, so this change was done so that cryptography logic gets concentrated in the libraries.

How do you use these constants in Teku?

We have a utility class which flattens a list of commitments/proofs for example and we calculate the expected size based on these constants to initialize the array e.g. https://github.com/Consensys/teku/blob/master/infrastructure/kzg/src/main/java/tech/pegasys/teku/kzg/CKZG4844Utils.java

We also use in tests https://github.com/Consensys/teku/blob/master/infrastructure/kzg/src/test/java/tech/pegasys/teku/kzg/CKZG4844Test.java

I suppose not an issue to replicate those in Teku internally in the module, but would lead to duplication.

Stefan, we've decided to revert this change. So these constants are public again for all bindings. With the exception of the BYTES_PER_G* constants because load_trusted_setup takes a flat array of bytes now.

I believe this affects how clients see the return types. It appears as a slice instead of an array of a known size. This isn't right.

This reverts commit e689135.

bindings/nim/kzg.nim

jtraglia and others added 30 commits November 14, 2023 13:24

Add initial implementation of DAS functions

d2a97c1

Add rust bindings for DAS functions

dba45a0

Refactor names for types/consts

0d1875a

For go bindings, use heap and pointers

a8e4785

Add verify_sample_proof_batch function

224ecfe

I have included Go bindings for this new method. I will add Rust bindings later. This will break the Rust bindings build until then, sorry about that.

Fix a few nits in new function

7eb54ce

Add more comments

7604da9

Add new batch func to Rust bindings

4a20215

Add multi blob test for batch func

9c1d3f2

Start to add java bindings

4cd978e

Add batch verification method to java bindings

0acf0d9

Use more complex sample structure

eeb8afc

The sample structure now includes the data, proof, and indices. This should make things easier to read and understand. Bindings will be broken for a little while. Going to remove the 2d functions, as I don't think they are that useful at the moment.

Update go bindings

455e106

Clean up go bindings a little

7e0ef29

Delete 2d functions

5a085e8

Update java bindings

d974692

Use appropriate style in java bindings

c841e0b

Remove references to BLOB_COUNT

c6dba3b

Add samplesToBlob and recoverSamples to java bindings

0ee25f5

Improve samples_to_blob

fbf580b

Replace calloc with JNI allocation

f40f937

Rename two functions

d7a932c

Rename SAMPLE_SIZE to FIELD_ELEMENTS_PER_SAMPLE

579ebb9

Update rust bindings

488d4e0

Fix comment

e262a66

Use fast g1 lincomb implementation

3484da2

Rename sample to cell

a758b25

Fix comment indentations for cell params

802ee97

Add pub methods for cell and export das items (#386)

68ee8e2

Add benchmark for verifying columns

e9c6801

jtraglia added 2 commits July 12, 2024 12:43

Use ulong instead of int

ecc33f7

Remove debug prints

455bb22

jtraglia requested a review from a team July 12, 2024 18:25

flcl42 reviewed Jul 12, 2024

View reviewed changes

bindings/csharp/Ckzg.Bindings/Ckzg.cs Outdated Show resolved Hide resolved

flcl42 reviewed Jul 12, 2024

View reviewed changes

jtraglia added 5 commits July 12, 2024 14:58

Use TestCaseSource for new C# tests

1296b34

Improve function docs for C and C#

57db1db

Clarify doc comment

50fd406

Replace cellCount with len(cells)

38e3a00

Remove unused include in Java bindings

e781b8b

StefanBratanov reviewed Jul 15, 2024

View reviewed changes

jtraglia added 4 commits July 15, 2024 11:48

Add C test for verify_cell_kzg_proof_batch

742f6a8

Clean up C tests some more

7af67a4

Make CellsPerExtBlob in Go bindings public

e689135

I believe this affects how clients see the return types. It appears as a slice instead of an array of a known size. This isn't right.

Revert "Make CellsPerExtBlob in Go bindings public"

0e753e2

This reverts commit e689135.

jimmygchen mentioned this pull request Jul 17, 2024

chore: update the trusted setup to match the consensus-specs version sigp/lighthouse#6117

Merged

jtraglia added 4 commits July 17, 2024 14:51

Merge remote-tracking branch 'origin/main' into das

ce7c857

Revert fuzz changes

4694410

Revert changes to Rust bindings for new fuzzer

e7da294

Revert test_files.iter() to test_files

07923d0

jangko reviewed Jul 18, 2024

View reviewed changes

bindings/nim/kzg.nim Outdated Show resolved Hide resolved

bindings/nim/kzg.nim Outdated Show resolved Hide resolved

bindings/nim/kzg.nim Outdated Show resolved Hide resolved

bindings/nim/kzg.nim Outdated Show resolved Hide resolved

asn-d6 and others added 5 commits July 18, 2024 08:02

g1_lincomb_fast(): Simplify point at infinity weed out (#454)

b805483

Fix formatting

08fba8b

Apply review feedback to Nim's load_trusted_setup

7a99c25

Make some constants public again

0c96ac1

Merge branch 'main' into das

9990d88

asn-d6 merged commit 3714021 into main Jul 18, 2024
38 checks passed

agnxsh mentioned this pull request Jul 29, 2024

upstream c-kzg4844 (csources) to 3644ae9959f1d4c6c710db5750b85950f8b556ae status-im/nim-kzg4844#29

Closed

jtraglia mentioned this pull request Aug 15, 2024

Use TestCaseSource in C# reference tests #486

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for EIP-7594 #452

Add support for EIP-7594 #452

jtraglia commented Jul 12, 2024 •

edited

Loading

flcl42 Jul 12, 2024

jtraglia Jul 12, 2024

flcl42 Jul 12, 2024

jtraglia Jul 12, 2024

StefanBratanov Jul 15, 2024 •

edited

Loading

asn-d6 Jul 15, 2024

StefanBratanov Jul 18, 2024 •

edited

Loading

jtraglia Jul 18, 2024

Add support for EIP-7594 #452

Add support for EIP-7594 #452

Conversation

jtraglia commented Jul 12, 2024 • edited Loading

Notable changes to the bindings

Notable changes to the core implementation

Other notable changes

flcl42 Jul 12, 2024

Choose a reason for hiding this comment

jtraglia Jul 12, 2024

Choose a reason for hiding this comment

flcl42 Jul 12, 2024

Choose a reason for hiding this comment

jtraglia Jul 12, 2024

Choose a reason for hiding this comment

StefanBratanov Jul 15, 2024 • edited Loading

Choose a reason for hiding this comment

asn-d6 Jul 15, 2024

Choose a reason for hiding this comment

StefanBratanov Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

jtraglia Jul 18, 2024

Choose a reason for hiding this comment

jtraglia commented Jul 12, 2024 •

edited

Loading

StefanBratanov Jul 15, 2024 •

edited

Loading

StefanBratanov Jul 18, 2024 •

edited

Loading