IntersectMBO / cardano-cli

This repository contains sources for the command-line interface (CLI) tool for interacting with the Cardano blockchain.
Apache License 2.0
39 stars 15 forks source link

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

Open mkoura opened 5 days ago

mkoura commented 5 days ago

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

1.

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 2. 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 commented 2 days ago

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 commented 2 days ago

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

https://github.com/IntersectMBO/cardano-cli/blob/31212c8bf6512b839bdeaf6f7a3784e5fb46f9ea/cardano-cli/src/Cardano/CLI/EraBased/Run/Governance/Actions.hs#L391

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 commented 2 days ago

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 commented 2 days ago

We should implement cost model input validation: