Address Review Feedback

IntersectMBO · Dec 2, 2024 · c14d5cb · c14d5cb
1 parent fee4982
commit c14d5cb
Show file tree

Hide file tree

Showing 26 changed files with 179 additions and 177 deletions.
diff --git a/ouroboros-network/ouroboros-network.cabal b/ouroboros-network/ouroboros-network.cabal
@@ -79,7 +79,7 @@ library
     Ouroboros.Network.Diffusion
     Ouroboros.Network.Diffusion.Common
     Ouroboros.Network.Diffusion.Configuration
-    Ouroboros.Network.Diffusion.MinimalP2P
+    Ouroboros.Network.Diffusion.P2P
     Ouroboros.Network.Diffusion.NonP2P
     Ouroboros.Network.Diffusion.Policies
     Ouroboros.Network.ExitPolicy

diff --git a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Diffusion/Testnet/Cardano.hs b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Diffusion/Testnet/Cardano.hs
@@ -1594,7 +1594,7 @@ prop_connect_failure (AbsIOError ioerr) =
          $ evs
     ) noEvents absInfo script
   where
-    -- must be in sync with rethrowPolicy in `Ouroboros.Network.Diffusion.MinimalP2P`
+    -- must be in sync with rethrowPolicy in `Ouroboros.Network.Diffusion.P2P`
     isFatal :: IOErrorType -> Bool
     isFatal ResourceExhausted    = True
     isFatal UnsupportedOperation = True

diff --git a/...oros-network/sim-tests-lib/Test/Ouroboros/Network/Diffusion/Testnet/Cardano/Simulation.hs b/...oros-network/sim-tests-lib/Test/Ouroboros/Network/Diffusion/Testnet/Cardano/Simulation.hs
@@ -1228,7 +1228,7 @@ diffusionSimulation
           cardanoExtraArgs :: CardanoArgumentsExtra m
           cardanoExtraArgs =
             CardanoArgumentsExtra {
-              caeSyncPeerTargets                  = snd peerTargets
+              caeGenesisPeerTargets               = snd peerTargets
             , caeReadUseBootstrapPeers            = readUseBootstrapPeers
             , caeMinBigLedgerPeersForTrustedState = defaultMinBigLedgerPeersForTrustedState
             , caeConsensusMode                    = consensusMode
@@ -1237,11 +1237,11 @@ diffusionSimulation
           cardanoChurnArgs :: CardanoPeerChurnArgs m
           cardanoChurnArgs =
             CardanoPeerChurnArgs {
-              cpcaModeVar             = churnModeVar
-            , cpcaReadFetchMode       = pure FetchModeDeadline
-            , cpcaSyncPeerTargets     = caeSyncPeerTargets cardanoExtraArgs
-            , cpcaReadUseBootstrap    = caeReadUseBootstrapPeers cardanoExtraArgs
-            , cpcaConsensusMode       = consensusMode
+              cpcaModeVar            = churnModeVar
+            , cpcaReadFetchMode      = pure FetchModeDeadline
+            , cpcaGenesisPeerTargets = caeGenesisPeerTargets cardanoExtraArgs
+            , cpcaReadUseBootstrap   = caeReadUseBootstrapPeers cardanoExtraArgs
+            , cpcaConsensusMode      = consensusMode
             }
 
           arguments :: Arguments (CardanoArgumentsExtra m) (CardanoPeerChurnArgs m) PeerTrustable m
@@ -1286,9 +1286,8 @@ diffusionSimulation
                      arguments
                      (CPST.empty consensusMode (MinBigLedgerPeersForTrustedState 0))
                      (cardanoExtraArgsToPeerSelectionActions cardanoExtraArgs)
-                     CPRP.empty
                      CPSV.empty
-                     CPRP.cardanoPublicRootPeersActions
+                     CPRP.cardanoPublicRootPeersAPI
                      (cardanoPeerSelectionGovernorArgs readUseLedgerPeers peerSharing (iLedgerPeersConsensusInterface interfaces))
                      CPSV.cardanoPeerSelectionStatetoCounters
                      requestPublicRootPeers'

diff --git a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Diffusion/Testnet/Minimal/Node.hs b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Diffusion/Testnet/Minimal/Node.hs
@@ -93,7 +93,7 @@ import Simulation.Network.Snocket (AddressType (..), FD)
 
 import GHC.Exception (Exception)
 import Ouroboros.Network.Diffusion.Common qualified as Common
-import Ouroboros.Network.Diffusion.MinimalP2P (runM)
+import Ouroboros.Network.Diffusion.P2P (runM)
 import Ouroboros.Network.PeerSelection.Governor.Types
            (PeerSelectionGovernorArgs)
 import Ouroboros.Network.PeerSelection.LedgerPeers (NumberOfPeers)
@@ -108,7 +108,7 @@ import Ouroboros.Network.PeerSelection.RelayAccessPoint (DomainAccessPoint,
 import Ouroboros.Network.PeerSelection.RootPeersDNS.DNSActions (DNSLookupType)
 import Ouroboros.Network.PeerSelection.State.LocalRootPeers (HotValency,
            WarmValency)
-import Ouroboros.Network.PeerSelection.Types (PublicExtraPeersActions (..))
+import Ouroboros.Network.PeerSelection.Types (PublicExtraPeersAPI (..))
 import Test.Ouroboros.Network.Diffusion.Node.ChainDB (addBlock,
            getBlockPointSet)
 import Test.Ouroboros.Network.Diffusion.Node.Kernel (NodeKernel (..), NtCAddr,
@@ -198,9 +198,8 @@ run :: forall extraArgs extraState extraActions extraAPI
     -> Arguments extraArgs extraChurnArgs extraFlags m
     -> extraState
     -> extraActions
-    -> extraPeers
     -> extraCounters
-    -> PublicExtraPeersActions extraPeers NtNAddr
+    -> PublicExtraPeersAPI extraPeers NtNAddr
     -> (forall muxMode responderCtx ntnVersionData bytes a b .
         PeerSelectionGovernorArgs
           extraState
@@ -244,9 +243,9 @@ run :: forall extraArgs extraState extraActions extraAPI
     -> Tracer m (TraceLabelPeer NtNAddr (TraceFetchClientState BlockHeader))
     -> m Void
 run blockGeneratorArgs limits ni na
-    emptyExtraState extraActions emptyExtraPeers
-    emptyExtraCounters extraPeersActions psArgs
-    psToExtraCounters requestPublicRootPeers peerChurnGovernor
+    emptyExtraState extraActions emptyExtraCounters
+    extraPeersAPI psArgs psToExtraCounters
+    requestPublicRootPeers peerChurnGovernor
     tracersExtra tracerBlockFetch =
     Node.withNodeKernelThread blockGeneratorArgs
       $ \ nodeKernel nodeKernelThread -> do
@@ -455,9 +454,8 @@ run blockGeneratorArgs limits ni na
       , Common.daBulkChurnInterval      = 300
       , Common.daReadLedgerPeerSnapshot = pure Nothing -- ^ tested independently
       , Common.daEmptyExtraState        = emptyExtraState
-      , Common.daEmptyExtraPeers        = emptyExtraPeers
       , Common.daEmptyExtraCounters     = emptyExtraCounters
-      , Common.daExtraPeersActions      = extraPeersActions
+      , Common.daExtraPeersAPI          = extraPeersAPI
       , Common.daExtraActions           = extraActions
       , Common.daExtraChurnArgs         = aExtraChurnArgs na
       , Common.daExtraArgs              = aExtraArgs na

diff --git a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection.hs b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection.hs
@@ -4018,14 +4018,12 @@ _governorFindingPublicRoots targetNumberOfRootPeers readDomains readUseBootstrap
         , updateWithState = const (const (pure ()))
         , extraDecisions  =
             ExtraGuardedDecisions {
-              preBlocking     =
-                [ \_ psa pst -> Cardano.monitorBootstrapPeersFlag   psa pst
-                , \_ psa pst -> Cardano.monitorLedgerStateJudgement psa pst
-                , \_ _   pst -> Cardano.waitForSystemToQuiesce          pst
-                ]
-            , postBlocking    = []
-            , preNonBlocking  = []
-            , postNonBlocking = []
+              preBlocking     = \_ psa pst ->
+                  Cardano.monitorBootstrapPeersFlag   psa pst
+                <> Cardano.monitorLedgerStateJudgement psa pst
+                <> Cardano.waitForSystemToQuiesce          pst
+            , postBlocking    = mempty
+            , postNonBlocking = mempty
             , requiredTargetsAction              = \_ -> Cardano.targetPeers
             , requiredLocalRootsAction           = \_ -> Cardano.localRoots
             , enableProgressMakingActions        = \st ->
@@ -4082,7 +4080,7 @@ _governorFindingPublicRoots targetNumberOfRootPeers readDomains readUseBootstrap
                   deactivatePeerConnection = error "deactivatePeerConnection",
                   closePeerConnection      = error "closePeerConnection"
                 },
-                readOriginalLocalRootPeers = return [],
+                readLocalRootPeersFromFile = return [],
                 readInboundPeers = pure Map.empty,
                 getLedgerStateCtx =
                   LedgerPeersConsensusInterface {
@@ -4096,14 +4094,14 @@ _governorFindingPublicRoots targetNumberOfRootPeers readDomains readUseBootstrap
                           writeTVar olocVar a
                     }
                   },
-                originalPeerSelectionTargets = targets,
+                peerSelectionTargets = targets,
                 readLedgerPeerSnapshot = pure Nothing,
                 extraActions = CardanoPeerSelectionActions {
-                  cpsaSyncPeerTargets = targets,
+                  cpsaGenesisPeerTargets = targets,
                   cpsaReadUseBootstrapPeers = readUseBootstrapPeers
                 },
                 extraStateToExtraCounters = CPSV.cardanoPeerSelectionStatetoCounters,
-                extraPeersActions = CPRP.cardanoPublicRootPeersActions
+                extraPeersAPI = CPRP.cardanoPublicRootPeersAPI
               }
 
     targets :: PeerSelectionTargets

diff --git a/...ros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/Cardano/MockEnvironment.hs b/...ros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/Cardano/MockEnvironment.hs
@@ -86,9 +86,10 @@ import Cardano.Network.LedgerPeerConsensusInterface
            (CardanoLedgerPeersConsensusInterface (..))
 import Cardano.Network.PeerSelection.Bootstrap (UseBootstrapPeers (..),
            requiresBootstrapPeers)
-import Cardano.Network.PeerSelection.Governor.Monitor (localRoots,
-           monitorBootstrapPeersFlag, monitorLedgerStateJudgement, targetPeers,
+import Cardano.Network.PeerSelection.Governor.Monitor
+           (monitorBootstrapPeersFlag, monitorLedgerStateJudgement,
            waitForSystemToQuiesce)
+import Cardano.Network.PeerSelection.Governor.Monitor qualified as Cardano
 import Cardano.Network.PeerSelection.Governor.PeerSelectionActions
            (CardanoPeerSelectionActions (..))
 import Cardano.Network.PeerSelection.Governor.PeerSelectionState
@@ -315,16 +316,14 @@ governorAction mockEnv@GovernorMockEnvironment {
         , updateWithState = const (const (pure ()))
         , extraDecisions  =
             ExtraGuardedDecisions {
-              preBlocking                        =
-                [ \_ psa pst -> monitorBootstrapPeersFlag   psa pst
-                , \_ psa pst -> monitorLedgerStateJudgement psa pst
-                , \_ _   pst -> waitForSystemToQuiesce          pst
-                ]
-            , postBlocking                       = []
-            , preNonBlocking                     = []
-            , postNonBlocking                    = []
-            , requiredTargetsAction              = \_ -> targetPeers
-            , requiredLocalRootsAction           = \_ -> localRoots
+              preBlocking                        = \_ psa pst ->
+                  monitorBootstrapPeersFlag   psa pst
+                <> monitorLedgerStateJudgement psa pst
+                <> waitForSystemToQuiesce          pst
+            , postBlocking                       = mempty
+            , postNonBlocking                    = mempty
+            , requiredTargetsAction              = \_ -> Cardano.targetPeers
+            , requiredLocalRootsAction           = \_ -> Cardano.localRoots
 
               -- No inbound peers should be used when the node is using bootstrap peers.
             , enableProgressMakingActions        =
@@ -505,7 +504,7 @@ mockPeerSelectionActions' tracer
                           connsVar
                           outboundConnectionsStateVar =
     PeerSelectionActions {
-      readOriginalLocalRootPeers
+      readLocalRootPeersFromFile
         = return
         $ LocalRootPeers.toGroups
         $ LocalRootPeers.mapPeers
@@ -540,12 +539,12 @@ mockPeerSelectionActions' tracer
       },
       readInboundPeers = pure Map.empty,
       readLedgerPeerSnapshot = pure Nothing,
-      originalPeerSelectionTargets = originalPeerTargets,
+      peerSelectionTargets = originalPeerTargets,
       extraActions = CardanoPeerSelectionActions {
-        cpsaSyncPeerTargets = peerTargets,
+        cpsaGenesisPeerTargets = peerTargets,
         cpsaReadUseBootstrapPeers = readUseBootstrapPeers
       },
-      extraPeersActions = CPRP.cardanoPublicRootPeersActions,
+      extraPeersAPI = CPRP.cardanoPublicRootPeersAPI,
       extraStateToExtraCounters = CPSV.cardanoPeerSelectionStatetoCounters
     }
   where

diff --git a/ouroboros-network/src/Cardano/Network/ArgumentsExtra.hs b/ouroboros-network/src/Cardano/Network/ArgumentsExtra.hs
@@ -11,8 +11,9 @@ import Ouroboros.Network.PeerSelection.Governor.Types
 --
 data CardanoArgumentsExtra m =
   CardanoArgumentsExtra {
-    -- | selection targets for the peer governor
-    caeSyncPeerTargets                  :: PeerSelectionTargets
+    -- | Genesis selection targets for the peer governor
+    caeGenesisPeerTargets               :: PeerSelectionTargets
+
   , caeReadUseBootstrapPeers            :: STM m UseBootstrapPeers
 
   -- | For Genesis, this sets the floor for minimum number of

diff --git a/ouroboros-network/src/Cardano/Network/PeerSelection/Governor/Monitor.hs b/ouroboros-network/src/Cardano/Network/PeerSelection/Governor/Monitor.hs
@@ -72,12 +72,12 @@ targetPeers :: (MonadSTM m, Ord peeraddr)
             => PeerSelectionActions CardanoPeerSelectionState (CardanoPeerSelectionActions m) extraPeers extraFlags extraAPI extraCounters peeraddr peerconn m
             -> PeerSelectionState CardanoPeerSelectionState PeerTrustable extraPeers peeraddr peerconn
             -> Guarded (STM m) (TimedDecision m CardanoPeerSelectionState PeerTrustable extraPeers peeraddr peerconn)
-targetPeers PeerSelectionActions{ originalPeerSelectionTargets,
+targetPeers PeerSelectionActions{ peerSelectionTargets,
                                   readPeerSelectionTargets,
                                   extraActions = CardanoPeerSelectionActions {
-                                    cpsaSyncPeerTargets
+                                    cpsaGenesisPeerTargets
                                   },
-                                  extraPeersActions
+                                  extraPeersAPI
                                 }
             st@PeerSelectionState{
               publicRootPeers,
@@ -104,11 +104,11 @@ targetPeers PeerSelectionActions{ originalPeerSelectionTargets,
       let targets' =
             case (cpstLedgerStateJudgement, cpstConsensusMode) of
               (YoungEnough, GenesisMode)
-                | churnTargets == cpsaSyncPeerTargets ->
-                  originalPeerSelectionTargets
+                | churnTargets == cpsaGenesisPeerTargets ->
+                  peerSelectionTargets
               (TooOld, GenesisMode)
-                | churnTargets == originalPeerSelectionTargets ->
-                  cpsaSyncPeerTargets
+                | churnTargets == peerSelectionTargets ->
+                  cpsaGenesisPeerTargets
               _otherwise -> churnTargets
 
       -- nb. first check is redundant in Genesis mode
@@ -141,7 +141,7 @@ targetPeers PeerSelectionActions{ originalPeerSelectionTargets,
 
           -- We have to enforce that local and big ledger peers are disjoint.
           publicRootPeers' =
-            PublicRootPeers.difference (differenceExtraPeers extraPeersActions)
+            PublicRootPeers.difference (differenceExtraPeers extraPeersAPI)
               publicRootPeers (LocalRootPeers.keysSet localRootPeers')
 
       return $ \_now -> Decision {
@@ -169,7 +169,7 @@ localRoots :: forall extraActions extraAPI extraCounters peeraddr peerconn m.
            -> PeerSelectionState CardanoPeerSelectionState PeerTrustable (CardanoPublicRootPeers peeraddr) peeraddr peerconn
            -> Guarded (STM m) (TimedDecision m CardanoPeerSelectionState PeerTrustable (CardanoPublicRootPeers peeraddr) peeraddr peerconn)
 localRoots actions@PeerSelectionActions{ readLocalRootPeers
-                                       , extraPeersActions
+                                       , extraPeersAPI
                                        }
            st@PeerSelectionState{
              localRootPeers,
@@ -227,7 +227,7 @@ localRoots actions@PeerSelectionActions{ readLocalRootPeers
           -- that the local and public sets are non-overlapping.
           --
           publicRootPeers' =
-            PublicRootPeers.difference (differenceExtraPeers extraPeersActions)
+            PublicRootPeers.difference (differenceExtraPeers extraPeersAPI)
               publicRootPeers
               localRootPeersSet
 
@@ -277,7 +277,7 @@ localRoots actions@PeerSelectionActions{ readLocalRootPeers
       return $ \_now ->
 
           assert (Set.isSubsetOf
-                    (PublicRootPeers.toSet (extraPeersToSet extraPeersActions)
+                    (PublicRootPeers.toSet (extraPeersToSet extraPeersAPI)
                                            publicRootPeers')
                    (KnownPeers.toSet knownPeers'))
         . assert (Set.isSubsetOf
@@ -344,7 +344,7 @@ monitorBootstrapPeersFlag :: ( MonadSTM m
                           -> PeerSelectionState CardanoPeerSelectionState extraFlags (CardanoPublicRootPeers peeraddr) peeraddr peerconn
                           -> Guarded (STM m) (TimedDecision m CardanoPeerSelectionState extraFlags (CardanoPublicRootPeers peeraddr) peeraddr peerconn)
 monitorBootstrapPeersFlag PeerSelectionActions { extraActions = CardanoPeerSelectionActions { cpsaReadUseBootstrapPeers }
-                                               , extraPeersActions
+                                               , extraPeersAPI
                                                }
                           st@PeerSelectionState { knownPeers
                                                 , establishedPeers
@@ -377,7 +377,7 @@ monitorBootstrapPeersFlag PeerSelectionActions { extraActions = CardanoPeerSelec
                    nonEstablishedBootstrapPeers
                    knownPeers
              , publicRootPeers =
-                 PublicRootPeers.difference (differenceExtraPeers extraPeersActions)
+                 PublicRootPeers.difference (differenceExtraPeers extraPeersAPI)
                    publicRootPeers
                    nonEstablishedBootstrapPeers
              , extraState = cpst {
@@ -417,7 +417,7 @@ monitorLedgerStateJudgement PeerSelectionActions{
                                   clpciGetLedgerStateJudgement = readLedgerStateJudgement
                                 }
                               }
-                            , extraPeersActions
+                            , extraPeersAPI
                             }
                             st@PeerSelectionState{ publicRootPeers,
                                                    knownPeers,
@@ -493,7 +493,7 @@ monitorLedgerStateJudgement PeerSelectionActions{
                   nonEstablishedBootstrapPeers
                   knownPeers
             , publicRootPeers =
-                PublicRootPeers.difference (differenceExtraPeers extraPeersActions)
+                PublicRootPeers.difference (differenceExtraPeers extraPeersAPI)
                   publicRootPeers
                   nonEstablishedBootstrapPeers
             , publicRootBackoffs = 0

diff --git a/ouroboros-network/src/Cardano/Network/PeerSelection/Governor/PeerSelectionActions.hs b/ouroboros-network/src/Cardano/Network/PeerSelection/Governor/PeerSelectionActions.hs
@@ -16,7 +16,7 @@ data CardanoPeerSelectionActions m =
     -- | Retrieve peer targets for Genesis & non-Genesis modes
     -- from node's configuration for the current state
     --
-    cpsaSyncPeerTargets       :: PeerSelectionTargets
+    cpsaGenesisPeerTargets    :: PeerSelectionTargets
 
     -- | Read the current bootstrap peers flag
   , cpsaReadUseBootstrapPeers :: STM m UseBootstrapPeers
@@ -25,11 +25,11 @@ data CardanoPeerSelectionActions m =
 cardanoExtraArgsToPeerSelectionActions :: CardanoArgumentsExtra m
                                        -> CardanoPeerSelectionActions m
 cardanoExtraArgsToPeerSelectionActions CardanoArgumentsExtra {
-                                         caeSyncPeerTargets
+                                         caeGenesisPeerTargets
                                        , caeReadUseBootstrapPeers
                                        } =
   CardanoPeerSelectionActions {
-    cpsaSyncPeerTargets       = caeSyncPeerTargets
+    cpsaGenesisPeerTargets       = caeGenesisPeerTargets
   , cpsaReadUseBootstrapPeers = caeReadUseBootstrapPeers
   }