From 07c233b9fc9373cd5292e8939566179f00fe1cc1 Mon Sep 17 00:00:00 2001 From: Marcin Szamotulski Date: Thu, 25 Apr 2024 16:53:31 +0200 Subject: [PATCH] peer-selection: added tests for promiscuous mode * Extended the `GovernorMockEnvironment` with a `TimedScript UseLedgerPeers`. * added `prop_governor_promiscuous_mode` --- .../Test/Ouroboros/Network/LedgerPeers.hs | 2 + .../Test/Ouroboros/Network/PeerSelection.hs | 89 +++++++++++++++++++ .../Network/PeerSelection/Instances.hs | 26 ++++-- .../Network/PeerSelection/MockEnvironment.hs | 75 ++++++++++++---- .../Network/PeerSelection/Governor.hs | 1 + 5 files changed, 169 insertions(+), 24 deletions(-) diff --git a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/LedgerPeers.hs b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/LedgerPeers.hs index 2009a0e8540..3aae41ed196 100644 --- a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/LedgerPeers.hs +++ b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/LedgerPeers.hs @@ -86,6 +86,8 @@ instance Arbitrary ArbitraryLedgerStateJudgement where shrink (ArbitraryLedgerStateJudgement TooOld) = [] +-- TODO: import the `SlotNo` instance from +-- `Test.Ouroboros.Network.PeerSelection.Instances` newtype ArbitrarySlotNo = ArbitrarySlotNo { getArbitrarySlotNo :: SlotNo diff --git a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection.hs b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection.hs index c82aa0b3eae..7341b2f67f6 100644 --- a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection.hs +++ b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection.hs @@ -190,6 +190,7 @@ tests = , testProperty "node uses ledger peers in non-sensitive mode" prop_governor_uses_ledger_peers ] + , testProperty "promiscuous mode" prop_governor_promiscuous_mode ] , testGroup "issues" [ testProperty "3233" prop_issue_3233 @@ -761,6 +762,7 @@ envEventCredits TraceEnvActivatePeer {} = 0 envEventCredits TraceEnvDeactivatePeer {} = 0 envEventCredits TraceEnvCloseConn {} = 0 +envEventCredits TraceEnvUseLedgerPeers {} = 30 envEventCredits TraceEnvSetLedgerStateJudgement {} = 30 envEventCredits TraceEnvSetUseBootstrapPeers {} = 30 @@ -3426,6 +3428,77 @@ prop_governor_uses_ledger_peers env = in counterexample (intercalate "\n" $ map show $ usesLedgerPeers) $ all snd usesLedgerPeers + +prop_governor_promiscuous_mode :: GovernorMockEnvironment -> Property +prop_governor_promiscuous_mode env = + let events = Signal.eventsFromListUpToTime (Time (10 * 60 * 60)) + . selectPeerSelectionTraceEvents + . runGovernorInMockEnvironment + $ env + + counters :: Signal (PeerSelectionSetsWithSizes PeerAddr) + counters = + selectGovState peerSelectionStateToView events + + -- accumulate local roots + localRoots :: Signal (Set PeerAddr) + localRoots = + Signal.keyedUntil + (\case + Just (GovernorEvent (TraceLocalRootPeersChanged a _)) -> LocalRootPeers.keysSet a + Just (MockEnvEvent (TraceEnvSetLocalRoots a)) -> LocalRootPeers.keysSet a + _ -> Set.empty + ) + (\_ -> Set.empty) + (\_ -> False) + . Signal.fromChangeEvents Nothing + . fmap Just + $ events + + publicRoots :: Signal (Set PeerAddr) + publicRoots = + Signal.keyedUntil + PublicRootPeers.toSet + (\_ -> Set.empty) + (\_ -> False) + . selectGovState Governor.publicRootPeers + $ events + + promiscuousMode :: Signal Bool + promiscuousMode = + Signal.fromChangeEvents True + $ selectGovPromiscuousMode events + + in counterexample (intercalate "\n" $ show <$> Signal.eventsToList events) + $ signalProperty 20 show + (\(cs, localRootSet, publicRootSet, pm) -> + if not pm + then + -- we need to remove local and public roots. They are changing + -- over time, and a node might keep using them, event though the + -- node is configured as a non-promiscuous node. + -- + -- This makes this test only effective if a node starts in + -- non-promiscuous mode, until it is reconfigured. This can + -- discover some bugs in `readPromiscuousMode` but certainly not + -- all. + -- + -- TODO: write a more effective test. + Set.null (fst (viewKnownBootstrapPeers cs) + Set.\\ localRootSet + Set.\\ publicRootSet) + && Set.null (fst (viewKnownBigLedgerPeers cs) + Set.\\ localRootSet + Set.\\ publicRootSet) + && Set.null (fst (viewKnownSharedPeers cs) + Set.\\ localRootSet + Set.\\ publicRootSet) + else True) + ((,,,) <$> counters + <*> localRoots + <*> publicRoots + <*> promiscuousMode) + -- -- Utils for properties -- @@ -3444,6 +3517,18 @@ selectGovEvents = Signal.selectEvents (\case GovernorEvent e -> Just $! e _ -> Nothing) +selectGovCounters :: Events TestTraceEvent + -> Events PeerSelectionCounters +selectGovCounters = Signal.selectEvents + (\case GovernorCounters e -> Just $! e + _ -> Nothing) + +selectGovPromiscuousMode :: Events TestTraceEvent + -> Events Bool +selectGovPromiscuousMode = Signal.selectEvents + (\case GovernorPromiscuousMode e -> Just $! e + _ -> Nothing) + selectGovState :: Eq a => (forall peerconn. Governor.PeerSelectionState PeerAddr peerconn -> a) -> Events TestTraceEvent @@ -3601,6 +3686,7 @@ prop_issue_3550 = prop_governor_target_established_below defaultMaxTime $ pickColdPeersToForget = Script (PickFirst :| []), peerSharingFlag = PeerSharingEnabled, useBootstrapPeers = Script ((DontUseBootstrapPeers, NoDelay) :| []), + useLedgerPeers = Script ((UseLedgerPeers Always, NoDelay) :| []), ledgerStateJudgement = Script ((YoungEnough, NoDelay) :| []) } @@ -3637,6 +3723,7 @@ prop_issue_3515 = prop_governor_nolivelock $ pickColdPeersToForget = Script (PickFirst :| []), peerSharingFlag = PeerSharingEnabled, useBootstrapPeers = Script ((DontUseBootstrapPeers, NoDelay) :| []), + useLedgerPeers = Script ((UseLedgerPeers Always, NoDelay) :| []), ledgerStateJudgement = Script ((YoungEnough, NoDelay) :| []) } @@ -3673,6 +3760,7 @@ prop_issue_3494 = prop_governor_nofail $ pickColdPeersToForget = Script (PickFirst :| []), peerSharingFlag = PeerSharingEnabled, useBootstrapPeers = Script ((DontUseBootstrapPeers, NoDelay) :| []), + useLedgerPeers = Script ((UseLedgerPeers Always, NoDelay) :| []), ledgerStateJudgement = Script ((YoungEnough, NoDelay) :| []) } @@ -3725,6 +3813,7 @@ prop_issue_3233 = prop_governor_nolivelock $ pickColdPeersToForget = Script (PickFirst :| []), peerSharingFlag = PeerSharingEnabled, useBootstrapPeers = Script ((DontUseBootstrapPeers, NoDelay) :| []), + useLedgerPeers = Script ((UseLedgerPeers Always, NoDelay) :| []), ledgerStateJudgement = Script ((YoungEnough, NoDelay) :| []) } diff --git a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/Instances.hs b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/Instances.hs index 80ce4b454d2..5023eeb3f78 100644 --- a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/Instances.hs +++ b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/Instances.hs @@ -1,6 +1,8 @@ +{-# LANGUAGE DerivingVia #-} {-# LANGUAGE GeneralizedNewtypeDeriving #-} {-# LANGUAGE NamedFieldPuns #-} {-# LANGUAGE OverloadedStrings #-} +{-# LANGUAGE StandaloneDeriving #-} {-# OPTIONS_GHC -Wno-orphans #-} @@ -16,19 +18,22 @@ module Test.Ouroboros.Network.PeerSelection.Instances ) where import Data.Text.Encoding (encodeUtf8) -import Data.Word (Word32) +import Data.Word (Word32, Word64) + +import Cardano.Slotting.Slot (SlotNo (..)) import Ouroboros.Network.PeerSelection.Governor import Data.Hashable import Data.IP qualified as IP +import Ouroboros.Network.PeerSelection.Bootstrap (UseBootstrapPeers (..)) +import Ouroboros.Network.PeerSelection.LedgerPeers.Type (AfterSlot (..), + UseLedgerPeers (..)) import Ouroboros.Network.PeerSelection.PeerAdvertise (PeerAdvertise (..)) import Ouroboros.Network.PeerSelection.PeerSharing (PeerSharing (..)) +import Ouroboros.Network.PeerSelection.PeerTrustable (PeerTrustable (..)) import Ouroboros.Network.PeerSelection.RelayAccessPoint (DomainAccessPoint (..), RelayAccessPoint (..)) - -import Ouroboros.Network.PeerSelection.Bootstrap (UseBootstrapPeers (..)) -import Ouroboros.Network.PeerSelection.PeerTrustable (PeerTrustable (..)) import Ouroboros.Network.Testing.Utils (ShrinkCarefully, prop_shrink_nonequal, prop_shrink_valid) import Test.QuickCheck @@ -51,7 +56,7 @@ instance Arbitrary PeerAddr where arbitrary = PeerAddr <$> arbitrarySizedNatural shrink _ = [] - +deriving via Word64 instance Arbitrary SlotNo instance Arbitrary PeerAdvertise where arbitrary = elements [ DoAdvertisePeer, DoNotAdvertisePeer ] @@ -64,6 +69,11 @@ instance Arbitrary PeerSharing where shrink PeerSharingDisabled = [] shrink PeerSharingEnabled = [PeerSharingDisabled] +instance Arbitrary AfterSlot where + arbitrary = oneof [ pure Always + , After <$> arbitrary + ] + instance Arbitrary UseBootstrapPeers where arbitrary = frequency [ (1, pure DontUseBootstrapPeers) , (1, UseBootstrapPeers <$> arbitrary) @@ -72,6 +82,12 @@ instance Arbitrary UseBootstrapPeers where shrink DontUseBootstrapPeers = [] shrink (UseBootstrapPeers _) = [DontUseBootstrapPeers] +instance Arbitrary UseLedgerPeers where + arbitrary = frequency + [ (2, pure DontUseLedgerPeers) + , (8, UseLedgerPeers <$> arbitrary) + ] + instance Arbitrary PeerTrustable where arbitrary = elements [ IsNotTrustable, IsTrustable ] diff --git a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/MockEnvironment.hs b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/MockEnvironment.hs index 28468f9116a..ba13f89fa12 100644 --- a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/MockEnvironment.hs +++ b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/MockEnvironment.hs @@ -60,6 +60,7 @@ import Control.Tracer (Tracer (..), contramap, traceWith) import Ouroboros.Network.ExitPolicy import Ouroboros.Network.PeerSelection.Governor hiding (PeerSelectionState (..)) +import Ouroboros.Network.PeerSelection.Governor qualified as Governor import Ouroboros.Network.PeerSelection.State.LocalRootPeers qualified as LocalRootPeers import Ouroboros.Network.Testing.Data.Script (PickScript, Script (..), @@ -138,6 +139,7 @@ data GovernorMockEnvironment = GovernorMockEnvironment { pickColdPeersToForget :: !(PickScript PeerAddr), peerSharingFlag :: !PeerSharing, useBootstrapPeers :: !(TimedScript UseBootstrapPeers), + useLedgerPeers :: !(TimedScript UseLedgerPeers), ledgerStateJudgement :: !(TimedScript LedgerStateJudgement) } deriving (Show, Eq) @@ -209,17 +211,23 @@ governorAction mockEnv = do publicStateVar <- makePublicPeerSelectionStateVar lsjVar <- playTimedScript (contramap TraceEnvSetLedgerStateJudgement tracerMockEnv) (ledgerStateJudgement mockEnv) + lpVar <- playTimedScript (contramap TraceEnvUseLedgerPeers tracerMockEnv) + (useLedgerPeers mockEnv) usbVar <- playTimedScript (contramap TraceEnvSetUseBootstrapPeers tracerMockEnv) (useBootstrapPeers mockEnv) debugStateVar <- StrictTVar.newTVarIO (emptyPeerSelectionState (mkStdGen 42)) countersVar <- StrictTVar.newTVarIO emptyPeerSelectionCounters policy <- mockPeerSelectionPolicy mockEnv - actions <- mockPeerSelectionActions tracerMockEnv mockEnv (readTVar usbVar) (readTVar lsjVar) policy + actions <- mockPeerSelectionActions tracerMockEnv mockEnv + (readTVar usbVar) + (readTVar lpVar) + (readTVar lsjVar) + policy let interfaces = PeerSelectionInterfaces { countersVar, publicStateVar, debugStateVar, - -- peer selection tests are not relying on `UseLedgerPeers` + -- TODO: peer selection tests are not relying on `UseLedgerPeers` readUseLedgerPeers = return DontUseLedgerPeers } @@ -228,7 +236,7 @@ governorAction mockEnv = do labelThisThread "outbound-governor" _ <- peerSelectionGovernor tracerTracePeerSelection - tracerDebugPeerSelection + (tracerDebugPeerSelection <> tracePromiscuousMode interfaces actions) tracerTracePeerSelectionCounters (mkStdGen 42) actions @@ -266,6 +274,7 @@ data TraceMockEnv = TraceEnvAddPeers !PeerGraph | TraceEnvPeersStatus !(Map PeerAddr PeerStatus) | TraceEnvSetUseBootstrapPeers !UseBootstrapPeers | TraceEnvSetLedgerStateJudgement !LedgerStateJudgement + | TraceEnvUseLedgerPeers !UseLedgerPeers deriving Show mockPeerSelectionActions :: forall m. @@ -274,6 +283,7 @@ mockPeerSelectionActions :: forall m. => Tracer m TraceMockEnv -> GovernorMockEnvironment -> STM m UseBootstrapPeers + -> STM m UseLedgerPeers -> STM m LedgerStateJudgement -> PeerSelectionPolicy PeerAddr m -> m (PeerSelectionActions PeerAddr (PeerConn m) m) @@ -285,6 +295,7 @@ mockPeerSelectionActions tracer targets } readUseBootstrapPeers + readUseLedgerPeers getLedgerStateJudgement policy = do scripts <- Map.fromList <$> @@ -314,7 +325,12 @@ mockPeerSelectionActions tracer traceWith tracer (TraceEnvSetPublicRoots publicRootPeers) --TODO: make dynamic return $ mockPeerSelectionActions' tracer env policy - scripts targetsVar readUseBootstrapPeers getLedgerStateJudgement peerConns onlyLocalOutboundConnsVar + scripts targetsVar + readUseBootstrapPeers + readUseLedgerPeers + getLedgerStateJudgement + peerConns + onlyLocalOutboundConnsVar where proxy :: Proxy m proxy = Proxy @@ -337,6 +353,7 @@ mockPeerSelectionActions' :: forall m. -> Map PeerAddr (TVar m PeerShareScript, TVar m PeerSharingScript, TVar m ConnectionScript) -> TVar m PeerSelectionTargets -> STM m UseBootstrapPeers + -> STM m UseLedgerPeers -> STM m LedgerStateJudgement -> TVar m (Map PeerAddr (TVar m PeerStatus)) -> TVar m OutboundConnectionsState @@ -351,6 +368,7 @@ mockPeerSelectionActions' tracer scripts targetsVar readUseBootstrapPeers + readUseLedgerPeers readLedgerStateJudgement connsVar outboundConnectionsStateVar = @@ -390,6 +408,7 @@ mockPeerSelectionActions' tracer usingBootstrapPeers <- atomically $ requiresBootstrapPeers <$> readUseBootstrapPeers <*> readLedgerStateJudgement + useLedgerPeers <- atomically readUseLedgerPeers -- If the ledger state is YoungEnough we should get ledger peers. -- Otherwise we should get bootstrap peers let publicConfigPeers = PublicRootPeers.getPublicConfigPeers publicRootPeers @@ -399,17 +418,19 @@ mockPeerSelectionActions' tracer result = if usingBootstrapPeers then PublicRootPeers.fromBootstrapPeers bootstrapPeers - else case ledgerPeersKind of - AllLedgerPeers - | Set.null ledgerPeers -> - PublicRootPeers.fromPublicRootPeers publicConfigPeers - | otherwise -> - PublicRootPeers.fromLedgerPeers ledgerPeers - BigLedgerPeers - | Set.null ledgerPeers -> - PublicRootPeers.fromPublicRootPeers publicConfigPeers - | otherwise -> - PublicRootPeers.fromBigLedgerPeers bigLedgerPeers + else case useLedgerPeers of + DontUseLedgerPeers -> PublicRootPeers.empty + UseLedgerPeers _ -> case ledgerPeersKind of + AllLedgerPeers + | Set.null ledgerPeers -> + PublicRootPeers.fromPublicRootPeers publicConfigPeers + | otherwise -> + PublicRootPeers.fromLedgerPeers ledgerPeers + BigLedgerPeers + | Set.null ledgerPeers -> + PublicRootPeers.fromPublicRootPeers publicConfigPeers + | otherwise -> + PublicRootPeers.fromBigLedgerPeers bigLedgerPeers traceWith tracer (TraceEnvRootsResult (Set.toList (PublicRootPeers.toSet result))) return (result, ttl) @@ -599,10 +620,11 @@ mockPeerSelectionPolicy GovernorMockEnvironment { -- Utils for properties -- -data TestTraceEvent = GovernorDebug !(DebugPeerSelection PeerAddr) - | GovernorEvent !(TracePeerSelection PeerAddr) - | GovernorCounters !PeerSelectionCounters - | MockEnvEvent !TraceMockEnv +data TestTraceEvent = GovernorDebug !(DebugPeerSelection PeerAddr) + | GovernorEvent !(TracePeerSelection PeerAddr) + | GovernorCounters !PeerSelectionCounters + | GovernorPromiscuousMode !Bool + | MockEnvEvent !TraceMockEnv -- Warning: be careful with writing properties that rely -- on trace events from both the governor and from the -- environment. These events typically occur in separate @@ -678,6 +700,16 @@ tracerTracePeerSelection = contramap f tracerTestTraceEvent tracerDebugPeerSelection :: Tracer (IOSim s) (DebugPeerSelection PeerAddr) tracerDebugPeerSelection = GovernorDebug `contramap` tracerTestTraceEvent +tracePromiscuousMode :: PeerSelectionInterfaces PeerAddr (PeerConn (IOSim s)) (IOSim s) + -> PeerSelectionActions PeerAddr (PeerConn (IOSim s)) (IOSim s) + -> Tracer (IOSim s) (DebugPeerSelection PeerAddr) +tracePromiscuousMode interfaces actions = Tracer $ \(TraceGovernorState _ _ st) -> do + promiscuousMode <- atomically $ readPromiscuousMode interfaces + (Governor.peerSharing actions) + (Governor.bootstrapPeersFlag st) + (peerSelectionStateToView st) + traceWith tracerTestTraceEvent (GovernorPromiscuousMode promiscuousMode) + tracerTracePeerSelectionCounters :: Tracer (IOSim s) PeerSelectionCounters tracerTracePeerSelectionCounters = contramap GovernorCounters tracerTestTraceEvent @@ -760,6 +792,7 @@ instance Arbitrary GovernorMockEnvironment where pickColdPeersToForget <- arbitraryPickScript arbitrarySubsetOfPeers peerSharingFlag <- arbitrary useBootstrapPeers <- arbitrary + useLedgerPeers <- arbitrary ledgerStateJudgementList <- fmap getArbitraryLedgerStateJudgement <$> arbitrary ledgerStateJudgementDelays <- listOf1 (elements [NoDelay, ShortDelay]) let ledgerStateJudgementWithDelay = @@ -841,6 +874,7 @@ instance Arbitrary GovernorMockEnvironment where pickColdPeersToForget, peerSharingFlag, useBootstrapPeers, + useLedgerPeers, ledgerStateJudgement } = -- Special rule for shrinking the peerGraph because the localRootPeers @@ -883,6 +917,9 @@ instance Arbitrary GovernorMockEnvironment where ++ [ env { useBootstrapPeers = useBootstrapPeers' } | useBootstrapPeers' <- shrink useBootstrapPeers ] + ++ [ env { useLedgerPeers = useLedgerPeers' } + | useLedgerPeers' <- shrink useLedgerPeers + ] ++ [ env { ledgerStateJudgement = fmap (first getArbitraryLedgerStateJudgement) ledgerStateJudgement' } | ledgerStateJudgement' <- shrink (fmap (first ArbitraryLedgerStateJudgement) ledgerStateJudgement) ] diff --git a/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor.hs b/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor.hs index de910e8fc1b..8d549780fd2 100644 --- a/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor.hs +++ b/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor.hs @@ -46,6 +46,7 @@ module Ouroboros.Network.PeerSelection.Governor , emptyPeerSelectionState , ChurnMode (..) , peerSelectionStateToView + , readPromiscuousMode ) where import Data.Foldable (traverse_)