Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CIP-0069? | Plutus Script Type Uniformization #321

Merged
merged 6 commits into from
Jun 6, 2023

Conversation

zygomeb
Copy link
Contributor

@zygomeb zygomeb commented Aug 23, 2022

The aim of the CIP is to allow more secure designs, streamline the developer experience and increase the design space of Cardano validators.

All three purposes are achieved by removing the Datum out of a spending script's signature, to be the exact same as every other type of script. Datum is made available in ScriptPurpose instead.

This change lets us use one script as both a spending and a minting validator, meaning that we resolve a well known protocol design problem of 'mutually dependent' validators, where a minting validator needs as parameter a spending validator, and the spending validator needs as parameter the minting one.
With this CIP, they can be merged into a single script, enabling us to enable the two-way (or more) dependency where necessary.


see rendered Markdown

Copy link
Contributor

@L-as L-as left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great

CIP-0069/README.md Outdated Show resolved Hide resolved
CIP-0069/README.md Outdated Show resolved Hide resolved
CIP-0069/README.md Outdated Show resolved Hide resolved
CIP-0069/README.md Outdated Show resolved Hide resolved
CIP-0069/README.md Outdated Show resolved Hide resolved
CIP-0069/README.md Show resolved Hide resolved
CIP-0069/README.md Outdated Show resolved Hide resolved
@michaelpj
Copy link
Contributor

michaelpj commented Sep 17, 2022

I don't hate this, it's the most plausible variant of this proposal that I've seen. I think the fundamental idea of using a sum-type to carry the arguments to the script (whatever they are) is sound. However, I don't agree with putting this information in ScriptPurpose. ScriptPurpose serves to identify a script execution, that's why it's used (for example) in the redeemers map as a key. The datum is not needed for that, so adding it is redundant and will lead to including expensive extra data in various places.

So I think we would need a new type, say ScriptArgs. Then we could have something like:

data ScriptArgs = 
  RedeemerOnly Redeemer
  RedeemerAndDatum Redeemer Datum

type ScriptType = ScriptArgs -> ScriptContext -> ()

or even (un-tupling ScriptContext)

type ScriptType = ScriptPurpose -> ScriptArgs -> TxInfo -> ()

That leaves an ugly unchecked dependence between ScriptPurpose and ScriptArgs, since only certain combinations of those are possible. But I think that's still nicer than pushing the ScriptArgs information into ScriptPurpose.

Having a separate ScriptArgs type also opens up the possibility of having scripts that take other kinds of argument in future. For example, we could easily support scripts that don't even take a redeemer.


Regarding the motivation for this proposal, I'm not 100% convinced that the desire to use the very same script in multiple contexts is compelling enough on its own. But I think something like what I sketched above has other advantages that make it compelling, in particular making us more flexible in how we pass arguments in future.

@WhatisRT
Copy link
Contributor

I'm not sure this would actually require a change to the ledger or Plutus. If you think of it from the perspective of Plutus core I'm sure you would be able to write a script that works whether there's a datum present or not, and I think you should also be able to properly look at all the arguments (but maybe my understanding of how Plutus core is executed is wrong here). If that's the case, then this could be done with any changes in the node (and thus no HF).

@michaelpj
Copy link
Contributor

I'm not sure this would actually require a change to the ledger or Plutus. If you think of it from the perspective of Plutus core I'm sure you would be able to write a script that works whether there's a datum present or not

It's pretty awkward, since the datum comes first. You don't get the thing that tells you what kind of script you are until last.

@mchakravarty
Copy link

@michaelpj I totally agree with that you are writing and also think that having something like ScriptArgs is the better approach.

@polinavino
Copy link

I agree in general with ScriptArgs being a good way to go about having a type that allows for passing different sets of arguments, but a nice way of doing this would probably require some kind of HF-type changes. So, if this is happening, better to plan to wrap it up with an upcoming HF - I figure?

CIP-0069/README.md Outdated Show resolved Hide resolved
CIP-0069/README.md Outdated Show resolved Hide resolved
CIP-0069/README.md Outdated Show resolved Hide resolved
@KtorZ
Copy link
Member

KtorZ commented Sep 27, 2022

I think I failed to understand the motivation behind this proposal. The abstract mentions the problem of mutually dependent scripts and then focuses on the datum (?). However, spending scripts are parameterized by datums, which only are populated at runtime. Hence, a datum itself does not change a script (or its corresponding hash). In case a policy needs to specifically access a datum, it can embed it in its code, and a spending script can also refer to the policy without issue (for the datum is external to the spending script).

The only case where this doesn't play out well is when the datum isn't known upfront. But I don't see how this proposal addresses this shortcoming as well. Am I missing something?

