docs: add documentation for disperser v2 grpc api and related functions/structs #1104
Conversation
| // A KZG commitment | ||
| // G1Commitment represents the serialized coordinates of a G1 KZG commitment. | ||
| // We use gnark-crypto so adopt its serialization, which is big-endian. See: | ||
| // https://github.com/Consensys/gnark-crypto/blob/779e884dabb38b92e677f4891286637a3d2e5734/ecc/bn254/fp/element.go#L862 |
There was a problem hiding this comment.
Do they have a spec to link to so we don't resort to code?
| // A KZG commitment | ||
| // G1Commitment represents the serialized coordinates of a G1 KZG commitment. | ||
| // We use gnark-crypto so adopt its serialization, which is big-endian. See: | ||
| // https://github.com/Consensys/gnark-crypto/blob/779e884dabb38b92e677f4891286637a3d2e5734/ecc/bn254/fp/element.go#L862 | ||
| message G1Commitment { | ||
| // The X coordinate of the KZG commitment. This is the raw byte representation of the field element. | ||
| bytes x = 1; |
There was a problem hiding this comment.
Would be good to specify the num of bytes here
| // KZG commitment, degree proof, the actual degree, and data length in number of symbols (field elements). | ||
| // It deserializes into https://github.com/Layr-Labs/eigenda/blob/ce89dab18d2f8f55004002e17dd3a18529277845/encoding/data.go#L27 | ||
| // | ||
| // See https://github.com/Layr-Labs/eigenda/blob/master/docs/spec/attestation/encoding.md#validation-via-kzg |
There was a problem hiding this comment.
You are mixing use of "master" and a fixed version of branch in these github links. Shall we use it consistently?
IMO, referencing to master might make it difficult to maintain (e.g. that doc may even be removed); a fixed version has its downside of getting stale, but will at least a usable link.
| bytes commitment = 1; | ||
| // A commitment to the blob data with G2 SRS, used to work with length_proof | ||
| // such that the claimed length below is verifiable. | ||
| // such that the claimed length below is verifiable. |
| bytes length_commitment = 2; | ||
| // A proof that the degree of the polynomial used to generate the blob commitment is valid. | ||
| // It is computed such that the coefficient of the polynomial is committing with the G2 SRS | ||
| // at the end of the highest order. | ||
| // It is computed such that the coefficient of the polynomial is committing with the G2 SRS |
There was a problem hiding this comment.
Can we just say like it's a commit to x^(SRSOrder-n) * P(x), where P(x) is polynomial of degree n representing the blob? This is not only clear, but concise.
| bytes length_proof = 3; | ||
| // The length specifies the degree of the polynomial used to generate the blob commitment. The length | ||
| // must equal to the degree + 1, and it must be a power of 2. | ||
| // The length of the blob in symbols (field elements), which must be a power of 2. |
There was a problem hiding this comment.
This is the one that I still have question about. I think it's just a upper bound of length/degree, rather than the length/degree of the poly.
| // | ||
| // The size of this byte array may be any size as long as it does not exceed the maximum length of 16MiB. | ||
| // (In the future, the 16MiB limit may be increased, but this is not guaranteed to happen.) | ||
| // Validation rules: |
There was a problem hiding this comment.
I think size must also be >0
| // ToByteArray converts a list of Fr to a byte array | ||
| // ToByteArray serializes a slice of fields elements to a slice of bytes. | ||
| // The byte array is created by serializing each Fr element in big-endian format. | ||
| // It is the reverse operation of ToFrArray. |
There was a problem hiding this comment.
Technically not quite the reverse, since ToFrArray adds padding, yet this function doesn't remove any padding
Doesn't necessarily need to be changed, though
Why are these changes needed?
The v2 interfaces are missing documentation, which makes it hard for intergration to interface with core. Adding some ad-hoc documentation here for thing that I needed to lookup today, but would be useful to do a uniform pass on the API to make sure its documentation is complete at some point.
Just learned about #1082, so therre might be some redundancy here. Probably should merge that one first (it's documents every more broadly) and then I'll rebase mine on top (I have more precise documentation for much fewer things).
Checks