ChainUpdates: model bad peer behavior

ouroboros-consensus-test/ouroboros-consensus-test.cabal

@@ -293,6 +293,7 @@ test-suite test-infra
                        Test.Util.Split.Tests
 
   build-depends:       base
+                     , containers
                      , QuickCheck
                      , tasty
                      , tasty-quickcheck

ouroboros-consensus-test/src/Test/Util/ChainUpdates.hs

@@ -6,13 +6,17 @@
 module Test.Util.ChainUpdates (
     ChainUpdate (..)
   , UpdateBehavior (..)
+  , behaviorValidity
+  , blocksFromChainUpdates
+  , classifyBehavior
   , genChainUpdates
   , toChainUpdates
     -- * Tests
   , prop_genChainUpdates
   ) where
 
 import           Control.Monad.State.Strict
+import qualified Data.Set as Set
 
 import           Test.QuickCheck
 
@@ -43,6 +47,11 @@ toChainUpdates = concatMap $ \case
     SwitchFork pt bs -> Chain.RollBack pt : map Chain.AddBlock bs
     AddBlock b       -> Chain.AddBlock b  : []
 
+blocksFromChainUpdates :: [ChainUpdate] -> [TestBlock]
+blocksFromChainUpdates = concatMap $ \case
+    SwitchFork _ blks -> blks
+    AddBlock blk      -> [blk]
+
 {-------------------------------------------------------------------------------
   Generating ChainUpdates
 -------------------------------------------------------------------------------}
@@ -72,7 +81,7 @@ data UpdateBehavior =
     -- particular, this includes:
     --
     --  * All blocks involved are valid.
-    --  * Every 'ChainUpdate' improves the chain.
+    --  * No 'ChainUpdate' causes the chain to regress.
     SelectedChainBehavior
   | -- | Chain updates tracking the tentative chain of an honest node (in the
     -- context of diffusion pipelining). This is similiar to
