Merge #1006

1006: Fix apparent performance calculation r=KtorZ a=Anviking

# Issue Number

Should probably be a bug.

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] I fixed the way we calculate `S (number of slots in the epoch)`. 


# Comments

## Comparison

Setup:
```bash
jormungandr --genesis-block-hash 6a40cf890d84981353457fcab6c892af57ee3c3286b33b530cd46b1af5b0e3a7 \
    --rest-listen 127.0.0.1:8080 \
    --storage /Users/Johannes/.local/share/cardano-wallet/jormungandr/testnet/chain \
    --config ../testnet.yml

cardano-wallet-jormungandr serve --genesis-block-hash 6a40cf890d84981353457fcab6c892af57ee3c3286b33b530cd46b1af5b0e3a7 --node-port 8080
```

### Previously (uncapped)

Using the first commit to show the uncapped apparent performances.

```bash
cardano-wallet-jormungandr stake-pool list | jq '.[] | "\(.metrics.controlled_stake.quantity),\(.metrics.produced_blocks.quantity) \(.apparent_performance)"' | tr -d '"'
Ok.
10000000000000,2 2.2908997271066665
10000000000000,2 2.2908997271066665
10000000000000,1 1.1454498635533332
10000000000000,1 1.1454498635533332
799999987900,0 0
10000000000000,0 0
8039991840700,0 0
9886999984600,0 0
```

### Now (uncapped)

```bash
cardano-wallet-jormungandr stake-pool list | jq '.[] | "\(.metrics.controlled_stake.quantity),\(.metrics.produced_blocks.quantity) \(.apparent_performance)"' | tr -d '"'
Ok.
10000000000000,2 0.19636283375199998
10000000000000,2 0.19636283375199998
10000000000000,1 0.09818141687599999
10000000000000,1 0.09818141687599999
799999987900,0 0
10000000000000,0 0
8039991840700,0 0
9886999984600,0 0
```



<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
Co-authored-by: KtorZ <matthias.benkort@gmail.com>

lib/core/src/Cardano/Pool/Metrics.hs

@@ -51,7 +51,13 @@ import Cardano.Wallet.Network
     , staticBlockchainParameters
     )
 import Cardano.Wallet.Primitive.Types
-    ( BlockHeader (..), EpochNo (..), PoolId (..), SlotId (..) )
+    ( BlockHeader (..)
+    , EpochLength (..)
+    , EpochNo (..)
+    , PoolId (..)
+    , SlotId (..)
+    , SlotNo (unSlotNo)
+    )
 import Control.Monad
     ( forM, forM_, when )
 import Control.Monad.IO.Class
@@ -214,7 +220,7 @@ newStakePoolLayer db@DBLayer{..} nl tr = StakePoolLayer
             computeProgress nodeTip >>= throwE . ErrMetricsIsUnsynced
 
         perfs <- liftIO $
-            readPoolsPerformances db nodeEpoch
+            readPoolsPerformances db epochLength (nodeTip ^. #slotId)
 
         case combineMetrics distr prod perfs of
             Right x -> return
@@ -229,6 +235,10 @@ newStakePoolLayer db@DBLayer{..} nl tr = StakePoolLayer
         [x] -> return $ Just x
         _ -> return Nothing
 
+
+    (_, bp) = staticBlockchainParameters nl
+    epochLength = bp ^. #getEpochLength
+
     mkStakePool
         :: PoolId
         -> ( Quantity "lovelace" Word64
@@ -268,14 +278,24 @@ newStakePoolLayer db@DBLayer{..} nl tr = StakePoolLayer
 
 readPoolsPerformances
     :: DBLayer m
-    -> EpochNo
+    -> EpochLength
+    -> SlotId
     -> m (Map PoolId Double)
-readPoolsPerformances DBLayer{..} (EpochNo epochNo) = do
-    let range = [max 0 (fromIntegral epochNo - 14) .. fromIntegral epochNo]
+readPoolsPerformances DBLayer{..} (EpochLength el) tip = do
+    let range = [max 0 (currentEpoch - 14) .. currentEpoch]
     atomically $ fmap avg $ forM range $ \ep -> calculatePerformance
+        (slotsInEpoch ep)
         <$> (Map.fromList <$> readStakeDistribution ep)
         <*> (count <$> readPoolProduction ep)
   where
+    currentEpoch = tip ^. #epochNumber
+
+    slotsInEpoch :: EpochNo -> Int
+    slotsInEpoch e =
+        if e == currentEpoch
+        then fromIntegral $ unSlotNo $ tip ^. #slotNumber
+        else fromIntegral el
+
     -- | Performances are computed over many epochs to cope with the fact that
     -- our data is sparse (regarding stake distribution at least).
     --
@@ -305,23 +325,24 @@ readPoolsPerformances DBLayer{..} (EpochNo epochNo) = do
 -- practice, be greater than 1 if a stake pool produces more than it is
 -- expected.
 calculatePerformance
-    :: Map PoolId (Quantity "lovelace" Word64)
+    :: Int
+    -> Map PoolId (Quantity "lovelace" Word64)
     -> Map PoolId (Quantity "block" Word64)
     -> Map PoolId Double
-calculatePerformance mStake mProd =
+calculatePerformance nTotal mStake mProd =
     let
         stakeButNotProd = traverseMissing $ \_ _ -> 0
         prodButNoStake  = dropMissing
-        stakeAndProd sTotal nTotal = zipWithMatched $ \_ s n ->
+        stakeAndProd sTotal = zipWithMatched $ \_ s n ->
             if (nTotal == 0 || s == Quantity 0) then
                 0
             else
-                min 1 ((double n / nTotal) * (sTotal / double s))
+                min 1 ((double n / fromIntegral nTotal) * (sTotal / double s))
     in
         Map.merge
             stakeButNotProd
             prodButNoStake
-            (stakeAndProd (sumQ mStake) (sumQ mProd))
+            (stakeAndProd (sumQ mStake))
             mStake
             mProd
   where

lib/core/test/unit/Cardano/Pool/MetricsSpec.hs

@@ -14,6 +14,7 @@ import Cardano.Pool.Metrics
     ( Block (..), calculatePerformance, combineMetrics )
 import Cardano.Wallet.Primitive.Types
     ( BlockHeader (..)
+    , Coin (..)
     , EpochLength (..)
     , Hash (..)
     , PoolId (..)
@@ -30,15 +31,18 @@ import Data.Quantity
 import Data.Word
     ( Word32, Word64 )
 import Test.Hspec
-    ( Spec, describe, it )
+    ( Spec, describe, it, shouldBe )
 import Test.QuickCheck
     ( Arbitrary (..)
+    , NonNegative (..)
     , Property
     , checkCoverage
+    , choose
     , classify
     , counterexample
     , cover
     , elements
+    , frequency
     , property
     , vectorOf
     , (===)
@@ -63,6 +67,9 @@ spec = do
         it "performances are always between 0 and 1"
             $ property prop_performancesBounded01
 
+        describe "golden test cases" $ do
+            performanceGoldens
+
 {-------------------------------------------------------------------------------
                                 Properties
 -------------------------------------------------------------------------------}
@@ -101,18 +108,55 @@ prop_combineIsLeftBiased mStake mProd mPerf =
 prop_performancesBounded01
     :: Map PoolId (Quantity "lovelace" Word64)
     -> Map PoolId (Quantity "block" Word64)
+    -> (NonNegative Int)
     -> Property
-prop_performancesBounded01 mStake mProd =
+prop_performancesBounded01 mStake mProd (NonNegative emptySlots) =
     all (between 0 1) performances
     & counterexample (show performances)
     & classify (all (== 0) performances) "all null"
   where
     performances :: [Double]
-    performances = Map.elems $ calculatePerformance mStake mProd
+    performances = Map.elems $ calculatePerformance slots mStake mProd
+
+    slots :: Int
+    slots = emptySlots +
+        fromIntegral (Map.foldl (\y (Quantity x) -> (y + x)) 0 mProd)
 
     between :: Ord a => a -> a -> a -> Bool
     between inf sup x = x >= inf && x <= sup
 
+
+performanceGoldens :: Spec
+performanceGoldens = do
+    it "50% stake, producing 8/8 blocks => performance=1.0" $ do
+        let stake      = mkStake      [ (poolA, 1), (poolB, 1) ]
+        let production = mkProduction [ (poolA, 8), (poolB, 0) ]
+        let performances = calculatePerformance 8 stake production
+        Map.lookup poolA performances `shouldBe` (Just 1)
+
+    it "50% stake, producing 4/8 blocks => performance=1.0" $ do
+        let stake      = mkStake      [ (poolA, 1), (poolB, 1) ]
+        let production = mkProduction [ (poolA, 4), (poolB, 0) ]
+        let performances = calculatePerformance 8 stake production
+        Map.lookup poolA performances `shouldBe` (Just 1)
+
+    it "50% stake, producing 2/8 blocks => performance=0.5" $ do
+        let stake      = mkStake      [ (poolA, 1), (poolB, 1) ]
+        let production = mkProduction [ (poolA, 2), (poolB, 0) ]
+        let performances = calculatePerformance 8 stake production
+        Map.lookup poolA performances `shouldBe` (Just 0.5)
+
+    it "50% stake, producing 0/8 blocks => performance=0.0" $ do
+        let stake      = mkStake      [ (poolA, 1), (poolB, 1) ]
+        let production = mkProduction [ (poolA, 0), (poolB, 0) ]
+        let performances = calculatePerformance 8 stake production
+        Map.lookup poolA performances `shouldBe` (Just 0)
+  where
+    poolA = PoolId "athena"
+    poolB = PoolId "nemesis"
+    mkStake = Map.map Quantity . Map.fromList
+    mkProduction = Map.map Quantity . Map.fromList
+
 {-------------------------------------------------------------------------------
                                  Arbitrary
 -------------------------------------------------------------------------------}
@@ -139,14 +183,34 @@ instance Arbitrary (Hash tag) where
         <$> vectorOf 8 (elements (['a'..'f'] ++ ['0'..'9']))
 
 instance Arbitrary Block where
-     arbitrary = genericArbitrary
-     shrink = genericShrink
+   arbitrary = genericArbitrary
+   shrink = genericShrink
 
 instance Arbitrary (Quantity "block" Word32) where
-     arbitrary = Quantity . fromIntegral <$> (arbitrary @Word32)
-
-instance Arbitrary (Quantity any Word64) where
-     arbitrary = Quantity . fromIntegral <$> (arbitrary @Word64)
+    arbitrary = Quantity . fromIntegral <$> (arbitrary @Word32)
+    shrink (Quantity x) = map Quantity $ shrink x
+
+instance Arbitrary (Quantity "block" Word64) where
+    arbitrary = Quantity . fromIntegral <$> (arbitrary @Word32)
+    shrink (Quantity x) = map Quantity $ shrink x
+
+instance Arbitrary (Quantity "lovelace" Word64) where
+    arbitrary = Quantity . fromIntegral . unLovelace <$> (arbitrary @Lovelace)
+    shrink (Quantity x) = map Quantity $ shrink x
+
+-- TODO: Move to a shared location for Arbitrary newtypes
+newtype Lovelace = Lovelace { unLovelace :: Word64 }
+instance Arbitrary Lovelace where
+    shrink (Lovelace x) = map Lovelace $ shrink x
+    arbitrary = do
+        n <- choose (0, 100)
+        Lovelace <$> frequency
+            [ (8, return n)
+            , (2, choose (minLovelace, maxLovelace))
+            ]
+      where
+        minLovelace = fromIntegral . getCoin $ minBound @Coin
+        maxLovelace = fromIntegral . getCoin $ maxBound @Coin
 
 instance Arbitrary PoolId where
     shrink _  = []