Added missing v2 protobuf documentation. #1082

cody-littley · 2025-01-08T15:53:24Z

Why are these changes needed?

Adds missing v2 protobuf documentation.

Signed-off-by: Cody Littley <[email protected]>

api/proto/common/common.proto

Signed-off-by: Cody Littley <[email protected]>

api/proto/common/common.proto

jianoaix · 2025-01-08T20:29:05Z

api/proto/disperser/v2/disperser_v2.proto

  // If any one of the 32 bytes elements is outside the range, the whole request is deemed as invalid, and rejected.
  bytes data = 1;
+  // The header contains metadata about the blob.


Do users need to specify all quorums (including required ones), or just custom quorums?

Talked to Ian. All quorums must be specified. Documentation updated as follows:

// quorum_numbers is the list of quorum numbers that the blob is part of. // All quorums must be specified (including required quorums). repeated uint32 quorum_numbers = 2;

@jianoaix please resolve if you are happy with this response.

@cody-littley can we add what are required quorums v.s. custom quorums, and the current required quorums (0/ETH, 1/EIGEN)?

Updated comment as follows:

// quorum_numbers is the list of quorum numbers that the blob is part of. // All quorums must be specified (including required quorums). // // The following quorums are currently required: // - 0: ETH // - 1: EIGEN repeated uint32 quorum_numbers = 2;

To be clear, v2 does not explicitly enforce the required quorums. It only enforces that price for using subset of the required quorums is the same as using all required quorums. So technically, users are free to choose whatever set of quorums as necessary (with custom verification method).

This sounds more complicated than before by giving more choice to users. I would prefer an enforcement and reject requests missing required quorums.

How does pricing work here? Are they charged per quorum? Where is the pricing defined?

As for payments, reservation allows custom quorum numbers and doesn't enforce required quorums, but on-demand payments is only allowed for on-demand quorums.

To Sam's question, there's no specific logic on pricing per quorum. For on-demand, pricing is currently defined across all required quorums in the contracts, which is a simple fixed price per symbol. For reservation (set by EigenDA governance only), we left pricing to the BD team and is not defined at all.

(Pricing for reservations is in the making for permissionless reservations. Curious to learn what you think makes sense)

Here's a ticket for adding a check at the API server: https://linear.app/eigenlabs/issue/EGDA-804/add-required-quorums-check-in-api-server

api/proto/disperser/v2/disperser_v2.proto

api/proto/retriever/v2/retriever.proto

api/proto/disperser/v2/disperser_v2.proto

jianoaix · 2025-01-08T20:45:17Z

