-
Notifications
You must be signed in to change notification settings - Fork 87
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
Refactor PeerSelection.RootPeersDNS #4625
Conversation
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/DNS/LedgerPeers.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/DNS/LedgerPeers.hs
Outdated
Show resolved
Hide resolved
9c5a794
to
ea2b5e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it looks good, but a few things needs to be addressed.
ouroboros-network/sim-tests/Test/Ouroboros/Network/Diffusion/Node.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/sim-tests/Test/Ouroboros/Network/Diffusion/Node.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/DNS/LocalRootPeers.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/DNS/LocalRootPeers.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/DNS/LedgerPeers.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/PeerSelectionActions.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/PeerSelectionActions.hs
Outdated
Show resolved
Hide resolved
c775c6f
to
af11b3b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/KnownPeers.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/KnownPeers.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/PeerStateActions.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/PeerStateActions.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/PeerStateActions.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/sim-tests/Test/Ouroboros/Network/PeerSelection/PeerGraph.hs
Outdated
Show resolved
Hide resolved
a4873f4
to
e0d61a9
Compare
ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/LedgerPeers.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/PeerStateActions.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/Monitor.hs
Outdated
Show resolved
Hide resolved
- Refactor `withLedgerPeers` & `resolveDomainAccessPoint` - Removes `withLedgerPeers` (only used in tests now) - `withPeerSelectionActions` is responsible for fetching and resolving all peers - Renames `resolveDomainAccessPoint` to `resolveLedgerPeers` - Splits `Ouroboros.Network.PeerSelection.RootPeersDNS` into - `Ouroboros.Network.PeerSelection.DNS.RootPeers` - `Ouroboros.Network.PeerSelection.DNS.LocalRoots` - `Ouroboros.Network.PeerSelection.DNS.LedgerPeers` - `Ouroboros.Network.PeerSelection.DNS.DNSSemaphore`
Add no assertion failure to testnet test suite Fix livelock test and peer seleection dodgy trace test
Also use targets from the right source of truth
7d0e692
to
fd160bf
Compare
bc2051c
to
5480712
Compare
ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection.hs
Outdated
Show resolved
Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/Types.hs
Outdated
Show resolved
Hide resolved
We are hiding the `peerconn` in an existential type, so we no longer need to map it `()`. This saves quite a lot of memory. This patch also removes the `Functor` instance of `PeerSelectionState`, which is no longer needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
withLedgerPeers
&resolveDomainAccessPoint
withLedgerPeers
(only used in tests now)withPeerSelectionActions
is responsible for fetching and resolving all peersresolveDomainAccessPoint
toresolveLedgerPeers
Ouroboros.Network.PeerSelection.RootPeersDNS
intoOuroboros.Network.PeerSelection.DNS.RootPeers
Ouroboros.Network.PeerSelection.DNS.LocalRoots
Ouroboros.Network.PeerSelection.DNS.LedgerPeers
Ouroboros.Network.PeerSelection.DNS.DNSSemaphore
Closes #4606