Praos chain order: fix issue no-related transitivity edge case

When we only compare the issue numbers when the issuers and (new) the slot
numbers are the same, the transitivity laws work out. The motivation for the
issue number is not impacted by this change.

See the PR description for more details.

Author: amesgen

ouroboros-consensus-protocol/changelog.d/20240503_194253_alexander.esgen_praos_chain_order_truly_transitive.md

@@ -0,0 +1,9 @@
+### Non-Breaking
+
+- Changed the Praos chain order such that for two blocks `A` and `B` by the same
+  issuer with the same block number, `A` is now preferred over `B` only if `A`
+  has a higher issue number *and* (new) `A` and `B` are in the same slot.
+
+  This is in line with the motiviation for the issue number tiebreaker, and
+  fixes the transitivity of the `Ord PraosChainSelectView` instance in a special
+  case.

ouroboros-consensus-protocol/src/ouroboros-consensus-protocol/Ouroboros/Consensus/Protocol/Praos/Common.hs

@@ -92,7 +92,7 @@ comparePraos ::
   -> Ordering
 comparePraos tiebreakerFlavor =
        (compare `on` csvChainLength)
-    <> when' ((==) `on` csvIssuer) (compare `on` csvIssueNo)
+    <> when' issueNoArmed (compare `on` csvIssueNo)
     <> when' vrfArmed (compare `on` Down . csvTieBreakVRF)
   where
     -- When the predicate @p@ returns 'True', use the given comparison function,
@@ -104,6 +104,12 @@ comparePraos tiebreakerFlavor =
     when' p comp a1 a2 =
         if p a1 a2 then comp a1 a2 else EQ
 
+    -- Only compare the issue numbers when the issuers and slots are identical.
+    -- Note that this case implies the VRFs also coincide.
+    issueNoArmed v1 v2 =
+           csvSlotNo v1 == csvSlotNo v2
+        && csvIssuer v1 == csvIssuer v2
+
     -- Whether to do a VRF comparison.
     vrfArmed v1 v2 = case tiebreakerFlavor of
         UnrestrictedVRFTiebreaker       -> True
@@ -120,8 +126,9 @@ comparePraos tiebreakerFlavor =
 --
 -- 1. By chain length, with longer chains always preferred.
 --
--- 2. If the tip of each chain was issued by the same agent, then we prefer
---    the chain whose tip has the highest ocert issue number.
+-- 2. If the tip of each chain was issued by the same agent and they have the
+--    same slot number, prefer the chain whose tip has the highest ocert issue
+--    number.
 --
 -- 3. By a VRF value from the chain tip, with lower values preferred. See
 --    @pTieBreakVRFValue@ for which one is used.
@@ -142,8 +149,13 @@ instance Crypto c => Ord (PraosChainSelectView c) where
 --
 -- 1. Chain length, with longer chains always preferred.
 --
--- 2. If the tip of each chain was issued by the same agent, then we prefer the
---    candidate if it has a higher ocert issue number.
+-- 2. If the tip of each chain was issued by the same agent and had the same
+--    slot number, then we prefer the candidate if it has a higher ocert issue
+--    number.
+--
+--     Note that this condition is equivalent to the VRFs being identical, as
+--     the VRF is a deterministic function of the issuer VRF key, the slot and
+--     the epoch nonce, and VRFs are collision-resistant.
 --
 -- 3. Depending on the 'VRFTiebreakerFlavor':
 --
@@ -182,7 +194,8 @@ instance Crypto c => Ord (PraosChainSelectView c) where
 --    block issuer can use their cold key to issue a new hot key with a higher
 --    opcert issue number and set up a new pool. Due to this tiebreaker rule,
 --    the blocks minted by that pool will take precedence (allowing the actual
---    block issuer to decide on eg the block contents and the predecessor), and
+--    block issuer to decide on eg the block contents and the predecessor) over
+--    blocks with the same block and slot number minted by the attacker, and
 --    they will end up on the honest chain quickly, which means that the
 --    adversary can't extend any chain containing such a block as it would
 --    violate the monotonicity requirement on opcert issue numbers.

ouroboros-consensus-protocol/test/protocol-test/Test/Consensus/Protocol/Praos/SelectView.hs

@@ -43,7 +43,7 @@ instance Crypto c => Arbitrary (PraosChainSelectView c) where
       csvChainLength <- BlockNo <$> choose (1, size)
       csvSlotNo      <- SlotNo  <$> choose (1, size)
       csvIssuer      <- elements knownIssuers
-      csvIssueNo     <- genIssueNo
+      csvIssueNo     <- choose (1, 10)
       pure PraosChainSelectView {
           csvChainLength
         , csvSlotNo
@@ -63,20 +63,6 @@ instance Crypto c => Arbitrary (PraosChainSelectView c) where
          randomSeed = mkQCGen 4 -- chosen by fair dice roll
          numIssuers = 10
 
-     -- TODO Actually randomize this once the issue number tiebreaker has been
-     -- fixed to be transitive. See the document in
-     -- https://github.com/IntersectMBO/ouroboros-consensus/pull/891 for
-     -- details.
-     --
-     -- TL;DR: In an edge case, the issue number tiebreaker prevents the
-     -- chain order from being transitive. This could be fixed relatively
-     -- easily, namely by swapping the issue number tiebreaker and the VRF
-     -- tiebreaker. However, this is technically not backwards-compatible,
-     -- impacting the current pre-Conway diffusion pipelining scheme.
-     --
-     -- See https://github.com/IntersectMBO/ouroboros-consensus/issues/1075.
-     genIssueNo = pure 1
-
      -- The header VRF is a deterministic function of the issuer VRF key, the
      -- slot and the epoch nonce. Additionally, for any particular chain, the
      -- slot determines the epoch nonce.