Skip to content

Commit 62de5da

Browse files
cootcoot
committed
connection-manager: use ConnStateId to identifiy connectionsm
We cannot use `ConnectionId` since it's unknown for some transitions, while `ConnStateId` has the same life time as a connection inside the connection-manager. The drawback is that it's might be more difficult to associate it with a given `ConnectionId`. If this will be the case, a trace needs to be wrapped in `WithName (Maybe (ConnectionId peerAddr))` or something similar.
1 parent 3b0e049 commit 62de5da

File tree

9 files changed

+141
-142
lines changed

9 files changed

+141
-142
lines changed

‎ouroboros-network-framework/sim-tests/Test/Ouroboros/Network/ConnectionManager.hs‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ import Test.Tasty.QuickCheck (testProperty)
5959

6060
import Ouroboros.Network.ConnectionId (ConnectionId (..))
6161
import Ouroboros.Network.ConnectionManager.Core qualified as CM
62+
import Ouroboros.Network.ConnectionManager.State qualified as CM
6263
import Ouroboros.Network.ConnectionManager.Test.Utils (verifyAbstractTransition)
6364
import Ouroboros.Network.ConnectionManager.Types
6465
import Ouroboros.Network.MuxMode
@@ -649,8 +650,8 @@ mkConnectionHandler snocket =
649650

650651
type TestConnectionState m = CM.ConnectionState Addr (Handle m) Void Version m
651652
type TestConnectionManagerTrace = CM.Trace Addr ()
652-
type TestTransitionTrace m = TransitionTrace Addr (TestConnectionState m)
653-
type TestAbstractTransitionTrace = AbstractTransitionTrace Addr
653+
type TestTransitionTrace m = TransitionTrace CM.ConnStateId (TestConnectionState m)
654+
type TestAbstractTransitionTrace = AbstractTransitionTrace CM.ConnStateId
654655

655656
newtype SkewedBool = SkewedBool Bool
656657
deriving Show
@@ -783,11 +784,7 @@ prop_valid_transitions (Fixed rnd) (SkewedBool bindToLocalAddress) scheduleMap =
783784
:: ConnectionManager Mx.InitiatorResponderMode (FD (IOSim s))
784785
Addr (Handle m) Void (IOSim s)) -> do
785786
fd <- open snocket TestFamily
786-
case myAddress of
787-
Just localAddr ->
788-
bind snocket fd localAddr
789-
Nothing ->
790-
pure ()
787+
traverse_ (bind snocket fd) myAddress
791788

792789
let go :: HasCallStack
793790
=> [Async (IOSim s) ()]

‎ouroboros-network-framework/sim-tests/Test/Ouroboros/Network/Server2/Sim.hs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ import Network.Mux qualified as Mux
7474
import Ouroboros.Network.ConnectionHandler
7575
import Ouroboros.Network.ConnectionId
7676
import Ouroboros.Network.ConnectionManager.Core qualified as CM
77+
import Ouroboros.Network.ConnectionManager.State qualified as CM
7778
import Ouroboros.Network.ConnectionManager.Types
7879
import Ouroboros.Network.InboundGovernor qualified as IG
7980
import Ouroboros.Network.InboundGovernor.State (ConnectionState (..))
@@ -627,7 +628,7 @@ multinodeExperiment
627628
=> Tracer m (WithName (Name peerAddr)
628629
(RemoteTransitionTrace peerAddr))
629630
-> Tracer m (WithName (Name peerAddr)
630-
(AbstractTransitionTrace peerAddr))
631+
(AbstractTransitionTrace CM.ConnStateId))
631632
-> Tracer m (WithName (Name peerAddr)
632633
(IG.Trace peerAddr))
633634
-> Tracer m (WithName (Name peerAddr)
@@ -2242,7 +2243,7 @@ multiNodeSimTracer :: ( Alternative (STM m), Monad m, MonadFix m
22422243
-> Tracer m
22432244
(WithName (Name SimAddr) (RemoteTransitionTrace SimAddr))
22442245
-> Tracer m
2245-
(WithName (Name SimAddr) (AbstractTransitionTrace SimAddr))
2246+
(WithName (Name SimAddr) (AbstractTransitionTrace CM.ConnStateId))
22462247
-> Tracer m
22472248
(WithName (Name SimAddr) (IG.Trace SimAddr))
22482249
-> Tracer m

‎ouroboros-network-framework/src/Ouroboros/Network/ConnectionManager/Core.hs‎

Lines changed: 89 additions & 83 deletions
Large diffs are not rendered by default.

‎ouroboros-network-framework/src/Ouroboros/Network/ConnectionManager/State.hs‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1-
{-# LANGUAGE LambdaCase #-}
2-
{-# LANGUAGE NamedFieldPuns #-}
3-
{-# LANGUAGE ScopedTypeVariables #-}
4-
{-# LANGUAGE TypeApplications #-}
1+
{-# LANGUAGE DerivingStrategies #-}
2+
{-# LANGUAGE GeneralizedNewtypeDeriving #-}
3+
{-# LANGUAGE LambdaCase #-}
4+
{-# LANGUAGE NamedFieldPuns #-}
5+
{-# LANGUAGE ScopedTypeVariables #-}
6+
{-# LANGUAGE TypeApplications #-}
57
-- Undecidable instances are need for 'Show' instance of 'ConnectionState'.
6-
{-# LANGUAGE QuantifiedConstraints #-}
8+
{-# LANGUAGE QuantifiedConstraints #-}
79

810
module Ouroboros.Network.ConnectionManager.State
911
( -- * ConnectionManagerState API
@@ -146,21 +148,20 @@ newMutableConnState peerAddr freshIdSupply connState = do
146148
let prevAbs = abstractState (Known prev)
147149
, prevAbs /= currAbs -> pure
148150
$ TraceDynamic
149-
$ WithName connStateId
150-
$ TransitionTrace peerAddr
151-
$ mkAbsTransition prevAbs
152-
currAbs
151+
( WithName peerAddr
152+
(TransitionTrace connStateId $ mkAbsTransition prevAbs currAbs)
153+
:: ConnectionTransitionTrace peerAddr)
153154
Nothing -> pure
154155
$ TraceDynamic
155-
$ WithName connStateId
156-
$ TransitionTrace peerAddr
156+
( WithName peerAddr
157+
$ TransitionTrace connStateId
157158
$ mkAbsTransition TerminatedSt
158159
currAbs
160+
:: ConnectionTransitionTrace peerAddr)
159161
_ -> pure DontTrace
160162
)
161163
return $ MutableConnState { connStateId, connVar }
162164

163-
164165
abstractState :: MaybeUnknown (ConnectionState muxMode peerAddr m a b)
165166
-> AbstractState
166167
abstractState = \case

‎ouroboros-network-framework/src/Ouroboros/Network/ConnectionManager/Types.hs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -890,8 +890,8 @@ mkAbsTransition from to = Transition { fromState = from
890890
, toState = to
891891
}
892892

893-
data TransitionTrace' peerAddr state = TransitionTrace
894-
{ ttPeerAddr :: peerAddr -- TODO: use ConnectionId
893+
data TransitionTrace' id state = TransitionTrace
894+
{ ttPeerAddr :: id -- ^ an id of a connection, not necessarily an address, e.g. `State.ConnStateId`
895895
, ttTransition :: Transition' state
896896
}
897897
deriving Functor

‎ouroboros-network-framework/testlib/Ouroboros/Network/ConnectionManager/Test/Experiments.hs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ import Network.TypedProtocol.ReqResp.Type as ReqResp
7474
import Ouroboros.Network.ConnectionHandler
7575
import Ouroboros.Network.ConnectionId
7676
import Ouroboros.Network.ConnectionManager.Core qualified as CM
77+
import Ouroboros.Network.ConnectionManager.State qualified as CM
7778
import Ouroboros.Network.ConnectionManager.Types
7879
import Ouroboros.Network.Context
7980
import Ouroboros.Network.ControlMessage
@@ -250,7 +251,7 @@ withInitiatorOnlyConnectionManager
250251
=> name
251252
-- ^ identifier (for logging)
252253
-> Timeouts
253-
-> Tracer m (WithName name (AbstractTransitionTrace peerAddr))
254+
-> Tracer m (WithName name (AbstractTransitionTrace CM.ConnStateId))
254255
-> Tracer m (WithName name
255256
(CM.Trace
256257
peerAddr
@@ -419,7 +420,7 @@ withBidirectionalConnectionManager
419420
-> Timeouts
420421
-- ^ identifier (for logging)
421422
-> Tracer m (WithName name (RemoteTransitionTrace peerAddr))
422-
-> Tracer m (WithName name (AbstractTransitionTrace peerAddr))
423+
-> Tracer m (WithName name (AbstractTransitionTrace CM.ConnStateId))
423424
-> Tracer m (WithName name
424425
(CM.Trace
425426
peerAddr
@@ -597,7 +598,7 @@ withBidirectionalConnectionManager name timeouts
597598

598599

599600
reqRespSizeLimits :: forall req resp. ProtocolSizeLimits (ReqResp req resp)
600-
ByteString
601+
ByteString
601602
reqRespSizeLimits = ProtocolSizeLimits
602603
{ sizeLimitForState
603604
, dataSize = fromIntegral . LBS.length

‎ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Testnet.hs‎

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import Ouroboros.Network.BlockFetch (PraosFetchMode (..),
4949
import Ouroboros.Network.ConnectionHandler (ConnectionHandlerTrace)
5050
import Ouroboros.Network.ConnectionId
5151
import Ouroboros.Network.ConnectionManager.Core qualified as CM
52+
import Ouroboros.Network.ConnectionManager.State qualified as CM
5253
import Ouroboros.Network.ConnectionManager.Test.Timeouts (TestProperty (..),
5354
classifyActivityType, classifyEffectiveDataFlow,
5455
classifyNegotiatedDataFlow, classifyPrunings, classifyTermination,
@@ -376,16 +377,9 @@ prop_connection_manager_transitions_coverage defaultBearerInfo diffScript =
376377
diffScript
377378
iosimTracer
378379

379-
events :: [AbstractTransitionTrace NtNAddr]
380-
events = mapMaybe (\case DiffusionConnectionManagerTransitionTrace st ->
381-
Just st
382-
_ -> Nothing
383-
)
384-
. Trace.toList
385-
. fmap (\(WithTime _ (WithName _ b)) -> b)
386-
. withTimeNameTraceEvents
387-
@DiffusionTestTrace
388-
@NtNAddr
380+
events :: [AbstractTransitionTrace CM.ConnStateId]
381+
events = fmap (\((WithName _ b)) -> b)
382+
. selectTraceEventsDynamic' @_ @(CM.ConnectionTransitionTrace NtNAddr)
389383
. Trace.take 125000
390384
$ runSimTrace sim
391385

@@ -2937,7 +2931,7 @@ prop_diffusion_cm_valid_transitions ioSimTrace traceNumber =
29372931
where
29382932
verify_cm_valid_transitions :: Trace () DiffusionTestTrace -> Property
29392933
verify_cm_valid_transitions events =
2940-
let abstractTransitionEvents :: Trace () (AbstractTransitionTrace NtNAddr)
2934+
let abstractTransitionEvents :: Trace () (AbstractTransitionTrace CM.ConnStateId)
29412935
abstractTransitionEvents =
29422936
selectDiffusionConnectionManagerTransitionEvents events
29432937

@@ -3084,7 +3078,7 @@ prop_diffusion_cm_valid_transition_order ioSimTrace traceNumber =
30843078
where
30853079
verify_cm_valid_transition_order :: Trace () (WithName NtNAddr (WithTime DiffusionTestTrace)) -> Property
30863080
verify_cm_valid_transition_order events =
3087-
let abstractTransitionEvents :: Trace () (WithName NtNAddr (WithTime (AbstractTransitionTrace NtNAddr)))
3081+
let abstractTransitionEvents :: Trace () (WithName NtNAddr (WithTime (AbstractTransitionTrace CM.ConnStateId)))
30883082
abstractTransitionEvents =
30893083
selectDiffusionConnectionManagerTransitionEvents' events
30903084

@@ -3298,10 +3292,10 @@ prop_unit_reconnect =
32983292
diffScript
32993293
iosimTracer
33003294

3301-
events :: [Events DiffusionTestTrace]
3295+
events :: [Events (WithName NtNAddr DiffusionTestTrace)]
33023296
events = Trace.toList
33033297
. fmap ( Signal.eventsFromList
3304-
. fmap (\(WithName _ (WithTime t b)) -> (t, b))
3298+
. fmap (\(WithName addr (WithTime t b)) -> (t, WithName addr b))
33053299
)
33063300
. splitWithNameTrace
33073301
. fmap (\(WithTime t (WithName name b)) -> WithName name (WithTime t b))
@@ -3316,26 +3310,27 @@ prop_unit_reconnect =
33163310
<$> events
33173311

33183312
where
3319-
verify_consistency :: Events DiffusionTestTrace -> Property
3313+
verify_consistency :: Events (WithName NtNAddr DiffusionTestTrace) -> Property
33203314
verify_consistency events =
33213315
let govEstablishedPeersSig :: Signal (Set NtNAddr)
33223316
govEstablishedPeersSig =
33233317
selectDiffusionPeerSelectionState'
33243318
(EstablishedPeers.toSet . Governor.establishedPeers)
3325-
events
3319+
(wnEvent <$> events)
33263320

3327-
govConnectionManagerTransitionsSig :: [E (AbstractTransitionTrace NtNAddr)]
3321+
govConnectionManagerTransitionsSig :: [E (WithName NtNAddr (AbstractTransitionTrace CM.ConnStateId))]
33283322
govConnectionManagerTransitionsSig =
3329-
Signal.eventsToListWithId
3323+
Signal.eventsToListWithId
33303324
$ Signal.selectEvents
33313325
(\case
3332-
DiffusionConnectionManagerTransitionTrace tr -> Just tr
3333-
_ -> Nothing
3326+
WithName addr (DiffusionConnectionManagerTransitionTrace tr)
3327+
-> Just (WithName addr tr)
3328+
_ -> Nothing
33343329
) events
33353330

33363331
in conjoin
3337-
$ map (\(E ts a) -> case a of
3338-
TransitionTrace addr (Transition _ TerminatedSt) ->
3332+
$ map (\(E ts (WithName addr a)) -> case a of
3333+
TransitionTrace _ (Transition _ TerminatedSt) ->
33393334
eventually ts (Set.notMember addr) govEstablishedPeersSig
33403335
_ -> True -- TODO: Do the opposite
33413336
)
@@ -4043,7 +4038,7 @@ prop_diffusion_timeouts_enforced ioSimTrace traceNumber =
40434038
where
40444039
verify_timeouts :: Trace () (Time, DiffusionTestTrace) -> Property
40454040
verify_timeouts events =
4046-
let transitionSignal :: Trace (SimResult ()) [(Time, AbstractTransitionTrace NtNAddr)]
4041+
let transitionSignal :: Trace (SimResult ()) [(Time, AbstractTransitionTrace CM.ConnStateId)]
40474042
transitionSignal = Trace.fromList (MainReturn (Time 0) (Labelled (ThreadId []) (Just "main")) () [])
40484043
. Trace.toList
40494044
. groupConns snd abstractStateIsFinalTransition
@@ -4171,7 +4166,7 @@ selectTimedDiffusionPeerSelectionActionsEvents =
41714166

41724167
selectDiffusionConnectionManagerTransitionEvents
41734168
:: Trace () DiffusionTestTrace
4174-
-> Trace () (AbstractTransitionTrace NtNAddr)
4169+
-> Trace () (AbstractTransitionTrace CM.ConnStateId)
41754170
selectDiffusionConnectionManagerTransitionEvents =
41764171
Trace.fromList ()
41774172
. mapMaybe
@@ -4181,7 +4176,7 @@ selectDiffusionConnectionManagerTransitionEvents =
41814176

41824177
selectDiffusionConnectionManagerTransitionEvents'
41834178
:: Trace () (WithName NtNAddr (WithTime DiffusionTestTrace))
4184-
-> Trace () (WithName NtNAddr (WithTime (AbstractTransitionTrace NtNAddr)))
4179+
-> Trace () (WithName NtNAddr (WithTime (AbstractTransitionTrace CM.ConnStateId)))
41854180
selectDiffusionConnectionManagerTransitionEvents' =
41864181
Trace.fromList ()
41874182
. mapMaybe
@@ -4193,7 +4188,7 @@ selectDiffusionConnectionManagerTransitionEvents' =
41934188

41944189
selectDiffusionConnectionManagerTransitionEventsTime
41954190
:: Trace () (Time, DiffusionTestTrace)
4196-
-> Trace () (Time, AbstractTransitionTrace NtNAddr)
4191+
-> Trace () (Time, AbstractTransitionTrace CM.ConnStateId)
41974192
selectDiffusionConnectionManagerTransitionEventsTime =
41984193
Trace.fromList ()
41994194
. mapMaybe

‎ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Testnet/Internal.hs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import Network.TypedProtocol.PingPong.Type qualified as PingPong
7373

7474
import Ouroboros.Network.ConnectionHandler (ConnectionHandlerTrace)
7575
import Ouroboros.Network.ConnectionManager.Core qualified as CM
76+
import Ouroboros.Network.ConnectionManager.State qualified as CM
7677
import Ouroboros.Network.ConnectionManager.Types (AbstractTransitionTrace)
7778
import Ouroboros.Network.ConsensusMode
7879
import Ouroboros.Network.Diffusion.P2P qualified as Diff.P2P
@@ -921,7 +922,7 @@ data DiffusionTestTrace =
921922
(ConnectionHandlerTrace NtNVersion NtNVersionData))
922923
| DiffusionDiffusionSimulationTrace DiffusionSimulationTrace
923924
| DiffusionConnectionManagerTransitionTrace
924-
(AbstractTransitionTrace NtNAddr)
925+
(AbstractTransitionTrace CM.ConnStateId)
925926
| DiffusionInboundGovernorTransitionTrace
926927
(IG.RemoteTransitionTrace NtNAddr)
927928
| DiffusionInboundGovernorTrace (IG.Trace NtNAddr)
@@ -1289,11 +1290,7 @@ diffusionSimulation
12891290
. tracerWithName ntnAddr
12901291
. tracerWithTime
12911292
$ nodeTracer
1292-
, Diff.P2P.dtConnectionManagerTransitionTracer = contramap
1293-
DiffusionConnectionManagerTransitionTrace
1294-
. tracerWithName ntnAddr
1295-
. tracerWithTime
1296-
$ nodeTracer
1293+
, Diff.P2P.dtConnectionManagerTransitionTracer = nullTracer
12971294
, Diff.P2P.dtServerTracer = contramap DiffusionServerTrace
12981295
. tracerWithName ntnAddr
12991296
. tracerWithTime

‎ouroboros-network/src/Ouroboros/Network/Diffusion/P2P.hs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ import Ouroboros.Network.Socket (configureSocket, configureSystemdSocket)
8181

8282
import Ouroboros.Network.ConnectionHandler
8383
import Ouroboros.Network.ConnectionManager.Core qualified as CM
84+
import Ouroboros.Network.ConnectionManager.State qualified as CM
8485
import Ouroboros.Network.ConnectionManager.InformationChannel
8586
(newInformationChannel)
8687
import Ouroboros.Network.ConnectionManager.Types
@@ -188,7 +189,7 @@ data TracersExtra ntnAddr ntnVersion ntnVersionData
188189
ntnVersionData))
189190

190191
, dtConnectionManagerTransitionTracer
191-
:: Tracer m (AbstractTransitionTrace ntnAddr)
192+
:: Tracer m (AbstractTransitionTrace CM.ConnStateId)
192193

193194
, dtServerTracer
194195
:: Tracer m (Server.Trace ntnAddr)

0 commit comments

Comments
 (0)