@KtorZ KtorZ changed the title CIP-0069? | Script Signature Unification CIP-0069? | Plutus Script Type Uniformization Sep 27, 2022
@KtorZ
Copy link
Member

KtorZ commented Sep 27, 2022

  • After discussion during the CIP editors meeting, @L-as kindly clarified the motivation behind the CIP -- which I thereby suggests to make more apparent: the ability to use a script as both a minting policy and a spending condition; effectively pattern-matching on the script purpose to distinguish it.
  • I am also suggesting a title change: 'Plutus Script Type Uniformization' since the word 'Signature' is a bit overloaded in our space.
  • Overall, I'd suggest to also rewrite some parts of the proposal from the point of view of it being adopted; and not using conditional tone or addressing reviewers. Think about developers reading the proposals years from now.

@L-as
Copy link
Contributor

L-as commented Sep 27, 2022

@michaelpj

So I think we would need a new type, say ScriptArgs. Then we could have something like:

I think it's not great to represent the same choice multiple times.
Rather than type ScriptType = ScriptPurpose -> ScriptArgs -> TxInfo -> (), it seems more sensible to me to take what's inside ScriptPurpose right now and put it into the hypothetical ScriptArgs, essentially, renaming ScriptPurpose to ScriptArgs and including the Datum in the Spending constructor. This would be cleaner IMO.

Then

data ScriptArgs
  = Minting CurrencySymbol
  | Spending BuiltinData TxOutRef
  | Rewarding StakingCredential
  | Certifying DCert

type ScriptType = ScriptArgs -> TxInfo -> exists a. a

@michaelpj
Copy link
Contributor

@L-as I discussed that earlier:

However, I don't agree with putting this information in ScriptPurpose. ScriptPurpose serves to identify a script execution, that's why it's used (for example) in the redeemers map as a key. The datum is not needed for that, so adding it is redundant and will lead to including expensive extra data in various places.

We need ScriptPurpose to be just ScriptPurpose. I guess we could have a ScriptArgs that partially duplicates the information in ScriptPurpose, but that feels quite ugly.

@L-as
Copy link
Contributor

L-as commented Sep 27, 2022

@michaelpj Thanks, that makes sense. If we think in terms of efficiency, just putting ScriptPurpose first is also a viable solution, but means we can't type a unified minting policy and validator in PLC unless we introduce pi types or equality types/GADTs.

@michaelpj
Copy link
Contributor

If we think in terms of efficiency, just putting ScriptPurpose first is also a viable solution, but means we can't type a unified minting policy and validator in PLC unless we introduce pi types or equality types/GADTs.

I'm not sure why you say that. The version in my comment would work fine, I think. It's not as precisely typed as we could do with fancier types, but it's expressive enough that you could use the same script in all contexts.

@zygomeb
Copy link
Contributor Author

zygomeb commented Sep 29, 2022

Thank you all for your constructive and kind responses. I'll reword the proposal to make the tone match the expectation, and provide more context if it is still needed.

@michaelpj would it make more sense to instead of introducing the unchecked dependency with these types

data ScriptArgs = 
  RedeemerOnly Redeemer
  RedeemerAndDatum Redeemer Datum

type ScriptType = ScriptArgs -> ScriptContext -> ()

and instead go and push further for unified types and make it

data ScriptArgs = 
  RedeemerAndDatum Redeemer Datum

type ScriptType = ScriptArgs -> ScriptContext -> ()

What this does is feed scripts that did not require prior arguments of Datum, a datum that is equal to (). This could be a better API and if this were to make it in say PlutusV3 we'd be able to discuss if it were possible to make use of this parameter for purposes more general than just a datum. In spirit making it a unification in signature proper and while there is a one way dependency (i.e. if datum is not a unit then it cannot be Minting, Certifying or Withdrawing) it is much better, I believe, than having as much duplication as we do with the original datatype.

An alternative route here to unified API with less dependencies which I describe in my (optional) paragraph is to move the datum into the ScriptPurpose which may be even more elegant. It would make the proposed types

data ScriptArgs = 
  RedeemerOnly Redeemer
  
type ScriptType = ScriptArgs -> ScriptContext -> ()

As the Datum would be provided in script purpose. No dependencies at all.

@michaelpj
Copy link
Contributor

What this does is feed scripts that did not require prior arguments of Datum, a datum that is equal to ()

This is basically isomorphic to what I suggested except that you can't distinguish the case where you're not supposed to have a datum, and adds a special case for the datum that you give to things that don't have one. I don't find this simpler than my proposal, which just represents the two cases with two cases. I don't think it introduces any more duplication, just a sum type.

