search

IntersectMBO / cardano-ledger

The ledger implementation and specifications of the Cardano blockchain.
Apache License 2.0
249 stars 157 forks source link

The maxMajorPV field of Globals is unused #3682

Open nfrisby opened 10 months ago

nfrisby commented 10 months ago

Using an up-to-date commit

nfrisby@frisbycomp:~/cardano-haskell/cardano-ledger$ git show origin/master --no-patch
commit 63d98c3f8e9eb2878cec3ab71c54fc40c798ac01 (origin/master, origin/HEAD)
Author: Tim Sheard <sheard@pdx.edu>
Date:   Thu Aug 24 15:45:07 2023 -0400

    Fix inactive PoolStake not counting as Drep Stake (#3676)

    * Changed freshDRepPulser to use map form IncrementalStake rather than SnapShot.

I see that the non-testing code propagates maxMajorPV but never actually uses it.

nfrisby@frisbycomp:~/cardano-haskell/cardano-ledger$ git grep -C1 -w -e maxMajorPV origin/master -- '*hs' | grep -v -e test-suite -e testlib
origin/master:eras/shelley/impl/src/Cardano/Ledger/Shelley/Genesis.hs-  Globals
origin/master:eras/shelley/impl/src/Cardano/Ledger/Shelley/Genesis.hs:mkShelleyGlobals genesis epochInfoAc maxMajorPV =
origin/master:eras/shelley/impl/src/Cardano/Ledger/Shelley/Genesis.hs-  Globals
--
origin/master:eras/shelley/impl/src/Cardano/Ledger/Shelley/Genesis.hs-    , maxLovelaceSupply = sgMaxLovelaceSupply genesis
origin/master:eras/shelley/impl/src/Cardano/Ledger/Shelley/Genesis.hs:    , maxMajorPV = maxMajorPV
origin/master:eras/shelley/impl/src/Cardano/Ledger/Shelley/Genesis.hs-    , networkId = sgNetworkId genesis
--
--
--
--
--
--
--
--
--
--
--
--
--
origin/master:libs/cardano-ledger-core/src/Cardano/Ledger/BaseTypes.hs-  -- ^ Quorum for update system votes and MIR certificates
origin/master:libs/cardano-ledger-core/src/Cardano/Ledger/BaseTypes.hs:  , maxMajorPV :: !Version
origin/master:libs/cardano-ledger-core/src/Cardano/Ledger/BaseTypes.hs-  -- ^ All blocks invalid after this protocol version
--
origin/master:libs/cardano-ledger-pretty/src/Cardano/Ledger/Pretty.hs-      , ("quorum", pretty quor)
origin/master:libs/cardano-ledger-pretty/src/Cardano/Ledger/Pretty.hs:      , ("maxMajorPV", prettyA maxmaj)
origin/master:libs/cardano-ledger-pretty/src/Cardano/Ledger/Pretty.hs-      , ("maxLovelaceSupply", pretty maxlove)

I therefore propose we remove it from the Globals data type.

nfrisby commented 10 months ago

The intended check is being done, just not by the ledger rules.

For TPraos, the max major version is passed as the first argument to chainChecks, which does not receive a Globals argument. It is called from Consensus code, which carries its own "config" that has the value in it. Similarly for Praos, but the check itself is also in the Consensus code base (since TPraos has not been fully relocated to the Consensus repo). See the instances of the envelopeChecks method, which is only applied to headers.