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.

Sign up for GitHub

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

Jump to bottom

Define ping/pong payload extensions #348

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Define ping/pong payload extensions #348

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a603389
Define ping/pong payload extensions
KolbyML Oct 21, 2024
cedecda
fix: resolve PR concerns
KolbyML Oct 22, 2024
3d76802
fix: update links
KolbyML Oct 22, 2024
3a68292
fix: add capabilities message and outline how to upgrade a standard m…
KolbyML Oct 23, 2024
56fa823
Update ping-custom-payload-extensions.md
KolbyML Oct 23, 2024
a0aa9ba
fix: remove networks that don't exist yet
KolbyML Oct 24, 2024
3ab5275
fix: some wording in capabilities payload
KolbyML Oct 24, 2024
b01d69a
fix: remove word version
KolbyML Oct 24, 2024
014dd45
fix: push changes before going to bed
KolbyML Oct 25, 2024
c06e6ae
fix: spelling issues
KolbyML Nov 21, 2024
313c977
fix: resolve PR concerns
KolbyML Nov 22, 2024
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
7 changes: 1 addition & 6 deletions beacon-chain/beacon-network.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,7 @@ The beacon chain network supports the following protocol messages:

#### `Ping.custom_data` & `Pong.custom_data`

In the beacon chain network the `custom_payload` field of the `Ping` and `Pong` messages is the serialization of an SSZ Container specified as `custom_data`:

```python
custom_data = Container(data_radius: uint256)
custom_payload = serialize(custom_data)
```
In the beacon chain network the `custom_payload` field of the `Ping` and `Pong` messages is the serialization of an SSZ Container specified as [Type 1 Basic Radius Payload](../ping-payload-extensions/extensions/type-1.md)

### Routing Table

Expand Down
7 changes: 1 addition & 6 deletions canonical-transaction-index/canonical-transaction-index-network.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,7 @@ The canonical transaction index network supports the following protocol messages

#### `Ping.custom_data` & `Pong.custom_data`

In the canonical transaction index network the `custom_payload` field of the `Ping` and `Pong` messages is the serialization of an SSZ Container specified as `custom_data`:

```
custom_data = Container(data_radius: uint256)
custom_payload = SSZ.serialize(custom_data)
```
In the canonical transaction index network the `custom_payload` field of the `Ping` and `Pong` messages is the serialization of an SSZ Container specified as [Type 1 Basic Radius Payload](../ping-payload-extensions/extensions/type-1.md)


### Routing Table
Expand Down
6 changes: 1 addition & 5 deletions history/history-network.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,8 @@ The history network supports the following protocol messages:

#### `Ping.custom_data` & `Pong.custom_data`

In the history network the `custom_payload` field of the `Ping` and `Pong` messages is the serialization of an SSZ Container specified as `custom_data`:
In the history network the `custom_payload` field of the `Ping` and `Pong` messages is the serialization of an SSZ Container specified as [Type 2 History Radius Payload](../ping-payload-extensions/extensions/type-2.md)

```python
custom_data = Container(data_radius: uint256)
custom_payload = SSZ.serialize(custom_data)
```

### Routing Table

Expand Down
12 changes: 12 additions & 0 deletions ping-payload-extensions/extensions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# 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 |
|---|---|---|---|---|
| [0](extensions/type-0.md) | Capabilities | All | Provides a list of enabled types | Yes |
| [1](extensions/type-1.md) | Basic Radius Payload | State, Beacon, Canonical Transaction Index, Transaction Gossip | Provides the nodes Radius | Yes |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| [1](extensions/type-1.md) | Basic Radius Payload | State, Beacon, Canonical Transaction Index, Transaction Gossip | Provides the nodes Radius | Yes |
| [1](extensions/type-1.md) | Basic Radius Payload | State, Beacon | Provides the nodes Radius | Yes |

Since these other networks don't exist yet, I don't think we should reference them.

| [2](extensions/type-2.md) | History Radius Payload | History | Provides the nodes radius and ephemeral header count | Yes |
| [3](extensions/type-3.md) | Client Info | All | It will return you something like `trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0` | Yes |
26 changes: 26 additions & 0 deletions ping-payload-extensions/extensions/template.md
View file
Open in desktop
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]
)
```
19 changes: 19 additions & 0 deletions ping-payload-extensions/extensions/type-0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Capabilities Payload

Any network can only have Portal client can only every have 500 extensions enabled per Portal sub-networks.

This payload provides a list of u16's each u16 provide in the list corresponds to an an enabled extension type.

Ping and Pong Payload
```python

MAX_CAPABILITIES_LENGTH = 500

capabilities = SSZ.serialize(List[u16, MAX_CAPABILITIES_LENGTH])

