Merge pull request #1320 from Concordium/fix-1319

Fix bug in calculation of absolute block height

Author: td202

CHANGELOG.md

@@ -2,6 +2,13 @@
 
 ## Unreleased changes
 
+## 8.0.3
+
+- Fix a bug where, after a protocol update in consensus version 1 (P6 onwards), a node may
+  miscalculate the absolute height of blocks when it is restarted. (#1319)
+- Fix a bug where `GetBlockInfo` reports the parent block of a genesis block to be the last
+  finalized block of the previous genesis index, instead of the terminal block.
+
 ## 8.0.2
 
 - Fix a bug where the P7->P8 protocol update affects payday timing.

concordium-consensus/src/Concordium/KonsensusV1/SkovMonad.hs

@@ -465,10 +465,9 @@ data ExistingSkov pv m = ExistingSkov
       esState :: !(SkovV1State pv),
       -- | The hash of the current genesis block.
       esGenesisHash :: !BlockHash,
-      -- | The (relative) height of the last finalized block.
-      esLastFinalizedHeight :: !BlockHeight,
-      -- | The effective protocol update if one has occurred.
-      esProtocolUpdate :: !(Maybe ProtocolUpdate)
+      -- | The effective protocol update if one has occurred, together with the relative
+      --  block height of the terminal block.
+      esProtocolUpdate :: !(Maybe (ProtocolUpdate, BlockHeight))
     }
 
 -- | Internal type used for deriving 'HasDatabaseHandlers' and 'LMDBAccountMap.HasDatabaseHandlers'
@@ -541,8 +540,6 @@ initialiseExistingSkovV1 genesisBlockHeightInfo bakerCtx handlerCtx unliftSkov g
                                           _notifiedProtocolUpdate = Nothing
                                         },
                                   esGenesisHash = initialSkovData ^. currentGenesisHash,
-                                  esLastFinalizedHeight =
-                                    blockHeight (initialSkovData ^. lastFinalized),
                                   esProtocolUpdate = effectiveProtocolUpdate
                                 }
                     return $ Just es

concordium-consensus/src/Concordium/KonsensusV1/TreeState/StartUp.hs

@@ -181,8 +181,9 @@ loadSkovData ::
     RuntimeParameters ->
     -- | Set to 'True' if a rollback occurred before loading the skov
     Bool ->
-    -- | The 'SkovData' and, if the consensus is shutdown, the effective protocol update.
-    m (SkovData pv, Maybe ProtocolUpdate)
+    -- | The 'SkovData' and, if the consensus is shutdown, the effective protocol update and
+    --  relative block height of the terminal block.
+    m (SkovData pv, Maybe (ProtocolUpdate, BlockHeight))
 loadSkovData _genesisBlockHeight _runtimeParameters didRollback = do
     _persistentRoundStatus <- LowLevel.lookupCurrentRoundStatus
     mLatestFinEntry <- LowLevel.lookupLatestFinalizationEntry
@@ -238,10 +239,6 @@ loadSkovData _genesisBlockHeight _runtimeParameters didRollback = do
     -- seedstate, the consensus is shutdown, so we determine the shutdown trigger block, which
     -- will be the terminal block. The terminal block may differ from the last finalized block.
     let consensusIsShutdown = lastFinSeedState ^. shutdownTriggered
-    _terminalBlock <-
-        if consensusIsShutdown
-            then Present <$> findShutdownTriggerBlock lastFinBlock
-            else return Absent
     (currentEpoch, lastEpochFinEntry) <-
         if lastFinSeedState ^. epochTransitionTriggered
             && not consensusIsShutdown
@@ -288,16 +285,18 @@ loadSkovData _genesisBlockHeight _runtimeParameters didRollback = do
                   _focusBlock = lastFinBlock
                 }
     let _statistics = Stats.initialConsensusStatistics
-    protocolUpdate <-
+    (_terminalBlock, protocolUpdate) <-
         if consensusIsShutdown
             then do
                 getProtocolUpdateStatus (bpState lastFinBlock) >>= \case
