-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Shrex client server specification #4665
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
base: shrex_spec
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## shrex_spec #4665 +/- ##
=============================================
Coverage ? 36.06%
=============================================
Files ? 304
Lines ? 20170
Branches ? 0
=============================================
Hits ? 7274
Misses ? 11944
Partials ? 952 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| - Retrieving data from local storage | ||
| - Sending responses back to clients | ||
|
|
||
| ### Request Handling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in certain cases (ND), not found comes with data too (non incl proof)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually false. Server sends "OK" along with non-inclusion proof
specs/src/shrex/client-server.md
Outdated
| - **NOT FOUND**: Requested data is not available | ||
| - **INTERNAL ERROR**: Server encountered an error | ||
|
|
||
| 2. **Data (if OK)**: The actual requested data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also include non-incl proof if ND not found
specs/src/shrex/client-server.md
Outdated
|
|
||
| - Connection failures | ||
| - Timeouts | ||
| - Invalid data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meaning invalid data sent back from server?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
relic
walldiss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs to add:
- transport protocol spec (libp2p streaming)
- message flow. Status message definition and proto. Error model in status message
- reference to shwap spec defining encoding and payload of requests and responses
- peer manager integration, reference to peer manager spec
I haven't added this spec yet. My idea was to add it when I will be merging all specs into a single.
Client-Server does not interact with peer manager, but Getter does. I will update spec with Additionally, I can add a list of available endpoints |
| ### Request Messages | ||
|
|
||
| Request messages are binary encoded. | ||
|
|
||
| The request is serialized to binary format and written directly to the stream. Each request type includes: | ||
|
|
||
| - Height: Block height for the requested data | ||
| - Type-specific parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requests use shwap id and associated encoding. We can be more specific here and link to the shwap spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reworked
| ### Response Messages | ||
|
|
||
| #### Status Message | ||
|
|
||
| Every server response begins with a status message: | ||
|
|
||
| ```protobuf | ||
| enum Status { | ||
| INVALID = 0; // Invalid/unknown status | ||
| OK = 1; // Data found and will be sent | ||
| NOT_FOUND = 2; // Requested data not found | ||
| INTERNAL = 3; // Internal server error | ||
| } | ||
|
|
||
| message Response { | ||
| Status status = 1; | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After handling requests server must send Status message indicating result of handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
specs/src/shrex/client-server.md
Outdated
| ## Supported Endpoints | ||
|
|
||
| SHREX supports multiple endpoint types for retrieving different kinds of data. All endpoints follow the same request-response pattern but return different data structures. | ||
|
|
||
| ### Common Types | ||
|
|
||
| ```protobuf | ||
| message Share { | ||
| bytes data = 1; | ||
| } | ||
|
|
||
| enum AxisType { | ||
| ROW = 0; | ||
| COL = 1; | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should list shwap IDs and associated shawp.Containers. Encoding is defined in shwap spec, so no need to make copy of it here. But make sure to make to link them here. Formatting as table might look better:
| protocol-id | request (shwap.id) | response payload (shwap.container)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
| - Retrieving data from local storage | ||
| - Sending responses back to clients | ||
|
|
||
| ### Request Handling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually false. Server sends "OK" along with non-inclusion proof
Rendered