BasicRadiusPayload = Container(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be named CapabilitiesPayload?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch

type: 0,
payload: capabilities
)
```

15 changes: 15 additions & 0 deletions ping-payload-extensions/extensions/type-1.md
View file
Open in desktop
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
)
```

14 changes: 14 additions & 0 deletions ping-payload-extensions/extensions/type-2.md
View file
Open in desktop
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
)
```
34 changes: 34 additions & 0 deletions ping-payload-extensions/extensions/type-3.md
View file
Open in desktop
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.


### 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

client_info = SSZ.serialize(Container(client_info: ByteList[MAX_CLIENT_INFO_BYTE_LENGTH]))

ClientInfoPayload = Container(
type: 3,
payload: client_info
)
```
110 changes: 110 additions & 0 deletions ping-payload-extensions/ping-custom-payload-extensions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
# 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 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**: what payload type is it
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- **type**: what payload type is it
- **type**: numeric identifier which tells clients how the `payload` field should be decoded.

- **payload**: a ssz ByteList of max length 1100 which contents are specified the type field
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- **payload**: a ssz ByteList of max length 1100 which contents are specified the type field
- **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/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.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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.
> 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 is for if an extension is asking for something and it doesn't exist.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This error code is for if an extension is asking for something and it doesn't exist.
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)
Copy link
Member

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.

Copy link
Member Author

@KolbyML KolbyML Nov 21, 2024 •
edited
Loading

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 for

  • getting better statistics of our network,
    • useful for glados
    • tells us what platforms our popular and we should support better e.x. x86/arm/riscv5
  • allows us to better track what client is creating bugs on the network. Are they running an old commit? Is it a CPU architecture specific issue? etc

If this isn't a standard extension it would require

  • one ping to get capabilities
  • one ping to get 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.

Copy link
Member

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.

Copy link
Collaborator

@kdeme kdeme Nov 21, 2024 •
edited
Loading

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?

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps just add it as a field in the capabilities extension?

I actually like this. Removes an additional message pair and makes it so that this data gets transmitted implicitly with the other message.

Copy link
Member Author

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.

Copy link
Collaborator

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 :).

Copy link
Member Author

@KolbyML KolbyML Nov 22, 2024 •
edited
Loading

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 :).

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


## 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`.

## Does implementing non-standard-extensions require a hardfork?
No only changing a sub-networks standard extension requires a hard fork.
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 like the use of the term "hard fork" here. It collides too much with the core ethereum protocol use and in my opinion makes it sound more severe than it actually is.

Copy link
Member Author

Choose a reason for hiding this comment

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

A hard fork is just a rule change that occurs on all clients. In this case it would just be a timestamp which when reached all Portal clients would change X functionality for the base Portal Protocol.

Hardforks will be required regardless I expect in the future, there will be networking changes we make to calls other then ping/pong. To ensure a smooth not breaking change hardforks, might be a good way to do this.

Another way to do this could potentially be adding some kind of Portal Version message, where clients would always speak in the oldest version or something along those lines, but I feel that solution would be a lot more complex than hard forks. Ping/Pong is only a small part of the Portal Wire Spec which could change over time. We don't want to break interop when shipping Portal Wire Spec upgrades in the future. Ping Extensions make needing

  • hard forks less common
  • and less bloat to the core Portal Wire Spec

to name a few

If we want to use a different term then "hard fork", I am fine with that and I am open to suggestions. I struggle to think of a better understood word to describe what is happening though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

my concern is not so much about semantics, but rather about the fact that you still require to have a client aligned release mechanism, see also comment at #348 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that using the term 'hard fork' doesn't sound right. Portal doesn't take part in consensus and doesn't really fork because it's not a blockchain in itself. Of course we can have breaking protocol changes that require all clients to be updated at the same time so perhaps a better term would be 'breaking protocol change' or 'non backwards compatible protocol upgrade'.

Copy link
Member Author

@KolbyML KolbyML Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I agree that using the term 'hard fork' doesn't sound right. Portal doesn't take part in consensus and doesn't really fork because it's not a blockchain in itself. Of course we can have breaking protocol changes that require all clients to be updated at the same time so perhaps a better term would be 'breaking protocol change' or 'non backwards compatible protocol upgrade'.

A hard fork is a type of update to a blockchain protocol that is not backward-compatible. This means that nodes (computers running the blockchain software) must upgrade to the new protocol to remain part of the blockchain network. Nodes that do not update will follow the older protocol, resulting in a split in the blockchain into two separate networks.

breaking protocol change or non backwards compatible protocol upgrade sounds like somebody is trying to say or describe Hardfork, without saying hardfork. A hard fork is a breaking protocol change, that could be related to consensus, or the p2p protocol. A hard fork describes a break in the road which splits a network into 2. A non backwards compatible protocol upgrade is a hardfork, as it forks the p2p network into 2 which can't talk with each other.