@@ -83,12 +92,42 @@ data UpdateBehavior =
     --  2. @'SwitchFork' (prevPoint blk) [blk']@ where @blk'@ is preferable to
     --     @blk@.
     TentativeChainBehavior
+  | -- | Chain updates involving invalid blocks only arisable by bugs, malice or
+    -- incorrect configuration.
+    InvalidChainBehavior
   deriving stock (Show, Eq, Enum, Bounded)
+  deriving stock
+    ( -- | Note that this 'Ord' instance (and hence the order of constructors)
+      -- is semantically imporant, cf. 'genChainUpdates'.
+      Ord
+    )
+
+-- | Whether an 'UpdateBehavior' should cause a ChainSync and/or BlockFetch
+-- client to disconnect from its peer.
+behaviorValidity :: UpdateBehavior -> Validity
+behaviorValidity = \case
+    SelectedChainBehavior  -> Valid
+    TentativeChainBehavior -> Valid
+    InvalidChainBehavior   -> Invalid
 
+-- | Generate a sequence of chain updates. The given 'UpdateBehavior' is only
+-- used as an "upper bound" for what kind of updates are generated, i.e.
+-- specifying 'TentativeChainBehavior' might (rarely) result in a sequence for
+-- which 'classifyBehavior' returns 'SelectedChainBehavior'.
+--
+-- Concretely, we have the law
+--
+-- > classifyBehavior updates <= behavior
+--
+-- for all
+--
+-- > updates <- forAll $ genChainUpdates behavior k n
 genChainUpdates
   :: UpdateBehavior
   -> SecurityParam
-  -> Int  -- ^ The number of updates to generate
+  -> Int
+     -- ^ An indicator of how many updates to generate. The actual number of
+     -- updates will be proportional with a low factor.
   -> Gen [ChainUpdate]
 genChainUpdates updateBehavior securityParam n =
         getChainUpdates
@@ -100,7 +139,7 @@ genChainUpdateState ::
   -> Int
   -> ChainUpdateState
   -> Gen ChainUpdateState
-genChainUpdateState updateBehavior securityParam n =
+genChainUpdateState behavior securityParam n =
     execStateT (replicateM_ n genChainUpdate)
   where
     -- Modify the state
@@ -109,27 +148,30 @@ genChainUpdateState updateBehavior securityParam n =
 
     k = fromIntegral $ maxRollbacks securityParam
 
-    genChainUpdate = do
-      ChainUpdateState { cusCurrentChain = chain } <- get
-      let genValid =
+    genChainUpdate = frequency' $
+           -- Generate two normal updates, as the other option generates two
+           -- updates, in order to keep the number of updates proportional to n.
+           [ (5, replicateM_ 2 genNormalUpdate) ]
+        <> [ (1, genTrapTentativeBlock)
+           | behavior == TentativeChainBehavior
+           ]
+      where
+        -- Generate a single update, either AddBlock or SwitchFork
+        genNormalUpdate = do
+            chain <- gets cusCurrentChain
             frequency'
-              [ (3, genAddBlock Valid)
+              [ (3, genAddBlock =<< genValidity)
               , ( if Chain.null chain then 0 else 1
                 , genSwitchFork (choose (1, k))
                 )
               ]
-      frequency' $
-        (5, replicateM_ 2 genValid) :
-        [ (1, genInvalidBlock) | updateBehavior == TentativeChainBehavior ]
 
     genBlockToAdd validity = do
         ChainUpdateState { cusCurrentChain = chain } <- get
         block <- lift $ case Chain.head chain of
           Nothing      -> setValidity . firstBlock <$> genForkNo
           Just curHead -> do
-            forkNo <- case validity of
-              Valid   ->  genForkNo
-              Invalid -> pure 3
+            forkNo <- genForkNo
             return
               . modifyFork (const forkNo)
               . setValidity
@@ -144,8 +186,9 @@ genChainUpdateState updateBehavior securityParam n =
             , (1, choose (1, 2))
             ]
           -- Blocks with equal hashes have to have equal validity, so we reserve
-          -- a specific ForkNo for invalid blocks to ensure this.
-          Invalid -> pure 3
+          -- specific ForkNos for invalid blocks to ensure this. Note that the
+          -- concrete numbers chosen here don't have any real semantics.
+          Invalid -> elements [3, 4]
 
     genAddBlock validity = do
       block <- genBlockToAdd validity
@@ -156,13 +199,17 @@ genChainUpdateState updateBehavior securityParam n =
       rollBackBlocks <- lift genRollBackBlocks
       let chain' = Chain.drop rollBackBlocks chain
       modify $ setChain chain'
-      blocks <- replicateM rollBackBlocks (genBlockToAdd Valid)
+      blocks <- replicateM rollBackBlocks (genBlockToAdd =<< genValidity)
       modify $ addUpdate (SwitchFork (Chain.headPoint chain') blocks)
 
-    genInvalidBlock = do
+    genTrapTentativeBlock = do
       genAddBlock Invalid
       genSwitchFork (pure 1)
 
+    genValidity = case behavior of
+      InvalidChainBehavior -> frequency' [ (4, pure Valid), (1, pure Invalid) ]
+      _                    -> pure Valid
+
 -- | Variant of 'frequency' that allows for transformers of 'Gen'
 frequency' :: (MonadTrans t, Monad (t Gen)) => [(Int, t Gen a)] -> t Gen a
 frequency' [] = error "frequency' used with empty list"
@@ -175,6 +222,37 @@ frequency' xs0 = lift (choose (1, tot)) >>= (`pick` xs0)
       | otherwise = pick (n-k) xs
     pick _ _  = error "pick used with empty list"
 
+-- | Classify the 'UpdateBehavior' of a sequence of 'ChainUpdate's based on
+-- their validities.
+--
+-- PRECONDITION: The updates fit on each other.
+classifyBehavior :: [ChainUpdate] -> UpdateBehavior
+classifyBehavior updates
+    | null invalidBlocks
+    = SelectedChainBehavior
+    | noInvalidBlockExtended && invalidBlocksImproving
+    = TentativeChainBehavior
+    | otherwise
+    = InvalidChainBehavior
+  where
+    -- The behavior is tentative iff:
+    --  1. The sequence of invalid blocks is strictly improving.
+    invalidBlocksImproving = strictlyIncreasing $ blockNo <$> invalidBlocks
+    --  2. No block ever extends an invalid block.
+    noInvalidBlockExtended = all predecessorIsValid allBlocks
+
+    allBlocks = blocksFromChainUpdates updates
+    invalidBlocks = filter ((Invalid ==) . tbValid) allBlocks
+
+    predecessorIsValid =
+        \blk -> blockPrevHash blk `Set.notMember` invalidBlockHashes
+      where
+        invalidBlockHashes =
+          Set.fromList $ BlockHash . blockHash <$> invalidBlocks
+
+    strictlyIncreasing :: Ord a => [a] -> Bool
+    strictlyIncreasing as = and $ zipWith (<) as (tail as)
+
 -- | Test that applying the generated updates gives us the same chain
 -- as @cusCurrentChain@.
 prop_genChainUpdates :: SecurityParam -> Int -> Property
@@ -184,5 +262,5 @@ prop_genChainUpdates securityParam n =
       Just (cusCurrentChain cus)
   where
     genCUS = do
-      behavior <- chooseEnum (minBound, maxBound)
+      behavior <- arbitraryBoundedEnum
       genChainUpdateState behavior securityParam n emptyUpdateState

ouroboros-consensus-test/src/Test/Util/Schedule.hs

@@ -5,6 +5,7 @@
 -- | Utilities to schedule actions per 'Tick'.
 module Test.Util.Schedule (
     Schedule (..)
+  , SchedulingStrategy (..)
   , genSchedule
   , joinSchedule
   , lastTick
@@ -49,11 +50,22 @@ lastTick = fromMaybe (Tick 0) . maxKey . getSchedule
     maxKey :: forall k v. Map k v -> Maybe k
     maxKey = fmap (fst . fst) . Map.maxViewWithKey
 
+-- | A scheduling strategy specifies how to distribute elements across a
+-- sequence of ticks.
+data SchedulingStrategy =
+     -- | The default strategy. Most ticks will have no associated elements, but
+     -- if they do, there can be multiple.
+     DefaultSchedulingStrategy
+   | -- | Like 'DefaultSchedulingStrategy', but with at most one element per
+     -- tick.
+     SingleItemPerTickStrategy
+  deriving stock (Show, Eq, Ord, Enum, Bounded)
+
 -- | Spread out elements over a schedule, i.e. schedule a number of
 -- elements to be processed on each tick. Most ticks will have no
 -- associated elements.
-genSchedule :: [a] -> Gen (Schedule a)
-genSchedule = fmap Schedule . go Map.empty 1
+genSchedule :: SchedulingStrategy -> [a] -> Gen (Schedule a)
+genSchedule strat = fmap Schedule . go Map.empty 1
   where
     go :: Map Tick [a]
        -> Tick
@@ -62,10 +74,15 @@ genSchedule = fmap Schedule . go Map.empty 1
     go !schedule tick as
       | null as = return schedule
       | otherwise    = do
-        nbAs <- frequency [ (2, return 0), (1, choose (1, 5)) ]
+        nbAs <- genNumElemsPerTick
         let (this, rest) = splitAt nbAs as
         go (Map.insert tick this schedule) (succ tick) rest
 
+    genNumElemsPerTick = case strat of
+        DefaultSchedulingStrategy ->
+          frequency [ (2, return 0), (1, choose (1, 5)) ]
+        SingleItemPerTickStrategy -> elements [0, 1]
+
 -- | Repeatedly remove the last entry (highest 'Tick')
 shrinkSchedule :: Schedule a -> [Schedule a]
 shrinkSchedule =