CSJ: prefer the Objector when replacing a Dynamo (#1417)

See the various commits. The `CSJ: prefer the Objector when replacing a
Dynamo` commit in particular explains the key change within this PR.

Author: nfrisby

ouroboros-consensus-diffusion/changelog.d/20250307_082223_nick.frisby_debug_genesis_test_failure.md

@@ -0,0 +1,23 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Patch
+
+- A bullet item for the Patch category.
+
+-->
+<!--
+### Non-Breaking
+
+- A bullet item for the Non-Breaking category.
+
+-->
+
+### Breaking
+
+- Added a new CSJ tracer to ChainSync client interface.
+- Renamed the existing tracer in `Ouroboros.Consensus.Node.Tracers.Tracers`.

ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Network/NodeToNode.hs

@@ -581,6 +581,7 @@ mkApps kernel Tracers {..} mkCodecs ByteLimits {..} genChainSyncTimeout lopBucke
         (getFetchClientRegistry kernel) them $
         CsClient.bracketChainSyncClient
             (contramap (TraceLabelPeer them) (Node.chainSyncClientTracer (getTracers kernel)))
+            (contramap (TraceLabelPeer them) (Node.csjTracer             (getTracers kernel)))
             (CsClient.defaultChainDbView (getChainDB kernel))
             (getChainSyncHandles kernel)
             (getGsmState       kernel)

ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node/Tracers.hs

@@ -72,7 +72,8 @@ data Tracers' remotePeer localPeer blk f = Tracers
   , consensusErrorTracer          :: f SomeException
   , gsmTracer                     :: f (TraceGsmEvent (Tip blk))
   , gddTracer                     :: f (TraceGDDEvent remotePeer blk)
-  , csjTracer                     :: f (CSJumping.TraceEvent remotePeer)
+  , csjTracer                     :: f (TraceLabelPeer remotePeer (CSJumping.TraceEventCsj remotePeer blk))
+  , dbfTracer                     :: f (CSJumping.TraceEventDbf remotePeer)
   }
 
 instance (forall a. Semigroup (f a))
@@ -97,6 +98,7 @@ instance (forall a. Semigroup (f a))
       , gsmTracer                     = f gsmTracer
       , gddTracer                     = f gddTracer
       , csjTracer                     = f csjTracer
+      , dbfTracer                     = f dbfTracer
       }
     where
       f :: forall a. Semigroup a
@@ -129,6 +131,7 @@ nullTracers = Tracers
     , gsmTracer                     = nullTracer
     , gddTracer                     = nullTracer
     , csjTracer                     = nullTracer