An alternative route here to unified API with less dependencies which I describe in my (optional) paragraph is to move the datum into the ScriptPurpose which may be even more elegant.

As I said earlier, I don't like this: #321 (comment)

@zygomeb
Copy link
Contributor Author

zygomeb commented Oct 25, 2022

I've changed the proposal to be aligned with the suggestions of @michaelpj and incorporated the typo and style fixes as suggested

CIP-0069/README.md Outdated Show resolved Hide resolved
CIP-0069/README.md Outdated Show resolved Hide resolved

## Path to Active

The appropriate changes need to be made to the node code and a new language major version be introduced.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section needs to contain an implementation plan approved by the Plutus team. Once this is covered, we can proceed and merge it as Proposed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss this in the CIP editors meeting

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @zygomeb the next meeting, after having been delayed 1 week, has now been postponed 1 additional day (CIP Editors Meeting #58 (30 Nov, 9:30am UTC))


## Specification

### Removing the datum argument
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not yet a sufficient specification. The ledger does not know about any of these types. The redeemer and datum are passed as values of type Data. So: what argument exactly is going to be passed instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure exactly what is the answer here. Would you have any suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to look at it from the persective of cardano-ledger

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not plutus-ledger-api (which doesn't do anything)

@michaelpj
Copy link
Contributor

An objection that occurs to me. This means that it is no longer as simple to run a script as it used to be: you can't just apply it to the redeemer and the datum, you now need to construct an additional structure. (Indeed, writing this I realised that I don't even know what that structure is, hence my comment in the text). I don't think this is a killer argument: it's already non-trivial to run a script since you need the ScriptContext, which really only the ledger can produce easily. But it's worth pointing out as a consequence.

I think this is broadly a good idea, but it's a big change to the interface and I'm a bit nervous that there are problems I haven't thought of. So I'd probably like to think about it more before I'm wholly in favour.

I also don't think the Plutus team will implement this any time soon - it doesn't seem high enough priority.

@L-as
Copy link
Contributor

L-as commented Nov 12, 2022

to run "a script"

Isn't it the other way around? Before you had to special-case validators.

I also don't think the Plutus team will implement this any time soon

Maybe MLabs can take this on? I imagine it will only need changes to cardano-ledger's Alonzo, Babbage eras and also plutus-ledger-api? Seems simple.

@fallen-icarus
Copy link

@nielstron It's cool that you found a way to make it work but

The single only drawback of this approach is that the second argument to a validator may never be PlutusData with constructor id 0

this is a pretty hefty cost IMO. Currently most tooling does not help a developer avoid using the constructor id 0 which means it now falls on the developer to remember. Indeed, most tooling encourages using the 0 constructor. This makes it another potential place where normal human error will cause issues. The hack seems to be a "chicken or egg" situation: the cost is only "bearable" if the tooling is already there to guide developers into using it correctly.

It can also nicely be emulated using the hack described above.

I'm not sure how you handle plutus versions with eopsin but if the Maybe Datum proposal is implemented, AFAICT the hack wouldn't be necessary at all for plutus v3 scripts since all plutus v3 scripts would have the same signature.

@nielstron
Copy link
Contributor

nielstron commented Jan 24, 2023

@fallen-icarus Eopsin guides you through this. And sensible tooling that doesn't disclose the contructor ID to the developer (which should be the case for very clean tools) can simply start counting at 1 when assigning ids. Naturally, this workaround is part of the tooling, not of the written code and should be handled by the tooling either way.

Yes for plutus v3 its not necessary to do this workaround. But one could already start developing with the maybe datum schema in eopsin and PlutusV2 using the hack. the tool will then transparently upgrade to PlutusV3 and removes the hack from the compilation chain when the new plutus version is availalable.


Unifying of the script signature is a very elegant solution to the problem, streamlining the experience of developing on cardano.
Given that accessing the datum is almost always made by a spending script, it makes sense to introduce that argument back to the `ScriptPurpose` that now plays a more important role.
It begs the question if it should be added as an argument to all validators, to further emphasize that fact.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found these last two sentences difficult to understand.

@michaelpj
Copy link
Contributor

I still want to do something like this.

Problems:

  • This is still lacking a proper specification. But I think that will work well with Sums-of-products mk II IntersectMBO/plutus#5099, because then we can use native Plutus Core sums-of-products to encode the ScriptArgs argument, instead of relying on Data.
  • There are no acceptance criteria. I propose: a) plutus and cardano-ledger have been updated with the new design, b) the new design has been incorporated into a new ledger language
  • There is no implementation plan. Maybe the Plutus team might do it, it depends on how much time we have before the next HF and Plutus ledger language.
  • I am still generically worried about the design. I am sorry that this is so vague, but these sorts of design decision are very sticky so I would really like to be confident that we're moving to somewhere good. The existing design is a clear point in the space (about as simple as you can get), if we make things more complex we have far more options.
    • For example, the question of whether the ScriptPurpose should be part of the ScriptContext argument or its own argument.

