Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Discussion): canonical api draft #4041

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions api/V1/blob_service.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
syntax = "proto3";

package celestia.node.v1.blob;

option go_package = "github.com/celestiaorg/celestia-node/api/v1/blob";

import "common.proto";
import "types.proto";
import "proof.proto";

message SubmitOptions {
double gas_price = 1; // Amount to be paid per gas unit
uint64 gas = 2; // Gas amount
string key_name = 3; // Keystore key for signing
Comment on lines +12 to +14
Copy link
Member

Choose a reason for hiding this comment

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

I'd make them optional too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't all options optional?

optional string fee_granter = 4; // Account paying for transaction
optional uint64 nonce = 5; // Transaction nonce
}

message GetBlobRequest {
uint64 height = 1;
bytes namespace = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

i've been thinking, it might be nice to make namespace a distinct type, where the API could then support some nicer features, like submitting in raw text, decode/encode, etc etc. At present, it feels very literal and as designed by someone who knows the internals, vs a developer using the API

bytes commitment = 3;
optional bool provide_commitment_proof = 4;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this field. The initial idea was to divide an inclusion proof and the commitment proof(blob proof). The difference between them the blob proof additionally contains subtree roots to build the commitment(link. After discussions with @walldiss we agreed that it makes sense to always return the full(blob) proof.

optional celestia.node.v1.common.Options options = 5;
}

message GetBlobResponseV0 {
celestia.node.v1.common.Status status = 1;
celestia.node.v1.types.BlobWithProofV0 blob = 2;
}

message GetBlobResponseV1 {
celestia.node.v1.common.Status status = 1;
celestia.node.v1.types.BlobWithProofV1 blob = 2;
}

message GetBlobsRequest {
uint64 height = 1;
bytes namespaces = 2;
optional celestia.node.v1.common.Options options = 3;
}

message GetBlobsResponseV0 {
celestia.node.v1.common.Status status = 1;
repeated celestia.node.v1.types.BlobWithProofV0 blobs = 2;
}

message GetBlobsResponseV1 {
celestia.node.v1.common.Status status = 1;
repeated celestia.node.v1.types.BlobWithProofV1 blobs = 2;
}

message SubmitDataRequest {
Copy link
Member

Choose a reason for hiding this comment

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

What about submitting data under multiple namespaces?

I'd suggest to add one more method:

message SubmitBlobsRequest {
  repeated Blobs blobs = 1; // blobs from different namespaces can be set
 SubmitOptions options = 2;
}

bytes namespace = 1;
repeated bytes data = 2;
SubmitOptions options = 3;
}

message SubmitDataResponse {
celestia.node.v1.common.Status status = 1;
uint64 height = 2;
repeated bytes commitments = 3;
}
Comment on lines +59 to +63
Copy link
Member

Choose a reason for hiding this comment

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

Let's include the tx hash here. iirc, some teams are using SubmitPFB in the state module only bc it additionally provides a tx hash.


message SubscribeAllRequest {
bytes namespaces = 1;
uint64 from_height = 2;
Copy link
Member

Choose a reason for hiding this comment

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

Let's clarify this: from_height should not be used to get the historical data(we have get methods for these purposes). Subscriptions should be used for the recent data only.

optional celestia.node.v1.common.Options options = 3;
}

message SubscribeAllResponse {
celestia.node.v1.common.Status status = 1;
uint64 height = 2;
repeated celestia.node.v1.types.BlobWithProofV1 blobs = 3;
}

message BlobsSubscribeRequest {
bytes namespaces = 1;
uint64 from_height = 2;
repeated bytes commitment = 3;
optional celestia.node.v1.common.Options options = 4;
}
Comment on lines +77 to +82
Copy link
Member

Choose a reason for hiding this comment

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

Such a method could be useful in case we support async blob submission. but iirc, it's not feasible rn as error handling in this case might be tricky.


message BlobsSubscribeResponse {
bytes commitment = 1;
celestia.node.v1.types.BlobWithProofV1 blob = 2;
uint64 height = 3;
}

// BlobService - handles blob operations
service BlobService {
// Core operations
rpc GetBlob(GetBlobRequest) returns (GetBlobResponseV0);
Copy link
Member

Choose a reason for hiding this comment

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

what if the submitted blob is v1,v2...? Maybe it's better to have a generic BlobResponse and extend the blob's message with additional optional fields?

rpc GetBlobs(GetBlobsRequest) returns (GetBlobsResponseV0);
rpc SubmitData(SubmitDataRequest) returns (SubmitDataResponse);

// Subscription operations
rpc SubscribeBlobs(BlobsSubscribeRequest) returns (stream BlobsSubscribeResponse);
rpc SubscribeAll(SubscribeAllRequest) returns (stream SubscribeAllResponse);
Copy link
Contributor

Choose a reason for hiding this comment

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

i think it'd be really clean to have a single rpc in the service here

blobs.Subscribe(SubscriptionRequest)

That that would have make any config that differentiates all from a particular namespace or commitment like a filter, ie in plain english "i am subscribing to everything UNLESS i specify some options/filters in my subscription request configuration"

}
41 changes: 41 additions & 0 deletions api/V1/common.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
syntax = "proto3";

package celestia.node.v1.common;

option go_package = "github.com/celestiaorg/celestia-node/api/v1/common";

// Common status codes used across all services
enum StatusCode {
// Reserve 0 for unspecified
STATUS_UNSPECIFIED = 0;
// Success codes (1-99)
STATUS_OK = 1;
// Error codes (100+)
// ERROR_DATA_NOT_AVAILABLE is returned if data availability sampling gurantees was required for request
// and failed. Keep in mind that data availability sampling will be ensured for all blocks in
// sampling window till the requested height
ERROR_DATA_NOT_AVAILABLE = 100;
Comment on lines +13 to +17
Copy link
Member

Choose a reason for hiding this comment

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

Will the caller be able to differentiate between e.g. sampling timing out, and other cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this only returned by the SharesAvailable method?

// Fraud was detected in the chain before requested block
ERROR_FRAUD_DETECTED = 101;
Copy link
Contributor

Choose a reason for hiding this comment

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

What would cause this error? (i.e. more specifically)

ERROR_TIMEOUT = 102;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are all timeouts handed by gRPC itself i.e. contexts in the case of go. Do they need to be included here?

// invalid input was submit as part of request. More information is provided in Status message field.
ERROR_INVALID_REQUEST = 103;
// unexpected error happened during handling the request
ERROR_INTERNAL = 104;
// method is not implemented by the server
ERROR_NOT_IMPLEMENTED = 105;
}

// Common message types used across services
message Status {
StatusCode code = 1;
string message = 2;
}

message Options {
// if verify_available is enabled, node will additionally ensure
// data availability on top of fetching requiested data
bool verify_available = 1;
// when proofs_only is enabled, proofs will be returned without sending data.
bool proofs_only = 2;
}
46 changes: 46 additions & 0 deletions api/V1/fraud_service.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
syntax = "proto3";

package celestia.node.v1.fraud;

option go_package = "github.com/celestiaorg/celestia-node/api/v1/fraud";

// BadEncodingFraudProof represents a proof of fraudulent data encoding
message BadEncodingFraudProof {
uint64 height = 1;
// TODO: add concrete fields
}

// Request to get all Bad Encoding Fraud Proofs from a specific height
message GetAllBEFPRequest {
uint64 from_height = 1;
uint64 max_amount = 2; // For pagination
}

message GetAllBEFPResponse {
repeated BadEncodingFraudProof fraud_proofs = 1;
}

message SubscribeBEFPRequest {
uint64 from_height = 1;
}

message SubscribeBEFPResponse {
BadEncodingFraudProof fraud_proofs = 1;
}

message VerificationResponse {
bool valid = 1;
string error = 2;
}

// BadEncodingFraudProofService - handles operations related to Bad Encoding Fraud Proofs
service BadEncodingFraudProofService {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the plan around supporting BEFPs? I've heard various things like we're not supporting them at the moment but perhaps plan to.

// Get all fraud proofs from a specific height with pagination
rpc GetAll(GetAllBEFPRequest) returns (GetAllBEFPResponse);

// Subscribe to receive fraud proofs as they are discovered
rpc SubscribeBEFPs(SubscribeBEFPRequest) returns (stream SubscribeBEFPResponse);
Copy link
Contributor

Choose a reason for hiding this comment

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

again, cleaner naming here would be to NOT repeat BEFP everywhere, its already in the name of the service,

eg:

BadEncodingFraudProofService.Subscribe() doesn't need the stutter of BEFP again if the service's concern is befp already


// Verify a given fraud proof
rpc Verify(BadEncodingFraudProof) returns (VerificationResponse);
}
69 changes: 69 additions & 0 deletions api/V1/proof_service.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
syntax = "proto3";

package celestia.node.v1.proof;

option go_package = "github.com/celestiaorg/celestia-node/api/v1/proof";

import "common.proto";
import "types.proto";

message VerifyDataRequest {
bytes root = 1;
bytes namespace = 2;
Proof proof = 3;
optional bytes data = 4; // Not used for non-inclusion proofs
}

message VerifySharesRequestV0 {
bytes row_root = 1;
// Each row includes its proof to row root
repeated celestia.node.v1.types.SharesWithProof rows = 2;
}

message VerifySharesRequestV1 {
bytes root = 1;
repeated celestia.node.v1.types.Share shares = 2;
DataRootProof proof = 3;
}

message VerifyNamespaceRequestV0 {
repeated bytes row_roots = 1;
bytes namespace = 2;
repeated celestia.node.v1.types.SharesWithProof rows = 3;
}

message VerifyNamespaceRequestV1 {
bytes root = 1;
bytes namespace = 2;
repeated celestia.node.v1.types.Share shares = 3;
DataRootProof proof = 4;
}

message VerifyCommitmentRequest {
bytes root = 1;
bytes namespace = 2;
bytes commitment = 3;
DataRootProof proof = 4;
}

message VerifyProofResponse {
celestia.node.v1.common.Status status = 1;
bool valid = 2;
}

// ProofsService - verification service that supports both V0 and V1 approaches
service ProofsService {
// V0 verification of inclusion of shares. Requires row root to perform verification.
// Does not verifies namespace rules of provided shares and should be used only
// to verify inclusion of data spread over multiple namespaces
rpc VerifySharesV0(VerifySharesRequestV0) returns (VerifyProofResponse);
Copy link
Contributor

Choose a reason for hiding this comment

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

The Verify prefix may give the impression that this service is verifying the proofs but the way I understand it, this service just gets the proof for the client to use (perhaps they verify it)

// Performs what VerifySharesV0 does with additional namespace rules verification on top.
rpc VerifyNamespaceV0(VerifyNamespaceRequestV0) returns (VerifyProofResponse);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not plan to support entire namespace proofs in v1?


// V1 verification
// VerifyDataProof verifies raw blob data using data root proof
rpc VerifyDataProof(VerifyDataRequest) returns (VerifyProofResponse);
// VerifyCommitmentProof verifies inclusion of blob, by using commitment and commitment data root proof.
// Provided proof type should be PROOF_TYPE_COMMITMENT
rpc VerifyCommitmentProof(VerifyCommitmentRequest) returns (VerifyProofResponse);
Copy link
Contributor

Choose a reason for hiding this comment

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

Semantic nit: I know the usage of Commitment is widespread already but I find it both vague and misleading (to commit over data is to sign over it). To make it more understandable to the users I would separate them into the two data structs they are proving inclusion over:

  1. BlobInclusionProofs
  2. ShareInclusionProofs (I think the way you have this, this can also be a range)

}
Comment on lines +54 to +69
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer separate v0 and v1 proof services (especially if we plan on deprecating v0 in the future)

39 changes: 39 additions & 0 deletions api/V1/proofs.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
syntax = "proto3";

package celestia.node.v1.proof;

option go_package = "github.com/celestiaorg/celestia-node/api/v1/proof";

// Proof system supports two verification approaches:
// 1. Row roots (V0) - Uses row roots from Data Availability Header (DAH) for verification
// 2. Data root (V1) - Uses data root proofs directly, more efficient and doesn't require DAH
enum ProofType {
Copy link
Contributor

Choose a reason for hiding this comment

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

proofs.Type imo

PROOF_TYPE_UNSPECIFIED = 0;
// Data proofs
PROOF_TYPE_DATA_INCLUSION = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

DATA_INCLUSION

// Contains leaf hashes for the range where namespace would be
PROOF_TYPE_DATA_NON_INCLUSION = 2;
// Commitment proof is used for blobs only.
// It contains first share of the blob and its inclusion proof
PROOF_TYPE_COMMITMENT = 3;
// TBD: structure for commitment non-inclusion
PROOF_TYPE_COMMITMENT_NON_INCLUSION = 4;
Comment on lines +19 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used anywhere?

}

// Core proof structure used in both V0 (row roots) and V1 (data root) verification
message Proof {
int32 start = 1; // Start index in EDS
int32 end = 2; // End index in EDS
repeated bytes nodes = 3; // NMT proof nodes (48 bytes each: namespace bounds + hash)
bytes leaf_hash = 4; // Used for non-inclusion proofs (40 bytes: namespace + hash)
}

message DataRootProof {
ProofType type = 1;
Proof proof = 2;
}

message RowRootProof {
Copy link
Member

@vgonkivs vgonkivs Jan 17, 2025

Choose a reason for hiding this comment

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

As we discussed with @walldiss, he does not want to import data structures from different repos. So, let's copy past it from the tendermint

Proof proof = 1;
uint64 row_index = 2;
}
Loading
Loading