Copy link
Contributor

Choose a reason for hiding this comment

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

A hard fork is a type of update to a blockchain protocol that is not backward-compatible. This means that nodes (computers running the blockchain software) must upgrade to the new protocol to remain part of the blockchain network. Nodes that do not update will follow the older protocol, resulting in a split in the blockchain into two separate networks.

Portal isn't exactly a blockchain protocol, it's a p2p network that contains blockchain data which follows a blockchain protocol. The term fork comes from the fact that the block history forks. If bittorrent had a breaking change, it wouldn't be called a hard fork. Anyway, its just semantics I guess.

Copy link
Member Author

Choose a reason for hiding this comment

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

A hard fork is a type of update to a blockchain protocol that is not backward-compatible. This means that nodes (computers running the blockchain software) must upgrade to the new protocol to remain part of the blockchain network. Nodes that do not update will follow the older protocol, resulting in a split in the blockchain into two separate networks.

Portal isn't exactly a blockchain protocol, it's a p2p network that contains blockchain data which follows a blockchain protocol. The term fork comes from the fact that the block history forks. If bittorrent had a breaking change, it wouldn't be called a hard fork. Anyway, its just semantics I guess.

I think this is very fair and it is a symantic issue, ideally their is a single word to refer to non backwards compatible protocol upgrade as it is a bit long when talking. One issue with breaking P2P changes on Portal is we don't share a protocol version with peers. Of course breaking p2p changes can happen on more then just the ping/pong wire message. Most Protocols like bittorrent would support a range of protocol versions and have a negotiation stage to find out what is the shared minimum version supported. The issue with Portal is we don't have any of that infrastructure as we don't have that type of handshake. So I do think we are constrained to do breaking changes through timestamps as Portal connections are shortlived, and Portal's P2P protocol has no idea of a version.

Copy link
Member

Choose a reason for hiding this comment

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

I'm 👎 on using the term hard fork for this. It name collides with the core protocol terminology and will be confusing. I recognize that the term may be correct in definition but the name collision is a deal breaker for me.

I also will re-iterate that I don't think we should try to define how this process will work because I don't think we know and trying to define it corners us into future behavior that may not be necessary or optimal. We should leave this process undefined.

Copy link
Member Author

Choose a reason for hiding this comment

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

sure, I updated the PR to remove the term



## 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.

### What is the process for changing a sub-network standard extension

The process of changing a Portal sub-networks standard extension is through a hard fork set by a timestamp. When the timestamp is reached usage of the old standard extension is replaced by the new standard extension. Details on what standard extensions for what networks are being replace are specified in the fork's document.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For me the usefulness of these ping/pong payload extensions is to avoid having to do something like this in the first place.

If we don't avoid this hard-forking, then I don't really understand the point of adding this complexity.

But imo it is possible to do it without hard-forking, as is also explained in the chapter below this?

Copy link
Member

Choose a reason for hiding this comment

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

I would suggest that we remove this section and leave the process undefined. I think we are getting into the realm of over-specifying this piece of the protocol.

Copy link
Member Author

Choose a reason for hiding this comment

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

A client would never need to call capabilities to know what the current standard extension is.

The usefulness of ping extensions is not requiring all parties to agree to make a change to the protocol or add a feature.

Of course supporting the base protocol is required i.e. standard extensions, but this allows Portal clients to make optimizations such as the "welcome basket" proposal #350 Carver proposed, without requiring all implementations to support it. Which is important as we don't want new features to gate keep new clients from joining the network, but also not all clients would likely support "welcome baskets" it is a p2p optimization which isn't required by the core protocol to work. It is a bootstrapping optimization.

Ping extensions don't change the base protocol much, new clients can just return empty for capabilities and only support X networks standard extension. Ping Extensions is opt in and hence it prevents creating a situation of the protocol being "too tall to ride( or participate/implement a client)."

If a client wants a feature what is not popular or doesn't make sense for all clients to implement it they can, which I think is important long term. Especially as not only L1 will be using Portal. The base Portal P2P protocol should be simple, and Ping extensions allow us to keep it simple. Well still allowing for clients to experiment or add new features, which maybe one day become standard extensions if they become popular, but there is no requirement for that, which is one of the important parts of this proposal.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My comment is not about the usefulness of the extensions. It is about this specific section which talks about the requirement of a hard fork.

I think this should be designed in such way that a hard fork is not required at all, else I don't see the point of it, and with the capabilities extension this should be feasible.

Copy link
Member Author

@KolbyML KolbyML Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Are we assuming

  1. ping's are only done by long-lived connections i.e. the other node is in your routing table? All short lived connections are done through find nodes or find content RFC/RFN etc

or

  1. ping's are sent by both long-lived and short-lived connections.

?

