Define ping/pong payload extensions

[KolbyML]

This is a WIP and I am looking to get early feedback

I propose ping/pong custom_payload extensions. A typed versioned format with the goal of making Portal Ping/Pong a flexible extension and preventing protocol changes from breaking other clients when updated versions are deployed. This will also give us to time rollout updated ping/pong endpoints before deprecating older versions.

[r4f4ss]

Considering the goal of "prevent future protocol breaking changes, more flexible, easier less risk upgrades regarding", imagine the scenario where a ping receive a custom payload with trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0. Is it suposed the client to know wich portal network protocol version is associated with this particular trin version? This will demand a mapping. As a solution, the custom payload would return protocol specs version (e.g. pn1)(if specs is not versioned, could be).

Also, for privacy reasons, some client may decide to do not reveal its implementation/version/system, thys payload may be optional, or possibly a free string with defaut value trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0.

[KolbyML]

Considering the goal of "prevent future protocol breaking changes, more flexible, easier less risk upgrades regarding", imagine the scenario where a ping receive a custom payload with trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0. Is it suposed the client to know wich portal network protocol version is associated with this particular trin version? This will demand a mapping. As a solution, the custom payload would return protocol specs version (e.g. pn1)(if specs is not versioned, could be).

Also, for privacy reasons, some client may decide to do not reveal its implementation/version/system, thys payload may be optional, or possibly a free string with defaut value trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0.

I don't think the PR was read because some of the questions you asked were answered.

Second EL clients have a similar type of message no?

[r4f4ss]

I don't think the PR was read because some of the questions you asked were answered.

Sorry, it is in ping-payload-extensions/ping-custom-payload-extensions.md, yes, it is answered.

[r4f4ss]

Second EL clients have a similar type of message no?

Yes, they have node identifier (name+userIdentity+version+os+arch+compiler), and at least Geth do not allow to replace this identifier using flags, only userIdentity is configurable.

This info is available through geth RPC, just could not find wich message in devP2P protocol return this info, anyways it is public.

[KolbyML]

Second EL clients have a similar type of message no?

Yes, they have node identifier (name+userIdentity+version+os+arch+compiler), and at least Geth do not allow to replace this identifier using flags, only userIdentity is configurable.

This info is available through geth RPC, just could not find wich message in devP2P protocol return this info, anyways it is public.

I think you can request it over devp2p or maybe discv4 I am exactly sure which one it is, but that is how https://ethernodes.org/ this website works to my knowledge.

[KolbyML]

@kdeme @pipermerriam I implemented the suggestions. I'm ready for another look

[morph-dev]

I might have missed it, but how is upgrade supposed to work?

Let's say we want to introduce new StateRadiusPayload that will be used instead of BasicRadiusPayload on state network. How is transition supposed to work? The only way that I see it:

1. define new type in spec (using new integer)
2. clients implement support for new type, but never send Ping request with it (but support replying to it)
3. once all developers agree, we start sending Ping requests with new type (still supporting replying to old type)
4. at some point in the future, stop supporting old type

Am I missing something? Maybe this should be documented as well?

[kdeme]

4.

Am I missing something? Maybe this should be documented as well?

Yes, I have the same remark as @morph-dev basically. There is currently nothing defined of how nodes would share their "capabilities" (= what they support), thus nodes cannot negotiate over which actual types/versions to use, and it would work rather on a trial&error basis, which is far from ideal.

And with the current 1,2,3,4 steps of above, I do not see much difference compared to not having this type/version system and just deciding on a specific moment in time where the payload changes. Because here you still need to do same currently (step 3).

[KolbyML]

@morph-dev @kdeme

Ok I added

• A capabilities extension
• Outlined the 3 absolutely required extensions
• Specified something called a standard extension which requires a fork to change what a subnetwork's standard extension is, but you can call a standard extension without needing to do a trail or error or send a capabilities payload. It is always assumed to be there unless changed via hard fork.

A standard-extension is basically just an extension which is required by a subnetwork and can be assumed exists by default.

Portal implementations can add support for or use non-standard extensions without a hard fork.

So before this type system every change to custom_payloads would require a hard fork, now only changing a subnetworks standard-extensions does, which lays room for people to build extensions without requiring hardforks or requiring every implementation to support an optional feature.

I think there is value in this extension system as we can use it instead of extending or changing the Portal-Wire-Protocol. Which as a by-product makes Portal-Wire-Protocol more stable, as there shouldn't need to be a reason to change it. Or needing to do a hard fork to add a new feature to custom_payloads, unless it is update to a network's standard extension of course.

[morph-dev]

Looks good from my point of view.

[pipermerriam]

I'm fine doing my cleanup post merge. It's all tweaks to the language and it'll be easier as a pull request than as PR feedback.

[KolbyML]

I am generally pro having a easily upgradable protocol

I think client implementors should be able to extend Portal's P2P protocol for client/(multi-client) specific usecases without relying on all teams to come to consensus for all changes to the protocol or for new features. Of course there should be a base Portal protocol all implementations must implement. Clients shouldn't be prisoners of all implementations to add a specific features. Allowing clients to extend the Portal protocol without requiring all other parties to implement the complexity for a feature I think is high value. This already exists today in devp2p and there are a few non-standard protocols which provide a lot of value for the respective clients. For example one of the reasons Nethermind's sync is so fast is because they use a custom devp2p protocol which allows for getting more detailed information of what blocks X client has avaliable.

1. I'd like it to be very clear to not require a hard fork. Else I don't see the point of making it complex (see my review comment at code line).

Hard forks are required for changing the standard extension for a Portal sub-network. Since it is implied by default all clients support X protocol version.

Of course, a node could decide to send only the "latest Sub-network standard extension" and nothing else and it would remain the same as now.

This would be the case for all Portal implementations which don't want to support extensions, they are intentionally optional.

When a newer standard extension pops up, the capabilities retrieval will be needed however, but there is no way around this anyhow, even when you would do this type of flexibility on portal wire protocol level

Retrieval of capabilities wouldn't be need by any clients, unless they want to access non-standard protocols. This would be opt in, which is an important aspect of this Proposal. A standard extension pops up would be changed through a hardfork, until then the extension wouldn't be a standard extension. Clients don't need to call capabilities to find out what standard extensions are supported by any clients as they are required by the protocol and changed by hardforks which happen during timestamps

So calling capabilities would never be needed unless a client specifically opt into wanting to use non-standard extensions.

So it would require always two messages at minimum I think (well, one could try to just launch on trial and error), assuming the client info extension is not mandatory.

This PR doesn't change the minimum required messages sent from what is currently done on the network. The difference is clients would now be able to optionally access other ping extensions.

I am generally pro having a easily upgradable protocol

So in short I would say easily upgradable protocol isn't even one of the major reasons why this proposal would be beneficial.

[KolbyML]

Regarding @kdeme comment about inefficiency.

I think that we can have the best of both. I would suggest that clients not open with a capabilities message, but instead just keep the same current approach and open with a request for the radius. If the client responds with an error, you can fallback to requesting their capabilities...

This is already the expectation so I don't think ^ is a best of both, what was wrote above is already the intended functionality. Ping extensions are meant to change nothing for implementations which don't want them. Other then needing to send 0 extensions supported, if capabilities are request

I don't think it makes sense for clients to open with Capability as it is optional at the end of the day. It isn't required for the base Portal Protocol to work. Clients might open with Capability if I manual make a JSON-RPC ping request to a new node I haven't talk to yet, but I don't expect this to be the first call in a majority of cases.

So in short I don't think there are any disagreements on this point, unless I misunderstood something

And realistically, I see this being an API that we are likely to use for "debug" purposes and for easier upgrades/expansion of the useful metadata about stored data that can share with each other.

I see ping extensions as a great way to not bloat the base Portal P2P protocol. I think Ping Extensions are a great environment to allow for early experiments with "Welcome Basket's" https://github.com/ethereum/portal-network-specs/issues/350 we can test it out on trin, without requiring other Portal clients to implement it, as it is a optimization at the end of the day. If it is popular enough we can make it a standard used by all implementations. If it doesn't gain popularity it can remain a Trin specific feature which is fine, or also be supported by a subset of Portal implementations. This is one example, but I can think of a few more if needed :grin:

All clients won't support all extensions and this is an intended feature. When L2's adopt Portal (it is a question of when) they can remain complaint with the base Portal Protocol (instead of making breaking changes to add new features), and instead rely on Ping extensions to add desired features, which don't make sense for the base Portal Protocol.

I think Ping Extensions are aligned with the idea of keeping the "height to ride" low for client implementers, well still allowing the flexibility for clients who want more to have more. Different teams have different resources, some features we want to support in Trin might not make sense in Samba or Shisui maybe X feature isn't a priority for them, or they don't have the resources to implement it. I don't think that should block Trin or any other team from innovating. Or we should pressure smaller teams to add features they don't need for their usecases/users

[kdeme]

Regarding @kdeme comment about inefficiency.

I think that we can have the best of both. I would suggest that clients not open with a capabilities message, but instead just keep the same current approach and open with a request for the radius. If the client responds with an error, you can fallback to requesting their capabilities...

And realistically, I see this being an API that we are likely to use for "debug" purposes and for easier upgrades/expansion of the useful metadata about stored data that can share with each other.

Yes, true, that could be an approach to keep the messages similar as is. Assuming the client information extension is not mandatory.

Now, I read remarks about how a client may decide to not implement this or that extension, which is true. And that it allows for quick development/special features/innovation/etc., which is all true, I don't question this. But the part that seems to be missing is that a client MUST still reply to an unsupported message, and thus in practice more messages could be transferred anyhow, whether useful or not because it is supported or not. It kind of depends on the implementation checking capabilities or just doing fire-and-forget. This is what I mean with potential inefficiency.

When comparing with devp2p additional protocols, I think the main difference is that this one is ingrained within the main protocol. This does make it more lean as it can be adjusted per sub-network, so I'm not necessarily against that. I'm just raising questions on efficiency (and the hardfork thingie), that's all.

[bhartnett]

In order to reduce the number of ping/pong messages for clients that support multiple of the standand and non-standard capabilities is there a way to encode a single message that has a list of types and an another list/Container of the respective type payloads? In other words, put multiple messages together. After learning the capabilities of a peer it would make sense to send a ping message containing all the message types that you and your peer both support. This is just an optimization idea I guess but not sure if it would work well in practice.

[KolbyML]

In order to reduce the number of ping/pong messages for clients that support multiple of the standand and non-standard capabilities is there a way to encode a single message that has a list of types and an another list/Container of the respective type payloads? In other words, put multiple messages together. After learning the capabilities of a peer it would make sense to send a ping message containing all the message types that you and your peer both support. This is just an optimization idea I guess but not sure if it would work well in practice.

This could be a new extension type in the future. Of course there is complexity in this because if you requested too many types you would need to use uTP for the transfer. I expect some ping extensions to use uTP of course, just noting it. "Welcome Baskets" would most like use uTP for example.

Since this is an optimization and there doesn't seem to be any clear use case at the moment, and it can be done simply by adding a new ping extension, I don't think this idea blocks this PR. But if it is desired in the future it can be done with new Ping extensions, so it isn't like what you are proposing wouldn't be possible, it is an idea in the future that can be done if it makes sense at some point in time.