api/proto/disperser/v2/disperser_v2.proto

 message BlobStatusReply {
  // The status of the blob.
  BlobStatus status = 1;
  // The signed batch
  SignedBatch signed_batch = 2;
+  // BlobVerificationInfo is the information needed to verify the inclusion of a blob in a batch.
  BlobVerificationInfo blob_verification_info = 3;


This probably be easier to understand if it's named as something like "BlobInclusionInfo"

@ian-shim are you ok with this rename? If "yes", let me know and I'll do the refactor.

I don't have a strong opinion on this. Summoning the naming council @samlaf @mooselumph @litt3

I agree that BlobInclusionInfo is much better. (in a previous chat about naming, we actually came up with the same potential improvement)

yeessssssss +10000 to this. @litt3 and I had this name change already proposed in austin's diagram.

yea it's more accurate. I had to have a few turns in mind to comprehend BlobVerficationInfo.

Please note the renaming may affect more than "eigenda" and "eigenda-proxy" repos, e.g. https://github.com/Layr-Labs/hokulea/blob/master/crates/eigenda/src/certificate.rs#L50

That's fine, that'll teach @bxue-l2 for not using prost to generate his proto structs :P

tracking task in this ticket: https://linear.app/eigenlabs/issue/EGDA-804/add-required-quorums-check-in-api-server

api/proto/disperser/v2/disperser_v2.proto

Signed-off-by: Cody Littley <[email protected]>

api/proto/common/common.proto

Signed-off-by: Cody Littley <[email protected]>

jianoaix

LGTM

We may iterate further from @samlaf PR

api/proto/common/common.proto

jianoaix · 2025-01-13T23:46:33Z

api/proto/node/v2/node_v2.proto

  rpc StoreChunks(StoreChunksRequest) returns (StoreChunksReply) {}
+  // NodeInfo fetches metadata about the node.
  rpc NodeInfo(NodeInfoRequest) returns (NodeInfoReply) {}


Side note: maybe we should rename this to GetNodeInfo (to use a verb which is more consistent naming)

Other side note: Why is NodeInfo present on both the Dispersal and Retrieval services? Is it currently possible for a node to run those services separately on different machines?

Renamed to GetNodeInfo() (both Retrieval and Dispersal services).

I'm unsure why we duplicate this. @ian-shim can you comment?

They can technically run those services separately on different machines. I duplicated them so they're consistent with v1 API.

~~task tracked in this ticket: https://linear.app/eigenlabs/issue/EGDA-804/add-required-quorums-check-in-api-server~~

@cody-littley think that's the wrong linear ticket

Couldn't find the original ticket (not sure what happened). Created a new one: https://linear.app/eigenlabs/issue/EGDA-809/getnodeinfo-rpc-appears-twice

jianoaix · 2025-01-14T00:07:57Z

api/proto/disperser/v2/disperser_v2.proto

 message DisperseBlobReply {
  // The status of the blob associated with the blob key.
  BlobStatus result = 1;
+  // The unique identifier for the blob. Unique even if blob data is the identical.


This is the first time the users see blob key. We may probably better document it eg:

how long is this key? (32 bytes, length always fixed)

how is it defined? (hash of BlobHeader)
etc

Agree. I'll suggest the same edit here I suggested for the blob_key in GetChunksRequest

Suggested change

// The unique identifier for the blob. Unique even if blob data is the identical.

// The unique 32 bytes identifier for the blob.

// The blob_key is the keccak hash of the rlp serialization of the BlobHeader, as computed here:

// https://github.com/Layr-Labs/eigenda/blob/0f14d1c90b86d29c30ff7e92cbadf2762c47f402/core/v2/serialization.go#L30

// The blob_key must thus be unique for every request, even if the same blob is being disperser.

// Meaning the blob_header must be different for each request.

What happens if we submit the exact same request to DisperseBlob? Does the Disperser return an error? Or does it just return the blob status of the blob identified by that request?

I've adopted your suggested replacement.

What happens if we submit the exact same request to DisperseBlob?

That's a good question. @jianoaix @jianoaix do you know the answer?

If the exact same request was made again after a successful initial request (not necessarily successful dispersal, but successful dispersal request), the second request will return AlreadyExists error

Oh snap that's a good point always trips me that the errors are not represented in the proto file itself... easy to forget about them. This is a good reminder that we should be documenting the grpc errors that the different endpoints can return, and on which conditions.

For eg here @cody-littley can you add a comment saying that if the same blob_header is submitted, a grpc AlreadyExists error is returned.

For reference the error is returned here.

Added the following comment:

// Note that attempting to disperse a blob with the same blob key as a previously dispersed blob may cause // the disperser to reject the blob (DisperseBlob() RPC will return an error). bytes blob_key = 2;

8facb9c

scheduled follow up discussion

api/proto/common/v2/common_v2.proto

api/proto/disperser/v2/disperser_v2.proto

samlaf · 2025-01-14T03:58:55Z

api/proto/disperser/v2/disperser_v2.proto

  // If any one of the 32 bytes elements is outside the range, the whole request is deemed as invalid, and rejected.
  bytes data = 1;
+  // The header contains metadata about the blob.


@jianoaix please resolve if you are happy with this response.

samlaf · 2025-01-14T04:01:41Z

api/proto/disperser/v2/disperser_v2.proto

 message DisperseBlobReply {
  // The status of the blob associated with the blob key.
  BlobStatus result = 1;
+  // The unique identifier for the blob. Unique even if blob data is the identical.


Agree. I'll suggest the same edit here I suggested for the blob_key in GetChunksRequest

samlaf · 2025-01-14T04:07:45Z

api/proto/disperser/v2/disperser_v2.proto

 message DisperseBlobReply {
  // The status of the blob associated with the blob key.
  BlobStatus result = 1;
+  // The unique identifier for the blob. Unique even if blob data is the identical.


Suggested change

// The unique identifier for the blob. Unique even if blob data is the identical.

// The unique 32 bytes identifier for the blob.

// The blob_key is the keccak hash of the rlp serialization of the BlobHeader, as computed here:

// https://github.com/Layr-Labs/eigenda/blob/0f14d1c90b86d29c30ff7e92cbadf2762c47f402/core/v2/serialization.go#L30

// The blob_key must thus be unique for every request, even if the same blob is being disperser.

// Meaning the blob_header must be different for each request.

samlaf · 2025-01-14T04:11:37Z

api/proto/disperser/v2/disperser_v2.proto

 message DisperseBlobReply {
  // The status of the blob associated with the blob key.
  BlobStatus result = 1;
+  // The unique identifier for the blob. Unique even if blob data is the identical.


What happens if we submit the exact same request to DisperseBlob? Does the Disperser return an error? Or does it just return the blob status of the blob identified by that request?

Signed-off-by: Cody Littley <[email protected]>

api/grpc/common/v2/common_v2.pb.go

litt3 · 2025-01-14T17:41:42Z

api/proto/disperser/v2/disperser_v2.proto

 message BlobStatusReply {
  // The status of the blob.
  BlobStatus status = 1;
  // The signed batch
  SignedBatch signed_batch = 2;
+  // BlobVerificationInfo is the information needed to verify the inclusion of a blob in a batch.
  BlobVerificationInfo blob_verification_info = 3;


I agree that BlobInclusionInfo is much better. (in a previous chat about naming, we actually came up with the same potential improvement)

api/grpc/common/v2/common_v2.pb.go

api/grpc/common/common.pb.go

api/grpc/disperser/v2/disperser_v2.pb.go

api/grpc/node/v2/node_v2_grpc.pb.go

api/proto/node/v2/node_v2.proto

api/proto/common/v2/common_v2.proto

jianoaix · 2025-01-14T21:01:49Z

api/proto/common/v2/common_v2.proto

@@ -3,21 +3,39 @@ package common.v2;
 import "common/common.proto";
 option go_package = "github.com/Layr-Labs/eigenda/api/grpc/common/v2";

-
+// BlobHeader contains the information needed to disperse a blob to the EigenDA network.


IMO this one-liner could be more about what does BlobHeader describe (definitive properties of a blob), rather than where it's used

Changed to the following:

// BlobHeader contains the information describing a blob and the way it is to be dispersed.

b5734f3

jianoaix · 2025-01-14T21:02:27Z

api/proto/common/v2/common_v2.proto

@@ -3,21 +3,39 @@ package common.v2;
 import "common/common.proto";
 option go_package = "github.com/Layr-Labs/eigenda/api/grpc/common/v2";

-
+// BlobHeader contains the information needed to disperse a blob to the EigenDA network.
+// It can be thought of as an "eigenDA tx", in that it plays a purpose similar to an eth_tx to disperse a 4844 blob.


"It" means BlobHeader? I think Blob (which is different than BlobHeader) is analogous to a txn

No the blob is the payload. the blobheader is the tx, specifying price, commitment, etc.

Now I don't know what is blob/blobheader/payload/txn... https://en.wikipedia.org/wiki/Semantic_satiation

I switched the wording as follows. Note that this paragraph moved into DisperseBlobRequest (as requested by a prior comment).

// This header can be thought of as an "eigenDA tx", in that it plays a purpose similar to an eth_tx to disperse a // 4844 blob. Note that a call to DisperseBlob requires the blob and the blobHeader, which is similar to how // dispersing a blob to ethereum requires sending a tx whose data contains the hash of the kzg commit of the blob, // which is dispersed separately.

api/proto/common/v2/common_v2.proto

Signed-off-by: Cody Littley <[email protected]>

samlaf · 2025-01-16T23:59:57Z

api/proto/common/v2/common_v2.proto

  // Blob version
  uint32 version = 1;


We need to describe what this version is, and that it's linked to

eigenda/contracts/src/interfaces/IEigenDAStructs.sol

Line 49 in c4567f9

struct VersionedBlobParams {

@0x0aa0 can you add some description explaining how this version is used wrt the onchain thresholds and params?

Blob versions are pushed onchain by EigenDA governance in an append only fashion and store the maximum number of operators, number of chunks, and coding rate for a blob. On blob verification, these values are checked against supplied or default security thresholds to validate the security assumptions of the blobs availability.

Thanks! I've added this paragraph to the field's documentation.

b5eee20

api/proto/common/common.proto

Signed-off-by: Cody Littley <[email protected]>

jianoaix · 2025-01-17T17:53:49Z

api/proto/common/v2/common_v2.proto

+// BlobCertificate contains a full description of a blob and how it is dispersed. Part of the certificate
+// is provided by the blob submitter (i.e. the blob header), and part is provided by the disperser (i.e. the relays).
+// Validator nodes eventually sign the blob certificate once they are in custody of the required chunks
+// (note that the signature is indirect; validators sign the hash of a Batch, which contains the blob certificate).
 message BlobCertificate {


Since we are re-evaluating the naming, I think we may consider this name as well. It may be something like TBSBlobCertificate, TBS stands for "To Be Signed".

It's a practice seen in https://www.rfc-editor.org/rfc/rfc5280#section-4.1.1
Where a actual certificate is like:

Certificate ::= SEQUENCE { tbsCertificate TBSCertificate, signatureAlgorithm AlgorithmIdentifier, signature BIT STRING }

I've created a ticket to revisit this. I'd prefer not to rename any additional things as a part of this PR. https://linear.app/eigenlabs/issue/EGDA-808/rename-blobcertificate

Signed-off-by: Cody Littley <[email protected]>

samlaf

I'm happy with the state of this PR. In favor of merging and then having follow-up PRs. Big thanks @cody-littley !

Added missing v2 protobuf documentation.

8facb9c

Signed-off-by: Cody Littley <[email protected]>

cody-littley self-assigned this Jan 8, 2025

cody-littley added 5 commits January 8, 2025 09:56

Mark docs as generated code.

ea0efe6

Signed-off-by: Cody Littley <[email protected]>

Tweak attributes

ef6577b

Signed-off-by: Cody Littley <[email protected]>

Tweak attributes

ecf88b2

Signed-off-by: Cody Littley <[email protected]>

Moar ignored file types.

acce07a

Don't hide bindings until we check them in CI.

da50930

Signed-off-by: Cody Littley <[email protected]>

cody-littley commented Jan 8, 2025

View reviewed changes

api/proto/common/common.proto Outdated Show resolved Hide resolved

cody-littley added 2 commits January 8, 2025 14:02

Fix todo.

87ea921

Signed-off-by: Cody Littley <[email protected]>

Cleanup.

a9eb536

Signed-off-by: Cody Littley <[email protected]>

cody-littley requested review from ian-shim, mooselumph and jianoaix January 8, 2025 20:04

jianoaix reviewed Jan 8, 2025

View reviewed changes

api/proto/common/common.proto Outdated Show resolved Hide resolved

jianoaix reviewed Jan 8, 2025

View reviewed changes

Made some suggested changes.

d651a87

Signed-off-by: Cody Littley <[email protected]>

jianoaix reviewed Jan 9, 2025

View reviewed changes

api/proto/common/common.proto Show resolved Hide resolved

cody-littley added 4 commits January 10, 2025 08:38

Merge branch 'master' into v2-proto-docs

5bb47bf

Signed-off-by: Cody Littley <[email protected]>

Doc improvements

ec4aa5b

Signed-off-by: Cody Littley <[email protected]>

Merge branch 'master' into v2-proto-docs

ca1e3de

Made suggested changes.

005ce47

cody-littley marked this pull request as ready for review January 10, 2025 20:15

cody-littley added 3 commits January 10, 2025 14:17

Make suggested changes.

ba5b14d

Signed-off-by: Cody Littley <[email protected]>

Cleanup.

5a6726e

Signed-off-by: Cody Littley <[email protected]>

Recompile docs.

08cccc7

Signed-off-by: Cody Littley <[email protected]>

samlaf mentioned this pull request Jan 13, 2025

docs: add documentation for disperser v2 grpc api and related functions/structs #1104

Open

5 tasks

jianoaix approved these changes Jan 13, 2025

View reviewed changes

api/proto/common/common.proto Outdated Show resolved Hide resolved

jianoaix reviewed Jan 13, 2025

View reviewed changes

jianoaix reviewed Jan 14, 2025

View reviewed changes

samlaf reviewed Jan 14, 2025

View reviewed changes

cody-littley added 3 commits January 14, 2025 08:45

Made suggested changes.

46c7d25

Signed-off-by: Cody Littley <[email protected]>

Made suggested changes.

fdc398d

Signed-off-by: Cody Littley <[email protected]>

Made suggested changes.

89b4838

Signed-off-by: Cody Littley <[email protected]>

litt3 reviewed Jan 14, 2025

View reviewed changes

jianoaix reviewed Jan 14, 2025

View reviewed changes

samlaf reviewed Jan 15, 2025

View reviewed changes

api/proto/common/v2/common_v2.proto Outdated Show resolved Hide resolved

cody-littley added 2 commits January 15, 2025 11:59

Made suggested changes.

4558e55

Signed-off-by: Cody Littley <[email protected]>

Made suggested changes.

0366e2c

Signed-off-by: Cody Littley <[email protected]>

samlaf reviewed Jan 16, 2025

View reviewed changes

samlaf reviewed Jan 17, 2025

View reviewed changes

api/proto/common/common.proto Outdated Show resolved Hide resolved

litt3 approved these changes Jan 17, 2025

View reviewed changes

Made suggested changes.

b5734f3

Signed-off-by: Cody Littley <[email protected]>

jianoaix reviewed Jan 17, 2025

View reviewed changes

Make suggested change.

b5eee20

Signed-off-by: Cody Littley <[email protected]>

samlaf approved these changes Jan 17, 2025

View reviewed changes

cody-littley merged commit 9a4bdc0 into Layr-Labs:master Jan 17, 2025
8 checks passed

cody-littley deleted the v2-proto-docs branch January 17, 2025 20:23

-  // The unique identifier for the blob. Unique even if blob data is the identical.
+  // The unique 32 bytes identifier for the blob.
+  // The blob_key is the keccak hash of the rlp serialization of the BlobHeader, as computed here:
+  // https://github.com/Layr-Labs/eigenda/blob/0f14d1c90b86d29c30ff7e92cbadf2762c47f402/core/v2/serialization.go#L30
+  // The blob_key must thus be unique for every request, even if the same blob is being disperser.
+  // Meaning the blob_header must be different for each request.

Added missing v2 protobuf documentation. #1082

Added missing v2 protobuf documentation. #1082

Conversation

cody-littley commented Jan 8, 2025

Why are these changes needed?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cody-littley Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cody-littley Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

jianoaix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ian-shim Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

cody-littley Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cody-littley Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cody-littley Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

cody-littley Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cody-littley Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cody-littley Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samlaf left a comment

Choose a reason for hiding this comment

cody-littley Jan 15, 2025 •

edited

Loading

cody-littley Jan 15, 2025 •

edited

Loading

ian-shim Jan 14, 2025 •

edited

Loading

cody-littley Jan 15, 2025 •

edited

Loading

cody-littley Jan 14, 2025 •

edited

Loading

cody-littley Jan 15, 2025 •

edited

Loading

cody-littley Jan 15, 2025 •

edited

Loading

cody-littley Jan 17, 2025 •

edited

Loading

cody-littley Jan 17, 2025 •

edited

Loading