Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Praos obsolete node check #3891

Merged
merged 1 commit into from
Jul 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
nfrisby marked this conversation as resolved.
Show resolved Hide resolved
(ProtVer m _) = lvProtocolVersion lv
maxHeaderSize = lvMaxHeaderSize lv
maxBodySize = lvMaxBodySize lv
bhv = mkHeaderView hdr
Expand Down
15 changes: 15 additions & 0 deletions ouroboros-consensus/docs/interface-CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down