-
Notifications
You must be signed in to change notification settings - Fork 721
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
WIP: Updates due to the addition of handshake queries #5100
Closed
jprider63
wants to merge
7
commits into
IntersectMBO:bolt12/peer-sharing
from
GaloisInc:3907-handshake-parameters
Closed
WIP: Updates due to the addition of handshake queries #5100
jprider63
wants to merge
7
commits into
IntersectMBO:bolt12/peer-sharing
from
GaloisInc:3907-handshake-parameters
Conversation
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
jprider63
requested review from
deepfire,
jutaro,
mgmeier,
dcoutts,
Jimbo4350,
erikd,
newhoggy,
JaredCorduan and
CarlosLopezDeLara
as code owners
April 14, 2023 21:02
Adds new flag 'PeerSharing' can have 3 possible values: - `NoPeerSharing` - `PeerSharingPrivate` - `PeerSharingPublic` This flag interacts with `PeerAdvertise` values in the following way: `AdvertisePeer` is local to the configuration of a given node. A node can be willing to share those peers that it has configured as `PeerAdvertise`. However `PeerSharing` is about wether or not the peer (itself) wishes to participate on `PeerSharing` or not or if it allows others to share its address to others. `PeerSharing` has priority over `AdvertisePeer`, example: BP has `NoPeerSharing` flag (meaning it won't participate in Peer Sharing, not even run the miniprotocol). Relay has BP as local peer configured as `AdvertisePeer` (probably misconfigured it) When handshake between the BP and the Relay happens the Relay is going to see that BP does not wish to participate in PeerSharing and so it is not going to perform peer sharing with it nor share it to others. Here is the `combinePeerInformation` function that decides the sharing interaction semantics between the two flags. ``` -- Combine a 'PeerSharing' value and a 'PeerAdvertise' value into a -- resulting 'PeerSharing' that can be used to decide if we should -- share or not the given Peer. According to the following rules: -- -- - If no PeerSharing value is known then there's nothing we can assess -- - If a peer is not participating in Peer Sharing ignore all other information -- - If a peer said it wasn't okay to share its address, respect that no matter what. -- - If a peer was privately configured with DoNotAdvertisePeer respect that no matter -- what. -- combinePeerInformation :: PeerSharing -> PeerAdvertise -> PeerSharing combinePeerInformation NoPeerSharing _ = NoPeerSharing combinePeerInformation PeerSharingPrivate _ = PeerSharingPrivate combinePeerInformation PeerSharingPublic DoNotAdvertisePeer = PeerSharingPrivate combinePeerInformation _ _ = PeerSharingPublic ``` And this is the function that is used to filter the result list: https://github.com/input-output-hk/ouroboros-network/blob/master/ouroboros-network/src/Ouroboros/Network/PeerSelection/KnownPeers.hs#L394 Also relevant: https://github.com/input-output-hk/ouroboros-network/blob/master/ouroboros-network/src/Ouroboros/Network/Diffusion/P2P.hs#L1014
jprider63
force-pushed
the
3907-handshake-parameters
branch
from
April 20, 2023 03:10
ab09b2e
to
12cc3d9
Compare
jprider63
requested review from
a team,
coot,
fmaste and
cleverca22
as code owners
April 20, 2023 03:10
9 tasks
coot
force-pushed
the
bolt12/peer-sharing
branch
3 times, most recently
from
April 25, 2023 11:18
bb13756
to
7e46b10
Compare
coot
force-pushed
the
bolt12/peer-sharing
branch
from
April 28, 2023 07:26
7e46b10
to
278501e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This makes a few updates given API changes from the addition of handshake queries (IntersectMBO/ouroboros-network#3907). Additional changes are likely, but I started running into compilation mismatches with ouroboros-network. It looks like the index-state now points to
cardano-haskell-packages 2023-04-13T17:16:53Z
onmaster
. Is there a way to tell which ouroboros-network commit this corresponds to?The corresponding ouroboros-network PR is here: IntersectMBO/ouroboros-network#4256