search

tweag / cooked-validators

MIT License
39 stars 11 forks source link

Add the `TxOutRef` to `TxSkelRedeemerForReferencedScript` #330

Closed carlhammann closed 1 year ago

carlhammann commented 1 year ago

This PR changes the API for reference scripts, so that 

TxSkelRedeemerForReferencedScript :: SpendsScriptConstrs redeemer => Pl.TxOutRef -> redeemer -> TxSkelRedeemer

where the first argument of that constructor is the UTxO where the reference script is stored.

Contrarily to what I wrote on the PR that brought reference scripts, I'm now of the opinion that the benefit that one will never be able to forget the reference script outweighs the confusion that might be caused by the fact that there are now two places on the TxSkel where reference inputs might be added (namely the redeemers and the txSkelInsReference).

This also allows for even more informative errors at transaction generation time, which I also test.

carlhammann commented 1 year ago

If you add a reference input that contains a certain script and use TxSkelRedeemer to spend an input from the same script, you'll still run into the ExtraneousScriptWitnessesUTXOW error from the ledger. Even if @gabrielhdt's encountering this error sparked the discussion that led to this PR, I'm not certain any more that we should prevent this scenario from happening at transaction generation time: What do you think?

gabrielhdt commented 1 year ago

I don't understand why the txSkelInsReference is still needed, don't you have all the needed references in the arguments of the constructors that are in the txSkelIns map?

carlhammann commented 1 year ago

I don't understand why the txSkelInsReference is still needed, don't you have all the needed references in the arguments of the constructors that are in the txSkelIns map?

You might still want to use reference inputs, even if you're nor using the scripts stored in the "reference script" field of these inputs. For example, you might want to reference an input just to look at the datum it contains.

mmontin commented 1 year ago

@carlhammann I think we should bump this. Can you handle the conflicts so I can review and we go on with the merging process ?

carlhammann commented 1 year ago

@mmontin This branch is now up to date with main. You can review :smiley: