TOSQUASH add higher-level comments about clocks

ouroboros-consensus/test/consensus-test/Test/Consensus/MiniProtocol/ChainSync/Client.hs

@@ -22,6 +22,25 @@
 -- them to be thrown based on the mock state changes (exceptions such as
 -- "fork is deep", "up-stream node asked for an invalid rollback", etc.).
 --
+-- The client's (simulated) wall-clock matters in this test because the
+-- ChainSync client has special handling for headers that arrive before the
+-- wall-clock reaches the onset of the header's claimed slot, which is
+-- inevitable even with only honest peers due to non-global clocks
+-- drifting/etc. This test advances time in a way that is unrealistic but does
+-- allow for some headers to arrive early (but not so early that the client
+-- disconnects from the server).
+--
+-- The approach to the clocks is as follows. A logical clock drives the whole
+-- test; it ticks along the naturals. Each tick causes the local and upstream
+-- chains to update and that's the primary content of the whole test. However,
+-- the /first/ thing that happens at the start of each logical tick is the
+-- client's simulated wall-clock advances (via a single 'threadDelay' call) to
+-- the onset of the greatest slot involved in any of that logical tick's chain
+-- updates /less/ the randomly-chosen local clock skew. Thus, if the greatest
+-- header involved in some logical tick is part of an upstream chain update,
+-- then it will arrive as a near-future header. Finally, recall that the
+-- @io-sim@ layer means those delays happen nearly instantaneously with respect
+-- to the real world wall-clock.
 module Test.Consensus.MiniProtocol.ChainSync.Client (tests) where
 
 import           Cardano.Crypto.DSIGN.Mock
@@ -339,6 +358,11 @@ runChainSync skew securityParam (ClientUpdates clientUpdates)
           , systemTimeWait    = pure ()
           }
 
+        inFutureCheck :: InFutureCheck.HeaderInFutureCheck m TestBlock
+        inFutureCheck = InFutureCheck.realHeaderInFutureCheck skew systemTime
+          -- Note that this tests passes in the exact difference between the
+          -- client and server's clocks as the tolerable clock skew.
+
         client :: StrictTVar m (AnchoredFragment (Header TestBlock))
                -> Consensus ChainSyncClientPipelined
                     TestBlock
@@ -347,7 +371,7 @@ runChainSync skew securityParam (ClientUpdates clientUpdates)
                    (pipelineDecisionLowHighMark 10 20)
                    chainSyncTracer
                    nodeCfg
-                   (InFutureCheck.realHeaderInFutureCheck skew systemTime)
+                   inFutureCheck
                    chainDbView
                    (maxBound :: NodeToNodeVersion)
                    (return Continue)
@@ -380,6 +404,9 @@ runChainSync skew securityParam (ClientUpdates clientUpdates)
                     atomically $ writeTVar varLocalClock $ toSkewedOnset slot
                 _      -> error "impossible! bad mkClockUpdates"
 
+              -- TODO interleave the client and server chain update
+              -- applications in a more interesting way?
+
               -- Client
               doTick clientUpdates tick $ \chainUpdates ->
                 atomically $ modifyTVar varClientState $ updateClientState chainUpdates
@@ -589,6 +616,8 @@ data ChainSyncClientSetup = ChainSyncClientSetup
     -- ^ Blocks that are discovered to be invalid.
   , clientSlowBy  :: ClockSkew
     -- ^ The server's clock minus the client's clock.
+    --
+    -- This is also passed to the code-under-test as the tolerable clock skew.
   }
   deriving (Show)