+    , dbfTracer                     = nullTracer
     }
 
 showTracers :: ( Show blk
@@ -164,6 +167,7 @@ showTracers tr = Tracers
     , gsmTracer                     = showTracing tr
     , gddTracer                     = showTracing tr
     , csjTracer                     = showTracing tr
+    , dbfTracer                     = showTracing tr
     }
 
 {-------------------------------------------------------------------------------

ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/NodeKernel.hs

@@ -410,7 +410,7 @@ initInternalState NodeKernelArgs { tracers, chainDB, registry, cfg
           (GSM.gsmStateToLedgerJudgement <$> readTVar varGsmState)
         blockFetchInterface :: BlockFetchConsensusInterface (ConnectionId addrNTN) (Header blk) blk m
         blockFetchInterface = BlockFetchClientInterface.mkBlockFetchConsensusInterface
-          (csjTracer tracers)
+          (dbfTracer tracers)
           (configBlock cfg)
           (BlockFetchClientInterface.defaultChainDbView chainDB)
           varChainSyncHandles

ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/BlockTree.hs

@@ -27,10 +27,12 @@ import           Cardano.Slotting.Slot (SlotNo (unSlotNo))
 import           Data.Foldable (asum)
 import           Data.Function ((&))
 import           Data.Functor ((<&>))
+import           Data.List (sortOn)
 import           Data.Maybe (fromJust, fromMaybe)
+import           Data.Ord (Down (Down))
 import qualified Data.Vector as Vector
 import           Ouroboros.Consensus.Block.Abstract (blockNo, blockSlot,
-                     fromWithOrigin, unBlockNo)
+                     fromWithOrigin, pointSlot, unBlockNo)
 import qualified Ouroboros.Network.AnchoredFragment as AF
 import           Text.Printf (printf)
 
@@ -164,26 +166,33 @@ prettyBlockTree :: AF.HasHeader blk => BlockTree blk -> [String]
 prettyBlockTree blockTree =
   ["slots:   " ++ unwords (map (printf "%2d" . unSlotNo) [veryFirstSlot .. veryLastSlot])]
     ++ [printTrunk honestFragment]
-    ++ (map printBranch adversarialFragments)
+    ++ (map (uncurry printBranch) adversarialFragments)
 
   where
     honestFragment = btTrunk blockTree
-    adversarialFragments = map btbSuffix (btBranches blockTree)
+    adversarialFragments =
+        sortOn (Down . pointSlot . AF.anchorPoint . snd)
+      $ zip [1..]
+      $ map btbSuffix (btBranches blockTree)
 
     veryFirstSlot = firstNo $ honestFragment
 
     veryLastSlot =
       foldl max 0 $
         map
           lastNo
-          (honestFragment : adversarialFragments)
+          (honestFragment : map snd adversarialFragments)
 
     printTrunk :: AF.HasHeader blk => AF.AnchoredFragment blk -> String
     printTrunk = printLine (\_ -> "trunk:  ─")
 
-    printBranch :: AF.HasHeader blk => AF.AnchoredFragment blk -> String
-    printBranch = printLine $ \firstSlot ->
-      "      " ++ replicate (3 * fromIntegral (unSlotNo (firstSlot - veryFirstSlot))) ' ' ++ " ╰─"
+    printBranch :: AF.HasHeader blk => Int -> AF.AnchoredFragment blk -> String
+    printBranch idx = printLine $ \firstSlot ->
+      let sidx = "  (" ++ show idx ++ ")"
+          pad = replicate 6 ' '
+          prefix = sidx ++ drop (length sidx) pad
+       in
+       prefix ++ replicate (3 * fromIntegral (unSlotNo (firstSlot - veryFirstSlot))) ' ' ++ " ╰─"
 
     printLine :: AF.HasHeader blk => (SlotNo -> String) -> AF.AnchoredFragment blk -> String
     printLine printHeader fragment =

ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/PeerSimulator/ChainSync.hs

@@ -12,7 +12,8 @@ module Test.Consensus.PeerSimulator.ChainSync (
 
 import           Control.Exception (SomeException)
 import           Control.Monad.Class.MonadTimer.SI (MonadTimer)
-import           Control.Tracer (Tracer (Tracer), nullTracer, traceWith)
+import           Control.Tracer (Tracer (Tracer), contramap, nullTracer,
+                     traceWith)
 import           Data.Proxy (Proxy (..))
 import           Network.TypedProtocol.Codec (AnyMessage)
 import           Ouroboros.Consensus.Block (Header, Point)
@@ -153,6 +154,7 @@ runChainSyncClient
   channel =
     bracketChainSyncClient
       nullTracer
+      (contramap (TraceCsjEvent peerId) tracer)
       chainDbView
       varHandles
       (pure Syncing)

ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/PeerSimulator/Trace.hs

@@ -33,7 +33,9 @@ import           Ouroboros.Consensus.Genesis.Governor (DensityBounds (..),
 import           Ouroboros.Consensus.MiniProtocol.ChainSync.Client
                      (TraceChainSyncClientEvent (..))
 import           Ouroboros.Consensus.MiniProtocol.ChainSync.Client.Jumping
-                     (Instruction (..), JumpInstruction (..), JumpResult (..))
+                     (Instruction (..), JumpInstruction (..), JumpResult (..),
+                     TraceCsjReason (..), TraceEventCsj (..),
+                     TraceEventDbf (..))
 import           Ouroboros.Consensus.MiniProtocol.ChainSync.Client.State
                      (ChainSyncJumpingJumperState (..),
                      ChainSyncJumpingState (..), DynamoInitState (..),
@@ -136,6 +138,8 @@ data TraceEvent blk
   | TraceBlockFetchClientTerminationEvent PeerId TraceBlockFetchClientTerminationEvent
   | TraceGenesisDDEvent (TraceGDDEvent PeerId blk)
   | TraceChainSyncSendRecvEvent PeerId String (TraceSendRecv (ChainSync (Header blk) (Point blk) (Tip blk)))
+  | TraceDbfEvent (TraceEventDbf PeerId)
+  | TraceCsjEvent PeerId (TraceEventCsj PeerId blk)
   | TraceOther String
 
 -- * 'TestBlock'-specific tracers for the peer simulator
@@ -189,6 +193,8 @@ traceEventTestBlockWith setTickTime tracer0 tracer = \case
     TraceBlockFetchClientTerminationEvent peerId traceEvent -> traceBlockFetchClientTerminationEventTestBlockWith peerId tracer traceEvent
     TraceGenesisDDEvent gddEvent -> traceWith tracer (terseGDDEvent gddEvent)
     TraceChainSyncSendRecvEvent peerId peerType traceEvent -> traceChainSyncSendRecvEventTestBlockWith peerId peerType tracer traceEvent
+    TraceDbfEvent traceEvent -> traceDbjEventWith tracer traceEvent
+    TraceCsjEvent peerId traceEvent -> traceCsjEventWith peerId tracer traceEvent
     TraceOther msg -> traceWith tracer msg
 
 traceSchedulerEventTestBlockWith ::
@@ -498,6 +504,40 @@ traceChainSyncSendRecvEventTestBlockWith pid ptp tracer = \case
         MsgIntersectNotFound tip -> "MsgIntersectNotFound " ++ terseTip tip
         MsgDone -> "MsgDone"
 
+traceDbjEventWith ::
+  Tracer m String ->
+  TraceEventDbf PeerId ->
+  m ()
+traceDbjEventWith tracer = traceWith tracer . \case
+    RotatedDynamo old new -> "Rotated dynamo from " ++ condense old ++ " to " ++ condense new
+
+traceCsjEventWith ::
+  PeerId ->
+  Tracer m String ->
+  TraceEventCsj PeerId TestBlock ->
+  m ()
+traceCsjEventWith peer tracer = f . \case
+    BecomingObjector mbOld -> "is now the Objector" ++ replacing mbOld
+    BlockedOnJump -> "is a happy Jumper blocked on the next CSJ instruction"
+    InitializedAsDynamo -> "initialized as the Dynamo"
+    NoLongerDynamo mbNew reason -> g reason ++ " and so is no longer the Dynamo" ++ replacedBy mbNew
+    NoLongerObjector mbNew reason -> g reason ++ " and so is no longer the Objector" ++ replacedBy mbNew
+    SentJumpInstruction p -> "instructed Jumpers to " ++ tersePoint p
+  where
+    f = traceUnitWith tracer ("CSJ " ++ condense peer)
+
+    g = \case
+      BecauseCsjDisconnect -> "disconnected"
+      BecauseCsjDisengage  -> "disengaged"
+
+    replacedBy = \case
+      Nothing -> ""
+      Just new -> ", replaced by: " ++ condense new
+
+    replacing = \case
+      Nothing -> ""
+      Just old -> ", replacing: " ++ condense old
+
 prettyDensityBounds :: [(PeerId, DensityBounds TestBlock)] -> [String]
 prettyDensityBounds bounds =
   showPeers (second showBounds <$> bounds)

ouroboros-consensus/changelog.d/20250307_082219_nick.frisby_debug_genesis_test_failure.md

@@ -0,0 +1,22 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Patch
+
+- A bullet item for the Patch category.
+
+-->
+<!--
+### Non-Breaking
+
+- A bullet item for the Non-Breaking category.
+
+-->
+
+### Breaking
+
+- Added a new CSJ tracer to ChainSync client interface.

ouroboros-consensus/ouroboros-consensus.cabal

@@ -308,6 +308,7 @@ library
     sop-core ^>=0.5,
     sop-extras ^>=0.2,
     streaming,
+    strict >=0.1 && <0.6,
     strict-checked-vars ^>=0.2,
     strict-mvar ^>=1.5,
     strict-sop-core ^>=0.1,

ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/BlockFetch/ClientInterface.hs

@@ -186,7 +186,7 @@ mkBlockFetchConsensusInterface ::
      , Ord peer
      , LedgerSupportsProtocol blk
      )
-  => Tracer m (CSJumping.TraceEvent peer)
+  => Tracer m (CSJumping.TraceEventDbf peer)
   -> BlockConfig blk
   -> ChainDbView m blk
   -> CSClient.ChainSyncClientHandleCollection peer m blk

ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs

@@ -78,6 +78,7 @@ import           Control.Monad (join, void)
 import           Control.Monad.Class.MonadTimer (MonadTimer)
 import           Control.Monad.Except (runExcept, throwError)
 import           Control.Tracer
+import           Data.Foldable (traverse_)
 import           Data.Functor ((<&>))
 import           Data.Kind (Type)
 import           Data.Map.Strict (Map)
@@ -333,6 +334,7 @@ bracketChainSyncClient ::
     , MonadTimer m
     )
  => Tracer m (TraceChainSyncClientEvent blk)
+ -> Tracer m (Jumping.TraceEventCsj peer blk)
  -> ChainDbView m blk
  -> ChainSyncClientHandleCollection peer m blk
     -- ^ The kill handle and states for each peer, we need the whole map because we
@@ -348,6 +350,7 @@ bracketChainSyncClient ::
  -> m a
 bracketChainSyncClient
     tracer
+    tracerCsj
     ChainDbView { getIsInvalidBlock }
     varHandles
     getGsmState
@@ -414,7 +417,8 @@ bracketChainSyncClient
       bracket_ insertHandle deleteHandle $ f Jumping.noJumping
 
     withCSJCallbacks lopBucket csHandleState (CSJEnabled csjEnabledConfig) f =
-      bracket (acquireContext lopBucket csHandleState csjEnabledConfig) releaseContext $ \peerContext ->
+      bracket (acquireContext lopBucket csHandleState csjEnabledConfig) releaseContext $ \(peerContext, mbEv) -> do
+        traverse_ (traceWith (Jumping.tracer peerContext)) mbEv
         f $ Jumping.mkJumping peerContext
 
     acquireContext lopBucket cschState (CSJEnabledConfig jumpSize) = do
@@ -423,7 +427,7 @@ bracketChainSyncClient
           initialGsmState <- getGsmState
           updateLopBucketConfig lopBucket initialGsmState time
           cschJumpInfo <- newTVar Nothing
-          context <- Jumping.makeContext varHandles jumpSize
+          context <- Jumping.makeContext varHandles jumpSize tracerCsj
           Jumping.registerClient context peer cschState $ \cschJumping -> ChainSyncClientHandle
             { cschGDDKill = throwTo tid DensityTooLow
             , cschOnGsmStateChanged = updateLopBucketConfig lopBucket
@@ -432,7 +436,9 @@ bracketChainSyncClient
             , cschJumpInfo
             }
 
-    releaseContext = atomically . Jumping.unregisterClient
+    releaseContext (peerContext, _mbEv) = do
+        mbEv <- atomically $ Jumping.unregisterClient peerContext
+        traverse_ (traceWith (Jumping.tracer peerContext)) mbEv
 
     invalidBlockWatcher varState =
         invalidBlockRejector