If 1. is true I think a forkless mechanism makes sense as the latancy cost of calling capabilities is only high if the connection is shortlived. If ping isn't used for short-lived connections, then the additional latency incurred isn't a big deal.

if 2. is true I think the fork upgrade method for standard extensions make sense as it wouldn't make sense to upgrade via capabilities unless

  • So for "close nodes" upgrading to the newest protocol or using extensions could make sense as those will be long lived connections in the routing table
  • if you are trying to request meta data, calling capabilities would make sense
  • participate in features not supported by the full node set

but if you were a short-lived ping connection, upgrading wouldn't make sense, only using the base standard extension required by the protocol would make sense

I need to put more thought into this as these are only my additional thoughts. I didn't conceive 1. could be an option till just now so I still need to spend time thinking which assumption is actually true 1. or 2.

A forkless method makes a lot of sense for devp2p as they only have long-lived connections. Since Portal is by default a short-lived protocol, having a forked upgrade method for the base standard extensions required by the protocol makes sense if 2. is true, as you wouldn't want to incur the latency for short-lived connections.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting to think about it from short vs long lived connections.

However to avoid confusion I'd suggest to call it something else, because in Portal all connections are short lived.

But from reading your comment I think you mean to differentiate between:

  1. Close neighbors and/or nodes in routing table, that are thus frequently contacted for example to verify that they are alive/responsive.
  2. Nodes that are occasionally contacted when looking up nodes or content.

I think that the second group is rarely going to receive a ping from a "regular" node. When running a recursive find nodes or find content it would not require a ping.
When doing gossip it would typically hit fairly close nodes (unless it is a bridge node initiating gossip) and not use a ping (unless you launch these actively to get radius info, which fluffy does not as that would be to slow).
I'd have to further think about it in which cases a ping would still be done to these nodes.

I can see crawlers and nodes that run bridges perhaps hitting those nodes also, as they want to see the full view.

Copy link
Member Author

@KolbyML KolbyML Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I guess there is more thinking to be done, some of this we would only need to worry about when we propose the first breaking change so I don't think we need to pick a path immediately

The 2 paths I see are

  1. We always require capabilities is called first (on the first communication so in long term connections this would be a one time thing, assuming they are in each respective routing table) to find the latest shared version of the respective subnetworks standard extension. This path would be forkless, but it would always occur 1 additional round trip delay initially to make a standard extension ping, as there could be multiple support standard extension in this system

  2. We do a fork system, where all client implementations would always support a base standard extension for the subnetwork to work/function. This system would make it optional if an implementation wanted to call capabilities or not and hence would not have the additional RT time unless opted into.

Both have trade offs, to ask bluntly which path do you prefer?

### Can a node use the next standard extension for a network before the hardfork
Yes there is no limitation for a Portal Client implementation using the next standard extension or a 3rd party extension before the hard fork has taken place. A requirement of this flexibility is that the new extension being used in-place of the standard extension must still fulfill the duties of the current standard extension. One downside of using new Standard Extension's before the fork is you won't know if a peer supported it yet unless you did either a
- trial and error to see if the peer supports the call
- Sent a [Type 0: Capabilities Payload](extensions/type-0.md)

This can be useful for
- accessing functionality before the hard fork has taken place in a none breaking way
- Extending the protocol with implementation specific heuristics.

It is only recommended to do this for peers in an implementations routing table as most connections are too short lived to make the addition call worth it.
7 changes: 1 addition & 6 deletions state/state-network.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,7 @@ The execution state network supports the following protocol messages:

#### `Ping.custom_data` & `Pong.custom_data`

In the execution state network the `custom_payload` field of the `Ping` and `Pong` messages is the serialization of an SSZ Container specified as `custom_data`:

```
custom_data = Container(data_radius: uint256)
custom_payload = SSZ.serialize(custom_data)
```
In the execution state network the `custom_payload` field of the `Ping` and `Pong` messages is the serialization of an SSZ Container specified as [Type 1 Basic Radius Payload](../ping-payload-extensions/extensions/type-1.md)

#### POKE Mechanism

Expand Down
14 changes: 1 addition & 13 deletions transaction-gossip/transaction-gossip.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,7 @@ The network uses the PING/PONG/FINDNODES/FOUNDNODES/OFFER/ACCEPT messages from t

The network uses the standard XOR distance function.

### PING payload

```
Ping.custom_payload := ssz_serialize(custom_data)
custom_data := Container(transaction_radius: uint256)
```

### PONG payload

```
Pong.custom_payload := ssz_serialize(custom_data)
custom_data := Container(transaction_radius: uint256)
```
In the execution state network the `custom_payload` field of the `Ping` and `Pong` messages is the serialization of an SSZ Container specified as [Type 1 Basic Radius Payload](../ping-payload-extensions/extensions/type-1.md)

## Content Keys

Expand Down
Loading