-
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
Open
KolbyML
wants to merge
11
commits into
ethereum:master
Choose a base branch
from
KolbyML:ping-pong-extensions
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
a603389
Define ping/pong payload extensions
KolbyML cedecda
fix: resolve PR concerns
KolbyML 3d76802
fix: update links
KolbyML 3a68292
fix: add capabilities message and outline how to upgrade a standard m…
KolbyML 56fa823
Update ping-custom-payload-extensions.md
KolbyML a0aa9ba
fix: remove networks that don't exist yet
KolbyML 3ab5275
fix: some wording in capabilities payload
KolbyML b01d69a
fix: remove word version
KolbyML 014dd45
fix: push changes before going to bed
KolbyML c06e6ae
fix: spelling issues
KolbyML 313c977
fix: resolve PR concerns
KolbyML File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
# This document lists defined extensions | ||
This is a list and short description of all the extensions | ||
|
||
- Extensions can be supported by all sub-networks or a subsection | ||
|
||
|
||
| Type number | Name | Supported sub-networks | Short Description | Is this call Required to Implement | | ||
|---|---|---|---|---| | ||
| [0](extensions/type-0.md) | Client Info and Capabilities | All | Returns client info e.x. `trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0` and a list of enabled extensions | Yes | | ||
| [1](extensions/type-1.md) | Basic Radius Payload | State, Beacon | Provides the nodes Radius | Yes | | ||
| [2](extensions/type-2.md) | History Radius Payload | History | Provides the nodes radius and ephemeral header count | Yes | |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
# [Title] | ||
|
||
[Description] | ||
|
||
Ping payload | ||
```python | ||
|
||
[Payload] = SSZ.serialize(Container([Key Value Pairs])) | ||
|
||
|
||
[Container Name] = Container( | ||
type: [Type Number], | ||
payload: [Payload] | ||
) | ||
``` | ||
|
||
Pong payload | ||
```python | ||
|
||
[Payload] = SSZ.serialize(Container([Key Value Pairs])) | ||
|
||
[Container Name] = Container( | ||
type: [Type Number], | ||
payload: [Payload] | ||
) | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
# Client Info and Capabilities Payload | ||
|
||
## Client Info | ||
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` | ||
|
||
#### Privacy Concerns | ||
Clients can optionally return an empty string for privacy reasons, this is not recommended as client info helps researchers understand the network. | ||
|
||
## Capabilities | ||
Portal clients can only have max 400 extensions enabled per sub-network. | ||
|
||
This payload provides a list of u16's each u16 provide in the list corresponds to an enabled extension type. | ||
|
||
## Payload Outline | ||
|
||
Ping and Pong Payload | ||
```python | ||
|
||
MAX_CLIENT_INFO_BYTE_LENGTH = 200 | ||
MAX_CAPABILITIES_LENGTH = 400 | ||
|
||
client_info_and_capabilities = SSZ.serialize(Container( | ||
client_info: ByteList[MAX_CLIENT_INFO_BYTE_LENGTH] | ||
capabilities: List[u16, MAX_CAPABILITIES_LENGTH] | ||
)) | ||
|
||
CapabilitiesPayload = Container( | ||
type: 0, | ||
payload: client_info_and_capabilities | ||
) | ||
``` | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
# Basic Radius Payload | ||
|
||
A basic Ping/Pong payload which only contains the nodes radius | ||
|
||
Ping and Pong Payload | ||
```python | ||
|
||
basic_radius = SSZ.serialize(Container(data_radius: U256)) | ||
|
||
BasicRadiusPayload = Container( | ||
type: 1, | ||
payload: basic_radius | ||
) | ||
``` | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# History Radius Payload | ||
|
||
A specialized radius payload for the history network which contains field for how many ephemeral headers the node holds. | ||
|
||
Ping and Pong Payload | ||
```python | ||
|
||
history_radius = SSZ.serialize(Container(data_radius: U256, ephemeral_header_count=U16)) | ||
|
||
HistoryRadiusPayload = Container( | ||
type: 1, | ||
payload: history_radius | ||
) | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
# Ping Custom Payload Extensions | ||
|
||
## Motivation | ||
|
||
Ping Payload Extensions. There is a minimal set of `standard extensions` which require a breaking change to update. This framework allows Portal clients to implement `non standard extensions` which don't require a breaking change to deploy to the network. A more flexible way to extend the Protocol without bloating the [Portal-Wire-Protocol](../portal-wire-protocol.md) | ||
|
||
# Type's | ||
|
||
There are 65536 unique type ids. | ||
|
||
Types 0-10_000 and 65436-65535 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**: numeric identifier which tells clients how the `payload` field should be decoded. | ||
- **payload**: the SSZ encoded extension payload | ||
|
||
|
||
```python | ||
CustomPayloadExtensionsFormat = Container( | ||
type: u16, | ||
payload: ByteList[max_length=1100] | ||
) | ||
``` | ||
|
||
## Ping vs Pong | ||
The relationship between Ping and Pong message will be determined by the requirements of the type. | ||
|
||
Currently type 1,2,3 are symmetrical, having the same payload for both request and response. This symmetry is not required. Extensions may define separate payloads for Ping and Pong within the same type. | ||
|
||
|
||
### 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 | ||
|
||
error_payload = SSZ.serialize(Container(error_code: u16, message: ByteList[MAX_ERROR_BYTE_LENGTH])) | ||
|
||
ErrorPayload = Container( | ||
type: 65535, | ||
payload: error_payload | ||
) | ||
``` | ||
|
||
### Error Code's | ||
|
||
#### 0: Extension not supported | ||
This code should be returned if the extension isn't supported. This error should only be returned if | ||
- The extension isn't supported | ||
- The extension isn't a required extension for specified Portal Network. | ||
|
||
#### 1: Requested data not found | ||
This error code should be used when a client is unable to provide the necessary data for the response. | ||
|
||
#### 2: Failed to decode payload | ||
Wasn't able to decode the payload | ||
|
||
#### 3: System error | ||
A critical error happened and the ping can't be processed | ||
|
||
## Standard extensions | ||
|
||
Every Portal Network Client is required to support 3 extensions | ||
- [Type 0: Capabilities Payload](extensions/type-0.md): useful for finding what ping extensions a client supports | ||
- Sub-network standard extension. For history this is currently [Type 2: History Radius Payload](extensions/type-2.md). For State and Beacon the base extension is [Type 1: Basic Radius Payload](extensions/type-1.md). These are extensions what are crucial. This standard extensions are specified the respective sub-networks specification | ||
- Type 3: [Get Client info Payload](extensions/type-3.md) | ||
|
||
## What is the [Type 0: Capabilities Payload](extensions/type-0.md) for | ||
It is for Portal implementations which want to see what extensions a peer supports. Not all extensions need to be implemented by all parties. So in order for a peer to find if an extension is implemented a [Type 0: Capabilities Payload](extensions/type-0.md) should be exchanged. | ||
|
||
Non-required extension offer a way for Portal implementations to offer extended functionality to its users without requiring every Portal implementing party to agree to a new feature. This allows for a diverse set of use cases to be fulfilled without requiring every implementer implement every extension, or requiring the need to bloat the minimal [Portal-Wire-Protocol](../portal-wire-protocol.md) with new `Message Types`. | ||
|
||
## How do sub-network standard extension's work | ||
sub-network standard extension's are fundamental extensions that are required for a Portal sub-network to function. They must be supported by the sub-networks implementations. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I suggest we remove this from the standard set. Clients should support it but I don't think it makes sense to make it be required.
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.
I think it is fine to make it so a valid option is an empty string for privacy inclined users, but I don't think we should remove it from being a standard extension, as 99.9% of honest Portal nodes would support it.
Client Info
is important forIf this isn't a standard extension it would require
capabilities
client info
If 99.9% of honest clients are going to support this call, I think it should be a standard extension, with an opt out to return an empty string by the 0.001% of users which have a privacy concern.
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.
I think I agree with this. Will set the default path towards clients supporting this which I think is positive for network monitoring/health.
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.
If it is fine to just be an empty string, then it is a bit silly to send this around as another message req/rep for those that want to keep that private.
Perhaps just add it as a field in the capabilities extension?
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.
I actually like this. Removes an additional message pair and makes it so that this data gets transmitted implicitly with the other message.
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.
I will update the PR to make it so client info is a part of the capabilities extension, this will limit the max amount of ping extensions a client can have enabled at a time to 450 or 400 instead of 500 to reserve byte space for the client enough, but 400 extensions should be more then enough so it isn't a deal breaker at the end of the day.
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.
To be honest, I don't think anybody would want to be writing Portal implementations if they have to deal with 400 (or even 40 for that matter) ping extensions :).
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.
The whole point is they are optional, so Portal implementation that didn't want to implement something wouldn't have to. I don't know what will happen in the future or how extensions we would be adopted. So who knows how many would end up being used.
I don't see an issue in having some head room