-                    ProtocolUpdated pu -> return $ Just pu
+                    ProtocolUpdated pu -> do
+                        termBlock <- findShutdownTriggerBlock lastFinBlock
+                        return (Present termBlock, Just (pu, blockHeight termBlock))
                     PendingProtocolUpdates{} ->
                         throwM . TreeStateInvariantViolation $
                             "Consensus is shut down, but no effective protocol update was present \
                             \in the last finalized block."
-            else return Nothing
+            else return (Absent, Nothing)
     return (SkovData{..}, protocolUpdate)
 
 -- | Load the certified blocks from the low-level database into the tree state.
@@ -454,7 +453,7 @@ loadCertifiedBlocks = do
                     Nothing -> liftIO DiffMap.newEmptyReference
                     Just diffMapReference -> return diffMapReference
         newDifferenceMap <- reconstructAccountDifferenceMap (bpState blockPointer) parentDiffMapReference accountsToInsert
-        -- append to the accummulator with this new difference map reference
+        -- append to the accumulator with this new difference map reference
         let loadedBlocks' = HM.insert (getHash storedBlock) newDifferenceMap loadedBlocks
 
         cacheBlockState (bpState blockPointer)

concordium-consensus/src/Concordium/MultiVersion.hs

@@ -1366,7 +1366,7 @@ startupSkov genesis = do
                                 activateConfiguration (newVersionV1 newEConfig)
                                 liftSkovV1Update newEConfig checkForProtocolUpdateV1
                         case esProtocolUpdate of
-                            Just protocolUpdate
+                            Just (protocolUpdate, terminalBlockHeight)
                                 | Right upd <- ProtocolUpdateV1.checkUpdate @pv protocolUpdate -> do
                                     let nextSPV = ProtocolUpdateV1.updateNextProtocolVersion upd
                                     -- A protocol update has occurred for this configuration, so
@@ -1377,7 +1377,7 @@ startupSkov genesis = do
                                         nextSPV
                                         activateThis
                                         (genIndex + 1)
-                                        (localToAbsoluteBlockHeight genHeight esLastFinalizedHeight + 1)
+                                        (localToAbsoluteBlockHeight genHeight terminalBlockHeight + 1)
                             _ -> do
                                 -- This is still the current configuration (i.e. no protocol update
                                 -- has occurred, or the protocol update is not supported), so

concordium-consensus/src/Concordium/Queries.hs

@@ -754,13 +754,20 @@ getBlockInfo =
                         -- The block is the genesis block of a non-initial chain, so we use the
                         -- hash of the last finalized block of the previous chain as the parent block.
                         -- Since the genesis index is non-zero, we know that there will be a previous
-                        -- chain, and that it will be shut down with the last finalized block being
-                        -- terminal.
+                        -- chain, and that it will be shut down. For consensus version 0, the last
+                        -- finalized block will be the terminal block. For consensus version 1, we
+                        -- explicitly get the terminal block (which may not be the last finalized
+                        -- block).
+                        let errorNoTerminal = error "Updated consensus is missing terminal block."
                         lift $
                             liftSkovQueryAtGenesisIndex
                                 (biGenesisIndex - 1)
                                 (getHash <$> lastFinalizedBlock)
-                                (use (SkovV1.lastFinalized . to getHash))
+                                ( use
+                                    ( SkovV1.terminalBlock
+                                        . to (getHash . fromOption errorNoTerminal)
+                                    )
+                                )
                     else getHash <$> bpParent bp
             biBlockLastFinalized <- getHash <$> bpLastFinalized bp
             let biBlockHeight = localToAbsoluteBlockHeight (vc1GenesisHeight vc) (SkovV1.blockHeight bp)

concordium-node/Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "concordium_node"
-version = "8.0.2" # must be kept in sync with 'is_compatible_version' in 'src/configuration.rs'
+version = "8.0.3" # must be kept in sync with 'is_compatible_version' in 'src/configuration.rs'
 description = "Concordium Node"
 authors = ["Concordium <developers@concordium.com>"]
 exclude = [".gitignore", ".gitlab-ci.yml", "test/**/*","**/**/.gitignore","**/**/.gitlab-ci.yml"]