NOD-525: Optimize baker finalization awake updates (#1322)

* NOD-525: Optimize baker finalization awake updates

Prevent redundant writes in bsoMarkFinalizationAwakeBakers by only
updating  BakerPoolRewardDetails when a baker's finalization state
actually changes. This  avoids unnecessary tree rehashing on each block.

Technical details:
- Added check before updating BakerPoolRewardDetails
- Added tests for bsoMarkFinalizationAwakeBakers

Author: drsk0

concordium-consensus/src/Concordium/GlobalState/Persistent/BlockState.hs

@@ -3823,13 +3823,21 @@ doMarkFinalizationAwakeBakers pbs bids = do
         case binarySearchI bcBakerId bpc bid of
             Nothing -> return bprs
             Just (i, _) -> do
-                mBPRs <- LFMBT.update setAwake (fromIntegral i) bprs
+                mBPRs <- MTL.runExceptT $ LFMBT.update setAwake (fromIntegral i) bprs
                 case mBPRs of
-                    Nothing ->
+                    -- error is used to signal that there is no change
+                    Left () -> return bprs
+                    Right Nothing ->
                         error "Invariant violation: unable to find baker in baker pool reward details tree"
-                    Just ((), newBPRs) ->
+                    Right (Just ((), newBPRs)) ->
                         return newBPRs
-    setAwake bpr =
+    setAwake bpr = do
+        -- If there's nothing to do, use throwError to skip the update.
+        when
+            ( finalizationAwake bpr
+                && all (== emptySuspensionInfo) (suspensionInfo bpr)
+            )
+            $ MTL.throwError ()
         return
             ( (),
               bpr

concordium-consensus/tests/scheduler/SchedulerTests/KonsensusV1/EpochTransition.hs

@@ -953,6 +953,29 @@ testSuspendPrimedSnapshotPaydayCombo accountConfigs = runTestBlockState @P8 $ do
     startEpoch = 10
     startTriggerTime = 1000
 
+testMarkFinalizationAwakeBakers :: [AccountConfig 'AccountV3] -> Assertion
+testMarkFinalizationAwakeBakers accountConfigs = runTestBlockState @P7 $ do
+    bs0 <- makeInitialState accountConfigs (transitionalSeedState startEpoch startTriggerTime) 1
+    bprd0 <- bsoGetBakerPoolRewardDetails bs0
+    let activeBakerIds0 = Map.keys bprd0
+    -- `bsoMarkFinalizationAwakeBakers` should update reward details only on change
+    -- of their finalizationAwake / suspensionInfo field. Hence, the operation
+    -- should be idempotent. This also checks, that the early return in the
+    -- implementation of `markFinalizerAwake` works.
+    bs1 <- bsoMarkFinalizationAwakeBakers bs0 activeBakerIds0
+    bs2 <- bsoMarkFinalizationAwakeBakers bs1 activeBakerIds0
+    bprd1 <- bsoGetBakerPoolRewardDetails bs1
+    bprd2 <- bsoGetBakerPoolRewardDetails bs2
+    liftIO $
+        assertBool
+            "bsoMarkFinalizationAwakeBakers updates reward details"
+            (bprd0 /= bprd1)
+    liftIO $
+        assertEqual "bsoMarkFinalizationAwakeBakers is idempotent" bprd1 bprd2
+  where
+    startEpoch = 10
+    startTriggerTime = 1000
+
 tests :: Spec
 tests = parallel $ describe "EpochTransition" $ do
     it "testEpochTransitionNoPaydayNoSnapshot" $
@@ -977,3 +1000,5 @@ tests = parallel $ describe "EpochTransition" $ do
         forAll (genAccountConfigs False) testSuspendPrimedSnapshotOnly
     it "testSuspendPrimedSnapshotPaydayCombo" $
         forAll (genAccountConfigs False) testSuspendPrimedSnapshotPaydayCombo
+    it "testMarkFinalizationAwakeBakers" $
+        forAll (genAccountConfigs False) testMarkFinalizationAwakeBakers