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.

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

Extend cardano-ping to output handshake negotiation parameters #3907

Closed
Tracked by #3887
coot opened this issue Jul 18, 2022 · 4 comments · Fixed by #4256
Closed
Tracked by #3887

Extend cardano-ping to output handshake negotiation parameters #3907

coot opened this issue Jul 18, 2022 · 4 comments · Fixed by #4256
Labels
cardano-ping Iessues / PRs related to cardano-ping

Comments

@coot
Copy link
Contributor

coot commented Jul 18, 2022

This can be useful to discover what are the capabilities of the remote end. We want to use this to check how many P2P nodes are deployed on mainnet.

@coot coot added cardano-ping Iessues / PRs related to cardano-ping networking labels Jul 18, 2022
@coot coot mentioned this issue Jul 18, 2022
6 tasks
@vfrsilva vfrsilva mentioned this issue Oct 26, 2022
4 tasks
@coot
Copy link
Contributor Author

coot commented Dec 7, 2022

The handshake decoder is implemented by cardano-ping here. A reference codec from ouroboros-network-protocols.

This issue requires:

{ type: `node-to-node`
, version: 10
, networkMagic: 123456
, diffusionMode: 'InitiatorOnly'
}

@coot coot changed the title Extend cardano-ping to output handshake negotiation setting Extend cardano-ping to output handshake negotiation parameters Dec 7, 2022
@coot
Copy link
Contributor Author

coot commented Dec 8, 2022

It turns out, as pointed out by @karknu, cardano-ping already prints the version number together with negotiated parameters. But this is not good enough, we need a way to know what the inbound side original version data is, not the one that were negotiated. This requires some changes.

I suggest that we add extra boolean flag in NodeTo{Node,Client}VersionData called query. If it was received the inbound side will replay not with the negotiated options but with its original parameters. Then the outbound & inbound sides can compute the intersection on their end (they will agree, version data form a monoid, although it might not be commutative when we introduce peer-sharing). If the inbound side does not agree with what it would receive it can reply with MsgRefuse instead of MsgAcceptVersion. This way we don't need to modify the protocol itself.

Or we modify the handshake to always act like that (from the next protocol version).

We can also consider if the handshake should just terminate in the query mode, although think this is not necessary.

@jprider63
Copy link
Contributor

@coot, for the first approach, is my understanding correct that the idea is to add a query flag that asks the other party to return a list of supported versions instead of the negotiated options? Then we can log this list and each side can independently compute the negotiated options. Doesn't this require modifying the protocol? What needs to happen to properly modify the protocol?

Can you point me to where in the code the other party handles this request and how the options are negotiated?

Also, it seems like cardano-ping re-implements some of the data types and their codecs. Do you know why this is the case and it doesn't import them?

@coot
Copy link
Contributor Author

coot commented Jan 3, 2023

NodeTo{Node,Client}VersionData can have different serialisation for different NodeTo{Node,Client}Version (we use the same type just for convenience, we used to have a GADT but it made things more complex than necessary). Since this flag will only be true for the new versions, the protocol can have a slightly different behaviour.

Here are the links:

Then there are node-to-node and node-to-client specific types, e.g. versions, version data and their serialisation / deserialisation; they are defined in these modules:

@coot coot linked a pull request Jan 9, 2023 that will close this issue
9 tasks
iohk-bors bot added a commit that referenced this issue May 8, 2023
4256: Add a boolean query flag to request supported versions r=coot a=jprider63

# Description

This is a WIP PR implementing #3907. Specifically, it adds a boolean query flag to `NodeTo{Node,Client}VersionData` that causes the other party to return their list of supported versions instead of the negotiated version. I still need to do more testing. 



Co-authored-by: James Parker <[email protected]>
Co-authored-by: Marcin Szamotulski <[email protected]>
Co-authored-by: JP <[email protected]>
@iohk-bors iohk-bors bot closed this as completed in #4256 May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cardano-ping Iessues / PRs related to cardano-ping
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants