-
Notifications
You must be signed in to change notification settings - Fork 85
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
Define ping/pong payload extensions #348
base: master
Are you sure you want to change the base?
Changes from 1 commit
a603389
cedecda
3d76802
3a68292
56fa823
a0aa9ba
3ab5275
b01d69a
014dd45
c06e6ae
313c977
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# This document lists defined extensions | ||
Some extensions are network specific some are generic | ||
|
||
This is a list and short description of all the extensions | ||
|
||
|
||
| Type number | Name | Supported sub-networks | Short Description | Is this call Required to Implement | | ||
|---|---|---|---|---| | ||
| [1](extensions/type-1.md) | History Distance | History | Allows you to get the radius and other nodes metadata | Yes | | ||
| [2](extensions/type-2.md) | State Distance | State | Allows you to get the state nodes radius | Yes | | ||
| [3](extensions/type-3.md) | Beacon Distance | Beacon | Allows you to get the beacon nodes radius | Yes | | ||
| [4](extensions/type-4.md) | Portal Canonical Transaction Distance | Portal Canonical Transaction | Allows you to get the Portal Canonical Transaction nodes radius | Yes | | ||
| [5](extensions/type-5.md) | Transaction Gossip Distance | Transaction Gossip | Allows you to get the Transaction Gossip nodes radius | Yes | | ||
| [10](extensions/type-10.md) | Client Info | All | It will return you something like `trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0` | Yes | |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# [Title] | ||
|
||
[Description] | ||
|
||
## version [Version Number] | ||
|
||
[Version Description] | ||
|
||
Ping payload | ||
```python | ||
[Container Name] = Container( | ||
type: [Type Number], | ||
version: [Type Version], | ||
payload: Container([Payload Description]) | ||
) | ||
``` | ||
|
||
Pong payload | ||
```python | ||
[Container Name] = Container( | ||
type: [Type Number], | ||
version: [Type Version], | ||
payload: Container([Payload Description]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, so using BasicRadiusPayload = Container(data_radius: U256)
HistoryRadiusPayload = Container(data_radius: U256, ephemeral_header_count=U16)
ClientVersion = Container(version_string: ByteList[max_length=XXX])
StateNetworkDataPayload = Union[BasicRadiusPayload, ClientVersion]
HistoryNetworkCustomDataPayload = Union[HistoryRadiusPayload, Clientversion] What am I missing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am assuming this would remove the I am against unions because
I don't want to need to make an awkward upgrade if we run out of selectors. Also using a union would make it so parties couldn't make custom extensions which I think it important. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the only one of those reasons that I'm compelled by is that unions are akward to implement any kind of rolling upgrade with. Given that, I think I'd advocate for a single There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Said explicitely, it would be Container(type: u16, payload: ByteList[max_length=XXXX]) where clients implement application specific logic for decoding the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is also what I mentioned as 1 of the two options here: #348 (comment) And I agree that type + version is quite overkill, especially with the amount of bytes reserved for each. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sounds like the play and is aligned with what Kim said. I think a u32 would still be very nice. I might be being a little hyperbolic, but I wouldn't want to deal with the possibility of running out of types. So it will probably be fine with a u16, but if we did run out that would be a dreadful experience to deal with. But it is probably fine There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For errors I think a u16 is more then enough to be clear* |
||
) | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# Portal History Network Distance | ||
|
||
This payload is only supposed to be used for the history network | ||
|
||
## version 1 | ||
|
||
Ping and Pong Payload | ||
```python | ||
HistoryNetworkDistanceV1 = Container( | ||
type: 1, | ||
version: 1, | ||
payload: Container(data_radius: uint256) | ||
) | ||
``` | ||
|
||
## version 2 | ||
Ping and Pong Payload | ||
```python | ||
HistoryNetworkDistanceV2 = Container( | ||
type: 1, | ||
version: 2, | ||
payload: Container(data_radius: uint256, ephemeral_header_count: uint16) | ||
) | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
# Get Client info | ||
|
||
This payload is only supposed to be usable from all Networks. This payload allows you to figure out peer's client info. Which will be useful for censuses. | ||
|
||
## version 1 | ||
|
||
### Type Specifications | ||
|
||
Client info are ASCII hex encoded strings. | ||
|
||
Client info strings consist of 4 parts | ||
- client name (e.x. `trin`,`fluffy`) | ||
- client version + short commit (e.x. `0.1.1-2b00d730`) | ||
- operating system + cpu archtecture (e.x. `linux-x86_64`) | ||
- programming language + language version (e.x. `rustc1.81.0`) | ||
|
||
Example | ||
- String: `trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0` | ||
- Hex encoding: `0x7472696E2F302E312E312D32623030643733302F6C696E75782D7838365F36342F7275737463312E38312E30` | ||
|
||
|
||
Max length of a client info we should accept | ||
- MAX_CLIENT_INFO_BYTE_LENGTH = 200 | ||
|
||
|
||
Ping and Pong payload | ||
```python | ||
|
||
HistoryNetworkDistanceV1 = Container( | ||
type: 10, | ||
version: 1, | ||
payload: Container(client_info: ByteList[MAX_CLIENT_INFO_BYTE_LENGTH]) | ||
) | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# Portal State Network Distance | ||
|
||
This payload is only supposed to be used for the state network | ||
|
||
## version 1 | ||
|
||
Ping and Pong payload | ||
```python | ||
StateNetworkDistanceV1 = Container( | ||
type: 2, | ||
version: 1, | ||
payload: Container(data_radius: uint256) | ||
) | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# Portal Beacon Network Distance | ||
|
||
This payload is only supposed to be used for the beacon network | ||
|
||
## version 1 | ||
|
||
Ping and Pong payload | ||
```python | ||
BeaconNetworkDistanceV1 = Container( | ||
type: 3, | ||
version: 1, | ||
payload: Container(data_radius: uint256) | ||
) | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# Portal Canonical Transaction Index Network Distance | ||
|
||
This payload is only supposed to be used for the Canonical Transaction Index Network | ||
|
||
## version 1 | ||
|
||
Ping and Pong payload | ||
```python | ||
CanonicalTransactionIndexNetworkDistanceV1 = Container( | ||
type: 4, | ||
version: 1, | ||
payload: Container(data_radius: uint256) | ||
) | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# Portal Transaction Gossip Network Distance | ||
|
||
This payload is only supposed to be used for the Transaction Gossip network | ||
|
||
## version 1 | ||
|
||
Ping and Pong payload | ||
```python | ||
Transaction GossipNetworkDistanceV1 = Container( | ||
type: 5, | ||
version: 1, | ||
payload: Container(data_radius: uint256) | ||
) | ||
``` |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,67 @@ | ||||||
# Ping Custom Payload Extensions | ||||||
|
||||||
## The problem | ||||||
|
||||||
Over time we will need to change what is sent over ping/pong `custom_payload`'s to adjust for what is needed in the future. | ||||||
Currently the only way to update a `custom_payload` is through breaking changes which break protocol compatible between Portal implementations. | ||||||
As we get users, deploying breaking changes to mainnet will no longer be an option as we are aiming for a 100% uptime. | ||||||
|
||||||
## The Solution | ||||||
|
||||||
Ping Custom Payload Extensions. A versioned type prefixed format where we can upgrade endpoints on a new version well giving a window for all Portal implementations to update before deprecating the older version. This will allow clients to implement new functionality without breaking compatibility with the standard specification. | ||||||
|
||||||
# Type's | ||||||
|
||||||
There are 4_294_967_296 unique type ids. | ||||||
|
||||||
Types 0-10_000 and 4_294_957_295-4_294_967_296 are reserved for for future upgrades. | ||||||
|
||||||
The rest are first come first serve, but they should still be defined in this repo to avoid overlaps. | ||||||
|
||||||
|
||||||
## Requirements | ||||||
|
||||||
All payloads used in the Ping `custom_payload` MUST follow the `Ping Custom Payload Extensions` format. | ||||||
|
||||||
## Custom Payload Extensions Format | ||||||
|
||||||
- **type**: what payload type is it | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
- **verison**: what version of the type it is | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
- **payload**: a ssz container which contents are specified by the type and version field | ||||||
|
||||||
|
||||||
```python | ||||||
CustomPayloadExtensionsFormat = Container( | ||||||
type: Bytes4, | ||||||
version: Bytes4, | ||||||
payload: Container(inner payload is defined by type and version) | ||||||
) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way this currently is defined, it means that one has to write some custom code to only decode the encoded type and version, at which point it can then know how to decode the other Unless you meant to have the |
||||||
``` | ||||||
|
||||||
## Ping vs Pong | ||||||
The relationship between Ping and Pong message will be determined by the requirements of the type/version. | ||||||
|
||||||
Currently type 1,2,3 are mirrored formats, but there is room for a Ping `custom_payload` to specify details about what it wants to request, then pong handles it. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
|
||||||
### Error responses | ||||||
If the ping receiver can't handle the ping for any reason the pong should return an error payload | ||||||
|
||||||
Pong payload | ||||||
```python | ||||||
|
||||||
# Max ASCII hex encoded strings length | ||||||
MAX_ERROR_BYTE_LENGTH = 300 | ||||||
|
||||||
CustomPayloadExtensionsFormat = Container( | ||||||
type: 4_294_967_295, | ||||||
version: 1, | ||||||
payload: Container(error_code: Bytes4, message: ByteList[MAX_ERROR_BYTE_LENGTH]) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should have given some context here. I would think
|
||||||
) | ||||||
``` | ||||||
|
||||||
### Error Code's | ||||||
|
||||||
- 0: Extension not supported | ||||||
- 1: Requested data not found | ||||||
- 2: System error | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're going to provide suggested error codes, I would suggest we expand this a bit and include clear language around how we suggest they be used that includes examples. |
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.
These feel potentially redundant. Seems like we could simply have a
type
and if we end up needing to upgrade one of these types, we can use a new type byte.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.
sure that is fine