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

PlutusV3 cost model update with 299 items stripped to 251 #924

Closed
mkoura opened this issue Oct 4, 2024 · 5 comments
Closed

PlutusV3 cost model update with 299 items stripped to 251 #924

mkoura opened this issue Oct 4, 2024 · 5 comments
Assignees

Comments

@mkoura
Copy link
Contributor

mkoura commented Oct 4, 2024

Description

When submitting cost model update where PlutusV3 cost model has 299 entries (this will be the case in Conway protocol version 10) and the cost model is map (not list), some of the items are stripped.

When printing out the proposal using gov-state, the PlutusV3 is a list of 251 items.

When providing the PlutusV3 cost model items as list, the proposal has correctly 299 items.

Steps to Reproduce

cardano-cli conway governance action create-protocol-parameters-update --testnet --governance-action-deposit 100000000 --deposit-return-stake-verification-key-file test_update_cost_models_addr0_stake.vkey --anchor-url "http://www.pparam-action-dqdd.com" --anchor-data-hash d03fbf098c734ad9671d9b48aec2063b4ba472fd59be8e39102c28fd2f1c848b --cost-model-file cost_models_pv10.json --out-file test_update_cost_models_pparams_update.action

cardano-cli conway transaction build --tx-in "4bdfbea9010a99a1638484cd6b2dfb7a3ccacf89d0ca64113e9756cc94b498ff#0" --proposal-file test_update_cost_models_pparams_update.action --change-address addr_test1qpfpxjqe4568tgyuyfww0utyzqc64pqulp6agaqrugc8pk2hx7an3xfrhlkwwl83lnv0fnm3h6lvztmyhwykt93n9llslgr8wd --witness-override 1 --out-file test_update_cost_models_ci0_iuf_action_tx.body --testnet-magic 42

Additional Context

Tested with cardano node rev 9671e7b6a1b91f5a530722937949b86deafaad43

Cost model, action and transaction: issue_cost_model_stripped.tar.gz

@smelc
Copy link
Contributor

smelc commented Oct 7, 2024

The bug can also be witnessed without doing step 2, thanks to conway governance action view as follows:

> cabal run cardano-cli -- conway governance action view --action-file test_update_cost_models_pparams_update.action --output-json --out-file test_update_cost_models_pparams_update.action.view
> cat test_update_cost_models_pparams_update.action.view | jq '."governance action"."contents".[1]."costModels".PlutusV3 | length'
251

cc @CarlosLopezDeLara

@smelc
Copy link
Contributor

smelc commented Oct 7, 2024

Without too much surprise, this has to do with deserialization of the cost models file. At this point here:

pure . addCostModelsToEraBasedProtocolParametersUpdate alonzoOnwards costModels $

The size of the PlutusV3 model is 251, as witnessed when debugging like this:

      Just (Cmd.CostModelsFile alonzoOnwards costModelsFile) -> do
        liftIO $ putStrLn $ "Reading cost models from: " <> unFile costModelsFile
        costModels :: L.CostModels <-
          firstExceptT GovernanceActionsCmdCostModelsError $
            readCostModels costModelsFile
        let x = Map.size $ L.costModelToMap $ snd ((Map.toList $ L.costModelsValid costModels) !! 2)
        liftIO $ putStrLn $ "PlutusV3 cost models size:" <> show x
        pure . addCostModelsToEraBasedProtocolParametersUpdate alonzoOnwards costModels $
          uppNewPParams eraBasedPParams'

@carbolymer
Copy link
Contributor

carbolymer commented Oct 7, 2024

New V3 primitves were added in plutus-1.34 which is not integrated in cardano-cli yet. cardano-api-9.3, used by cardano-cli-9.4.1 uses plutus-core-1.32.

Ledger uses that ParamName type to determine the number of parameters: https://github.com/IntersectMBO/cardano-ledger/blob/master/libs/cardano-ledger-core/src/Cardano/Ledger/Plutus/CostModels.hs#L208

It's a ledger design decision to silently truncate excess parameters from cost model map, so that's why this issue is so confusing.

@carbolymer
Copy link
Contributor

carbolymer commented Oct 7, 2024

We should implement cost model input validation:

@CarlosLopezDeLara
Copy link
Contributor

Resolved by #949

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants