Skip to content

Conversation

jianoaix
Copy link
Contributor

Why are these changes needed?

Make it clear / explicit about the chunk encoding format for clients of the Node API.

Checks

  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

Comment on lines +96 to +103
// [KZG proof: 32 bytes]
// [Coeff 1: 32 bytes]
// [Coeff 2: 32 bytes]
// ...
// [Coeff n: 32 bytes]
//
// The KZG proof is a point on G1 and is serialized with bn254.G1Affine.Bytes().
// The coefficients are field elements in bn254 and serialized with fr.Element.Marshal().
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing a few parts. How about this?

// [version]                  1 byte         currently, only version 1 is supported
// [chunkLength]              7 bytes        little endian
// Chunk 1:
//     [proof]                32 bytes
//     [coeff 1]              32 bytes
//     [coeff 2]              32 bytes
//     ...
//     [coeff chunkLength]    32 bytes      +-------------------------------------------------+
// Chunk 2:                                 | point on G1                                     |
//     [proof]                32 bytes <----| serialized via bn254.G1Affine.Bytes()           |
//     [coeff 1]              32 bytes      | github.com/consensys/gnark-crypto/ecc/bn254     |
//     [coeff 2]              32 bytes      +-------------------------------------------------+
//     ...
//     [coeff chunkLength]    32 bytes
// ...
// Chunk n:                                 +-------------------------------------------------+
//     [proof]                32 bytes      | field element in bn256                          |
//     [coeff 1]              32 bytes <--- | serialized via fr.Element.Marshal()             |
//     [coeff 2]              32 bytes      | github.com/consensys/gnark-crypto/ecc/bn254/fr  |
//     ...                                  +-------------------------------------------------+
//     [coeff chunkLength]    32 bytes
//
// The size of each individual chunk (proof+coeffs) is 32*(1 + chunkLength) bytes. If the total length of the GNARK
// encoded bytes is S, the number of chunks (n) is equal to (S-8) / (32*(1 + chunkLength)) bytes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, this documentation would not use golang code as part of its specification (i.e. we should look at things like fr.Element.Marshal() and write town the byte format in the doc). But this is at least better than not having any documentation at all. Later when we have more time, perhaps worth expanding the docs in this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think version etc are in chunk here, they are only in bundle / chunks in db, not when it's returned

This is the encoding for one chunk, there is no "chunk 1, 2...n" here though

The golang is the only one we have, if there are others, we can put them in the list here

@jianoaix jianoaix merged commit be1be6b into Layr-Labs:master Feb 20, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants