niols' review

IntersectMBO · Apr 24, 2024 · c0ec3b6 · c0ec3b6
1 parent f951bf7
commit c0ec3b6
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 9 deletions.
diff --git a/...ros-consensus-diffusion/test/consensus-test/Test/Consensus/PeerSimulator/NodeLifecycle.hs b/...ros-consensus-diffusion/test/consensus-test/Test/Consensus/PeerSimulator/NodeLifecycle.hs
@@ -40,12 +40,17 @@ import           Test.Util.TestBlock (TestBlock, testInitExtLedger)
 
 -- | Resources used for a single live interval of the node, constructed when the
 -- node is started.
--- When the node is shut down, 'lnCopyToImmDb' is used to persist the immutable
+-- When the node is shut down, 'lnCopyToImmDb' is used to persist the current
 -- chain.
 data LiveNode blk m = LiveNode {
     lnChainDb          :: ChainDB m blk
   , lnStateViewTracers :: StateViewTracers blk m
   , lnStateTracer      :: Tracer m ()
+
+    -- | Write persistent ChainDB state (the immutable and volatile DBs, but not
+    -- the ledger and GSM state) to the VFS TVars to preserve it for the next
+    -- interval.
+    -- Returns the immutable tip's slot for tracing.
   , lnCopyToImmDb      :: m (WithOrigin SlotNo)
 
     -- | The set of peers that should be started.
@@ -65,9 +70,9 @@ data LiveIntervalResult blk = LiveIntervalResult {
   , lirActive      :: Set PeerId
   }
 
--- | Resources used by the handlers in 'NodeLifecycle' to shut down running
--- components, construct tracers used for single intervals, and reset and
--- persist state.
+-- | Resources used by the handlers 'lifecycleStart' and 'lifecycleStop' to
+-- shut down running components, construct tracers used for single intervals,
+-- and reset and persist state.
 data LiveResources blk m = LiveResources {
     lrRegistry :: ResourceRegistry m
   , lrPeerSim  :: PeerSimulatorResources m blk
@@ -98,6 +103,10 @@ data NodeLifecycle blk m = NodeLifecycle {
     -- | The minimum tick duration that triggers a node downtime.
     -- If this is 'Nothing', downtimes are disabled.
     nlMinDuration :: Maybe DiffTime
+
+    -- | Start the node with prior state.
+    -- For the first start, this must be called with an empty 'lirPeerResults'
+    -- and the initial set of all peers in 'lirActive'.
   , nlStart       :: LiveIntervalResult blk -> m (LiveNode blk m)
   , nlShutdown    :: LiveNode blk m -> m (LiveIntervalResult blk)
   }
@@ -135,7 +144,7 @@ mkChainDb resources = do
   where
     LiveResources {lrRegistry, lrTracer, lrConfig, lrCdb, lrLoEVar} = resources
 
--- | Allocate all the resources that depend on the the results of previous live
+-- | Allocate all the resources that depend on the results of previous live
 -- intervals, the ChainDB and its persisted state.
 restoreNode ::
   IOLike m =>

diff --git a/ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/PeerSimulator/Run.hs b/ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/PeerSimulator/Run.hs
@@ -211,7 +211,7 @@ dispatchTick :: forall m blk.
   LiveNode blk m ->
   (Int, (DiffTime, Peer (NodeState blk))) ->
   m (LiveNode blk m)
-dispatchTick tracer varHandles peers lifecycle node (number, (duration, Peer pid state)) = do
+dispatchTick tracer varHandles peers lifecycle node (number, (duration, Peer pid state)) =
   case peers Map.!? pid of
     Just PeerResources {prUpdateState} -> do
       traceNewTick
@@ -225,7 +225,6 @@ dispatchTick tracer varHandles peers lifecycle node (number, (duration, Peer pid
       | Just minInterval <- nlMinDuration
       , duration > minInterval
       = do
-        threadDelay 0.00001
         results <- nlShutdown node
         threadDelay duration
         nlStart results
@@ -319,9 +318,30 @@ startNode schedulerConfig genesisTest interval = do
       -- the registry is closed and all threads related to the peer are
       -- killed.
       withRegistry $ \peerRegistry -> do
-        (csClient, csServer) <- startChainSyncConnectionThread peerRegistry lrTracer lrConfig chainDbView fetchClientRegistry prShared prChainSync chainSyncTimeouts_ chainSyncLoPBucketConfig lnStateViewTracers handles
+        (csClient, csServer) <-
+          startChainSyncConnectionThread
+          peerRegistry
+          lrTracer
+          lrConfig
+          chainDbView
+          fetchClientRegistry
+          prShared
+          prChainSync
+          chainSyncTimeouts_
+          chainSyncLoPBucketConfig
+          lnStateViewTracers
+          handles
         BlockFetch.startKeepAliveThread peerRegistry fetchClientRegistry pid
-        (bfClient, bfServer) <- startBlockFetchConnectionThread peerRegistry lrTracer lnStateViewTracers fetchClientRegistry (pure Continue) prShared prBlockFetch blockFetchTimeouts_
+        (bfClient, bfServer) <-
+          startBlockFetchConnectionThread
+          peerRegistry
+          lrTracer
+          lnStateViewTracers
+          fetchClientRegistry
+          (pure Continue)
+          prShared
+          prBlockFetch
+          blockFetchTimeouts_
         waitAnyThread [csClient, csServer, bfClient, bfServer]
   -- The block fetch logic needs to be started after the block fetch clients
   -- otherwise, an internal assertion fails because getCandidates yields more