-
Notifications
You must be signed in to change notification settings - Fork 22
create-protocol-parameters-update: validate cost models' size #1012
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |||||
| {-# LANGUAGE RankNTypes #-} | ||||||
| {-# LANGUAGE ScopedTypeVariables #-} | ||||||
| {-# LANGUAGE TupleSections #-} | ||||||
| {-# LANGUAGE TypeApplications #-} | ||||||
| {-# LANGUAGE TypeFamilies #-} | ||||||
|
|
||||||
| module Cardano.CLI.Read | ||||||
|
|
@@ -119,11 +120,14 @@ | |||||
| import Cardano.CLI.Types.Governance | ||||||
| import Cardano.CLI.Types.Key | ||||||
| import qualified Cardano.Crypto.Hash as Crypto | ||||||
| import qualified PlutusLedgerApi.V1.ParamName as PlutusV1 | ||||||
| import qualified PlutusLedgerApi.V2.ParamName as PlutusV2 | ||||||
| import qualified PlutusLedgerApi.V3.ParamName as PlutusV3 | ||||||
|
|
||||||
| import Prelude | ||||||
|
|
||||||
| import Control.Exception (bracket, displayException) | ||||||
| import Control.Monad (forM, unless, when) | ||||||
| import Control.Monad (forM, forM_, unless, when) | ||||||
| import qualified Data.Aeson as Aeson | ||||||
| import Data.Bifunctor | ||||||
| import Data.ByteString (ByteString) | ||||||
|
|
@@ -134,6 +138,7 @@ | |||||
| import Data.Function ((&)) | ||||||
| import Data.IORef (IORef, newIORef, readIORef, writeIORef) | ||||||
| import qualified Data.List as List | ||||||
| import qualified Data.Map as Map | ||||||
| import Data.Proxy (Proxy (..)) | ||||||
| import Data.String | ||||||
| import Data.Text (Text) | ||||||
|
|
@@ -1057,6 +1062,9 @@ | |||||
| = CostModelsErrorReadFile (FileError ()) | ||||||
| | CostModelsErrorJSONDecode FilePath String | ||||||
| | CostModelsErrorEmpty FilePath | ||||||
| | -- | @CostModelsErrorWrongSize expected actual@ indicates that the cost model | ||||||
| -- has @actual@ entries, but @expected@ entries were expected. | ||||||
| CostModelsErrorWrongSize Int Int | ||||||
| deriving Show | ||||||
|
|
||||||
| instance Error CostModelsError where | ||||||
|
|
@@ -1067,6 +1075,11 @@ | |||||
| "Error decoding JSON cost model at " <> pshow fp <> ": " <> pretty err <> formatExplanation | ||||||
| CostModelsErrorEmpty fp -> | ||||||
| "The decoded cost model was empty at: " <> pshow fp <> formatExplanation | ||||||
| CostModelsErrorWrongSize expected actual -> | ||||||
| "The decoded cost model has the wrong size: expected " | ||||||
| <> pretty expected | ||||||
| <> ", but got " | ||||||
| <> pretty actual | ||||||
| where | ||||||
| formatExplanation = | ||||||
| vsep | ||||||
|
|
@@ -1086,13 +1099,38 @@ | |||||
| ] | ||||||
|
|
||||||
| readCostModels | ||||||
| :: File L.CostModels In | ||||||
| :: () | ||||||
| => ShelleyBasedEra era | ||||||
| -> File L.CostModels In | ||||||
| -> ExceptT CostModelsError IO L.CostModels | ||||||
| readCostModels (File fp) = do | ||||||
| readCostModels sbe (File fp) = do | ||||||
| bytes <- handleIOExceptT (CostModelsErrorReadFile . FileIOError fp) $ LBS.readFile fp | ||||||
| costModels <- firstExceptT (CostModelsErrorJSONDecode fp) . except $ Aeson.eitherDecode bytes | ||||||
| when (null $ fromAlonzoCostModels costModels) $ throwE $ CostModelsErrorEmpty fp | ||||||
| forM_ (allValues @L.Language) $ checkCostModelSize costModels | ||||||
| return costModels | ||||||
| where | ||||||
| checkCostModelSize :: L.CostModels -> L.Language -> ExceptT CostModelsError IO () | ||||||
| checkCostModelSize models lang = | ||||||
| case Map.lookup lang (L.costModelsValid models) of | ||||||
| Nothing -> pure () | ||||||
| Just (model :: L.CostModel) -> do | ||||||
| let actual = Map.size (L.costModelToMap model) | ||||||
| expected = languageToParameterCount lang | ||||||
| unless (expected == actual) $ throwE $ CostModelsErrorWrongSize expected actual | ||||||
| return () | ||||||
|
Comment on lines
+1117
to
+1121
Check warningCode scanning / HLint Redundant return Warning
cardano-cli/src/Cardano/CLI/Read.hs:(1117,38)-(1121,17): Warning: Redundant return
Found: do let actual = Map.size (L.costModelToMap model) expected = languageToParameterCount lang unless (expected == actual) $ throwE $ CostModelsErrorWrongSize expected actual return () Perhaps: do let actual = Map.size (L.costModelToMap model) expected = languageToParameterCount lang unless (expected == actual) $ throwE $ CostModelsErrorWrongSize expected actual |
||||||
| allValues :: forall a. (Bounded a, Enum a) => [a] | ||||||
| allValues = [minBound :: a .. maxBound] | ||||||
| languageToParameterCount :: L.Language -> Int | ||||||
| languageToParameterCount = \case | ||||||
| L.PlutusV1 -> length $ allValues @PlutusV1.ParamName -- 166 | ||||||
| L.PlutusV2 -> | ||||||
| let nbParamNames = length $ allValues @PlutusV2.ParamName in -- 185 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
😄 |
||||||
| caseShelleyToBabbageOrConwayEraOnwards | ||||||
| (const $ nbParamNames - 10) -- Ten parameters were added to V2 in Conway, need to remove them here | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not so sure about hardcoding that number 10. We should account for the fact that the number of params can change in the future, however it's unlikely. That's why there's jumping over many hoops in |
||||||
| (const nbParamNames) | ||||||
| sbe | ||||||
| L.PlutusV3 -> length $ allValues @PlutusV3.ParamName -- 297 | ||||||
|
|
||||||
| -- Misc | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| { | ||
| "type": "Governance proposal", | ||
| "description": "Update protocol parameters proposal", | ||
| "cborHex": "84193039581de18f4a3466a404c11eb410313015b88e447d81b60089e25f611600e6058400f6a112a1019f1a0003236119032c01011903e819023b00011903e8195e7104011903e818201a0001ca761928eb041959d818641959d818641959d818641959d818641959d818641959d81864186418641959d81864194c5118201a0002acfa182019b551041a000363151901ff00011a00015c3518201a000797751936f404021a0002ff941a0006ea7818dc0001011903e8196ff604021a0003bd081a00034ec5183e011a00102e0f19312a011a00032e801901a5011a0002da781903e819cf06011a00013a34182019a8f118201903e818201a00013aac0119e143041903e80a1a00030219189c011a00030219189c011a0003207c1901d9011a000330001901ff0119ccf3182019fd40182019ffd5182019581e18201940b318201a00012adf18201a0002ff941a0006ea7818dc0001011a00010f92192da7000119eabb18201a0002ff941a0006ea7818dc0001011a0002ff941a0006ea7818dc0001011a0011b22c1a0005fdde00021a000c504e197712041a001d6af61a0001425b041a00040c660004001a00014fab18201a0003236119032c010119a0de18201a00033d7618201979f41820197fb8182019a95d1820197df718201995aa18201a0223accc0a1a009063b91903fd0a1a02515e841980b30afff6826b6578616d706c652e636f6d5820c7ddb5b493faa4d3d2d679847740bdce0c5d358d56f9b1470ca67f5652a02745" | ||
| "cborHex": "84193039581de18f4a3466a404c11eb410313015b88e447d81b60089e25f611600e6058400f6a112a1019f1a0003236119032c01011903e819023b00011903e8195e7104011903e818201a0001ca761928eb041959d818641959d818641959d818641959d818641959d818641959d81864186418641959d81864194c5118201a0002acfa182019b551041a000363151901ff00011a00015c3518201a000797751936f404021a0002ff941a0006ea7818dc0001011903e8196ff604021a0003bd081a00034ec5183e011a00102e0f19312a011a00032e801901a5011a0002da781903e819cf06011a00013a34182019a8f118201903e818201a00013aac0119e143041903e80a1a00030219189c011a00030219189c011a0003207c1901d9011a000330001901ff0119ccf3182019fd40182019ffd5182019581e18201940b318201a00012adf18201a0002ff941a0006ea7818dc0001011a00010f92192da7000119eabb18201a0002ff941a0006ea7818dc0001011a0002ff941a0006ea7818dc0001011a0011b22c1a0005fdde00021a000c504e197712041a001d6af61a0001425b041a00040c660004001a00014fab18201a0003236119032c010119a0de18201a00033d7618201979f41820197fb8182019a95d1820197df718201995aa18201a0223accc0a1a009063b91903fd0a1a02515e841980b30a00000000000000000000fff6826b6578616d706c652e636f6d5820c7ddb5b493faa4d3d2d679847740bdce0c5d358d56f9b1470ca67f5652a02745" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,6 +173,16 @@ | |
| 10, | ||
| 38887044, | ||
| 32947, | ||
| 10 | ||
| 10, | ||
| 0, | ||
| 0, | ||
| 0, | ||
| 0, | ||
| 0, | ||
| 0, | ||
| 0, | ||
| 0, | ||
| 0, | ||
| 0 | ||
| ] | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think those checks should be in
cardano-apiso we can reuse them in decoding genesis as well, because genesis can contain PV2 and PV3 models. This is used only in starting testnets afaik.