From d75ca17fc4dc4c9b60c81f8d7a1e362648941378 Mon Sep 17 00:00:00 2001 From: Jared Corduan Date: Mon, 11 Jul 2022 21:16:04 -0400 Subject: [PATCH] Fix Praos obsolete node check The obsolete node check in the Praos ledger protocol is suppose to compare the current major protocol version (from the protocol parameters in the ledger state) with the global "max major protocol version" value. Currently, however, it is checking that the global max major value is not bigger than the protocol version listed in the block header (which is not supposed to have any semantic meaning). This PR fixes the check to perform the same logic done by the TPraos ledger protocol, ie the logic in the ledger function chainChecks. To do this, `Views.LedgerView` now contains the current protocol parameters. --- .../Consensus/Protocol/Praos/Translate.hs | 5 +++-- .../Ouroboros/Consensus/Protocol/Praos/Views.hs | 9 ++++++--- .../Consensus/Shelley/Ledger/SupportsProtocol.hs | 3 ++- .../Ouroboros/Consensus/Shelley/Protocol/Praos.hs | 4 ++-- ouroboros-consensus/docs/interface-CHANGELOG.md | 15 +++++++++++++++ 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/ouroboros-consensus-protocol/src/Ouroboros/Consensus/Protocol/Praos/Translate.hs b/ouroboros-consensus-protocol/src/Ouroboros/Consensus/Protocol/Praos/Translate.hs index da8212c4f9d..7da4605c526 100644 --- a/ouroboros-consensus-protocol/src/Ouroboros/Consensus/Protocol/Praos/Translate.hs +++ b/ouroboros-consensus-protocol/src/Ouroboros/Consensus/Protocol/Praos/Translate.hs @@ -21,7 +21,7 @@ import Ouroboros.Consensus.Protocol.Praos (ConsensusConfig (..), Praos, PraosParams (..), PraosState (..), Ticked (TickedPraosLedgerView)) import Ouroboros.Consensus.Protocol.Praos.Views - (LedgerView (lvMaxBodySize, lvMaxHeaderSize)) + (LedgerView (lvMaxBodySize, lvMaxHeaderSize, lvProtocolVersion)) import qualified Ouroboros.Consensus.Protocol.Praos.Views as Views import Ouroboros.Consensus.Protocol.TPraos (TPraos, TPraosParams (..), TPraosState (tpraosStateChainDepState, tpraosStateLastSlot)) @@ -70,7 +70,8 @@ instance Views.LedgerView { Views.lvPoolDistr = coercePoolDistr lvPoolDistr, lvMaxHeaderSize = SL.ccMaxBHSize lvChainChecks, - lvMaxBodySize = SL.ccMaxBBSize lvChainChecks + lvMaxBodySize = SL.ccMaxBBSize lvChainChecks, + lvProtocolVersion = SL.ccProtocolVersion lvChainChecks } where coercePoolDistr :: SL.PoolDistr c1 -> SL.PoolDistr c2 diff --git a/ouroboros-consensus-protocol/src/Ouroboros/Consensus/Protocol/Praos/Views.hs b/ouroboros-consensus-protocol/src/Ouroboros/Consensus/Protocol/Praos/Views.hs index 00dc55083ce..e7b3b2990f6 100644 --- a/ouroboros-consensus-protocol/src/Ouroboros/Consensus/Protocol/Praos/Views.hs +++ b/ouroboros-consensus-protocol/src/Ouroboros/Consensus/Protocol/Praos/Views.hs @@ -7,6 +7,7 @@ module Ouroboros.Consensus.Protocol.Praos.Views ( import Cardano.Crypto.KES (SignedKES) import Cardano.Crypto.VRF (CertifiedVRF, VRFAlgorithm (VerKeyVRF)) +import Cardano.Ledger.BaseTypes (ProtVer) import Cardano.Ledger.Crypto (KES, VRF) import Cardano.Ledger.Keys (KeyRole (BlockIssuer), VKey) import qualified Cardano.Ledger.Shelley.API as SL @@ -39,10 +40,12 @@ data HeaderView crypto = HeaderView data LedgerView crypto = LedgerView { -- | Stake distribution - lvPoolDistr :: SL.PoolDistr crypto, + lvPoolDistr :: SL.PoolDistr crypto, -- | Maximum header size - lvMaxHeaderSize :: !Natural, + lvMaxHeaderSize :: !Natural, -- | Maximum block body size - lvMaxBodySize :: !Natural + lvMaxBodySize :: !Natural, + -- | Current protocol version + lvProtocolVersion :: !ProtVer } deriving (Show) diff --git a/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Ledger/SupportsProtocol.hs b/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Ledger/SupportsProtocol.hs index 273b6f0d3d2..81ccf569834 100644 --- a/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Ledger/SupportsProtocol.hs +++ b/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Ledger/SupportsProtocol.hs @@ -99,7 +99,8 @@ instance Praos.LedgerView { Praos.lvPoolDistr = nesPd, Praos.lvMaxBodySize = getField @"_maxBBSize" . SL.esPp $ nesEs, - Praos.lvMaxHeaderSize = getField @"_maxBHSize" . SL.esPp $ nesEs + Praos.lvMaxHeaderSize = getField @"_maxBHSize" . SL.esPp $ nesEs, + Praos.lvProtocolVersion = getField @"_protocolVersion" . SL.esPp $ nesEs } -- | Currently the Shelley+ ledger is hard-coded to produce a TPraos ledger diff --git a/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Protocol/Praos.hs b/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Protocol/Praos.hs index 6b7a1bc2244..780b1d0591b 100644 --- a/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Protocol/Praos.hs +++ b/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Protocol/Praos.hs @@ -73,8 +73,8 @@ instance PraosCrypto c => ProtocolHeaderSupportsEnvelope (Praos c) where BlockSizeTooLarge (bhviewBSize bhv) maxBodySize where pp = praosParams cfg - (MaxMajorProtVer m) = praosMaxMajorPV pp - (ProtVer maxpv _) = hbProtVer $ headerBody hdr + (MaxMajorProtVer maxpv) = praosMaxMajorPV pp + (ProtVer m _) = lvProtocolVersion lv maxHeaderSize = lvMaxHeaderSize lv maxBodySize = lvMaxBodySize lv bhv = mkHeaderView hdr diff --git a/ouroboros-consensus/docs/interface-CHANGELOG.md b/ouroboros-consensus/docs/interface-CHANGELOG.md index 636a41df9e2..53b4640be77 100644 --- a/ouroboros-consensus/docs/interface-CHANGELOG.md +++ b/ouroboros-consensus/docs/interface-CHANGELOG.md @@ -56,6 +56,21 @@ may appear out of chronological order. The internals of each entry are organized similar to https://keepachangelog.com/en/1.1.0/, adapted to our plan explained above. +## Circa 2022-07-20 + +### Fixed + +- The obsolete node check in the new 'Praos' protocol was not performing the + check that was intended. It is supposed to check that the current protocol + version is no greater than the max major protocol version. It was instead + checking that the max major protocol version was not greater than the + protocol version listed in the block header (which is currently not supposed + to have any semantic meaning, and is used to manually check the readiness + of the network for a hard fork). Note that this mistake is only in the Praos + protocol, not in TPraos. The consequence of this incorrect check is that + nodes will not properly halt after a hard fork when they do not have the + required software for the hard fork. + ## Circa 2022-07-13 ### Added