IntersectMBO/cardano-ledger

Can we deprecate `scriptDataHash` please?

Closed this issue · 2 comments

From the transaction building point of view, there is little value in having a scriptDataHash field

Both redeemers and datums in the witness set could be moved in the transaction body.

this would have the added benefit of script contexts being only dependent on the transaction body.

language views are redundant.

in the event one or more scripts are evaluated with the wrong cost models only 2 scenarios can happen:

  1. costs are lower: implies that one or more redeemers will not have enough execution units for the respective script execution, making the transaction invalid

  2. costs are higher: implies that an higher fee must be paid to cover for the additional execution units.

Additionally, with Conway allowing for the community to change parameters, costs may be constantly subject to change, possibly resulting in the cryptic error PPViewHashesDontMatch with little more information on how the intended hash should be calculated.

a possible solution would then be to modify tx_body and tx_wintess_set as follows

transaction_body =
  { 0 : set<transaction_input>             ; inputs
  , 1 : [* transaction_output]
  , 2 : coin                               ; fee
  , ? 3 : slot_no                          ; time to live
  , ? 4 : certificates
  , ? 5 : withdrawals
  , ? 7 : auxiliary_data_hash
  , ? 8 : slot_no                          ; validity interval start
  , ? 9 : mint
  , ? 13 : nonempty_set<transaction_input> ; collateral inputs
  , ? 14 : required_signers
  , ? 15 : network_id
  , ? 16 : transaction_output              ; collateral return
  , ? 17 : coin                            ; total collateral
  , ? 18 : nonempty_set<transaction_input> ; reference inputs
  , ? 19 : voting_procedures               ; New; Voting procedures
  , ? 20 : proposal_procedures             ; New; Proposal procedures
  , ? 21 : coin                            ; New; current treasury value
  , ? 22 : positive_coin                   ; New; donation
  , ? 23: nonempty_set<plutus_data>
  , ? 24: redeemers
  }

transaction_witness_set =
  { ? 0: nonempty_set<vkeywitness>
  , ? 1: nonempty_set<native_script>
  , ? 2: nonempty_set<bootstrap_witness>
  , ? 3: nonempty_set<plutus_v1_script>
  , ? 6: nonempty_set<plutus_v2_script>
  , ? 7: nonempty_set<plutus_v3_script>
  }

I totally appreciate your interest in making Cardano better. However, this kind of drastic changes must go through a proper CIP process. So, if you feel like there are clear benefits to making such changes, by all means please submit a CIP and if there is an agreement between the community members, researchers and developers about the proposed solution, then the Ledger team be happy to implement such a change in the next era.

With respect to redeemers and datums, I am not quite sure why they were placed into the TxWits instead of TxBody, but I can tell you with confidence that it would be a pretty big breaking change moving them from TxWitnesses into the TxBody that would have to be done over multiple years, while supporting both locations for at least a couple of eras. I personally don't think there is enough benefit in doing a change like that especially upon consideration of all of the costs associated with it. That being said, I might be completely wrong about it. That is exactly why CIP process and communication with all of the relevant parties is important.

My personal take on it: we cannot deprecate scriptIntegrityHash, because there is actually a reason why it exists in the first place, namely: language view is not redundant. One of the core restrictions on the script execution is that it cannot depend on anything in the ledger state. If we remove the language view it will turn deterministic phase-1 validation failure into a non-deterministic phase-2 validation failure that depends on the CostModel in the ledger state.

So, these assumptions are invalid:

language views are redundant.

As described above

costs are lower: implies that one or more redeemers will not have enough execution units for the respective script execution, making the transaction invalid

It will make transaction phase-2 invalid, which will still result in it being placed on chain and collateral collected, except the user that constructed and submitted transaction would not have expected this to happen.

Additionally, with Conway allowing for the community to change parameters, costs may be constantly subject to change

It is not as lax as it sounds in reality. There are a lot of guardians in place before parameters can change. In case of CostModels:

  • There is a guardrails script that prevents dangerous values
  • Proposal would need to provide concrete and reproducible proof why these new values are safe and accurate
  • Then vast majority of the community will need to vote in favor of such parameter change, at least initially it will be:
    • 67% of DRep stake will have to vote in favor on such proposal
    • as well as 66% of CC members.
      In other words we should not be expecting the CostModels to change all that frequently.

I am closing this as "won't fix", but, as I said earlier, feel free to move this discussion to the CIPs repo

@lehins I find the points raised compelling, I will not open a CIP given the impact

However something has to be done regarding PPViewHashesDontMatch

Every time the error is thrown it takes an unnecessary amount of resources to fix properly

Resources that could greatly be reduced if at least in the local tx submission the error could report the expected input data to obtain the intended hash