Add ping version 2 description. #526
Conversation
sync/p2p_messages.md
Outdated
| ### Version 2 | ||
|
|
||
| - `Versions :: list` | ||
| - `Protocol :: binary` - type of message. |
There was a problem hiding this comment.
Could you please elaborate a bit more on this field? It's name is protocol, it's description is "type of message".
In general I'd like to see somewhere documented how this "Versions" field should work and what is used for.
There was a problem hiding this comment.
At the moment, the only protocol is ping. It specifies the node supports v1 and v2 pings. It will be used to decide which version of a message should be used. I'll add an explanation.
| - `Difficulty :: int` - the total difficulty of the chain. | ||
| - `TopHash :: byte_array` | ||
| - `sync_allowed :: bool` - if the sender of this ping message is accepting | ||
| - `SyncAllowed :: bool` - if the sender of this ping message is accepting |
There was a problem hiding this comment.
Since we have a capability field now, isn't it appropriate to move this there? Sounds like a capability to me.
There was a problem hiding this comment.
It could be, the current example capability is more complicated though, it is for a sync capability "chain_poi", which is:
{height, int}
{root_hash, binary}
{genesis, binary}
{top, binary}
{poi, binary}
A downside would be that it increases the size of the ping.
There was a problem hiding this comment.
Okay, maybe a misunderstand "capabilities"
There was a problem hiding this comment.
I don't think we are sure that is how capabilities should be used, let's see when we actually find a use-case for them. My idea was that the capabilities should signal what additional messages that the node will understand and react to 🤔
There was a problem hiding this comment.
Ulf wanted to use it for additional methods of sync. aeternity/aeternity#4189
| - `GenesisHash :: byte_array` | ||
| - `Height :: int` - current height. |
There was a problem hiding this comment.
Isn't height kind of "per database" (node) field? Why is it shared? If you know the remote TopHash isn't it possible to derive its height as well?
There was a problem hiding this comment.
I asked Ulf and this is his response, "It's true that the height can be derived, but only by first downloading headers. While working on fast synch strategies, I felt that getting the height up front was a convenience at fairly low cost."
| - `Difficulty :: int` - the total difficulty of the chain. | ||
| - `TopHash :: byte_array` | ||
| - `sync_allowed :: bool` - if the sender of this ping message is accepting | ||
| - `SyncAllowed :: bool` - if the sender of this ping message is accepting |
There was a problem hiding this comment.
I don't think we are sure that is how capabilities should be used, let's see when we actually find a use-case for them. My idea was that the capabilities should signal what additional messages that the node will understand and react to 🤔
This PR is supported by Æternity Foundation and Æternity Crypto Foundation