Nonetheless, I think we should merge this CIP as Proposed. Ideally after the above have been addressed (except for the last one).

@L-as
Copy link
Contributor

L-as commented Feb 6, 2023

Seems to me like scripts should all take only one argument, which is a SOP. One index/constructor per script type, which includes per-script data and info:

data ScriptInfo = Minting Redeemer CurrencySymbol TxInfo | Spending Redeemer Datum TxOutRef TxInfo | ...

type Script = ScriptInfo -> Any

This seems simpler than the current design while being more powerful IMO.

Thoughts? @michaelpj

@michaelpj
Copy link
Contributor

That's another possibility. This is what I mean when I say the design space is bigger now...

Note that I now think that having a separate ScriptPurpose may actually be desirable: that allows us to, say, run a spending script with no datum by running script (RedeemerOnly redeemer) (Spending txOutRef) txInfo. Your version can't do that because we have to link the script purpose to the expected arguments. There are quite a few desires to be satisfied here!

However, we could still have a single SOP for all arguments, and just have it have a ScriptPurpose field.

The main advantage I can see to your proposal is that it would make it easier to add new call patterns in future that change how the script context is passed, e.g. not including the TxInfo. I'm not sure if that's desirable or not...

@imikushin

This comment was marked as duplicate.

@michaelpj
Copy link
Contributor

Please don't cross-post like this, now I don't know where to respond. I'm going to respond in the other CIP where you brought it up.

@KtorZ KtorZ added Category: Plutus Proposals belonging to the 'Plutus' category. and removed Candidate CIP labels Mar 18, 2023
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zygomeb @michaelpj @L-as @KtorZ - this was reviewed in the CIP meeting today which had no Plutus representatives present. Present editors' feeling based on this proposal's age and acknowledgement as at least a working alternative (via comments above) is that should be merged as Proposed pending Last Check... or deprecated in the equally near future if any irreconcilable problems.

@rphair rphair added the State: Last Check Review favourable with disputes resolved; staged for merging. label May 16, 2023
@L-as
Copy link
Contributor

L-as commented May 18, 2023

Thank you!

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last changes for CIP-0001 compliance, plus the round of agreement at the last meeting, I think make this ready to merge at the meeting coming up in a few minutes. In fact @KtorZ @Ryun1 @SebastienGllmt I think we should treat this as a Last Check item on the agenda and be ready to merge it on the spot unless there are any objections.

@KtorZ
Copy link
Member

KtorZ commented Jun 6, 2023

I missed the last editor meeting so I'll try to catch up in the coming one but .... it seems to me that the CIP is far from being in a "proposable" state? As far as I can tell, MPJ had valid comments regarding how the specification isn't detailed enough.

While the motivation is clear, I don't think the proposed solution is sufficiently specified?

Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the following comment: #321 (comment)

I am in the end okay merging this CIP for the sake of recording a proposed solution to #497. The specification is still incomplete and insufficient for someone to implement this, and there's clearly no implementer. but it has the merit of being a well-formulated idea and motivation to a known problem.

@rphair rphair merged commit ece5ee5 into cardano-foundation:master Jun 6, 2023
@rphair rphair removed the State: Last Check Review favourable with disputes resolved; staged for merging. label Jun 6, 2023
Ryun1 pushed a commit to Ryun1/CIPs that referenced this pull request Jul 28, 2023
* 0069 first version

* incorporate typo and language fixes, remove optional section

* incorporare discussion changes, some typos

* Update CIP-0069/README.md

Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>

* Update CIP-0069/README.md

Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>

* Update CIP-0069 to comply with latest CIP-0001

---------

Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>
Co-authored-by: KtorZ <matthias.benkort@gmail.com>
Ryun1 pushed a commit to Ryun1/CIPs that referenced this pull request Nov 17, 2023
* 0069 first version

* incorporate typo and language fixes, remove optional section

* incorporare discussion changes, some typos

* Update CIP-0069/README.md

Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>

* Update CIP-0069/README.md

Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>

* Update CIP-0069 to comply with latest CIP-0001

---------

Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>
Co-authored-by: KtorZ <matthias.benkort@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Plutus Proposals belonging to the 'Plutus' category.
Projects
None yet
Development

Successfully merging this pull request may close these issues.