Improve `Pays` API

[florentc]

This adds the following constructors which were missing, and use them in the test suite.

• paysPKDatum
• paysPKInlineDatum
• paysPKDatumHash
• paysPKDatumWithReferenceScript
• paysPKInlineDatumWithReferenceScript
• paysPKDatumHashWithReferenceScript
• paysScriptWithReferenceScript
• paysScriptInlineDatumWithReferenceScript
• paysScriptDatumHashWithReferenceScript

[florentc]

Not going to argue on this, I had the exact same feeling while adding those :+1:

We could indeed only keep paysPK, paysPKDatum, paysScript, and introduce modifiers inlined and hashed that would pattern match on the TxSkelOutDatum (and do nothing if it is NoOutputDatum), and withReferenceScript and withStakingCredential which would override the associated fields in ConcreteOutput. Note this would imply removing the ...WithReferenceScript, ...InlineDatum, and ...DatumHash that already exist (which is fine and desirable IMO).

[carlhammann]

We could indeed only keep paysPK, paysPKDatum, paysScript, and introduce modifiers

I like that idea a lot!

[florentc]

I implemented the modifiers. Feel free to tinker with the names and ordering of parameters. For example withReferenceScript and withStakingCredential are intended to be used in their infix form: paysPK alice (lovelaceValueOf 8_000_000) `withReferenceScript` script. withDatumHash and withInlineDatum are not postfixed though, so it may be weird when we start to combine everything.

On another topic, I had to add a few constraints to the Pays constructors to be able to implement the modifiers.

[carlhammann]

I'm beginning to really like it. What would you think of making it completely regular, such that paysScript and paysPK both only receive the recipient and a value, and

withDatum, withDatumHash, withInlineDatum :: TxSkelOutDatumConstrs datum => TxSkelOut -> datum -> TxSkelOut

in analogy with withReferenceScript and withStakingCredential? The drawback would be that we lose the type safety of the validator-datum pair afforded by our current smart constructors. In the light of attacks like the one tried here, we might sometimes even like to break that correspondence, though.

Maybe we can do this, while also keeping three smart constructors for scripts as part of the API, an document withDatum and friends saying that their use is mostly for when you

• want to pay a datum to a private key, or
• pay a datum of the wrong type to a script.

[florentc]

I agree that a withDatum*** datum set of modifiers would nicely complement the toolset in case we want to go off road with unexpected datums (and it makes for a nice coherent API).

However, 99% of the time, we do want the types to fit (validator and datum, be it hashed, resolved hash, or inlined), and I think our API should take that into account. We should keep ways to specify "pays script" (for all three datum storage policies) that check the types match, and make it the obvious default regular way to go. Smart contracts are sometimes a bit convoluted, and I think we don't want to lose time being puzzled by weird onchain failures at runtime before realizing we swapped two datums by mistake.

I think going off-road with the datum types should definitely be a feature, but maybe not first class citizen. Something you switch on on purpose, in a visible way.

[carlhammann]

99% of the time, we do want the types to fit (validator and datum, be it hashed, resolved hash, or inlined), and I think our API should take that into account.

I completely agree. I propose we

• keep the three smart constructors paysScript*** we have at the moment,
• have only one paysPK constructor, and
• have the withDatum*** modifiers for when you want to
  • pay a datum to a PK, or
  • pay a "wrong" datum to a script.

[florentc]

Agreed and implemented. I also introduced paysScriptNoDatum to be used with the type unsafe withDatum family of modifiers.