Plutonomicon / cardano-transaction-lib

A Purescript library for building smart contract transactions on Cardano
https://plutonomicon.github.io/cardano-transaction-lib/
MIT License
92 stars 50 forks source link

Improve applyArgs error message #1319

Open Luis-omega opened 1 year ago

Luis-omega commented 1 year ago

It would be useful for users to know the arguments that make applyArgs to fail.

_Originally posted by @Luis-omega in https://github.com/Plutonomicon/cardano-transaction-lib/pull/1260#discussion_r1042942466_

zmrocze commented 1 year ago

i wanted to add some description here:

Applying arguments doesn't evaluate the script! It wraps the script in few Apply constructors and thats it (haskell implementation the same and damn was I surprised). Also scripts are untyped. So there's no notion of script evaluation failing when applied to parameters.

So what are the possible errors of "applyArgs"? Various serialization errors. Both script and param data makes a roundtrip:

Purescript values --csl-serialization--> bytestrings in javascript --pallas-deserialization--> rust values --pallas-serialization--> js/purescript script

For params we start with PlutusData so any error in the roundtrip concerning params would be a ctl bug. With scripts we don't have an ADT representation, scripts are newtypes on bytestrings for us. So user may import an invalid script from a text envelope and then applying params would fail at some step (in "pallas-deserialization" i think).

zmrocze commented 1 year ago

What do you think about doing a sanity check serialization roundtrip before wraping script bytestrings in PlutusScript constructor? Then we would have the assertion on PlutusScript values being valid scripts. We could then make the type of "applyArgs" be PlutusScript -> [PlutusData] -> PlutusScript as every error would be an actual bug?

klntsky commented 1 year ago

Applying arguments doesn't evaluate the script! It wraps the script in few Apply constructors and thats it

@zmrocze does the above mean we don't actually need to have any understanding of UPLC to perform parameter application? Maybe we could just apply script arguments on a binary level - like, we know the byte representation of Apply, right? And the rest is probably just byte array concatenation, isn't it?

If so, would it be feasible to drop the wasm dependency altogether and simply go with some ByteArray manipulations?

klntsky commented 1 year ago

@zmrocze

What do you think about doing a sanity check serialization roundtrip before wraping script bytestrings in PlutusScript constructor?

I always considered the fact that CTL is UPLC-blind a good thing, but having one more sanity check is even more good thing if the code is here anyway.

However, I am concerned with Plutus versions compatibility. To what extend does the uplc package validate the script? What happens if there is a new version of plutus with a new supported opcode and the uplc crate hasn't been updated? How can we test that the set of scripts acceptable by the node version we use is the same as the one we can expect uplc to parse?

zmrocze commented 1 year ago

@klntsky

  1. yes, it looks like we can do this on bytearrays directly.

  2. uplc would fail deserialisation if its not updated to a new plutus version. This could maybe be tested based on node queried plutus cost model, which lists more or less the builtins. This wouldn't be a full test tho, adding completely new term constructors in PlutusV17 or changing the serialization would pass.